Skip to content

Commit

Permalink
Merge pull request #795 from eclipse-tractusx/fix/734-show-default-po…
Browse files Browse the repository at this point in the history
…licies-in-GET-policies-request

Fix/734 show default policies in get policies request
  • Loading branch information
ds-jhartmann authored Jul 12, 2024
2 parents 996b3ac + 1a52e53 commit 7d9f9c5
Show file tree
Hide file tree
Showing 9 changed files with 251 additions and 51 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ _**For better traceability add the corresponding GitHub issue number in each cha
### Fixed

- Access and Usage Policy Validation flow correction. #757
- GET /irs/policies and GET /irs/policies/paged return the configured default policies if no custom default policy is defined now. #734
- IRS Policy Validation does not accept subset of AND constraint any longer. #649

### Changed
Expand Down
6 changes: 4 additions & 2 deletions docs/src/api/irs-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,8 @@ paths:
description: Lists the registered policies that should be accepted in EDC negotiation.
operationId: getAllowedPoliciesByBpn
parameters:
- description: List of business partner numbers.
- description: List of business partner numbers. This may also contain the value
"default" in order to query the default policies.
in: query
name: businessPartnerNumbers
required: false
Expand Down Expand Up @@ -988,7 +989,8 @@ paths:
Example: `page=1&size=20`
operationId: getPoliciesPaged
parameters:
- description: List of business partner numbers.
- description: List of business partner numbers. This may also contain the value
"default" in order to query the default policies.
in: query
name: businessPartnerNumbers
required: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,9 @@ public CreatePoliciesResponse registerAllowedPolicy(@Valid @RequestBody final Cr
@PreAuthorize("hasAuthority('" + IrsRoles.ADMIN_IRS + "')")
public Map<String, List<PolicyResponse>> getPolicies(//
@RequestParam(required = false) //
@ValidListOfBusinessPartnerNumbers //
@Parameter(description = "List of business partner numbers.") //
@ValidListOfBusinessPartnerNumbers(allowDefault = true) //
@Parameter(description = "List of business partner numbers. "
+ "This may also contain the value \"default\" in order to query the default policies.") //
final List<String> businessPartnerNumbers //
) {

Expand Down Expand Up @@ -256,8 +257,9 @@ public Page<PolicyResponse> getPoliciesPaged(//
@Parameter(description = "Page configuration", hidden = true) //
final Pageable pageable, //
@RequestParam(required = false) //
@ValidListOfBusinessPartnerNumbers //
@Parameter(name = "businessPartnerNumbers", description = "List of business partner numbers.") //
@ValidListOfBusinessPartnerNumbers(allowDefault = true) //
@Parameter(name = "businessPartnerNumbers", description = "List of business partner numbers. "
+ "This may also contain the value \"default\" in order to query the default policies.") //
final List<String> businessPartnerNumbers) {

if (pageable.getPageSize() > MAX_PAGE_SIZE) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,16 @@ public List<AcceptedPolicy> getConfiguredDefaultPolicies() {

public Map<String, List<Policy>> getAllStoredPolicies() {
final Map<String, List<Policy>> bpnToPolicies = persistence.readAll();
if (bpnToPolicies.isEmpty()) {
return Map.of("", allowedPoliciesFromConfig);
if (containsNoDefaultPolicyAvailable(bpnToPolicies)) {
bpnToPolicies.put("default", allowedPoliciesFromConfig);
}
return bpnToPolicies;
}

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

public void deletePolicy(final String policyId) {

log.info("Getting all policies to find correct BPN");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,38 @@
public class BusinessPartnerNumberListValidator
implements ConstraintValidator<ValidListOfBusinessPartnerNumbers, List<String>> {

private static final String DEFAULT = "default";

/**
* Regex for BPN.
*/
public static final String BPN_REGEX = "(BPNL)[\\w\\d]{10}[\\w\\d]{2}";

private static final Pattern PATTERN = Pattern.compile(BPN_REGEX);

private boolean allowDefault;

@Override
public void initialize(final ValidListOfBusinessPartnerNumbers constraintAnnotation) {
this.allowDefault = constraintAnnotation.allowDefault();
}

@Override
public boolean isValid(final List<String> value, final ConstraintValidatorContext context) {
public boolean isValid(final List<String> businessPartnerNumbers, final ConstraintValidatorContext context) {

// allow null and empty here (in order to allow flexible combination with @NotNull and @NotEmpty)
if (value == null || value.isEmpty()) {
if (businessPartnerNumbers == null || businessPartnerNumbers.isEmpty()) {
return true;
}

for (int index = 0; index < value.size(); index++) {
if (!PATTERN.matcher(value.get(index)).matches()) {
for (int index = 0; index < businessPartnerNumbers.size(); index++) {
final String bpn = businessPartnerNumbers.get(index);

if (allowDefault && DEFAULT.equals(bpn)) {
return true;
}

if (!PATTERN.matcher(bpn).matches()) {
context.disableDefaultConstraintViolation();
final String msg = "The business partner number at index %d is invalid (should conform to pattern '%s')";
context.buildConstraintViolationWithTemplate(msg.formatted(index, BPN_REGEX)).addConstraintViolation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,13 @@
Class<?>[] groups() default { };

Class<? extends Payload>[] payload() default { };

/**
* Whether to allow "default" as a valid value
* (This is used in {@link org.eclipse.tractusx.irs.policystore.controllers.PolicyStoreController}
* for filtering default policies).
*
* @return the value of the flag
*/
boolean allowDefault() default false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.time.Clock;
import java.time.OffsetDateTime;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand Down Expand Up @@ -385,12 +386,12 @@ void deletePolicyForEachBpn_success() {
void deletePolicy_deleteSuccessful() {
// ARRANGE
final String policyId = randomPolicyId();
when(persistenceMock.readAll()).thenReturn(Map.of(BPN, List.of(Policy.builder()
.policyId(policyId)
.createdOn(null)
.validUntil(null)
.permissions(null)
.build())));
when(persistenceMock.readAll()).thenReturn(new HashMap<>(Map.of(BPN, List.of(Policy.builder()
.policyId(policyId)
.createdOn(null)
.validUntil(null)
.permissions(null)
.build()))));

// ACT
testee.deletePolicy(policyId);
Expand All @@ -404,12 +405,12 @@ void deletePolicy_exceptionFromPolicyPersistence_shouldReturnHttpStatus500() {

// ACT
final String policyId = randomPolicyId();
when(persistenceMock.readAll()).thenReturn(Map.of(BPN, List.of(Policy.builder()
.policyId(policyId)
.createdOn(null)
.validUntil(null)
.permissions(null)
.build())));
when(persistenceMock.readAll()).thenReturn(new HashMap<>(Map.of(BPN, List.of(Policy.builder()
.policyId(policyId)
.createdOn(null)
.validUntil(null)
.permissions(null)
.build()))));
doThrow(new PolicyStoreException("")).when(persistenceMock).delete(BPN, policyId);

// ASSERT
Expand All @@ -424,7 +425,7 @@ void deletePolicy_whenPolicyNotFound_shouldReturnHttpStatus404() {
final String notExistingPolicyId = randomPolicyId();
final String policyId = randomPolicyId();
when(persistenceMock.readAll()).thenReturn(
Map.of(BPN, List.of(Policy.builder().policyId(policyId).build())));
new HashMap<>(Map.of(BPN, List.of(Policy.builder().policyId(policyId).build()))));

// ASSERT
assertThatThrownBy(() -> testee.deletePolicy(notExistingPolicyId)).isInstanceOf(
Expand Down Expand Up @@ -459,7 +460,7 @@ void updatePolicies_shouldUpdateBpnAndValidUntil() {
.validUntil(originalValidUntil)
.permissions(permissions)
.build();
when(persistenceMock.readAll()).thenReturn(Map.of(originalBpn, List.of(testPolicy)));
when(persistenceMock.readAll()).thenReturn(new HashMap<>(Map.of(originalBpn, List.of(testPolicy))));
// get policies for bpn
when(persistenceMock.readAll(originalBpn)).thenReturn(List.of(testPolicy));

Expand Down Expand Up @@ -494,7 +495,7 @@ void updatePolicies_shouldAddPolicyToEachBpn() {
.validUntil(validUntil)
.permissions(permissions)
.build();
when(persistenceMock.readAll()).thenReturn(Map.of("bpn2", List.of(testPolicy)));
when(persistenceMock.readAll()).thenReturn(new HashMap<>(Map.of("bpn2", List.of(testPolicy))));
when(persistenceMock.readAll("bpn2")).thenReturn(List.of(testPolicy));

// ACT
Expand Down Expand Up @@ -543,7 +544,7 @@ void updatePolicies_shouldAssociateEachGivenPolicyWithEachGivenBpn() {
// BPN1 without any policies

// BPN2 with testPolicy1 and testPolicy2
when(persistenceMock.readAll()).thenReturn(Map.of(bpn2, List.of(testPolicy1, testPolicy2)));
when(persistenceMock.readAll()).thenReturn(new HashMap<>(Map.of(bpn2, List.of(testPolicy1, testPolicy2))));
when(persistenceMock.readAll(bpn2)).thenReturn(List.of(Policy.builder()
.policyId(policyId1)
.createdOn(createdOn)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,20 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.verify;

import java.lang.annotation.Annotation;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import jakarta.validation.ConstraintValidatorContext;
import jakarta.validation.Payload;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Answers;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

Expand All @@ -43,9 +44,6 @@ class BusinessPartnerNumberListValidatorTest {
public static final String VALID_BPN_1 = "BPNL1234567890AB";
public static final String VALID_BPN_2 = "BPNL123456789012";

@InjectMocks
private BusinessPartnerNumberListValidator validator;

@Captor
private ArgumentCaptor<String> messageCaptor;

Expand All @@ -54,24 +52,26 @@ class BusinessPartnerNumberListValidatorTest {

@Test
void withEmptyListOfStrings() {
assertThat(validator.isValid(Collections.emptyList(), contextMock)).isTrue();
assertThat(new BusinessPartnerNumberListValidatorBuilder().build()
.isValid(Collections.emptyList(),
contextMock)).isTrue();
}

@Test
void withNull() {
assertThat(validator.isValid(null, contextMock)).isTrue();
assertThat(new BusinessPartnerNumberListValidatorBuilder().build().isValid(null, contextMock)).isTrue();
}

@Test
void withValidListOfStrings() {
List<String> validList = Arrays.asList(VALID_BPN_1, VALID_BPN_2);
assertThat(validator.isValid(validList, contextMock)).isTrue();
assertThat(new BusinessPartnerNumberListValidatorBuilder().build().isValid(validList, contextMock)).isTrue();
}

@Test
void withListContainingInvalidBPN() {
List<String> invalidList = Arrays.asList(VALID_BPN_1, "INVALID_BPN", VALID_BPN_2);
assertThat(validator.isValid(invalidList, contextMock)).isFalse();
assertThat(new BusinessPartnerNumberListValidatorBuilder().build().isValid(invalidList, contextMock)).isFalse();
verify(contextMock).buildConstraintViolationWithTemplate(messageCaptor.capture());
assertThat(messageCaptor.getValue()).contains("BPN").contains(" index 1 ").contains("invalid");
}
Expand All @@ -86,8 +86,93 @@ void withListContainingInvalidBPN() {
"ERRRES"
})
void withInvalidBPN(final String invalidBPN) {
assertThat(validator.isValid(Collections.singletonList(invalidBPN), contextMock)).isFalse();
assertThat(new BusinessPartnerNumberListValidatorBuilder().build()
.isValid(Collections.singletonList(invalidBPN),
contextMock)).isFalse();
verify(contextMock).buildConstraintViolationWithTemplate(messageCaptor.capture());
assertThat(messageCaptor.getValue()).contains("BPN").contains(" index 0 ").contains("invalid");
}

@Test
void withAllowDefaultTrue_goodCase() {
final BusinessPartnerNumberListValidator validator = new BusinessPartnerNumberListValidatorBuilder().allowDefault(
true).build();
final List<String> listWithDefault = Arrays.asList("BPNL1234567890AB", "default");
assertThat(validator.isValid(listWithDefault, contextMock)).isTrue();
}

@Test
void withAllowDefaultTrue_badCase() {
final BusinessPartnerNumberListValidator validator = new BusinessPartnerNumberListValidatorBuilder().build();
final List<String> listWithDefault = Arrays.asList("BPNL1234567890AB", "default");
assertThat(validator.isValid(listWithDefault, contextMock)).isFalse();
verify(contextMock).buildConstraintViolationWithTemplate(messageCaptor.capture());
assertThat(messageCaptor.getValue()).startsWith("The business partner number at index 1 is invalid");
}

/**
* Builder for BusinessPartnerNumberListValidator.
*/
public static class BusinessPartnerNumberListValidatorBuilder {

private String message = "Invalid list of business partner numbers";
private Class<?>[] groups = new Class<?>[0];
private Class<? extends Payload>[] payload = new Class[0];
private boolean allowDefault = false;

public BusinessPartnerNumberListValidatorBuilder setMessage(String message) {
this.message = message;
return this;
}

public BusinessPartnerNumberListValidatorBuilder setGroups(Class<?>[] groups) {
this.groups = groups;
return this;
}

public BusinessPartnerNumberListValidatorBuilder setPayload(Class<? extends Payload>[] payload) {
this.payload = payload;
return this;
}

public BusinessPartnerNumberListValidatorBuilder allowDefault(boolean allowDefault) {
this.allowDefault = allowDefault;
return this;
}

public BusinessPartnerNumberListValidator build() {

final var annotation = new ValidListOfBusinessPartnerNumbers() {

@Override
public Class<? extends Annotation> annotationType() {
return ValidListOfBusinessPartnerNumbers.class;
}

@Override
public String message() {
return message;
}

@Override
public Class<?>[] groups() {
return groups;
}

@Override
public Class<? extends Payload>[] payload() {
return payload;
}

@Override
public boolean allowDefault() {
return allowDefault;
}
};

final BusinessPartnerNumberListValidator validator = new BusinessPartnerNumberListValidator();
validator.initialize(annotation);
return validator;
}
}
}
Loading

0 comments on commit 7d9f9c5

Please sign in to comment.