Added java.lang.Iterable support

[lyubomyr-shaydariv]

No description provided.

[lyubomyr-shaydariv]

Related tickets: #428, #672, #691, #1386.

[critterloversrv]

I'm new at this am I suppose to download or this

[critterloversrv]

I'm new at this am I suppose to download or this
Or just the ones like this should fix 672

[Marcono1234]

Maybe there should be a warning somewhere because Iterable is not guaranteed to support calling iterator() multiple times (see DirectoryStream), so it might be surprising why after serializing an object, it suddenly behaves differently.

Additionally doesn't com.google.gson.internal.bind.CollectionTypeAdapterFactory.Adapter have to be adjusted as well by this pull request?

[lyubomyr-shaydariv]

Maybe there should be a warning somewhere because Iterable is not guaranteed to support calling iterator() multiple times (see DirectoryStream), so it might be surprising why after serializing an object, it suddenly behaves differently.

I agree to some extent, but that interface has been added since 1.7 whereas Gson is still strongly 1.6-oriented, so even if that stream behaves surprisingly, let's just consider that implementation (and those like that) poorly designed violating the LSP. Why: calling the Iterable.iterator() method contract is not declared or documented to throw exceptions. I have never used that stream, but it looks like it has to be an iterator, not iterable as it violates the LSP for being "iterable", with closeable capabilities. By the way, later in Java 1.8 Stream is closeable and I find it done right. Also, even if anyone supplies such one-time-only iterables (with a special iterable type adapter on their own -- so adding the built-in won't break), they must have seen the illegal state exception and collect its result to a truly iterable collection, as the surprising behavior is described as a bold waning right in the Java 1.7 Javadoc. And finally, people, as StackOverflow tells, tend to misuse Gson in many cases and get runtime exceptions: for example, there are dozens questions of why a JavaFX object/property cannot be serialized (duplicate names on serializing) or why de/serializing an Android parcelable results in totally different behavior on different Android versions (obviosly, in both cases the ReflectiveTypeAdapterFactory works perfect only with classes they can control in full). So I wouldn't add warnings or Javadocs.

[lyubomyr-shaydariv]

Additionally doesn't com.google.gson.internal.bind.CollectionTypeAdapterFactory.Adapter have to be adjusted as well by this pull request?

I was thinking of this too, and I'm not sure but it's probably better to leave the current "collection-only" implementation as is and only mix the iterable support into it. The collection type adapter factory is declared public, and it might be used externally, not via Gson indirectly.

[Marcono1234]

Ah nevermind, I am rather certain that you definitely have to adjust CollectionTypeAdapterFactory.Adapter . It currently only works due to Java's type erasure and only for Iterables which are also instances of Collection. I am pretty sure that this code would throw ClassCastExceptions once you try to serialize Iterables which do not extend Collection (note also that this should have a separate test method).

[lyubomyr-shaydariv]

I am pretty sure that this code would throw ClassCastExceptions once you try to serialize Iterables which do not extend Collection (note also that this should have a separate test method).

Hm, a good point. I definitely missed that it and going to check it again. Thanks for pointing that out!

[lyubomyr-shaydariv]

@Marcono1234
I'll most likely close this PR due to my poor design and my wrong assumptions. Once I patched the collection type adapter factory for Iterable, I realized that now it can be serialized, but cannot be deserialized without a custom deserializer: there is no way to add elements to the iterable that is not a collection. In the force-pushed commit, there is a test that serializes Chars implements Iterable<Character>, but cannot be deserialize into a Chars instance properly causing a class cast exception since the default Iterable constructor is configured to ArrayList that obviously cannot be cast to Chars (that is effectively a generator, not a "plain" collection of elements).

I believe this is the biggest "why" why Gson does not support it out of box due to the symmetry issues and I feel ashamed to be blind not seeing it earlier.

[lyubomyr-shaydariv]

From 86a605c4b33a9fb18ff76b9101b25d65e7e7f3d1 Mon Sep 17 00:00:00 2001
From: Lyubomyr Shaydariv <[email protected]>
Date: Sun, 23 Sep 2018 12:11:24 +0300
Subject: [PATCH] Added java.lang.Iterable support

---
 gson/src/main/java/com/google/gson/Gson.java  |  1 +
 .../com/google/gson/internal/$Gson$Types.java | 16 ++++++
 .../gson/internal/ConstructorConstructor.java |  8 +++
 .../bind/CollectionTypeAdapterFactory.java    | 32 +++++++++---
 .../gson/functional/CollectionTest.java       | 49 +++++++++++++++++++
 5 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/gson/src/main/java/com/google/gson/Gson.java b/gson/src/main/java/com/google/gson/Gson.java
