more flexible solution with eval functions

eclipse-tractusx · Jul 17, 2024 · 711d187 · 711d187
1 parent 524b4c2
commit 711d187
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 32 deletions.
diff --git a/...usx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java b/...usx/edc/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunction.java
@@ -24,18 +24,28 @@
 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.
  */
 public class BusinessPartnerNumberPermissionFunction implements AtomicConstraintFunction<Permission> {
 
     private static final List<Operator> 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<Operator> 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<Failure, Boolean> reportFailure(PolicyContext context) {
+        return f -> {
+            context.reportProblem(f.getFailureDetail());
+            return false;
+        };
+    }
+
+    private Result<Boolean> 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<Boolean> 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<Boolean> 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/validation/businesspartner/functions/BusinessPartnerNumberPermissionFunctionTest.java b/...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