feat: rework tests to explicit given-when-then pattern (#86)

igor-baiborodine · Oct 15, 2023 · e712789 · e712789
1 parent 579b4a7
commit e712789
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 20 deletions.
diff --git a/concurrent-test/booking-payload.json b/concurrent-test/booking-payload.json
@@ -1,8 +1,8 @@
 {
-  "uuid": "UUID",
   "email": "EMAIL",
   "fullName": "FULL_NAME",
   "startDate": "START_DATE",
   "endDate": "END_DATE",
-  "campsiteId": "1"
+  "campsiteId": "1",
+  "active": "true"
 }
diff --git a/concurrent-test/create-bookings.sh b/concurrent-test/create-bookings.sh
@@ -6,22 +6,19 @@ start_date="$1"
 end_date="$2"
 base_url="$3"
 
-payload1=$(< concurrent-test/booking-payload.json sed -e "s/UUID/$(uuidgen)/g" \
-  | sed -e "s/EMAIL/[email protected]/g" \
+payload1=$(< concurrent-test/booking-payload.json sed -e "s/EMAIL/[email protected]/g" \
   | sed -e "s/FULL_NAME/John Smith 1/g" \
   | sed -e "s/START_DATE/${start_date}/g" \
   | sed -e "s/END_DATE/${end_date}/g")
 printf "Create payload 1: %s\n" "$payload1"
 
-payload2=$(< concurrent-test/booking-payload.json sed -e "s/UUID/$(uuidgen)/g" \
-  | sed -e "s/EMAIL/[email protected]/g" \
-  | sed -e "s/FULL_NAME/John Smith /g" \
+payload2=$(< concurrent-test/booking-payload.json sed -e "s/EMAIL/[email protected]/g" \
+  | sed -e "s/FULL_NAME/John Smith 2/g" \
   | sed -e "s/START_DATE/${start_date}/g" \
   | sed -e "s/END_DATE/${end_date}/g")
 printf "Create payload 2: %s\n" "$payload2"
 
-payload3=$(< concurrent-test/booking-payload.json sed -e "s/UUID/$(uuidgen)/g" \
-  | sed -e "s/EMAIL/[email protected]/g" \
+payload3=$(< concurrent-test/booking-payload.json sed -e "s/EMAIL/[email protected]/g" \
   | sed -e "s/FULL_NAME/John Smith 3/g" \
   | sed -e "s/START_DATE/${start_date}/g" \
   | sed -e "s/END_DATE/${end_date}/g")

diff --git a/concurrent-test/update-booking.sh b/concurrent-test/update-booking.sh
@@ -5,10 +5,8 @@ set -e
 start_date="$1"
 end_date="$2"
 base_url="$3"
-uuid="$(uuidgen)"
 
-payload=$(< concurrent-test/booking-payload.json sed -e "s/UUID/$uuid/g" \
-  | sed -e "s/EMAIL/[email protected]/g" \
+payload=$(< concurrent-test/booking-payload.json sed -e "s/EMAIL/[email protected]/g" \
   | sed -e "s/FULL_NAME/John Smith 1/g" \
   | sed -e "s/START_DATE/$start_date/g" \
   | sed -e "s/END_DATE/$end_date/g")
@@ -17,6 +15,9 @@ printf "Create payload: %s\n" "$payload"
 response=$(curl -X POST -H "Content-Type: application/json" -d "$payload" "$base_url"/v2/booking)
 printf "Response: %s\n" "$response"
 
+uuid=$(echo "$response" | sed -En 's/.*"uuid":"([^"]*).*/\1/p')
+printf "UUID: %s\n" "$uuid"
+
 payload1=${response//\"campsiteId\"\:1/\"campsiteId\"\:2}
 printf "Update payload 1: %s\n" "$payload1"
 

diff --git a/src/main/java/com/kiroule/campsite/booking/api/contract/v2/dto/BookingDto.java b/src/main/java/com/kiroule/campsite/booking/api/contract/v2/dto/BookingDto.java
@@ -29,6 +29,8 @@ public class BookingDto {
   /** Business ID */
   private UUID uuid;
 
+  private Long version;
+
   @NotNull private Long campsiteId;
 
   @NotEmpty

diff --git a/src/main/java/com/kiroule/campsite/booking/api/service/BookingServiceImpl.java b/src/main/java/com/kiroule/campsite/booking/api/service/BookingServiceImpl.java
@@ -1,6 +1,7 @@
 package com.kiroule.campsite.booking.api.service;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
 import static java.lang.String.format;
 import static java.util.Objects.isNull;
 import static java.util.UUID.randomUUID;
@@ -75,6 +76,7 @@ public Booking findByUuid(UUID uuid) {
   public Booking createBooking(Booking booking) {
 
     checkArgument(isNull(booking.getUuid()), "New booking must not have UUID");
+    checkArgument(isNull(booking.getVersion()), "New booking must not have version");
     checkArgument(booking.isActive(), "Booking must be active");
 
     validateVacantDates(booking);
@@ -92,13 +94,13 @@ public Booking createBooking(Booking booking) {
       backoff = @Backoff(delay = 500, maxDelay = 1000))
   public Booking updateBooking(Booking booking) {
 
-    // cancelBooking method should be used to cancel booking
+    // update should not be used to cancel booking
     checkArgument(booking.isActive(), "Booking must be active");
-    var persistedBooking = findByUuid(booking.getUuid());
+    var existingBooking = findByUuid(booking.getUuid());
+    checkState(existingBooking.isActive(), "Non-active booking cannot be updated");
     validateVacantDates(booking);
 
-    booking.setId(persistedBooking.getId());
-    booking.setVersion(persistedBooking.getVersion());
+    booking.setId(existingBooking.getId());
     var bookingEntity = bookingMapper.toBookingEntity(booking);
     var savedBookingEntity = bookingRepository.saveAndFlush(bookingEntity);
 

diff --git a/src/test/java/com/kiroule/campsite/booking/api/BaseIT.java b/src/test/java/com/kiroule/campsite/booking/api/BaseIT.java
@@ -1,8 +1,10 @@
 package com.kiroule.campsite.booking.api;
 
+import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
+
 import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.test.context.ActiveProfiles;
 
-@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
+@SpringBootTest(webEnvironment = RANDOM_PORT)
 @ActiveProfiles("in-memory-db")
 public abstract class BaseIT {}
diff --git a/src/test/java/com/kiroule/campsite/booking/api/TestDataHelper.java b/src/test/java/com/kiroule/campsite/booking/api/TestDataHelper.java
@@ -120,4 +120,8 @@ public BookingEntity createBookingEntity(
 
     return savedBookingEntity;
   }
+
+  public BookingEntity updateBookingEntity(BookingEntity bookingEntity) {
+    return bookingRepository.save(bookingEntity);
+  }
 }
diff --git a/src/test/java/com/kiroule/campsite/booking/api/controller/BookingControllerIT.java b/src/test/java/com/kiroule/campsite/booking/api/controller/BookingControllerIT.java
@@ -127,6 +127,7 @@ void given_booking_dates_not_available__then_status_bad_request() {
       BookingDto bookingDto =
           nextBookingDto().toBuilder()
               .uuid(null)
+              .version(null)
               .campsiteId(campsiteEntity.getId())
               .startDate(bookingEntity.getStartDate())
               .endDate(bookingEntity.getEndDate())
@@ -195,6 +196,7 @@ void happy_path() {
       BookingDto bookingDto =
           nextBookingDto().toBuilder()
               .uuid(bookingEntity.getUuid())
+              .version(bookingEntity.getVersion())
               .campsiteId(bookingEntity.getCampsiteId())
               .startDate(bookingEntity.getStartDate().plusDays(1))
               .endDate(bookingEntity.getEndDate().plusDays(2))
@@ -210,7 +212,11 @@ void happy_path() {
               .put(BASE_PATH + "/{uuid}")
               .as(BookingDto.class);
       // then
-      assertThat(updatedBookingDto).usingRecursiveComparison().isEqualTo(bookingDto);
+      assertThat(updatedBookingDto)
+          .usingRecursiveComparison()
+          .ignoringFields("version")
+          .isEqualTo(bookingDto);
+      assertThat(updatedBookingDto.getVersion()).isEqualTo(1L);
     }
 
     @Test
@@ -244,6 +250,37 @@ void given_other_existing_booking_with_same_booking_dates__then_status_bad_reque
               bookingDto1.getStartDate(), bookingDto1.getEndDate());
       assertThat(apiError.getMessage()).isEqualTo(message);
     }
+
+    @Test
+    void given_existing_booking_was_updated_by_another_transaction__then_status_conflict() {
+      // given
+      CampsiteEntity campsiteEntity = testDataHelper.createCampsiteEntity();
+      BookingEntity bookingEntity = testDataHelper.createBookingEntity(campsiteEntity.getId());
+      BookingDto bookingDto =
+          nextBookingDto().toBuilder()
+              .uuid(bookingEntity.getUuid())
+              .version(bookingEntity.getVersion())
+              .campsiteId(bookingEntity.getCampsiteId())
+              .startDate(bookingEntity.getStartDate())
+              .endDate(bookingEntity.getEndDate().plusDays(2))
+              .active(bookingEntity.isActive())
+              .build();
+      testDataHelper.updateBookingEntity(
+          bookingEntity.toBuilder().endDate(bookingEntity.getEndDate().plusDays(5)).build());
+      // when
+      ApiError apiError =
+          given()
+              .pathParam("uuid", bookingDto.getUuid())
+              .contentType(APPLICATION_JSON_VALUE)
+              .body(bookingDto)
+              .when()
+              .put(BASE_PATH + "/{uuid}")
+              .as(ApiError.class);
+      // then
+      assertThat(apiError.getStatus()).isEqualTo(CONFLICT);
+      String message = "Optimistic locking error - booking was updated by another transaction";
+      assertThat(apiError.getMessage()).isEqualTo(message);
+    }
   }
 
   @Nested

diff --git a/src/test/java/com/kiroule/campsite/booking/api/service/BookingServiceImplTest.java b/src/test/java/com/kiroule/campsite/booking/api/service/BookingServiceImplTest.java
@@ -75,7 +75,7 @@ class CreateBooking {
 
     @Test
     void happy_path() {
-      Booking booking = nextBooking().toBuilder().uuid(null).build();
+      Booking booking = nextBooking().toBuilder().uuid(null).version(null).build();
       BookingEntity bookingEntity = nextBookingEntity();
 
       doReturn(emptyList())
@@ -99,7 +99,7 @@ void happy_path() {
     @Test
     void given_booking_dates_not_available__then_BookingDatesNotAvailableException_thrown() {
       // given
-      Booking booking = nextBooking().toBuilder().uuid(null).build();
+      Booking booking = nextBooking().toBuilder().uuid(null).version(null).build();
       Booking existingBooking =
           nextBooking().toBuilder()
               .startDate(booking.getStartDate())