Criteria. Separate ExpressionVisitor into Visitor and BiVisitor (with…

… payload) Not all users require payload with visitor pattern. Introduce a simpler visitor API
immutables · May 16, 2019 · ae11efa · ae11efa
1 parent 2b08e69
commit ae11efa
Show file tree

Hide file tree

Showing 12 changed files with 91 additions and 47 deletions.
diff --git a/criteria/common/src/org/immutables/criteria/InMemoryExpressionEvaluator.java b/criteria/common/src/org/immutables/criteria/InMemoryExpressionEvaluator.java
@@ -54,10 +54,10 @@ public static <T> Predicate<T> of(Expression expression) {
   public boolean test(T instance) {
     Objects.requireNonNull(instance, "instance");
     final LocalVisitor visitor = new LocalVisitor(instance);
-    return Boolean.TRUE.equals(expression.accept(visitor, null));
+    return Boolean.TRUE.equals(expression.accept(visitor));
   }
 
-  private static class LocalVisitor implements ExpressionVisitor<Object, Void> {
+  private static class LocalVisitor implements ExpressionVisitor<Object> {
 
     private final ValueExtractor<Object> extractor;
 
@@ -66,14 +66,14 @@ private LocalVisitor(Object instance) {
     }
 
     @Override
-    public Object visit(Call call, Void context) {
+    public Object visit(Call call) {
       final Operator op = call.getOperator();
       final List<Expression> args = call.getArguments();
 
       if (op == Operators.EQUAL || op == Operators.NOT_EQUAL) {
         Preconditions.checkArgument(args.size() == 2, "Size should be 2 for %s but was %s", op, args.size());
-        final Object left = args.get(0).accept(this, null);
-        final Object right = args.get(1).accept(this, null);
+        final Object left = args.get(0).accept(this);
+        final Object right = args.get(1).accept(this);
 
         if (left == UNKNOWN || right == UNKNOWN) {
           return UNKNOWN;
@@ -85,12 +85,12 @@ public Object visit(Call call, Void context) {
 
       if (op == Operators.IN || op == Operators.NOT_IN) {
         Preconditions.checkArgument(args.size() == 2, "Size should be 2 for %s but was %s", op, args.size());
-        final Object left = args.get(0).accept(this, null);
+        final Object left = args.get(0).accept(this);
         if (left == UNKNOWN) {
           return UNKNOWN;
         }
         @SuppressWarnings("unchecked")
-        final Iterable<Object> right = (Iterable<Object>) args.get(1).accept(this, null);
+        final Iterable<Object> right = (Iterable<Object>) args.get(1).accept(this);
         Preconditions.checkNotNull(right, "not expected to be null %s", args.get(1));
         final Stream<Object> stream = StreamSupport.stream(right.spliterator(), false);
 
@@ -99,7 +99,7 @@ public Object visit(Call call, Void context) {
 
       if (op == Operators.IS_ABSENT || op == Operators.IS_PRESENT) {
         Preconditions.checkArgument(args.size() == 1, "Size should be 1 for %s but was %s", op, args.size());
-        final Object left = args.get(0).accept(this, null);
+        final Object left = args.get(0).accept(this);
 
         if (left instanceof java.util.Optional) {
           Optional<?> opt = (java.util.Optional<?>) left;
@@ -123,7 +123,7 @@ public Object visit(Call call, Void context) {
         Preconditions.checkArgument(!args.isEmpty(), "empty args for %s", op);
         boolean prev = Boolean.TRUE;
         for (Expression exp:args) {
-          Object result = exp.accept(this, null);
+          Object result = exp.accept(this);
           if (Boolean.FALSE.equals(result)) {
             return Boolean.FALSE;
           } else if (result == null || result == UNKNOWN) {
@@ -140,7 +140,7 @@ public Object visit(Call call, Void context) {
         Preconditions.checkArgument(!args.isEmpty(), "empty args for %s", op);
         boolean prev = Boolean.FALSE;
         for (Expression exp:args) {
-          Object result = exp.accept(this, null);
+          Object result = exp.accept(this);
           if (Boolean.TRUE.equals(result)) {
             return Boolean.TRUE;
           } else if (result == null || result == UNKNOWN) {
@@ -160,12 +160,12 @@ public Object visit(Call call, Void context) {
         Preconditions.checkArgument(args.size() == 2, "Size should be 2 for %s but was %s", op, args.size());
 
         @SuppressWarnings("unchecked")
-        Comparable<Object> left = (Comparable<Object>) args.get(0).accept(this, null);
+        Comparable<Object> left = (Comparable<Object>) args.get(0).accept(this);
         if (left == UNKNOWN || left == null) {
           return UNKNOWN;
         }
         @SuppressWarnings("unchecked")
-        Comparable<Object> right = (Comparable<Object>) args.get(1).accept(this, null);
+        Comparable<Object> right = (Comparable<Object>) args.get(1).accept(this);
         if (right == UNKNOWN || right == null) {
           return UNKNOWN;
         }
@@ -187,12 +187,12 @@ public Object visit(Call call, Void context) {
     }
 
     @Override
-    public Object visit(Literal literal, Void context) {
+    public Object visit(Literal literal) {
       return literal.value();
     }
 
     @Override
-    public Object visit(Path path, Void context) {
+    public Object visit(Path path) {
       return extractor.extract(path.path());
     }
   }

diff --git a/criteria/common/src/org/immutables/criteria/constraints/DebugExpressionVisitor.java b/criteria/common/src/org/immutables/criteria/constraints/DebugExpressionVisitor.java
@@ -7,7 +7,7 @@
 /**
  * Used to output expression tree as string. Useful for debugging expressions.
  */
-public class DebugExpressionVisitor<Void> implements ExpressionVisitor<Void, Void> {
+public class DebugExpressionVisitor<Void> implements ExpressionVisitor<Void> {
 
   private final PrintWriter writer;
 
@@ -18,27 +18,27 @@ public DebugExpressionVisitor(PrintWriter writer) {
   }
 
   @Override
-  public Void visit(Call call, Void context) {
+  public Void visit(Call call) {
     writer.println();
     writer.print(String.join("", Collections.nCopies(depth * 2, " ")));
     writer.print("call op=" + call.getOperator());
     for (Expression expr: call.getArguments()) {
       depth++;
-      expr.accept(this, null);
+      expr.accept(this);
       depth--;
     }
     return null;
   }
 
   @Override
-  public Void visit(Literal literal, Void context) {
+  public Void visit(Literal literal) {
     writer.print(" literal=");
     writer.print(literal.value());
     return null;
   }
 
   @Override
-  public Void visit(Path path, Void context) {
+  public Void visit(Path path) {
     writer.print(" path=");
     writer.print(path.path());
     return null;

diff --git a/criteria/common/src/org/immutables/criteria/constraints/DnfExpression.java b/criteria/common/src/org/immutables/criteria/constraints/DnfExpression.java
@@ -30,7 +30,7 @@ static DnfExpression create() {
 
   @Nullable
   @Override
-  public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+  public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
     return simplify().accept(visitor, context);
   }
 

diff --git a/criteria/common/src/org/immutables/criteria/constraints/Expression.java b/criteria/common/src/org/immutables/criteria/constraints/Expression.java
@@ -5,13 +5,17 @@
 /**
  * Expression is a <a href="https://cs.lmu.edu/~ray/notes/ir/">Intermediate Representation</a> (IR)
  * generated by criteria DSL (front-end in compiler terminology).
- *
+ * <p>
  * Usually, it is represented as <a href="https://en.wikipedia.org/wiki/Abstract_syntax_tree">AST</a>.
- *
  */
 public interface Expression {
 
   @Nullable
-  <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context);
+  <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context);
 
-}
+  @Nullable
+  default <R> R accept(ExpressionVisitor<R> visitor) {
+    return accept(Expressions.toBiVisitor(visitor), null);
+  }
+
+}
diff --git a/criteria/common/src/org/immutables/criteria/constraints/ExpressionBiVisitor.java b/criteria/common/src/org/immutables/criteria/constraints/ExpressionBiVisitor.java
@@ -0,0 +1,19 @@
+package org.immutables.criteria.constraints;
+
+import javax.annotation.Nullable;
+
+/**
+ * Visitor which also accepts a payload.
+ *
+ * @param <V> visitor return type
+ * @param <C> context type
+ */
+public interface ExpressionBiVisitor<V, C> {
+
+  V visit(Call call, @Nullable C context);
+
+  V visit(Literal literal, @Nullable C context);
+
+  V visit(Path path, @Nullable C context);
+
+}
diff --git a/criteria/common/src/org/immutables/criteria/constraints/ExpressionVisitor.java b/criteria/common/src/org/immutables/criteria/constraints/ExpressionVisitor.java
@@ -1,18 +1,18 @@
 package org.immutables.criteria.constraints;
 
-import javax.annotation.Nullable;
-
 /**
+ * Visitor pattern for traversing a tree of expressions.
+ *
+ * Consider using {@link ExpressionBiVisitor} if you need to propagate some context / payload.
  *
  * @param <V> visitor return type
- * @param <C> context type
  */
-public interface ExpressionVisitor<V, C> {
+public interface ExpressionVisitor<V> {
 
-  V visit(Call call, @Nullable C context);
+  V visit(Call call);
 
-  V visit(Literal literal, @Nullable C context);
+  V visit(Literal literal);
 
-  V visit(Path path, @Nullable C context);
+  V visit(Path path);
 
 }
diff --git a/criteria/common/src/org/immutables/criteria/constraints/Expressions.java b/criteria/common/src/org/immutables/criteria/constraints/Expressions.java
@@ -27,7 +27,7 @@ public String path() {
 
       @Nullable
       @Override
-      public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+      public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
         return visitor.visit(this, context);
       }
     };
@@ -52,7 +52,7 @@ public Type valueType() {
 
       @Nullable
       @Override
-      public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+      public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
         return visitor.visit(this, context);
       }
     };
@@ -87,10 +87,32 @@ private static  Expression reduce(Operator operator, Iterable<? extends Expressi
     return call(operator, expressions);
   }
 
+  /**
+   * Converts a {@link ExpressionVisitor} into a {@link ExpressionBiVisitor} (with ignored payload).
+   */
+  static <V> ExpressionBiVisitor<V, Void> toBiVisitor(ExpressionVisitor<V> visitor) {
+    return new ExpressionBiVisitor<V, Void>() {
+      @Override
+      public V visit(Call call, @Nullable Void context) {
+        return visitor.visit(call);
+      }
+
+      @Override
+      public V visit(Literal literal, @Nullable Void context) {
+        return visitor.visit(literal);
+      }
+
+      @Override
+      public V visit(Path path, @Nullable Void context) {
+        return visitor.visit(path);
+      }
+    };
+  }
+
   /**
    * Combines expressions <a href="https://en.wikipedia.org/wiki/Disjunctive_normal_form">Disjunctive normal form</a>
    */
-  public static  Expression dnf(Operator operator, Expression existing, Expression newExpression) {
+  public static Expression dnf(Operator operator, Expression existing, Expression newExpression) {
     if (!(operator == Operators.AND || operator == Operators.OR)) {
       throw new IllegalArgumentException(String.format("Expected %s for operator but got %s",
               Arrays.asList(Operators.AND, Operators.OR), operator));
@@ -129,7 +151,7 @@ public Operator getOperator() {
 
       @Nullable
       @Override
-      public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+      public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
         return visitor.visit(this, context);
       }
     };

diff --git a/criteria/common/src/org/immutables/criteria/constraints/NilExpression.java b/criteria/common/src/org/immutables/criteria/constraints/NilExpression.java
@@ -15,7 +15,7 @@ private NilExpression() {}
 
   @Nullable
   @Override
-  public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+  public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
     throw new UnsupportedOperationException(String.format("Can't visit %s", getClass().getSimpleName()));
   }
 

diff --git a/criteria/common/src/org/immutables/criteria/constraints/NullLiteral.java b/criteria/common/src/org/immutables/criteria/constraints/NullLiteral.java
@@ -29,7 +29,7 @@ public Type valueType() {
 
   @Nullable
   @Override
-  public <R, C> R accept(ExpressionVisitor<R, C> visitor, @Nullable C context) {
+  public <R, C> R accept(ExpressionBiVisitor<R, C> visitor, @Nullable C context) {
     return visitor.visit(this, context);
   }
 }
diff --git a/criteria/common/test/org/immutables/criteria/PersonTest.java b/criteria/common/test/org/immutables/criteria/PersonTest.java
@@ -92,7 +92,7 @@ public void debug() {
             .firstName.isEqualTo("John");
 
     StringWriter out = new StringWriter();
-    crit.expression().accept(new DebugExpressionVisitor<>(new PrintWriter(out)), null);
+    crit.expression().accept(new DebugExpressionVisitor<>(new PrintWriter(out)));
     check(out.toString()).isNonEmpty();
   }
 

diff --git a/criteria/mongo/src/org/immutables/criteria/mongo/MongoVisitor.java b/criteria/mongo/src/org/immutables/criteria/mongo/MongoVisitor.java
@@ -19,15 +19,14 @@
 import org.immutables.criteria.constraints.Operators;
 import org.immutables.criteria.constraints.Path;
 
-import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Objects;
 import java.util.stream.Collectors;
 
 /**
  * Generates mongo query using visitor API.
  */
-class MongoVisitor implements ExpressionVisitor<BsonValue, Void> {
+class MongoVisitor implements ExpressionVisitor<BsonValue> {
 
   private final CodecRegistry registry;
 
@@ -36,7 +35,7 @@ class MongoVisitor implements ExpressionVisitor<BsonValue, Void> {
   }
 
   @Override
-  public BsonValue visit(Call call, @Nullable Void context) {
+  public BsonValue visit(Call call) {
     final Operator op = call.getOperator();
     final List<Expression> args = call.getArguments();
 
@@ -45,8 +44,8 @@ public BsonValue visit(Call call, @Nullable Void context) {
       Preconditions.checkArgument(args.get(0) instanceof Path, "first argument should be path access");
       Preconditions.checkArgument(args.get(1) instanceof Literal, "second argument should be literal");
 
-      BsonValue field = args.get(0).accept(this, null);
-      BsonValue value = args.get(1).accept(this, null);
+      BsonValue field = args.get(0).accept(this);
+      BsonValue value = args.get(1).accept(this);
       Preconditions.checkNotNull(field, "field");
       Preconditions.checkNotNull(value, "value");
 
@@ -60,7 +59,7 @@ public BsonValue visit(Call call, @Nullable Void context) {
       final BsonDocument doc = new BsonDocument();
 
       final BsonArray array = call.getArguments().stream()
-              .map(a -> a.accept(this, null))
+              .map(a -> a.accept(this))
               .collect(Collectors.toCollection(BsonArray::new));
 
       doc.put(op == Operators.AND ? "$and" : "$or", array);
@@ -72,7 +71,7 @@ public BsonValue visit(Call call, @Nullable Void context) {
   }
 
   @Override
-  public BsonValue visit(Literal literal, @Nullable Void context) {
+  public BsonValue visit(Literal literal) {
     final Object value = literal.value();
 
     if (value == null) {
@@ -96,7 +95,7 @@ public BsonValue visit(Literal literal, @Nullable Void context) {
   }
 
   @Override
-  public BsonString visit(Path path, @Nullable Void context) {
+  public BsonString visit(Path path) {
     return new BsonString(path.path());
   }
 }
diff --git a/criteria/mongo/src/org/immutables/criteria/mongo/Mongos.java b/criteria/mongo/src/org/immutables/criteria/mongo/Mongos.java
@@ -16,7 +16,7 @@ private Mongos() {}
    */
   static <T> Bson toBson(CodecRegistry registry, Expression expression) {
     MongoVisitor visitor = new MongoVisitor(registry);
-    return expression.accept(visitor, null).asDocument();
+    return expression.accept(visitor).asDocument();
   }
 
 }