From df5777ee5a69830fc408bfb5624ea28d75f3e777 Mon Sep 17 00:00:00 2001 From: Praneet Loke <1466314+praneetloke@users.noreply.github.com> Date: Mon, 8 Apr 2024 21:58:28 -0700 Subject: [PATCH 1/4] WIP fix for collisions with return types for list funcs that use allOf definition --- pkg/openapi.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/pkg/openapi.go b/pkg/openapi.go index 9b430b6..8d0415d 100644 --- a/pkg/openapi.go +++ b/pkg/openapi.go @@ -23,6 +23,7 @@ import ( const ( componentsSchemaRefPrefix = "#/components/schemas/" + typesSchemaRefPrefix = "#/types/" jsonMimeType = "application/json" parameterLocationPath = "path" pathSeparator = "/" @@ -458,6 +459,21 @@ func (o *OpenAPIContext) genListFunc(pathItem openapi3.PathItem, returnTypeSchem return nil, errors.Wrap(err, "generating property type spec for response schema") } + actualTypeTok := strings.TrimPrefix(outputPropType.Ref, typesSchemaRefPrefix) + tokParts := strings.Split(actualTypeTok, ":") + actualTypeName := tokParts[2] + if strings.EqualFold(actualTypeName, funcName) { + newTypeName := actualTypeName + "Items" + outputType := funcPkgCtx.pkg.Types[actualTypeTok] + tokParts[2] = newTypeName + newTypeTok := strings.Join(tokParts, ":") + funcPkgCtx.pkg.Types[newTypeTok] = outputType + + delete(funcPkgCtx.pkg.Types, actualTypeTok) + + outputPropType.Ref = typesSchemaRefPrefix + newTypeTok + } + return &pschema.FunctionSpec{ Description: pathItem.Description, Inputs: &pschema.ObjectTypeSpec{ @@ -912,10 +928,6 @@ func (ctx *resourceContext) propertyTypeSpec(parentName string, propSchema opena typName := ToPascalCase(schemaName) tok := fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName) - if schemaName == "sql_mode" { - glog.Info("") - } - typeSchema, ok := ctx.openapiComponents.Schemas[schemaName] if !ok { return nil, false, errors.Errorf("definition %s not found in resource %s", schemaName, parentName) @@ -1022,7 +1034,8 @@ func (ctx *resourceContext) propertyTypeSpec(parentName string, propSchema opena return nil, false, errors.Wrap(err, "generating properties from allOf schema definition") } - tok := fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, ToPascalCase(parentName)) + typName := ToPascalCase(parentName) + tok := fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName) ctx.pkg.Types[tok] = pschema.ComplexTypeSpec{ ObjectTypeSpec: pschema.ObjectTypeSpec{ Description: propSchema.Value.Description, From 54d92a6db8068f2575ce00a401b575c6bd0748d5 Mon Sep 17 00:00:00 2001 From: Praneet Loke <1466314+praneetloke@users.noreply.github.com> Date: Mon, 8 Apr 2024 22:07:44 -0700 Subject: [PATCH 2/4] Check for non-empty ref --- pkg/openapi.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/pkg/openapi.go b/pkg/openapi.go index 8d0415d..6627c77 100644 --- a/pkg/openapi.go +++ b/pkg/openapi.go @@ -459,19 +459,23 @@ func (o *OpenAPIContext) genListFunc(pathItem openapi3.PathItem, returnTypeSchem return nil, errors.Wrap(err, "generating property type spec for response schema") } - actualTypeTok := strings.TrimPrefix(outputPropType.Ref, typesSchemaRefPrefix) - tokParts := strings.Split(actualTypeTok, ":") - actualTypeName := tokParts[2] - if strings.EqualFold(actualTypeName, funcName) { - newTypeName := actualTypeName + "Items" - outputType := funcPkgCtx.pkg.Types[actualTypeTok] - tokParts[2] = newTypeName - newTypeTok := strings.Join(tokParts, ":") - funcPkgCtx.pkg.Types[newTypeTok] = outputType - - delete(funcPkgCtx.pkg.Types, actualTypeTok) - - outputPropType.Ref = typesSchemaRefPrefix + newTypeTok + // Rename the output type if it's the same as the func name. + if outputPropType.Ref != "" { + actualTypeTok := strings.TrimPrefix(outputPropType.Ref, typesSchemaRefPrefix) + tokParts := strings.Split(actualTypeTok, ":") + actualTypeName := tokParts[2] + + if strings.EqualFold(actualTypeName, funcName) { + newTypeName := actualTypeName + "Items" + outputType := funcPkgCtx.pkg.Types[actualTypeTok] + tokParts[2] = newTypeName + newTypeTok := strings.Join(tokParts, ":") + funcPkgCtx.pkg.Types[newTypeTok] = outputType + + delete(funcPkgCtx.pkg.Types, actualTypeTok) + + outputPropType.Ref = typesSchemaRefPrefix + newTypeTok + } } return &pschema.FunctionSpec{ From 4f038193d630d987dd9cbc8fc88d2d643168919d Mon Sep 17 00:00:00 2001 From: Praneet Loke <1466314+praneetloke@users.noreply.github.com> Date: Sat, 13 Apr 2024 10:58:00 -0700 Subject: [PATCH 3/4] Add a test --- pkg/bug_131_test.go | 31 +++++++++++ pkg/openapi.go | 5 ++ pkg/testdata/bug_131_openapi.yml | 96 ++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+) create mode 100644 pkg/bug_131_test.go create mode 100644 pkg/testdata/bug_131_openapi.yml diff --git a/pkg/bug_131_test.go b/pkg/bug_131_test.go new file mode 100644 index 0000000..f1c0177 --- /dev/null +++ b/pkg/bug_131_test.go @@ -0,0 +1,31 @@ +package pkg + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBug131(t *testing.T) { + mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "bug_131_openapi.yml")) + + openAPICtx := &OpenAPIContext{ + Doc: *testOpenAPIDoc, + Pkg: &testPulumiPkg, + } + + csharpNamespaces := map[string]string{ + "": "Provider", + } + + providerMetadata, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces) + assert.Nil(t, err) + assert.NotNil(t, providerMetadata) + + funcSpec, ok := testPulumiPkg.Functions[packageName+":actions/v2:listActions"] + assert.True(t, ok, "Expected to find function listActions in the Pulumi package spec") + + assert.NotEmpty(t, funcSpec.Outputs.Properties["items"].Ref) + assert.Equal(t, funcSpec.Outputs.Properties["items"].Ref, "#/types/fake-package:actions/v2:ListActionsItems") +} diff --git a/pkg/openapi.go b/pkg/openapi.go index 6627c77..7b0a4be 100644 --- a/pkg/openapi.go +++ b/pkg/openapi.go @@ -460,6 +460,11 @@ func (o *OpenAPIContext) genListFunc(pathItem openapi3.PathItem, returnTypeSchem } // Rename the output type if it's the same as the func name. + // This can happen if the response type schema uses an allOf + // definition because there is no single authoritative type + // to use as the name of the resulting type, so the resulting + // type is named using the parent name, which would be the + // function name in this case. if outputPropType.Ref != "" { actualTypeTok := strings.TrimPrefix(outputPropType.Ref, typesSchemaRefPrefix) tokParts := strings.Split(actualTypeTok, ":") diff --git a/pkg/testdata/bug_131_openapi.yml b/pkg/testdata/bug_131_openapi.yml new file mode 100644 index 0000000..2fe02c1 --- /dev/null +++ b/pkg/testdata/bug_131_openapi.yml @@ -0,0 +1,96 @@ +openapi: 3.1.0 +info: + title: Fake API + version: "2.0" +servers: + - url: https://api.fake.com + description: production + +components: + schemas: + meta_properties: + type: object + description: Information about the response itself. + properties: + total: + description: Number of objects returned by the request. + type: integer + example: 1 + meta: + type: object + properties: + meta: + allOf: + - $ref: "#/components/schemas/meta_properties" + - required: + - total + required: + - meta + action: + type: object + properties: + id: + type: integer + description: >- + A unique numeric ID that can be used to identify and reference an + action. + example: 36804636 + + parameters: + per_page: + in: query + name: per_page + required: false + description: Number of items returned per page + schema: + type: integer + minimum: 1 + default: 20 + maximum: 200 + example: 2 + page: + in: query + name: page + required: false + description: Which 'page' of paginated results to return. + schema: + type: integer + minimum: 1 + default: 1 + example: 1 + + responses: + actions: + description: >- + The results will be returned as a JSON object with an actions key. This + will be set to an array filled with action objects containing the + standard action attributes + content: + application/json: + schema: + allOf: + - type: object + properties: + actions: + type: array + items: + $ref: "#/components/schemas/action" + - $ref: "#/components/schemas/meta" + +paths: + /v2/actions: + get: + operationId: actions_list + summary: List All Actions + description: >- + This will be the entire list of actions taken on your account, so it + will be quite large. As with any large collection returned by the API, + the results will be paginated with only 20 on each page by default. + tags: + - Actions + parameters: + - $ref: "#/components/parameters/per_page" + - $ref: "#/components/parameters/page" + responses: + "200": + $ref: "#/components/responses/actions" From 768b32419d6e320b3726e17691f36429ae68a457 Mon Sep 17 00:00:00 2001 From: Praneet Loke <1466314+praneetloke@users.noreply.github.com> Date: Sat, 13 Apr 2024 11:12:57 -0700 Subject: [PATCH 4/4] Update test --- pkg/bug_131_test.go | 22 +++++++++++++++++----- pkg/testdata/bug_131_openapi.yml | 31 ++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/pkg/bug_131_test.go b/pkg/bug_131_test.go index f1c0177..c7687b2 100644 --- a/pkg/bug_131_test.go +++ b/pkg/bug_131_test.go @@ -23,9 +23,21 @@ func TestBug131(t *testing.T) { assert.Nil(t, err) assert.NotNil(t, providerMetadata) - funcSpec, ok := testPulumiPkg.Functions[packageName+":actions/v2:listActions"] - assert.True(t, ok, "Expected to find function listActions in the Pulumi package spec") - - assert.NotEmpty(t, funcSpec.Outputs.Properties["items"].Ref) - assert.Equal(t, funcSpec.Outputs.Properties["items"].Ref, "#/types/fake-package:actions/v2:ListActionsItems") + t.Run("Renamed", func(t *testing.T) { + funcSpec, ok := testPulumiPkg.Functions[packageName+":actions/v2:listActions"] + assert.True(t, ok, "Expected to find function listActions in the Pulumi package spec") + + assert.NotEmpty(t, funcSpec.Outputs.Properties["items"].Ref) + assert.Equal(t, funcSpec.Outputs.Properties["items"].Ref, "#/types/fake-package:actions/v2:ListActionsItems") + }) + + // Test that a response type without an allOf definition in its + // response type is named with the suffix Properties. + t.Run("PropertiesResponseType", func(t *testing.T) { + funcSpec, ok := testPulumiPkg.Functions[packageName+":actions2/v2:listActions2"] + assert.True(t, ok, "Expected to find function listActions2 in the Pulumi package spec") + + assert.NotEmpty(t, funcSpec.Outputs.Properties["items"].Ref) + assert.Equal(t, funcSpec.Outputs.Properties["items"].Ref, "#/types/fake-package:actions2/v2:ListActions2Properties") + }) } diff --git a/pkg/testdata/bug_131_openapi.yml b/pkg/testdata/bug_131_openapi.yml index 2fe02c1..156e80c 100644 --- a/pkg/testdata/bug_131_openapi.yml +++ b/pkg/testdata/bug_131_openapi.yml @@ -76,21 +76,38 @@ components: items: $ref: "#/components/schemas/action" - $ref: "#/components/schemas/meta" + actions2: + description: >- + The results will be returned as a JSON object with an actions key. This + will be set to an array filled with action objects containing the + standard action attributes + content: + application/json: + schema: + type: object + properties: + actions: + type: array + items: + $ref: "#/components/schemas/action" paths: /v2/actions: get: operationId: actions_list - summary: List All Actions - description: >- - This will be the entire list of actions taken on your account, so it - will be quite large. As with any large collection returned by the API, - the results will be paginated with only 20 on each page by default. - tags: - - Actions parameters: - $ref: "#/components/parameters/per_page" - $ref: "#/components/parameters/page" responses: "200": $ref: "#/components/responses/actions" + + /v2/actions2: + get: + operationId: actions2_list + parameters: + - $ref: "#/components/parameters/per_page" + - $ref: "#/components/parameters/page" + responses: + "200": + $ref: "#/components/responses/actions2"