fix: support classes that extend or implement an Iterable (#2397)

* Fix extended iterables in the parser * Add test and reformat * Accept unknown return types * Fix formatting --------- Co-authored-by: Anton Platonov <platosha@gmail.com>
AceDev24 · May 15, 2024 · ca70e63 · ca70e63
1 parent c354663
commit ca70e63
Show file tree

Hide file tree

Showing 8 changed files with 157 additions and 3 deletions.
diff --git a/packages/java/endpoint/src/main/java/com/vaadin/hilla/ExplicitNullableTypeCheckerHelper.java b/packages/java/endpoint/src/main/java/com/vaadin/hilla/ExplicitNullableTypeCheckerHelper.java
@@ -35,6 +35,7 @@
 import java.util.stream.Collectors;
 
 import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.vaadin.hilla.parser.utils.Generics;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -156,8 +157,17 @@ private String checkIterable(Iterable<?> value, Type expectedType) {
                     .getActualTypeArguments()[0];
             iterableDescription = "collection";
         } else if (expectedType instanceof Class<?>) {
-            itemType = ((Class<?>) expectedType).getComponentType();
-            iterableDescription = "array";
+            var expectedClass = ((Class<?>) expectedType);
+
+            if (Iterable.class.isAssignableFrom(expectedClass)) {
+                // Let's deal with classes extending or implementing an iterator
+                itemType = Generics.getExactIterableType(expectedClass)
+                        .orElse(Object.class);
+                iterableDescription = "collection";
+            } else {
+                itemType = expectedClass.getComponentType();
+                iterableDescription = "array";
+            }
         }
 
         for (Object item : value) {

diff --git a/packages/java/endpoint/src/test/java/com/vaadin/hilla/ExplicitNullableTypeCheckerTest.java b/packages/java/endpoint/src/test/java/com/vaadin/hilla/ExplicitNullableTypeCheckerTest.java
@@ -237,6 +237,24 @@ public void should_Recursively_Check_List_Items() {
         verify(checker).checkValueForType("bar", String.class);
     }
 
+    private static class MyList extends ArrayList<String> {
+    }
+
+    @Test
+    public void should_Recognize_Extended_List() {
+        ExplicitNullableTypeCheckerHelper checker = spy(helper);
+
+        MyList list = new MyList();
+        list.add("foo");
+        list.add("bar");
+
+        checker.checkValueForType(list, MyList.class);
+        verify(checker).checkValueForType(list, MyList.class);
+
+        verify(checker).checkValueForType("foo", String.class);
+        verify(checker).checkValueForType("bar", String.class);
+    }
+
     @Test
     public void should_ReturnNull_When_GivenNonNullItems_InListType() {
         Assert.assertNull(explicitNullableTypeChecker.checkValueForType(

diff --git a/...-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/TypeSignaturePlugin.java b/...-backbone/src/main/java/com/vaadin/hilla/parser/plugins/backbone/TypeSignaturePlugin.java
@@ -1,5 +1,6 @@
 package com.vaadin.hilla.parser.plugins.backbone;
 
+import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.AnnotatedType;
 import java.lang.reflect.Type;
 import java.util.List;
@@ -27,8 +28,9 @@
 import com.vaadin.hilla.parser.plugins.backbone.nodes.MethodParameterNode;
 import com.vaadin.hilla.parser.plugins.backbone.nodes.PropertyNode;
 import com.vaadin.hilla.parser.plugins.backbone.nodes.TypeSignatureNode;
-
 import com.vaadin.hilla.parser.plugins.backbone.nodes.TypedNode;
+import com.vaadin.hilla.parser.utils.Generics;
+
 import io.swagger.v3.oas.models.media.ArraySchema;
 import io.swagger.v3.oas.models.media.ComposedSchema;
 import io.swagger.v3.oas.models.media.Content;
@@ -177,6 +179,14 @@ private List<SignatureModel> getReferredTypes(SignatureModel signature) {
 
             if (!typeArguments.isEmpty()) {
                 items = List.of(typeArguments.get(0));
+            } else {
+                // Let's deal with classes extending or implementing an iterator
+                var cls = (Class<?>) ((ClassRefSignatureModel) signature)
+                        .getClassInfo().get();
+                items = Generics.getExactIterableType(cls)
+                        .map(type -> List
+                                .of(SignatureModel.of((AnnotatedElement) type)))
+                        .orElse(items);
             }
         } else if (signature.isOptional()) {
             var typeArguments = ((ClassRefSignatureModel) signature)

diff --git a/...one/src/test/java/com/vaadin/hilla/parser/plugins/backbone/iterable/IterableEndpoint.java b/...one/src/test/java/com/vaadin/hilla/parser/plugins/backbone/iterable/IterableEndpoint.java
@@ -25,6 +25,14 @@ public Iterable<Foo> getFooIterable() {
         return Arrays.asList(new Foo(), new Foo());
     }
 
+    public SpecializedIterable getSpecializedIterable() {
+        return new SpecializedIterable();
+    }
+
+    public SpecializedIterableCustom getSpecializedIterableCustom() {
+        return new SpecializedIterableCustom();
+    }
+
     public List<Foo> getFooList() {
         return new ArrayList<>();
     }
@@ -51,6 +59,12 @@ public Iterator<T> iterator() {
         }
     }
 
+    public static class SpecializedIterable extends ArrayList<String> {
+    }
+
+    public static class SpecializedIterableCustom extends ArrayList<Foo> {
+    }
+
     public static class Foo {
         public String bar = "bar";
     }

diff --git a/...ackbone/src/test/resources/com/vaadin/hilla/parser/plugins/backbone/iterable/openapi.json b/...ackbone/src/test/resources/com/vaadin/hilla/parser/plugins/backbone/iterable/openapi.json
@@ -180,6 +180,58 @@
           }
         }
       }
+    },
+    "/IterableEndpoint/getSpecializedIterable": {
+      "post": {
+        "tags": ["IterableEndpoint"],
+        "operationId": "IterableEndpoint_getSpecializedIterable_POST",
+        "responses": {
+          "200": {
+            "description": "",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "array",
+                  "nullable": true,
+                  "items" : {
+                    "type" : "string",
+                    "nullable" : true
+                  },
+                  "x-class-name": "com.vaadin.hilla.parser.plugins.backbone.iterable.IterableEndpoint$SpecializedIterable"
+                }
+              }
+            }
+          }
+        }
+      }
+    },
+    "/IterableEndpoint/getSpecializedIterableCustom": {
+      "post": {
+        "tags": ["IterableEndpoint"],
+        "operationId": "IterableEndpoint_getSpecializedIterableCustom_POST",
+        "responses": {
+          "200": {
+            "description": "",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "type": "array",
+                  "nullable": true,
+                  "items": {
+                    "nullable": true,
+                    "anyOf": [
+                      {
+                        "$ref": "#/components/schemas/com.vaadin.hilla.parser.plugins.backbone.iterable.IterableEndpoint$Foo"
+                      }
+                    ]
+                  },
+                  "x-class-name": "com.vaadin.hilla.parser.plugins.backbone.iterable.IterableEndpoint$SpecializedIterableCustom"
+                }
+              }
+            }
+          }
+        }
+      }
     }
   },
   "components": {

diff --git a/packages/java/parser-jvm-utils/pom.xml b/packages/java/parser-jvm-utils/pom.xml
@@ -68,5 +68,11 @@
             <groupId>com.fasterxml.jackson.datatype</groupId>
             <artifactId>jackson-datatype-jdk8</artifactId>
         </dependency>
+        <!-- Small reflection library -->
+        <dependency>
+            <groupId>com.vaadin.external</groupId>
+            <artifactId>gentyref</artifactId>
+            <version>1.2.0.vaadin1</version>
+        </dependency>
     </dependencies>
 </project>
diff --git a/packages/java/parser-jvm-utils/src/main/java/com/vaadin/hilla/parser/utils/Generics.java b/packages/java/parser-jvm-utils/src/main/java/com/vaadin/hilla/parser/utils/Generics.java
@@ -0,0 +1,43 @@
+package com.vaadin.hilla.parser.utils;
+
+import com.googlecode.gentyref.GenericTypeReflector;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Optional;
+
+/**
+ * Utility class for working with generics.
+ */
+public class Generics {
+    /**
+     * Returns the exact type of the elements in an iterable class.
+     *
+     * @param cls
+     *            the class to get the exact type of
+     * @return the exact type of the elements in the iterable class
+     */
+    public static Optional<Type> getExactIterableType(Class<?> cls) {
+        try {
+            // We can find the exact type by looking at the iterator method
+            // return type
+            var method = cls.getMethod("iterator");
+            var exactReturnType = GenericTypeReflector
+                    .getExactReturnType(method, cls);
+
+            // We know the format of the method, so we can safely cast the type
+            if (exactReturnType instanceof ParameterizedType) {
+                return Optional.of(((ParameterizedType) exactReturnType)
+                        .getActualTypeArguments()[0]);
+            }
+
+            return Optional.empty();
+        } catch (NoSuchMethodException e) {
+            // This should really never happen if the passed class is an
+            // Iterable
+            throw new IllegalArgumentException(
+                    "Class " + cls.getName() + " is not an Iterable");
+        }
+    }
+}
diff --git a/packages/java/parser-jvm-utils/src/main/java/module-info.java b/packages/java/parser-jvm-utils/src/main/java/module-info.java
@@ -7,6 +7,7 @@
     requires com.fasterxml.jackson.datatype.jsr310;
     requires com.fasterxml.jackson.datatype.jdk8;
     requires com.fasterxml.jackson.module.paramnames;
+    requires gentyref;
 
     exports com.vaadin.hilla.parser.utils;
     exports com.vaadin.hilla.parser.jackson;