diff --git a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java index cd276d9c33..fe62a2eb9f 100644 --- a/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java +++ b/irs-policy-store/src/main/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreService.java @@ -37,6 +37,7 @@ import java.util.Optional; import java.util.TreeSet; import java.util.stream.Collectors; +import java.util.stream.Stream; import jakarta.json.JsonObject; import lombok.extern.slf4j.Slf4j; @@ -95,7 +96,7 @@ private interface ConfiguredDefaultPolicy { private static final String MISSING_REQUEST_FIELD_MESSAGE = "Request does not contain all required fields. " + "Missing: %s"; - private static final String DEFAULT = "default"; + static final String DEFAULT = "default"; public PolicyStoreService(final DefaultAcceptedPoliciesConfig defaultAcceptedPoliciesConfig, final PolicyPersistence persistence, final EdcTransformer edcTransformer, final Clock clock) { @@ -178,6 +179,12 @@ public List getStoredPolicies(final List bpnls) { storedPolicies.addAll(persistence.readAll(bpn)); } + // Policies not associated with a BPN (default policies) should only be returned + // if there are no policies that are registered for this BPN explicitly (see #199). + if (storedPolicies.isEmpty()) { + storedPolicies.addAll(persistence.readAll(DEFAULT_BLOB_NAME)); + } + if (storedPolicies.isEmpty()) { log.info("Policy store is empty, returning default values from config"); return allowedPoliciesFromConfig; @@ -276,22 +283,29 @@ private Optional findPolicy(final String policyId, final List bp @Override public List getAcceptedPolicies(final String bpn) { + if (bpn == null) { - return getAllStoredPolicies().values() - .stream() - .flatMap(Collection::stream) - .map(this::toAcceptedPolicy) - .toList(); + return getAllPolicies(); } - final ArrayList policies = new ArrayList<>(); - policies.addAll(getStoredPolicies(List.of(bpn))); - policies.addAll(getStoredPolicies(List.of(DEFAULT_BLOB_NAME))); + final List storedPolicies = getStoredPolicies(List.of(bpn)); + final Stream result = sortByPolicyId(storedPolicies); + return result.map(this::toAcceptedPolicy).toList(); + } + + private static Stream sortByPolicyId(final List policies) { final TreeSet result = new TreeSet<>(Comparator.comparing(Policy::getPolicyId)); result.addAll(policies); + return result.stream(); + } - return result.stream().map(this::toAcceptedPolicy).toList(); + private List getAllPolicies() { + return getAllStoredPolicies().values() + .stream() + .flatMap(Collection::stream) + .map(this::toAcceptedPolicy) + .toList(); } private AcceptedPolicy toAcceptedPolicy(final Policy policy) { @@ -300,6 +314,7 @@ private AcceptedPolicy toAcceptedPolicy(final Policy policy) { private List createDefaultPolicyFromConfig( final DefaultAcceptedPoliciesConfig defaultAcceptedPoliciesConfig) { + final List constraints = new ArrayList<>(); defaultAcceptedPoliciesConfig.getAcceptedPolicies() .forEach(acceptedPolicy -> constraints.add( diff --git a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java index b6eb45aee0..40bbcf386e 100644 --- a/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java +++ b/irs-policy-store/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java @@ -49,6 +49,7 @@ import org.eclipse.edc.core.transform.TypeTransformerRegistryImpl; import org.eclipse.edc.jsonld.TitaniumJsonLd; import org.eclipse.edc.spi.monitor.ConsoleMonitor; +import org.eclipse.tractusx.irs.edc.client.policy.AcceptedPolicy; import org.eclipse.tractusx.irs.edc.client.policy.Constraint; import org.eclipse.tractusx.irs.edc.client.policy.Constraints; import org.eclipse.tractusx.irs.edc.client.policy.Operator; @@ -90,6 +91,7 @@ class PolicyStoreServiceTest { @Captor private ArgumentCaptor policyCaptor; + @Captor private ArgumentCaptor bpnCaptor; @@ -129,7 +131,7 @@ void registerPolicy_withBpnNull_shouldStoreAsDefault() { } @Test - void registerPolicy() { + void registerPolicy_success() { // ARRANGE final OffsetDateTime now = OffsetDateTime.now(); @@ -243,10 +245,10 @@ void doRegisterPolicy_withMissingConstraintShouldThrowException() { } @Nested - class GetPoliciesTests { + class GetStoredPoliciesTests { @Test - void getStoredPolicies() { + void getStoredPolicies_shouldReturnAllPoliciesStoredForTheBpn() { // ARRANGE final List policies = List.of(createPolicy("test1"), createPolicy("test2"), createPolicy("test3")); @@ -260,9 +262,11 @@ void getStoredPolicies() { } @Test - void getDefaultStoredPoliciesWhenEmpty() { + void getStoredPolicies_whenNoPoliciesForBpn_shouldReturnTheConfiguredDefaultPolicies() { // ARRANGE + + // default policy configuration final DefaultAcceptedPoliciesConfig.AcceptedPolicy acceptedPolicy1 = new DefaultAcceptedPoliciesConfig.AcceptedPolicy( EXAMPLE_ACCEPTED_LEFT_OPERAND, "eq", EXAMPLE_ALLOWED_NAME); final DefaultAcceptedPoliciesConfig.AcceptedPolicy acceptedPolicy2 = new DefaultAcceptedPoliciesConfig.AcceptedPolicy( @@ -284,9 +288,13 @@ void getDefaultStoredPoliciesWhenEmpty() { assertThat(constraints.getOr()).hasSize(2); assertThat(constraints.getAnd()).hasSize(2); } + } + + @Nested + class GetAcceptedPoliciesTests { @Test - void shouldReturnDefaultPolicyWhenBpnIsEmpty() { + void getAcceptedPolicies_whenParameterBpnIsNull_shouldReturnTheConfiguredDefaultPolicy() { // ARRANGE when(persistenceMock.readAll()).thenReturn(emptyMap()); @@ -295,7 +303,35 @@ void shouldReturnDefaultPolicyWhenBpnIsEmpty() { final var acceptedPolicies = testee.getAcceptedPolicies(null); // ASSERT - assertThat(acceptedPolicies.get(0).policy().getPolicyId()).isEqualTo("default-policy"); + final String policyIdOfConfiguredDefaultPolicy = "default-policy"; + assertThat(acceptedPolicies.get(0).policy().getPolicyId()).isEqualTo(policyIdOfConfiguredDefaultPolicy); + } + + @Test + void getAcceptedPolicies_whenNoPoliciesAssociatedWithTheGivenBpn_shouldReturnTheRegisteredDefaultPolicies() { + + // ARRANGE + when(persistenceMock.readAll(BPN)).thenReturn(emptyList()); + + // policy registered without BPN should be used as default policy (see #199) + // this overrides the configured default policy (see the previous test above) + final String defaultPolicyId1 = "registered-default-policy-1"; + final String defaultPolicyId2 = "registered-default-policy-2"; + when(persistenceMock.readAll(PolicyStoreService.DEFAULT)).thenReturn(List.of( + // default policy 1 + createPolicy(defaultPolicyId1), + // default policy 2 + createPolicy(defaultPolicyId2))); + + // ACT + final var acceptedPolicies = testee.getAcceptedPolicies(BPN); + + // ASSERT + final List policyIds = acceptedPolicies.stream() + .map(AcceptedPolicy::policy) + .map(Policy::getPolicyId) + .toList(); + assertThat(policyIds).containsExactlyInAnyOrder(defaultPolicyId1, defaultPolicyId2); } } @@ -320,7 +356,7 @@ private Constraints createConstraints() { class DeletePolicyTests { @Test - void deletePolicy_success() { + void deletePolicy_deleteSuccessful() { // ARRANGE when(persistenceMock.readAll()).thenReturn(Map.of(BPN, List.of(new Policy("testId", null, null, null)))); @@ -332,7 +368,7 @@ void deletePolicy_success() { } @Test - void deletePolicy_internalServerError() { + void deletePolicy_exceptionFromPolicyPersistence_shouldReturnHttpStatus500() { // ACT final String policyId = "testId"; @@ -345,7 +381,7 @@ void deletePolicy_internalServerError() { } @Test - void deletePolicy_policyNotFound() { + void deletePolicy_whenPolicyNotFound_shouldReturnHttpStatus404() { // ACT final String policyId = "notExistingPolicyId"; @@ -360,10 +396,10 @@ void deletePolicy_policyNotFound() { } @Nested - class UpdatePolicyTests { + class UpdatePoliciesTests { @Test - void updatePolicy_withBpnAndValidUntilChanged() { + void updatePolicies_shouldUpdateBpnAndValidUntil() { // ARRANGE final String policyId = "testId"; @@ -392,7 +428,6 @@ void updatePolicy_withBpnAndValidUntilChanged() { assertThat(policyCaptor.getValue().getValidUntil()).isEqualTo(expectedValidUntil); } - @SuppressWarnings("unchecked") @Test void updatePolicies_shouldAddPolicyToEachBpn() { @@ -426,7 +461,6 @@ void updatePolicies_shouldAddPolicyToEachBpn() { assertThat(bpnCaptor.getAllValues().get(1)).isEqualTo("bpn2"); } - @SuppressWarnings("unchecked") @Test void updatePolicies_shouldAssociateEachGivenPolicyWithEachGivenBpn() { @@ -477,7 +511,7 @@ void updatePolicies_shouldAssociateEachGivenPolicyWithEachGivenBpn() { } @Test - void updatePolicy_shouldThrowResponseStatusException() { + void updatePolicies_exceptionFromPolicyPersistence_shouldReturnHttpStatus500() { // ARRANGE final String policyId = "testId";