You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by pa...@apache.org on 2018/02/11 19:43:17 UTC

[lang] LANG-1356: Add bypass option for classes to recursive and reflective EqualsBuilder

Repository: commons-lang
Updated Branches:
  refs/heads/master 2ce404940 -> 2e9f3a801


LANG-1356: Add bypass option for classes to recursive and reflective EqualsBuilder

Patch supplied by Yathos UG


Project: http://git-wip-us.apache.org/repos/asf/commons-lang/repo
Commit: http://git-wip-us.apache.org/repos/asf/commons-lang/commit/2e9f3a80
Tree: http://git-wip-us.apache.org/repos/asf/commons-lang/tree/2e9f3a80
Diff: http://git-wip-us.apache.org/repos/asf/commons-lang/diff/2e9f3a80

Branch: refs/heads/master
Commit: 2e9f3a80146262511ca7bcdd3411f095dff4951d
Parents: 2ce4049
Author: pascalschumacher <pa...@gmx.net>
Authored: Sun Feb 11 20:43:05 2018 +0100
Committer: pascalschumacher <pa...@gmx.net>
Committed: Sun Feb 11 20:43:05 2018 +0100

----------------------------------------------------------------------
 src/changes/changes.xml                         |  1 +
 .../commons/lang3/builder/EqualsBuilder.java    | 40 +++++++++++++++++--
 .../lang3/builder/EqualsBuilderTest.java        | 42 ++++++++++++++++++++
 3 files changed, 79 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/commons-lang/blob/2e9f3a80/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 61eca68..5bfedf4 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -55,6 +55,7 @@ The <action> type attribute can be add,update,fix,remove.
     <action issue="LANG-1367" type="update" dev="ggregory" due-to="Gary Gregory">ObjectUtils.identityToString(Object) and friends should allocate builders and buffers with a size</action>
     <action issue="LANG-1352" type="add" dev="pschumacher" due-to="Ruslan Sibgatullin">EnumUtils.getEnumIgnoreCase and isValidEnumIgnoreCase methods added</action>
     <action issue="LANG-1372" type="add" dev="pschumacher" due-to="Sérgio Ozaki">Add ToStringSummary annotation</action>
