Skip to content

Commit

Permalink
Consider path parameters common to all operations (#148)
Browse files Browse the repository at this point in the history
Also:

* Ensure there is no oneOf definition before defaulting to a simple type

* Exclude id property from required outputs copied from response body schema. Handle property type if its ref is to a schema with a oneOf definition

* Sanitize names that contain HTTP verbs
  • Loading branch information
praneetloke authored May 11, 2024
1 parent 3c2683d commit 04e99a6
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,10 @@
"build.env": {
"GOPRIVATE": "github.com/cloudy-sky-software/pulumi-render,github.com/cloudy-sky-software/pulschema"
}
},
"yaml.schemas": {
"https://raw.githubusercontent.com/OAI/OpenAPI-Specification/main/schemas/v3.1/schema.json": [
"**/testdata/*.yml"
]
}
}
40 changes: 40 additions & 0 deletions pkg/common_path_params_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package pkg

import (
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
)

// TestCommonPathParams tests if path params that are
// common to all operations are considered.
func TestCommonPathParams(t *testing.T) {
mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "common_path_params_openapi.yml"))

openAPICtx := &OpenAPIContext{
Doc: *testOpenAPIDoc,
Pkg: &testPulumiPkg,
}

csharpNamespaces := map[string]string{
"": "Provider",
}

_, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces)
assert.Nil(t, err)

subResource, ok := testPulumiPkg.Resources["fake-package:fakeresource/v2:SubResource"]
assert.Truef(t, ok, "Expected to find a resource called SubResource: %v", testPulumiPkg.Resources)

// Ensure that the input properties for the resource contains
// the expected id property.
assert.Contains(t, subResource.InputProperties, "id")

