Merge branch 'main' into chore/improve-api-comparison-test

CHANGELOG.md

@@ -8,8 +8,13 @@ _**For better traceability add the corresponding GitHub issue number in each cha
 
 ## [Unreleased]
 
+### Fixed
+
+- Improved exception handling concerning invalid date format in search parameters for `GET /irs/policies/paged`. #639
+
 ### Changed
 
+- Improved documentation for `GET /irs/policies/paged` endpoint. #639
 - Cleanup in IrsApplicationTest.generatedOpenApiMatchesContract 
   (removed obsolete ignoringFields, improved assertion message)
 

docs/src/api/irs-api.yaml

@@ -1042,19 +1042,21 @@ paths:
       - Policy Store API
   /irs/policies/paged:
     get:
-      description: |
-        Fetch a page of policies with options to filter and sort.
-
-        - **Filtering:**
-          `search=<property>,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],<value>`.
-          Example: `search=BPN,STARTS_WITH,BPNL12&search=policyId,STARTS_WITH,policy2`.
-
-        - **Sorting:**
-          `sort=<property>,[asc|desc]`.
-          Example: `sort=BPN,asc&sort=policyId,desc`.
-
-        - **Paging:**
-          Example: `page=1&size=20`
+      description: "Fetch a page of policies with options to filter and sort.\n \n\
+        Example:\n```\nsearch=BPN,STARTS_WITH,BPNL12&search=validUntil,AFTER_LOCAL_DATE,2024-12-11&sort=validUntil,asc&sort=BPN,asc\n\
+        ```\n \n### Filtering\n \n`search=<Field>,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],<Value>`.\n\
+        \ \nExample: `search=BPN,STARTS_WITH,BPNL12&search=policyId,STARTS_WITH,policy2`.\n\
+        \ \n| Field        | Supported Operations                     | Value Format\
+        \         |\n|--------------|------------------------------------------|----------------------|\n\
+        | `BPN`        | `EQUALS`, `STARTS_WITH`                  | any string   \
+        \        |\n| `policyId`   | `EQUALS`, `STARTS_WITH`                  | any\
+        \ string           |\n| `action`     | `EQUALS`                          \
+        \       | `use` or `access`    |\n| `createdOn`  | `BEFORE_LOCAL_DATE`, `AFTER_LOCAL_DATE`\
+        \  | `yyyy-MM-dd`         |\n| `validUntil` | `BEFORE_LOCAL_DATE`, `AFTER_LOCAL_DATE`\
+        \  | `yyyy-MM-dd`         |\n\n### Sorting\n`sort=[BPN|policyId|action|createdOn|validUntil],[asc|desc]`.\n\
+        \ \nExample: `sort=BPN,asc&sort=policyId,desc`. _(default: `BPN,asc`)_\n\n\
+        ### Paging\n \nExample: `page=1&size=20` _(default page size: 10, maximal\
+        \ page size: 1000)_\n"
       operationId: getPoliciesPaged
       parameters:
       - description: List of business partner numbers. This may also contain the value
@@ -1096,7 +1098,8 @@ paths:
           description: Authorization refused by server.
       security:
       - api_key: []
-      summary: Find policies.
+      summary: "Find registered policies that should be accepted in EDC negotiation\
+        \ (with filtering, sorting and paging)."
       tags:
       - Policy Store API
   /irs/policies/{policyId}:

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java

@@ -23,6 +23,9 @@
 import java.time.LocalTime;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