index 27f3ee92..30498ffc 100644
--- a/gson/src/main/java/com/google/gson/Gson.java
+++ b/gson/src/main/java/com/google/gson/Gson.java
@@ -270,6 +270,7 @@ public final class Gson {
 
     // type adapters for composite and user-defined types
     factories.add(new CollectionTypeAdapterFactory(constructorConstructor));
+    factories.add(new CollectionTypeAdapterFactory(constructorConstructor, true));
     factories.add(new MapTypeAdapterFactory(constructorConstructor, complexMapKeySerialization));
     this.jsonAdapterFactory = new JsonAdapterAnnotationTypeAdapterFactory(constructorConstructor);
     factories.add(jsonAdapterFactory);
diff --git a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java
index adea605f..5a38adfa 100644
--- a/gson/src/main/java/com/google/gson/internal/$Gson$Types.java
+++ b/gson/src/main/java/com/google/gson/internal/$Gson$Types.java
@@ -294,6 +294,22 @@ public final class $Gson$Types {
         : ((Class<?>) array).getComponentType();
   }
 
+  /**
+   * Returns the element type of this iterable type.
+   * @throws IllegalArgumentException if this type is not an iterable.
+   */
+  public static Type getIterableElementType(Type context, Class<?> contextRawType) {
+    Type iterableType = getSupertype(context, contextRawType, Iterable.class);
+
+    if (iterableType instanceof WildcardType) {
+      iterableType = ((WildcardType)iterableType).getUpperBounds()[0];
+    }
+    if (iterableType instanceof ParameterizedType) {
+      return ((ParameterizedType) iterableType).getActualTypeArguments()[0];
+    }
+    return Object.class;
+  }
+
   /**
    * Returns the element type of this collection type.
    * @throws IllegalArgumentException if this type is not a collection.
diff --git a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
index 5fab4601..8d3aedd6 100644
--- a/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
+++ b/gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java
@@ -177,6 +177,14 @@ public final class ConstructorConstructor {
       }
     }
 
+    if (Iterable.class.isAssignableFrom(rawType)) {
+      return new ObjectConstructor<T>() {
+        @Override public T construct() {
+          return (T) new ArrayList<Object>();
+        }
+      };
+    }
+
     if (Map.class.isAssignableFrom(rawType)) {
       if (ConcurrentNavigableMap.class.isAssignableFrom(rawType)) {
         return new ObjectConstructor<T>() {
diff --git a/gson/src/main/java/com/google/gson/internal/bind/CollectionTypeAdapterFactory.java b/gson/src/main/java/com/google/gson/internal/bind/CollectionTypeAdapterFactory.java
index 1d57844a..5d79b56d 100644
--- a/gson/src/main/java/com/google/gson/internal/bind/CollectionTypeAdapterFactory.java
+++ b/gson/src/main/java/com/google/gson/internal/bind/CollectionTypeAdapterFactory.java
@@ -35,9 +35,15 @@ import java.util.Collection;
  */
 public final class CollectionTypeAdapterFactory implements TypeAdapterFactory {
   private final ConstructorConstructor constructorConstructor;
+  private final boolean supportIterable;
 
   public CollectionTypeAdapterFactory(ConstructorConstructor constructorConstructor) {
+    this(constructorConstructor, false);
+  }
+
+  public CollectionTypeAdapterFactory(ConstructorConstructor constructorConstructor, boolean supportIterable) {
     this.constructorConstructor = constructorConstructor;
+    this.supportIterable = supportIterable;
   }
 
   @Override
@@ -45,11 +51,11 @@ public final class CollectionTypeAdapterFactory implements TypeAdapterFactory {
     Type type = typeToken.getType();
 
     Class<? super T> rawType = typeToken.getRawType();
-    if (!Collection.class.isAssignableFrom(rawType)) {
+    if (!isRawTypeSupported(rawType)) {
       return null;
     }
 
-    Type elementType = $Gson$Types.getCollectionElementType(type, rawType);
+    Type elementType = getElementType(type, rawType);
     TypeAdapter<?> elementTypeAdapter = gson.getAdapter(TypeToken.get(elementType));
     ObjectConstructor<T> constructor = constructorConstructor.get(typeToken);
 
@@ -58,7 +64,21 @@ public final class CollectionTypeAdapterFactory implements TypeAdapterFactory {
     return result;
   }
 
-  private static final class Adapter<E> extends TypeAdapter<Collection<E>> {
+  private boolean isRawTypeSupported(Class<?> rawType) {
+    if (supportIterable) {
+      return Iterable.class.isAssignableFrom(rawType);
+    }
+    return Collection.class.isAssignableFrom(rawType);
+  }
+
+  private Type getElementType(Type type, Class<?> rawType) {
+    if (supportIterable) {
+      return $Gson$Types.getIterableElementType(type, rawType);
+    }
+    return $Gson$Types.getCollectionElementType(type, rawType);
+  }
+
+  private static final class Adapter<E> extends TypeAdapter<Iterable<E>> {
     private final TypeAdapter<E> elementTypeAdapter;
     private final ObjectConstructor<? extends Collection<E>> constructor;
 
@@ -86,14 +106,14 @@ public final class CollectionTypeAdapterFactory implements TypeAdapterFactory {
       return collection;
     }
 
-    @Override public void write(JsonWriter out, Collection<E> collection) throws IOException {
-      if (collection == null) {
+    @Override public void write(JsonWriter out, Iterable<E> iterable) throws IOException {
+      if (iterable == null) {
         out.nullValue();
         return;
       }
 
       out.beginArray();
-      for (E element : collection) {
+      for (E element : iterable) {
         elementTypeAdapter.write(out, element);
       }
       out.endArray();
diff --git a/gson/src/test/java/com/google/gson/functional/CollectionTest.java b/gson/src/test/java/com/google/gson/functional/CollectionTest.java
index 8aa36e21..85bef1b2 100644
--- a/gson/src/test/java/com/google/gson/functional/CollectionTest.java
+++ b/gson/src/test/java/com/google/gson/functional/CollectionTest.java
@@ -25,6 +25,7 @@ import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.NoSuchElementException;
 import java.util.PriorityQueue;
 import java.util.Queue;
 import java.util.Set;
@@ -59,6 +60,27 @@ public class CollectionTest extends TestCase {
     gson = new Gson();
   }
 
+  public void testCollectionSubstitutesIterable() {
+    BagOfPrimitives foo = new BagOfPrimitives(1L, 2, true, "foo");
+    BagOfPrimitives bar = new BagOfPrimitives(3L, 4, false, "bar");
+    Iterable<BagOfPrimitives> before = Arrays.asList(foo, bar);
+    Type iterableType = new TypeToken<Iterable<BagOfPrimitives>>() {}.getType();
+    Type collectionType = new TypeToken<Collection<BagOfPrimitives>>() {}.getType();
+    String actualJson = gson.toJson(before, iterableType);
+    String expectedJson = gson.toJson(before, collectionType);
+    assertEquals(expectedJson, actualJson);
+    Iterable<BagOfPrimitives> after = gson.fromJson(actualJson, iterableType);
+    assertTrue(after instanceof Collection);
+    assertEquals(before, after);
+  }
+
+  public void testIterableThatIsNotACollection() {
+    Iterable<Character> charsBefore = new Chars('0', '9');
+    String json = gson.toJson(charsBefore, Chars.class);
+    assertEquals("[\"0\",\"1\",\"2\",\"3\",\"4\",\"5\",\"6\",\"7\",\"8\",\"9\"]", json);
+    Chars charsAfter = gson.fromJson(json, Chars.class);
+  }
+
   public void testTopLevelCollectionOfIntegersSerialization() {
     Collection<Integer> target = Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9);
     Type targetType = new TypeToken<Collection<Integer>>() {}.getType();
@@ -413,4 +435,31 @@ public class CollectionTest extends TestCase {
     assertEquals("hello", small.inSmall);
   }
 
+  private static final class Chars implements Iterable<Character> {
+
+    private final char from;
+    private final char to;
+
+    private Chars(char from, char to) {
+      this.from = from;
+      this.to = to;
+    }
+
+    @Override public Iterator<Character> iterator() {
+      return new Iterator<Character>() {
+        private char i = from;
+        @Override public boolean hasNext() {
+          return i <= to;
+        }
+        @Override public Character next() {
+          if ( !hasNext() ) {
+            throw new NoSuchElementException();
+          }
+          return i++;
+        }
+        @Override public void remove() { throw new UnsupportedOperationException(); }
+      };
+    }
+  }
+
 }
-- 
2.26.2

[Marcono1234]

@lyubomyr-shaydariv, actually breaking symmetry is not only a problem for your pull request, but a general one, see #1708.
I have created #1709 now which only adds (de-)serialization support for Iterable, but not for any subtypes, which is therefore not affected by the symmetry problems.

[lyubomyr-shaydariv]

@Marcono1234
Well... I dropped the PR because the symmetry cannot be guaranteed by the simple Gson core patching. I see two ways:

• (1) The Iterable problem should be both documented explaining why it is not supported out of box and why the user should prefer Collection (actually just explaining why Gson works like that now: collections can be added to), and a bidirectional type adapter factory with subtype support might be added to the gson-extras (like the polymorphic type adapter is a part of the extras package, not the core). I still think that serialization should be bi-directional and support subtypes with no exceptions.
• (2) OR maybe implement the full Iterable support by adding a new "adding" interface to conform the "adding problem". For example, Gson provides InstanceCreator to instantiate types that cannot be easily instantiated by Gson itself (like interfaces). What if add something like "IterableAdder"? This would bloat the Gson API a little, but most likely it is not worth it (i.e., why not use Collection?). I think this way does not have enough strength too, and I don't see the way of supporting "addable" iterables this way.

In short, I don't think Gson should support Iterable.

[Marcono1234]

I still think that serialization should be bi-directional and support subtypes with no exceptions.

Yes, that is why the pull request I created only supports Iterable and not any sub types. This preserves symmetry because it allows the deserialization to freely choose any Iterable sub type it wants (in this case ArrayList). I assume that should cover some use cases where support for Iterable (de-)serialization was requested (see also the example on that pull request), though probably not all.