Fix issue with multidimensional lists code generation (#1093)

* Add transient modifier to all generated metafields Closes #1071 * Fix issue with multidimensional lists code generation Closes #1084 * Fix test fixture
apollographql · Nov 2, 2018 · 3a7a660 · 3a7a660
1 parent ad0b35e
commit 3a7a660
Show file tree

Hide file tree

Showing 31 changed files with 835 additions and 288 deletions.
diff --git a/apollo-api/src/main/java/com/apollographql/apollo/api/ResponseWriter.java b/apollo-api/src/main/java/com/apollographql/apollo/api/ResponseWriter.java
@@ -23,22 +23,24 @@ public interface ResponseWriter {
   void writeList(@NotNull ResponseField field, @Nullable List values, @NotNull ListWriter listWriter);
 
   interface ListWriter {
-    void write(@Nullable Object value, @NotNull ListItemWriter listItemWriter);
+    void write(@Nullable List items, @NotNull ListItemWriter listItemWriter);
   }
 
   interface ListItemWriter {
-    void writeString(@Nullable Object value);
+    void writeString(@Nullable String value);
 
-    void writeInt(@Nullable Object value);
+    void writeInt(@Nullable Integer value);
 
-    void writeLong(@Nullable Object value);
+    void writeLong(@Nullable Long value);
 
-    void writeDouble(@Nullable Object value);
+    void writeDouble(@Nullable Double value);
 
-    void writeBoolean(@Nullable Object value);
+    void writeBoolean(@Nullable Boolean value);
 
     void writeCustom(@NotNull ScalarType scalarType, @Nullable Object value);
 
     void writeObject(@Nullable ResponseFieldMarshaller marshaller);
+
+    void writeList(@Nullable List items, @NotNull ListWriter listWriter);
   }
 }
diff --git a/apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/ResponseFieldSpec.kt b/apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/ResponseFieldSpec.kt
@@ -278,52 +278,77 @@ class ResponseFieldSpec(
   private fun writeListItemStatement(listItemType: TypeName, marshaller: CodeBlock): CodeBlock {
     fun writeScalar(): CodeBlock {
       val writeMethod = SCALAR_LIST_ITEM_WRITE_METHODS[listItemType] ?: "writeString"
-      return CodeBlock.builder()
-          .add(
-              if (listItemType.isEnum(context)) {
-                CodeBlock.of("\$L.\$L(((\$L) \$L).rawValue());\n", RESPONSE_LIST_ITEM_WRITER_PARAM.name, writeMethod,
-                    listItemType, OBJECT_VALUE_PARAM.name)
-              } else {
-                CodeBlock.of("\$L.\$L(\$L);\n", RESPONSE_LIST_ITEM_WRITER_PARAM.name, writeMethod,
-                    OBJECT_VALUE_PARAM.name)
-              })
-          .build()
+      return CodeBlock.builder().let {
+        if (listItemType.isEnum(context)) {
+          it.addStatement("\$L.\$L(((\$T) \$L).rawValue())", RESPONSE_LIST_ITEM_WRITER_PARAM.name, writeMethod,
+              listItemType, ITEM_VALUE_PARAM.name)
+        } else {
+          it.addStatement(
+              "\$L.\$L((\$T) \$L)", RESPONSE_LIST_ITEM_WRITER_PARAM.name, writeMethod, listItemType,
+              ITEM_VALUE_PARAM.name
+          )
+        }
+      }.build()
     }
 
     fun writeCustom(): CodeBlock {
       val customScalarEnum = CustomEnumTypeSpecBuilder.className(context)
       val customScalarEnumConst = normalizeGraphQlType(irField.type, recursive = true).toUpperCase(Locale.ENGLISH)
       return CodeBlock.builder()
           .addStatement("\$L.writeCustom(\$T.\$L, \$L)", RESPONSE_LIST_ITEM_WRITER_PARAM.name,
-              customScalarEnum, customScalarEnumConst, OBJECT_VALUE_PARAM.name)
+              customScalarEnum, customScalarEnumConst, ITEM_VALUE_PARAM.name)
           .build()
     }
 
     fun writeObject(): CodeBlock {
       return CodeBlock.builder()
-          .addStatement("\$L.writeObject(((\$L) \$L).\$L)", RESPONSE_LIST_ITEM_WRITER_PARAM.name, listItemType,
-              OBJECT_VALUE_PARAM.name, marshaller)
+          .addStatement("\$L.writeObject(((\$T) \$L).\$L)", RESPONSE_LIST_ITEM_WRITER_PARAM.name, listItemType,
+              ITEM_VALUE_PARAM.name, marshaller)
+          .build()
+    }
+
+    fun writeList(): CodeBlock {
+      val rawFieldType = listItemType.listParamType()
+      val readItemCode = writeListItemStatement(rawFieldType, marshaller)
+      val listWriterType = TypeSpec.anonymousClassBuilder("")
+          .addSuperinterface(ResponseWriter.ListWriter::class.java)
+          .addMethod(MethodSpec
+              .methodBuilder("write")
+              .addModifiers(Modifier.PUBLIC)
+              .addAnnotation(Override::class.java)
+              .addParameter(ITEMS_VALUE_PARAM)
+              .addParameter(RESPONSE_LIST_ITEM_WRITER_PARAM)
+              .addCode(readItemCode)
+              .build())
+          .build()
+      return CodeBlock.builder()
+          .addStatement("\$L.writeList((\$T) \$L, \$L)", RESPONSE_LIST_ITEM_WRITER_PARAM.name, ClassNames.LIST,
+              ITEM_VALUE_PARAM.name, listWriterType)
           .build()
     }
 
     return when {
+      listItemType.isList() -> writeList()
       irField.type.isCustomScalarType(context) -> writeCustom()
       listItemType.isScalar(context) -> writeScalar()
       else -> writeObject()
+    }.let {
+      CodeBlock.builder()
+          .beginControlFlow("for (Object \$L : \$L)", ITEM_VALUE_PARAM.name, ITEMS_VALUE_PARAM.name)
+          .add(it)
+          .endControlFlow()
+          .build()
     }
   }
 
   private fun writeListCode(writerParam: CodeBlock, fieldParam: CodeBlock, marshaller: CodeBlock): CodeBlock {
-    var listItemType = normalizedFieldSpec.type.listParamType()
-    while (listItemType.isList()) {
-      listItemType = listItemType.listParamType()
-    }
+    val listItemType = normalizedFieldSpec.type.listParamType()
     val listWriterType = TypeSpec.anonymousClassBuilder("")
         .addSuperinterface(ResponseWriter.ListWriter::class.java)
         .addMethod(MethodSpec.methodBuilder("write")
             .addModifiers(Modifier.PUBLIC)
             .addAnnotation(Override::class.java)
-            .addParameter(OBJECT_VALUE_PARAM)
+            .addParameter(ITEMS_VALUE_PARAM)
             .addParameter(RESPONSE_LIST_ITEM_WRITER_PARAM)
             .addCode(writeListItemStatement(listItemType, marshaller))
             .build()
@@ -442,7 +467,8 @@ class ResponseFieldSpec(
         ParameterSpec.builder(ResponseReader::class.java, "reader").build()
     private val RESPONSE_LIST_ITEM_READER_PARAM =
         ParameterSpec.builder(ResponseReader.ListItemReader::class.java, "listItemReader").build()
-    private val OBJECT_VALUE_PARAM = ParameterSpec.builder(Object::class.java, "value").build()
+    private val ITEMS_VALUE_PARAM = ParameterSpec.builder(List::class.java, "items").build()
+    private val ITEM_VALUE_PARAM = ParameterSpec.builder(TypeName.OBJECT, "item").build()
     private val RESPONSE_LIST_ITEM_WRITER_PARAM =
         ParameterSpec.builder(ResponseWriter.ListItemWriter::class.java, "listItemWriter").build()
 

diff --git a/apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/ir/Field.kt b/apollo-compiler/src/main/kotlin/com/apollographql/apollo/compiler/ir/Field.kt
@@ -133,15 +133,20 @@ data class Field(
       // However, we need to also encode any extra information from the `type` field
       // eg, [lists], nonNulls!, [[nestedLists]], [nonNullLists]!, etc
       val normalizedName = formatClassName()
-      if (type.startsWith("[")) {
-        // array type
-        return if (type.endsWith("!")) "[$normalizedName]!" else "[$normalizedName]"
-      } else if (type.endsWith("!")) {
-        // non-null type
-        return "$normalizedName!"
-      } else {
-        // nullable type
-        return normalizedName
+      return when {
+        type.startsWith("[") -> {// array type
+          type.count { it == '[' }.let {
+            "[".repeat(it) + normalizedName + "]".repeat(it)
+          }.let {
+            if (type.endsWith("!")) "$it!" else it
+          }
+        }
+        type.endsWith("!") -> {// non-null type
+          "$normalizedName!"
+        }
+        else -> {// nullable type
+          normalizedName
+        }
       }
     } else {
       return type

diff --git a/apollo-compiler/src/test/graphql/com/example/arguments_hardcoded/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/arguments_hardcoded/TestQuery.java
@@ -121,8 +121,10 @@ public ResponseFieldMarshaller marshaller() {
         public void marshal(ResponseWriter writer) {
           writer.writeList($responseFields[0], reviews.isPresent() ? reviews.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Review) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Review) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/custom_scalar_type/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/custom_scalar_type/TestQuery.java
@@ -273,16 +273,20 @@ public void marshal(ResponseWriter writer) {
           writer.writeCustom((ResponseField.CustomTypeField) $responseFields[2], birthDate);
           writer.writeList($responseFields[3], appearanceDates, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeCustom(CustomType.DATE, value);
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeCustom(CustomType.DATE, item);
+              }
             }
           });
           writer.writeCustom((ResponseField.CustomTypeField) $responseFields[4], fieldWithUnsupportedType);
           writer.writeCustom((ResponseField.CustomTypeField) $responseFields[5], profileLink);
           writer.writeList($responseFields[6], links, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeCustom(CustomType.URL, value);
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeCustom(CustomType.URL, item);
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/custom_scalar_type_warnings/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/custom_scalar_type_warnings/TestQuery.java
@@ -208,8 +208,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeString($responseFields[0], __typename);
           writer.writeList($responseFields[1], links, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeCustom(CustomType.URL, value);
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeCustom(CustomType.URL, item);
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/enum_type/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/enum_type/TestQuery.java
@@ -234,8 +234,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeString($responseFields[1], name);
           writer.writeList($responseFields[2], appearsIn, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeString(((com.example.enum_type.type.Episode) value).rawValue());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeString(((Episode) item).rawValue());
+              }
             }
           });
           writer.writeString($responseFields[3], firstAppearsIn.rawValue());

diff --git a/...mpiler/src/test/graphql/com/example/fragment_friends_connection/fragment/HeroDetails.java b/...mpiler/src/test/graphql/com/example/fragment_friends_connection/fragment/HeroDetails.java
@@ -204,8 +204,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/fragment_in_fragment/AllStarships.java b/apollo-compiler/src/test/graphql/com/example/fragment_in_fragment/AllStarships.java
@@ -222,8 +222,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeString($responseFields[0], __typename);
           writer.writeList($responseFields[1], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/...compiler/src/test/graphql/com/example/fragment_in_fragment/fragment/StarshipFragment.java b/...compiler/src/test/graphql/com/example/fragment_in_fragment/fragment/StarshipFragment.java
@@ -206,8 +206,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeString($responseFields[0], __typename);
           writer.writeList($responseFields[1], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/fragment_with_inline_fragment/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/fragment_with_inline_fragment/TestQuery.java
@@ -271,8 +271,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeString($responseFields[1], name);
           writer.writeList($responseFields[2], appearsIn, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeString(((com.example.fragment_with_inline_fragment.type.Episode) value).rawValue());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeString(((Episode) item).rawValue());
+              }
             }
           });
           fragments.marshaller().marshal(writer);

diff --git a/...iler/src/test/graphql/com/example/fragment_with_inline_fragment/fragment/HeroDetails.java b/...iler/src/test/graphql/com/example/fragment_with_inline_fragment/fragment/HeroDetails.java
@@ -362,8 +362,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge1) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge1) item).marshaller());
+              }
             }
           });
         }
