From e6ade31ee150bd87b9d2aa746c8e376ad9890fa0 Mon Sep 17 00:00:00 2001 From: Will Nicholson Date: Tue, 20 Oct 2020 14:27:16 +0100 Subject: [PATCH 1/2] Stop using references to collections that won't be serialized. Types with collections are a bit hairy, what if it's first read as Set but later we serialize the same thing as SortedSet? Can't reuse the original value but instead need a copy constructor, and that's a lot of effort to code --- .../joda/beans/ser/bin/BeanReferences.java | 21 +++++++++++++------ .../org/joda/beans/ser/SerTestHelper.java | 13 ++++++++++++ .../ser/bin/TestSerializeReferencingBin.java | 13 ++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/joda/beans/ser/bin/BeanReferences.java b/src/main/java/org/joda/beans/ser/bin/BeanReferences.java index 339a178b..701a1fa1 100644 --- a/src/main/java/org/joda/beans/ser/bin/BeanReferences.java +++ b/src/main/java/org/joda/beans/ser/bin/BeanReferences.java @@ -132,13 +132,13 @@ private void findReferencesBean( return; } - // has this object been seen before, if so no need to check it again - int result = objects.compute(base, BeanReferences::incrementOrOne); - if (result > 1) { - return; - } - if (base instanceof Bean) { + // has this object been seen before, if so no need to check it again + int result = objects.compute(base, BeanReferences::incrementOrOne); + if (result > 1) { + return; + } + addClassInfo(base, declaredClass); Bean bean = (Bean) base; if (settings.getConverter().isConvertible(bean.getClass())) { @@ -166,11 +166,20 @@ private void findReferencesBean( } else if (parentIterator != null) { SerIterator childIterator = settings.getIteratorFactory().createChild(base, parentIterator); if (childIterator != null) { + // shouldn't try and reuse references to collections findReferencesIterable(childIterator, objects); } else { + int result = objects.compute(base, BeanReferences::incrementOrOne); + if (result > 1) { + return; + } addClassInfo(base, declaredClass); } } else { + int result = objects.compute(base, BeanReferences::incrementOrOne); + if (result > 1) { + return; + } addClassInfo(base, declaredClass); } } diff --git a/src/test/java/org/joda/beans/ser/SerTestHelper.java b/src/test/java/org/joda/beans/ser/SerTestHelper.java index f9429515..796fbf87 100644 --- a/src/test/java/org/joda/beans/ser/SerTestHelper.java +++ b/src/test/java/org/joda/beans/ser/SerTestHelper.java @@ -19,10 +19,12 @@ import java.util.Currency; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.TimeZone; +import java.util.TreeSet; import org.joda.beans.sample.Address; import org.joda.beans.sample.Company; @@ -228,6 +230,17 @@ public static ImmGenericCollections testGenericInterfaces( .build(); } + public static ImmGenericCollections testGenericInterfacesCollections() { + return ImmGenericCollections.builder() + .map(ImmutableMap.of( + "First", Arrays.asList("A", "B"), + "First1", ImmutableList.of("A", "B"), + "Third1", new TreeSet<>(ImmutableList.of("A", "B")), + "Third", new HashSet<>(Arrays.asList("A", "B")), + "Second", testCollections())) + .build(); + } + public static ImmKeyList testIntermediateInterfaces() { // second serialized as JodaConvertInterface, non-bean // third and fourth are serialized as an intermediate Joda-Convert interface INamedKey diff --git a/src/test/java/org/joda/beans/ser/bin/TestSerializeReferencingBin.java b/src/test/java/org/joda/beans/ser/bin/TestSerializeReferencingBin.java index 928c5619..196776f0 100644 --- a/src/test/java/org/joda/beans/ser/bin/TestSerializeReferencingBin.java +++ b/src/test/java/org/joda/beans/ser/bin/TestSerializeReferencingBin.java @@ -87,6 +87,19 @@ public void test_writeJodaConvertInterface() { BeanAssert.assertBeanEquals(bean, array); } + @Test + public void test_writeJodaConvertInterfaceCollections() { + ImmGenericCollections array = SerTestHelper.testGenericInterfacesCollections(); + + byte[] bytes = JodaBeanSer.COMPACT.binWriterReferencing().write(array); + //System.out.println(JodaBeanBinReader.visualize(bytes)); + + ImmGenericCollections bean = + (ImmGenericCollections) JodaBeanSer.COMPACT.binReader().read(bytes); + //System.out.println(bean); + BeanAssert.assertBeanEquals(bean, array); + } + @Test public void test_writeIntermediateInterface() { ImmKeyList array = SerTestHelper.testIntermediateInterfaces(); From 6b34aa7491e9b53f075a2d7551337060e32042b6 Mon Sep 17 00:00:00 2001 From: Will Nicholson Date: Tue, 20 Oct 2020 14:41:09 +0100 Subject: [PATCH 2/2] Add child iterator --- src/main/java/org/joda/beans/ser/bin/BeanReferences.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/joda/beans/ser/bin/BeanReferences.java b/src/main/java/org/joda/beans/ser/bin/BeanReferences.java index 701a1fa1..33e40118 100644 --- a/src/main/java/org/joda/beans/ser/bin/BeanReferences.java +++ b/src/main/java/org/joda/beans/ser/bin/BeanReferences.java @@ -166,6 +166,9 @@ private void findReferencesBean( } else if (parentIterator != null) { SerIterator childIterator = settings.getIteratorFactory().createChild(base, parentIterator); if (childIterator != null) { + if (childIterator.metaTypeRequired()) { + objects.compute(childIterator.metaTypeName(), BeanReferences::incrementOrOne); + } // shouldn't try and reuse references to collections findReferencesIterable(childIterator, objects); } else {