svn commit: r1813640 - in /ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity: GenericDelegator.java GenericEntity.java GenericPK.java GenericValue.java

[mbrohl]

Author: mbrohl
Date: Sat Oct 28 15:19:56 2017
New Revision: 1813640

URL: http://svn.apache.org/viewvc?rev=1813640&view=rev
Log:
Improved: Fixing defects reported by FindBugs, package
org.apache.ofbiz.entity.
(OFBIZ-9716)

I modified the patch slightly.

Thanks Julian Leichert for reporting and providing the patch.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericDelegator.java
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericEntity.java
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericPK.java
    ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericValue.java

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericDelegator.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericDelegator.java?rev=1813640&r1=1813639&r2=1813640&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericDelegator.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericDelegator.java Sat Oct 28 15:19:56 2017
@@ -114,9 +114,9 @@ public class GenericDelegator implements
     protected EntityCrypto crypto = null;
 
     /** A ThreadLocal variable to allow other methods to specify a user identifier (usually the userLoginId, though technically the Entity Engine doesn't know anything about the UserLogin entity) */
-    protected static ThreadLocal<List<String>> userIdentifierStack = new ThreadLocal<List<String>>();
+    private static final ThreadLocal<List<String>> userIdentifierStack = new ThreadLocal<List<String>>();
     /** A ThreadLocal variable to allow other methods to specify a session identifier (usually the visitId, though technically the Entity Engine doesn't know anything about the Visit entity) */
-    protected static ThreadLocal<List<String>> sessionIdentifierStack = new ThreadLocal<List<String>>();
+    private static final ThreadLocal<List<String>> sessionIdentifierStack = new ThreadLocal<List<String>>();
 
     private boolean testMode = false;
     private boolean testRollbackInProgress = false;
@@ -786,7 +786,7 @@ public class GenericDelegator implements
             value.setDelegator(this);
 
             // if audit log on for any fields, save new value with no old value because it's a create
