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

fix(policy-api):[#734] return configured default policies if no custo… #791

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

### 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() {
ds-jhartmann marked this conversation as resolved.
Show resolved Hide resolved
return ValidListOfBusinessPartnerNumbers.class;
}

@Override
public String message() {
ds-jhartmann marked this conversation as resolved.
Show resolved Hide resolved
return message;
}

@Override
public Class<?>[] groups() {
ds-jhartmann marked this conversation as resolved.
Show resolved Hide resolved
return groups;
}

@Override
public Class<? extends Payload>[] payload() {
ds-jhartmann marked this conversation as resolved.
Show resolved Hide resolved
return payload;
}

@Override
public boolean allowDefault() {
ds-jhartmann marked this conversation as resolved.
Show resolved Hide resolved
return allowDefault;
}
};

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