Bugfix: the base criterion converter can interpret lists of values for IN

core/defaults/src/test/java/org/eclipse/dataspaceconnector/core/defaults/assetindex/AssetPredicateConverterTest.java

@@ -19,6 +19,8 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import java.util.List;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
@@ -77,7 +79,7 @@ void convert_operatorIn() {
                 .version("6.9")
                 .property("test-property", "somevalue")
                 .build();
-        var criterion = new Criterion(Asset.PROPERTY_NAME, "in", "(bob, alice)");
+        var criterion = new Criterion(Asset.PROPERTY_NAME, "in", List.of("bob", "alice"));
         var pred = converter.convert(criterion);
         assertThat(pred).isNotNull().accepts(asset);
 

core/defaults/src/test/java/org/eclipse/dataspaceconnector/core/defaults/assetindex/InMemoryAssetIndexTest.java

@@ -29,7 +29,6 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import static java.lang.String.format;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.eclipse.dataspaceconnector.spi.asset.AssetSelectorExpression.SELECT_ALL;
 
@@ -131,7 +130,7 @@ void queryAsset_operatorIn() {
         index.accept(testAsset2, createDataAddress(testAsset2));
         index.accept(testAsset3, createDataAddress(testAsset3));
 
-        var inExpr = format("(  %s )", String.join(", ", List.of(testAsset1.getId(), testAsset2.getId())));
+        var inExpr = List.of(testAsset1.getId(), testAsset2.getId());
         var selector = AssetSelectorExpression.Builder.newInstance()
                 .constraint(Asset.PROPERTY_ID, "IN", inExpr)
                 .build();
@@ -150,7 +149,7 @@ void queryAsset_operatorIn_notIn() {
         index.accept(testAsset2, createDataAddress(testAsset2));
         index.accept(testAsset3, createDataAddress(testAsset3));
 
-        var inExpr = format("(  %s )", String.join(", ", List.of("test-id1", "test-id2")));
+        var inExpr = List.of("test-id1", "test-id2");
         var selector = AssetSelectorExpression.Builder.newInstance()
                 .constraint(Asset.PROPERTY_ID, "IN", inExpr)
                 .build();
@@ -169,7 +168,7 @@ void queryAsset_operatorIn_noBrackets() {
         index.accept(testAsset2, createDataAddress(testAsset2));
         index.accept(testAsset3, createDataAddress(testAsset3));
 
-        var inExpr = String.join(", ", List.of(testAsset1.getId(), testAsset2.getId()));
+        var inExpr = List.of(testAsset1.getId(), testAsset2.getId());
         var selector = AssetSelectorExpression.Builder.newInstance()
                 .constraint(Asset.PROPERTY_ID, "IN", inExpr)
                 .build();
@@ -188,7 +187,7 @@ void queryAsset_operatorIn_noBracketsNoSpaces() {
         index.accept(testAsset2, createDataAddress(testAsset2));
         index.accept(testAsset3, createDataAddress(testAsset3));
 
-        var inExpr = String.join(",", List.of(testAsset1.getId(), testAsset2.getId()));
+        var inExpr = List.of(testAsset1.getId(), testAsset2.getId());
         var selector = AssetSelectorExpression.Builder.newInstance()
                 .constraint(Asset.PROPERTY_ID, "IN", inExpr)
                 .build();

spi/core-spi/src/main/java/org/eclipse/dataspaceconnector/spi/query/BaseCriterionToPredicateConverter.java

@@ -17,7 +17,7 @@
 
 import org.jetbrains.annotations.NotNull;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.Objects;
 import java.util.function.Predicate;
 
@@ -34,8 +34,10 @@ public Predicate<T> convert(Criterion criterion) {
         var operator = criterion.getOperator().toLowerCase();
 
         switch (operator) {
-            case "=": return equalPredicate(criterion);
-            case "in": return inPredicate(criterion);
+            case "=":
+                return equalPredicate(criterion);
+            case "in":
+                return inPredicate(criterion);
             default:
                 throw new IllegalArgumentException(String.format("Operator [%s] is not supported by this converter!", criterion.getOperator()));
         }
@@ -65,13 +67,19 @@ private Predicate<T> equalPredicate(Criterion criterion) {
     private Predicate<T> inPredicate(Criterion criterion) {
         return t -> {
             String property = property((String) criterion.getOperandLeft(), t);
-            var items = ((String) criterion.getOperandRight())
-                    .replace("(", "")
-                    .replace(")", "")
-                    .replace(" ", "")
-                    .split(",");
 
-            return Arrays.asList(items).contains(property);
+            var rightOp = criterion.getOperandRight();
+
+
+            if (rightOp instanceof Iterable) {
+                var items = new ArrayList<String>();
+                ((Iterable<?>) rightOp).forEach(o -> items.add(o.toString()));
+                return items.contains(property);
+            } else {
+                throw new IllegalArgumentException("Operator IN requires the right-hand operand to be an " + Iterable.class.getName() + " but was " + rightOp.getClass().getName());
+            }
+
+
         };
     }
 

spi/core-spi/src/test/java/org/eclipse/dataspaceconnector/spi/query/BaseCriterionToPredicateConverterTest.java

@@ -17,7 +17,10 @@
 import org.eclipse.dataspaceconnector.common.reflection.ReflectionUtil;
 import org.junit.jupiter.api.Test;
 
+import java.util.List;
+
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 class BaseCriterionToPredicateConverterTest {
 
@@ -33,8 +36,17 @@ void convertEqual() {
     }
 
     @Test
-    void convertIn() {
+    void convertIn_throwsException() {
         var predicate = converter.convert(new Criterion("value", "in", "(first, second)"));
+        assertThatThrownBy(() -> predicate.test(new TestObject("first")))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("Operator IN requires the right-hand operand to be an");
+
+    }
+
+    @Test
+    void convertIn() {
+        var predicate = converter.convert(new Criterion("value", "in", List.of("first", "second")));
 
         assertThat(predicate)
                 .accepts(new TestObject("first"), new TestObject("second"))
@@ -50,17 +62,17 @@ protected <R> R property(String key, Object object) {
     }
 
     private static class TestObject {
+        private final String value;
+
+        private TestObject(String value) {
+            this.value = value;
+        }
+
         @Override
         public String toString() {
             return "TestObject{" +
                     "value='" + value + '\'' +
                     '}';
         }
-
-        private final String value;
-
-        private TestObject(String value) {
-            this.value = value;
-        }
     }
 }