+    <action issue="LANG-1356" type="add" dev="pschumacher" due-to="Yathos UG">Add bypass option for classes to recursive and reflective EqualsBuilder</action>
   </release>
 
   <release version="3.7" date="2017-11-04" description="New features and bug fixes. Requires Java 7, supports Java 8, 9, 10.">

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/2e9f3a80/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java
index d2cf7c7..60a532e 100644
--- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java
+++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java
@@ -19,8 +19,10 @@ package org.apache.commons.lang3.builder;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 import org.apache.commons.lang3.ArrayUtils;
@@ -213,6 +215,7 @@ public class EqualsBuilder implements Builder<Boolean> {
 
     private boolean testTransients = false;
     private boolean testRecursive = false;
+    private List<Class<?>> bypassReflectionClasses;
     private Class<?> reflectUpToClass = null;
     private String[] excludeFields = null;
 
@@ -223,7 +226,9 @@ public class EqualsBuilder implements Builder<Boolean> {
      * @see Object#equals(Object)
      */
     public EqualsBuilder() {
-        // do nothing for now.
+        // set up default classes to bypass reflection for
+        bypassReflectionClasses = new ArrayList<Class<?>>();
+        bypassReflectionClasses.add(String.class); //hashCode field being lazy but not transient
     }
 
     //-------------------------------------------------------------------------
@@ -251,6 +256,23 @@ public class EqualsBuilder implements Builder<Boolean> {
     }
 
     /**
+     * <p>Set <code>Class</code>es whose instances should be compared by calling their <code>equals</code>
+     * although being in recursive mode. So the fields of theses classes will not be compared recursively by reflection.</p>
+     *
+     * <p>Here you should name classes having non-transient fields which are cache fields being set lazily.<br>
+     * Prominent example being {@link String} class with its hash code cache field. Due to the importance
+     * of the <code>String</code> class, it is included in the default bypasses classes. Usually, if you use
+     * your own set of classes here, remember to include <code>String</code> class, too.</p>
+     * @param bypassReflectionClasses  classes to bypass reflection test
+     * @return EqualsBuilder - used to chain calls.
+     * @since 3.8
+     */
+    public EqualsBuilder setBypassReflectionClasses(List<Class<?>> bypassReflectionClasses) {
+        this.bypassReflectionClasses = bypassReflectionClasses;
+        return this;
+    }
+
+    /**
      * Set the superclass to reflect up to at reflective tests.
      * @param reflectUpToClass the super class to reflect up to
      * @return EqualsBuilder - used to chain calls.
@@ -458,6 +480,10 @@ public class EqualsBuilder implements Builder<Boolean> {
      *
      * <p>Field names listed in field <code>excludeFields</code> will be ignored.</p>
      *
+     * <p>If either class of the compared objects is contained in
+     * <code>bypassReflectionClasses</code>, both objects are compared by calling
+     * the equals method of the left hand object with the right hand object as an argument.</p>
+     *
      * @param lhs  the left hand object
      * @param rhs  the left hand object
      * @return EqualsBuilder - used to chain calls.
@@ -503,10 +529,16 @@ public class EqualsBuilder implements Builder<Boolean> {
             if (testClass.isArray()) {
                 append(lhs, rhs);
             } else {
-                reflectionAppend(lhs, rhs, testClass);
-                while (testClass.getSuperclass() != null && testClass != reflectUpToClass) {
-                    testClass = testClass.getSuperclass();
+                //If either class is being excluded, call normal object equals method on lhsClass.
+                if (bypassReflectionClasses != null
+                        && (bypassReflectionClasses.contains(lhsClass) || bypassReflectionClasses.contains(rhsClass))) {
+                    isEquals = lhs.equals(rhs);
+                } else {
                     reflectionAppend(lhs, rhs, testClass);
+                    while (testClass.getSuperclass() != null && testClass != reflectUpToClass) {
+                        testClass = testClass.getSuperclass();
+                        reflectionAppend(lhs, rhs, testClass);
+                    }
                 }
             }
         } catch (final IllegalArgumentException e) {

http://git-wip-us.apache.org/repos/asf/commons-lang/blob/2e9f3a80/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java
index 23651f6..05f1da9 100644
--- a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java
+++ b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java
@@ -168,6 +168,19 @@ public class EqualsBuilderTest {
         }
     }
 
+    static class TestRecursiveGenericObject<T> {
+
+        private final T a;
+
+        TestRecursiveGenericObject(final T a) {
+            this.a = a;
+        }
+
+        public T getA() {
+            return a;
+        }
+    }
+
     static class TestRecursiveObject {
         private final TestRecursiveInnerObject a;
         private final TestRecursiveInnerObject b;
@@ -419,6 +432,35 @@ public class EqualsBuilderTest {
     }
 
     @Test
+    public void testObjectRecursiveGenericInteger() {
+        final TestRecursiveGenericObject<Integer> o1_a = new TestRecursiveGenericObject<Integer>(1);
+        final TestRecursiveGenericObject<Integer> o1_b = new TestRecursiveGenericObject<Integer>(1);
+        final TestRecursiveGenericObject<Integer> o2 = new TestRecursiveGenericObject<Integer>(2);
+
+        assertTrue(new EqualsBuilder().setTestRecursive(true).append(o1_a, o1_b).isEquals());
+        assertTrue(new EqualsBuilder().setTestRecursive(true).append(o1_b, o1_a).isEquals());
+
+        assertFalse(new EqualsBuilder().setTestRecursive(true).append(o1_b, o2).isEquals());
+    }
+
+    @Test
+    public void testObjectRecursiveGenericString() {
+        // Note: Do not use literals, because string literals are always mapped by same object (internal() of String))!
+        String s1_a = String.valueOf(1);
+        final TestRecursiveGenericObject<String> o1_a = new TestRecursiveGenericObject<String>(s1_a);
+        final TestRecursiveGenericObject<String> o1_b = new TestRecursiveGenericObject<String>(String.valueOf(1));
+        final TestRecursiveGenericObject<String> o2 = new TestRecursiveGenericObject<String>(String.valueOf(2));
+
+        // To trigger bug reported in LANG-1356, call hashCode only on string in instance o1_a
+        s1_a.hashCode();
+
+        assertTrue(new EqualsBuilder().setTestRecursive(true).append(o1_a, o1_b).isEquals());
+        assertTrue(new EqualsBuilder().setTestRecursive(true).append(o1_b, o1_a).isEquals());
+
+        assertFalse(new EqualsBuilder().setTestRecursive(true).append(o1_b, o2).isEquals());
+    }
+
+    @Test
     public void testObjectRecursive() {
         final TestRecursiveInnerObject i1_1 = new TestRecursiveInnerObject(1);
         final TestRecursiveInnerObject i1_2 = new TestRecursiveInnerObject(1);