Thanks Michael! On Fri, 27 Jul 2018, 08:48 Michael Brohl, <[email protected]> wrote:
> Hi Scott, > > thanks for spotting this, I will fix it! > > Regards, > > Michael > > > Am 26.07.18 um 23:40 schrieb Scott Gray: > > 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()); > >> } > >> } > >> > >> > >> > > >
