This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 1a6598c51 Align ReflectionDiffBuilder with AbstractReflection and add
cycle detection (#1778).
1a6598c51 is described below
commit 1a6598c51336de5611632fc672848919b0abfec3
Author: Gary Gregory <[email protected]>
AuthorDate: Sun Aug 23 19:10:43 2026 -0400
Align ReflectionDiffBuilder with AbstractReflection and add cycle
detection (#1778).
- Fix action order.
- Sort members.
---
src/changes/changes.xml | 2 +-
.../lang3/builder/ReflectionDiffBuilder.java | 38 +--
.../lang3/builder/ReflectionDiffBuilderTest.java | 300 ++++++++++-----------
3 files changed, 170 insertions(+), 170 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 264f3a821..b084f5694 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -46,7 +46,6 @@ The <action> type attribute can be add,update,fix,remove.
<body>
<release version="3.21.0" date="YYY-MM-DD" description="This is a feature
and maintenance release. Java 8 or later is required.">
<!-- FIX -->
- <action type="fix" dev="ggregory" due-to="Gaurav Pandey,
Gary Gregory">Align ReflectionDiffBuilder with AbstractReflection and add cycle
detection to prevent StackOverflowError on cyclic object graphs.</action>
<action type="fix" dev="ggregory" due-to="Javid Khan,
Gary Gregory">Stop ExtendedMessageFormat seekNonWs reading past the pattern
end.</action>
<action type="fix" dev="ggregory" due-to="ThrawnCA">Fix
spelling and grammar in StringUtils #1486.</action>
<action type="fix" dev="ggregory" due-to="Michael
Hausegger, Gary Gregory">Add ConversionTest assertions to increase coverage
#1489.</action>
@@ -250,6 +249,7 @@ java.lang.NullPointerException: Cannot invoke
<action type="fix" dev="ggregory" due-to="Gary
Gregory">Fix SpotBugs USO_UNSAFE_METHOD_SYNCHRONIZATION in CharSet.</action>
<action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Keep StopWatch.formatSplitTime from clamping splits to int millis
(#1777).</action>
<action type="fix" dev="ggregory" due-to="gaurav kumar
pandey, Gary Gregory">Keep StringUtils left, right, mid, and overlay off
surrogate pair boundaries (#1776).</action>
+ <action type="fix" dev="ggregory" due-to="Gaurav Pandey,
Gary Gregory">Align ReflectionDiffBuilder with AbstractReflection and add cycle
detection to prevent StackOverflowError on cyclic object graphs.</action>
<!-- ADD -->
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add JavaVersion.JAVA_27.</action>
<action type="add" dev="ggregory" due-to="Gary
Gregory">Add SystemUtils.IS_JAVA_27.</action>
diff --git
a/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java
b/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java
index e2209ccfc..a1c4a37bd 100644
--- a/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java
+++ b/src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java
@@ -142,6 +142,17 @@ public Builder<T> setExcludeFieldNames(final String...
excludeFieldNames) {
*/
private static final ThreadLocal<Set<Pair<IDKey, IDKey>>> REGISTRY =
ThreadLocal.withInitial(HashSet::new);
+ /**
+ * Constructs a new {@link Builder}.
+ *
+ * @param <T> type of the left and right object.
+ * @return A new {@link Builder}.
+ * @since 3.15.0
+ */
+ public static <T> Builder<T> builder() {
+ return new Builder<>();
+ }
+
/**
* Gets the registry of object pairs being traversed by the reflection
* methods in the current thread.
@@ -179,6 +190,14 @@ static void register(final Object lhs, final Object rhs) {
register(lhs, rhs, getRegistry());
}
+ private static String[] toExcludeFieldNames(final String[]
excludeFieldNames) {
+ if (excludeFieldNames == null) {
+ return ArrayUtils.EMPTY_STRING_ARRAY;
+ }
+ // clone and remove nulls
+ return
ArraySorter.sort(ReflectionToStringBuilder.toNoNullStringArray(excludeFieldNames));
+ }
+
/**
* Unregisters the given object pair.
*
@@ -193,25 +212,6 @@ static void unregister(final Object lhs, final Object rhs)
{
unregister(lhs, rhs, getRegistry(), REGISTRY);
}
- /**
- * Constructs a new {@link Builder}.
- *
- * @param <T> type of the left and right object.
- * @return A new {@link Builder}.
- * @since 3.15.0
- */
- public static <T> Builder<T> builder() {
- return new Builder<>();
- }
-
- private static String[] toExcludeFieldNames(final String[]
excludeFieldNames) {
- if (excludeFieldNames == null) {
- return ArrayUtils.EMPTY_STRING_ARRAY;
- }
- // clone and remove nulls
- return
ArraySorter.sort(ReflectionToStringBuilder.toNoNullStringArray(excludeFieldNames));
- }
-
private final DiffBuilder<T> diffBuilder;
/**
diff --git
a/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java
b/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java
index 20c3aa6ce..2df6b21ab 100644
---
a/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java
+++
b/src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java
@@ -46,6 +46,39 @@ private static class AtomicIntegerWrapper {
}
}
+ private static final class CycleDiffableNode implements
Diffable<CycleDiffableNode> {
+ @SuppressWarnings("unused")
+ private CycleDiffableNode self;
+ @SuppressWarnings("unused")
+ private final String value;
+
+ CycleDiffableNode(final String value) {
+ this.value = value;
+ }
+
+ @Override
+ public DiffResult<CycleDiffableNode> diff(final CycleDiffableNode obj)
{
+ return ReflectionDiffBuilder.<CycleDiffableNode>builder()
+ .setDiffBuilder(DiffBuilder.<CycleDiffableNode>builder()
+ .setLeft(this)
+ .setRight(obj)
+ .setStyle(ToStringStyle.SHORT_PREFIX_STYLE)
+ .build())
+ .build()
+ .build();
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ return EqualsBuilder.reflectionEquals(this, obj);
+ }
+
+ @Override
+ public int hashCode() {
+ return HashCodeBuilder.reflectionHashCode(this);
+ }
+ }
+
private static class FloatWrapper {
private /* not final might not matter for the test. */ float value;
@@ -152,6 +185,39 @@ public int hashCode() {
}
+ private static final class MutualDiffableNode implements
Diffable<MutualDiffableNode> {
+ @SuppressWarnings("unused")
+ private MutualDiffableNode other;
+ @SuppressWarnings("unused")
+ private final String name;
+
+ MutualDiffableNode(final String name) {
+ this.name = name;
+ }
+
+ @Override
+ public DiffResult<MutualDiffableNode> diff(final MutualDiffableNode
obj) {
+ return ReflectionDiffBuilder.<MutualDiffableNode>builder()
+ .setDiffBuilder(DiffBuilder.<MutualDiffableNode>builder()
+ .setLeft(this)
+ .setRight(obj)
+ .setStyle(ToStringStyle.SHORT_PREFIX_STYLE)
+ .build())
+ .build()
+ .build();
+ }
+
+ @Override
+ public boolean equals(final Object obj) {
+ return EqualsBuilder.reflectionEquals(this, obj);
+ }
+
+ @Override
+ public int hashCode() {
+ return HashCodeBuilder.reflectionHashCode(this);
+ }
+ }
+
@SuppressWarnings("unused")
private static final class TypeTestChildClass extends TypeTestClass {
String field = "a";
@@ -258,6 +324,90 @@ void testBigIntegerDifference() {
assertEquals(1, list.getNumberOfDiffs());
}
+ @Test
+ void testBuilderGetAndSetForceAccessible() {
+ final TypeTestClass first = new TypeTestClass();
+ final TypeTestClass second = new TypeTestClass();
+ final ReflectionDiffBuilder.Builder<TypeTestClass> builder =
ReflectionDiffBuilder.<TypeTestClass>builder()
+
.setDiffBuilder(DiffBuilder.<TypeTestClass>builder().setLeft(first).setRight(second).build())
+ .setForceAccessible(true);
+ final ReflectionDiffBuilder<TypeTestClass> diffBuilder = builder.get();
+ assertNotNull(diffBuilder);
+ assertTrue(diffBuilder.isForceAccessible());
+ assertEquals(0, diffBuilder.build().getNumberOfDiffs());
+ }
+
+ @Test
+ void testCycleAsymmetric() {
+ final CycleDiffableNode first = new CycleDiffableNode("a");
+ final CycleDiffableNode second = new CycleDiffableNode("a");
+ first.self = first;
+ second.self = null;
+
+ final DiffResult<CycleDiffableNode> result = first.diff(second);
+ assertEquals(1, result.getNumberOfDiffs());
+ assertEquals("self", result.getDiffs().get(0).getFieldName());
+ assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
+ }
+
+ @Test
+ void testCycleMutuallyReferential() {
+ final MutualDiffableNode a = new MutualDiffableNode("node");
+ final MutualDiffableNode b = new MutualDiffableNode("node");
+ a.other = b;
+ b.other = a;
+
+ final MutualDiffableNode c = new MutualDiffableNode("node");
+ final MutualDiffableNode d = new MutualDiffableNode("node");
+ c.other = d;
+ d.other = c;
+
+ final DiffResult<MutualDiffableNode> result = a.diff(c);
+ assertEquals(0, result.getNumberOfDiffs());
+ assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
+ }
+
+ @Test
+ void testCycleMutuallyReferentialWithDifference() {
+ final MutualDiffableNode a = new MutualDiffableNode("nodeA");
+ final MutualDiffableNode b = new MutualDiffableNode("nodeB");
+ a.other = b;
+ b.other = a;
+
+ final MutualDiffableNode c = new MutualDiffableNode("nodeA");
+ final MutualDiffableNode d = new MutualDiffableNode("nodeChanged");
+ c.other = d;
+ d.other = c;
+
+ final DiffResult<MutualDiffableNode> result = a.diff(c);
+ assertEquals(1, result.getNumberOfDiffs());
+ assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
+ }
+
+ @Test
+ void testCycleSelfReferential() {
+ final CycleDiffableNode first = new CycleDiffableNode("a");
+ final CycleDiffableNode second = new CycleDiffableNode("a");
+ first.self = first;
+ second.self = second;
+
+ final DiffResult<CycleDiffableNode> result = first.diff(second);
+ assertEquals(0, result.getNumberOfDiffs());
+ assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
+ }
+
+ @Test
+ void testCycleSelfReferentialWithDifference() {
+ final CycleDiffableNode first = new CycleDiffableNode("a");
+ final CycleDiffableNode second = new CycleDiffableNode("b");
+ first.self = first;
+ second.self = second;
+
+ final DiffResult<CycleDiffableNode> result = first.diff(second);
+ assertEquals(2, result.getNumberOfDiffs());
+ assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
+ }
+
@Test
void testDifferenceInInherited_field() {
final TypeTestChildClass firstObject = new TypeTestChildClass();
@@ -546,154 +696,4 @@ void testTransientFieldDifference() {
assertEquals(0, list.getNumberOfDiffs());
}
- private static final class CycleDiffableNode implements
Diffable<CycleDiffableNode> {
- @SuppressWarnings("unused")
- private CycleDiffableNode self;
- @SuppressWarnings("unused")
- private final String value;
-
- CycleDiffableNode(final String value) {
- this.value = value;
- }
-
- @Override
- public boolean equals(final Object obj) {
- return EqualsBuilder.reflectionEquals(this, obj);
- }
-
- @Override
- public int hashCode() {
- return HashCodeBuilder.reflectionHashCode(this);
- }
-
- @Override
- public DiffResult<CycleDiffableNode> diff(final CycleDiffableNode obj)
{
- return ReflectionDiffBuilder.<CycleDiffableNode>builder()
- .setDiffBuilder(DiffBuilder.<CycleDiffableNode>builder()
- .setLeft(this)
- .setRight(obj)
- .setStyle(ToStringStyle.SHORT_PREFIX_STYLE)
- .build())
- .build()
- .build();
- }
- }
-
- private static final class MutualDiffableNode implements
Diffable<MutualDiffableNode> {
- @SuppressWarnings("unused")
- private MutualDiffableNode other;
- @SuppressWarnings("unused")
- private final String name;
-
- MutualDiffableNode(final String name) {
- this.name = name;
- }
-
- @Override
- public boolean equals(final Object obj) {
- return EqualsBuilder.reflectionEquals(this, obj);
- }
-
- @Override
- public int hashCode() {
- return HashCodeBuilder.reflectionHashCode(this);
- }
-
- @Override
- public DiffResult<MutualDiffableNode> diff(final MutualDiffableNode
obj) {
- return ReflectionDiffBuilder.<MutualDiffableNode>builder()
- .setDiffBuilder(DiffBuilder.<MutualDiffableNode>builder()
- .setLeft(this)
- .setRight(obj)
- .setStyle(ToStringStyle.SHORT_PREFIX_STYLE)
- .build())
- .build()
- .build();
- }
- }
-
- @Test
- void testCycleSelfReferential() {
- final CycleDiffableNode first = new CycleDiffableNode("a");
- final CycleDiffableNode second = new CycleDiffableNode("a");
- first.self = first;
- second.self = second;
-
- final DiffResult<CycleDiffableNode> result = first.diff(second);
- assertEquals(0, result.getNumberOfDiffs());
- assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
- }
-
- @Test
- void testCycleSelfReferentialWithDifference() {
- final CycleDiffableNode first = new CycleDiffableNode("a");
- final CycleDiffableNode second = new CycleDiffableNode("b");
- first.self = first;
- second.self = second;
-
- final DiffResult<CycleDiffableNode> result = first.diff(second);
- assertEquals(2, result.getNumberOfDiffs());
- assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
- }
-
- @Test
- void testCycleMutuallyReferential() {
- final MutualDiffableNode a = new MutualDiffableNode("node");
- final MutualDiffableNode b = new MutualDiffableNode("node");
- a.other = b;
- b.other = a;
-
- final MutualDiffableNode c = new MutualDiffableNode("node");
- final MutualDiffableNode d = new MutualDiffableNode("node");
- c.other = d;
- d.other = c;
-
- final DiffResult<MutualDiffableNode> result = a.diff(c);
- assertEquals(0, result.getNumberOfDiffs());
- assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
- }
-
- @Test
- void testCycleMutuallyReferentialWithDifference() {
- final MutualDiffableNode a = new MutualDiffableNode("nodeA");
- final MutualDiffableNode b = new MutualDiffableNode("nodeB");
- a.other = b;
- b.other = a;
-
- final MutualDiffableNode c = new MutualDiffableNode("nodeA");
- final MutualDiffableNode d = new MutualDiffableNode("nodeChanged");
- c.other = d;
- d.other = c;
-
- final DiffResult<MutualDiffableNode> result = a.diff(c);
- assertEquals(1, result.getNumberOfDiffs());
- assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
- }
-
- @Test
- void testCycleAsymmetric() {
- final CycleDiffableNode first = new CycleDiffableNode("a");
- final CycleDiffableNode second = new CycleDiffableNode("a");
- first.self = first;
- second.self = null;
-
- final DiffResult<CycleDiffableNode> result = first.diff(second);
- assertEquals(1, result.getNumberOfDiffs());
- assertEquals("self", result.getDiffs().get(0).getFieldName());
- assertTrue(ReflectionDiffBuilder.getRegistry().isEmpty(), "Registry
must be empty after diff");
- }
-
- @Test
- void testBuilderGetAndSetForceAccessible() {
- final TypeTestClass first = new TypeTestClass();
- final TypeTestClass second = new TypeTestClass();
- final ReflectionDiffBuilder.Builder<TypeTestClass> builder =
ReflectionDiffBuilder.<TypeTestClass>builder()
-
.setDiffBuilder(DiffBuilder.<TypeTestClass>builder().setLeft(first).setRight(second).build())
- .setForceAccessible(true);
- final ReflectionDiffBuilder<TypeTestClass> diffBuilder = builder.get();
- assertNotNull(diffBuilder);
- assertTrue(diffBuilder.isForceAccessible());
- assertEquals(0, diffBuilder.build().getNumberOfDiffs());
- }
-
}