fix(type-safe-api): fix several regressions in the new code generation (

#845)

* fix(type-safe-api): add missing imports for parameter models

Parameter models were not being imported in the API client in the typescript runtime.

References #841

* fix(type-safe-api): fix duplicate 200 response for operations with default responses

OpenAPI generator treated "default" responses as responses with code 0, however parseOpenapi treats
default as 200. Adjust code generation to follow OpenAPI for backwards compatibility and to avoid
duplicate 200 responses. Note that this should be revisited such that code generation allows for
any response code other than those defined to be returned for the default response, rather than 0.

References #841

* fix(type-safe-api): add missing imports for composed models

Composed models (using all-of, one-of or any-of) are generated as "mixins" rather than using
inheritance, to match openapi generator's behaviour. We therefore need to ensure models import all
types from the composed properties.

References #841

packages/type-safe-api/scripts/type-safe-api/generators/generate-next.ts

@@ -15,9 +15,11 @@ import _kebabCase from "lodash/kebabCase";
 import _orderBy from "lodash/orderBy";
 import _uniq from "lodash/uniq";
 import _uniqBy from "lodash/uniqBy";
+import _isEqual from "lodash/isEqual";
 import { OpenAPIV3 } from "openapi-types";
 import * as parseOpenapi from "parse-openapi";
 import { getOperationResponses } from "parse-openapi/dist/parser/getOperationResponses";
+import { getOperationResponse } from "parse-openapi/dist/parser/getOperationResponse";
 
 const TSAPI_WRITE_FILE_START = "###TSAPI_WRITE_FILE###";
 const TSAPI_WRITE_FILE_END = "###/TSAPI_WRITE_FILE###";
@@ -427,7 +429,8 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
   // Augment operations with additional data
   data.services.forEach((service) => {
 
-    // Keep track of the response models we need the service (ie api client) to import
+    // Keep track of the request and response models we need the service (ie api client) to import
+    const requestModelImports: string[] = [];
     const responseModelImports: string[] = [];
 
     service.operations.forEach((op) => {
@@ -452,12 +455,24 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
         // Add all response models to the response model imports
         responseModelImports.push(...responses.filter(r => r.export === "reference").map(r => r.type));
 
-        // parseOpenapi does not distinguish between returning an "any" or returning "void"
-        // We distinguish this by looking back each response in the spec, and checking whether it
-        // has content
+        const defaultResponse = resolveIfRef(spec, specOp.responses?.['default']);
+
         [...responses, ...op.results].forEach((response) => {
+          // Check whether this response is actually the "default" response.
+          if (response.code === 200 && defaultResponse && _isEqual(response, getOperationResponse(spec, defaultResponse, 200))) {
+            // For backwards compatibility with OpenAPI generator, we set the response code for the default response to 0.
+            // See: https://github.com/OpenAPITools/openapi-generator/blob/8f2676c5c2bcbcc41942307e5c8648cee38bcc44/modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenResponse.java#L622
+            // TODO: we should likely revisit this to make the handler wrappers more intuitive for the default response case, as
+            // the code 0 would actually need to be returned by the server for marshalling etc to work for the model associated with
+            // the default response.
+            response.code = 0;
+          }
+
           const matchingSpecResponse = specOp.responses[`${response.code}`];
 
+          // parseOpenapi does not distinguish between returning an "any" or returning "void"
+          // We distinguish this by looking back each response in the spec, and checking whether it
+          // has content
           if (matchingSpecResponse) {
             // Resolve the ref if necessary
             const specResponse = resolveIfRef(spec, matchingSpecResponse);
@@ -486,8 +501,12 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
 
       // Loop through the parameters
       op.parameters.forEach((parameter) => {
-        const specParameter = specParametersByName[parameter.prop];
+        // Add the request model import
+        if (parameter.export === "reference") {
+          requestModelImports.push(parameter.type);
+        }
 
+        const specParameter = specParametersByName[parameter.prop];
         const specParameterSchema = resolveIfRef(spec, specParameter?.schema);
 
         if (specParameterSchema) {
@@ -541,7 +560,7 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
     service.operations = _orderBy(service.operations, (op) => op.name);
 
     // Add the models to import
-    (service as any).modelImports = _orderBy(_uniq([...service.imports, ...responseModelImports]));
+    (service as any).modelImports = _orderBy(_uniq([...service.imports, ...requestModelImports, ...responseModelImports]));
 
     // Add the service class name
     (service as any).className = `${service.name}Api`;
@@ -554,15 +573,20 @@ const buildData = (inSpec: OpenAPIV3.Document, metadata: any) => {
     if (matchingSpecModel) {
       const specModel = isRef(matchingSpecModel) ? resolveRef(spec, matchingSpecModel.$ref) as OpenAPIV3.SchemaObject : matchingSpecModel;
 
+      // Resolve properties inherited from composed schemas
+      const composedProperties = resolveComposedProperties(data, model);
+      (model as any).resolvedProperties = composedProperties;
+
       // Add unique imports
-      (model as any).uniqueImports = _orderBy(_uniq(model.imports));
+      (model as any).uniqueImports = _orderBy(_uniq([
+        ...model.imports,
+        // Include composed property imports, if any
+        ...composedProperties.filter(p => p.export === "reference").map(p => p.type),
+      ]));
 
       // Add deprecated flag if present
       (model as any).deprecated = specModel.deprecated || false;
 
-      // Resolve properties inherited from composed schemas
-      (model as any).resolvedProperties = resolveComposedProperties(data, model);
-
       // If the model has "additionalProperties" there should be a "dictionary" property
       if (specModel.additionalProperties) {
         (model as any).additionalPropertiesProperty = model.properties.find(p => !p.name && p.export === "dictionary");

packages/type-safe-api/test/resources/specs/allof-model.yaml

@@ -0,0 +1,54 @@
+openapi: 3.0.3
+info:
+  version: 1.0.0
+  title: My API
+  description: See https://github.com/aws/aws-pdk/issues/841
+paths:
+  /hello:
+    get:
+      operationId: sayHello
+      responses:
+        200:
+          description: Successful response
+          content:
+            'application/json':
+              schema:
+                $ref: '#/components/schemas/Template'
+components:
+  schemas:
+    Template:
+      allOf:
+        - $ref: '#/components/schemas/TemplateBase'
+        - $ref: '#/components/schemas/TemplateBody'
+        - title: Template
+          description: |
+            Represents a complete template in the system.
+    TemplateBase:
+      type: object
+      title: TemplateBase
+      description: |
+        Represents the base properties of a template.
+      additionalProperties: false
+      required:
+        - id
+      properties:
+        id:
+          $ref: '#/components/schemas/TemplateID'
+    TemplateBody:
+      type: object
+      title: TemplateBody
+      description: |
+        Represents the body of a template.
+      additionalProperties: false
+      properties:
+        parent_id:
+          $ref: '#/components/schemas/TemplateID'
+        boolean:
+          type: boolean
+          description: A boolean value.
+    TemplateID:
+      type: string
+      format: uuid
+      title: TemplateID
+      description: The unique identifier for a template.
+      maxLength: 36

packages/type-safe-api/test/resources/specs/default-response.yaml

@@ -0,0 +1,51 @@
+openapi: 3.0.3
+info:
+  version: 1.0.0
+  title: My API
+  description: See https://github.com/aws/aws-pdk/issues/841
+paths:
+  /hello:
+    get:
+      operationId: sayHello
+      x-handler:
+        language: typescript
+      parameters:
+        - in: query
+          name: name
+          schema:
+            type: string
+          required: true
+      responses:
+        200:
+          description: Successful response
+          content:
+            'application/json':
+              schema:
+                $ref: '#/components/schemas/SayHelloResponseContent'
+        default:
+          description: An error due to the client not being authorized to access the resource
+          content:
+            'application/json':
+              schema:
+                $ref: '#/components/schemas/ServiceUnavailableErrorResponseContent'
+components:
+  schemas:
+    SayHelloResponseContent:
+      type: object
+      properties:
+        id:
+          $ref: '#/components/schemas/HelloId'
+        message:
+          type: string
+      required:
+        - message
+    HelloId:
+      type: string
+      format: uuid
+    ServiceUnavailableErrorResponseContent:
+      type: object
+      properties:
+        message:
+          type: string
+      required:
+        - message