diff --git a/.gitignore b/.gitignore index 9f53138f92b..54e6bf9539c 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,6 @@ site-content # jenv's version file .java-version + +# Netbeans +nb-configuration.xml \ No newline at end of file diff --git a/pom.xml b/pom.xml index a8bccdb9e6f..ed213215cd5 100644 --- a/pom.xml +++ b/pom.xml @@ -466,9 +466,7 @@ [9,) - - - -Xmx512m --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.time=ALL-UNNAMED --add-opens java.base/java.time.chrono=ALL-UNNAMED + -Xmx512m diff --git a/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java b/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java index 0782e04ad76..6439dbce3ab 100644 --- a/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java @@ -115,6 +115,11 @@ private static void reflectionAppend( final boolean useTransients, final String[] excludeFields) { + if (Reflection.isNonIntrospectibleClass(lhs) && lhs instanceof Comparable) { + builder.append(lhs, rhs); + return; + } + final Field[] fields = clazz.getDeclaredFields(); AccessibleObject.setAccessible(fields, true); for (int i = 0; i < fields.length && builder.comparison == 0; i++) { 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 8a6a1e7d95c..ca7b6570805 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -22,7 +22,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import org.apache.commons.lang3.ArrayUtils; @@ -731,7 +733,7 @@ public EqualsBuilder append(final Object lhs, final Object rhs) { // to be inlined appendArray(lhs, rhs); } else // The simple case, not an array, just test the element - if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass)) { + if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass) && !Reflection.isNonIntrospectibleClass(lhsClass)) { reflectionAppend(lhs, rhs); } else { isEquals = lhs.equals(rhs); @@ -956,11 +958,47 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } try { - if (testClass.isArray()) { + if (testClass.isArray() || Reflection.isNonIntrospectibleClass(testClass)) { append(lhs, rhs); - } else //If either class is being excluded, call normal object equals method on lhsClass. - if (bypassReflectionClasses != null + } else if (Collection.class.isAssignableFrom(testClass)) { + // Right now this also handles Sets. + // Which is likely fine for TreeSets and any other sorted Collection + // But it would not be easy to implement this for e.g. a HashSet + // The good thing is that this behaviour is backward compatible with + // pre-jpms handling where we did reflect into the internal structures. + + Collection lColl = (Collection) lhs; + Collection rColl = (Collection) rhs; + if (lColl.size() != rColl.size()) { + isEquals = false; + return this; + } + final Iterator lIt = lColl.iterator(); + final Iterator rIt = rColl.iterator(); + while (lIt.hasNext() || rIt.hasNext()) { + if (lIt.hasNext() != rIt.hasNext()) { + // if one iterator has no more entries but the other one does, then it cannot be equal + isEquals = false; + return this; + } else { + reflectionAppend(lIt.next(), rIt.next()); + } + } + } else if (Map.class.isAssignableFrom(testClass)) { + Map lMap = (Map) lhs; + Map rMap = (Map) rhs; + if (lMap.size() != rMap.size()) { + isEquals = false; + return this; + } + for (Map.Entry e : lMap.entrySet()) { + if (isEquals) { + append(e.getValue(), rMap.get(e.getKey())); + } + } + } else if (bypassReflectionClasses != null && (bypassReflectionClasses.contains(lhsClass) || bypassReflectionClasses.contains(rhsClass))) { + //If either class is being excluded, call normal object equals method on lhsClass. isEquals = lhs.equals(rhs); } else { reflectionAppend(lhs, rhs, testClass); diff --git a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java index cc24f27245f..4a622c5ad35 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -180,6 +180,10 @@ private static void reflectionAppend(final Object object, final Class clazz, if (isRegistered(object)) { return; } + if (Reflection.isNonIntrospectibleClass(object)) { + builder.append(object); + return; + } try { register(object); // The elements in the returned array are not sorted and are not in any particular order. diff --git a/src/main/java/org/apache/commons/lang3/builder/Reflection.java b/src/main/java/org/apache/commons/lang3/builder/Reflection.java index fe7ade599a5..b9737e780bc 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,13 +18,62 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; +import java.time.OffsetDateTime; +import java.time.OffsetTime; +import java.time.Period; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** * Package-private reflection code. */ final class Reflection { + /** + * Some classes known to not be introspectable. + * Those are classes where we would blow up when we try to access internal information via reflection. + */ + private static final Set KNOWN_NON_INTROSPECTIBLE_CLASSES = new HashSet<>(); + { + KNOWN_NON_INTROSPECTIBLE_CLASSES.addAll(Arrays.asList( + Boolean.class, + Character.class, + String.class, + Byte.class, + Short.class, + Integer.class, + Long.class, + Float.class, + Double.class, + java.util.Date.class, + java.sql.Date.class, + LocalDate.class, + LocalDateTime.class, + LocalTime.class, + OffsetTime.class, + OffsetDateTime.class, + Instant.class, + Duration.class, + Period.class + )); + } + + /** + * this is a cache for {@link #isNonIntrospectibleClass(Class)}. + * Downside: this has a Class as key, so it *might* lock a ClassLoader. + * TODO think about making this cache optional? Otoh we only cache classes we touch via the reflection methods. + */ + private static Map nonIntrospectibleClasses = new ConcurrentHashMap<>(); + /** * Delegates to {@link Field#get(Object)} and rethrows {@link IllegalAccessException} as {@link IllegalArgumentException}. * @@ -41,4 +90,44 @@ static Object getUnchecked(final Field field, final Object obj) { } } + /** + * Check whether the class internas of the given object can be accessed. This might return false in Java9 + * and thus lead to a difference in behaviour if modularity is activated or not. + * But it's the best we can do. + * + * @return {@code true} if the given class internas not accessible + */ + static boolean isNonIntrospectibleClass(Object o) { + return o != null && isNonIntrospectibleClass(o.getClass()); + } + + /** + * Check whether the given class internas can be accessed. This might return false in Java9 + * and thus lead to a difference in behaviour if modularity is activated or not. + * But it's the best we can do. + * + * @return {@code true} if the given class internas not accessible + */ + static boolean isNonIntrospectibleClass(Class clazz) { + if (KNOWN_NON_INTROSPECTIBLE_CLASSES.contains(clazz)) { + return true; + } + Boolean isAccessible = nonIntrospectibleClasses.get(clazz); + if (isAccessible != null) { + return isAccessible; + } + try { + final Field[] declaredFields = clazz.getDeclaredFields(); + for (Field f : declaredFields) { + f.setAccessible(true); + } + + nonIntrospectibleClasses.put(clazz, false); + return false; + } catch (Exception e) { + nonIntrospectibleClasses.put(clazz, true); + return true; + } + } + } diff --git a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java index 06c9be8178d..0c5082043fa 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Comparator; +import java.util.Map; import java.util.Objects; import org.apache.commons.lang3.ArraySorter; @@ -848,8 +849,13 @@ public String toString() { validate(); + if (handleNativeClasses()) { + return super.toString(); + } + Class clazz = getObject().getClass(); appendFieldsIn(clazz); + while (clazz.getSuperclass() != null && clazz != getUpToClass()) { clazz = clazz.getSuperclass(); appendFieldsIn(clazz); @@ -867,4 +873,21 @@ private void validate() { } } + private boolean handleNativeClasses() { + Object value = getObject(); + if (value.getClass().isArray() || value instanceof Collection || value instanceof Map) { + // we first need to unregister the value to be able to handle it in the style + ToStringStyle.unregister(value); + getStyle().append(getStringBuffer(), null, value, true); + return true; + } + + if (Reflection.isNonIntrospectibleClass(value)) { + getStringBuffer().append("value=").append(getObject().toString()); + return true; + } + + return false; + } + } 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 983fac38b7c..d13a495fdcd 100644 --- a/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/EqualsBuilderTest.java @@ -27,7 +27,9 @@ import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.apache.commons.lang3.AbstractLangTest; import org.apache.commons.lang3.reflect.MethodUtils; @@ -1142,6 +1144,24 @@ public void testObjectsBypassReflectionClasses() { assertTrue(new EqualsBuilder().setBypassReflectionClasses(bypassReflectionClasses).isEquals()); } + @Test + public void testArrayListComparison() { + final List a = Arrays.asList("A", "B", "C"); + final List b = Arrays.asList("A", "B", "C"); + assertTrue(new EqualsBuilder().setTestRecursive(true).append(a, b).isEquals()); + } + + @Test + public void testMapComparison() { + final Map a = new HashMap<>(); + final Map b = new HashMap<>(); + a.put(2, "B"); + a.put(1, "A"); + b.put(1, "A"); + b.put(2, "B"); + assertTrue(new EqualsBuilder().setTestRecursive(true).append(a, b).isEquals()); + } + @Test public void testRaggedArray() { final long[][] array1 = new long[2][]; diff --git a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java index 23c8f29422e..100f607cd50 100644 --- a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -229,21 +230,33 @@ public void assertReflectionArray(final String expected, final Object actual) { */ @Test public void test_setUpToClass_invalid() { - final Integer val = Integer.valueOf(5); + final HirAFixture val = new HirAFixture(); final ReflectionToStringBuilder test = new ReflectionToStringBuilder(val); assertThrows(IllegalArgumentException.class, () -> test.setUpToClass(String.class)); test.toString(); } + private static class HirAFixture extends HirBFixture { + int x = 1; + } + + private static class HirBFixture extends HirCFixture { + int y = 2; + } + + private static class HirCFixture { + int z = 3; + } + /** * Tests ReflectionToStringBuilder setUpToClass(). */ @Test public void test_setUpToClass_valid() { - final Integer val = Integer.valueOf(5); + final HirAFixture val = new HirAFixture(); final ReflectionToStringBuilder test = new ReflectionToStringBuilder(val); - test.setUpToClass(Number.class); - test.toString(); + test.setUpToClass(HirBFixture.class); + assertEquals(val.toString() + "[x=1,y=2]", test.toString()); } @Test @@ -752,6 +765,12 @@ public void testObjectArray() { assertEquals(baseStr + "[]", new ToStringBuilder(base).append((Object) array).toString()); } + @Test + public void testArrayList() { + List list = Arrays.asList(23, 12, 39); + assertEquals(baseStr + "[[23, 12, 39]]", new ToStringBuilder(base).append(list).toString()); + } + @Test public void testObjectBuild() { final Integer i3 = Integer.valueOf(3); @@ -972,17 +991,14 @@ public void testReflectionHierarchyArrayList() { // LANG-1337 without this, the generated string can differ depending on the JVM version/vendor final List list = new ArrayList<>(ARRAYLIST_INITIAL_CAPACITY); final String baseString = toBaseString(list); - final String expectedWithTransients = baseString - + "[elementData={,,,,,,,,,},size=0,modCount=0]"; + final String expected = baseString + + "[[]]"; final String toStringWithTransients = ToStringBuilder.reflectionToString(list, null, true); - if (!expectedWithTransients.equals(toStringWithTransients)) { - assertEquals(expectedWithTransients, toStringWithTransients); - } - final String expectedWithoutTransients = baseString + "[size=0]"; + assertEquals(expected, toStringWithTransients); + + // no difference as Collections and Maps are not handled via reflection anymore final String toStringWithoutTransients = ToStringBuilder.reflectionToString(list, null, false); - if (!expectedWithoutTransients.equals(toStringWithoutTransients)) { - assertEquals(expectedWithoutTransients, toStringWithoutTransients); - } + assertEquals(expected, toStringWithoutTransients); } @Test