diff --git a/src/main/java/com/scmspain/controller/TweetController.java b/src/main/java/com/scmspain/controller/TweetController.java index 55ce7cd..bebfac6 100644 --- a/src/main/java/com/scmspain/controller/TweetController.java +++ b/src/main/java/com/scmspain/controller/TweetController.java @@ -1,5 +1,6 @@ package com.scmspain.controller; +import com.scmspain.controller.command.DiscardTweetCommand; import com.scmspain.controller.command.PublishTweetCommand; import com.scmspain.entities.Tweet; import com.scmspain.services.TweetService; @@ -23,10 +24,23 @@ public List listAllTweets() { return this.tweetService.listAllTweets(); } + @GetMapping("/discarded") + public List listDiscardedTweets() { + return this.tweetService.listDiscardedTweets(); + } + + @PostMapping("/tweet") @ResponseStatus(CREATED) - public void publishTweet(@RequestBody PublishTweetCommand publishTweetCommand) { - this.tweetService.publishTweet(publishTweetCommand.getPublisher(), publishTweetCommand.getTweet()); + public Tweet publishTweet(@RequestBody PublishTweetCommand publishTweetCommand) { + return this.tweetService.publishTweet(publishTweetCommand.getPublisher(), publishTweetCommand.getTweet()); + } + + //I would name this /tweet but with PUT, because it is basically an update. + @PostMapping("/discarded") + @ResponseStatus(CREATED) + public Tweet discardTweet(@RequestBody DiscardTweetCommand discardTweetCommand) { + return this.tweetService.discardTweet(discardTweetCommand.getTweet()); } @ExceptionHandler(IllegalArgumentException.class) diff --git a/src/main/java/com/scmspain/controller/command/DiscardTweetCommand.java b/src/main/java/com/scmspain/controller/command/DiscardTweetCommand.java new file mode 100644 index 0000000..7ed0a4a --- /dev/null +++ b/src/main/java/com/scmspain/controller/command/DiscardTweetCommand.java @@ -0,0 +1,19 @@ +package com.scmspain.controller.command; + +/** + * Created by hespoz on 3/28/18. + */ +public class DiscardTweetCommand { + + private String tweet; + + public String getTweet() { + return tweet; + } + + public void setTweet(String tweet) { + this.tweet = tweet; + } + +} + diff --git a/src/main/java/com/scmspain/entities/Tweet.java b/src/main/java/com/scmspain/entities/Tweet.java index 3616a94..e5a3034 100644 --- a/src/main/java/com/scmspain/entities/Tweet.java +++ b/src/main/java/com/scmspain/entities/Tweet.java @@ -7,12 +7,16 @@ @Entity public class Tweet { + @Id @GeneratedValue private Long id; @Column(nullable = false) private String publisher; - @Column(nullable = false, length = 140) + + //Remove constrain because now, we can save more than 140 characters, including the urls. + @Column(nullable = false) + private String tweet; @Column (nullable=true) private Long pre2015MigrationStatus = 0L; diff --git a/src/main/java/com/scmspain/services/TweetService.java b/src/main/java/com/scmspain/services/TweetService.java index d61bc9d..20d6860 100644 --- a/src/main/java/com/scmspain/services/TweetService.java +++ b/src/main/java/com/scmspain/services/TweetService.java @@ -1,6 +1,7 @@ package com.scmspain.services; import com.scmspain.entities.Tweet; +import com.scmspain.utils.ValidateUtils; import org.springframework.boot.actuate.metrics.writer.Delta; import org.springframework.boot.actuate.metrics.writer.MetricWriter; import org.springframework.stereotype.Service; @@ -10,6 +11,7 @@ import javax.transaction.Transactional; import java.util.ArrayList; import java.util.List; +import java.util.Optional; @Service @Transactional @@ -23,31 +25,51 @@ public TweetService(EntityManager entityManager, MetricWriter metricWriter) { } /** - Push tweet to repository + Push tweet to repository - I decide to return something because before the response was empty. Parameter - publisher - creator of the Tweet Parameter - text - Content of the Tweet - Result - recovered Tweet + Result - publised Tweet */ - public void publishTweet(String publisher, String text) { - if (publisher != null && publisher.length() > 0 && text != null && text.length() > 0 && text.length() < 140) { - Tweet tweet = new Tweet(); - tweet.setTweet(text); - tweet.setPublisher(publisher); - - this.metricWriter.increment(new Delta("published-tweets", 1)); - this.entityManager.persist(tweet); - } else { + public Tweet publishTweet(String publisher, String text) { + + //We can work more in the validation methods a DTOs with a list of errors. + if(!ValidateUtils.nonEmpty(publisher)) { + throw new IllegalArgumentException("Publisher can not be null"); + } + + if(!ValidateUtils.isValidTweet(text)) { throw new IllegalArgumentException("Tweet must not be greater than 140 characters"); } + + Tweet tweet = new Tweet(); + tweet.setTweet(text); + tweet.setPublisher(publisher); + + this.metricWriter.increment(new Delta("published-tweets", 1)); + this.entityManager.persist(tweet); + + return tweet; + } /** - Recover tweet from repository - Parameter - id - id of the Tweet to retrieve - Result - retrieved Tweet - */ - public Tweet getTweet(Long id) { - return this.entityManager.find(Tweet.class, id); + Discard tweet - I decide to return something because before the response was empty. + Parameter - id - id of the Tweet to discard + Result - discarded Tweet + */ + public Tweet discardTweet(String tweetId) { + + if (!ValidateUtils.isValidTweet(tweetId)) { + throw new IllegalArgumentException("Wrong id, should be a number"); + } + + Tweet tweet = Optional.ofNullable(this.entityManager.find(Tweet.class, Long.parseLong(tweetId))) + .orElseThrow(() -> new IllegalArgumentException(String.format("There is no tweet with id %s ", tweetId))); + + tweet.setPre2015MigrationStatus(99l); + this.entityManager.persist(tweet); + + return tweet; } /** @@ -58,11 +80,13 @@ public Tweet getTweet(Long id) { public List listAllTweets() { List result = new ArrayList(); this.metricWriter.increment(new Delta("times-queried-tweets", 1)); - TypedQuery query = this.entityManager.createQuery("SELECT id FROM Tweet AS tweetId WHERE pre2015MigrationStatus<>99 ORDER BY id DESC", Long.class); - List ids = query.getResultList(); - for (Long id : ids) { - result.add(getTweet(id)); - } - return result; + + //I had to refactor this, there is no reason to go grab first the list of the Ids an then query the database n times again. This is performance killers! + return this.entityManager.createQuery("SELECT tweet FROM Tweet tweet WHERE pre2015MigrationStatus<>99 ORDER BY id DESC", Tweet.class).getResultList(); } + + public List listDiscardedTweets() { + return this.entityManager.createQuery("SELECT tweet FROM Tweet tweet WHERE pre2015MigrationStatus=99 ORDER BY id DESC", Tweet.class).getResultList(); + } + } diff --git a/src/main/java/com/scmspain/utils/ValidateUtils.java b/src/main/java/com/scmspain/utils/ValidateUtils.java new file mode 100644 index 0000000..3e1c872 --- /dev/null +++ b/src/main/java/com/scmspain/utils/ValidateUtils.java @@ -0,0 +1,37 @@ +package com.scmspain.utils; + +import java.util.Optional; +import java.util.regex.Pattern; + +/** + * Created by hespoz on 3/27/18. + */ +public class ValidateUtils { + + private final static String URL_REGEX ="((http:\\/\\/|https:\\/\\/)([^\\s]+)?)"; + + public static String cleanTweet(String text) { + return Pattern.compile(URL_REGEX) + .matcher(Optional.ofNullable(text).orElse("")) + .replaceAll("").trim(); + } + + public static boolean isValidTweet(String text) { + String clearedStr = cleanTweet(text); + return clearedStr.length() > 0 && clearedStr.length() <= 140; + } + + public static boolean nonEmpty(String text) { + return text != null && text.length() > 0; + } + + public static boolean validTweetId(String id) { + try { + Long.parseLong(id); + return true; + } catch (NumberFormatException nfe) { + return false; + } + } + +} diff --git a/src/test/java/com/scmspain/controller/TweetControllerTest.java b/src/test/java/com/scmspain/controller/TweetControllerTest.java index 4368add..8b0e14a 100644 --- a/src/test/java/com/scmspain/controller/TweetControllerTest.java +++ b/src/test/java/com/scmspain/controller/TweetControllerTest.java @@ -2,12 +2,14 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.scmspain.configuration.TestConfiguration; +import com.scmspain.entities.Tweet; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.MediaType; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MvcResult; @@ -24,34 +26,68 @@ import static org.springframework.test.web.servlet.setup.MockMvcBuilders.webAppContextSetup; @RunWith(SpringRunner.class) + +/* + * I add this to restart the H2 db each time a test is executed. Other approach will be to use @BeforeAll. + * But for this we need to make the setUp method static, and in this case that will affect the autowired of WebApplicationContext. + * This is a little bit slow, but works fine! + */ +@DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD) @SpringBootTest(classes = TestConfiguration.class) public class TweetControllerTest { + + private static final String LONG_TWEET_WITH_LINK = "We are Schibsted Spain (look at our home page http://www.schibsted.es/ ), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!"; + private static final String LONG_TWEET_WITH_NO_LINK = "We are Schibsted Spain (look at our home page XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX ), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!"; + @Autowired private WebApplicationContext context; private MockMvc mockMvc; @Before - public void setUp() { + public void setUp() throws Exception { this.mockMvc = webAppContextSetup(this.context).build(); } + @Test public void shouldReturn200WhenInsertingAValidTweet() throws Exception { mockMvc.perform(newTweet("Prospect", "Breaking the law")) - .andExpect(status().is(201)); + .andExpect(status().is(201)); + } + + @Test + public void shouldReturn200WhenInsertingAnLongTweetWithUrl() throws Exception { + mockMvc.perform(newTweet("Schibsted Spain", LONG_TWEET_WITH_LINK)) + .andExpect(status().is(201)); + } + + + @Test + public void shouldReturn400WhenSentNullPublisher() throws Exception { + mockMvc.perform(newTweet(null, LONG_TWEET_WITH_LINK)) + .andExpect(status().is(400)); + } + + @Test + public void shouldReturn400WhenSentBlankPublisher() throws Exception { + mockMvc.perform(newTweet("", LONG_TWEET_WITH_LINK)) + .andExpect(status().is(400)); } @Test - public void shouldReturn400WhenInsertingAnInvalidTweet() throws Exception { - mockMvc.perform(newTweet("Schibsted Spain", "We are Schibsted Spain (look at our home page http://www.schibsted.es/), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!")) + public void shouldReturn400WhenInsertingATweetWithMoreThan140Characters() throws Exception { + mockMvc.perform(newTweet("Schibsted Spain", LONG_TWEET_WITH_NO_LINK)) .andExpect(status().is(400)); } + @Test public void shouldReturnAllPublishedTweets() throws Exception { - mockMvc.perform(newTweet("Yo", "How are you?")) + + mockMvc.perform(newTweet("Yo", "How are you, Rap?")) .andExpect(status().is(201)); + MvcResult getResult = mockMvc.perform(get("/tweet")) .andExpect(status().is(200)) .andReturn(); @@ -60,10 +96,79 @@ public void shouldReturnAllPublishedTweets() throws Exception { assertThat(new ObjectMapper().readValue(content, List.class).size()).isEqualTo(1); } + @Test + public void shouldFailDiscardedTweetNotFound() throws Exception { + mockMvc.perform(discardTweet("333342")) + .andExpect(status().is(400)); + } + + @Test + public void shouldFailDiscardedTweetNotValidTweetId() throws Exception { + mockMvc.perform(discardTweet("NotValidId")) + .andExpect(status().is(400)); + } + + + @Test + public void shouldReturnAllDiscardedTweets() throws Exception { + + mockMvc.perform(newTweet("Yo", "How are you, Ayn Rand?")) + .andExpect(status().is(201)); + + mockMvc.perform(newTweet("Yo", "How are you, Alan Pike?")) + .andExpect(status().is(201)); + + mockMvc.perform(newTweet("Yo", "How are you, Friedman?")) + .andExpect(status().is(201)); + + + MvcResult getResult = mockMvc.perform(get("/tweet")) + .andExpect(status().is(200)) + .andReturn(); + + String content = getResult.getResponse().getContentAsString(); + List tweetList = new ObjectMapper().readValue(content, List.class); + assertThat(tweetList.size()).isEqualTo(3); + + + //Discard one tweet. + mockMvc.perform(discardTweet("3")) + .andExpect(status().is(201)); + + //Check if the list of tweets now has only 2 tweets. + MvcResult getResultTweetsAfterDiscard = mockMvc.perform(get("/tweet")) + .andExpect(status().is(200)) + .andReturn(); + + assertThat(new ObjectMapper().readValue(getResultTweetsAfterDiscard.getResponse().getContentAsString(), List.class).size()).isEqualTo(2); + + + //Check if discarded tweets list has only 1 discarded tweet. + MvcResult getResultDiscardedTweets = mockMvc.perform(get("/discarded")) + .andExpect(status().is(200)) + .andReturn(); + + assertThat(new ObjectMapper().readValue(getResultDiscardedTweets.getResponse().getContentAsString(), List.class).size()).isEqualTo(1); + + + } + + private MockHttpServletRequestBuilder discardTweet(String tweetId) { + return post("/discarded") + .contentType(MediaType.APPLICATION_JSON_UTF8) + .content(format("{\"tweet\": \"%s\"}", tweetId)); + } + private MockHttpServletRequestBuilder newTweet(String publisher, String tweet) { + final String jsonString = + publisher != null ? + format("{\"publisher\": \"%s\", \"tweet\": \"%s\"}", publisher, tweet) + : + format("{\"publisher\": null, \"tweet\": \"%s\"}", tweet); + return post("/tweet") .contentType(MediaType.APPLICATION_JSON_UTF8) - .content(format("{\"publisher\": \"%s\", \"tweet\": \"%s\"}", publisher, tweet)); + .content(jsonString); } } diff --git a/src/test/java/com/scmspain/services/TweetServiceTest.java b/src/test/java/com/scmspain/services/TweetServiceTest.java index ac88fe5..ce9b429 100644 --- a/src/test/java/com/scmspain/services/TweetServiceTest.java +++ b/src/test/java/com/scmspain/services/TweetServiceTest.java @@ -11,6 +11,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + public class TweetServiceTest { private EntityManager entityManager; private MetricWriter metricWriter; @@ -35,4 +37,24 @@ public void shouldInsertANewTweet() throws Exception { public void shouldThrowAnExceptionWhenTweetLengthIsInvalid() throws Exception { tweetService.publishTweet("Pirate", "LeChuck? He's the guy that went to the Governor's for dinner and never wanted to leave. He fell for her in a big way, but she told him to drop dead. So he did. Then things really got ugly."); } + + + @Test + public void shouldDiscardTweet() throws Exception { + + final String tweetId = "1"; + + Tweet tweet = new Tweet(); + tweet.setId(1l); + tweet.setPublisher("ApaYCan"); + tweet.setTweet("Repampanos!"); + + when(entityManager.find(Tweet.class, Long.parseLong(tweetId))).thenReturn(tweet); + + tweetService.discardTweet(tweetId); + + verify(entityManager).persist(any(Tweet.class)); + + } + } diff --git a/src/test/java/com/scmspain/utils/ValidateUtilsTest.java b/src/test/java/com/scmspain/utils/ValidateUtilsTest.java new file mode 100644 index 0000000..9bcdfdc --- /dev/null +++ b/src/test/java/com/scmspain/utils/ValidateUtilsTest.java @@ -0,0 +1,82 @@ +package com.scmspain.utils; + +import static org.junit.Assert.*; + +import org.junit.Test; + +/** + * Created by hespoz on 3/27/18. + */ +public class ValidateUtilsTest { + + private static final String LONG_TWEET = "Lorem Ipsum es simplemente el texto de relleno de las imprentas y archivos de texto. Lorem Ipsum ha sido el texto de relleno estándar de las industrias desde el año 500 cuando https://www.validurl.com"; + private static final String CLEAN_LONG_TWEET = "Lorem Ipsum es simplemente el texto de relleno de las imprentas y archivos de texto. Lorem Ipsum ha sido el texto de relleno estándar de las industrias desde el año 500 cuando"; + + private static final String ORIGINAL_TWEET = "We are Schibsted Spain (look at our home page http://www.schibsted.es/ ), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!"; + private static final String CLEAN_TWEET = "We are Schibsted Spain (look at our home page ), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!"; + + private static final String LONG_TWEET_140 = "We are Schibsted Spain (look at our home page), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!gsdgsdfafwerqrqwerwer1"; + private static final String LONG_TWEET_141 = "We are Schibsted Spain (look at our home page), we own Vibbo, InfoJobs, fotocasa, coches.net and milanuncios. Welcome!gsdgsdfafwerqrqwerwer11"; + + //Tests for ValidateUtils.cleanTweet + @Test + public void shouldGetExpectedCleanString() throws Exception { + assertEquals(ValidateUtils.cleanTweet(ORIGINAL_TWEET), CLEAN_TWEET); + assertEquals(ValidateUtils.cleanTweet(LONG_TWEET), CLEAN_LONG_TWEET); + } + + @Test + public void shouldGetEmptyCleanTweet_StringIsNull() throws Exception { + assertEquals("", ValidateUtils.cleanTweet(null)); + } + + //Tests for ValidateUtils.isValidTweet + @Test + public void shouldTweetNotPassValidation_StringIsNull() throws Exception { + assertFalse(ValidateUtils.isValidTweet(null)); + } + + @Test + public void shouldTweetNotPassValidation_StringIsTooLong() throws Exception { + assertFalse(ValidateUtils.isValidTweet(LONG_TWEET)); + } + + + @Test + public void shouldTweetPassValidation() throws Exception { + assertTrue(ValidateUtils.isValidTweet(ORIGINAL_TWEET)); + assertTrue(ValidateUtils.isValidTweet(LONG_TWEET_140)); + assertFalse(ValidateUtils.isValidTweet(LONG_TWEET_141)); + } + + + //Tests for ValidateUtils.nonEmpty + @Test + public void shouldReturnFalse_TextIsNull() throws Exception { + assertFalse(ValidateUtils.nonEmpty(null)); + } + + @Test + public void shouldReturnFalse_TextIsBlank() throws Exception { + assertFalse(ValidateUtils.nonEmpty("")); + } + + @Test + public void shouldReturnTrue_TextIsValidString() throws Exception { + assertTrue(ValidateUtils.nonEmpty("user")); + } + + //Test ValidateUtils.validTweetId + + @Test + public void shouldReturnTrue_IdIsValid() throws Exception { + assertTrue(ValidateUtils.validTweetId("1")); + } + + @Test + public void shouldReturnTrue_IdIsNotValid() throws Exception { + assertFalse(ValidateUtils.validTweetId("No a valid id")); + } + + +}