Improved: Refactoring ‘EntityCondition’ - Remove EntityConditionBase …

…class

(OFBIZ-10691)

This is removing an abuse of inheritance for code reuse which was·
breaking Liskov substitution principle.  This has been achieved with
the following changes:

* The ‘Serializable’ interface which was implemented by
‘EntityConditionBase’ is now implemented directly by the·
‘EntityCondition’, ‘EntityConditionValue’, and ‘EntityOperator’
classes.

* ‘emptyList’ and ‘_emptyMap’ useless static members has been removed
and ‘emptyAliases’ has been move down to the ‘EntityConditionValue’
subclass.

* The ‘castBoolean’ method has been removed due to the automatic
boxing/unboxing of boolean and Boolean values which make it useless.

* ‘equals’ and ‘hashCode’ pseudo abstract methods (meaning methods
        throwing an unsupported operation exception) has been moved down to
the ‘EntityCondition’ and ‘EntityConditionValue’ sub classes.
‘Objects#equals’ and ‘Objects#hashCode’ standard methods has been
used in the EntityCondition class hierarchy instead.

* The ‘getColName’ methods has been moved down to the·
‘EntityConditionValue’ class which is the unique class using those
methods.

* The remaining static methods ‘getField’ and ‘addValue’ has been
moved to a new ‘EntityConditionUtils’ class.  The unused override of
‘addValue’ has been removed which allowed us to make this method
static.

Thanks Mathieu for the contribution

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

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

@@ -93,14 +93,14 @@ public void addSqlValue(StringBuilder sql, ModelEntity entity, List<EntityCondit
             ecv.addSqlValue(sql, entity, entityConditionParams, false, datasourceInfo);
             field = ecv.getModelField(entity);
         } else if (compat && lhs instanceof String) {
-            field = getField(entity, (String) lhs);
+            field = EntityConditionUtils.getField(entity, (String) lhs);
             if (field == null) {
                 sql.append(lhs);
             } else {
                 sql.append(field.getColName());
             }
         } else {
-            addValue(sql, null, lhs, entityConditionParams);
+            EntityConditionUtils.addValue(sql, null, lhs, entityConditionParams);
             field = null;
         }
 
@@ -125,7 +125,7 @@ protected void makeRHSWhereStringValue(ModelEntity entity, List<EntityConditionP
             }
             ecv.addSqlValue(sql, entity, entityConditionParams, false, datasourceInfo);
         } else {
-            addValue(sql, field, rhs, entityConditionParams);
+            EntityConditionUtils.addValue(sql, field, rhs, entityConditionParams);
         }
     }
 

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

@@ -18,6 +18,7 @@
  *******************************************************************************/
 package org.apache.ofbiz.entity.condition;
 
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -42,7 +43,7 @@
  *
  */
 @SuppressWarnings("serial")
