From ec4a0fdfac5f8e27aec46f9e04a255cf31b40686 Mon Sep 17 00:00:00 2001 From: Henri Yandell Date: Sat, 27 Jan 2007 07:11:08 +0000 Subject: [PATCH] Applying a modified version of Maarten Coene's patch for #LANG-69. All unit tests pass; opinions would be very welcome though. git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/lang/trunk@500495 13f79535-47bb-0310-9956-ffa450edef68 --- .../builder/ReflectionToStringBuilder.java | 153 +++--------------- .../commons/lang/builder/ToStringStyle.java | 117 ++++++++++++-- .../lang/builder/ToStringBuilderTest.java | 110 ++++++------- 3 files changed, 183 insertions(+), 197 deletions(-) diff --git a/src/java/org/apache/commons/lang/builder/ReflectionToStringBuilder.java b/src/java/org/apache/commons/lang/builder/ReflectionToStringBuilder.java index 7e2e30921..ccf56bac6 100644 --- a/src/java/org/apache/commons/lang/builder/ReflectionToStringBuilder.java +++ b/src/java/org/apache/commons/lang/builder/ReflectionToStringBuilder.java @@ -95,57 +95,6 @@ * @version $Id$ */ public class ReflectionToStringBuilder extends ToStringBuilder { - /** - *

- * A registry of objects used by reflectionToString methods to detect cyclical object references and - * avoid infinite loops. - *

