From 4c6e6113b848bdaa21b7ca6c6019cf3995080dd3 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Tue, 23 Jul 2024 09:53:19 +0100 Subject: [PATCH 01/17] fix: created specific validation for isAllOf operator --- .../functions/BusinessPartnerGroupFunction.java | 10 +++++++++- .../functions/BusinessPartnerGroupFunctionTest.java | 4 +++- 2 files changed, 12 insertions(+), 2 deletions(-) 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..e382b0f52 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 @@ -86,7 +86,7 @@ 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_ALL_OF, this::evaluateIsAllOf); OPERATOR_EVALUATOR_MAP.put(IS_ANY_OF, this::evaluateIn); OPERATOR_EVALUATOR_MAP.put(IS_NONE_OF, this::evaluateNotEquals); } @@ -177,6 +177,14 @@ private Boolean evaluateEquals(BpnGroupHolder bpnGroupHolder) { return bpnGroupHolder.allowedGroups.equals(bpnGroupHolder.assignedGroups); } + private boolean evaluateIsAllOf(BpnGroupHolder bpnGroupHolder) { + var assigned = bpnGroupHolder.assignedGroups; + return bpnGroupHolder.allowedGroups + .stream() + .distinct() + .allMatch(assigned::contains); + } + /** * Internal utility class to hold the list of assigned groups for a BPN, and the list of groups specified in the policy ("allowed groups"). */ 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..98f257f73 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 @@ -183,7 +183,9 @@ public Stream provideArguments(ExtensionContext extensionCo 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", IS_ALL_OF, List.of(TEST_GROUP_1, TEST_GROUP_2, "different-group", "another-different-group"), true), + 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), false), Arguments.of("Disjoint groups", IS_ANY_OF, List.of("different-group", "another-different-group"), false), From 3b775608fa74420032cae1dd3f2ad485a32e2e4e Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Tue, 23 Jul 2024 15:25:20 +0100 Subject: [PATCH 02/17] fix: adjusted behaviour of NEQ and IS_NONE_OF --- .../BusinessPartnerGroupFunction.java | 24 +++++++---- .../BusinessPartnerGroupFunctionTest.java | 41 +++++++++++-------- 2 files changed, 41 insertions(+), 24 deletions(-) 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 e382b0f52..de2f05005 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 @@ -88,10 +88,9 @@ public BusinessPartnerGroupFunction(BusinessPartnerStore store) { OPERATOR_EVALUATOR_MAP.put(IN, this::evaluateIn); OPERATOR_EVALUATOR_MAP.put(IS_ALL_OF, this::evaluateIsAllOf); OPERATOR_EVALUATOR_MAP.put(IS_ANY_OF, this::evaluateIn); - OPERATOR_EVALUATOR_MAP.put(IS_NONE_OF, this::evaluateNotEquals); + 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,12 +119,15 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } - var bpn = participantAgent.getIdentity(); var groups = store.resolveForBpn(bpn); // BPN not found in database if (groups.failed()) { + if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { + return true; + } + policyContext.reportProblem(groups.getFailureDetail()); return false; } @@ -133,6 +135,10 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P // BPN was found, but it does not have groups assigned. if (assignedGroups.isEmpty()) { + if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { + return true; + } + policyContext.reportProblem("No groups were assigned to BPN " + bpn); return false; } @@ -148,15 +154,15 @@ 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; } @@ -170,7 +176,7 @@ private Boolean evaluateIn(BpnGroupHolder bpnGroupHolder) { } private Boolean evaluateNotEquals(BpnGroupHolder bpnGroupHolder) { - return !evaluateIn(bpnGroupHolder); + return !evaluateEquals(bpnGroupHolder); } private Boolean evaluateEquals(BpnGroupHolder bpnGroupHolder) { @@ -185,6 +191,10 @@ private boolean evaluateIsAllOf(BpnGroupHolder bpnGroupHolder) { .allMatch(assigned::contains); } + private boolean evaluateIsNoneOf(BpnGroupHolder bpnGroupHolder) { + return !evaluateIn(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"). */ 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 98f257f73..d84f072ca 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 @@ -35,6 +35,7 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; +import org.junit.jupiter.params.provider.EnumSource; import java.util.Collections; import java.util.List; @@ -50,6 +51,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; @@ -82,14 +84,14 @@ void setUp() { void evaluate_noParticipantAgentOnContext() { reset(context); assertThat(function.evaluate(EQ, "test-group", createPermission(EQ, List.of()), context)).isFalse(); - verify(context).reportProblem(eq("ParticipantAgent not found on PolicyContext")); + verify(context).reportProblem("ParticipantAgent not found on PolicyContext"); } @ParameterizedTest(name = "Invalid operator {0}") @ArgumentsSource(InvalidOperatorProvider.class) @DisplayName("Invalid operators, expect report in policy context") void evaluate_invalidOperator(Operator invalidOperator) { - when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of())); + when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of())); assertThat(function.evaluate(invalidOperator, "test-group", createPermission(invalidOperator, List.of()), context)).isFalse(); verify(context).reportProblem(endsWith("but was [" + invalidOperator.name() + "]")); } @@ -98,7 +100,7 @@ void evaluate_invalidOperator(Operator invalidOperator) { @DisplayName("Right-hand operand is not String or Collection") void evaluate_rightOperandNotStringOrCollection() { when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(List.of("test-group"))); - when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); assertThat(function.evaluate(EQ, 42, createPermission(EQ, List.of("test-group")), context)).isFalse(); assertThat(function.evaluate(EQ, 42L, createPermission(EQ, List.of("test-group")), context)).isFalse(); @@ -117,29 +119,29 @@ void evaluate_rightOperandNotStringOrCollection() { 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(context.getContextData(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; + @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) + @ParameterizedTest + void evaluate_noEntryForBpn(final Operator operator) { 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(context.getContextData(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(); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); } - @Test - void evaluate_noGroupsAssignedToBpn() { - var operator = NEQ; + @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) + @ParameterizedTest + void evaluate_noGroupsAssignedToBpn(final Operator operator) { 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(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(Collections.emptyList())); - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); } private Permission createPermission(Operator op, List rightOperand) { @@ -174,8 +176,9 @@ 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), @@ -187,11 +190,15 @@ public Stream provideArguments(ExtensionContext extensionCo 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), false), - 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) ); } } From c01cb196176eb5022cf780f8646de4c77549886f Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Tue, 23 Jul 2024 15:39:32 +0100 Subject: [PATCH 03/17] fix: grouped empty groups response to a method --- .../BusinessPartnerGroupFunction.java | 25 +++++------ .../BusinessPartnerGroupFunctionTest.java | 41 ++++++++++++++----- 2 files changed, 44 insertions(+), 22 deletions(-) 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 de2f05005..b9768af96 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 @@ -124,23 +124,14 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P // BPN not found in database if (groups.failed()) { - if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { - return true; - } - - policyContext.reportProblem(groups.getFailureDetail()); - return false; + return evaluateEmptyGroupsResponse(operator, policyContext, groups.getFailureDetail()); } + var assignedGroups = groups.getContent(); // BPN was found, but it does not have groups assigned. if (assignedGroups.isEmpty()) { - if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { - return true; - } - - policyContext.reportProblem("No groups were assigned to BPN " + bpn); - return false; + return evaluateEmptyGroupsResponse(operator, policyContext, "No groups were assigned to BPN " + bpn); } // right-operand is anything other than String or Collection @@ -153,6 +144,16 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return OPERATOR_EVALUATOR_MAP.get(operator).apply(new BpnGroupHolder(assignedGroups, rightOperand)); } + private boolean evaluateEmptyGroupsResponse(final Operator operator, final PolicyContext policyContext, + final String problemMessage) { + if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { + return true; + } + + policyContext.reportProblem(problemMessage); + return false; + } + private List parseRightOperand(Object rightValue, PolicyContext context) { if (rightValue instanceof String value) { var tokens = value.split(","); 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 d84f072ca..53722a2bf 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 @@ -19,6 +19,11 @@ package org.eclipse.tractusx.edc.validation.businesspartner.functions; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + import org.eclipse.edc.policy.engine.spi.PolicyContext; import org.eclipse.edc.policy.model.AtomicConstraint; import org.eclipse.edc.policy.model.LiteralExpression; @@ -37,11 +42,6 @@ import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.EnumSource; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.stream.Stream; - import static org.assertj.core.api.Assertions.assertThat; import static org.eclipse.edc.policy.model.Operator.EQ; import static org.eclipse.edc.policy.model.Operator.GEQ; @@ -57,8 +57,9 @@ import static org.eclipse.edc.policy.model.Operator.NEQ; import static org.eclipse.edc.spi.agent.ParticipantAgent.PARTICIPANT_IDENTITY; import static org.eclipse.tractusx.edc.validation.businesspartner.functions.BusinessPartnerGroupFunction.BUSINESS_PARTNER_CONSTRAINT_KEY; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.endsWith; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @@ -126,22 +127,42 @@ void evaluate_validOperator(String ignored, Operator operator, List assi @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) @ParameterizedTest - void evaluate_noEntryForBpn(final Operator operator) { + void evaluate_noEntryForBpnWithNegativeOperator_shouldBeTrue(final Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); when(context.getContextData(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)).isTrue(); + assertTrue(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); } @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) @ParameterizedTest - void evaluate_noGroupsAssignedToBpn(final Operator operator) { + void evaluate_noGroupsAssignedToBpnWithNegativeOperator_shouldBeTrue(final Operator operator) { + var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(Collections.emptyList())); + + assertTrue(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + } + + @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) + @ParameterizedTest + void evaluate_noEntryForBpnWithPositiveOperator_shouldBeFalse(final Operator operator) { + var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.notFound("foobar")); + + assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + } + + @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) + @ParameterizedTest + void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(final Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(Collections.emptyList())); - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); + assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); } private Permission createPermission(Operator op, List rightOperand) { From ea9c6e3c2f0f04cd6173d3db95eb856679da3cf3 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Tue, 23 Jul 2024 16:49:37 +0100 Subject: [PATCH 04/17] fix: improved tests with message validation --- .../functions/BusinessPartnerGroupFunctionTest.java | 2 ++ 1 file changed, 2 insertions(+) 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 53722a2bf..2f7f04ed8 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 @@ -153,6 +153,7 @@ void evaluate_noEntryForBpnWithPositiveOperator_shouldBeFalse(final Operator ope when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.notFound("foobar")); assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + verify(context).reportProblem("foobar"); } @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) @@ -163,6 +164,7 @@ void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(final Oper when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(Collections.emptyList())); assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + verify(context).reportProblem("No groups were assigned to BPN " + TEST_BPN); } private Permission createPermission(Operator op, List rightOperand) { From 79fe25691b72f0e0f7f2cb228310a7bab48165f7 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Tue, 23 Jul 2024 16:49:56 +0100 Subject: [PATCH 05/17] fix: improving method returns empty list instead of null --- .../functions/BusinessPartnerGroupFunction.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 b9768af96..699c2f21f 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 @@ -28,6 +28,7 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -136,7 +137,7 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P // right-operand is anything other than String or Collection var rightOperand = parseRightOperand(rightValue, policyContext); - if (rightOperand == null) { + if (rightOperand.isEmpty()) { return false; } @@ -164,7 +165,7 @@ private List parseRightOperand(Object rightValue, PolicyContext context) } context.reportProblem(format("Right operand expected to be either String or a Collection, but was %s", rightValue.getClass())); - return null; + return Collections.emptyList(); } private Boolean evaluateIn(BpnGroupHolder bpnGroupHolder) { From 23f1668a4d536107edea488302c15b947780c9bf Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 08:27:05 +0100 Subject: [PATCH 06/17] fix: rolled back minor changes --- .../BusinessPartnerGroupFunction.java | 7 ++-- .../BusinessPartnerGroupFunctionTest.java | 35 +++++++++---------- 2 files changed, 20 insertions(+), 22 deletions(-) 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 699c2f21f..5d1fdb7fb 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 @@ -137,7 +137,7 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P // right-operand is anything other than String or Collection var rightOperand = parseRightOperand(rightValue, policyContext); - if (rightOperand.isEmpty()) { + if (rightOperand == null) { return false; } @@ -145,8 +145,7 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return OPERATOR_EVALUATOR_MAP.get(operator).apply(new BpnGroupHolder(assignedGroups, rightOperand)); } - private boolean evaluateEmptyGroupsResponse(final Operator operator, final PolicyContext policyContext, - final String problemMessage) { + private boolean evaluateEmptyGroupsResponse(Operator operator, PolicyContext policyContext, String problemMessage) { if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { return true; } @@ -165,7 +164,7 @@ private List parseRightOperand(Object rightValue, PolicyContext context) } context.reportProblem(format("Right operand expected to be either String or a Collection, but was %s", rightValue.getClass())); - return Collections.emptyList(); + return null; } private Boolean evaluateIn(BpnGroupHolder bpnGroupHolder) { 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 2f7f04ed8..e036c86c6 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 @@ -57,9 +57,8 @@ import static org.eclipse.edc.policy.model.Operator.NEQ; import static org.eclipse.edc.spi.agent.ParticipantAgent.PARTICIPANT_IDENTITY; import static org.eclipse.tractusx.edc.validation.businesspartner.functions.BusinessPartnerGroupFunction.BUSINESS_PARTNER_CONSTRAINT_KEY; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.endsWith; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; @@ -84,7 +83,7 @@ void setUp() { @DisplayName("PolicyContext does not carry ParticipantAgent") void evaluate_noParticipantAgentOnContext() { reset(context); - assertThat(function.evaluate(EQ, "test-group", createPermission(EQ, List.of()), context)).isFalse(); + assertThat(function.evaluate(EQ, eq("test-group"), createPermission(EQ, List.of()), context)).isFalse(); verify(context).reportProblem("ParticipantAgent not found on PolicyContext"); } @@ -92,7 +91,7 @@ void evaluate_noParticipantAgentOnContext() { @ArgumentsSource(InvalidOperatorProvider.class) @DisplayName("Invalid operators, expect report in policy context") void evaluate_invalidOperator(Operator invalidOperator) { - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of())); + when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of())); assertThat(function.evaluate(invalidOperator, "test-group", createPermission(invalidOperator, List.of()), context)).isFalse(); verify(context).reportProblem(endsWith("but was [" + invalidOperator.name() + "]")); } @@ -101,7 +100,7 @@ void evaluate_invalidOperator(Operator invalidOperator) { @DisplayName("Right-hand operand is not String or Collection") void evaluate_rightOperandNotStringOrCollection() { when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.success(List.of("test-group"))); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + when(context.getContextData(eq(ParticipantAgent.class))).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); assertThat(function.evaluate(EQ, 42, createPermission(EQ, List.of("test-group")), context)).isFalse(); assertThat(function.evaluate(EQ, 42L, createPermission(EQ, List.of("test-group")), context)).isFalse(); @@ -120,50 +119,50 @@ void evaluate_rightOperandNotStringOrCollection() { void evaluate_validOperator(String ignored, Operator operator, List assignedBpn, boolean expectedOutcome) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + 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); } @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) @ParameterizedTest - void evaluate_noEntryForBpnWithNegativeOperator_shouldBeTrue(final Operator operator) { + void evaluate_noEntryForBpnWithNegativeOperator_shouldBeTrue(Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + 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")); - assertTrue(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); } @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) @ParameterizedTest - void evaluate_noGroupsAssignedToBpnWithNegativeOperator_shouldBeTrue(final Operator operator) { + void evaluate_noGroupsAssignedToBpnWithNegativeOperator_shouldBeTrue(Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + 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())); - assertTrue(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); } @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) @ParameterizedTest - void evaluate_noEntryForBpnWithPositiveOperator_shouldBeFalse(final Operator operator) { + void evaluate_noEntryForBpnWithPositiveOperator_shouldBeFalse(Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + 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")); - assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); verify(context).reportProblem("foobar"); } @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) @ParameterizedTest - void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(final Operator operator) { + void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(Operator operator) { var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); - when(context.getContextData(ParticipantAgent.class)).thenReturn(new ParticipantAgent(Map.of(), Map.of(PARTICIPANT_IDENTITY, TEST_BPN))); + 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())); - assertFalse(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); verify(context).reportProblem("No groups were assigned to BPN " + TEST_BPN); } From 4ee3ec64df02c818e49d3a0dfa8ca2fb7936df7b Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 09:06:10 +0100 Subject: [PATCH 07/17] fix: refactored method that verifies empty groups --- .../BusinessPartnerGroupFunction.java | 58 ++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) 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 5d1fdb7fb..ee08380ab 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,11 +24,11 @@ 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.StoreResult; import org.eclipse.tractusx.edc.validation.businesspartner.spi.BusinessPartnerStore; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -79,6 +79,7 @@ public class BusinessPartnerGroupFunction implements AtomicConstraintFunction { public static final String BUSINESS_PARTNER_CONSTRAINT_KEY = TX_NAMESPACE + "BusinessPartnerGroup"; private static final List ALLOWED_OPERATORS = List.of(EQ, NEQ, IN, IS_ALL_OF, IS_ANY_OF, IS_NONE_OF); + private static final List NEGATIVE_OPERATORS = List.of(NEQ, IS_NONE_OF); private static final Map> OPERATOR_EVALUATOR_MAP = new HashMap<>(); private final BusinessPartnerStore store; @@ -123,16 +124,11 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P var bpn = participantAgent.getIdentity(); var groups = store.resolveForBpn(bpn); - // BPN not found in database - if (groups.failed()) { - return evaluateEmptyGroupsResponse(operator, policyContext, groups.getFailureDetail()); - } - - var assignedGroups = groups.getContent(); + var errorMessage = evaluateEmptyGroups(groups, operator, bpn); - // BPN was found, but it does not have groups assigned. - if (assignedGroups.isEmpty()) { - return evaluateEmptyGroupsResponse(operator, policyContext, "No groups were assigned to BPN " + bpn); + if (errorMessage != null) { + policyContext.reportProblem(errorMessage); + return false; } // right-operand is anything other than String or Collection @@ -141,17 +137,45 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } + var assignedGroups = groups.getContent(); + //call evaluator function return OPERATOR_EVALUATOR_MAP.get(operator).apply(new BpnGroupHolder(assignedGroups, rightOperand)); } - private boolean evaluateEmptyGroupsResponse(Operator operator, PolicyContext policyContext, String problemMessage) { - if (NEQ.equals(operator) || IS_NONE_OF.equals(operator)) { - return true; + /** + * Verify if BPN was found in the database and if the BPN has any group assigned. If one of those validations fail, + * it will return an error message, otherwise it will return {@code null}. + * The evaluation is prematurely aborted (returns {@code null}) if: + *
    + *
  • {@link Operator} is a negative operator. Check {@link BusinessPartnerGroupFunction#NEGATIVE_OPERATORS}
    • + *
+ * + * @param groups result of the BPN search on the database + * @param operator operator sent for evaluation + * @param bpn participant agent identifier + * @return an error message in case of empty groups or empty assigned groups, {@code null} otherwise. + */ + private String evaluateEmptyGroups(StoreResult> groups, Operator operator, String bpn) { + var isNegativeOperator = NEGATIVE_OPERATORS.contains(operator); + + if (isNegativeOperator) { + return null; + } + + // BPN not found in database + if (groups.failed()) { + return groups.getFailureDetail(); } - policyContext.reportProblem(problemMessage); - return false; + var assignedGroups = groups.getContent(); + + // BPN was found, but it does not have groups assigned. + if (assignedGroups.isEmpty()) { + return "No groups were assigned to BPN " + bpn; + } + + return null; } private List parseRightOperand(Object rightValue, PolicyContext context) { @@ -193,6 +217,10 @@ private boolean evaluateIsAllOf(BpnGroupHolder bpnGroupHolder) { } private boolean evaluateIsNoneOf(BpnGroupHolder bpnGroupHolder) { + if (bpnGroupHolder.assignedGroups == null) { + return true; + } + return !evaluateIn(bpnGroupHolder); } From 88cbe469cafeaf42a2dfc8d77b628301b6fab5f6 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 09:08:16 +0100 Subject: [PATCH 08/17] fix: adjusted last matcher with eq() method --- .../functions/BusinessPartnerGroupFunctionTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e036c86c6..ce42a3cba 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 @@ -83,8 +83,8 @@ void setUp() { @DisplayName("PolicyContext does not carry ParticipantAgent") void evaluate_noParticipantAgentOnContext() { reset(context); - assertThat(function.evaluate(EQ, eq("test-group"), createPermission(EQ, List.of()), context)).isFalse(); - verify(context).reportProblem("ParticipantAgent not found on PolicyContext"); + assertThat(function.evaluate(EQ, "test-group", createPermission(EQ, List.of()), context)).isFalse(); + verify(context).reportProblem(eq("ParticipantAgent not found on PolicyContext")); } @ParameterizedTest(name = "Invalid operator {0}") From 57fb15749a165d8fc381cf3542b46f8d624c793c Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 09:51:55 +0100 Subject: [PATCH 09/17] fix: adjusted imports order to be compatible with checkstyle --- .../functions/BusinessPartnerGroupFunctionTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 ce42a3cba..ec01d07dc 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 @@ -19,11 +19,6 @@ package org.eclipse.tractusx.edc.validation.businesspartner.functions; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.stream.Stream; - import org.eclipse.edc.policy.engine.spi.PolicyContext; import org.eclipse.edc.policy.model.AtomicConstraint; import org.eclipse.edc.policy.model.LiteralExpression; @@ -42,6 +37,11 @@ import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.EnumSource; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + import static org.assertj.core.api.Assertions.assertThat; import static org.eclipse.edc.policy.model.Operator.EQ; import static org.eclipse.edc.policy.model.Operator.GEQ; From 33bb8832f7fda7ecd4fd333d0348749915faa9cc Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 14:45:57 +0100 Subject: [PATCH 10/17] fix: simplified groups validation with the operator and adjusted validation for isAllOf --- .../BusinessPartnerGroupFunction.java | 60 ++++++------------- 1 file changed, 19 insertions(+), 41 deletions(-) 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 ee08380ab..944638bc5 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,7 +24,6 @@ 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.StoreResult; import org.eclipse.tractusx.edc.validation.businesspartner.spi.BusinessPartnerStore; import java.util.Arrays; @@ -79,7 +78,7 @@ public class BusinessPartnerGroupFunction implements AtomicConstraintFunction { public static final String BUSINESS_PARTNER_CONSTRAINT_KEY = TX_NAMESPACE + "BusinessPartnerGroup"; private static final List ALLOWED_OPERATORS = List.of(EQ, NEQ, IN, IS_ALL_OF, IS_ANY_OF, IS_NONE_OF); - private static final List NEGATIVE_OPERATORS = List.of(NEQ, IS_NONE_OF); + private static final List POSITIVE_OPERATORS = List.of(EQ, IN, IS_ALL_OF, IS_ANY_OF); private static final Map> OPERATOR_EVALUATOR_MAP = new HashMap<>(); private final BusinessPartnerStore store; @@ -99,8 +98,8 @@ public BusinessPartnerGroupFunction(BusinessPartnerStore store) { *
    *
  • No {@link ParticipantAgent} was found on the {@link PolicyContext}
    • *
  • The operator is invalid. Check {@link BusinessPartnerGroupFunction#ALLOWED_OPERATORS} for valid operators.
    • - *
  • No database entry was found for the BPN (taken from the {@link ParticipantAgent})
    • - *
  • A database entry was found, but no group-IDs were assigned
    • + *
  • No database entry was found for the BPN (taken from the {@link ParticipantAgent}) and the operator is 'positive' {@link BusinessPartnerGroupFunction#POSITIVE_OPERATORS}
    • + *
  • A database entry was found, but no group-IDs were assigned and the operator is 'positive' {@link BusinessPartnerGroupFunction#POSITIVE_OPERATORS}
    • *
  • The right value is anything other than {@link String} or {@link Collection}
    • *
*/ @@ -124,9 +123,19 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P var bpn = participantAgent.getIdentity(); var groups = store.resolveForBpn(bpn); - var errorMessage = evaluateEmptyGroups(groups, operator, bpn); + var isPositiveOperator = POSITIVE_OPERATORS.contains(operator); + var errorMessage = ""; - if (errorMessage != null) { + // BPN not found in database + if (groups.failed()) { + errorMessage = groups.getFailureDetail(); + + // BPN was found, but it does not have groups assigned. + } else if (groups.getContent().isEmpty()) { + errorMessage = "No groups were assigned to BPN " + bpn; + } + + if (!errorMessage.isBlank() && isPositiveOperator) { policyContext.reportProblem(errorMessage); return false; } @@ -143,41 +152,6 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return OPERATOR_EVALUATOR_MAP.get(operator).apply(new BpnGroupHolder(assignedGroups, rightOperand)); } - /** - * Verify if BPN was found in the database and if the BPN has any group assigned. If one of those validations fail, - * it will return an error message, otherwise it will return {@code null}. - * The evaluation is prematurely aborted (returns {@code null}) if: - *
    - *
  • {@link Operator} is a negative operator. Check {@link BusinessPartnerGroupFunction#NEGATIVE_OPERATORS}
    • - *
- * - * @param groups result of the BPN search on the database - * @param operator operator sent for evaluation - * @param bpn participant agent identifier - * @return an error message in case of empty groups or empty assigned groups, {@code null} otherwise. - */ - private String evaluateEmptyGroups(StoreResult> groups, Operator operator, String bpn) { - var isNegativeOperator = NEGATIVE_OPERATORS.contains(operator); - - if (isNegativeOperator) { - return null; - } - - // BPN not found in database - if (groups.failed()) { - return groups.getFailureDetail(); - } - - var assignedGroups = groups.getContent(); - - // BPN was found, but it does not have groups assigned. - if (assignedGroups.isEmpty()) { - return "No groups were assigned to BPN " + bpn; - } - - return null; - } - private List parseRightOperand(Object rightValue, PolicyContext context) { if (rightValue instanceof String value) { var tokens = value.split(","); @@ -209,6 +183,10 @@ private Boolean evaluateEquals(BpnGroupHolder bpnGroupHolder) { } private boolean evaluateIsAllOf(BpnGroupHolder bpnGroupHolder) { + if (bpnGroupHolder.allowedGroups.isEmpty()) { + return false; + } + var assigned = bpnGroupHolder.assignedGroups; return bpnGroupHolder.allowedGroups .stream() From bd2e95a3e6cba08418641af33e5a556df2eb3cdd Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 24 Jul 2024 14:46:48 +0100 Subject: [PATCH 11/17] fix: added more test scenarios for left-hand operand not empty and right-hand operand empty --- .../BusinessPartnerGroupFunctionTest.java | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) 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 ec01d07dc..39311d729 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 @@ -117,7 +117,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)); @@ -166,6 +165,18 @@ void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(Operator o verify(context).reportProblem("No groups were assigned to BPN " + TEST_BPN); } + @ArgumentsSource(OperatorForEmptyGroupsProvider.class) + @ParameterizedTest + void evaluate_groupsAssignedButNoGroupsSentToEvaluate(Operator operator, boolean expectedOutcome) { + var allowedGroups = List.of(); + var assignedBpnGroups = 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(assignedBpnGroups)); + + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isEqualTo(expectedOutcome); + } + private Permission createPermission(Operator op, List rightOperand) { return Permission.Builder.newInstance() .constraint(AtomicConstraint.Builder.newInstance() @@ -224,4 +235,18 @@ public Stream provideArguments(ExtensionContext extensionCo ); } } + + private static class OperatorForEmptyGroupsProvider implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + return Stream.of( + Arguments.of(EQ, false), + Arguments.of(NEQ, true), + Arguments.of(IN, false), + Arguments.of(IS_ALL_OF, false), + Arguments.of(IS_ANY_OF, false), + Arguments.of(IS_NONE_OF, true) + ); + } + } } From ca334329798b263eca82dce096c887747ee229f2 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Fri, 26 Jul 2024 14:17:21 +0100 Subject: [PATCH 12/17] fix: added more test scenarios for both left-hand operand and right-hand operand being empty --- .../BusinessPartnerGroupFunction.java | 4 ++++ .../BusinessPartnerGroupFunctionTest.java | 24 ++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) 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 944638bc5..f737ed7ab 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 @@ -175,6 +175,10 @@ private Boolean evaluateIn(BpnGroupHolder bpnGroupHolder) { } private Boolean evaluateNotEquals(BpnGroupHolder bpnGroupHolder) { + if (bpnGroupHolder.allowedGroups.isEmpty()) { + return true; + } + return !evaluateEquals(bpnGroupHolder); } 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 39311d729..c5b743b45 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 @@ -167,9 +167,9 @@ void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(Operator o @ArgumentsSource(OperatorForEmptyGroupsProvider.class) @ParameterizedTest - void evaluate_groupsAssignedButNoGroupsSentToEvaluate(Operator operator, boolean expectedOutcome) { + void evaluate_groupsAssignedButNoGroupsSentToEvaluate(Operator operator, List assignedBpnGroups, + boolean expectedOutcome) { var allowedGroups = List.of(); - var assignedBpnGroups = 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(assignedBpnGroups)); @@ -239,13 +239,21 @@ public Stream provideArguments(ExtensionContext extensionCo private static class OperatorForEmptyGroupsProvider implements ArgumentsProvider { @Override public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + var assignedBpnGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + return Stream.of( - Arguments.of(EQ, false), - Arguments.of(NEQ, true), - Arguments.of(IN, false), - Arguments.of(IS_ALL_OF, false), - Arguments.of(IS_ANY_OF, false), - Arguments.of(IS_NONE_OF, true) + Arguments.of(EQ, assignedBpnGroups, false), + Arguments.of(EQ, List.of(), false), + Arguments.of(NEQ, assignedBpnGroups, true), + Arguments.of(NEQ, List.of(), true), + Arguments.of(IN, assignedBpnGroups, false), + Arguments.of(IN, List.of(), false), + Arguments.of(IS_ALL_OF, assignedBpnGroups, false), + Arguments.of(IS_ALL_OF, List.of(), false), + Arguments.of(IS_ANY_OF, assignedBpnGroups, false), + Arguments.of(IS_ANY_OF, List.of(), false), + Arguments.of(IS_NONE_OF, assignedBpnGroups, true), + Arguments.of(IS_NONE_OF, List.of(), true) ); } } From 57d48c1f5a2a932f8f9a944b45cc9d52b6a25453 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 31 Jul 2024 13:33:36 +0100 Subject: [PATCH 13/17] fix: readjusted all validations and added condition for not found reason after searching for BPN --- .../BusinessPartnerGroupFunction.java | 57 ++++++----------- .../BusinessPartnerGroupFunctionTest.java | 62 +++++++------------ 2 files changed, 42 insertions(+), 77 deletions(-) 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 f737ed7ab..8bd672001 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; @@ -78,7 +80,6 @@ public class BusinessPartnerGroupFunction implements AtomicConstraintFunction { public static final String BUSINESS_PARTNER_CONSTRAINT_KEY = TX_NAMESPACE + "BusinessPartnerGroup"; private static final List ALLOWED_OPERATORS = List.of(EQ, NEQ, IN, IS_ALL_OF, IS_ANY_OF, IS_NONE_OF); - private static final List POSITIVE_OPERATORS = List.of(EQ, IN, IS_ALL_OF, IS_ANY_OF); private static final Map> OPERATOR_EVALUATOR_MAP = new HashMap<>(); private final BusinessPartnerStore store; @@ -87,8 +88,8 @@ 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::evaluateIsAllOf); - OPERATOR_EVALUATOR_MAP.put(IS_ANY_OF, this::evaluateIn); + 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); } @@ -98,8 +99,7 @@ public BusinessPartnerGroupFunction(BusinessPartnerStore store) { *
    *
  • No {@link ParticipantAgent} was found on the {@link PolicyContext}
    • *
  • The operator is invalid. Check {@link BusinessPartnerGroupFunction#ALLOWED_OPERATORS} for valid operators.
    • - *
  • No database entry was found for the BPN (taken from the {@link ParticipantAgent}) and the operator is 'positive' {@link BusinessPartnerGroupFunction#POSITIVE_OPERATORS}
    • - *
  • A database entry was found, but no group-IDs were assigned and the operator is 'positive' {@link BusinessPartnerGroupFunction#POSITIVE_OPERATORS}
    • + *
  • 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}
    • *
  • The right value is anything other than {@link String} or {@link Collection}
    • *
*/ @@ -123,21 +123,17 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P var bpn = participantAgent.getIdentity(); var groups = store.resolveForBpn(bpn); - var isPositiveOperator = POSITIVE_OPERATORS.contains(operator); - var errorMessage = ""; + var assignedGroups = groups.getContent(); // BPN not found in database if (groups.failed()) { - errorMessage = groups.getFailureDetail(); + boolean isNotFound = Reason.NOT_FOUND.equals(groups.getFailure().getReason()); + if (!isNotFound) { + policyContext.reportProblem(groups.getFailureDetail()); + return false; + } - // BPN was found, but it does not have groups assigned. - } else if (groups.getContent().isEmpty()) { - errorMessage = "No groups were assigned to BPN " + bpn; - } - - if (!errorMessage.isBlank() && isPositiveOperator) { - policyContext.reportProblem(errorMessage); - return false; + assignedGroups = List.of(); } // right-operand is anything other than String or Collection @@ -146,8 +142,6 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } - var assignedGroups = groups.getContent(); - //call evaluator function return OPERATOR_EVALUATOR_MAP.get(operator).apply(new BpnGroupHolder(assignedGroups, rightOperand)); } @@ -168,17 +162,10 @@ private List parseRightOperand(Object rightValue, PolicyContext context) 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) { - if (bpnGroupHolder.allowedGroups.isEmpty()) { - return true; - } - return !evaluateEquals(bpnGroupHolder); } @@ -186,24 +173,20 @@ private Boolean evaluateEquals(BpnGroupHolder bpnGroupHolder) { return bpnGroupHolder.allowedGroups.equals(bpnGroupHolder.assignedGroups); } - private boolean evaluateIsAllOf(BpnGroupHolder bpnGroupHolder) { - if (bpnGroupHolder.allowedGroups.isEmpty()) { - return false; + private boolean evaluateIsAnyOf(BpnGroupHolder bpnGroupHolder) { + if (bpnGroupHolder.allowedGroups.isEmpty() && bpnGroupHolder.assignedGroups.isEmpty()) { + return true; } - var assigned = bpnGroupHolder.assignedGroups; - return bpnGroupHolder.allowedGroups + var allowedGroups = bpnGroupHolder.allowedGroups; + return bpnGroupHolder.assignedGroups .stream() .distinct() - .allMatch(assigned::contains); + .anyMatch(allowedGroups::contains); } private boolean evaluateIsNoneOf(BpnGroupHolder bpnGroupHolder) { - if (bpnGroupHolder.assignedGroups == null) { - return true; - } - - return !evaluateIn(bpnGroupHolder); + return !evaluateIsAnyOf(bpnGroupHolder); } /** 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 c5b743b45..ea0a9d739 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 @@ -60,6 +60,7 @@ import static org.mockito.ArgumentMatchers.endsWith; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -123,46 +124,27 @@ void evaluate_validOperator(String ignored, Operator operator, List assi assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isEqualTo(expectedOutcome); } - @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) + @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN", "IS_NONE_OF", "NEQ"}) @ParameterizedTest - void evaluate_noEntryForBpnWithNegativeOperator_shouldBeTrue(Operator operator) { + void evaluate_failedResolveForBpn_shouldBeFalse(Operator operator) { 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.notFound("foobar")); - - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); - } - - @EnumSource(value = Operator.class, names = {"NEQ", "IS_NONE_OF"}) - @ParameterizedTest - void evaluate_noGroupsAssignedToBpnWithNegativeOperator_shouldBeTrue(Operator operator) { - 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(Collections.emptyList())); - - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); - } - - @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) - @ParameterizedTest - void evaluate_noEntryForBpnWithPositiveOperator_shouldBeFalse(Operator operator) { - 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.notFound("foobar")); + when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.generalError("foobar")); assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); verify(context).reportProblem("foobar"); } - @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN"}) - @ParameterizedTest - void evaluate_noGroupsAssignedToBpnWithPositiveOperator_shouldBeFalse(Operator operator) { - var allowedGroups = List.of(TEST_GROUP_1, TEST_GROUP_2); + @Test + void evaluate_failedResolveForBpn_shouldEvaluateBasedOnOperator() { + var allowedGroups = List.of(); + 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.success(Collections.emptyList())); + when(store.resolveForBpn(TEST_BPN)).thenReturn(StoreResult.notFound("foobar")); - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); - verify(context).reportProblem("No groups were assigned to BPN " + TEST_BPN); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isTrue(); + verify(context, never()).reportProblem("foobar"); } @ArgumentsSource(OperatorForEmptyGroupsProvider.class) @@ -214,14 +196,14 @@ public Stream provideArguments(ExtensionContext extensionCo 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"), 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), 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), @@ -238,22 +220,22 @@ public Stream provideArguments(ExtensionContext extensionCo private static class OperatorForEmptyGroupsProvider implements ArgumentsProvider { @Override - public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + 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(), false), + Arguments.of(EQ, List.of(), true), Arguments.of(NEQ, assignedBpnGroups, true), - Arguments.of(NEQ, List.of(), true), + Arguments.of(NEQ, List.of(), false), Arguments.of(IN, assignedBpnGroups, false), - Arguments.of(IN, List.of(), false), + Arguments.of(IN, List.of(), true), Arguments.of(IS_ALL_OF, assignedBpnGroups, false), - Arguments.of(IS_ALL_OF, List.of(), false), + Arguments.of(IS_ALL_OF, List.of(), true), Arguments.of(IS_ANY_OF, assignedBpnGroups, false), - Arguments.of(IS_ANY_OF, List.of(), false), + Arguments.of(IS_ANY_OF, List.of(), true), Arguments.of(IS_NONE_OF, assignedBpnGroups, true), - Arguments.of(IS_NONE_OF, List.of(), true) + Arguments.of(IS_NONE_OF, List.of(), false) ); } } From 729fbb363da77220183fd3cf3ac34bff9abbdf8c Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Mon, 5 Aug 2024 14:45:00 +0100 Subject: [PATCH 14/17] fix: adjusted checkstyle --- .../functions/BusinessPartnerGroupFunctionTest.java | 1 - 1 file changed, 1 deletion(-) 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 ea0a9d739..3f3d9ffcf 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 @@ -37,7 +37,6 @@ import org.junit.jupiter.params.provider.ArgumentsSource; import org.junit.jupiter.params.provider.EnumSource; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Stream; From 8879a6a1998e34604e43fc77ba363ee15be571b9 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 7 Aug 2024 09:17:19 +0100 Subject: [PATCH 15/17] fix: removed Reason validation for failed BPN search --- .../BusinessPartnerGroupFunction.java | 9 ++------- .../BusinessPartnerGroupFunctionTest.java | 20 ++++--------------- 2 files changed, 6 insertions(+), 23 deletions(-) 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 8bd672001..a8ce9e6fa 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 @@ -127,13 +127,8 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P // BPN not found in database if (groups.failed()) { - boolean isNotFound = Reason.NOT_FOUND.equals(groups.getFailure().getReason()); - if (!isNotFound) { - policyContext.reportProblem(groups.getFailureDetail()); - return false; - } - - assignedGroups = List.of(); + policyContext.reportProblem(groups.getFailureDetail()); + return false; } // right-operand is anything other than String or Collection 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 3f3d9ffcf..ec7c3d323 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 @@ -123,27 +123,15 @@ void evaluate_validOperator(String ignored, Operator operator, List assi assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isEqualTo(expectedOutcome); } - @EnumSource(value = Operator.class, names = {"EQ", "IS_ANY_OF", "IS_ALL_OF", "IN", "IS_NONE_OF", "NEQ"}) - @ParameterizedTest - void evaluate_failedResolveForBpn_shouldBeFalse(Operator operator) { - 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.generalError("foobar")); - - assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); - verify(context).reportProblem("foobar"); - } - @Test - void evaluate_failedResolveForBpn_shouldEvaluateBasedOnOperator() { - var allowedGroups = List.of(); + 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)).isTrue(); - verify(context, never()).reportProblem("foobar"); + assertThat(function.evaluate(operator, allowedGroups, createPermission(operator, allowedGroups), context)).isFalse(); + verify(context).reportProblem("foobar"); } @ArgumentsSource(OperatorForEmptyGroupsProvider.class) From e7f4ceecd97607d4b2ac01ff7424feeaf82a35cf Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Wed, 7 Aug 2024 09:18:40 +0100 Subject: [PATCH 16/17] fix: adjusted list instantiation --- .../functions/BusinessPartnerGroupFunctionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ec7c3d323..009b0a9bf 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 @@ -138,7 +138,7 @@ void evaluate_failedResolveForBpn_shouldBeFalse() { @ParameterizedTest void evaluate_groupsAssignedButNoGroupsSentToEvaluate(Operator operator, List assignedBpnGroups, boolean expectedOutcome) { - var allowedGroups = List.of(); + 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(assignedBpnGroups)); From bb0e863babf22f06b5dd06799fd9542518ae79f3 Mon Sep 17 00:00:00 2001 From: danielkenji01 Date: Thu, 8 Aug 2024 11:42:14 +0100 Subject: [PATCH 17/17] fix: adjusted checkstyle --- .../functions/BusinessPartnerGroupFunctionTest.java | 2 -- 1 file changed, 2 deletions(-) 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 009b0a9bf..c26bd05d2 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 @@ -35,7 +35,6 @@ import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.ArgumentsProvider; import org.junit.jupiter.params.provider.ArgumentsSource; -import org.junit.jupiter.params.provider.EnumSource; import java.util.List; import java.util.Map; @@ -59,7 +58,6 @@ import static org.mockito.ArgumentMatchers.endsWith; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when;