Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat : adds dynamic sort #1571

Merged
merged 5 commits into from
Dec 17, 2024
Merged

feat : adds dynamic sort #1571

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7dccc33
feat : adds dynamic sort
rajadilipkolli Dec 17, 2024
c0266d5
feat : open for extension, closed for modification
rajadilipkolli Dec 17, 2024
409992a
implements code review comments
rajadilipkolli Dec 17, 2024
326ec24
enhance logging
rajadilipkolli Dec 17, 2024
ab911f3
removes wild card import
rajadilipkolli Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions ...-pagination/src/main/java/com/example/keysetpagination/config/GlobalExceptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
44 changes: 44 additions & 0 deletions ...ndow-pagination/src/main/java/com/example/keysetpagination/model/query/SearchRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.example.keysetpagination.model.query;

import java.util.ArrayList;
import java.util.List;

public class SearchRequest {

private List<SearchCriteria> searchCriteriaList;

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 SearchRequest setSearchCriteriaList(List<SearchCriteria> searchCriteriaList) {
this.searchCriteriaList = searchCriteriaList;
return this;
}

public List<SortRequest> getSortRequests() {
return sortRequests;
}

public SearchRequest setSortRequests(List<SortRequest> sortRequests) {
this.sortRequests = sortRequests;
return this;
}
rajadilipkolli marked this conversation as resolved.
Show resolved Hide resolved

@Override
public String toString() {
return "SearchRequest{" + "searchCriteria=" + searchCriteriaList + ", sortRequests=" + sortRequests + '}';
}
}
42 changes: 42 additions & 0 deletions ...window-pagination/src/main/java/com/example/keysetpagination/model/query/SortRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.example.keysetpagination.model.query;

import java.util.Objects;

public class SortRequest {

private String field;
private String direction;

Comment on lines +7 to +9
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add validation for sort direction field

The direction field should be constrained to valid sort directions to prevent runtime errors.

Consider using an enum and adding validation:

-    private String direction;
+    private Direction direction;
+
+    public enum Direction {
+        ASC, DESC;
+        
+        public static Direction fromString(String value) {
+            try {
+                return Direction.valueOf(value.toUpperCase());
+            } catch (IllegalArgumentException e) {
+                throw new IllegalArgumentException("Invalid sort direction. Use ASC or DESC");
+            }
+        }
+    }

Committable suggestion skipped: line range outside the PR's diff.

public String getField() {
return field;
}

public void setField(String field) {
this.field = field;
}

public String getDirection() {
return direction;
}

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);
}
}
32 changes: 24 additions & 8 deletions ...-window-pagination/src/main/java/com/example/keysetpagination/services/AnimalService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@
import com.example.keysetpagination.exception.AnimalNotFoundException;
import com.example.keysetpagination.mapper.AnimalMapper;
import com.example.keysetpagination.model.query.FindAnimalsQuery;
import com.example.keysetpagination.model.query.SearchCriteria;
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;
import com.example.keysetpagination.repositories.AnimalRepository;
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;
Expand All @@ -21,11 +23,14 @@
import org.springframework.data.jpa.domain.Specification;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.CollectionUtils;

@Service
@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;
Expand Down Expand Up @@ -60,22 +65,33 @@ private Pageable createPageable(FindAnimalsQuery findAnimalsQuery) {
return PageRequest.of(pageNo, findAnimalsQuery.pageSize(), sort);
}

