From 1dabf262c90d76958175e98f7ac9d0189fd7fbf2 Mon Sep 17 00:00:00 2001 From: contextshuffling <55522232+contextshuffling@users.noreply.github.com> Date: Sat, 23 Nov 2019 20:51:32 -0600 Subject: [PATCH] sort fields in ReflectionToStringBuilder for deterministic order (#481) --- .../builder/ReflectionToStringBuilder.java | 3 +++ .../MultilineRecursiveToStringStyleTest.java | 22 ++++++++-------- .../builder/RecursiveToStringStyleTest.java | 2 +- .../lang3/builder/ToStringBuilderTest.java | 26 +++++++++---------- 4 files changed, 28 insertions(+), 25 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 392cdb539..aeed52e39 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.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Comparator; import java.util.List; import org.apache.commons.lang3.ArrayUtils; @@ -639,7 +640,9 @@ public class ReflectionToStringBuilder extends ToStringBuilder { this.reflectionAppendArray(this.getObject()); return; } + // The elements in the returned array are not sorted and are not in any particular order. final Field[] fields = clazz.getDeclaredFields(); + Arrays.sort(fields, Comparator.comparing(Field::getName)); AccessibleObject.setAccessible(fields, true); for (final Field field : fields) { final String fieldName = field.getName(); diff --git a/src/test/java/org/apache/commons/lang3/builder/MultilineRecursiveToStringStyleTest.java b/src/test/java/org/apache/commons/lang3/builder/MultilineRecursiveToStringStyleTest.java index 4eb1db4cb..4c302b8e2 100644 --- a/src/test/java/org/apache/commons/lang3/builder/MultilineRecursiveToStringStyleTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/MultilineRecursiveToStringStyleTest.java @@ -46,12 +46,12 @@ public class MultilineRecursiveToStringStyleTest { final Bank bank = new Bank("ASF Bank"); customer.bank = bank; final String exp = getClassPrefix(customer) + "[" + BR - + " name=Douglas Adams," + BR + + " accounts=," + BR + " bank=" + getClassPrefix(bank) + "[" + BR + " name=ASF Bank" + BR + " ]," + BR - + " accounts=" + BR - + "]"; + + " name=Douglas Adams" + BR + + "]"; assertEquals(exp, toString(customer)); } @@ -84,8 +84,8 @@ public class MultilineRecursiveToStringStyleTest { final String exp = getClassPrefix(wa) + "[" + BR + " boolArray=," + BR + " charArray=," + BR - + " intArray=," + BR + " doubleArray=," + BR + + " intArray=," + BR + " longArray=," + BR + " stringArray=" + BR + "]"; @@ -103,8 +103,8 @@ public class MultilineRecursiveToStringStyleTest { + " true" + BR + " }," + BR + " charArray=," + BR - + " intArray=," + BR + " doubleArray=," + BR + + " intArray=," + BR + " longArray=," + BR + " stringArray=" + BR + "]"; @@ -121,8 +121,8 @@ public class MultilineRecursiveToStringStyleTest { + " a," + BR + " A" + BR + " }," + BR - + " intArray=," + BR + " doubleArray=," + BR + + " intArray=," + BR + " longArray=," + BR + " stringArray=" + BR + "]"; @@ -136,11 +136,11 @@ public class MultilineRecursiveToStringStyleTest { final String exp = getClassPrefix(wa) + "[" + BR + " boolArray=," + BR + " charArray=," + BR + + " doubleArray=," + BR + " intArray={" + BR + " 1," + BR + " 2" + BR + " }," + BR - + " doubleArray=," + BR + " longArray=," + BR + " stringArray=" + BR + "]"; @@ -154,11 +154,11 @@ public class MultilineRecursiveToStringStyleTest { final String exp = getClassPrefix(wa) + "[" + BR + " boolArray=," + BR + " charArray=," + BR - + " intArray=," + BR + " doubleArray={" + BR + " 1.0," + BR + " 2.0" + BR + " }," + BR + + " intArray=," + BR + " longArray=," + BR + " stringArray=" + BR + "]"; @@ -172,8 +172,8 @@ public class MultilineRecursiveToStringStyleTest { final String exp = getClassPrefix(wa) + "[" + BR + " boolArray=," + BR + " charArray=," + BR - + " intArray=," + BR + " doubleArray=," + BR + + " intArray=," + BR + " longArray={" + BR + " 1," + BR + " 2" + BR @@ -190,8 +190,8 @@ public class MultilineRecursiveToStringStyleTest { final String exp = getClassPrefix(wa) + "[" + BR + " boolArray=," + BR + " charArray=," + BR - + " intArray=," + BR + " doubleArray=," + BR + + " intArray=," + BR + " longArray=," + BR + " stringArray={" + BR + " a," + BR @@ -226,8 +226,8 @@ public class MultilineRecursiveToStringStyleTest { static class WithArrays { boolean[] boolArray; char[] charArray; - int[] intArray; double[] doubleArray; + int[] intArray; long[] longArray; String[] stringArray; } diff --git a/src/test/java/org/apache/commons/lang3/builder/RecursiveToStringStyleTest.java b/src/test/java/org/apache/commons/lang3/builder/RecursiveToStringStyleTest.java index 9ef82f1fb..5a438ddd3 100644 --- a/src/test/java/org/apache/commons/lang3/builder/RecursiveToStringStyleTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/RecursiveToStringStyleTest.java @@ -91,7 +91,7 @@ public class RecursiveToStringStyleTest { p.job.title = "Manager"; final String pBaseStr = p.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(p)); final String pJobStr = p.job.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(p.job)); - assertEquals(pBaseStr + "[name=John Doe,age=33,smoker=false,job=" + pJobStr + "[title=Manager]]", + assertEquals(pBaseStr + "[age=33,job=" + pJobStr + "[title=Manager],name=John Doe,smoker=false]", new ReflectionToStringBuilder(p, new RecursiveToStringStyle()).toString()); } 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 72ce96550..b5fead221 100644 --- a/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java +++ b/src/test/java/org/apache/commons/lang3/builder/ToStringBuilderTest.java @@ -543,7 +543,7 @@ public class ToStringBuilderTest { @Test public void testSelfInstanceTwoVarsReflectionObjectCycle() { final SelfInstanceTwoVarsReflectionTestFixture test = new SelfInstanceTwoVarsReflectionTestFixture(); - assertEquals(this.toBaseString(test) + "[typeIsSelf=" + this.toBaseString(test) + ",otherType=" + test.getOtherType().toString() + "]", test.toString()); + assertEquals(this.toBaseString(test) + "[otherType=" + test.getOtherType().toString() + ",typeIsSelf=" + this.toBaseString(test) + "]", test.toString()); } @@ -1140,16 +1140,16 @@ public class ToStringBuilderTest { public void testSimpleReflectionStatics() { final SimpleReflectionStaticFieldsFixture instance1 = new SimpleReflectionStaticFieldsFixture(); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt=12345,staticString=staticString]", ReflectionToStringBuilder.toString(instance1, null, false, true, SimpleReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt=12345,staticString=staticString]", ReflectionToStringBuilder.toString(instance1, null, true, true, SimpleReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, SimpleReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, SimpleReflectionStaticFieldsFixture.class)); } @@ -1160,16 +1160,16 @@ public class ToStringBuilderTest { public void testReflectionStatics() { final ReflectionStaticFieldsFixture instance1 = new ReflectionStaticFieldsFixture(); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345,instanceString=instanceString,instanceInt=67890]", + this.toBaseString(instance1) + "[instanceInt=67890,instanceString=instanceString,staticInt=12345,staticString=staticString]", ReflectionToStringBuilder.toString(instance1, null, false, true, ReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345,staticTransientString=staticTransientString,staticTransientInt=54321,instanceString=instanceString,instanceInt=67890,transientString=transientString,transientInt=98765]", + this.toBaseString(instance1) + "[instanceInt=67890,instanceString=instanceString,staticInt=12345,staticString=staticString,staticTransientInt=54321,staticTransientString=staticTransientString,transientInt=98765,transientString=transientString]", ReflectionToStringBuilder.toString(instance1, null, true, true, ReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345,instanceString=instanceString,instanceInt=67890]", + this.toBaseString(instance1) + "[instanceInt=67890,instanceString=instanceString,staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, ReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString=staticString,staticInt=12345,instanceString=instanceString,instanceInt=67890]", + this.toBaseString(instance1) + "[instanceInt=67890,instanceString=instanceString,staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, ReflectionStaticFieldsFixture.class)); } @@ -1180,16 +1180,16 @@ public class ToStringBuilderTest { public void testInheritedReflectionStatics() { final InheritedReflectionStaticFieldsFixture instance1 = new InheritedReflectionStaticFieldsFixture(); assertEquals( - this.toBaseString(instance1) + "[staticString2=staticString2,staticInt2=67890]", + this.toBaseString(instance1) + "[staticInt2=67890,staticString2=staticString2]", ReflectionToStringBuilder.toString(instance1, null, false, true, InheritedReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString2=staticString2,staticInt2=67890,staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt2=67890,staticString2=staticString2,staticInt=12345,staticString=staticString]", ReflectionToStringBuilder.toString(instance1, null, false, true, SimpleReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString2=staticString2,staticInt2=67890,staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt2=67890,staticString2=staticString2,staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, SimpleReflectionStaticFieldsFixture.class)); assertEquals( - this.toBaseString(instance1) + "[staticString2=staticString2,staticInt2=67890,staticString=staticString,staticInt=12345]", + this.toBaseString(instance1) + "[staticInt2=67890,staticString2=staticString2,staticInt=12345,staticString=staticString]", this.toStringWithStatics(instance1, null, SimpleReflectionStaticFieldsFixture.class)); }