From d29c62c0460de1e1c5a342025032cbb1ddcd8c13 Mon Sep 17 00:00:00 2001
From: joehni <joerg.schaible@gmx.de>
Date: Fri, 5 Jan 2018 13:20:27 +0100
Subject: [PATCH] Provide methods in AbstractCollectionConverter that read and
 write in a balanced way from and to the hierarchical stream.

---
 xstream-distribution/src/content/changes.html | 15 +++-
 .../AbstractCollectionConverter.java          | 89 +++++++++++++++++--
 .../collections/ArrayConverter.java           |  8 +-
 .../collections/CollectionConverter.java      |  7 +-
 .../converters/collections/MapConverter.java  | 15 ++--
 .../SingletonCollectionConverter.java         |  6 +-
 .../collections/SingletonMapConverter.java    | 11 +--
 .../collections/TreeMapConverter.java         |  4 +-
 .../collections/TreeSetConverter.java         |  5 +-
 .../extended/NamedCollectionConverter.java    | 17 +++-
 .../converters/extended/SubjectConverter.java |  8 +-
 11 files changed, 137 insertions(+), 48 deletions(-)
diff --git a/xstream-distribution/src/content/changes.html b/xstream-distribution/src/content/changes.html
index 2507c1aea..bb10f0110 100644
--- a/xstream-distribution/src/content/changes.html
+++ b/xstream-distribution/src/content/changes.html
@@ -1,7 +1,7 @@
 <html>
 <!--
  Copyright (C) 2005, 2006 Joe Walnes.
- Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 XStream committers.
+ Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018 XStream committers.
  All rights reserved.
  
  The software in this package is published under the terms of the BSD
@@ -73,6 +73,8 @@ <h2>Minor changes</h2>
 		implementations for EncodedByteArrayConverter. Prefer Base64 codec implementations of the Java runtime over
 		XStream's own one.</li>
 		<li>GHI:#97: Support to run out of the box in a Java 1.4 runtime is established again.</li>
+		<li>Provide methods in AbstractCollectionConverter that read and write in a balanced way from and to the
+		hierarchical stream.</li>
 		<li>Detect Java 10.</li>
 	</ul>
 
@@ -87,7 +89,18 @@ <h2>Stream compatibility</h2>
 	<h2>API changes</h2>
 
 	<ul>
+		<li>Added c.t.x.converters.collection.AbstractCollectionConverter.readBareItem(HierarchicalStreamReader, UnmarshallingContext, Object).</li>
+		<li>Added c.t.x.converters.collection.AbstractCollectionConverter.readCompleteItem(HierarchicalStreamReader, UnarshallingContext, Object).</li>
+		<li>Deprecated c.t.x.converters.collection.AbstractCollectionConverter.readItem(HierarchicalStreamReader, UnmarshallingContext, Object).</li>
+		<li>Added c.t.x.converters.collection.AbstractCollectionConverter.writeBareItem(Object, MarshallingContext, HierarchicalStreamWriter).</li>
+		<li>Added c.t.x.converters.collection.AbstractCollectionConverter.writeCompleteItem(Object, MarshallingContext, HierarchicalStreamWriter).</li>
+		<li>Deprecated c.t.x.converters.collection.AbstractCollectionConverter.writeItem(Object, MarshallingContext, HierarchicalStreamWriter).</li>
+		<li>Added c.t.x.converters.collection.AbstractCollectionConverter.writeNullItem(MarshallingContext, HierarchicalStreamWriter).</li>
 		<li>Added c.t.x.converters.extended.EncodedByteArrayConverter(StingCodec).</li>
+		<li>Added c.t.x.converters.extended.NamedCollectionConverter.readBareItem(HierarchicalStreamReader, UnmarshallingContext, Object).</li>
+		<li>Deprecated c.t.x.converters.extended.NamedCollectionConverter.readItem(HierarchicalStreamReader, UnmarshallingContext, Object).</li>
+		<li>Added c.t.x.converters.extended.NamedCollectionConverter.writeCompleteItem(Object, MarshallingContext, HierarchicalStreamWriter).</li>
+		<li>Deprecated c.t.x.converters.extended.NamedCollectionConverter.writeItem(Object, MarshallingContext, HierarchicalStreamWriter).</li>
 		<li>Added c.t.x.core.DefaultConverterLookup(Map).</li>
 		<li>Added c.t.x.core.util.JVM.getBase64Codec().</li>
 		<li>Added c.t.x.core.util.JVM.isJava10().</li>
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/AbstractCollectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/AbstractCollectionConverter.java
index e487478a3..399879ef0 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/AbstractCollectionConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/AbstractCollectionConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2003, 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2008, 2009, 2013, 2014, 2016 XStream Committers.
+ * Copyright (C) 2006, 2007, 2008, 2009, 2013, 2014, 2016, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -54,28 +54,107 @@ protected Mapper mapper() {
     @Override
     public abstract Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context);
 