-public abstract class EntityCondition extends EntityConditionBase implements IsEmpty {
+public abstract class EntityCondition implements IsEmpty, Serializable {
 
     public static <L,R,LL,RR> EntityExpr makeCondition(L lhs, EntityComparisonOperator<LL,RR> operator, R rhs) {
         return new EntityExpr(lhs, operator, rhs);
@@ -137,4 +138,15 @@ public Boolean eval(Delegator delegator, Map<String, ? extends Object> map) {
     abstract public boolean mapMatches(Delegator delegator, Map<String, ? extends Object> map);
 
     abstract public EntityCondition freeze();
+
+    @Override
+    public boolean equals(Object obj) {
+        throw new UnsupportedOperationException("equals:" + getClass().getName());
+    }
+
+    @Override
+    public int hashCode() {
+        throw new UnsupportedOperationException("hashCode: " + getClass().getName());
+    }
+
 }

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

@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.ofbiz.entity.condition;
+
+import java.util.List;
+
+import org.apache.ofbiz.entity.jdbc.SqlJdbcUtil;
+import org.apache.ofbiz.entity.model.ModelEntity;
+import org.apache.ofbiz.entity.model.ModelField;
+
+/**
+ * Auxiliary methods used by condition expressions.
+ */
+final class EntityConditionUtils {
+
+    /**
+     * Calls {@link ModelEntity#getField(String)} if the entity model is not null.
+     *
+     * @param modelEntity the entity model to query
+     * @param fieldName the name of the field to get from {@code ModelEntity}
+     * @return the field corresponding to {@code fieldName} in {@code ModelEntity}
+     */
+    static ModelField getField(ModelEntity modelEntity, String fieldName) {
+        return (modelEntity == null) ? null : modelEntity.getField(fieldName);
+    }
+
+    /**
+     * Calls {@link SqlJdbcUtil#addValue(StringBuilder, ModelField, Object, List)}
+     * if the condition parameters are not null.
+     *
+     * @param buffer the buffer that will receive the SQL dump
+     * @param field the field to dump
+     * @param value the value to dump
+     * @param params the condition parameters
+     */
+    static void addValue(StringBuilder buffer, ModelField field, Object value, List<EntityConditionParam> params) {
+        SqlJdbcUtil.addValue(buffer, params == null ? null : field, value, params);
+    }
+}

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

@@ -18,7 +18,10 @@
  *******************************************************************************/
 package org.apache.ofbiz.entity.condition;
 
+import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -34,8 +37,9 @@
  *
  */
 @SuppressWarnings("serial")
-public abstract class EntityConditionValue extends EntityConditionBase {
+public abstract class EntityConditionValue implements Serializable {
 
+    private static final Map<String, String> emptyAliases = Collections.unmodifiableMap(new HashMap<>());
     public static EntityConditionValue CONSTANT_NUMBER(Number value) { return new ConstantNumberValue(value); }
     public static final class ConstantNumberValue extends EntityConditionValue {
         private Number value;
@@ -109,4 +113,14 @@ public String toString() {
         toString(sql);
         return sql.toString();
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        throw new UnsupportedOperationException("equals:" + getClass().getName());
+    }
+
+    @Override
+    public int hashCode() {
+        throw new UnsupportedOperationException("hashCode: " + getClass().getName());
+    }
 }

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

@@ -22,6 +22,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.ofbiz.base.util.UtilDateTime;
 import org.apache.ofbiz.entity.Delegator;
@@ -73,12 +74,12 @@ public boolean equals(Object obj) {
             return false;
         }
         EntityDateFilterCondition other = (EntityDateFilterCondition) obj;
-        return equals(fromDateName, other.fromDateName) && equals(thruDateName, other.thruDateName);
+        return Objects.equals(fromDateName, other.fromDateName) && Objects.equals(thruDateName, other.thruDateName);
     }
 
     @Override
     public int hashCode() {
-        return hashCode(fromDateName) ^ hashCode(thruDateName);
+        return Objects.hashCode(fromDateName) ^ Objects.hashCode(thruDateName);
     }
 
     @Override

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

@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.base.util.ObjectType;
@@ -133,16 +134,6 @@ public void checkCondition(ModelEntity modelEntity) throws GenericModelException
         }
     }
 
-    @Override
-    protected void addValue(StringBuilder buffer, ModelField field, Object value, List<EntityConditionParam> params) {
-        if (rhs instanceof EntityFunction.UPPER) {
-            if (value instanceof String) {
-                value = ((String) value).toUpperCase(Locale.getDefault());
-            }
-        }
-        super.addValue(buffer, field, value, params);
-    }
-
     @Override
     public EntityCondition freeze() {
         return operator.freeze(lhs, rhs);
@@ -263,15 +254,13 @@ public boolean equals(Object obj) {
         if (!(obj instanceof EntityExpr)) {
             return false;
         }
-        EntityExpr other = (EntityExpr) obj;
-        return equals(lhs, other.lhs) && equals(operator, other.operator)
-                && equals(rhs, other.rhs);
+        EntityExpr ee = (EntityExpr) obj;
+        return Objects.equals(lhs, ee.lhs) && Objects.equals(operator, ee.operator) && Objects.equals(rhs, ee.rhs);
+
     }
 
     @Override
     public int hashCode() {
-        return hashCode(lhs) +
-               hashCode(operator) +
-               hashCode(rhs);
+        return Objects.hashCode(lhs) + Objects.hashCode(operator) + Objects.hashCode(rhs);
     }
 }