Skip to content

Commit

Permalink
Merge pull request #835 from dsmf/feat/734-handle-trial-to-modify-rea…
Browse files Browse the repository at this point in the history 
…donly-default-policies

feat(impl): [#734] Handling for modification attempts on read-only de…
  • Loading branch information
@ds-jhartmann
ds-jhartmann authored Jul 23, 2024
2 parents 37f9b24 + b5c75da commit 3eeee6a
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha

### Changed
- Default policies are now configured using JSON in accordance with the ODRL schema. #542
- Improved the exception handling for modification attempts on read-only default policies. Such actions now result in a 400 BAD REQUEST response with a user-friendly error message. #734

## [5.3.0] - 2024-07-15

Expand Down
30 changes: 30 additions & 0 deletions ...n/java/org/eclipse/tractusx/irs/policystore/exceptions/ResourceDoesNotExistException.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/********************************************************************************
* Copyright (c) 2022,2024 Bayerische Motoren Werke Aktiengesellschaft (BMW AG)
* Copyright (c) 2021,2024 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Apache License, Version 2.0 which is available at
* https://www.apache.org/licenses/LICENSE-2.0.
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/
package org.eclipse.tractusx.irs.policystore.exceptions;

/**
* Resource does not exist exception.
*/
public class ResourceDoesNotExistException extends PolicyStoreException {

public ResourceDoesNotExistException(final String msg) {
super(msg);
}
}
3 changes: 2 additions & 1 deletion ...ore/src/main/java/org/eclipse/tractusx/irs/policystore/persistence/PolicyPersistence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.eclipse.tractusx.irs.common.persistence.BlobPersistenceException;
import org.eclipse.tractusx.irs.edc.client.policy.Policy;
import org.eclipse.tractusx.irs.policystore.exceptions.PolicyStoreException;
import org.eclipse.tractusx.irs.policystore.exceptions.ResourceDoesNotExistException;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Service;

Expand Down Expand Up @@ -89,7 +90,7 @@ public void delete(final String bpn, final String policyId) {
final var policies = readAll(bpn);
final var modifiedPolicies = policies.stream().filter(p -> !p.getPolicyId().equals(policyId)).toList();
if (policies.size() == modifiedPolicies.size()) {
throw new PolicyStoreException("Policy with id '" + policyId + "' doesn't exists!");
throw new ResourceDoesNotExistException("Policy with id '%s' doesn't exists!".formatted(policyId));
}
save(bpn, modifiedPolicies);
}
Expand Down
35 changes: 30 additions & 5 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 @@ -49,6 +49,7 @@
import org.eclipse.tractusx.irs.edc.client.transformer.EdcTransformer;
import org.eclipse.tractusx.irs.policystore.config.DefaultAcceptedPoliciesConfig;
import org.eclipse.tractusx.irs.policystore.exceptions.PolicyStoreException;
import org.eclipse.tractusx.irs.policystore.exceptions.ResourceDoesNotExistException;
import org.eclipse.tractusx.irs.policystore.models.CreatePolicyRequest;
import org.eclipse.tractusx.irs.policystore.models.UpdatePolicyRequest;
import org.eclipse.tractusx.irs.policystore.persistence.PolicyPersistence;
Expand All @@ -63,7 +64,8 @@
@Service
@Slf4j
@SuppressWarnings({ "PMD.ExcessiveImports",
"PMD.TooManyMethods"
"PMD.TooManyMethods",
"PMD.GodClass"
})
public class PolicyStoreService implements AcceptedPoliciesProvider {

Expand Down Expand Up @@ -107,6 +109,8 @@ public Policy registerPolicy(final CreatePolicyRequest request) {
final Policy policy = edcTransformer.transformToIrsPolicy(policyJson);
policy.setValidUntil(request.validUntil());

handleTrialToModifyFallbackPolicy(policy.getPolicyId());

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

Expand Down Expand Up @@ -180,36 +184,45 @@ private static boolean containsNoDefaultPolicy(final Map<String, List<Policy>> b

public void deletePolicy(final String policyId) {

handleTrialToModifyFallbackPolicy(policyId);

log.info("Getting all policies to find correct BPN");
final List<String> bpnsContainingPolicyId = PolicyHelper.findBpnsByPolicyId(getAllStoredPolicies(), policyId);

if (bpnsContainingPolicyId.isEmpty()) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND,
"Policy with id '%s' not found".formatted(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.");
} else {
try {
log.info("Deleting policy with id {}", policyId);
bpnsContainingPolicyId.forEach(bpn -> persistence.delete(bpn, policyId));
} catch (final ResourceDoesNotExistException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, e.getMessage(), e);
} catch (final PolicyStoreException e) {
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage(), e);
}
}
}

public void deletePolicyForEachBpn(final String policyId, final List<String> bpnList) {

handleTrialToModifyFallbackPolicy(policyId);

try {
for (final String bpn : bpnList) {
persistence.delete(bpn, policyId);
}
} catch (final ResourceDoesNotExistException e) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, e.getMessage(), e);
} catch (final PolicyStoreException e) {
throw new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, e.getMessage(), e);
}
}