// Ensure that the "get" func also contains the id
// as an input properties.
getFunc, ok := testPulumiPkg.Functions["fake-package:fakeresource/v2:listSubResources"]
assert.Truef(t, ok, "Expected to find a list func listSubResources: %v", testPulumiPkg.Functions)
assert.NotNil(t, getFunc.Inputs)
assert.Contains(t, getFunc.Inputs.Properties, "id")
}
48 changes: 36 additions & 12 deletions pkg/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,9 @@ func (o *OpenAPIContext) GatherResourcesFromAPI(csharpNamespaces map[string]stri
// to create resources if the endpoint itself requires the ID of the resource.
if pathItem.Post == nil && !strings.HasSuffix(currentPath, "}") {
resourceName := getResourceTitleFromOperationID(pathItem.Put.OperationID, http.MethodPut, o.OperationIdsHaveTypeSpecNamespace)
if err := o.gatherResource(currentPath, resourceName, *resourceType, nil /*response type*/, pathItem.Put.Parameters, module); err != nil {
parameters := pathItem.Parameters
parameters = append(parameters, pathItem.Put.Parameters...)
if err := o.gatherResource(currentPath, resourceName, *resourceType, nil /*response type*/, parameters, module); err != nil {
return nil, o.Doc, errors.Wrapf(err, "generating resource for api path %s", currentPath)
}
}
Expand Down Expand Up @@ -371,13 +373,16 @@ func (o *OpenAPIContext) GatherResourcesFromAPI(csharpNamespaces map[string]stri

contract.Assertf(pathItem.Post.OperationID != "", "operationId is missing for path POST %s", currentPath)

jsonReq := pathItem.Post.RequestBody.Value.Content.Get(jsonMimeType)
if jsonReq.Schema.Value == nil {
return nil, o.Doc, errors.Errorf("path %s has no api schema definition for post method", currentPath)
var jsonReq *openapi3.MediaType
if pathItem.Post.RequestBody != nil {
jsonReq = pathItem.Post.RequestBody.Value.Content.Get(jsonMimeType)
if jsonReq.Schema.Value == nil {
return nil, o.Doc, errors.Errorf("path %s has no request body schema for post method", currentPath)
}
} else {
jsonReq = openapi3.NewMediaType().WithSchema(openapi3.NewSchema())
}

resourceRequestType := jsonReq.Schema.Value

// Usually 201 and 202 status codes don't have response bodies,
// but some OpenAPI specs seem to have a response body for those
// status codes. For example, DigitalOcean responds with 202
Expand Down Expand Up @@ -405,7 +410,10 @@ func (o *OpenAPIContext) GatherResourcesFromAPI(csharpNamespaces map[string]stri

resourceName := getResourceTitleFromOperationID(pathItem.Post.OperationID, http.MethodPost, o.OperationIdsHaveTypeSpecNamespace)

if err := o.gatherResource(currentPath, resourceName, *resourceRequestType, resourceResponseType, pathItem.Post.Parameters, module); err != nil {
resourceRequestType := jsonReq.Schema.Value
parameters := pathItem.Parameters
parameters = append(parameters, pathItem.Post.Parameters...)
if err := o.gatherResource(currentPath, resourceName, *resourceRequestType, resourceResponseType, parameters, module); err != nil {
return nil, o.Doc, errors.Wrapf(err, "generating resource for api path %s", currentPath)
}
}
Expand Down Expand Up @@ -437,7 +445,10 @@ func (o *OpenAPIContext) genListFunc(pathItem openapi3.PathItem, returnTypeSchem

requiredInputs := codegen.NewStringSet()
inputProps := make(map[string]pschema.PropertySpec)
for _, param := range pathItem.Get.Parameters {

parameters := pathItem.Parameters
parameters = append(parameters, pathItem.Get.Parameters...)
for _, param := range parameters {
if param.Value.In != parameterLocationPath {
continue
}
Expand Down Expand Up @@ -522,7 +533,11 @@ func (o *OpenAPIContext) genGetFunc(pathItem openapi3.PathItem, returnTypeSchema

requiredInputs := codegen.NewStringSet()
inputProps := make(map[string]pschema.PropertySpec)
for _, param := range pathItem.Get.Parameters {

parameters := pathItem.Parameters
parameters = append(parameters, pathItem.Get.Parameters...)

for _, param := range parameters {
if param.Value.In != parameterLocationPath {
continue
}
Expand Down Expand Up @@ -812,8 +827,12 @@ func (o *OpenAPIContext) gatherResourceProperties(resourceName string, requestBo
// If there is a response body schema, then add its required
// properties as well.
if responseBodySchema != nil {
for _, required := range responseBodySchema.Required {
requiredOutputs.Add(required)
for _, requiredProp := range responseBodySchema.Required {
if requiredProp == "id" {
continue
}

requiredOutputs.Add(requiredProp)
}
}

Expand Down Expand Up @@ -941,6 +960,7 @@ func (ctx *resourceContext) propertyTypeSpec(parentName string, propSchema opena
if propSchema.Ref != "" && !propSchema.Value.Type.Is(openapi3.TypeArray) && len(propSchema.Value.Enum) == 0 {
schemaName := strings.TrimPrefix(propSchema.Ref, componentsSchemaRefPrefix)
typName := ToPascalCase(schemaName)
typName = sanitizeResourceTitle(typName)
tok := fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName)

typeSchema, ok := ctx.openapiComponents.Schemas[schemaName]
Expand All @@ -954,19 +974,23 @@ func (ctx *resourceContext) propertyTypeSpec(parentName string, propSchema opena
// which are actually just simple types.
if !typeSchema.Value.Type.Is(openapi3.TypeObject) &&
len(typeSchema.Value.Properties) == 0 &&
len(typeSchema.Value.OneOf) == 0 &&
len(typeSchema.Value.AllOf) == 0 {
return &pschema.TypeSpec{
Type: typeSchema.Value.Type.Slice()[0],
}, false, nil
}

if len(typeSchema.Value.OneOf) > 0 {
return ctx.propertyTypeSpec(typName, *typeSchema)
}

newType := !ctx.visitedTypes.Has(tok)

if newType {
ctx.visitedTypes.Add(tok)

specs, requiredSpecs, err := ctx.genProperties(typName, *typeSchema.Value)

if err != nil {
return nil, false, errors.Wrapf(err, "generating properties for %s", typName)
}
Expand Down
17 changes: 15 additions & 2 deletions pkg/resource_naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,22 @@ func getResourceTitleFromOperationID(originalOperationID, method string, isSepar
return resourceTitle
}

var replaceKeywords = map[string]string{"POST": "Create", "Post": "Create", "post": "create", "DELETE": "Delete", "Delete": "Delete", "PUT": "Put", "Put": "Put", "Patch": "Patch", "PATCH": "Patch", "GET": "Get", "Get": "Get"}

func sanitizeResourceTitle(title string) string {
for match, replaceWith := range replaceKeywords {
if strings.Contains(title, match) {
title = strings.ReplaceAll(title, match, replaceWith)
break
}
}

return title
}

func getResourceTitleFromRequestSchema(schemaName string, schemaRef *openapi3.SchemaRef) string {
if schemaRef.Value.Title != "" {
return ToPascalCase(schemaRef.Value.Title)
return ToPascalCase(sanitizeResourceTitle(schemaRef.Value.Title))
}

var title string
Expand All @@ -112,7 +125,7 @@ func getResourceTitleFromRequestSchema(schemaName string, schemaRef *openapi3.Sc
title = result
}

return title
return sanitizeResourceTitle(title)
}

// ensureIDHierarchyInRequestPath ensures that the IDs in the path
Expand Down
58 changes: 58 additions & 0 deletions pkg/testdata/common_path_params_openapi.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
openapi: 3.1.0
info:
title: Fake API
version: "2.0"
servers:
- url: https://api.fake.com
description: production

components:
schemas:
a_string_prop:
type: string
request_object_type:
type: object
properties:
simple_prop:
$ref: "#/components/schemas/a_string_prop"
response_object_type:
type: object
properties:
another_prop:
$ref: "#/components/schemas/a_string_prop"

paths:
/v2/fakeResource/{id}/subResource:
parameters:
- name: id
in: path
required: true
schema:
type: string
get:
operationId: listSubResources
responses:
"200":
description: The response will be a JSON object with a key called `subResources` which is an array of sub-resource items.
content:
application/json:
schema:
properties:
subResources:
type: array
items:
$ref: "#/components/schemas/response_object_type"
post:
operationId: createSubResource
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/request_object_type"
responses:
"200":
description: The response will be a JSON object with a key called `action`.
content:
application/json:
schema:
$ref: "#/components/schemas/response_object_type"

0 comments on commit 04e99a6

Please sign in to comment.