Fix: Adjusted BPN validations on different operators

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:
      * <ul>
      *     <li>No {@link ParticipantAgent} was found on the {@link PolicyContext}</li>
      *     <li>The operator is invalid. Check {@link BusinessPartnerGroupFunction#ALLOWED_OPERATORS} for valid operators.</li>
-     *     <li>No database entry was found for the BPN (taken from the {@link ParticipantAgent})</li>
-     *     <li>A database entry was found, but no group-IDs were assigned</li>
+     *     <li>No database entry was found for the BPN (taken from the {@link ParticipantAgent}) and the {@link StoreFailure#getReason()} is different than {@link Reason#NOT_FOUND}</li>
      *     <li>The right value is anything other than {@link String} or {@link Collection}</li>
      * </ul>
      */
@@ -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,35 +142,48 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P
     }
 
     private List<String> 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").
      */

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,31 +115,33 @@ void evaluate_rightOperandNotStringOrCollection() {
     @ArgumentsSource(ValidOperatorProvider.class)
     @DisplayName("Valid operators, evaluating different circumstances")
     void evaluate_validOperator(String ignored, Operator operator, List<String> 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));
         assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isEqualTo(expectedOutcome);
     }
 
     @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<String> assignedBpnGroups,
+                                                          boolean expectedOutcome) {
+        List<String> 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<String> rightOperand) {
@@ -174,22 +176,51 @@ public Stream<? extends Arguments> 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<? extends Arguments> 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)
             );
         }
     }