Skip to content

Commit

Permalink
Merge branch 'main' into feat/639-support-any-iso-date
Browse files Browse the repository at this point in the history 
# Conflicts:
#	irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java
#	irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java
  • Loading branch information
@dsmf
dsmf committed Jul 23, 2024
2 parents ebff884 + cfd8475 commit 3a8f43e
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 88 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,18 @@ _**For better traceability add the corresponding GitHub issue number in each cha

## [Unreleased]

### Fixed

- Fixed already merged implementation from _"Access and Usage Policy Validation flow correction. #757"_
where fallback to default policy was not implemented correctly.
- Improved exception handling concerning invalid date format in search parameters for `GET /irs/policies/paged`. #639

### Changed

- The date search operators `AFTER_LOCAL_DATE` and `BEFORE_LOCAL_DATE` for fields `createdOn` and `validUntil` support any ISO date time now (relates to #639 and #750).
- Improved documentation for `GET /irs/policies/paged` endpoint. #639
- Cleanup in IrsApplicationTest.generatedOpenApiMatchesContract
(removed obsolete ignoringFields, improved assertion message)

## [5.4.0] - 2024-07-22

Expand Down
14 changes: 2 additions & 12 deletions irs-api/src/test/java/org/eclipse/tractusx/irs/IrsApplicationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,24 +90,14 @@ void generatedOpenApiMatchesContract() throws Exception {
final Map<String, Object> generatedYamlMap = mapper.readerForMapOf(Object.class).readValue(generatedYaml);

try {
// To correctly display both documentations examples - manual and generated by annotations -
// we need to remove verification for some "examples", otherwise one or another won't display correctly
assertThat(generatedYamlMap)

.usingRecursiveComparison()

.ignoringFields("components.schemas.PageResult.example")
.ignoringFields("components.schemas.AspectModels.example")
.ignoringFields("components.schemas.BatchOrderResponse.example")
.ignoringFields("components.schemas.Jobs.example")
.ignoringFields("components.schemas.Policy")
.ignoringFields("components.schemas.BatchResponse.example")

.isEqualTo(definedYamlMap);
} catch (AssertionError e) {
// write changed API to file for easier comparison
Files.writeString(Paths.get("../docs/src/api/irs-api.actual.yaml"), generatedYaml);
throw e;
throw new AssertionError("Please compare the generated irs-api.actual.yaml "
+ "with irs-api.yaml to find the differences easily!", e);
}

}
Expand Down
16 changes: 14 additions & 2 deletions irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/common/DateUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.time.format.DateTimeParseException;

import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;

/**
* Date utilities.
Expand Down Expand Up @@ -57,11 +58,22 @@ 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);
}
}

@SuppressWarnings("PMD.PreserveStackTrace") // this is intended here as we try to parse with different formats
Expand Down
39 changes: 29 additions & 10 deletions ...tore/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyPagingService.java
View file
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);
}
}
}
}

50 changes: 27 additions & 23 deletions ...store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
********************************************************************************/
package org.eclipse.tractusx.irs.policystore.services;

import static org.apache.commons.collections.CollectionUtils.isNotEmpty;
import static org.eclipse.tractusx.irs.common.persistence.BlobPersistence.DEFAULT_BLOB_NAME;

import java.time.Clock;
Expand Down Expand Up @@ -77,22 +78,10 @@ public class PolicyStoreService implements AcceptedPoliciesProvider {

private final Clock clock;

private static final String DEFAULT = "default";

/**
* Constants for the configured default policy.
* Under this "virtual" BPN custom default policies are stored
*/
private static final class ConfiguredDefaultPolicy {
/**
* ID for default policy (see TRI-1594)
*/
public static final String DEFAULT_POLICY_ID = "default-policy";

/**
* Lifetime for default policy in years (see TRI-1594)
*/
public static final int DEFAULT_POLICY_LIFETIME_YEARS = 5;
}
/* package */ static final String BPN_DEFAULT = "default";

public PolicyStoreService(final DefaultAcceptedPoliciesConfig defaultAcceptedPoliciesConfig,
final PolicyPersistence persistence, final EdcTransformer edcTransformer, final Clock clock) {
Expand All @@ -119,7 +108,7 @@ public Policy registerPolicy(final CreatePolicyRequest request) {
policy.setValidUntil(request.validUntil());

registeredPolicy = doRegisterPolicy(policy,
request.businessPartnerNumber() == null ? DEFAULT : request.businessPartnerNumber());
request.businessPartnerNumber() == null ? BPN_DEFAULT : request.businessPartnerNumber());

return registeredPolicy;
}
Expand Down Expand Up @@ -152,10 +141,10 @@ public Map<String, List<Policy>> getPolicies(final List<String> bpnList) {
}
}

public List<Policy> getStoredPolicies(final List<String> bpnls) {
log.info("Reading all stored polices for BPN {}", bpnls);
public List<Policy> getStoredPolicies(final List<String> businessPartnerNumbers) {
log.info("Reading all stored polices for BPN {}", businessPartnerNumbers);
final List<Policy> storedPolicies = new LinkedList<>();
for (final String bpn : bpnls) {
for (final String bpn : businessPartnerNumbers) {
storedPolicies.addAll(persistence.readAll(bpn));
}

Expand All @@ -180,13 +169,13 @@ public List<AcceptedPolicy> getConfiguredDefaultPolicies() {
public Map<String, List<Policy>> getAllStoredPolicies() {
final Map<String, List<Policy>> bpnToPolicies = persistence.readAll();
if (containsNoDefaultPolicy(bpnToPolicies)) {
bpnToPolicies.put(DEFAULT, allowedPoliciesFromConfig);
bpnToPolicies.put(BPN_DEFAULT, allowedPoliciesFromConfig);
}
return bpnToPolicies;
}

private static boolean containsNoDefaultPolicy(final Map<String, List<Policy>> bpnToPolicies) {
return bpnToPolicies.keySet().stream().noneMatch(key -> StringUtils.isEmpty(key) || DEFAULT.equals(key));
return bpnToPolicies.keySet().stream().noneMatch(key -> StringUtils.isEmpty(key) || BPN_DEFAULT.equals(key));
}

public void deletePolicy(final String policyId) {
Expand All @@ -200,7 +189,7 @@ public void deletePolicy(final String policyId) {
} else if (bpnsContainingPolicyId.stream().noneMatch(StringUtils::isNotEmpty)) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, //
"A configured default policy cannot be deleted. "
+ "It can be overridden by defining a default policy via the API instead.");
+ "It can be overridden by defining a default policy via the API instead.");
} else {
try {
log.info("Deleting policy with id {}", policyId);
Expand All @@ -224,7 +213,7 @@ public void deletePolicyForEachBpn(final String policyId, final List<String> bpn
public void updatePolicies(final UpdatePolicyRequest request) {
for (final String policyId : request.policyIds()) {
updatePolicy(policyId, request.validUntil(),
request.businessPartnerNumbers() == null ? List.of(DEFAULT) : request.businessPartnerNumbers());
request.businessPartnerNumbers() == null ? List.of(BPN_DEFAULT) : request.businessPartnerNumbers());
}
}

Expand Down Expand Up @@ -278,14 +267,29 @@ private Optional<Policy> findPolicy(final String policyId, final List<String> bp
public List<AcceptedPolicy> getAcceptedPolicies(final String bpn) {

if (bpn == null) {
return getConfiguredDefaultPolicies();
return getCustomPolicyOrFallback();
}

final List<Policy> storedPolicies = getStoredPolicies(List.of(bpn));
final Stream<Policy> result = sortByPolicyId(storedPolicies);
return result.map(this::toAcceptedPolicy).toList();
}

/**
* Gets either the custom default policies from the database
* or the fallback default policies from the configuration.
*
* @return list of default policies
*/
private List<AcceptedPolicy> getCustomPolicyOrFallback() {
final List<Policy> defaultPoliciesFromDb = getAllStoredPolicies().get(BPN_DEFAULT);
if (isNotEmpty(defaultPoliciesFromDb)) {
return defaultPoliciesFromDb.stream().map(this::toAcceptedPolicy).toList();
} else {
return getConfiguredDefaultPolicies();
}
}

private static Stream<Policy> sortByPolicyId(final List<Policy> policies) {
final TreeSet<Policy> result = new TreeSet<>(Comparator.comparing(Policy::getPolicyId));
result.addAll(policies);
Expand Down
37 changes: 33 additions & 4 deletions ...policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/common/DateUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@
import java.time.LocalTime;
import java.time.OffsetDateTime;
import java.time.ZoneOffset;
import java.time.format.DateTimeParseException;
import java.util.stream.Stream;

import org.jetbrains.annotations.NotNull;
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
import org.junit.jupiter.api.Test;
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 @@ -60,7 +63,6 @@ static Stream<Arguments> provideDatesForIsDateBefore() {
);
}

@NotNull
private static OffsetDateTime atStartOfDay(final String date) {
return LocalDate.parse(date).atStartOfDay().atOffset(ZoneOffset.UTC);
}
Expand Down Expand Up @@ -90,7 +92,6 @@ static Stream<Arguments> provideDatesForIsDateAfter() {
);
}

@NotNull
private static OffsetDateTime atEndOfDay(final String dateStr) {
return LocalDate.parse(dateStr).atTime(LocalTime.MAX).atOffset(ZoneOffset.UTC);
}
Expand All @@ -115,4 +116,32 @@ public void testIsDateWithoutTimeWithInvalidDate() {
IllegalArgumentException.class).hasMessageContaining("Invalid date format: invalid-date");
}

}
@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

0 comments on commit 3a8f43e

Please sign in to comment.