-            if (value != null && value.getModelEntity().getHasFieldWithAuditLog()) {
+            if (value.getModelEntity().getHasFieldWithAuditLog()) {
                 createEntityAuditLogAll(value, false, false);
             }
 
@@ -796,7 +796,7 @@ public class GenericDelegator implements
                 if (testMode) {
                     storeForTestRollback(new TestOperation(OperationType.INSERT, value));
                 }
-            } catch (GenericEntityException e) {
+            } catch (IllegalStateException | GenericEntityException e) {
                 // see if this was caused by an existing record before resetting the sequencer and trying again
                 // NOTE: use the helper directly so ECA rules, etc won't be run
 
@@ -843,7 +843,7 @@ public class GenericDelegator implements
 
             TransactionUtil.commit(beganTransaction);
             return value;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String entityName = value != null ? value.getEntityName() : "invalid Generic Value";
             String errMsg = "Failure in createSetNextSeqId operation for entity [" + entityName + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
@@ -877,7 +877,7 @@ public class GenericDelegator implements
             value.setDelegator(this);
 
             // if audit log on for any fields, save new value with no old value because it's a create
-            if (value != null && value.getModelEntity().getHasFieldWithAuditLog()) {
+            if (value.getModelEntity().getHasFieldWithAuditLog()) {
                 createEntityAuditLogAll(value, false, false);
             }
 
@@ -900,7 +900,7 @@ public class GenericDelegator implements
 
             TransactionUtil.commit(beganTransaction);
             return value;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in create operation for entity [" + (value != null ? value.getEntityName() : "value is null") + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1011,7 +1011,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_REMOVE, primaryKey, false);
             TransactionUtil.commit(beganTransaction);
             return num;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in removeByPrimaryKey operation for entity [" + primaryKey.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1068,7 +1068,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_REMOVE, value, false);
             TransactionUtil.commit(beganTransaction);
             return num;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in removeValue operation for entity [" + value.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1124,7 +1124,7 @@ public class GenericDelegator implements
             }
             TransactionUtil.commit(beganTransaction);
             return rowsAffected;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in removeByCondition operation for entity [" + entityName + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1206,7 +1206,7 @@ public class GenericDelegator implements
             }
             TransactionUtil.commit(beganTransaction);
             return rowsAffected;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in storeByCondition operation for entity [" + entityName + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1259,7 +1259,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_STORE, value, false);
             TransactionUtil.commit(beganTransaction);
             return retVal;
-        } catch (Exception e) {
+        } catch (IllegalStateException | GenericEntityException e) {
             String errMsg = "Failure in store operation for entity [" + value.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1345,7 +1345,7 @@ public class GenericDelegator implements
             }
             TransactionUtil.commit(beganTransaction);
             return numberChanged;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in storeAll operation: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1383,7 +1383,7 @@ public class GenericDelegator implements
             }
             TransactionUtil.commit(beganTransaction);
             return numRemoved;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in removeAll operation: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1456,7 +1456,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_FIND, (value == null ? primaryKey : value), false);
             TransactionUtil.commit(beganTransaction);
             return value;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in findOne operation for entity [" + entityName + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1495,7 +1495,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_FIND, primaryKey, false);
             TransactionUtil.commit(beganTransaction);
             return value;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in findByPrimaryKeyPartial operation for entity [" + primaryKey.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1593,7 +1593,7 @@ public class GenericDelegator implements
             }
             TransactionUtil.commit(beganTransaction);
             return list;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in findByCondition operation for entity [" + entityName + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1660,7 +1660,7 @@ public class GenericDelegator implements
             ecaRunner.evalRules(EntityEcaHandler.EV_RETURN, EntityEcaHandler.OP_FIND, dummyValue, false);
             TransactionUtil.commit(beganTransaction);
             return count;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in findListIteratorByCondition operation for entity [DynamicView]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -1691,7 +1691,7 @@ public class GenericDelegator implements
             List<GenericValue> result = helper.findByMultiRelation(value, modelRelationOne, modelEntityOne, modelRelationTwo, modelEntityTwo, orderBy);
             TransactionUtil.commit(beganTransaction);
             return result;
-        } catch (Exception e) {
+        } catch (GenericEntityException e) {
             String errMsg = "Failure in getMultiRelation operation for entity [" + value.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
             Debug.logError(e, errMsg, module);
             TransactionUtil.rollback(beganTransaction, errMsg, e);
@@ -2344,7 +2344,7 @@ public class GenericDelegator implements
                             if (highestSeqVal == null || seqVal > highestSeqVal.intValue()) {
                                 highestSeqVal = Integer.valueOf(seqVal);
                             }
-                        } catch (Exception e) {
+                        } catch (NumberFormatException e) {
                             Debug.logWarning("Error in make-next-seq-id converting SeqId [" + currentSeqId + "] in field: " + seqFieldName + " from entity: " + value.getEntityName() + " to a number: " + e.toString(), module);
                         }
                     }
@@ -2356,7 +2356,7 @@ public class GenericDelegator implements
 
                 // only commit the transaction if we started one...
                 TransactionUtil.commit(beganTransaction);
-            } catch (Exception e) {
+            } catch (GenericEntityException e) {
                 String errMsg = "Failure in setNextSubSeqId operation for entity [" + value.getEntityName() + "]: " + e.toString() + ". Rolling back transaction.";
                 Debug.logError(e, errMsg, module);
                 try {
@@ -2601,7 +2601,7 @@ public class GenericDelegator implements
         this.testMode = true;
     }
 
-    public final class TestOperation {
+    public final static class TestOperation {
         private final OperationType operation;
         private final GenericValue value;
 

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericEntity.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericEntity.java?rev=1813640&r1=1813639&r2=1813640&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericEntity.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericEntity.java Sat Oct 28 15:19:56 2017
@@ -46,6 +46,7 @@ import org.apache.ofbiz.base.util.Observ
 import org.apache.ofbiz.base.util.TimeDuration;
 import org.apache.ofbiz.base.util.UtilDateTime;
 import org.apache.ofbiz.base.util.UtilGenerics;
+import org.apache.ofbiz.base.util.UtilIO;
 import org.apache.ofbiz.base.util.UtilProperties;
 import org.apache.ofbiz.base.util.UtilValidate;
 import org.apache.ofbiz.base.util.UtilXml;
@@ -351,8 +352,7 @@ public class GenericEntity implements Ma
     public Delegator getDelegator() {
         if (internalDelegator == null) {
             if (delegatorName == null) delegatorName = "default";
-            if (delegatorName != null)
-                internalDelegator = DelegatorFactory.getDelegator(delegatorName);
+            internalDelegator = DelegatorFactory.getDelegator(delegatorName);
             if (internalDelegator == null) {
                 throw new IllegalStateException("[GenericEntity.getDelegator] could not find delegator with name " + delegatorName);
             }
@@ -449,7 +449,7 @@ public class GenericEntity implements Ma
             ModelFieldType type = null;
             try {
                 type = getDelegator().getEntityFieldType(getModelEntity(), modelField.getType());
-            } catch (GenericEntityException e) {
+            } catch (IllegalStateException | GenericEntityException e) {
                 Debug.logWarning(e, module);
             }
             if (type == null) {
@@ -471,9 +471,11 @@ public class GenericEntity implements Ma
                 if (value instanceof TimeDuration) {
                     try {
                         value = ObjectType.simpleTypeConvert(value, type.getJavaType(), null, null);
-                    } catch (GeneralException e) {}
+                    } catch (GeneralException e) {
+                        Debug.logError(e, module);
+                    }
                 } else if ((value instanceof String) && "byte[]".equals(type.getJavaType())) {
-                    value = ((String) value).getBytes();
+                    value = ((String) value).getBytes(UtilIO.getUtf8());
                 }
                 if (!ObjectType.instanceOf(value, type.getJavaType())) {
                     if (!("java.sql.Blob".equals(type.getJavaType()) && (value instanceof byte[] || ObjectType.instanceOf(value, ByteBuffer.class)))) {
@@ -529,8 +531,10 @@ public class GenericEntity implements Ma
 
         ModelFieldType type = null;
         try {
-            type = getDelegator().getEntityFieldType(getModelEntity(), field.getType());
-        } catch (GenericEntityException e) {
+            if (field != null) {
+                type = getDelegator().getEntityFieldType(getModelEntity(), field.getType());
+            }
+        } catch (IllegalStateException | GenericEntityException e) {
             Debug.logWarning(e, module);
         }
         if (type == null) throw new IllegalArgumentException("Type " + field.getType() + " not found");
@@ -633,7 +637,7 @@ public class GenericEntity implements Ma
         Object obj = get(name);
 
         if (obj == null) {
-            return null;
+            return false;
         }
         if (obj instanceof Boolean) {
             return (Boolean) obj;
@@ -709,7 +713,7 @@ public class GenericEntity implements Ma
         // this "hack" is needed for now until the Double/BigDecimal issues are all resolved
         Object value = get(name);
         if (value instanceof BigDecimal) {
-            return new Double(((BigDecimal) value).doubleValue());
+            return Double.valueOf(((BigDecimal) value).doubleValue());
         }
         return (Double) value;
     }
@@ -1148,7 +1152,7 @@ public class GenericEntity implements Ma
                 boolean b1 = obj instanceof byte [];
                 if (b1) {
                     byte [] binData = (byte [])obj;
-                    String strData = new String(Base64.base64Encode(binData));
+                    String strData = new String(Base64.base64Encode(binData), UtilIO.getUtf8());
                     cdataMap.put(name, strData);
                 } else {
                     Debug.logWarning("Field:" + name + " is not of type 'byte[]'. obj: " + obj, module);
@@ -1325,7 +1329,7 @@ public class GenericEntity implements Ma
                 // random encoding; just treat it as a series of raw bytes.
                 // This won't give the same output as the value stored in the
                 // database, but should be good enough for printing
-                curValue = HashCrypt.cryptBytes(null, null, encryptField.getBytes());
+                curValue = HashCrypt.cryptBytes(null, null, encryptField.getBytes(UtilIO.getUtf8()));
             }
             theString.append('[');
             theString.append(curKey);
@@ -1612,8 +1616,18 @@ public class GenericEntity implements Ma
             return "[null-field]";
         }
 
+        @Override
+        public int hashCode() {
+            return 42;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            return this == o;
+        }
+
         public int compareTo(NullField other) {
-            return this != other ? -1 : 0;
+            return equals(other) ? 0 : -1;
         }
     }
 }

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericPK.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericPK.java?rev=1813640&r1=1813639&r2=1813640&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericPK.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericPK.java Sat Oct 28 15:19:56 2017
@@ -21,6 +21,7 @@ package org.apache.ofbiz.entity;
 import java.util.Map;
 
 import org.apache.ofbiz.entity.model.ModelEntity;
+import org.apache.sis.internal.jdk7.Objects;
 
 /**
  * Generic Entity Primary Key Object
@@ -60,6 +61,11 @@ public class GenericPK extends GenericEn
     }
 
     @Override
+    public int hashCode() {
+        return Objects.hashCode(super.hashCode());
+    }
+
+    @Override
     public boolean equals(Object obj) {
         if (obj instanceof GenericPK) {
             return super.equals(obj);
@@ -71,7 +77,7 @@ public class GenericPK extends GenericEn
      *  @return Object that is a clone of this GenericPK
      */
     @Override
-    public Object clone() {
+    public GenericPK clone() {
         return GenericPK.create(this);
     }
 }

Modified: ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericValue.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericValue.java?rev=1813640&r1=1813639&r2=1813640&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericValue.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/GenericValue.java Sat Oct 28 15:19:56 2017
@@ -21,10 +21,13 @@ package org.apache.ofbiz.entity;
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.Arrays;
 
 import org.apache.ofbiz.base.util.Debug;
 import org.apache.ofbiz.entity.model.ModelEntity;
 
+
 /**
  * Generic Entity Value Object - Handles persistence for any defined entity.
  *
@@ -216,6 +219,11 @@ public class GenericValue extends Generi
     }
 
     @Override
+    public int hashCode() {
+        return Objects.hashCode(super.hashCode());
+    }
+
+    @Override
     public boolean equals(Object obj) {
         if (obj instanceof GenericValue) {
             return super.equals(obj);
@@ -243,6 +251,6 @@ public class GenericValue extends Generi
     }
 
     public static String getStackTraceAsString() {
-        return Thread.currentThread().getStackTrace().toString();
+        return Arrays.toString(Thread.currentThread().getStackTrace());
     }
 }