+    /**
+     * @deprecated As of upcoming use {@link #writeCompleteItem(Object, MarshallingContext, HierarchicalStreamWriter)}
+     *             instead.
+     */
+    @Deprecated
     protected void writeItem(final Object item, final MarshallingContext context,
             final HierarchicalStreamWriter writer) {
         // PUBLISHED API METHOD! If changing signature, ensure backwards compatibility.
         if (item == null) {
             // todo: this is duplicated in TreeMarshaller.start()
-            final String name = mapper().serializedClass(null);
-            ExtendedHierarchicalStreamWriterHelper.startNode(writer, name, Mapper.Null.class);
-            writer.endNode();
+            writeNullItem(context, writer);
         } else {
             final String name = mapper().serializedClass(item.getClass());
             ExtendedHierarchicalStreamWriterHelper.startNode(writer, name, item.getClass());
-            context.convertAnother(item);
+            writeBareItem(item, context, writer);
             writer.endNode();
         }
     }
 
+    /**
+     * Write an item of the collection into the writer including surrounding tags.
+     *
+     * @param item the item to write
+     * @param context the current marshalling context
+     * @param writer the target writer
+     * @since upcoming
+     */
+    protected void writeCompleteItem(final Object item, final MarshallingContext context,
+            final HierarchicalStreamWriter writer) {
+        writeItem(item, context, writer);
+    }
+
+    /**
+     * Write the bare item of the collection into the writer.
+     *
+     * @param item the item to write
+     * @param context the current marshalling context
+     * @param writer the target writer
+     * @since upcoming
+     */
+    protected void writeBareItem(final Object item, final MarshallingContext context,
+            final HierarchicalStreamWriter writer) {
+        context.convertAnother(item);
+    }
+
+    /**
+     * Write a null item of the collection into the writer. The method readItem should be able to process the written
+     * data i.e. it has to write the tags or may not write anything at all.
+     *
+     * @param context the current marshalling context
+     * @param writer the target writer
+     * @since upcoming
+     */
+    protected void writeNullItem(final MarshallingContext context, final HierarchicalStreamWriter writer) {
+        final String name = mapper().serializedClass(null);
+        ExtendedHierarchicalStreamWriterHelper.startNode(writer, name, Mapper.Null.class);
+        writer.endNode();
+    }
+
+    /**
+     * @deprecated As of upcoming use {@link #readBareItem(HierarchicalStreamReader, UnmarshallingContext, Object)} or
+     *             {@link #readCompleteItem(HierarchicalStreamReader, UnmarshallingContext, Object)} instead.
+     */
+    @Deprecated
     protected Object readItem(final HierarchicalStreamReader reader, final UnmarshallingContext context,
             final Object current) {
+        return readBareItem(reader, context, current);
+    }
+
+    /**
+     * Read a bare item of the collection from the reader.
+     *
+     * @param reader the source reader
+     * @param context the unmarshalling context
+     * @param current the target collection (if already available)
+     * @return the read item
+     * @since upcoming
+     */
+    protected Object readBareItem(final HierarchicalStreamReader reader, final UnmarshallingContext context,
+            final Object current) {
         final Class<?> type = HierarchicalStreams.readClassType(reader, mapper());
         return context.convertAnother(current, type);
     }
 
