Improved: Refactoring ‘EntityCondition’ - Rename EntityConditionFunct…

…ion class to EntityNotCondition

(OFBIZ-10691)

‘EntityConditionFunction’ was representing an expression containing a
unary prefix condition operator or in other words a one argument
predicate.  Actually, there exists only one meaningful implementation
for this kind of operators in the SQL language which is the NOT·
predicate.  Having a ‘EntityNotCondition’ class allows us to have
simpler semantics while achieving the same completeness in term of SQL·
support.

The ‘EntityConditionVisitor’ and its documentation has been adapted to
the new class hierarchy.

Thanks Mathieu for the contribution

git-svn-id: https://svn.apache.org/repos/asf/ofbiz/ofbiz-framework/trunk@1850376 13f79535-47bb-0310-9956-ffa450edef68

framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionVisitor.java

@@ -34,8 +34,8 @@
  * <pre>{@code
  *     EntityExpr expr;
  *     expr.accept(new EntityConditionVisitor() {
- *         public void visit(EntityConditionFunction func) {
- *              system.out.println("EntityConditionFunction");
+ *         public void visit(EntityNotCondition cond) {
+ *              system.out.println("EntityNotCondition");
  *         }
  *
  *         public <T extends EntityCondition> void visit(EntityConditionList<T> l) {
@@ -66,7 +66,7 @@
  *     class ContainsRawCondition implements EntityConditionVisitor {
  *         public boolean hasRawCondition = false;
  *
- *         public void visit(EntityConditionFunction func) {}
+ *         public void visit(EntityNotCondition cond) {}
  *         public void visit(EntityFieldMap m) {}
  *         public void visit(EntityDateFilterCondition df) {}
  *
@@ -104,12 +104,12 @@
  */
 public interface EntityConditionVisitor {
     /**
-     * Visits an entity condition function.
+     * Visits an entity NOT expression.
      *
-     * @param func the visited class
-     * @see EntityConditionFunction
+     * @param cond the visited class
+     * @see EntityNotCondition
      */
-    void visit(EntityConditionFunction func);
+    void visit(EntityNotCondition cond);
 
     /**
      * Visits a list of entity conditions.

framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityConditionFunction.java → framework/entity/src/main/java/org/apache/ofbiz/entity/condition/EntityNotCondition.java

@@ -21,62 +21,27 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.ofbiz.entity.Delegator;
 import org.apache.ofbiz.entity.GenericModelException;
 import org.apache.ofbiz.entity.config.model.Datasource;
 import org.apache.ofbiz.entity.model.ModelEntity;
 
 /**
- * Base class for entity condition functions.
- *
+ * A condition expression which is prefixed by the {@code NOT} unary operator.
  */
 @SuppressWarnings("serial")
-public abstract class EntityConditionFunction implements EntityCondition {
-
-    public static final int ID_NOT = 1;
-
-    public static class NOT extends EntityConditionFunction {
-        public NOT(EntityCondition nested) { super(ID_NOT, "NOT", nested); }
-        @Override
-        public boolean mapMatches(Delegator delegator, Map<String, ? extends Object> map) {
-            return !condition.mapMatches(delegator, map);
-        }
-        @Override
-        public EntityCondition freeze() {
-            return new NOT(condition.freeze());
-        }
-    }
-
-    protected Integer idInt = null;
-    protected String codeString = null;
-    protected EntityCondition condition = null;
-
-    protected EntityConditionFunction(int id, String code, EntityCondition condition) {
-        init(id, code, condition);
-    }
-
-    public void init(int id, String code, EntityCondition condition) {
-        idInt = id;
-        codeString = code;
-        this.condition = condition;
-    }
-
-    public void reset() {
-        idInt = null;
-        codeString = null;
-        this.condition = null;
-    }
-
-    public String getCode() {
-        if (codeString == null) {
-            return "null";
-        }
-        return codeString;
-    }
-
-    public int getId() {
-        return idInt;
+public class EntityNotCondition implements EntityCondition {
+    private EntityCondition condition;
+
+    /**
+     * Instantiates a negation condition expression.
+     *
+     * @param cond the condition to negate
+     */
+    public EntityNotCondition(EntityCondition cond) {
+        condition = cond;
     }
 
     @Override
@@ -86,16 +51,13 @@ public void accept(EntityConditionVisitor visitor) {
 
     @Override
     public boolean equals(Object obj) {
-        if (!(obj instanceof EntityConditionFunction)) {
-            return false;
-        }
-        EntityConditionFunction otherFunc = (EntityConditionFunction) obj;
-        return this.idInt.equals(otherFunc.idInt) && (this.condition != null ? condition.equals(otherFunc.condition) : otherFunc.condition != null);
+        return (obj instanceof EntityNotCondition)
+                && Objects.equals(condition, ((EntityNotCondition) obj).condition);
     }
 
     @Override
     public int hashCode() {
-        return idInt.hashCode() ^ condition.hashCode();
+        return "NOT".hashCode() ^ condition.hashCode();
     }
 
     @Override
@@ -105,20 +67,30 @@ public boolean isEmpty() {
 
     @Override
     public String makeWhereString(ModelEntity modelEntity, List<EntityConditionParam> entityConditionParams, Datasource datasourceInfo) {
-        StringBuilder sb = new StringBuilder();
-        sb.append(codeString).append('(');
-        sb.append(condition.makeWhereString(modelEntity, entityConditionParams, datasourceInfo));
-        sb.append(')');
-        return sb.toString();
+        return new StringBuilder()
+                .append("NOT(")
+                .append(condition.makeWhereString(modelEntity, entityConditionParams, datasourceInfo))
+                .append(')')
+                .toString();
     }
 
     @Override
     public void checkCondition(ModelEntity modelEntity) throws GenericModelException {
         condition.checkCondition(modelEntity);
     }
 
+    @Override
+    public boolean mapMatches(Delegator delegator, Map<String, ? extends Object> map) {
+        return !condition.mapMatches(delegator, map);
+    }
+
     @Override
     public String toString() {
         return makeWhereString();
     }
+
+    @Override
+    public EntityCondition freeze() {
+        return new EntityNotCondition(condition.freeze());
+    }
 }

framework/entity/src/test/java/org/apache/ofbiz/entity/EntityConditionVisitorTests.java

@@ -28,7 +28,7 @@
 
 import org.apache.ofbiz.entity.condition.EntityComparisonOperator;
 import org.apache.ofbiz.entity.condition.EntityCondition;
-import org.apache.ofbiz.entity.condition.EntityConditionFunction;
+import org.apache.ofbiz.entity.condition.EntityNotCondition;
 import org.apache.ofbiz.entity.condition.EntityConditionList;
 import org.apache.ofbiz.entity.condition.EntityConditionVisitor;
 import org.apache.ofbiz.entity.condition.EntityDateFilterCondition;
@@ -50,8 +50,8 @@ public void basicTest() {
         PrintWriter pw = new PrintWriter(os);
         expr.accept(new EntityConditionVisitor() {
             @Override
-            public void visit(EntityConditionFunction func) {
-                pw.print("EntityConditionFunction");
+            public void visit(EntityNotCondition cond) {
+                pw.print("EntityNotCondition");
             }
 
             @Override
@@ -90,7 +90,7 @@ public void complexTest() {
         class ContainsRawCondition implements EntityConditionVisitor {
             public boolean hasRawCondition = false;
 
-            @Override public void visit(EntityConditionFunction func) {}
+            @Override public void visit(EntityNotCondition cond) {}
             @Override public void visit(EntityFieldMap m) {}
             @Override public void visit(EntityDateFilterCondition df) {}