+import java.time.format.DateTimeParseException;
+
+import org.apache.commons.lang3.StringUtils;
 
 /**
  * Date utilities.
@@ -42,10 +45,21 @@ public static boolean isDateAfter(final OffsetDateTime dateTime, final String re
     }
 
     public static OffsetDateTime toOffsetDateTimeAtStartOfDay(final String dateString) {
-        return LocalDate.parse(dateString).atStartOfDay().atOffset(ZoneOffset.UTC);
+        return parseDate(dateString).atStartOfDay().atOffset(ZoneOffset.UTC);
     }
 
     public static OffsetDateTime toOffsetDateTimeAtEndOfDay(final String dateString) {
-        return LocalDate.parse(dateString).atTime(LocalTime.MAX).atOffset(ZoneOffset.UTC);
+        return parseDate(dateString).atTime(LocalTime.MAX).atOffset(ZoneOffset.UTC);
+    }
+
+    private static LocalDate parseDate(final String dateString) {
+        if (StringUtils.isBlank(dateString)) {
+            throw new IllegalArgumentException("Invalid date format (must not be blank)");
+        }
+        try {
+            return LocalDate.parse(dateString);
+        } catch (DateTimeParseException e) {
+            throw new IllegalArgumentException("Invalid date format (please refer to the documentation)", e);
+        }
     }
 }

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java

@@ -262,20 +262,38 @@ public List<String> autocomplete(
 
     @GetMapping("/policies/paged")
     @ResponseStatus(HttpStatus.OK)
-    @Operation(summary = "Find policies.", //
+    @Operation(summary = "Find registered policies that should be accepted in EDC negotiation "
+            + "(with filtering, sorting and paging).", //
                description = """
                        Fetch a page of policies with options to filter and sort.
+                       \s
+                       Example:
+                       ```
+                       search=BPN,STARTS_WITH,BPNL12&search=validUntil,AFTER_LOCAL_DATE,2024-12-11&sort=validUntil,asc&sort=BPN,asc
+                       ```
+                       \s
+                       ### Filtering
+                       \s
+                       `search=<Field>,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],<Value>`.
+                       \s
+                       Example: `search=BPN,STARTS_WITH,BPNL12&search=policyId,STARTS_WITH,policy2`.
+                       \s
+                       | Field        | Supported Operations                     | Value Format         |
+                       |--------------|------------------------------------------|----------------------|
+                       | `BPN`        | `EQUALS`, `STARTS_WITH`                  | any string           |
+                       | `policyId`   | `EQUALS`, `STARTS_WITH`                  | any string           |
+                       | `action`     | `EQUALS`                                 | `use` or `access`    |
+                       | `createdOn`  | `BEFORE_LOCAL_DATE`, `AFTER_LOCAL_DATE`  | `yyyy-MM-dd`         |
+                       | `validUntil` | `BEFORE_LOCAL_DATE`, `AFTER_LOCAL_DATE`  | `yyyy-MM-dd`         |
                        
-                       - **Filtering:**
-                         `search=<property>,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],<value>`.
-                         Example: `search=BPN,STARTS_WITH,BPNL12&search=policyId,STARTS_WITH,policy2`.
+                       ### Sorting
+                       `sort=[BPN|policyId|action|createdOn|validUntil],[asc|desc]`.
+                       \s
+                       Example: `sort=BPN,asc&sort=policyId,desc`. _(default: `BPN,asc`)_
                        
-                       - **Sorting:**
-                         `sort=<property>,[asc|desc]`.
-                         Example: `sort=BPN,asc&sort=policyId,desc`.
-                       
-                       - **Paging:**
-                         Example: `page=1&size=20`
+                       ### Paging
+                       \s
+                       Example: `page=1&size=20` _(default page size: 10, maximal page size: 1000)_
                        """, //
                security = @SecurityRequirement(name = API_KEY), //
                tags = { POLICY_API_TAG }, //

irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyPagingService.java

@@ -24,8 +24,6 @@
 import static org.eclipse.tractusx.irs.policystore.common.CommonConstants.PROPERTY_CREATED_ON;
 import static org.eclipse.tractusx.irs.policystore.common.CommonConstants.PROPERTY_POLICY_ID;
 import static org.eclipse.tractusx.irs.policystore.common.CommonConstants.PROPERTY_VALID_UNTIL;
-import static org.eclipse.tractusx.irs.policystore.common.DateUtils.isDateAfter;
-import static org.eclipse.tractusx.irs.policystore.common.DateUtils.isDateBefore;
 import static org.eclipse.tractusx.irs.policystore.models.SearchCriteria.Operation.AFTER_LOCAL_DATE;
 import static org.eclipse.tractusx.irs.policystore.models.SearchCriteria.Operation.BEFORE_LOCAL_DATE;
 import static org.eclipse.tractusx.irs.policystore.models.SearchCriteria.Operation.EQUALS;
@@ -44,14 +42,17 @@
 import org.apache.commons.lang3.StringUtils;
 import org.eclipse.tractusx.irs.edc.client.policy.Permission;
 import org.eclipse.tractusx.irs.edc.client.policy.Policy;
+import org.eclipse.tractusx.irs.policystore.common.DateUtils;
 import org.eclipse.tractusx.irs.policystore.models.PolicyWithBpn;
 import org.eclipse.tractusx.irs.policystore.models.SearchCriteria;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageImpl;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
+import org.springframework.http.HttpStatus;
 import org.springframework.stereotype.Service;
+import org.springframework.web.server.ResponseStatusException;
 
 /**
  * Paging helper service for policies.
@@ -307,12 +308,12 @@ private Predicate<PolicyWithBpn> getActionFilter(final SearchCriteria<?> searchC
         private Predicate<PolicyWithBpn> getCreatedOnFilter(final SearchCriteria<?> searchCriteria) {
             return switch (searchCriteria.getOperation()) {
                 case BEFORE_LOCAL_DATE -> p -> {
-                    final OffsetDateTime createdOn = p.policy().getCreatedOn();
-                    return isDateBefore(createdOn, searchCriteria.getValue().toString());
+                    final OffsetDateTime date = p.policy().getCreatedOn();
+                    return isDateBefore(searchCriteria, date);
                 };
                 case AFTER_LOCAL_DATE -> p -> {
-                    final OffsetDateTime createdOn = p.policy().getCreatedOn();
-                    return isDateAfter(createdOn, searchCriteria.getValue().toString());
+                    final OffsetDateTime date = p.policy().getCreatedOn();
+                    return isDateAfter(searchCriteria, date);
                 };
                 default -> throw new IllegalArgumentException(
                         MSG_PROPERTY_ONLY_SUPPORTS_THE_FOLLOWING_OPERATIONS.formatted(searchCriteria.getProperty(),
@@ -323,18 +324,36 @@ private Predicate<PolicyWithBpn> getCreatedOnFilter(final SearchCriteria<?> sear
         private Predicate<PolicyWithBpn> getValidUntilFilter(final SearchCriteria<?> searchCriteria) {
             return switch (searchCriteria.getOperation()) {
                 case BEFORE_LOCAL_DATE -> p -> {
-                    final OffsetDateTime createdOn = p.policy().getValidUntil();
-                    return isDateBefore(createdOn, searchCriteria.getValue().toString());
+                    final OffsetDateTime date = p.policy().getValidUntil();
+                    return isDateBefore(searchCriteria, date);
                 };
                 case AFTER_LOCAL_DATE -> p -> {
-                    final OffsetDateTime createdOn = p.policy().getValidUntil();
-                    return isDateAfter(createdOn, searchCriteria.getValue().toString());
+                    final OffsetDateTime date = p.policy().getValidUntil();
+                    return isDateAfter(searchCriteria, date);
                 };
                 default -> throw new IllegalArgumentException(
                         MSG_PROPERTY_ONLY_SUPPORTS_THE_FOLLOWING_OPERATIONS.formatted(searchCriteria.getProperty(),
                                 List.of(BEFORE_LOCAL_DATE, AFTER_LOCAL_DATE)));
             };
         }
+
+        private boolean isDateAfter(final SearchCriteria<?> searchCriteria, final OffsetDateTime date) {
+            try {
+                return DateUtils.isDateAfter(date, searchCriteria.getValue().toString());
+            } catch (IllegalArgumentException e) {
+                throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                        "Search by property '%s': %s".formatted(searchCriteria.getProperty(), e.getMessage()), e);
+            }
+        }
+
+        private boolean isDateBefore(final SearchCriteria<?> searchCriteria, final OffsetDateTime date) {
+            try {
+                return DateUtils.isDateBefore(date, searchCriteria.getValue().toString());
+            } catch (IllegalArgumentException e) {
+                throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
+                        "Search by property '%s': %s".formatted(searchCriteria.getProperty(), e.getMessage()), e);
+            }
+        }
     }
 }
 

irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java

@@ -20,16 +20,21 @@
 package org.eclipse.tractusx.irs.policystore.common;
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
 
 import java.time.LocalDate;
 import java.time.LocalTime;
 import java.time.OffsetDateTime;
 import java.time.ZoneOffset;
+import java.time.format.DateTimeParseException;
 import java.util.stream.Stream;
 
+import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.junit.jupiter.params.provider.NullSource;
+import org.junit.jupiter.params.provider.ValueSource;
 
 class DateUtilsTest {
 
@@ -64,4 +69,32 @@ static Stream<Arguments> provideDatesForIsDateAfter() {
                 Arguments.of(referenceDateTime, "2023-07-06", false));
     }
 
-}
+    @ParameterizedTest
+    @ValueSource(strings = { "3333-11-11T11:11:11.111Z",
+                             "3333-11-",
+                             "2222",
+                             "asdf"
+    })
+    void testInvalidDate(final String referenceDateStr) {
+        final ThrowingCallable call = () -> DateUtils.isDateAfter(OffsetDateTime.now(), referenceDateStr);
+        assertThatThrownBy(call).isInstanceOf(IllegalArgumentException.class)
+                                .hasMessageContaining("Invalid date")
+                                .hasMessageContaining("refer to the documentation")
+                                .hasCauseInstanceOf(DateTimeParseException.class);
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "   \t",
+                             " ",
+                             ""
+    })
+    @NullSource
+    void testInvalidDateBlank(final String referenceDateStr) {
+        final ThrowingCallable call = () -> DateUtils.isDateAfter(OffsetDateTime.now(), referenceDateStr);
+        assertThatThrownBy(call).isInstanceOf(IllegalArgumentException.class)
+                                .hasMessageContaining("Invalid date")
+                                .hasMessageContaining("must not be blank")
+                                .hasNoCause();
+    }
+
+}