From 4fd4d01ee50532a7f27799e6b9907bece58cc187 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Mon, 2 Dec 2024 19:24:27 +0100 Subject: [PATCH 01/18] adding Netbeans artifacts --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) 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 From 3d64ffb7e1ff66c2192f11a7560fbf9980def5ab Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Mon, 2 Dec 2024 19:26:11 +0100 Subject: [PATCH 02/18] LANG-1685 fix upToClass test --- pom.xml | 6 +++- .../lang3/builder/ToStringBuilderTest.java | 28 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/pom.xml b/pom.xml index dbe391f7fde..41ae273448d 100644 --- a/pom.xml +++ b/pom.xml @@ -202,6 +202,7 @@ src/site/resources/download_lang.cgi src/site/resources/release-notes/RELEASE-NOTES-*.txt src/test/resources/lang-708-input.txt + nb-configuration.xml @@ -460,7 +461,10 @@ - -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 + + -Xmx512m 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..50f36b08dff 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,34 @@ 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 @@ -751,6 +765,12 @@ public void testObjectArray() { assertEquals(baseStr + "[]", new ToStringBuilder(base).append(array).toString()); 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() { From 68a8c9c2bccaee2eda7eb177e4004ccf9ec7cd53 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Tue, 3 Dec 2024 08:28:42 +0100 Subject: [PATCH 03/18] LANG-1685 improve ToStringBuilder for Java9++ --- .../builder/ReflectionToStringBuilder.java | 24 +++++++++++++++++++ .../lang3/builder/ToStringBuilderTest.java | 15 +++++------- 2 files changed, 30 insertions(+), 9 deletions(-) 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..912733dc99e 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -20,9 +20,11 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.time.temporal.TemporalAccessor; 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; @@ -847,9 +849,14 @@ 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 +874,21 @@ private void validate() { } } + private boolean handleNativeClasses() { + Object value = getObject(); + if (value instanceof Number || value instanceof Boolean || value instanceof Character + || value instanceof TemporalAccessor ) { + getStringBuffer().append("value=").append(getObject().toString()); + return true; + } + + if (value.getClass().isArray() || value instanceof Collection || value instanceof Map) { + ToStringStyle.unregister(value); + getStyle().append(getStringBuffer(), null, value, true); + return true; + } + + return false; + } + } 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 50f36b08dff..ed682f71a2e 100644 --- a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java @@ -992,17 +992,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 From 9fd537a817fe476646369d76d165b7f4302c78b9 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Wed, 4 Dec 2024 13:33:48 +0100 Subject: [PATCH 04/18] LANG-1758 improve CompareToBuilder this also fixes the Java9++ InaccessibleObjectException for some constellations at least --- .../org/apache/commons/lang3/builder/CompareToBuilder.java | 6 ++++++ 1 file changed, 6 insertions(+) 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..6acf01a8a88 100644 --- a/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java @@ -19,6 +19,7 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.time.temporal.Temporal; import java.util.Collection; import java.util.Comparator; import java.util.Objects; @@ -115,6 +116,11 @@ private static void reflectionAppend( final boolean useTransients, final String[] excludeFields) { + if ((lhs instanceof CharSequence || lhs instanceof Number || lhs instanceof Temporal) && 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++) { From edb8ba8644597fd1a8eabb467cf25375d25adcac Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Wed, 4 Dec 2024 15:54:48 +0100 Subject: [PATCH 05/18] LANG-1265 LANG-1667 fix unallowed private access --- pom.xml | 5 ----- .../org/apache/commons/lang3/builder/EqualsBuilder.java | 7 +++++-- .../org/apache/commons/lang3/builder/HashCodeBuilder.java | 5 +++++ 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/pom.xml b/pom.xml index 41ae273448d..c197deada66 100644 --- a/pom.xml +++ b/pom.xml @@ -459,11 +459,6 @@ [9,) - - - -Xmx512m 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..67183a2855e 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -19,6 +19,7 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -731,7 +732,8 @@ 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) + && !(CharSequence.class.isAssignableFrom(lhsClass) || Number.class.isAssignableFrom(lhsClass) || Temporal.class.isAssignableFrom(lhsClass))) { reflectionAppend(lhs, rhs); } else { isEquals = lhs.equals(rhs); @@ -956,7 +958,8 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } try { - if (testClass.isArray()) { + if (testClass.isArray() || Number.class.isAssignableFrom(testClass) + || Temporal.class.isAssignableFrom(testClass) || CharSequence.class.isAssignableFrom(testClass)) { append(lhs, rhs); } else //If either class is being excluded, call normal object equals method on lhsClass. if (bypassReflectionClasses != null 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..00d3b1c3144 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -20,6 +20,7 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; +import java.time.temporal.Temporal; import java.util.Collection; import java.util.Comparator; import java.util.HashSet; @@ -180,6 +181,10 @@ private static void reflectionAppend(final Object object, final Class clazz, if (isRegistered(object)) { return; } + if (object instanceof CharSequence || object instanceof Number || object instanceof Temporal) { + builder.append(object); + return; + } try { register(object); // The elements in the returned array are not sorted and are not in any particular order. From 67335519cd5b4bf6a0161fa740b3bd9fb5cfdf49 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Wed, 4 Dec 2024 16:15:39 +0100 Subject: [PATCH 06/18] LANG-1265 fix checkstyle issues --- .../lang3/builder/ReflectionToStringBuilder.java | 10 +++++----- .../commons/lang3/builder/ToStringBuilderTest.java | 11 +++++------ 2 files changed, 10 insertions(+), 11 deletions(-) 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 912733dc99e..9a3f242db1d 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -849,14 +849,14 @@ 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); @@ -877,17 +877,17 @@ private void validate() { private boolean handleNativeClasses() { Object value = getObject(); if (value instanceof Number || value instanceof Boolean || value instanceof Character - || value instanceof TemporalAccessor ) { + || value instanceof TemporalAccessor) { getStringBuffer().append("value=").append(getObject().toString()); return true; } - + if (value.getClass().isArray() || value instanceof Collection || value instanceof Map) { ToStringStyle.unregister(value); getStyle().append(getStringBuffer(), null, value, true); return true; } - + return false; } 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 ed682f71a2e..100f607cd50 100644 --- a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java @@ -235,19 +235,18 @@ public void test_setUpToClass_invalid() { assertThrows(IllegalArgumentException.class, () -> test.setUpToClass(String.class)); test.toString(); } - - private static class HirAFixture extends HirBFixture{ + + private static class HirAFixture extends HirBFixture { int x = 1; } - private static class HirBFixture extends HirCFixture{ + private static class HirBFixture extends HirCFixture { int y = 2; } - + private static class HirCFixture { int z = 3; } - /** * Tests ReflectionToStringBuilder setUpToClass(). @@ -765,7 +764,7 @@ public void testObjectArray() { assertEquals(baseStr + "[]", new ToStringBuilder(base).append(array).toString()); assertEquals(baseStr + "[]", new ToStringBuilder(base).append((Object) array).toString()); } - + @Test public void testArrayList() { List list = Arrays.asList(23, 12, 39); From 0a41e5c4a9cef111ed481dc9c2a8a816c2e29168 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Wed, 4 Dec 2024 20:15:03 +0100 Subject: [PATCH 07/18] LANG-1265 pull the logic into central helper methods --- .../lang3/builder/CompareToBuilder.java | 3 +-- .../commons/lang3/builder/EqualsBuilder.java | 7 ++----- .../lang3/builder/HashCodeBuilder.java | 3 +-- .../commons/lang3/builder/Reflection.java | 21 +++++++++++++++++++ 4 files changed, 25 insertions(+), 9 deletions(-) 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 6acf01a8a88..483aa37cd9c 100644 --- a/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java @@ -19,7 +19,6 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.time.temporal.Temporal; import java.util.Collection; import java.util.Comparator; import java.util.Objects; @@ -116,7 +115,7 @@ private static void reflectionAppend( final boolean useTransients, final String[] excludeFields) { - if ((lhs instanceof CharSequence || lhs instanceof Number || lhs instanceof Temporal) && lhs instanceof Comparable) { + if (Reflection.isJavaInternalClass(lhs) && lhs instanceof Comparable) { builder.append(lhs, rhs); return; } 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 67183a2855e..0fbfd98cb08 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -19,7 +19,6 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.time.temporal.Temporal; import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -732,8 +731,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) - && !(CharSequence.class.isAssignableFrom(lhsClass) || Number.class.isAssignableFrom(lhsClass) || Temporal.class.isAssignableFrom(lhsClass))) { + if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass) && !Reflection.isJavaInternalClass(lhsClass)) { reflectionAppend(lhs, rhs); } else { isEquals = lhs.equals(rhs); @@ -958,8 +956,7 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } try { - if (testClass.isArray() || Number.class.isAssignableFrom(testClass) - || Temporal.class.isAssignableFrom(testClass) || CharSequence.class.isAssignableFrom(testClass)) { + if (testClass.isArray() || Reflection.isJavaInternalClass(testClass)) { append(lhs, rhs); } else //If either class is being excluded, call normal object equals method on lhsClass. if (bypassReflectionClasses != null 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 00d3b1c3144..a214b2ba7af 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -20,7 +20,6 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.time.temporal.Temporal; import java.util.Collection; import java.util.Comparator; import java.util.HashSet; @@ -181,7 +180,7 @@ private static void reflectionAppend(final Object object, final Class clazz, if (isRegistered(object)) { return; } - if (object instanceof CharSequence || object instanceof Number || object instanceof Temporal) { + if (Reflection.isJavaInternalClass(object)) { builder.append(object); return; } 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..1d7f140dcb3 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,6 +18,7 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; +import java.time.temporal.Temporal; import java.util.Objects; /** @@ -41,4 +42,24 @@ static Object getUnchecked(final Field field, final Object obj) { } } + /** + * Check whether the given object is of a type which is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc + * This is often needed as we cannot look into them via reflection since Java9. + * + * @return {@code true} if the given object is a Java internal class + */ + static boolean isJavaInternalClass(Object o) { + return o != null && isJavaInternalClass(o.getClass()); + } + + /** + * Check whether the given class is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc + * This is often needed as we cannot look into them via reflection since Java9. + * + * @return {@code true} if the given class is a Java internal class + */ + static boolean isJavaInternalClass(Class clazz) { + return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Temporal.class.isAssignableFrom(clazz); + } + } From 0bf6ceb0dc2f8380ef4f44f39831ed7f18c47097 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Wed, 4 Dec 2024 22:49:00 +0100 Subject: [PATCH 08/18] LANG-1265 unify to TemporalAccessor --- .../java/org/apache/commons/lang3/builder/Reflection.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 1d7f140dcb3..7fa9d514515 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,7 +18,7 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; -import java.time.temporal.Temporal; +import java.time.temporal.TemporalAccessor; import java.util.Objects; /** @@ -59,7 +59,7 @@ static boolean isJavaInternalClass(Object o) { * @return {@code true} if the given class is a Java internal class */ static boolean isJavaInternalClass(Class clazz) { - return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Temporal.class.isAssignableFrom(clazz); + return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || TemporalAccessor.class.isAssignableFrom(clazz); } } From bb7b412fa002f6d04291ad9294575acc72d21a8d Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 11:39:48 +0100 Subject: [PATCH 09/18] LANG-1265 Character is also a java native type * add Character handling * move RefletionToSTringBuilder to unified handling --- .../java/org/apache/commons/lang3/builder/Reflection.java | 4 +++- .../commons/lang3/builder/ReflectionToStringBuilder.java | 5 ++--- 2 files changed, 5 insertions(+), 4 deletions(-) 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 7fa9d514515..a4e11906b80 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -59,7 +59,9 @@ static boolean isJavaInternalClass(Object o) { * @return {@code true} if the given class is a Java internal class */ static boolean isJavaInternalClass(Class clazz) { - return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || TemporalAccessor.class.isAssignableFrom(clazz); + return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) + || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz) + || TemporalAccessor.class.isAssignableFrom(clazz); } } 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 9a3f242db1d..a5e61c44baa 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -20,7 +20,6 @@ import java.lang.reflect.AccessibleObject; import java.lang.reflect.Field; import java.lang.reflect.Modifier; -import java.time.temporal.TemporalAccessor; import java.util.Arrays; import java.util.Collection; import java.util.Comparator; @@ -876,13 +875,13 @@ private void validate() { private boolean handleNativeClasses() { Object value = getObject(); - if (value instanceof Number || value instanceof Boolean || value instanceof Character - || value instanceof TemporalAccessor) { + if (Reflection.isJavaInternalClass(value)) { getStringBuffer().append("value=").append(getObject().toString()); return true; } 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; From e353c3e591d6f5176687f2c5ac44ba1d6cdd9dda Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 13:43:53 +0100 Subject: [PATCH 10/18] LANG-1265 add EqualsBuilder support for Collections and Maps. --- .../commons/lang3/builder/EqualsBuilder.java | 42 ++++++++++++++++++- .../lang3/builder/EqualsBuilderTest.java | 20 +++++++++ 2 files changed, 60 insertions(+), 2 deletions(-) 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 0fbfd98cb08..342dc4c6a95 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; @@ -958,9 +960,45 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { try { if (testClass.isArray() || Reflection.isJavaInternalClass(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 (Object leftKey : lMap.keySet()) { + if (isEquals) { + append(lMap.get(leftKey), rMap.get(leftKey)); + } + } + } 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/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][]; From d3c8c9fa8617a1484716cdb5ac2cc6c40c9dfa86 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 14:15:27 +0100 Subject: [PATCH 11/18] LANG-1265 improve Map handling to make spotbugs happy --- .../org/apache/commons/lang3/builder/EqualsBuilder.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 342dc4c6a95..413c255b4a5 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -985,15 +985,15 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } } } else if (Map.class.isAssignableFrom(testClass)) { - Map lMap = (Map) lhs; - Map rMap = (Map) rhs; + Map lMap = (Map) lhs; + Map rMap = (Map) rhs; if (lMap.size() != rMap.size()) { isEquals = false; return this; } - for (Object leftKey : lMap.keySet()) { + for (Map.Entry e : lMap.entrySet()) { if (isEquals) { - append(lMap.get(leftKey), rMap.get(leftKey)); + append(e.getValue(), rMap.get(e.getKey())); } } } else if (bypassReflectionClasses != null From 60fd6cb49856e3702a008208c71ea5ac08ea96ad Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 14:39:45 +0100 Subject: [PATCH 12/18] remove nb-configuration.xml from rat excludes already is excluded via .gitignore exclusion it seems --- pom.xml | 1 - 1 file changed, 1 deletion(-) diff --git a/pom.xml b/pom.xml index c197deada66..73823abfb80 100644 --- a/pom.xml +++ b/pom.xml @@ -202,7 +202,6 @@ src/site/resources/download_lang.cgi src/site/resources/release-notes/RELEASE-NOTES-*.txt src/test/resources/lang-708-input.txt - nb-configuration.xml From f20639a039476826b9c146d96f64e6979ea7d772 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 20:30:09 +0100 Subject: [PATCH 13/18] LANG-1265 move to concrete types Not every CharSequence might implement equals, not every TemporalAccessor might be a java.* class. --- .../java/org/apache/commons/lang3/builder/Reflection.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 a4e11906b80..16760ace884 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,7 +18,7 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; -import java.time.temporal.TemporalAccessor; +import java.time.temporal.Temporal; import java.util.Objects; /** @@ -59,9 +59,9 @@ static boolean isJavaInternalClass(Object o) { * @return {@code true} if the given class is a Java internal class */ static boolean isJavaInternalClass(Class clazz) { - return CharSequence.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) + return String.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz) - || TemporalAccessor.class.isAssignableFrom(clazz); + || Temporal.class.isAssignableFrom(clazz); } } From ec4150bc13cfd2705e5d672f47706930af578c39 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Thu, 5 Dec 2024 22:06:31 +0100 Subject: [PATCH 14/18] LANG-1265 add Duration as additional non-reflection type --- src/main/java/org/apache/commons/lang3/builder/Reflection.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 16760ace884..fb88e252af9 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,6 +18,7 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; +import java.time.Duration; import java.time.temporal.Temporal; import java.util.Objects; @@ -61,7 +62,7 @@ static boolean isJavaInternalClass(Object o) { static boolean isJavaInternalClass(Class clazz) { return String.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz) - || Temporal.class.isAssignableFrom(clazz); + || Temporal.class.isAssignableFrom(clazz) || Duration.class.isAssignableFrom(clazz); } } From 08d913efdb08007ae6c6f7a2bcf430291cb0081c Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Fri, 6 Dec 2024 13:37:34 +0100 Subject: [PATCH 15/18] LANG-1265 adding Period as non reflexible class --- .../java/org/apache/commons/lang3/builder/Reflection.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 fb88e252af9..c74e24baa0c 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -19,6 +19,7 @@ import java.lang.reflect.Field; import java.time.Duration; +import java.time.Period; import java.time.temporal.Temporal; import java.util.Objects; @@ -62,7 +63,8 @@ static boolean isJavaInternalClass(Object o) { static boolean isJavaInternalClass(Class clazz) { return String.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz) - || Temporal.class.isAssignableFrom(clazz) || Duration.class.isAssignableFrom(clazz); + || Temporal.class.isAssignableFrom(clazz) || Duration.class.isAssignableFrom(clazz) + || Period.class.isAssignableFrom(clazz); } } From d951d0ef317660aaeb098919d5e1098031bc2b61 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Fri, 6 Dec 2024 14:39:02 +0100 Subject: [PATCH 16/18] LANG-1265 trying to solve inaccessibility in a generic way --- .../lang3/builder/CompareToBuilder.java | 2 +- .../commons/lang3/builder/EqualsBuilder.java | 4 +- .../lang3/builder/HashCodeBuilder.java | 2 +- .../commons/lang3/builder/Reflection.java | 47 ++++++++++++------- .../builder/ReflectionToStringBuilder.java | 10 ++-- 5 files changed, 40 insertions(+), 25 deletions(-) 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 483aa37cd9c..d6f036ff627 100644 --- a/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java @@ -115,7 +115,7 @@ private static void reflectionAppend( final boolean useTransients, final String[] excludeFields) { - if (Reflection.isJavaInternalClass(lhs) && lhs instanceof Comparable) { + if (Reflection.isInaccessibleClass(lhs) && lhs instanceof Comparable) { builder.append(lhs, rhs); return; } 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 413c255b4a5..16e140050f3 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -733,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) && !Reflection.isJavaInternalClass(lhsClass)) { + if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass) && !Reflection.isInaccessibleClass(lhsClass)) { reflectionAppend(lhs, rhs); } else { isEquals = lhs.equals(rhs); @@ -958,7 +958,7 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } try { - if (testClass.isArray() || Reflection.isJavaInternalClass(testClass)) { + if (testClass.isArray() || Reflection.isInaccessibleClass(testClass)) { append(lhs, rhs); } else if (Collection.class.isAssignableFrom(testClass)) { // Right now this also handles Sets. 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 a214b2ba7af..4f50e252e6b 100644 --- a/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java @@ -180,7 +180,7 @@ private static void reflectionAppend(final Object object, final Class clazz, if (isRegistered(object)) { return; } - if (Reflection.isJavaInternalClass(object)) { + if (Reflection.isInaccessibleClass(object)) { builder.append(object); return; } 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 c74e24baa0c..8bb68f5f6d6 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,16 +18,17 @@ package org.apache.commons.lang3.builder; import java.lang.reflect.Field; -import java.time.Duration; -import java.time.Period; -import java.time.temporal.Temporal; +import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; /** * Package-private reflection code. */ final class Reflection { + private static Map inaccessibleClasses = new ConcurrentHashMap<>(); + /** * Delegates to {@link Field#get(Object)} and rethrows {@link IllegalAccessException} as {@link IllegalArgumentException}. * @@ -45,26 +46,40 @@ static Object getUnchecked(final Field field, final Object obj) { } /** - * Check whether the given object is of a type which is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc - * This is often needed as we cannot look into them via reflection since Java9. + * 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 object is a Java internal class + * @return {@code true} if the given class internas not accessible */ - static boolean isJavaInternalClass(Object o) { - return o != null && isJavaInternalClass(o.getClass()); + static boolean isInaccessibleClass(Object o) { + return o != null && isInaccessibleClass(o.getClass()); } /** - * Check whether the given class is an internal java Class, like String, Integer, Boolean, LocalDateTime, etc - * This is often needed as we cannot look into them via reflection since Java9. + * 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 is a Java internal class + * @return {@code true} if the given class internas not accessible */ - static boolean isJavaInternalClass(Class clazz) { - return String.class.isAssignableFrom(clazz) || Number.class.isAssignableFrom(clazz) - || Boolean.class.isAssignableFrom(clazz) || Character.class.isAssignableFrom(clazz) - || Temporal.class.isAssignableFrom(clazz) || Duration.class.isAssignableFrom(clazz) - || Period.class.isAssignableFrom(clazz); + static boolean isInaccessibleClass(Class clazz) { + Boolean isAccessible = inaccessibleClasses.get(clazz); + if (isAccessible != null) { + return isAccessible; + } + try { + final Field[] declaredFields = clazz.getDeclaredFields(); + for (Field f : declaredFields) { + f.setAccessible(true); + } + + inaccessibleClasses.put(clazz, false); + return false; + } catch (Exception e) { + inaccessibleClasses.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 a5e61c44baa..8e908dcd71c 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -875,11 +875,6 @@ private void validate() { private boolean handleNativeClasses() { Object value = getObject(); - if (Reflection.isJavaInternalClass(value)) { - getStringBuffer().append("value=").append(getObject().toString()); - return true; - } - 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); @@ -887,6 +882,11 @@ private boolean handleNativeClasses() { return true; } + if (Reflection.isInaccessibleClass(value)) { + getStringBuffer().append("value=").append(getObject().toString()); + return true; + } + return false; } From 0d628cdade7fe481ba3048bd8f4f518c9dfa6df4 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Sat, 7 Dec 2024 14:34:00 +0100 Subject: [PATCH 17/18] LANG-1265 more work on inaccessible classes * cache for known inaccessible classes * add documentation for that cache. --- .../commons/lang3/builder/Reflection.java | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) 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 8bb68f5f6d6..055cc422656 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -18,8 +18,19 @@ 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; /** @@ -27,6 +38,40 @@ */ 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_INACCESSIBLE_CLASSES = new HashSet<>(); + { + KNOWN_INACCESSIBLE_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 #isInaccessibleClass(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 inaccessibleClasses = new ConcurrentHashMap<>(); /** @@ -64,6 +109,9 @@ static boolean isInaccessibleClass(Object o) { * @return {@code true} if the given class internas not accessible */ static boolean isInaccessibleClass(Class clazz) { + if (KNOWN_INACCESSIBLE_CLASSES.contains(clazz)) { + return true; + } Boolean isAccessible = inaccessibleClasses.get(clazz); if (isAccessible != null) { return isAccessible; From 0e916b83f24d6723c72412c64ecf1e6230d87aa5 Mon Sep 17 00:00:00 2001 From: Mark Struberg Date: Sat, 7 Dec 2024 14:51:14 +0100 Subject: [PATCH 18/18] LANG-1265 rename to isNonIntrospectibleClass txs to @erans for the name suggestion! --- .../lang3/builder/CompareToBuilder.java | 2 +- .../commons/lang3/builder/EqualsBuilder.java | 4 ++-- .../lang3/builder/HashCodeBuilder.java | 2 +- .../commons/lang3/builder/Reflection.java | 22 +++++++++---------- .../builder/ReflectionToStringBuilder.java | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) 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 d6f036ff627..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,7 +115,7 @@ private static void reflectionAppend( final boolean useTransients, final String[] excludeFields) { - if (Reflection.isInaccessibleClass(lhs) && lhs instanceof Comparable) { + if (Reflection.isNonIntrospectibleClass(lhs) && lhs instanceof Comparable) { builder.append(lhs, rhs); return; } 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 16e140050f3..ca7b6570805 100644 --- a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java @@ -733,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) && !Reflection.isInaccessibleClass(lhsClass)) { + if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass) && !Reflection.isNonIntrospectibleClass(lhsClass)) { reflectionAppend(lhs, rhs); } else { isEquals = lhs.equals(rhs); @@ -958,7 +958,7 @@ public EqualsBuilder reflectionAppend(final Object lhs, final Object rhs) { } try { - if (testClass.isArray() || Reflection.isInaccessibleClass(testClass)) { + if (testClass.isArray() || Reflection.isNonIntrospectibleClass(testClass)) { append(lhs, rhs); } else if (Collection.class.isAssignableFrom(testClass)) { // Right now this also handles Sets. 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 4f50e252e6b..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,7 +180,7 @@ private static void reflectionAppend(final Object object, final Class clazz, if (isRegistered(object)) { return; } - if (Reflection.isInaccessibleClass(object)) { + if (Reflection.isNonIntrospectibleClass(object)) { builder.append(object); return; } 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 055cc422656..b9737e780bc 100644 --- a/src/main/java/org/apache/commons/lang3/builder/Reflection.java +++ b/src/main/java/org/apache/commons/lang3/builder/Reflection.java @@ -42,9 +42,9 @@ 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_INACCESSIBLE_CLASSES = new HashSet<>(); + private static final Set KNOWN_NON_INTROSPECTIBLE_CLASSES = new HashSet<>(); { - KNOWN_INACCESSIBLE_CLASSES.addAll(Arrays.asList( + KNOWN_NON_INTROSPECTIBLE_CLASSES.addAll(Arrays.asList( Boolean.class, Character.class, String.class, @@ -68,11 +68,11 @@ final class Reflection { } /** - * this is a cache for {@link #isInaccessibleClass(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 inaccessibleClasses = new ConcurrentHashMap<>(); + private static Map nonIntrospectibleClasses = new ConcurrentHashMap<>(); /** * Delegates to {@link Field#get(Object)} and rethrows {@link IllegalAccessException} as {@link IllegalArgumentException}. @@ -97,8 +97,8 @@ static Object getUnchecked(final Field field, final Object obj) { * * @return {@code true} if the given class internas not accessible */ - static boolean isInaccessibleClass(Object o) { - return o != null && isInaccessibleClass(o.getClass()); + static boolean isNonIntrospectibleClass(Object o) { + return o != null && isNonIntrospectibleClass(o.getClass()); } /** @@ -108,11 +108,11 @@ static boolean isInaccessibleClass(Object o) { * * @return {@code true} if the given class internas not accessible */ - static boolean isInaccessibleClass(Class clazz) { - if (KNOWN_INACCESSIBLE_CLASSES.contains(clazz)) { + static boolean isNonIntrospectibleClass(Class clazz) { + if (KNOWN_NON_INTROSPECTIBLE_CLASSES.contains(clazz)) { return true; } - Boolean isAccessible = inaccessibleClasses.get(clazz); + Boolean isAccessible = nonIntrospectibleClasses.get(clazz); if (isAccessible != null) { return isAccessible; } @@ -122,10 +122,10 @@ static boolean isInaccessibleClass(Class clazz) { f.setAccessible(true); } - inaccessibleClasses.put(clazz, false); + nonIntrospectibleClasses.put(clazz, false); return false; } catch (Exception e) { - inaccessibleClasses.put(clazz, true); + 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 8e908dcd71c..0c5082043fa 100644 --- a/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java +++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java @@ -882,7 +882,7 @@ private boolean handleNativeClasses() { return true; } - if (Reflection.isInaccessibleClass(value)) { + if (Reflection.isNonIntrospectibleClass(value)) { getStringBuffer().append("value=").append(getObject().toString()); return true; }