algorand / generator

OpenAPI parser/generator.
9 stars 13 forks source link

Code generation: Top-level array response is unsupported #36

Open michaeldiamant opened 2 years ago

michaeldiamant commented 2 years ago

Subject of the issue

While exposing a new route in SDKs, code generation fails to produce working code. Provided we agree the OAS definition is valid, the ticket's request is to modify generator to support the use case.

It appears the problem is specific to returning a top-level array response. The affected route is "/v2/applications/{application-id}/boxes" in https://github.com/algorand/go-algorand/blob/feature/avm-box/daemon/algod/api/algod.oas2.json.

Your environment

N/A

Steps to reproduce

Here's a PR illustrating the

  1. Configure environment locally as documented in https://github.com/algorand/go-algorand-sdk/pull/340#issuecomment-1175246907.
  2. Generate Java/Go SDK code changes and observe broken compilation.

Expected behaviour

generator produces Java + Go SDK changes that compile + implement the spec.

Actual behaviour

Here's a PR illustrating the as-is behavior: https://github.com/algorand/go-algorand-sdk/pull/340 .

winder commented 2 years ago

After some experimenting, I was able to get part way through this. It is incomplete though, and introduces unwanted artifacts:

diff --git a/scripts/generate_go.sh b/scripts/generate_go.sh
index 0c6623e..d6a22df 100755
--- a/scripts/generate_go.sh
+++ b/scripts/generate_go.sh
@@ -14,9 +14,9 @@ function help() {
   exit
 }

-ALGOD_SPEC=/home/will/go/src/github.com/algorand/go-algorand/daemon/algod/api/algod.oas2.json
+ALGOD_SPEC=/home/will/algorand/go-algorand/daemon/algod/api/algod.oas2.json
 INDEXER_SPEC=/home/will/algorand/indexer/api/indexer.oas2.json
-SDK_DIR=/home/will/go/src/github.com/algorand/go-algorand-sdk
+SDK_DIR=/home/will/algorand/go-algorand-sdk
 TEMPLATE_DIR=/home/will/algorand/generator/go_templates

 PARAMS=""
diff --git a/src/main/java/com/algorand/sdkutils/generators/OpenApiParser.java b/src/main/java/com/algorand/sdkutils/generators/OpenApiParser.java
index c1e35f9..7ff5786 100644
--- a/src/main/java/com/algorand/sdkutils/generators/OpenApiParser.java
+++ b/src/main/java/com/algorand/sdkutils/generators/OpenApiParser.java
@@ -365,6 +365,10 @@ public class OpenApiParser {
             }
         }
         // type: array
+        else if (parentNode.has("type") && parentNode.get("type").asText().equals("array")) {
+            TypeDef typeObj = getType(parentNode, true, "", true, false);
+            properties.add(typeObj);
+        }
         else if (parentNode.has("schema") && parentNode.get("schema").has("type") && parentNode.get("schema").get("type").asText().equals("array")) {
             TypeDef typeObj = getType(parentNode.get("schema"), true, "", true, false);
             properties.add(typeObj);
@@ -533,10 +537,12 @@ public class OpenApiParser {
                         publisher.publish(Events.ENUM_DEFINITION, clsType);
                     }

+                    /*
                     if (!hasProperties(cls.getValue())) {
                         // If it has no properties, no class is needed for this type.
                         continue;
                     }
+                     */
                     String className = Tools.getCamelCase(cls.getKey(), true);
                     if (!filterList.isEmpty() && filterList.contains(className)) {
                         continue;
@@ -567,11 +573,13 @@ public class OpenApiParser {
                         publisher.publish(Events.REGISTER_RETURN_TYPE, new StructDef(rtype.getKey(), realType));
                         continue;
                     }
+                    /*
                     if (rSchema.get("properties") == null) {
                         // cannot make a class without properties
                         // this type is currently not supported
                         continue;
-                    }                    
+                    }
+                     */
                     String className = Tools.getCamelCase(rtype.getKey(), true);
                     if (!filterList.isEmpty() && filterList.contains(className)) {
                         continue;
diff --git a/src/test/java/com/algorand/sdkutils/generators/OpenApiParserTest.java b/src/test/java/com/algorand/sdkutils/generators/OpenApiParserTest.java
index 209217f..79f70af 100644
--- a/src/test/java/com/algorand/sdkutils/generators/OpenApiParserTest.java
+++ b/src/test/java/com/algorand/sdkutils/generators/OpenApiParserTest.java
@@ -45,7 +45,7 @@ class OpenApiParserTest {
         // Then - verify that some of the expected events

         // 'onEvent(evt, StructDef)` is called as expected
-        List<String> expectedStructs = ImmutableList.of("ApiResponse", "Category", "Pet", "Tag", "Order", "User");
+        List<String> expectedStructs = ImmutableList.of("ArrayResponse", "ApiResponse", "Category", "Pet", "Tag", "Order", "User");
         ArgumentCaptor<StructDef> structDefArgumentCaptor = ArgumentCaptor.forClass(StructDef.class);
         verify(subscriber, times(expectedStructs.size()))
                 .onEvent(any(), structDefArgumentCaptor.capture());
@@ -55,15 +55,15 @@ class OpenApiParserTest {
                 .containsAll(expectedStructs);

         // 'onEvent(evt)' - called 20 times, once for each END_QUERY/END_MODEL.
-        verify(subscriber, times(20))
+        verify(subscriber, times(22))
                 .onEvent(any());

         // 'onEvent(evt, []String)` - called 14 times, once for each NEW_QUERY.
-        verify(subscriber, times(14))
+        verify(subscriber, times(15))
                 .onEvent(any(), any(QueryDef.class));

         // Property, Path param, Query param, Body
-        verify(subscriber, times(42))
+        verify(subscriber, times(43))
                 .onEvent(any(), any(TypeDef.class));
     }
 }
\ No newline at end of file
diff --git a/src/test/resources/petstore.json b/src/test/resources/petstore.json
index dd53650..e887f84 100644
--- a/src/test/resources/petstore.json
+++ b/src/test/resources/petstore.json
@@ -42,6 +42,38 @@
     "http"
   ],
   "paths":{
+    "/array":{
+      "get":{
+        "tags":[
+          "pet"
+        ],
+        "summary":"return an array response",
+        "description":"",
+        "operationId":"array",
+        "produces":[
+          "application/json"
+        ],
+        "parameters":[
+        ],
+        "responses":{
+          "200":{
+            "description":"successful operation",
+            "schema":{
+              "$ref":"#/definitions/ArrayResponse"
+            }
+          }
+        },
+        "security":[
+          {
+            "petstore_auth":[
+              "write:pets",
+              "read:pets"
+            ]
+          }
+        ]
+      }
+    },
+
     "/pet/{petId}/uploadImage":{
       "post":{
         "tags":[
@@ -882,6 +914,16 @@
     }
   },
   "definitions":{
+    "ArrayResponse": {
+      "description": "an array of responses",
+      "schema": {
+        "type": "array",
+        "items": {
+          "type": "string",
+          "format": "byte"
+        }
+      }
+    },
     "ApiResponse":{
       "type":"object",
       "properties":{