diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunction.java b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunction.java index 5ad1154ab..d76aaf5cb 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunction.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunction.java @@ -24,6 +24,8 @@ import org.eclipse.edc.policy.model.Operator; import org.eclipse.edc.policy.model.Permission; import org.eclipse.edc.spi.agent.ParticipantAgent; +import org.eclipse.edc.spi.result.StoreFailure; +import org.eclipse.edc.spi.result.StoreFailure.Reason; import org.eclipse.tractusx.edc.validation.businesspartner.spi.BusinessPartnerStore; import java.util.Arrays; @@ -86,20 +88,18 @@ public BusinessPartnerGroupFunction(BusinessPartnerStore store) { OPERATOR_EVALUATOR_MAP.put(EQ, this::evaluateEquals); OPERATOR_EVALUATOR_MAP.put(NEQ, this::evaluateNotEquals); OPERATOR_EVALUATOR_MAP.put(IN, this::evaluateIn); - OPERATOR_EVALUATOR_MAP.put(IS_ALL_OF, this::evaluateEquals); - OPERATOR_EVALUATOR_MAP.put(IS_ANY_OF, this::evaluateIn); - OPERATOR_EVALUATOR_MAP.put(IS_NONE_OF, this::evaluateNotEquals); + OPERATOR_EVALUATOR_MAP.put(IS_ALL_OF, this::evaluateIn); + OPERATOR_EVALUATOR_MAP.put(IS_ANY_OF, this::evaluateIsAnyOf); + OPERATOR_EVALUATOR_MAP.put(IS_NONE_OF, this::evaluateIsNoneOf); } - /** * Policy evaluation function that checks whether a given BusinessPartnerNumber is covered by a given policy. * The evaluation is prematurely aborted (returns {@code false}) if: * */ @@ -120,22 +120,16 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } - var bpn = participantAgent.getIdentity(); var groups = store.resolveForBpn(bpn); + var assignedGroups = groups.getContent(); + // BPN not found in database if (groups.failed()) { policyContext.reportProblem(groups.getFailureDetail()); return false; } - var assignedGroups = groups.getContent(); - - // BPN was found, but it does not have groups assigned. - if (assignedGroups.isEmpty()) { - policyContext.reportProblem("No groups were assigned to BPN " + bpn); - return false; - } // right-operand is anything other than String or Collection var rightOperand = parseRightOperand(rightValue, policyContext); @@ -148,38 +142,51 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P } private List parseRightOperand(Object rightValue, PolicyContext context) { - if (rightValue instanceof String) { - var tokens = ((String) rightValue).split(","); + if (rightValue instanceof String value) { + var tokens = value.split(","); return Arrays.asList(tokens); } if (rightValue instanceof Collection) { return ((Collection) rightValue).stream().map(Object::toString).toList(); } - context.reportProblem(format("Right operand expected to be either String or a Collection, but was " + rightValue.getClass())); + context.reportProblem(format("Right operand expected to be either String or a Collection, but was %s", rightValue.getClass())); return null; } - + private Boolean evaluateIn(BpnGroupHolder bpnGroupHolder) { var assigned = bpnGroupHolder.assignedGroups; // checks whether both lists overlap - return bpnGroupHolder.allowedGroups - .stream() - .distinct() - .anyMatch(assigned::contains); + return bpnGroupHolder.allowedGroups.containsAll(assigned); } private Boolean evaluateNotEquals(BpnGroupHolder bpnGroupHolder) { - return !evaluateIn(bpnGroupHolder); + return !evaluateEquals(bpnGroupHolder); } private Boolean evaluateEquals(BpnGroupHolder bpnGroupHolder) { return bpnGroupHolder.allowedGroups.equals(bpnGroupHolder.assignedGroups); } + private boolean evaluateIsAnyOf(BpnGroupHolder bpnGroupHolder) { + if (bpnGroupHolder.allowedGroups.isEmpty() && bpnGroupHolder.assignedGroups.isEmpty()) { + return true; + } + + var allowedGroups = bpnGroupHolder.allowedGroups; + return bpnGroupHolder.assignedGroups + .stream() + .distinct() + .anyMatch(allowedGroups::contains); + } + + private boolean evaluateIsNoneOf(BpnGroupHolder bpnGroupHolder) { + return !evaluateIsAnyOf(bpnGroupHolder); + } + /** * Internal utility class to hold the list of assigned groups for a BPN, and the list of groups specified in the policy ("allowed groups"). */ private record BpnGroupHolder(List assignedGroups, List allowedGroups) { } -} +} \ No newline at end of file diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunctionTest.java b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunctionTest.java index 0d982037c..c7d7b3f9e 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunctionTest.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerGroupFunctionTest.java @@ -36,7 +36,6 @@ import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Stream; @@ -50,6 +49,7 @@ import static org.eclipse.edc.policy.model.Operator.IS_A; import static org.eclipse.edc.policy.model.Operator.IS_ALL_OF; import static org.eclipse.edc.policy.model.Operator.IS_ANY_OF; +import static org.eclipse.edc.policy.model.Operator.IS_NONE_OF; import static org.eclipse.edc.policy.model.Operator.LEQ; import static org.eclipse.edc.policy.model.Operator.LT; import static org.eclipse.edc.policy.model.Operator.NEQ; @@ -115,7 +115,6 @@ void evaluate_rightOperandNotStringOrCollection() { @ArgumentsSource(ValidOperatorProvider.class) @DisplayName("Valid operators, evaluating different circumstances") void evaluate_validOperator(String ignored, Operator operator, List assignedBpn, boolean expectedOutcome) { - var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(assignedBpn)); @@ -123,23 +122,26 @@ void evaluate_validOperator(String ignored, Operator operator, List assi } @Test - void evaluate_noEntryForBpn() { - var operator = NEQ; + void evaluate_failedResolveForBpn_shouldBeFalse() { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + var operator = EQ; when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.notFound("foobar")); assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); + verify(context).reportProblem("foobar"); } - @Test - void evaluate_noGroupsAssignedToBpn() { - var operator = NEQ; - var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + @ArgumentsSource(OperatorForEmptyGroupsProvider.class) + @ParameterizedTest + void evaluate_groupsAssignedButNoGroupsSentToEvaluate(Operator operator, List assignedBpnGroups, + boolean expectedOutcome) { + List allowedGroups = List.of(); + when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); - when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(Collections.emptyList())); + when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(assignedBpnGroups)); - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isEqualTo(expectedOutcome); } private Permission createPermission(Operator op, List rightOperand) { @@ -174,23 +176,52 @@ public Stream provideArguments(ExtensionContext extensionCo Arguments.of("Overlapping groups", EQ, List.of("different-group"), false), Arguments.of("Disjoint groups", NEQ, List.of("different-group", "another-different-group"), true), - Arguments.of("Overlapping groups", NEQ, List.of(TEST_GROUP_1, "different-group"), false), + Arguments.of("Overlapping groups", NEQ, List.of(TEST_GROUP_1, "different-group"), true), Arguments.of("Matching groups", NEQ, List.of(TEST_GROUP_1, TEST_GROUP_2), false), + Arguments.of("Empty groups", NEQ, List.of(), true), Arguments.of("Matching groups", IN, List.of(TEST_GROUP_1, TEST_GROUP_2), true), - Arguments.of("Overlapping groups", IN, List.of(TEST_GROUP_1, "different-group"), true), + Arguments.of("Overlapping groups", IN, List.of(TEST_GROUP_1, "different-group"), false), Arguments.of("Disjoint groups", IN, List.of("different-group", "another-different-group"), false), Arguments.of("Disjoint groups", IS_ALL_OF, List.of("different-group", "another-different-group"), false), Arguments.of("Matching groups", IS_ALL_OF, List.of(TEST_GROUP_1, TEST_GROUP_2), true), Arguments.of("Overlapping groups", IS_ALL_OF, List.of(TEST_GROUP_1, TEST_GROUP_2, "different-group", "another-different-group"), false), - + Arguments.of("Overlapping groups (1 overlap)", IS_ALL_OF, List.of(TEST_GROUP_1, "different-group"), false), + Arguments.of("Overlapping groups (1 overlap)", IS_ALL_OF, List.of(TEST_GROUP_1), true), Arguments.of("Disjoint groups", IS_ANY_OF, List.of("different-group", "another-different-group"), false), Arguments.of("Matching groups", IS_ANY_OF, List.of(TEST_GROUP_1, TEST_GROUP_2), true), Arguments.of("Overlapping groups (1 overlap)", IS_ANY_OF, List.of(TEST_GROUP_1, "different-group", "another-different-group"), true), - Arguments.of("Overlapping groups (2 overlap)", IS_ANY_OF, List.of(TEST_GROUP_1, TEST_GROUP_2, "different-group", "another-different-group"), true) + Arguments.of("Overlapping groups (2 overlap)", IS_ANY_OF, List.of(TEST_GROUP_1, TEST_GROUP_2, "different-group", "another-different-group"), true), + + Arguments.of("Disjoint groups", IS_NONE_OF, List.of("different-group", "another-different-group"), true), + Arguments.of("Matching groups", IS_NONE_OF, List.of(TEST_GROUP_1, TEST_GROUP_2), false), + Arguments.of("Overlapping groups", IS_NONE_OF, List.of(TEST_GROUP_1, "another-different-group"), false), + Arguments.of("Empty groups", IS_NONE_OF, List.of(), true) + ); + } + } + + private static class OperatorForEmptyGroupsProvider implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext extensionContext) { + var assignedBpnGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + + return Stream.of( + Arguments.of(EQ, assignedBpnGroups, false), + Arguments.of(EQ, List.of(), true), + Arguments.of(NEQ, assignedBpnGroups, true), + Arguments.of(NEQ, List.of(), false), + Arguments.of(IN, assignedBpnGroups, false), + Arguments.of(IN, List.of(), true), + Arguments.of(IS_ALL_OF, assignedBpnGroups, false), + Arguments.of(IS_ALL_OF, List.of(), true), + Arguments.of(IS_ANY_OF, assignedBpnGroups, false), + Arguments.of(IS_ANY_OF, List.of(), true), + Arguments.of(IS_NONE_OF, assignedBpnGroups, true), + Arguments.of(IS_NONE_OF, List.of(), false) ); } } -} +} \ No newline at end of file