+    /**
+     * Read an item of the collection including the tags from the reader.
+     *
+     * @param reader the source reader
+     * @param context the unmarshalling context
+     * @param current the target collection (if already available)
+     * @return the read item
+     * @since upcoming
+     */
+    protected Object readCompleteItem(final HierarchicalStreamReader reader, final UnmarshallingContext context,
+            final Object current) {
+        reader.moveDown();
+        final Object result = readItem(reader, context, current);
+        reader.moveUp();
+        return result;
+    }
+
     protected Object createCollection(final Class<?> type) {
         ErrorWritingException ex = null;
         final Class<?> defaultType = mapper().defaultImplementationOf(type);
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/ArrayConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/ArrayConverter.java
index 202e97338..42cfb91e9 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/ArrayConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/ArrayConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2003, 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2014, 2015 XStream Committers.
+ * Copyright (C) 2006, 2007, 2014, 2015, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -44,7 +44,7 @@ public void marshal(final Object source, final HierarchicalStreamWriter writer,
         final int length = Array.getLength(source);
         for (int i = 0; i < length; i++) {
             final Object item = Array.get(source, i);
-            writeItem(item, context, writer);
+            writeCompleteItem(item, context, writer);
         }
 
     }
@@ -54,10 +54,8 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli
         // read the items from xml into a list (the array size is not known until all items have been read)
         final List<Object> items = new ArrayList<>();
         while (reader.hasMoreChildren()) {
-            reader.moveDown();
-            final Object item = readItem(reader, context, null); // TODO: arg, what should replace null?
+            final Object item = readCompleteItem(reader, context, null); // TODO: arg, what should replace null?
             items.add(item);
-            reader.moveUp();
         }
         // now convertAnother the list into an array
         final Object array = Array.newInstance(context.getRequiredType().getComponentType(), items.size());
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java
index 662bb8838..bd120b905 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2003, 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014 XStream Committers.
+ * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -75,7 +75,7 @@ public boolean canConvert(final Class<?> type) {
     public void marshal(final Object source, final HierarchicalStreamWriter writer, final MarshallingContext context) {
         final Collection<?> collection = (Collection<?>)source;
         for (final Object item : collection) {
-            writeItem(item, context, writer);
+            writeCompleteItem(item, context, writer);
         }
     }
 
@@ -103,7 +103,8 @@ protected void populateCollection(final HierarchicalStreamReader reader, final U
 
     protected void addCurrentElementToCollection(final HierarchicalStreamReader reader,
             final UnmarshallingContext context, final Collection<?> collection, final Collection<?> target) {
-        final Object item = readItem(reader, context, collection);
+        @SuppressWarnings("deprecation")
+        final Object item = readItem(reader, context, collection); // call readBareItem when deprecated method is removed
         @SuppressWarnings("unchecked")
         final Collection<Object> targetCollection = (Collection<Object>)target;
         targetCollection.add(item);
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java
index e21719d69..adc75db9a 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2003, 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2008, 2010, 2011, 2012, 2013, 2014 XStream Committers.
+ * Copyright (C) 2006, 2007, 2008, 2010, 2011, 2012, 2013, 2014, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -84,8 +84,8 @@ public void marshal(final Object source, final HierarchicalStreamWriter writer,
         for (final Map.Entry<?, ?> entry : map.entrySet()) {
             ExtendedHierarchicalStreamWriterHelper.startNode(writer, entryName, entry.getClass());
 
-            writeItem(entry.getKey(), context, writer);
-            writeItem(entry.getValue(), context, writer);
+            writeCompleteItem(entry.getKey(), context, writer);
+            writeCompleteItem(entry.getValue(), context, writer);
 
             writer.endNode();
         }
@@ -115,13 +115,8 @@ protected void populateMap(final HierarchicalStreamReader reader, final Unmarsha
 
     protected void putCurrentEntryIntoMap(final HierarchicalStreamReader reader, final UnmarshallingContext context,
             final Map<?, ?> map, final Map<?, ?> target) {
-        reader.moveDown();
-        final Object key = readItem(reader, context, map);
-        reader.moveUp();
-
-        reader.moveDown();
-        final Object value = readItem(reader, context, map);
-        reader.moveUp();
+        final Object key = readCompleteItem(reader, context, map);
+        final Object value = readCompleteItem(reader, context, map);
 
         @SuppressWarnings("unchecked")
         final Map<Object, Object> targetMap = (Map<Object, Object>)target;
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonCollectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonCollectionConverter.java
index 9f99460e5..e53388360 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonCollectionConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonCollectionConverter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011, 2014 XStream Committers.
+ * Copyright (C) 2011, 2014, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -49,9 +49,7 @@ public boolean canConvert(final Class<?> type) {
 
     @Override
     public Collection<?> unmarshal(final HierarchicalStreamReader reader, final UnmarshallingContext context) {
-        reader.moveDown();
-        final Object item = readItem(reader, context, null);
-        reader.moveUp();
+        final Object item = readCompleteItem(reader, context, null);
         return context.getRequiredType() == LIST ? Collections.singletonList(item) : Collections.singleton(item);
     }
 }
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonMapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonMapConverter.java
index f0e216de0..7e6b611ae 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonMapConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/SingletonMapConverter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011, 2014 XStream Committers.
+ * Copyright (C) 2011, 2014, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -53,13 +53,8 @@ public boolean canConvert(final Class<?> type) {
     @Override
     public Map<?, ?> unmarshal(final HierarchicalStreamReader reader, final UnmarshallingContext context) {
         reader.moveDown();
-        reader.moveDown();
-        final Object key = readItem(reader, context, null);
-        reader.moveUp();
-
-        reader.moveDown();
-        final Object value = readItem(reader, context, null);
-        reader.moveUp();
+        final Object key = readCompleteItem(reader, context, null);
+        final Object value = readCompleteItem(reader, context, null);
         reader.moveUp();
 
         return Collections.singletonMap(key, value);
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java
index 401be3336..02b7140a8 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014, 2015, 2016 XStream Committers.
+ * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014, 2015, 2016, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -79,7 +79,7 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli
         @SuppressWarnings("unchecked")
         final Comparator<Object> comparator = (Comparator<Object>)unmarshalComparator(reader, context, result);
         if (result == null) {
-            result = comparator == null ? new TreeMap<>() : new TreeMap<>(comparator);
+            result = comparator == null || comparator == NULL_MARKER ? new TreeMap<>() : new TreeMap<>(comparator);
         }
         populateTreeMap(reader, context, result, comparator);
         return result;
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java
index 2e1d38b4a..02c497281 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2004, 2005 Joe Walnes.
- * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014, 2015, 2016 XStream Committers.
+ * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2014, 2015, 2016, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -164,7 +164,8 @@ public int size() {
             @Override
             protected void putCurrentEntryIntoMap(final HierarchicalStreamReader reader,
                     final UnmarshallingContext context, final Map<?, ?> map, final Map<?, ?> target) {
-                final Object key = readItem(reader, context, map);
+                @SuppressWarnings("deprecation")
+                final Object key = readItem(reader, context, map);  // call readBareItem when deprecated method is removed
                 @SuppressWarnings("unchecked")
                 final Map<Object, Object> checkedTarget = (Map<Object, Object>)target;
                 checkedTarget.put(key, key);
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedCollectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedCollectionConverter.java
index 0f2660169..d73a57bfe 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedCollectionConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedCollectionConverter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013, 2014 XStream Committers.
+ * Copyright (C) 2013, 2014, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -66,7 +66,18 @@ public NamedCollectionConverter(
     }
 
     @Override
-    protected void writeItem(final Object item, final MarshallingContext context, final HierarchicalStreamWriter writer) {
+    protected void writeCompleteItem(final Object item, final MarshallingContext context,
+            final HierarchicalStreamWriter writer) {
+        writeItem(item, context, writer);
+    }
+
+    /**
+     * @deprecated As of upcoming use {@link #writeCompleteItem(Object, MarshallingContext, HierarchicalStreamWriter)}
+     *             instead.
+     */
+    @Deprecated
+    @Override
+    protected void writeItem(Object item, MarshallingContext context, HierarchicalStreamWriter writer) {
         final Class<?> itemType = item == null ? Mapper.Null.class : item.getClass();
         ExtendedHierarchicalStreamWriterHelper.startNode(writer, name, itemType);
         if (!itemType.equals(type)) {
@@ -82,7 +93,7 @@ protected void writeItem(final Object item, final MarshallingContext context, fi
     }
 
     @Override
-    protected Object readItem(final HierarchicalStreamReader reader, final UnmarshallingContext context,
+    protected Object readBareItem(final HierarchicalStreamReader reader, final UnmarshallingContext context,
             final Object current) {
         final String className = HierarchicalStreams.readClassAttribute(reader, mapper());
         final Class<?> itemType = className == null ? type : mapper().realClass(className);
diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/extended/SubjectConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/extended/SubjectConverter.java
index 1e4749be8..e20c79675 100644
--- a/xstream/src/java/com/thoughtworks/xstream/converters/extended/SubjectConverter.java
+++ b/xstream/src/java/com/thoughtworks/xstream/converters/extended/SubjectConverter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2006, 2007, 2014, 2015 XStream Committers.
+ * Copyright (C) 2006, 2007, 2014, 2015, 2018 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -60,7 +60,7 @@ protected void marshalPrincipals(final Set<Principal> principals, final Hierarch
             final MarshallingContext context) {
         writer.startNode("principals");
         for (final Principal principal : principals) {
-            writeItem(principal, context, writer);
+            writeCompleteItem(principal, context, writer);
         }
         writer.endNode();
     };
@@ -114,9 +114,7 @@ protected Set<Principal> populateSet(final HierarchicalStreamReader reader, fina
         final Set<Principal> set = new HashSet<>();
         reader.moveDown();
         while (reader.hasMoreChildren()) {
-            reader.moveDown();
-            final Principal principal = (Principal)readItem(reader, context, set);
-            reader.moveUp();
+            final Principal principal = (Principal)readCompleteItem(reader, context, set);
             set.add(principal);
         }
         reader.moveUp();