- */ - private static ThreadLocal registry = new ThreadLocal() { - protected synchronized Object initialValue() { - // The HashSet implementation is not synchronized, - // which is just what we need here. - return new HashSet(); - } - }; - - /** - *

- * Returns the registry of objects being traversed by the reflectionToString methods in the current - * thread. - *

- * - * @return Set the registry of objects being traversed - */ - static Set getRegistry() { - return (Set) registry.get(); - } - - /** - *

- * Returns true if the registry contains the given object. Used by the reflection methods to avoid - * infinite loops. - *

- * - * @param value - * The object to lookup in the registry. - * @return boolean true if the registry contains the given object. - */ - static boolean isRegistered(Object value) { - return getRegistry().contains(value); - } - - /** - *

- * Registers the given object. Used by the reflection methods to avoid infinite loops. - *

- * - * @param value - * The object to register. - */ - static void register(Object value) { - getRegistry().add(value); - } /** *

@@ -462,22 +411,6 @@ public static String toStringExclude(Object object, String[] excludeFieldNames) return new ReflectionToStringBuilder(object).setExcludeFieldNames(excludeFieldNames).toString(); } - /** - *

- * Unregisters the given object. - *

- * - *

- * Used by the reflection methods to avoid infinite loops. - *

- * - * @param value - * The object to unregister. - */ - static void unregister(Object value) { - getRegistry().remove(value); - } - /** * Whether or not to append static fields. */ @@ -657,60 +590,29 @@ protected boolean accept(Field field) { * The class of object parameter */ protected void appendFieldsIn(Class clazz) { - if (isRegistered(this.getObject())) { - // The object has already been appended, therefore we have an - // object cycle. - // Append a simple Object.toString style string. The field name is - // already appended at this point. - this.appendAsObjectToString(this.getObject()); + if (clazz.isArray()) { + this.reflectionAppendArray(this.getObject()); return; } - try { - this.registerObject(); - if (clazz.isArray()) { - this.reflectionAppendArray(this.getObject()); - return; - } - Field[] fields = clazz.getDeclaredFields(); - AccessibleObject.setAccessible(fields, true); - for (int i = 0; i < fields.length; i++) { - Field field = fields[i]; - String fieldName = field.getName(); - if (this.accept(field)) { - try { - // Warning: Field.get(Object) creates wrappers objects - // for primitive types. - Object fieldValue = this.getValue(field); - if (isRegistered(fieldValue) && !field.getType().isPrimitive()) { - // A known field value has already been appended, - // therefore we have an object cycle, - // append a simple Object.toString style string. - this.getStyle().appendFieldStart(this.getStringBuffer(), fieldName); - this.appendAsObjectToString(fieldValue); - this.getStyle().appendFieldEnd(this.getStringBuffer(), fieldName); - // The recursion out of - // builder.append(fieldName, fieldValue); - // below will append the field - // end marker. - } else { - try { - this.registerObject(); - this.append(fieldName, fieldValue); - } finally { - this.unregisterObject(); - } - } - } catch (IllegalAccessException ex) { - // this can't happen. Would get a Security exception - // instead - // throw a runtime exception in case the impossible - // happens. - throw new InternalError("Unexpected IllegalAccessException: " + ex.getMessage()); - } + Field[] fields = clazz.getDeclaredFields(); + AccessibleObject.setAccessible(fields, true); + for (int i = 0; i < fields.length; i++) { + Field field = fields[i]; + String fieldName = field.getName(); + if (this.accept(field)) { + try { + // Warning: Field.get(Object) creates wrappers objects + // for primitive types. + Object fieldValue = this.getValue(field); + this.append(fieldName, fieldValue); + } catch (IllegalAccessException ex) { + //this can't happen. Would get a Security exception + // instead + //throw a runtime exception in case the impossible + // happens. + throw new InternalError("Unexpected IllegalAccessException: " + ex.getMessage()); } } - } finally { - this.unregisterObject(); } } @@ -789,15 +691,6 @@ public ToStringBuilder reflectionAppendArray(Object array) { return this; } - /** - *

- * Registers this builder's source object to avoid infinite loops when processing circular object references. - *

- */ - void registerObject() { - register(this.getObject()); - } - /** *

* Sets whether or not to append static fields. @@ -872,12 +765,4 @@ public String toString() { return super.toString(); } - /** - *

- * Unregisters this builder's source object to avoid infinite loops when processing circular object references. - *

- */ - void unregisterObject() { - unregister(this.getObject()); - } } diff --git a/src/java/org/apache/commons/lang/builder/ToStringStyle.java b/src/java/org/apache/commons/lang/builder/ToStringStyle.java index 5de41cddc..94637519e 100644 --- a/src/java/org/apache/commons/lang/builder/ToStringStyle.java +++ b/src/java/org/apache/commons/lang/builder/ToStringStyle.java @@ -19,7 +19,9 @@ import java.io.Serializable; import java.lang.reflect.Array; import java.util.Collection; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.apache.commons.lang.ClassUtils; import org.apache.commons.lang.ObjectUtils; @@ -94,6 +96,78 @@ public abstract class ToStringStyle implements Serializable { */ public static final ToStringStyle SIMPLE_STYLE = new SimpleToStringStyle(); + /** + *

+ * A registry of objects used by reflectionToString methods + * to detect cyclical object references and avoid infinite loops. + *

+ */ + private static ThreadLocal registry = new ThreadLocal() { + protected synchronized Object initialValue() { + // The HashSet implementation is not synchronized, + // which is just what we need here. + return new HashSet(); + } + }; + + /** + *

+ * Returns the registry of objects being traversed by the reflectionToString + * methods in the current thread. + *

+ * + * @return Set the registry of objects being traversed + */ + static Set getRegistry() { + return (Set) registry.get(); + } + + /** + *

+ * Returns true if the registry contains the given object. + * Used by the reflection methods to avoid infinite loops. + *

+ * + * @param value + * The object to lookup in the registry. + * @return boolean true if the registry contains the given + * object. + */ + static boolean isRegistered(Object value) { + return getRegistry().contains(value); + } + + /** + *

+ * Registers the given object. Used by the reflection methods to avoid + * infinite loops. + *

+ * + * @param value + * The object to register. + */ + static void register(Object value) { + if (value != null) { + getRegistry().add(value); + } + } + + /** + *

+ * Unregisters the given object. + *

+ * + *

+ * Used by the reflection methods to avoid infinite loops. + *

+ * + * @param value + * The object to unregister. + */ + static void unregister(Object value) { + getRegistry().remove(value); + } + /** * Whether to use the field names, the default is true. */ @@ -272,6 +346,7 @@ public void appendEnd(StringBuffer buffer, Object object) { removeLastFieldSeparator(buffer); } appendContentEnd(buffer); + unregister(object); } /** @@ -343,11 +418,14 @@ public void append(StringBuffer buffer, String fieldName, Object value, Boolean * @param detail output detail or not */ protected void appendInternal(StringBuffer buffer, String fieldName, Object value, boolean detail) { - if (ReflectionToStringBuilder.isRegistered(value) + if (isRegistered(value) && !(value instanceof Number || value instanceof Boolean || value instanceof Character)) { - ObjectUtils.appendIdentityToString(buffer, value); - - } else if (value instanceof Collection) { + appendCyclicObject(buffer, fieldName, value); + return; + } + register(value); +try { + if (value instanceof Collection) { if (detail) { appendDetail(buffer, fieldName, (Collection) value); } else { @@ -425,12 +503,31 @@ protected void appendInternal(StringBuffer buffer, String fieldName, Object valu } } else { - if (detail) { - appendDetail(buffer, fieldName, value); - } else { - appendSummary(buffer, fieldName, value); - } + if (detail) { + appendDetail(buffer, fieldName, value); + } else { + appendSummary(buffer, fieldName, value); + } } + } finally { + unregister(value); + } + } + + /** + *

Append to the toString an Object + * value that has been detected to participate in a cycle. This + * implementation will print the standard string value of the value.

+ * + * @param buffer the StringBuffer to populate + * @param fieldName the field name, typically not used as already appended + * @param value the value to add to the toString, + * not null + * + * @since 2.2 + */ + protected void appendCyclicObject(StringBuffer buffer, String fieldName, Object value) { + ObjectUtils.appendIdentityToString(buffer, value); } /** @@ -1301,6 +1398,7 @@ protected void appendSummary(StringBuffer buffer, String fieldName, boolean[] ar */ protected void appendClassName(StringBuffer buffer, Object object) { if (useClassName && object != null) { + register(object); if (useShortClassName) { buffer.append(getShortClassName(object.getClass())); } else { @@ -1317,6 +1415,7 @@ protected void appendClassName(StringBuffer buffer, Object object) { */ protected void appendIdentityHashCode(StringBuffer buffer, Object object) { if (this.isUseIdentityHashCode() && object!=null) { + register(object); buffer.append('@'); buffer.append(Integer.toHexString(System.identityHashCode(object))); } diff --git a/src/test/org/apache/commons/lang/builder/ToStringBuilderTest.java b/src/test/org/apache/commons/lang/builder/ToStringBuilderTest.java index d998721d6..d9d8f5306 100644 --- a/src/test/org/apache/commons/lang/builder/ToStringBuilderTest.java +++ b/src/test/org/apache/commons/lang/builder/ToStringBuilderTest.java @@ -168,7 +168,7 @@ public void testReflectionObjectArray() { assertEquals(baseStr + "[{,5,{3,6}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionLongArray() { @@ -177,7 +177,7 @@ public void testReflectionLongArray() { assertEquals(baseStr + "[{1,2,-3,4}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionIntArray() { @@ -186,7 +186,7 @@ public void testReflectionIntArray() { assertEquals(baseStr + "[{1,2,-3,4}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionShortArray() { @@ -195,7 +195,7 @@ public void testReflectionShortArray() { assertEquals(baseStr + "[{1,2,-3,4}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionyteArray() { @@ -204,7 +204,7 @@ public void testReflectionyteArray() { assertEquals(baseStr + "[{1,2,-3,4}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionCharArray() { @@ -213,7 +213,7 @@ public void testReflectionCharArray() { assertEquals(baseStr + "[{A,2,_,D}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionDoubleArray() { @@ -222,7 +222,7 @@ public void testReflectionDoubleArray() { assertEquals(baseStr + "[{1.0,2.9876,-3.00001,4.3}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionFloatArray() { @@ -231,7 +231,7 @@ public void testReflectionFloatArray() { assertEquals(baseStr + "[{1.0,2.9876,-3.00001,4.3}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionBooleanArray() { @@ -240,7 +240,7 @@ public void testReflectionBooleanArray() { assertEquals(baseStr + "[{true,false,false}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } // Reflection Array Array tests @@ -251,7 +251,7 @@ public void testReflectionFloatArrayArray() { assertEquals(baseStr + "[{{1.0,2.29686},,{NaN}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } @@ -261,7 +261,7 @@ public void testReflectionLongArrayArray() { assertEquals(baseStr + "[{{1,2},,{5}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionIntArrayArray() { @@ -270,7 +270,7 @@ public void testReflectionIntArrayArray() { assertEquals(baseStr + "[{{1,2},,{5}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionhortArrayArray() { @@ -279,7 +279,7 @@ public void testReflectionhortArrayArray() { assertEquals(baseStr + "[{{1,2},,{5}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionByteArrayArray() { @@ -288,7 +288,7 @@ public void testReflectionByteArrayArray() { assertEquals(baseStr + "[{{1,2},,{5}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionCharArrayArray() { @@ -297,7 +297,7 @@ public void testReflectionCharArrayArray() { assertEquals(baseStr + "[{{A,B},,{p}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionDoubleArrayArray() { @@ -306,7 +306,7 @@ public void testReflectionDoubleArrayArray() { assertEquals(baseStr + "[{{1.0,2.29686},,{NaN}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionBooleanArrayArray() { @@ -316,7 +316,7 @@ public void testReflectionBooleanArrayArray() { assertEquals(baseStr + "[{{true,false},,{false}}]", ToStringBuilder.reflectionToString(array)); array = null; assertReflectionArray("", array); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } // Reflection hierarchy tests @@ -326,7 +326,7 @@ public void testReflectionHierarchyArrayList() { String baseStr = this.toBaseString(base); assertEquals(baseStr + "[elementData={,,,,,,,,,},size=0,modCount=0]", ToStringBuilder.reflectionToString(base, null, true)); assertEquals(baseStr + "[size=0]", ToStringBuilder.reflectionToString(base, null, false)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionHierarchy() { @@ -353,7 +353,7 @@ public void testReflectionHierarchy() { assertEquals(baseStr + "[b=b,a=a]", ToStringBuilder.reflectionToString(baseB, null, false, List.class)); assertEquals(baseStr + "[b=b,a=a]", ToStringBuilder.reflectionToString(baseB, null, false, ReflectionTestFixtureA.class)); assertEquals(baseStr + "[b=b]", ToStringBuilder.reflectionToString(baseB, null, false, ReflectionTestFixtureB.class)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } static class ReflectionTestFixtureA { @@ -394,7 +394,7 @@ public void testReflectionArrayCycle() throws Exception { assertEquals( this.toBaseString(objects) + "[{" + this.toBaseString(objects) + "}]", ToStringBuilder.reflectionToString(objects)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } /** @@ -411,7 +411,7 @@ public void testReflectionArrayCycleLevel2() throws Exception { assertEquals( this.toBaseString(objectsLevel2) + "[{{" + this.toBaseString(objectsLevel2) + "}}]", ToStringBuilder.reflectionToString(objectsLevel2)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } public void testReflectionArrayArrayCycle() throws Exception { @@ -433,7 +433,7 @@ public void testReflectionArrayArrayCycle() throws Exception { + basicToString + "}}]", ToStringBuilder.reflectionToString(objects)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } /** @@ -514,9 +514,9 @@ public String toString() { public void testSimpleReflectionObjectCycle() throws Exception { SimpleReflectionTestFixture simple = new SimpleReflectionTestFixture(); simple.o = simple; - assertTrue(ReflectionToStringBuilder.getRegistry().isEmpty()); + assertTrue(ToStringStyle.getRegistry().isEmpty()); assertEquals(this.toBaseString(simple) + "[o=" + this.toBaseString(simple) + "]", simple.toString()); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } /** @@ -526,9 +526,9 @@ public void testSimpleReflectionObjectCycle() throws Exception { */ public void testSelfInstanceVarReflectionObjectCycle() throws Exception { SelfInstanceVarReflectionTestFixture test = new SelfInstanceVarReflectionTestFixture(); - assertTrue(ReflectionToStringBuilder.getRegistry().isEmpty()); + assertTrue(ToStringStyle.getRegistry().isEmpty()); assertEquals(this.toBaseString(test) + "[typeIsSelf=" + this.toBaseString(test) + "]", test.toString()); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } /** @@ -539,9 +539,9 @@ public void testSelfInstanceVarReflectionObjectCycle() throws Exception { */ public void testSelfInstanceTwoVarsReflectionObjectCycle() throws Exception { SelfInstanceTwoVarsReflectionTestFixture test = new SelfInstanceTwoVarsReflectionTestFixture(); - assertTrue(ReflectionToStringBuilder.getRegistry().isEmpty()); + assertTrue(ToStringStyle.getRegistry().isEmpty()); assertEquals(this.toBaseString(test) + "[typeIsSelf=" + this.toBaseString(test) + ",otherType=" + test.getOtherType().toString() + "]", test.toString()); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } @@ -558,9 +558,9 @@ public void testReflectionObjectCycle() throws Exception { assertEquals( this.toBaseString(a) + "[b=" + this.toBaseString(b) + "[a=" + this.toBaseString(a) + "]]", a.toString()); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } - + /** * Test a nasty combination of arrays and Objects pointing to each other. * objects[0] -> SimpleReflectionTestFixture[ o -> objects ] @@ -586,11 +586,15 @@ public void testReflectionArrayAndObjectCycle() throws Exception { + this.toBaseString(simple) + "}]", ToStringBuilder.reflectionToString(simple)); - this.validateEmptyReflectionRegistry(); + this.validateEmptyToStringStyleRegistry(); } - void validateEmptyReflectionRegistry() { - assertTrue(ReflectionToStringBuilder.getRegistry().isEmpty()); + void validateEmptyToStringStyleRegistry() { + if (!ToStringStyle.getRegistry().isEmpty()) { + System.out.println(ToStringStyle.getRegistry()); + } + + assertTrue(ToStringStyle.getRegistry().isEmpty()); } // End: Reflection cycle tests @@ -831,6 +835,25 @@ public void testBooleanArrayArray() { assertEquals(baseStr + "[]", new ToStringBuilder(base).append((Object) array).toString()); } + public void testObjectCycle() { + ObjectCycle a = new ObjectCycle(); + ObjectCycle b = new ObjectCycle(); + a.obj = b; + b.obj = a; + + String expected = toBaseString(a) + "[" + toBaseString(b) + "[" + toBaseString(a) + "]]"; + assertEquals(expected, a.toString()); + validateEmptyToStringStyleRegistry(); + } + + static class ObjectCycle { + Object obj; + + public String toString() { + return new ToStringBuilder(this).append(obj).toString(); + } + } + public void testSimpleReflectionStatics() { SimpleReflectionStaticFieldsFixture instance1 = new SimpleReflectionStaticFieldsFixture(); assertEquals( @@ -948,25 +971,4 @@ public void testReflectionNull() { assertEquals("", ReflectionToStringBuilder.toString(null)); } - /* Unit test for #36061 - public void testObjectCycle() { - ObjectCycle a = new ObjectCycle(); - ObjectCycle b = new ObjectCycle(); - a.obj = b; - b.obj = a; - - String expected = toBaseString(a) + "[" + toBaseString(b) + "[" + toBaseString(a) + "]]"; - assertEquals(expected, a.toString()); - validateEmptyReflectionRegistry(); - } - - static class ObjectCycle { - Object obj; - - public String toString() { - return new ToStringBuilder(this).append(obj).toString(); - } - } - */ - }