private boolean isFallbackPolicyFromConfiguration(final String policyId) {
return this.allowedPoliciesFromConfig.stream().map(Policy::getPolicyId).anyMatch(p -> p.equals(policyId));
}

public void updatePolicies(final UpdatePolicyRequest request) {
for (final String policyId : request.policyIds()) {
updatePolicy(policyId, request.validUntil(),
Expand All @@ -220,6 +233,8 @@ public void updatePolicies(final UpdatePolicyRequest request) {
public void updatePolicy(final String policyId, final OffsetDateTime newValidUntil,
final List<String> newBusinessPartnerNumbers) {

handleTrialToModifyFallbackPolicy(policyId);

log.info("Updating policy with id {}", policyId);

final List<String> businessPartnerNumbersContainingPolicyId = findBusinessPartnerNumbersByPolicyId(policyId);
Expand Down Expand Up @@ -304,4 +319,14 @@ private List<Policy> createDefaultPolicyFromConfig(final DefaultAcceptedPolicies
return StringMapper.mapFromBase64String(defaultPoliciesConfig.getAcceptedPolicies(), LIST_OF_POLICIES_TYPE);
}

private void handleTrialToModifyFallbackPolicy(final String policyId) {
if (isFallbackPolicyFromConfiguration(policyId)) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST,
("The policy '%s' is a configured default policy which cannot be modified. "
+ "However you can define custom default policies via the API by registering a policy "
+ "without a business partner number. "
+ "These will take precedence over configured default policies.").formatted(policyId));
}
}

}
5 changes: 3 additions & 2 deletions ...test/java/org/eclipse/tractusx/irs/policystore/controllers/PolicyStoreControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public class PolicyStoreControllerTest {
"@context": {
"odrl": "http://www.w3.org/ns/odrl/2/"
},
"@id": "e917f5f-8dac-49ac-8d10-5b4d254d2b48",
"@id": "%s",
"policy": {
"odrl:permission": [
{
Expand Down Expand Up @@ -118,7 +118,8 @@ class RegisterPolicyTests {
void registerAllowedPolicy() {
// arrange
final OffsetDateTime now = OffsetDateTime.now();
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(REGISTER_POLICY_EXAMPLE_PAYLOAD);
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(
REGISTER_POLICY_EXAMPLE_PAYLOAD.formatted("e917f5f-8dac-49ac-8d10-5b4d254d2b48"));

// act
final CreatePolicyRequest request = new CreatePolicyRequest(now.plusMinutes(1), null,
Expand Down
66 changes: 62 additions & 4 deletions ...e/src/test/java/org/eclipse/tractusx/irs/policystore/services/PolicyStoreServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,19 +149,38 @@ void setUp() {
@DisplayName("registerPolicy")
class RegisterPolicyTests {

@Test
void registerPolicy_whenFallbackPolicyId_shouldReturnHttpStatus400() {
final JsonObject policy = PolicyStoreTestUtil.toJsonObject(
REGISTER_POLICY_EXAMPLE_PAYLOAD.formatted(CONFIGURED_DEFAULT_POLICY_ID));
assertThatThrownBy(() -> testee.registerPolicy(CreatePolicyRequest.builder()
.businessPartnerNumber("BPNL1234567890AB")
.validUntil(OffsetDateTime.now(clock)
.plusYears(2))
.payload(policy)
.build())).isInstanceOf(
ResponseStatusException.class)
.hasMessageContaining(
"400 BAD_REQUEST")
.hasMessageContaining(
CONFIGURED_DEFAULT_POLICY_ID);
}

@Test
void registerPolicy_withBpnNull_shouldStoreAsDefault() {

// ARRANGE
final OffsetDateTime now = OffsetDateTime.now();
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(REGISTER_POLICY_EXAMPLE_PAYLOAD);
final String policyId = "e917f5f-8dac-49ac-8d10-5b4d254d2b48";
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(
REGISTER_POLICY_EXAMPLE_PAYLOAD.formatted(policyId));

// ACT
testee.registerPolicy(new CreatePolicyRequest(now, null, jsonObject));

// ASSERT
verify(persistenceMock).save(eq(PolicyStoreService.BPN_DEFAULT), policyCaptor.capture());
assertThat(policyCaptor.getValue().getPolicyId()).isEqualTo("e917f5f-8dac-49ac-8d10-5b4d254d2b48");
assertThat(policyCaptor.getValue().getPolicyId()).isEqualTo(policyId);
assertThat(policyCaptor.getValue().getValidUntil()).isEqualTo(now);
assertThat(policyCaptor.getValue().getPermissions()).hasSize(1);
}
Expand All @@ -171,7 +190,9 @@ void registerPolicy_success() {

// ARRANGE
final OffsetDateTime now = OffsetDateTime.now();
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(REGISTER_POLICY_EXAMPLE_PAYLOAD);
final String policyId = "e917f5f-8dac-49ac-8d10-5b4d254d2b48";
final JsonObject jsonObject = PolicyStoreTestUtil.toJsonObject(
REGISTER_POLICY_EXAMPLE_PAYLOAD.formatted(policyId));

// ACT
final OffsetDateTime validUntil = now.plusMonths(1);
Expand All @@ -182,7 +203,7 @@ void registerPolicy_success() {

// ASSERT
verify(persistenceMock).save(eq("BPNL00000123ABCD"), policyCaptor.capture());
assertThat(policyCaptor.getValue().getPolicyId()).isEqualTo("e917f5f-8dac-49ac-8d10-5b4d254d2b48");
assertThat(policyCaptor.getValue().getPolicyId()).isEqualTo(policyId);
assertThat(policyCaptor.getValue().getValidUntil()).isEqualTo(validUntil);
assertThat(policyCaptor.getValue().getPermissions()).isNotEmpty();

Expand Down Expand Up @@ -538,11 +559,48 @@ void deletePolicy_whenPolicyNotFound_shouldReturnHttpStatus404() {
notExistingPolicyId);
}

@Test
void deletePolicy_whenFallbackPolicy_shouldReturnHttpStatus400() {
assertThatThrownBy(() -> testee.deletePolicy(CONFIGURED_DEFAULT_POLICY_ID)).isInstanceOf(
ResponseStatusException.class)
.hasMessageContaining(
"400 BAD_REQUEST")
.hasMessageContaining(
CONFIGURED_DEFAULT_POLICY_ID);
}

@Test
void deletePolicyForEachBpn_whenFallbackPolicy_shouldReturnHttpStatus400() {
assertThatThrownBy(() -> testee.deletePolicyForEachBpn(CONFIGURED_DEFAULT_POLICY_ID,
List.of("BPNL1234567890AB", "BPNL1234567890CD"))).isInstanceOf(ResponseStatusException.class)
.hasMessageContaining("400 BAD_REQUEST")
.hasMessageContaining(
CONFIGURED_DEFAULT_POLICY_ID);
}

}

@Nested
class UpdatePoliciesTests {

@Test
void updatePolicies_whenFallbackPolicy_shouldReturnHttpStatus400() {
assertThatThrownBy(() -> testee.updatePolicies(
new UpdatePolicyRequest(OffsetDateTime.now(clock), List.of("BPNL1234567890AB", "BPNL1234567890CD"),
List.of(CONFIGURED_DEFAULT_POLICY_ID)))).isInstanceOf(ResponseStatusException.class)
.hasMessageContaining("400 BAD_REQUEST")
.hasMessageContaining(CONFIGURED_DEFAULT_POLICY_ID);
}

@Test
void updatePolicy_whenFallbackPolicy_shouldReturnHttpStatus400() {
assertThatThrownBy(() -> testee.updatePolicy(CONFIGURED_DEFAULT_POLICY_ID, OffsetDateTime.now(clock),
List.of("BPNL1234567890AB", "BPNL1234567890CD"))).isInstanceOf(ResponseStatusException.class)
.hasMessageContaining("400 BAD_REQUEST")
.hasMessageContaining(
CONFIGURED_DEFAULT_POLICY_ID);
}

@Test
void updatePolicies_shouldUpdateBpnAndValidUntil() {
// ARRANGE
Expand Down

0 comments on commit 3eeee6a

Please sign in to comment.