FYI, I think this commit accidentally introduced a weird import into GenericPK: import org.apache.sis.internal.jdk7.Objects
probably intended to be java.util.Objects Regards Scott On 28 October 2017 at 15:19, <[email protected]> wrote: > 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()); > } > } > > >
