From 84e57748585e67fc98d72c2a8162e56e41555d5f Mon Sep 17 00:00:00 2001 From: krzysztofwedrowicz Date: Thu, 26 Sep 2019 21:29:39 +0200 Subject: [PATCH 1/2] changed placeHold endpoint of PatronProfileController to be more restful --- .../web/PatronProfileControllerIT.java | 37 +++++++++++++++++-- .../web/PatronProfileController.java | 7 ++-- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java b/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java index 14d3cb3..49e96be 100644 --- a/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java +++ b/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java @@ -5,6 +5,7 @@ import io.pillopl.library.lending.LendingTestContext; import io.pillopl.library.lending.book.model.BookFixture; import io.pillopl.library.lending.patron.application.hold.CancelingHold; +import io.pillopl.library.lending.patron.application.hold.PlacingOnHold; import io.pillopl.library.lending.patron.model.PatronFixture; import io.pillopl.library.lending.patron.model.PatronId; import io.pillopl.library.lending.patronprofile.model.Checkout; @@ -20,6 +21,7 @@ import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.hateoas.MediaTypes; +import org.springframework.http.MediaType; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MockMvc; @@ -35,8 +37,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.springframework.http.HttpHeaders.CONTENT_TYPE; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -60,6 +61,9 @@ public class PatronProfileControllerIT { @MockBean private PatronProfiles patronProfiles; + @MockBean + private PlacingOnHold placingOnHold; + @MockBean private CancelingHold cancelingHold; @@ -160,6 +164,33 @@ public void shouldReturnResourceForCheckout() throws Exception { .andExpect(jsonPath("$._links.self.href", containsString("profiles/" + patronId.getPatronId() + "/checkouts/" + anotherBook.getBookId()))); } + @Test + public void shouldPlaceBookOnHold() throws Exception { + given(placingOnHold.placeOnHold(any())).willReturn(Try.success(Success)); + var request = "{\"bookId\":\"6e1dfec5-5cfe-487e-814e-d70114f5396e\", \"libraryBranchId\":\"a518e2ef-5f6c-43e3-a7fc-5d895e15be3a\",\"numberOfDays\":1}"; + + // expect + mvc.perform(post("/profiles/" + patronId.getPatronId() + "/holds") + .accept(MediaTypes.HAL_FORMS_JSON_VALUE) + .contentType(MediaType.APPLICATION_JSON) + .content(request)) + .andExpect(status().isOk()); + } + + @Test + public void shouldReturn500IfSomethingFailedWhileDuringPlacingOnHold() throws Exception { + given(placingOnHold.placeOnHold(any())).willReturn(Try.failure(new IllegalArgumentException())); + var request = "{\"bookId\":\"6e1dfec5-5cfe-487e-814e-d70114f5396e\", \"libraryBranchId\":\"a518e2ef-5f6c-43e3-a7fc-5d895e15be3a\",\"numberOfDays\":1}"; + + + // expect + mvc.perform(post("/profiles/" + patronId.getPatronId() + "/holds") + .accept(MediaTypes.HAL_FORMS_JSON_VALUE) + .contentType(MediaType.APPLICATION_JSON) + .content(request)) + .andExpect(status().isInternalServerError()); + } + @Test public void shouldCancelExistingHold() throws Exception { given(patronProfiles.fetchFor(patronId)).willReturn(profiles()); @@ -199,4 +230,4 @@ PatronProfile profiles() { new HoldsView(of(new Hold(bookId, anyDate))), new CheckoutsView(of(new Checkout(anotherBook, anotherDate)))); } -} \ No newline at end of file +} diff --git a/src/main/java/io/pillopl/library/lending/patronprofile/web/PatronProfileController.java b/src/main/java/io/pillopl/library/lending/patronprofile/web/PatronProfileController.java index 438ed22..58e9da9 100644 --- a/src/main/java/io/pillopl/library/lending/patronprofile/web/PatronProfileController.java +++ b/src/main/java/io/pillopl/library/lending/patronprofile/web/PatronProfileController.java @@ -97,12 +97,12 @@ ResponseEntity> findCheckout(@PathVariable UUID patronId, .getOrElse(notFound().build()); } - @PostMapping("/holds") - ResponseEntity placeHold(@RequestBody PlaceHoldRequest request) { + @PostMapping("/profiles/{patronId}/holds") + ResponseEntity placeHold(@PathVariable UUID patronId, @RequestBody PlaceHoldRequest request) { Try result = placingOnHold.placeOnHold( new PlaceOnHoldCommand( Instant.now(), - new PatronId(request.getPatronId()), + new PatronId(patronId), new LibraryBranchId(request.getLibraryBranchId()), new BookId(request.getBookId()), Option.of(request.getNumberOfDays()) @@ -183,7 +183,6 @@ class Checkout { @AllArgsConstructor(onConstructor = @__(@JsonCreator)) class PlaceHoldRequest { UUID bookId; - UUID patronId; UUID libraryBranchId; Integer numberOfDays; } From e838aa9ac9a5e0feab7f50819ef06372da6ecaaf Mon Sep 17 00:00:00 2001 From: krzysztofwedrowicz Date: Thu, 26 Sep 2019 21:43:50 +0200 Subject: [PATCH 2/2] improved CS --- .../lending/patronprofile/web/PatronProfileControllerIT.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java b/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java index 49e96be..8e32273 100644 --- a/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java +++ b/src/integration-test/groovy/io/pillopl/library/lending/patronprofile/web/PatronProfileControllerIT.java @@ -37,7 +37,9 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.springframework.http.HttpHeaders.CONTENT_TYPE; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -182,7 +184,6 @@ public void shouldReturn500IfSomethingFailedWhileDuringPlacingOnHold() throws Ex given(placingOnHold.placeOnHold(any())).willReturn(Try.failure(new IllegalArgumentException())); var request = "{\"bookId\":\"6e1dfec5-5cfe-487e-814e-d70114f5396e\", \"libraryBranchId\":\"a518e2ef-5f6c-43e3-a7fc-5d895e15be3a\",\"numberOfDays\":1}"; - // expect mvc.perform(post("/profiles/" + patronId.getPatronId() + "/holds") .accept(MediaTypes.HAL_FORMS_JSON_VALUE)