Details

      Description

      GenericDelegator.java:117, MS_SHOULD_BE_FINAL

      • MS: org.apache.ofbiz.entity.GenericDelegator.userIdentifierStack isn't final but should be

      This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

      GenericDelegator.java:119, MS_SHOULD_BE_FINAL

      • MS: org.apache.ofbiz.entity.GenericDelegator.sessionIdentifierStack isn't final but should be

      This static field public but not final, and could be changed by malicious code or by accident from another package. The field could be made final to avoid this vulnerability.

      GenericDelegator.java:324, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE

      • RV: Exceptional return value of java.util.concurrent.ExecutorService.submit(Runnable) ignored in org.apache.ofbiz.entity.GenericDelegator.initEntityEcaHandler()

      This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.

      GenericDelegator.java:452, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of entity, which is known to be non-null in org.apache.ofbiz.entity.GenericDelegator.getModelEntityMapByGroup(String)

      This method contains a redundant check of a known non-null value against the constant null.

      GenericDelegator.java:579, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of modelFieldTypeReader, which is known to be non-null in org.apache.ofbiz.entity.GenericDelegator.getModelFieldTypeReader(ModelEntity)

      This method contains a redundant check of a known non-null value against the constant null.

      GenericDelegator.java:597, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of modelFieldTypeReader, which is known to be non-null in org.apache.ofbiz.entity.GenericDelegator.getEntityFieldTypeNames(ModelEntity)

      This method contains a redundant check of a known non-null value against the constant null.

      GenericDelegator.java:789, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of value, which is known to be non-null in org.apache.ofbiz.entity.GenericDelegator.createSetNextSeqId(GenericValue)

      This method contains a redundant check of a known non-null value against the constant null.

      GenericDelegator.java:846, REC_CATCH_EXCEPTION

      • REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.createSetNextSeqId(GenericValue)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try

      { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:879, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE
      - RCN: Redundant nullcheck of value, which is known to be non-null in org.apache.ofbiz.entity.GenericDelegator.create(GenericValue)

      This method contains a redundant check of a known non-null value against the constant null.

      GenericDelegator.java:902, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.create(GenericValue)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... }

      catch (Exception e)

      { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:932, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.createOrStore(GenericValue)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something }

      as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try

      { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1013, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.removeByPrimaryKey(GenericPK)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... }

      catch (RuntimeException e)

      { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1070, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.removeValue(GenericValue)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; }

      catch (Exception e)

      { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1126, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.removeByCondition(String, EntityCondition)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1208, REC_CATCH_EXCEPTION, Priorit├Ąt: Niedrig

      • REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.storeByCondition(String, Map, EntityCondition)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try

      { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1261, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.store(GenericValue)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... }

      catch (Exception e)

      { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1347, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.storeAll(List, EntityStoreOptions)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something }

      as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try

      { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1385, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.removeAll(List)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... }

      catch (RuntimeException e)

      { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }
      GenericDelegator.java:1458, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.findOne(String, Map, boolean)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; }

      catch (Exception e)

      { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1497, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.findByPrimaryKeyPartial(GenericPK, Set)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1595, REC_CATCH_EXCEPTION

      • REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.findList(String, EntityCondition, Set, List, EntityFindOptions, boolean)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try

      { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1662, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.findCountByCondition(String, EntityCondition, EntityCondition, EntityFindOptions)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... }

      catch (Exception e)

      { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:1693, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.getMultiRelation(GenericValue, String, String, List)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something }

      as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try

      { ... } catch (RuntimeException e) { throw e; } catch (Exception e) { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:2358, REC_CATCH_EXCEPTION
      - REC: Exception is caught when Exception is not thrown in org.apache.ofbiz.entity.GenericDelegator.setNextSubSeqId(GenericValue, String, int, int)

      This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

      A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

      try { ... }

      catch (RuntimeException e)

      { throw e; }

      catch (Exception e)

      { ... deal with all non-runtime exceptions ... }

      GenericDelegator.java:2607, SIC_INNER_SHOULD_BE_STATIC

      • SIC: Should org.apache.ofbiz.entity.GenericDelegator$TestOperation be a static inner class?

      This class is an inner class, but does not use its embedded reference to the object which created it. This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary. If possible, the class should be made static.

      GenericDelegator.java:2638, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE

      • RV: Exceptional return value of java.util.concurrent.ExecutorService.submit(Runnable) ignored in org.apache.ofbiz.entity.GenericDelegator.initDistributedCacheClear()

      This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.

      GenericEntity.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED

      • Se: The field org.apache.ofbiz.entity.GenericEntity.observable is transient but isn't set by deserialization

      This class contains a field that is updated at multiple places in the class, thus it seems to be part of the state of the class. However, since the field is marked as transient and not set in readObject or readResolve, it will contain the default value in any deserialized instance of the class.

      GenericEntity.java:-1, SE_TRANSIENT_FIELD_NOT_RESTORED

      • Se: The field org.apache.ofbiz.entity.GenericEntity.modelEntity is transient but isn't set by deserialization

      This class contains a field that is updated at multiple places in the class, thus it seems to be part of the state of the class. However, since the field is marked as transient and not set in readObject or readResolve, it will contain the default value in any deserialized instance of the class.

      GenericEntity.java:75, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.entity.GenericEntity is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      GenericEntity.java:354, RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

      • RCN: Redundant nullcheck of org.apache.ofbiz.entity.GenericEntity.delegatorName, which is known to be non-null in org.apache.ofbiz.entity.GenericEntity.getDelegator()

      This method contains a redundant check of a known non-null value against the constant null.

      GenericEntity.java:474, DE_MIGHT_IGNORE
      DE: org.apache.ofbiz.entity.GenericEntity.set(String, Object, boolean) might ignore org.apache.ofbiz.base.util.GeneralException

      This method might ignore an exception. In general, exceptions should be handled or reported in some way, or they should be thrown out of the method.

      GenericEntity.java:476, DM_DEFAULT_ENCODING

      • Dm: Found reliance on default encoding in org.apache.ofbiz.entity.GenericEntity.set(String, Object, boolean): String.getBytes()

      Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

      GenericEntity.java:532, NP_NULL_ON_SOME_PATH

      • NP: Possible null pointer dereference of field in org.apache.ofbiz.entity.GenericEntity.setString(String, String)

      There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed. Of course, the problem might be that the branch or statement is infeasible and that the null pointer exception can't ever be executed; deciding that is beyond the ability of FindBugs.

      GenericEntity.java:540, SF_SWITCH_NO_DEFAULT

      • SF: Switch statement found in org.apache.ofbiz.entity.GenericEntity.setString(String, String) where default case is missing

      This method contains a switch statement where default case is missing. Usually you need to provide a default case.

      Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

      GenericEntity.java:542, DB_DUPLICATE_SWITCH_CLAUSES

      • DB: org.apache.ofbiz.entity.GenericEntity.setString(String, String) uses the same code for two switch clauses

      This method uses the same code to implement two clauses of a switch statement. This could be a case of duplicate code, but it might also indicate a coding mistake.

      GenericEntity.java:624, UCF_USELESS_CONTROL_FLOW

      • UCF: Useless control flow in org.apache.ofbiz.entity.GenericEntity.setNextSeqId()

      This method contains a useless control flow statement, where control flow continues onto the same place regardless of whether or not the branch is taken. For example, this is caused by having an empty statement block for an if statement:

      if (argv.length == 0)

      { // TODO: handle this case }

      GenericEntity.java:636, NP_BOOLEAN_RETURN_NULL

      • NP: org.apache.ofbiz.entity.GenericEntity.getBoolean(String) has Boolean return type and returns explicit null

      A method that returns either Boolean.TRUE, Boolean.FALSE or null is an accident waiting to happen. This method can be invoked as though it returned a value of type boolean, and the compiler will insert automatic unboxing of the Boolean value. If a null value is returned, this will result in a NullPointerException.

      GenericEntity.java:712, DM_FP_NUMBER_CTOR

      • Bx: org.apache.ofbiz.entity.GenericEntity.getDouble(String) invokes inefficient new Double(double) constructor; use Double.valueOf(double) instead

      Using new Double(double) is guaranteed to always result in a new object whereas Double.valueOf(double) allows caching of values to be done by the compiler, class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

      Unless the class must be compatible with JVMs predating Java 1.5, use either autoboxing or the valueOf() method when creating instances of Double and Float.

      GenericEntity.java:731, PZLA_PREFER_ZERO_LENGTH_ARRAYS

      • PZLA: Should org.apache.ofbiz.entity.GenericEntity.getBytes(String) return a zero length array rather than null?

      It is often a better design to return a length zero array rather than a null reference to indicate that there are no results (i.e., an empty list of results). This way, no explicit check for null is needed by clients of the method.

      On the other hand, using null to indicate "there is no answer to this question" is probably appropriate. For example, File.listFiles() returns an empty list if given a directory containing no files, and returns null if the file is not a directory.

      GenericEntity.java:1151, DM_DEFAULT_ENCODING

      • Dm: Found reliance on default encoding in org.apache.ofbiz.entity.GenericEntity.writeXmlText(PrintWriter, String): new String(byte[])

      Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

      GenericEntity.java:1328, DM_DEFAULT_ENCODING

      • Dm: Found reliance on default encoding in org.apache.ofbiz.entity.GenericEntity.toString(): String.getBytes()

      Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

      GenericEntity.java:1427, CN_IDIOM_NO_SUPER_CALL

      • CN: org.apache.ofbiz.entity.GenericEntity.clone() does not call super.clone()

      This non-final class defines a clone() method that does not call super.clone(). If this class ("A") is extended by a subclass ("B"), and the subclass B calls super.clone(), then it is likely that B's clone() method will return an object of type A, which violates the standard contract for clone().

      If all clone() methods call super.clone(), then they are guaranteed to use Object.clone(), which always returns an object of the correct type.

      GenericEntity.java:1592, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.entity.GenericEntity$NullGenericEntity is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      GenericEntity.java:1616, EQ_COMPARETO_USE_OBJECT_EQUALS

      • Eq: org.apache.ofbiz.entity.GenericEntity$NullField defines compareTo(GenericEntity$NullField) and uses Object.equals()

      This class defines a compareTo(...) method but inherits its equals() method from java.lang.Object. Generally, the value of compareTo should return zero if and only if equals returns true. If this is violated, weird and unpredictable failures will occur in classes such as PriorityQueue. In Java 5 the PriorityQueue.remove method uses the compareTo method, while in Java 6 it uses the equals method.

      From the JavaDoc for the compareTo method in the Comparable interface:

      It is strongly recommended, but not strictly required that (x.compareTo==0) == (x.equals). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

      GenericPK.java:32, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.entity.GenericPK is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      GenericPK.java:64, HE_EQUALS_NO_HASHCODE

      • HE: org.apache.ofbiz.entity.GenericPK defines equals but not hashCode

      This class overrides equals(Object), but does not override hashCode(). Therefore, the class may violate the invariant that equal objects must have equal hashcodes.

      GenericPK.java:75, CN_IDIOM_NO_SUPER_CALL

      • CN: org.apache.ofbiz.entity.GenericPK.clone() does not call super.clone()

      This non-final class defines a clone() method that does not call super.clone(). If this class ("A") is extended by a subclass ("B"), and the subclass B calls super.clone(), then it is likely that B's clone() method will return an object of type A, which violates the standard contract for clone().

      If all clone() methods call super.clone(), then they are guaranteed to use Object.clone(), which always returns an object of the correct type.

      GenericValue.java:33, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.entity.GenericValue is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      GenericValue.java:220, HE_EQUALS_NO_HASHCODE

      • HE: org.apache.ofbiz.entity.GenericValue defines equals but not hashCode

      This class overrides equals(Object), but does not override hashCode(). Therefore, the class may violate the invariant that equal objects must have equal hashcodes.

      GenericValue.java:231, CN_IDIOM_NO_SUPER_CALL

      • CN: org.apache.ofbiz.entity.GenericValue.clone() does not call super.clone()

      This non-final class defines a clone() method that does not call super.clone(). If this class ("A") is extended by a subclass ("B"), and the subclass B calls super.clone(), then it is likely that B's clone() method will return an object of type A, which violates the standard contract for clone().

      If all clone() methods call super.clone(), then they are guaranteed to use Object.clone(), which always returns an object of the correct type.

      GenericValue.java:234, SE_NO_SERIALVERSIONID

      • SnVI: org.apache.ofbiz.entity.GenericValue$NullGenericValue is Serializable; consider declaring a serialVersionUID

      This class implements the Serializable interface, but does not define a serialVersionUID field. A change as simple as adding a reference to a .class object will add synthetic fields to the class, which will unfortunately change the implicit serialVersionUID (e.g., adding a reference to String.class will generate a static field class$java$lang$String). Also, different source code to bytecode compilers may use different naming conventions for synthetic variables generated for references to class objects or inner classes. To ensure interoperability of Serializable across versions, consider adding an explicit serialVersionUID.

      GenericValue.java:246, DMI_INVOKING_TOSTRING_ON_ARRAY

      • USELESS_STRING: Invocation of toString on Thread.getStackTrace() in org.apache.ofbiz.entity.GenericValue.getStackTraceAsString()

      The code invokes toString on an array, which will generate a fairly useless result such as [C@16f0472. Consider using Arrays.toString to convert the array into a readable String that gives the contents of the array. See Programming Puzzlers, chapter 3, puzzle 12.

        Activity

        Hide
        jleichert Julian Leichert added a comment - - edited

        class GenericDelegator

        • removed redundant null-checks
        • added Multicatch
        • changed catch Exception to catch GenericEnitityException for better handling(multiple lines)
        • changed inner class to static to fix Findbug Warning

        class GenericEntity

        • removed redundant null-checks
        • added utf8 encoding to various methods
        • added Multicatch
        • changed return to false to prevent NullPointerException
        • changed new Double to Double.valueOf to improve performance
        • added equals and hashCode in NullGenericEntity to fix FindbugWarning in CompareTo() at line 1616

        class GenericPK

        • added hashCode
        • changed clone() to correct type

        class GenericValue

        • added hashCode
        • Line 254: changed to Arrays.toString() otherwise it would be an useless String

        GenericEntity.java:1427, CN_IDIOM_NO_SUPER_CALL
        GenericPK.java:75, CN_IDIOM_NO_SUPER_CALL
        GenericValue.java:231, CN_IDIOM_NO_SUPER_CALL
        GenericDelegator.java:324, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE
        GenericDelegator.java:2638, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE

        How should this be handled?

        Show
        jleichert Julian Leichert added a comment - - edited class GenericDelegator removed redundant null-checks added Multicatch changed catch Exception to catch GenericEnitityException for better handling(multiple lines) changed inner class to static to fix Findbug Warning class GenericEntity removed redundant null-checks added utf8 encoding to various methods added Multicatch changed return to false to prevent NullPointerException changed new Double to Double.valueOf to improve performance added equals and hashCode in NullGenericEntity to fix FindbugWarning in CompareTo() at line 1616 class GenericPK added hashCode changed clone() to correct type class GenericValue added hashCode Line 254: changed to Arrays.toString() otherwise it would be an useless String GenericEntity.java:1427, CN_IDIOM_NO_SUPER_CALL GenericPK.java:75, CN_IDIOM_NO_SUPER_CALL GenericValue.java:231, CN_IDIOM_NO_SUPER_CALL GenericDelegator.java:324, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE GenericDelegator.java:2638, RV_RETURN_VALUE_IGNORED_BAD_PRACTICE How should this be handled?
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Julian,

        your modified patch is in trunk r1813640.

        Show
        mbrohl Michael Brohl added a comment - Thanks Julian, your modified patch is in trunk r1813640.
        Hide
        suraj.khurana Suraj Khurana added a comment -

        Hi,

        I noticed in this patch getBoolean() of GenericEntity class is returning false instead of null.
        But occurrences related to same are not updated like shippingApplies method of ProductWorker class.

        Should I re-open this ticket or create a new one.

        Show
        suraj.khurana Suraj Khurana added a comment - Hi, I noticed in this patch getBoolean() of GenericEntity class is returning false instead of null. But occurrences related to same are not updated like shippingApplies method of ProductWorker class. Should I re-open this ticket or create a new one.
        Hide
        mbrohl Michael Brohl added a comment -

        Thanks Suraj Khurana,

        you are right, this change modifies the behaviour. We'll fix it soon.

        Show
        mbrohl Michael Brohl added a comment - Thanks Suraj Khurana , you are right, this change modifies the behaviour. We'll fix it soon.
        Hide
        Dennis Balkir Dennis Balkir added a comment -

        Thanks Suraj Khurana for noticing.
        I fixed it right there:

        Show
        Dennis Balkir Dennis Balkir added a comment - Thanks Suraj Khurana for noticing. I fixed it right there:
        Hide
        mbrohl Michael Brohl added a comment - - edited

        Thanks Suraj and Dennis,
        the fix is in r1814230.

        Show
        mbrohl Michael Brohl added a comment - - edited Thanks Suraj and Dennis, the fix is in r1814230.

          People

          • Assignee:
            mbrohl Michael Brohl
            Reporter:
            jleichert Julian Leichert
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development