From 7cab33b3caa6e5052c7c148c36368d985f6ac2e0 Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:28:34 +0200 Subject: [PATCH 1/4] 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 2/4] 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 7ef4abb037eecad41315b3e2d6de47cec87d94cf Mon Sep 17 00:00:00 2001 From: Matthias Fischer Date: Fri, 19 Jul 2024 14:43:34 +0200 Subject: [PATCH 3/4] 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 4/4] 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(); } }