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.

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

chore(exception-handling): [#639] improve exception handling concerni… #831

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

### Changed

- Improved documentation for `GET /irs/policies/paged` endpoint. #639
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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(),
Expand All @@ -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);
}
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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();
}

}
Loading