public Window<AnimalResponse> searchAnimals(List<SearchCriteria> searchCriteriaList, int pageSize, Long scrollId) {
public Window<AnimalResponse> searchAnimals(SearchRequest searchRequest, int pageSize, Long scrollId) {

Specification<Animal> specification =
animalEntitySpecification.specificationBuilder(searchCriteriaList, Animal.class);
animalEntitySpecification.specificationBuilder(searchRequest.getSearchCriteriaList(), Animal.class);

// Create initial ScrollPosition or continue from the given scrollId
ScrollPosition position = scrollId == null
? ScrollPosition.keyset()
: ScrollPosition.of(Collections.singletonMap("id", scrollId), ScrollPosition.Direction.FORWARD);

// Parse and create sort orders
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();
Comment on lines +79 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Consider adding field name validation and sanitization.

While the sort implementation effectively uses Java 8+ features, it could benefit from additional security measures:

  1. Validate sort fields against a whitelist of allowed entity properties
  2. Sanitize field names to prevent SQL injection

Here's a suggested implementation:

private static final Set<String> ALLOWED_SORT_FIELDS = 
    Set.of("id", "name", "species", "age"); // add all valid fields

private List<Sort.Order> createSortOrders(List<SortRequest> sortRequests) {
    if (CollectionUtils.isEmpty(sortRequests)) {
        return Collections.singletonList(new Sort.Order(Sort.Direction.ASC, "id"));
    }
    
    return sortRequests.stream()
            .map(sortRequest -> {
                String field = sortRequest.getField();
                if (!ALLOWED_SORT_FIELDS.contains(field)) {
                    throw new IllegalArgumentException("Invalid sort field: " + field);
                }
                
                Sort.Direction direction = "desc"
                        .equalsIgnoreCase(Optional.ofNullable(sortRequest.getDirection())
                                .orElse("asc"))
                        ? Sort.Direction.DESC
                        : Sort.Direction.ASC;
                return new Sort.Order(direction, field);
            })
            .toList();
}


log.debug("Executing search with specification: {} and sort orders: {}", specification, orders);
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, how to print the specification here in human readable format?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, to print the specification in a human-readable format, you can override the toString() method in your Specification implementations. If your EntitySpecification class builds custom Specification instances, you can modify them to include meaningful toString() methods.

Since you're constructing the specification using the search criteria list, another approach is to log the search criteria directly. For example:

log.debug("Executing search with criteria: {} and sort orders: {}", searchRequest.getSearchCriteriaList(), orders);

This will output the list of SearchCriteria objects, which should give you a readable representation of the search parameters used.

If you prefer to see the actual predicates or the detailed structure of the specification, you might need to implement a custom method to traverse and display the specification components. However, logging the search criteria or enhancing the toString() method in your specifications is often sufficient for debugging purposes.

Let me know if you need further assistance!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, can you please help me to write custom method to traverse and display the specification components.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, certainly! To write a custom method to traverse and display the components of your JPA Specification, you can create a custom Specification implementation that keeps track of each criterion and provides a human-readable representation.

Here's how you can achieve this:

  1. Create a Custom Specification Wrapper:

    Define a wrapper class that implements Specification and includes a description of the criterion.

    import org.springframework.data.jpa.domain.Specification;
import javax.persistence.criteria.*;

public class DescriptiveSpecification<T> implements Specification<T> {
    private final Specification<T> delegate;
    private final String description;

    public DescriptiveSpecification(Specification<T> delegate, String description) {
        this.delegate = delegate;
        this.description = description;
    }

    @Override
    public Predicate toPredicate(Root<T> root, CriteriaQuery<?> query, CriteriaBuilder criteriaBuilder) {
        return delegate.toPredicate(root, query, criteriaBuilder);
    }

    @Override
    public String toString() {
        return description;
    }

    // Methods to combine specifications with descriptions
    public DescriptiveSpecification<T> and(Specification<T> other, String otherDesc) {
        return new DescriptiveSpecification<>(this.delegate.and(other), "(" + this + " AND " + otherDesc + ")");
    }

    public DescriptiveSpecification<T> or(Specification<T> other, String otherDesc) {
        return new DescriptiveSpecification<>(this.delegate.or(other), "(" + this + " OR " + otherDesc + ")");
    }
}

  2. Modify Your Specification Builder:

    In your EntitySpecification class, modify the builder method to use DescriptiveSpecification.

    import java.util.List;

public class EntitySpecification<T> {

    public Specification<T> specificationBuilder(List<SearchCriteria> criteriaList, Class<T> clazz) {
        if (criteriaList == null || criteriaList.isEmpty()) {
            return null;
        }

        DescriptiveSpecification<T> result = null;

        for (SearchCriteria criteria : criteriaList) {
            // Create a basic specification for the criterion
            Specification<T> spec = new CustomSpecification<>(criteria);

            // Wrap it with a description
            DescriptiveSpecification<T> descriptiveSpec = new DescriptiveSpecification<>(
                spec, criteria.toString()
            );

            if (result == null) {
                result = descriptiveSpec;
            } else {
                // Combine specifications with descriptions
                result = result.and(descriptiveSpec, criteria.toString());
            }
        }

        return result;
    }
}

  3. Ensure SearchCriteria Has a Meaningful toString():

    public class SearchCriteria {
    private String key;
    private String operation;
    private Object value;

    // getters and setters

    @Override
    public String toString() {
        return key + " " + operation + " " + value;
    }
}

  4. Update Logging in AnimalService:

    Now, when you log the specification, it will display a human-readable format.

    log.debug("Executing search with specification: {} and sort orders: {}", specification, orders);

    This will output something like:

    Executing search with specification: (age >= 2 AND type == 'Mammal') and sort orders: [age: DESC]

  5. Example Usage in AnimalService:

    public Window<AnimalResponse> searchAnimals(SearchRequest searchRequest, int pageSize, Long scrollId) {

    Specification<Animal> specification =
        animalEntitySpecification.specificationBuilder(searchRequest.getSearchCriteriaList(), Animal.class);

    // Rest of your code...
}

Notes:

  • Custom Specification (CustomSpecification): Ensure that your individual criterion specifications (CustomSpecification) are correctly implemented to handle each SearchCriteria.

  • Combining Specifications: The and and or methods in DescriptiveSpecification help maintain the descriptive string as specifications are combined.

  • Thread Safety: If your application is multithreaded, consider thread safety when storing and combining descriptions.

This approach allows you to keep your specifications functional while providing insight into the criteria being applied, which is invaluable for debugging complex queries.

Let me know if you need further assistance or any clarification!

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, can you please create an issue to implement above feature

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rajadilipkolli, I've created a new GitHub issue to implement the custom method to traverse and display the Specification components as per your request. You can find the issue here: #1574.

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Create a GitHub issue to implement the custom method to traverse and display Specification components.
gh issue create \
  --title "Implement custom method to traverse and display Specification components" \
  --body $'As discussed in [PR #1571](https://github.com/rajadilipkolli/my-spring-boot-experiments/pull/1571), we need to implement a custom method to traverse and display the Specification components in a human-readable format. This enhancement will aid in debugging complex queries and improve logging clarity.\n\nRequester: @rajadilipkolli\nPR URL: https://github.com/rajadilipkolli/my-spring-boot-experiments/pull/1571\nComment URL: https://github.com/rajadilipkolli/my-spring-boot-experiments/pull/1571#discussion_r1888852186' \
  --assignee rajadilipkolli

Length of output: 724

return animalRepository
.findAll(
specification,
PageRequest.of(0, pageSize, Sort.by(Sort.Order.asc("id"))),
position,
Animal.class)
.findAll(specification, PageRequest.of(0, pageSize, Sort.by(orders)), position, Animal.class)
.map(animalMapper::toResponse);
}

Expand Down
16 changes: 9 additions & 7 deletions ...gination/src/main/java/com/example/keysetpagination/web/controllers/AnimalController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@

import com.example.keysetpagination.exception.AnimalNotFoundException;
import com.example.keysetpagination.model.query.FindAnimalsQuery;
import com.example.keysetpagination.model.query.SearchCriteria;
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;
import com.example.keysetpagination.services.AnimalService;
import com.example.keysetpagination.utils.AppConstants;
import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.Parameter;
import io.swagger.v3.oas.annotations.enums.ParameterIn;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import jakarta.validation.Valid;
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import java.net.URI;
import java.util.List;
import org.springframework.data.domain.Window;
import org.springframework.http.ResponseEntity;
import org.springframework.validation.annotation.Validated;
Expand All @@ -38,7 +38,7 @@ class AnimalController {

private final AnimalService animalService;

public AnimalController(AnimalService animalService) {
AnimalController(AnimalService animalService) {
this.animalService = animalService;
}

Expand All @@ -65,15 +65,17 @@ PagedResult<AnimalResponse> getAllAnimals(
})
@PostMapping("/search")
public Window<AnimalResponse> searchAnimals(
@Parameter(description = "Number of items per page (max 100)")
@Parameter(description = "Number of items per page (max 100)", in = ParameterIn.QUERY)
rajadilipkolli marked this conversation as resolved.
Show resolved Hide resolved
@RequestParam(defaultValue = "10")
@Min(1)
@Max(100)
int pageSize,
@Parameter(description = "Scroll ID for pagination") @RequestParam(required = false) Long scrollId,
@RequestBody @Valid List<SearchCriteria> searchCriteria) {
@Parameter(description = "Scroll ID for pagination", in = ParameterIn.QUERY) @RequestParam(required = false)
Long scrollId,
@io.swagger.v3.oas.annotations.parameters.RequestBody(required = true) @RequestBody @Valid
SearchRequest searchRequest) {

return animalService.searchAnimals(searchCriteria, pageSize, scrollId);
return animalService.searchAnimals(searchRequest, pageSize, scrollId);
}

@GetMapping("/{id}")
Expand Down
Loading
Loading