implements code review comments

rajadilipkolli · Dec 17, 2024 · 409992a · 409992a
1 parent c0266d5
commit 409992a
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 22 deletions.
diff --git a/...-pagination/src/main/java/com/example/keysetpagination/config/GlobalExceptionHandler.java b/...-pagination/src/main/java/com/example/keysetpagination/config/GlobalExceptionHandler.java
@@ -1,6 +1,7 @@
 package com.example.keysetpagination.config;
 
 import com.example.keysetpagination.exception.ResourceNotFoundException;
+import jakarta.validation.ConstraintViolationException;
 import java.net.URI;
 import java.time.Instant;
 import java.util.Comparator;
@@ -40,6 +41,23 @@ ProblemDetail onException(MethodArgumentNotValidException methodArgumentNotValid
         return problemDetail;
     }
 
+    @ExceptionHandler(ConstraintViolationException.class)
+    ProblemDetail onException(ConstraintViolationException constraintViolationException) {
+        List<ApiValidationError> apiValidationErrors = constraintViolationException.getConstraintViolations().stream()
+                .map(constraintViolation -> new ApiValidationError(
+                        null,
+                        constraintViolation.getPropertyPath().toString(),
+                        constraintViolation.getInvalidValue(),
+                        constraintViolation.getMessage()))
+                .sorted(Comparator.comparing(ApiValidationError::field))
+                .toList();
+        ProblemDetail problemDetail = ProblemDetail.forStatusAndDetail(
+                HttpStatusCode.valueOf(400), constraintViolationException.getMessage());
+        problemDetail.setTitle("Constraint Violation");
+        problemDetail.setProperty("violations", apiValidationErrors);
+        return problemDetail;
+    }
+
     @ExceptionHandler(Exception.class)
     ProblemDetail onException(Exception exception) {
         if (exception instanceof ResourceNotFoundException resourceNotFoundException) {

diff --git a/...ndow-pagination/src/main/java/com/example/keysetpagination/model/query/SearchRequest.java b/...ndow-pagination/src/main/java/com/example/keysetpagination/model/query/SearchRequest.java
@@ -1,5 +1,6 @@
 package com.example.keysetpagination.model.query;
 
+import java.util.ArrayList;
 import java.util.List;
 
 public class SearchRequest {
@@ -8,19 +9,36 @@ public class SearchRequest {
 
     private List<SortRequest> sortRequests;
 
+    public SearchRequest() {
+        this.searchCriteriaList = new ArrayList<>();
+        this.sortRequests = new ArrayList<>();
+    }
+
+    public SearchRequest(List<SearchCriteria> searchCriteriaList, List<SortRequest> sortRequests) {
+        this.searchCriteriaList = searchCriteriaList != null ? searchCriteriaList : new ArrayList<>();
+        this.sortRequests = sortRequests != null ? sortRequests : new ArrayList<>();
+    }
+
     public List<SearchCriteria> getSearchCriteriaList() {
         return searchCriteriaList;
     }
 
-    public void setSearchCriteriaList(List<SearchCriteria> searchCriteriaList) {
+    public SearchRequest setSearchCriteriaList(List<SearchCriteria> searchCriteriaList) {
         this.searchCriteriaList = searchCriteriaList;
+        return this;
     }
 
-    public List<SortRequest> getSortDtos() {
+    public List<SortRequest> getSortRequests() {
         return sortRequests;
     }
 
-    public void setSortDtos(List<SortRequest> sortRequests) {
+    public SearchRequest setSortRequests(List<SortRequest> sortRequests) {
         this.sortRequests = sortRequests;
+        return this;
+    }
+
+    @Override
+    public String toString() {
+        return "SearchRequest{" + "searchCriteria=" + searchCriteriaList + ", sortRequests=" + sortRequests + '}';
     }
 }
diff --git a/...window-pagination/src/main/java/com/example/keysetpagination/model/query/SortRequest.java b/...window-pagination/src/main/java/com/example/keysetpagination/model/query/SortRequest.java
@@ -1,5 +1,7 @@
 package com.example.keysetpagination.model.query;
 
+import java.util.Objects;
+
 public class SortRequest {
 
     private String field;
@@ -20,4 +22,21 @@ public String getDirection() {
     public void setDirection(String direction) {
         this.direction = direction;
     }
+
+    @Override
+    public String toString() {
+        return "SortRequest{field='" + field + "', direction=" + direction + '}';
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof SortRequest that)) return false;
+        return Objects.equals(field, that.field) && Objects.equals(direction, that.direction);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(field, direction);
+    }
 }
diff --git a/...-window-pagination/src/main/java/com/example/keysetpagination/services/AnimalService.java b/...-window-pagination/src/main/java/com/example/keysetpagination/services/AnimalService.java
@@ -5,15 +5,15 @@
 import com.example.keysetpagination.mapper.AnimalMapper;
 import com.example.keysetpagination.model.query.FindAnimalsQuery;
 import com.example.keysetpagination.model.query.SearchRequest;
-import com.example.keysetpagination.model.query.SortRequest;
 import com.example.keysetpagination.model.request.AnimalRequest;
 import com.example.keysetpagination.model.response.AnimalResponse;
 import com.example.keysetpagination.model.response.PagedResult;
 import com.example.keysetpagination.repositories.AnimalRepository;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
@@ -29,6 +29,8 @@
 @Transactional(readOnly = true)
 public class AnimalService {
 
+    private static final Logger log = LoggerFactory.getLogger(AnimalService.class);
+
     private final AnimalRepository animalRepository;
     private final AnimalMapper animalMapper;
     private final EntitySpecification<Animal> animalEntitySpecification;
@@ -74,21 +76,20 @@ public Window<AnimalResponse> searchAnimals(SearchRequest searchRequest, int pag
                 : ScrollPosition.of(Collections.singletonMap("id", scrollId), ScrollPosition.Direction.FORWARD);
 
         // Parse and create sort orders
-        List<Sort.Order> orders = new ArrayList<>();
-
-        if (!CollectionUtils.isEmpty(searchRequest.getSortDtos())) {
-            for (SortRequest sortRequest : searchRequest.getSortDtos()) {
-                Sort.Direction direction = "desc"
-                                .equalsIgnoreCase(Optional.ofNullable(sortRequest.getDirection())
-                                        .orElse("asc"))
-                        ? Sort.Direction.DESC
-                        : Sort.Direction.ASC;
-                orders.add(new Sort.Order(direction, sortRequest.getField()));
-            }
-        } else {
-            orders.add(new Sort.Order(Sort.Direction.ASC, "id"));
-        }
-
+        List<Sort.Order> orders = CollectionUtils.isEmpty(searchRequest.getSortRequests())
+                ? Collections.singletonList(new Sort.Order(Sort.Direction.ASC, "id"))
+                : searchRequest.getSortRequests().stream()
+                        .map(sortRequest -> {
+                            Sort.Direction direction = "desc"
+                                            .equalsIgnoreCase(Optional.ofNullable(sortRequest.getDirection())
+                                                    .orElse("asc"))
+                                    ? Sort.Direction.DESC
+                                    : Sort.Direction.ASC;
+                            return new Sort.Order(direction, sortRequest.getField());
+                        })
+                        .toList();
+
+        log.debug("Executing search with specification: {} and sort orders: {}", specification, orders);
         return animalRepository
                 .findAll(specification, PageRequest.of(0, pageSize, Sort.by(orders)), position, Animal.class)
                 .map(animalMapper::toResponse);

diff --git a/...nation/src/test/java/com/example/keysetpagination/web/controllers/AnimalControllerIT.java b/...nation/src/test/java/com/example/keysetpagination/web/controllers/AnimalControllerIT.java
@@ -434,7 +434,7 @@ void shouldReturnResultForLikeOperator() throws Exception {
                                         """))
                 .andExpect(status().isOk())
                 .andExpect(jsonPath(
-                        "$.content.size()", is(6))) // "Elephant", "Penguin", "Crocodile", ""Eagle"", "Whale", "Snake"
+                        "$.content.size()", is(6))) // "Elephant", "Penguin", "Crocodile", "Eagle", "Whale", "Snake"
                 .andExpect(jsonPath("$.last", is(true)));
     }
 
@@ -535,7 +535,7 @@ void shouldReturnResultForBetweenOperator() throws Exception {
                                             "field": "id",
                                             "values": [
                                                 "%d",
-                                                "%s"
+                                                "%d"
                                             ]
                                         }
                                     ]

diff --git a/...tion/src/test/java/com/example/keysetpagination/web/controllers/AnimalControllerTest.java b/...tion/src/test/java/com/example/keysetpagination/web/controllers/AnimalControllerTest.java
@@ -19,6 +19,7 @@
 import com.example.keysetpagination.entities.Animal;
 import com.example.keysetpagination.exception.AnimalNotFoundException;
 import com.example.keysetpagination.model.query.FindAnimalsQuery;
+import com.example.keysetpagination.model.query.SearchRequest;
 import com.example.keysetpagination.model.request.AnimalRequest;
 import com.example.keysetpagination.model.response.AnimalResponse;
 import com.example.keysetpagination.model.response.PagedResult;
@@ -113,6 +114,36 @@ void shouldFetchAllAnimalsWithCustomPageSize() throws Exception {
                 .andExpect(jsonPath("$.hasPrevious", is(true)));
     }
 
+    @Test
+    void shouldReturnBadRequestWhenPageSizeIsLessThanMin() throws Exception {
+        SearchRequest searchRequest = new SearchRequest();
+        mockMvc.perform(post("/api/animals/search")
+                        .param("pageSize", "0") // Less than minimum value of 1
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(objectMapper.writeValueAsString(searchRequest)))
+                .andExpect(status().isBadRequest());
+    }
+
+    @Test
+    void shouldReturnBadRequestWhenPageSizeExceedsMax() throws Exception {
+        SearchRequest searchRequest = new SearchRequest();
+        mockMvc.perform(post("/api/animals/search")
+                        .param("pageSize", "101") // Exceeds maximum value of 100
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(objectMapper.writeValueAsString(searchRequest)))
+                .andExpect(status().isBadRequest());
+    }
+
+    @Test
+    void shouldReturnOkWhenPageSizeIsWithinValidRange() throws Exception {
+        SearchRequest searchRequest = new SearchRequest();
+        mockMvc.perform(post("/api/animals/search")
+                        .param("pageSize", "50") // Within valid range (1-100)
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(objectMapper.writeValueAsString(searchRequest)))
+                .andExpect(status().isOk());
+    }
+
     @Test
     void shouldFindAnimalById() throws Exception {
         Long animalId = 1L;

diff --git a/jpa/keyset-pagination/boot-data-window-pagination/src/test/resources/logback-test.xml b/jpa/keyset-pagination/boot-data-window-pagination/src/test/resources/logback-test.xml
@@ -12,4 +12,5 @@
     <logger name="org.testcontainers" level="INFO"/>
     <logger name="com.github.dockerjava" level="WARN"/>
     <logger name="query-logger" level="DEBUG" />
+    <logger name="com.example.keysetpagination" level="DEBUG" />
 </configuration>