From 524b4c2b36d9b3e444fcedde816d6d664dc15843 Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Tue, 16 Jul 2024 16:33:32 +0200 Subject: [PATCH 1/2] feat: add support for more operators --- ...sinessPartnerNumberPermissionFunction.java | 58 ++++++--- ...ssPartnerNumberPermissionFunctionTest.java | 122 +++++++++++++++--- 2 files changed, 145 insertions(+), 35 deletions(-) diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java index a7339c16a..0414905a2 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java @@ -25,23 +25,37 @@ import org.eclipse.edc.policy.model.Permission; import org.eclipse.edc.spi.agent.ParticipantAgent; +import java.util.Arrays; +import java.util.List; import java.util.Optional; -import static java.lang.String.format; - /** * AtomicConstraintFunction to validate business partner numbers for edc permissions. */ public class BusinessPartnerNumberPermissionFunction implements AtomicConstraintFunction { - public BusinessPartnerNumberPermissionFunction() { - } + private static final List SUPPORTED_OPERATORS = Arrays.asList( + Operator.EQ, + Operator.IN, + Operator.NEQ, + Operator.IS_ANY_OF, + Operator.IS_A, + Operator.IS_NONE_OF, + Operator.IS_ALL_OF, + Operator.HAS_PART + ); + private static final List SCALAR_OPERATORS = Arrays.asList( + Operator.EQ, + Operator.NEQ, + Operator.HAS_PART + ); + @SuppressWarnings({ "unchecked", "rawtypes" }) @Override public boolean evaluate(Operator operator, Object rightValue, Permission rule, PolicyContext context) { - - if (operator != Operator.EQ) { - var message = format("As operator only 'EQ' is supported. Unsupported operator: '%s'", operator); + + if (!SUPPORTED_OPERATORS.contains(operator)) { + var message = "Operator %s is not supported. Supported operators: %s".formatted(operator, SUPPORTED_OPERATORS); context.reportProblem(message); return false; } @@ -58,19 +72,27 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } - if ((rightValue instanceof String businessPartnerNumberStr)) { - if (businessPartnerNumberStr.equals(identity)) { - return true; - } else { - context.reportProblem("Identity of the participant not matching the expected one: " + businessPartnerNumberStr); - return false; + if (SCALAR_OPERATORS.contains(operator)) { + if (rightValue instanceof String businessPartnerNumberStr) { + return switch (operator) { + case EQ -> businessPartnerNumberStr.equals(identity); + case NEQ -> !businessPartnerNumberStr.equals(identity); + case HAS_PART -> identity.contains(businessPartnerNumberStr); + default -> false; + }; } + context.reportProblem("Invalid right-value: operator '%s' requires a 'String' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); } else { - var message = format("Invalid right operand value: expected 'String' but got '%s'", - Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null)); - context.reportProblem(message); - return false; + if ((rightValue instanceof List numbers)) { + return switch (operator) { + case IN, IS_A, IS_ANY_OF -> numbers.contains(identity); + case IS_ALL_OF -> numbers.stream().allMatch(o -> o.equals(identity)); + case IS_NONE_OF -> !numbers.contains(identity); + default -> false; + }; + } + context.reportProblem("Invalid right-value: operator '%s' requires a 'List' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); } - + return false; } } diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java index 67cff19fa..ce00ba068 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java @@ -25,28 +25,32 @@ import org.eclipse.edc.spi.agent.ParticipantAgent; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.ArgumentsProvider; +import org.junit.jupiter.params.provider.ArgumentsSource; import java.util.List; +import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class BusinessPartnerNumberPermissionFunctionTest { + private final Permission permission = mock(); private BusinessPartnerNumberPermissionFunction validation; - private PolicyContext policyContext; private ParticipantAgent participantAgent; - private Permission permission = mock(); - @BeforeEach void beforeEach() { this.policyContext = mock(PolicyContext.class); @@ -58,21 +62,18 @@ void beforeEach() { }; } - @ParameterizedTest - @EnumSource(Operator.class) - void testFailsOnUnsupportedOperations(Operator operator) { - if (operator == Operator.EQ) { // only allowed operator - return; - } - assertFalse(validation.evaluate(operator, "foo", permission, policyContext)); - verify(policyContext).reportProblem(argThat(message -> message.contains("As operator only 'EQ' is supported"))); + @ParameterizedTest(name = "Illegal Operator {0}") + @ArgumentsSource(IllegalOperatorProvider.class) + void testFailsOnUnsupportedOperations(Operator illegalOperator) { + assertFalse(validation.evaluate(illegalOperator, "foo", permission, policyContext)); + verify(policyContext).reportProblem(argThat(message -> message.startsWith("Operator %s is not supported.".formatted(illegalOperator)))); } @Test void testFailsOnUnsupportedRightValue() { when(participantAgent.getIdentity()).thenReturn("foo"); assertFalse(validation.evaluate(Operator.EQ, 1, permission, policyContext)); - verify(policyContext).reportProblem(argThat(message -> message.contains("Invalid right operand value: expected 'String' but got"))); + verify(policyContext).reportProblem(argThat(message -> message.contains("Invalid right-value: operator 'EQ' requires a 'String' but got a 'java.lang.Integer'"))); } @Test @@ -98,14 +99,101 @@ void testValidationWhenSingleParticipantIsValid() { void testValidationFailsInvalidIdentity() { when(participantAgent.getIdentity()).thenReturn("bar"); assertThat(validation.evaluate(Operator.EQ, "foo", permission, policyContext)).isFalse(); - verify(policyContext).reportProblem(argThat(message -> message.contains("Identity of the participant not matching the expected one: foo"))); } @Test void testValidationForMultipleParticipants() { + when(participantAgent.getIdentity()).thenReturn("quazz"); + assertThat(validation.evaluate(Operator.IN, List.of("foo", "bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IN, List.of(1, "foo"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IN, List.of("bar", "bar"), permission, policyContext)).isFalse(); + } + + @Test + void evaluate_neq() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.NEQ, "bar", permission, policyContext)).isTrue(); + + // these two should report a problem + assertThat(validation.evaluate(Operator.NEQ, 1, permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.NEQ, List.of("foo", "bar"), permission, policyContext)).isFalse(); + verify(policyContext, times(2)).reportProblem(startsWith("Invalid right-value: operator 'NEQ' requires a 'String' but got a")); + } - assertFalse(validation.evaluate(Operator.IN, List.of("foo", "bar"), permission, policyContext)); - assertFalse(validation.evaluate(Operator.IN, List.of(1, "foo"), permission, policyContext)); - assertFalse(validation.evaluate(Operator.IN, List.of("bar", "bar"), permission, policyContext)); + @Test + void evaluate_hasPart() { + when(participantAgent.getIdentity()).thenReturn("quizzquazz"); + assertThat(validation.evaluate(Operator.HAS_PART, "quizz", permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.HAS_PART, "quazz", permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.HAS_PART, "zzqua", permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.HAS_PART, "zzqui", permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.HAS_PART, "Quizz", permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.HAS_PART, List.of("quizz"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.HAS_PART, List.of("quizz", "quazz"), permission, policyContext)).isFalse(); + verify(policyContext, times(2)).reportProblem(startsWith("Invalid right-value: operator 'HAS_PART' requires a 'String' but got a")); + } + + @Test + void evaluate_in() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.IN, List.of("foo", "bar"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IN, List.of("foo"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IN, List.of("bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IN, "bar", permission, policyContext)).isFalse(); + verify(policyContext).reportProblem("Invalid right-value: operator 'IN' requires a 'List' but got a 'java.lang.String'"); + } + + @Test + void evaluate_isAnyOf() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.IS_ANY_OF, List.of("foo", "bar"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_ANY_OF, List.of("foo"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_ANY_OF, List.of("bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_ANY_OF, "bar", permission, policyContext)).isFalse(); + verify(policyContext).reportProblem("Invalid right-value: operator 'IS_ANY_OF' requires a 'List' but got a 'java.lang.String'"); + + } + + @Test + void evaluate_isA() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.IS_A, List.of("foo", "bar"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_A, List.of("foo"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_A, List.of("bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_A, "bar", permission, policyContext)).isFalse(); + verify(policyContext).reportProblem("Invalid right-value: operator 'IS_A' requires a 'List' but got a 'java.lang.String'"); + + } + + @Test + void evaluate_isAllOf() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.IS_ALL_OF, List.of("foo", "bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_ALL_OF, List.of("foo"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_ALL_OF, List.of("bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_ALL_OF, "bar", permission, policyContext)).isFalse(); + verify(policyContext).reportProblem("Invalid right-value: operator 'IS_ALL_OF' requires a 'List' but got a 'java.lang.String'"); + } + + @Test + void evaluate_isNoneOf() { + when(participantAgent.getIdentity()).thenReturn("foo"); + assertThat(validation.evaluate(Operator.IS_NONE_OF, List.of("foo", "bar"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_NONE_OF, List.of("foo"), permission, policyContext)).isFalse(); + assertThat(validation.evaluate(Operator.IS_NONE_OF, List.of("bar"), permission, policyContext)).isTrue(); + assertThat(validation.evaluate(Operator.IS_NONE_OF, "bar", permission, policyContext)).isFalse(); + verify(policyContext).reportProblem("Invalid right-value: operator 'IS_NONE_OF' requires a 'List' but got a 'java.lang.String'"); + } + + private static class IllegalOperatorProvider implements ArgumentsProvider { + @Override + public Stream provideArguments(ExtensionContext extensionContext) throws Exception { + return Stream.of( + Arguments.of(Operator.GEQ), + Arguments.of(Operator.GT), + Arguments.of(Operator.LEQ), + Arguments.of(Operator.LT) + ); + } } } From 711d187f376183677c68baa9ee5ae18bf6e2e9f7 Mon Sep 17 00:00:00 2001 From: Paul Latzelsperger Date: Wed, 17 Jul 2024 14:35:13 +0200 Subject: [PATCH 2/2] more flexible solution with eval functions --- ...sinessPartnerNumberPermissionFunction.java | 84 ++++++++++++------- ...ssPartnerNumberPermissionFunctionTest.java | 6 +- 2 files changed, 58 insertions(+), 32 deletions(-) diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java index 0414905a2..3f3d08abe 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/main/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java @@ -24,10 +24,20 @@ 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.Failure; +import org.eclipse.edc.spi.result.Result; +import org.jetbrains.annotations.NotNull; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.Optional; +import java.util.function.Function; + +import static org.eclipse.edc.policy.model.Operator.EQ; +import static org.eclipse.edc.policy.model.Operator.HAS_PART; +import static org.eclipse.edc.spi.result.Result.failure; +import static org.eclipse.edc.spi.result.Result.success; /** * AtomicConstraintFunction to validate business partner numbers for edc permissions. @@ -35,7 +45,7 @@ public class BusinessPartnerNumberPermissionFunction implements AtomicConstraintFunction { private static final List SUPPORTED_OPERATORS = Arrays.asList( - Operator.EQ, + EQ, Operator.IN, Operator.NEQ, Operator.IS_ANY_OF, @@ -44,13 +54,7 @@ public class BusinessPartnerNumberPermissionFunction implements AtomicConstraint Operator.IS_ALL_OF, Operator.HAS_PART ); - private static final List SCALAR_OPERATORS = Arrays.asList( - Operator.EQ, - Operator.NEQ, - Operator.HAS_PART - ); - @SuppressWarnings({ "unchecked", "rawtypes" }) @Override public boolean evaluate(Operator operator, Object rightValue, Permission rule, PolicyContext context) { @@ -72,27 +76,51 @@ public boolean evaluate(Operator operator, Object rightValue, Permission rule, P return false; } - if (SCALAR_OPERATORS.contains(operator)) { - if (rightValue instanceof String businessPartnerNumberStr) { - return switch (operator) { - case EQ -> businessPartnerNumberStr.equals(identity); - case NEQ -> !businessPartnerNumberStr.equals(identity); - case HAS_PART -> identity.contains(businessPartnerNumberStr); - default -> false; - }; - } - context.reportProblem("Invalid right-value: operator '%s' requires a 'String' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); - } else { - if ((rightValue instanceof List numbers)) { - return switch (operator) { - case IN, IS_A, IS_ANY_OF -> numbers.contains(identity); - case IS_ALL_OF -> numbers.stream().allMatch(o -> o.equals(identity)); - case IS_NONE_OF -> !numbers.contains(identity); - default -> false; - }; - } - context.reportProblem("Invalid right-value: operator '%s' requires a 'List' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); + return switch (operator) { + case EQ, IS_ALL_OF -> checkEquality(identity, rightValue, operator) + .orElse(reportFailure(context)); + case NEQ -> checkEquality(identity, rightValue, operator) + .map(b -> !b) + .orElse(reportFailure(context)); + case HAS_PART -> checkStringContains(identity, rightValue) + .orElse(reportFailure(context)); + case IN, IS_A, IS_ANY_OF -> + checkListContains(identity, rightValue, operator).orElse(reportFailure(context)); + case IS_NONE_OF -> checkListContains(identity, rightValue, operator) + .map(b -> !b) + .orElse(reportFailure(context)); + default -> false; + }; + } + + private @NotNull Function reportFailure(PolicyContext context) { + return f -> { + context.reportProblem(f.getFailureDetail()); + return false; + }; + } + + private Result checkListContains(String identity, Object rightValue, Operator operator) { + if (rightValue instanceof List numbers) { + return success(numbers.contains(identity)); + } + return failure("Invalid right-value: operator '%s' requires a 'List' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); + } + + private Result checkStringContains(String identity, Object rightValue) { + if (rightValue instanceof String bpnString) { + return success(identity.contains(bpnString)); + } + return failure("Invalid right-value: operator '%s' requires a 'String' but got a '%s'".formatted(HAS_PART, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); + } + + @SuppressWarnings({ "unchecked", "rawtypes" }) + private Result checkEquality(String identity, Object rightValue, Operator operator) { + if (rightValue instanceof String bpnString) { + return success(Objects.equals(identity, bpnString)); + } else if (rightValue instanceof List bpnList) { + return success(bpnList.stream().allMatch(bpn -> Objects.equals(identity, bpn))); } - return false; + return failure("Invalid right-value: operator '%s' requires a 'String' or a 'List' but got a '%s'".formatted(operator, Optional.of(rightValue).map(Object::getClass).map(Class::getName).orElse(null))); } } diff --git a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java index ce00ba068..e178c3838 100644 --- a/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java +++ b/edc-extensions/bpn-validation/bpn-validation-core/src/test/java/org/eclipse/tractusx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java @@ -73,7 +73,7 @@ void testFailsOnUnsupportedOperations(Operator illegalOperator) { void testFailsOnUnsupportedRightValue() { when(participantAgent.getIdentity()).thenReturn("foo"); assertFalse(validation.evaluate(Operator.EQ, 1, permission, policyContext)); - verify(policyContext).reportProblem(argThat(message -> message.contains("Invalid right-value: operator 'EQ' requires a 'String' but got a 'java.lang.Integer'"))); + verify(policyContext).reportProblem(argThat(message -> message.contains("Invalid right-value: operator 'EQ' requires a 'String' or a 'List' but got a 'java.lang.Integer'"))); } @Test @@ -116,8 +116,7 @@ void evaluate_neq() { // these two should report a problem assertThat(validation.evaluate(Operator.NEQ, 1, permission, policyContext)).isFalse(); - assertThat(validation.evaluate(Operator.NEQ, List.of("foo", "bar"), permission, policyContext)).isFalse(); - verify(policyContext, times(2)).reportProblem(startsWith("Invalid right-value: operator 'NEQ' requires a 'String' but got a")); + assertThat(validation.evaluate(Operator.NEQ, List.of("foo", "bar"), permission, policyContext)).isTrue(); } @Test @@ -172,7 +171,6 @@ void evaluate_isAllOf() { assertThat(validation.evaluate(Operator.IS_ALL_OF, List.of("foo"), permission, policyContext)).isTrue(); assertThat(validation.evaluate(Operator.IS_ALL_OF, List.of("bar"), permission, policyContext)).isFalse(); assertThat(validation.evaluate(Operator.IS_ALL_OF, "bar", permission, policyContext)).isFalse(); - verify(policyContext).reportProblem("Invalid right-value: operator 'IS_ALL_OF' requires a 'List' but got a 'java.lang.String'"); } @Test