@@ -981,8 +983,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge2) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge2) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/hero_details/HeroDetails.java b/apollo-compiler/src/test/graphql/com/example/hero_details/HeroDetails.java
@@ -347,8 +347,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/hero_details_guava/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/hero_details_guava/TestQuery.java
@@ -347,8 +347,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/hero_details_java_optional/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/hero_details_java_optional/TestQuery.java
@@ -347,8 +347,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/apollo-compiler/src/test/graphql/com/example/hero_details_nullable/TestQuery.java b/apollo-compiler/src/test/graphql/com/example/hero_details_nullable/TestQuery.java
@@ -346,8 +346,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount);
           writer.writeList($responseFields[2], edges, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }

diff --git a/...-compiler/src/test/graphql/com/example/hero_details_semantic_naming/HeroDetailsQuery.java b/...-compiler/src/test/graphql/com/example/hero_details_semantic_naming/HeroDetailsQuery.java
@@ -347,8 +347,10 @@ public void marshal(ResponseWriter writer) {
           writer.writeInt($responseFields[1], totalCount.isPresent() ? totalCount.get() : null);
           writer.writeList($responseFields[2], edges.isPresent() ? edges.get() : null, new ResponseWriter.ListWriter() {
             @Override
-            public void write(Object value, ResponseWriter.ListItemWriter listItemWriter) {
-              listItemWriter.writeObject(((Edge) value).marshaller());
+            public void write(List items, ResponseWriter.ListItemWriter listItemWriter) {
+              for (Object item : items) {
+                listItemWriter.writeObject(((Edge) item).marshaller());
+              }
             }
           });
         }