From 865936f3415b162331a0f8934b74db73b61b6363 Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 13:51:52 +0200 Subject: [PATCH 1/6] chore(documentation): [#639] improve documentation - harmonize description of GET /irs/policies and /irs/policies/paged - describe supported attributes in filter and sort - document supported search operators and value format - document default and max page size --- docs/src/api/irs-api.yaml | 31 ++++++++------- .../controllers/PolicyStoreController.java | 38 ++++++++++++++----- 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/docs/src/api/irs-api.yaml b/docs/src/api/irs-api.yaml index fadd254c3..287cbc4bd 100644 --- a/docs/src/api/irs-api.yaml +++ b/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=,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],`. - Example: `search=BPN,STARTS_WITH,BPNL12&search=policyId,STARTS_WITH,policy2`. - - - **Sorting:** - `sort=,[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=,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],`.\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}: diff --git a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java index 62384f48f..beb077c6a 100644 --- a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java +++ b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreController.java @@ -262,20 +262,38 @@ public List 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=,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],`. + \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=,[EQUALS|STARTS_WITH|BEFORE_LOCAL_DATE|AFTER_LOCAL_DATE],`. - 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=,[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 }, // From 7cab33b3caa6e5052c7c148c36368d985f6ac2e0 Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:28:34 +0200 Subject: [PATCH 2/6] chore(exception-handling): [#639] improve exception handling concerning invalid date format in search parameters --- .../irs/policystore/common/DateUtils.java | 18 ++++++++- .../services/PolicyPagingService.java | 39 ++++++++++++++----- .../irs/policystore/common/DateUtilsTest.java | 32 ++++++++++++++- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java index 6665d74e8..07f1daf0c 100644 --- a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java +++ b/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)"); + } } } diff --git a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyPagingService.java b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyPagingService.java index 3f46dd833..19b77a107 100644 --- a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyPagingService.java +++ b/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,6 +42,7 @@ 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; @@ -51,7 +50,9 @@ 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 getActionFilter(final SearchCriteria searchC private Predicate 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 getCreatedOnFilter(final SearchCriteria sear private Predicate 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); + } + } } } diff --git a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java index a1032c03e..8a6c6f2b4 100644 --- a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java +++ b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java @@ -20,6 +20,7 @@ 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; @@ -27,9 +28,12 @@ import java.time.ZoneOffset; 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 +68,30 @@ static Stream provideDatesForIsDateAfter() { Arguments.of(referenceDateTime, "2023-07-06", false)); } -} \ No newline at end of file + @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"); + } + + @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"); + } + +} From 80442564940ce29ec09c7976c287e2a43e312435 Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:40:28 +0200 Subject: [PATCH 3/6] chore(exception-handling): [#639] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c6ec2c0..e2f156eb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ _**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 + ## [5.3.0] - 2024-07-15 ### Added From 6870d01c94a766e8b6dc0f77c952f90168d53897 Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:41:55 +0200 Subject: [PATCH 4/6] chore(documentation): [#639] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c6ec2c0..0bca8a354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ _**For better traceability add the corresponding GitHub issue number in each cha ## [Unreleased] +### Changed + +- Improved documentation for `GET /irs/policies/paged` endpoint. #639 + ## [5.3.0] - 2024-07-15 ### Added From 7ef4abb037eecad41315b3e2d6de47cec87d94cf Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:43:34 +0200 Subject: [PATCH 5/6] chore(exception-handling): [#639] update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e2f156eb1..a1f9b4553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha ### Fixed -- Improved exception handling concerning invalid date format in search parameters for GET /irs/policies/paged. #639 +- Improved exception handling concerning invalid date format in search parameters for `GET /irs/policies/paged`. #639 ## [5.3.0] - 2024-07-15 From 93bee2648f3eeebdceccd4c057825710a1cf2b6b Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 15:07:14 +0200 Subject: [PATCH 6/6] chore(exception-handling): [#639] Resolve PMD warning by preserving the cause in exception handling --- .../eclipse/tractusx/irs/policystore/common/DateUtils.java | 2 +- .../tractusx/irs/policystore/common/DateUtilsTest.java | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java index 07f1daf0c..11f5c8eb4 100644 --- a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java +++ b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java @@ -59,7 +59,7 @@ private static LocalDate parseDate(final String dateString) { try { return LocalDate.parse(dateString); } catch (DateTimeParseException e) { - throw new IllegalArgumentException("Invalid date format (please refer to the documentation)"); + throw new IllegalArgumentException("Invalid date format (please refer to the documentation)", e); } } } diff --git a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java index 8a6c6f2b4..3d2ebd8f4 100644 --- a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java +++ b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java @@ -26,6 +26,7 @@ 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; @@ -78,7 +79,8 @@ 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"); + .hasMessageContaining("refer to the documentation") + .hasCauseInstanceOf(DateTimeParseException.class); } @ParameterizedTest @@ -91,7 +93,8 @@ 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"); + .hasMessageContaining("must not be blank") + .hasNoCause(); } }