IGNITE-10301 GridToStringBuilder is broken for classes with inheritance - Fixes #5463.
Signed-off-by: Alexey Goncharuk <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/2fca099f Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/2fca099f Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/2fca099f Branch: refs/heads/ignite-10044 Commit: 2fca099fd30a5b425dd7d5798f57bd21bdbc9be1 Parents: 371586a Author: Dmitrii Ryabov <[email protected]> Authored: Thu Nov 29 11:05:42 2018 +0300 Committer: Alexey Goncharuk <[email protected]> Committed: Thu Nov 29 11:05:42 2018 +0300 ---------------------------------------------------------------------- .../util/tostring/GridToStringBuilder.java | 101 ++++++++++++++----- .../internal/util/tostring/SBLimitedLength.java | 1 - .../tostring/GridToStringBuilderSelfTest.java | 82 +++++++++++++++ 3 files changed, 160 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/2fca099f/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/GridToStringBuilder.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/GridToStringBuilder.java b/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/GridToStringBuilder.java index 5394e3b..9313733 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/GridToStringBuilder.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/GridToStringBuilder.java @@ -116,13 +116,21 @@ public class GridToStringBuilder { * have to keep a map of this objects pointed to the position of previous occurrence * and remove/add them in each {@code toString()} apply. */ - private static ThreadLocal<IdentityHashMap<Object, Integer>> savedObjects = new ThreadLocal<IdentityHashMap<Object, Integer>>() { - @Override protected IdentityHashMap<Object, Integer> initialValue() { + private static ThreadLocal<IdentityHashMap<Object, EntryReference>> savedObjects = new ThreadLocal<IdentityHashMap<Object, EntryReference>>() { + @Override protected IdentityHashMap<Object, EntryReference> initialValue() { return new IdentityHashMap<>(); } }; /** + * @param obj Object. + * @return Hexed identity hashcode. + */ + public static String identity(Object obj) { + return '@' + Integer.toHexString(System.identityHashCode(obj)); + } + + /** * Produces auto-generated output of string presentation for given object and its declaration class. * * @param <T> Type of the object. @@ -822,12 +830,12 @@ public class GridToStringBuilder { return; } - IdentityHashMap<Object, Integer> svdObjs = savedObjects.get(); + IdentityHashMap<Object, EntryReference> svdObjs = savedObjects.get(); - if (handleRecursion(buf, val, svdObjs)) + if (handleRecursion(buf, val, cls, svdObjs)) return; - svdObjs.put(val, buf.length()); + svdObjs.put(val, new EntryReference(buf.length())); try { if (cls.isArray()) @@ -974,19 +982,22 @@ public class GridToStringBuilder { boolean newStr = buf.length() == 0; - IdentityHashMap<Object, Integer> svdObjs = savedObjects.get(); + IdentityHashMap<Object, EntryReference> svdObjs = savedObjects.get(); if (newStr) - svdObjs.put(obj, buf.length()); + svdObjs.put(obj, new EntryReference(buf.length())); try { + int len = buf.length(); + String s = toStringImpl0(cls, buf, obj, addNames, addVals, addSens, addLen); if (newStr) return s; - // Called from another GTSB.toString(), so this string is already in the buffer and shouldn't be returned. - return ""; + buf.setLength(len); + + return s.substring(len); } finally { if (newStr) @@ -1021,7 +1032,17 @@ public class GridToStringBuilder { assert cd != null; - buf.a(cd.getSimpleClassName()).a(" ["); + buf.a(cd.getSimpleClassName()); + + EntryReference ref = savedObjects.get().get(obj); + + if (ref != null && ref.hashNeeded) { + buf.a(identity(obj)); + + ref.hashNeeded = false; + } + + buf.a(" ["); boolean first = true; @@ -1776,24 +1797,37 @@ public class GridToStringBuilder { * * @param buf String builder buffer. * @param obj Object. + * @param cls Class. * @param svdObjs Map with saved objects to handle recursion. * @return {@code True} if object is already saved and name@hash was added to buffer. * {@code False} if it wasn't saved previously and it should be saved. */ - private static boolean handleRecursion(SBLimitedLength buf, Object obj, IdentityHashMap<Object, Integer> svdObjs) { - Integer pos = svdObjs.get(obj); + private static boolean handleRecursion( + SBLimitedLength buf, + Object obj, + @NotNull Class cls, + IdentityHashMap<Object, EntryReference> svdObjs + ) { + EntryReference ref = svdObjs.get(obj); - if (pos == null) + if (ref == null) return false; - String name = obj.getClass().getSimpleName(); - String hash = '@' + Integer.toHexString(System.identityHashCode(obj)); + int pos = ref.pos; + + String name = cls.getSimpleName(); + String hash = identity(obj); String savedName = name + hash; + String charsAtPos = buf.impl().substring(pos, pos + savedName.length()); - if (!buf.isOverflowed() && buf.impl().indexOf(savedName, pos) != pos) { - buf.i(pos + name.length(), hash); + if (!buf.isOverflowed() && !savedName.equals(charsAtPos)) { + if (charsAtPos.startsWith(cls.getSimpleName())) { + buf.i(pos + name.length(), hash); - incValues(svdObjs, obj, hash.length()); + incValues(svdObjs, obj, hash.length()); + } + else + ref.hashNeeded = true; } buf.a(savedName); @@ -1808,14 +1842,35 @@ public class GridToStringBuilder { * @param obj Object. * @param hashLen Length of the object's hash. */ - private static void incValues(IdentityHashMap<Object, Integer> svdObjs, Object obj, int hashLen) { - Integer baseline = svdObjs.get(obj); + private static void incValues(IdentityHashMap<Object, EntryReference> svdObjs, Object obj, int hashLen) { + int baseline = svdObjs.get(obj).pos; + + for (IdentityHashMap.Entry<Object, EntryReference> entry : svdObjs.entrySet()) { + EntryReference ref = entry.getValue(); - for (IdentityHashMap.Entry<Object, Integer> entry : svdObjs.entrySet()) { - Integer pos = entry.getValue(); + int pos = ref.pos; if (pos > baseline) - entry.setValue(pos + hashLen); + ref.pos = pos + hashLen; + } + } + + /** + * + */ + private static class EntryReference { + /** Position. */ + int pos; + + /** First object entry needs hash to be written. */ + boolean hashNeeded; + + /** + * @param pos Position. + */ + private EntryReference(int pos) { + this.pos = pos; + hashNeeded = false; } } } http://git-wip-us.apache.org/repos/asf/ignite/blob/2fca099f/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/SBLimitedLength.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/SBLimitedLength.java b/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/SBLimitedLength.java index b524b3d..640555a 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/SBLimitedLength.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/util/tostring/SBLimitedLength.java @@ -62,7 +62,6 @@ public class SBLimitedLength extends GridStringBuilder { tail.reset(); } - /** * @return tail string builder. */ http://git-wip-us.apache.org/repos/asf/ignite/blob/2fca099f/modules/core/src/test/java/org/apache/ignite/internal/util/tostring/GridToStringBuilderSelfTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/util/tostring/GridToStringBuilderSelfTest.java b/modules/core/src/test/java/org/apache/ignite/internal/util/tostring/GridToStringBuilderSelfTest.java index d249914..bf33f4a 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/util/tostring/GridToStringBuilderSelfTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/util/tostring/GridToStringBuilderSelfTest.java @@ -40,6 +40,7 @@ import org.apache.ignite.testframework.junits.common.GridCommonTest; import static org.apache.ignite.IgniteSystemProperties.IGNITE_TO_STRING_COLLECTION_LIMIT; import static org.apache.ignite.IgniteSystemProperties.IGNITE_TO_STRING_MAX_LENGTH; +import static org.apache.ignite.internal.util.tostring.GridToStringBuilder.identity; /** * Tests for {@link GridToStringBuilder}. @@ -517,6 +518,39 @@ public class GridToStringBuilderSelfTest extends GridCommonAbstractTest { } /** + * + */ + public void testHierarchy() { + Wrapper w = new Wrapper(); + Parent p = w.p; + String hash = identity(p); + + checkHierarchy("Wrapper [p=Child [b=0, pb=Parent[] [null], super=Parent [a=0, pa=Parent[] [null]]]]", w); + + p.pa[0] = p; + + checkHierarchy("Wrapper [p=Child" + hash + + " [b=0, pb=Parent[] [null], super=Parent [a=0, pa=Parent[] [Child" + hash + "]]]]", w); + + ((Child)p).pb[0] = p; + + checkHierarchy("Wrapper [p=Child" + hash + " [b=0, pb=Parent[] [Child" + hash + + "], super=Parent [a=0, pa=Parent[] [Child" + hash + "]]]]", w); + } + + /** + * @param exp Expected. + * @param w Wrapper. + */ + private void checkHierarchy(String exp, Wrapper w) { + String wS = w.toString(); + + info(wS); + + assertEquals(exp, wS); + } + + /** * Test class. */ private static class TestClass1 { @@ -651,4 +685,52 @@ public class GridToStringBuilderSelfTest extends GridCommonAbstractTest { this.str = str; } } + + /** + * + */ + private static class Parent { + /** */ + private int a; + + /** */ + @GridToStringInclude + private Parent pa[] = new Parent[1]; + + /** {@inheritDoc} */ + @Override public String toString() { + return S.toString(Parent.class, this); + } + } + + /** + * + */ + private static class Child extends Parent { + /** */ + private int b; + + /** */ + @GridToStringInclude + private Parent pb[] = new Parent[1]; + + /** {@inheritDoc} */ + @Override public String toString() { + return S.toString(Child.class, this, super.toString()); + } + } + + /** + * + */ + private static class Wrapper { + /** */ + @GridToStringInclude + Parent p = new Child(); + + /** {@inheritDoc} */ + @Override public String toString() { + return S.toString(Wrapper.class, this); + } + } }
