Skip to content

Commit

Permalink
Update enum collisions test
Browse files Browse the repository at this point in the history
  • Loading branch information
praneetloke committed May 26, 2024
1 parent 461f8f0 commit 9e072b8
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 38 deletions.
30 changes: 15 additions & 15 deletions pkg/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -1444,31 +1444,31 @@ func (ctx *resourceContext) genEnumType(enumName string, propSchema openapi3.Sch

// Make sure that the type name we composed doesn't clash with another type
// already defined in the schema earlier. The same enum does show up in multiple
// places of specs, so we want to error only if they a) have the same name
// b) the list of values does not match.
// places of specs.
if other, ok := ctx.pkg.Types[tok]; ok {
if len(other.Enum) == 0 {
// The other type is not an enum, so we should
// distinguish this type as an enum.
return ctx.genEnumType(enumName+"Enum", propSchema)
}

same := len(enumSpec.Enum) == len(other.Enum)
for _, val := range other.Enum {
same = same && names.Has(val.Name)
}

if !same {
// It could be that the other type is not even
// an enum.
if len(other.Enum) == 0 {
typName = enumName + "Enum"
tok = fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName)
referencedTypeName = fmt.Sprintf("#/types/%s", tok)
} else if !strings.HasPrefix(typName, ctx.resourceName) {
// If the values are not the same and the type
if !strings.HasPrefix(typName, ctx.resourceName) {
// Since the values are not the same and the type
// is not already prefixed with the resource name,
// we'll just use a unique name for it.
// Check if the other type is an enum.
typName = ctx.resourceName + enumName
tok = fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName)
referencedTypeName = fmt.Sprintf("#/types/%s", tok)
return ctx.genEnumType(ctx.resourceName+enumName, propSchema)
} else {

Check failure on line 1466 in pkg/openapi.go

View workflow job for this annotation

GitHub Actions / lint-and-test

indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
msg := fmt.Sprintf("duplicate enum %q: %+v vs. %+v", tok, enumSpec.Enum, other.Enum)
// If we got here, it means that this enum type
// has different values than the one that we
// already processed _and_ is already prefixed
// with the resource name.
msg := fmt.Sprintf("duplicate enum with different values %q: %+v vs. %+v", tok, enumSpec.Enum, other.Enum)
return nil, &duplicateEnumError{msg: msg}
}
}
Expand Down
19 changes: 13 additions & 6 deletions pkg/prefix_enum_type_on_collision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,22 @@ func TestGenEnumType(t *testing.T) {

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

// Due to the ordering of the paths, the type that has an enum prop point to a ref
// would be encountered first and therefore would not have any collision.
assert.Equal(t, "#/types/fake-package:resource/v2:EnumProp", resourceSpec.InputProperties["enumProp"].Ref)
assert.Equal(t, "#/types/fake-package:resource/v2:AProp", resourceSpec.InputProperties["aProp"].Ref)

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

// When the next operation is encountered, the enum_prop property having an inline
// schema with different values should be renamed with the prefix of the resource.
assert.Equal(t, "#/types/fake-package:resource/v2:SomeOtherResourceEnumProp", resourceSpec2.InputProperties["enumProp"].Ref)
// Because AProp was already encountered and is not an enum, this resource's property
// a_prop which is an enum should be suffixed with Enum but not yet prefixed with the
// resource name.
assert.Equal(t, "#/types/fake-package:resource/v2:APropEnum", resourceSpec2.InputProperties["aProp"].Ref)

resourceSpec3, ok := testPulumiPkg.Resources["fake-package:resource/v2:LastResource"]
assert.Truef(t, ok, "Expected to find a resource called LastResource: %v", testPulumiPkg.Resources)
// When the next operation is encountered, the a_prop property having an inline
// schema with different values should be renamed with the prefix of the resource,
// as well as have the suffix Enum because, remember, we need to distinguish from
// the regular object-type property AProp.
assert.Equal(t, "#/types/fake-package:resource/v2:LastResourceAPropEnum", resourceSpec3.InputProperties["aProp"].Ref)
}
65 changes: 48 additions & 17 deletions pkg/testdata/prefix_enum_type_on_collision_openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,61 @@ components:
- a
- b
- c

a_string_prop:
type: string

a_prop:
type: object
properties:
prop:
type: string

request_object_type:
type: object
properties:
simple_prop:
$ref: "#/components/schemas/a_string_prop"
# This prop is a ref to another type that is
# not an enum to simulate a collision between
# similarly-named properties that are not
# enums.
a_prop:
$ref: "#/components/schemas/a_prop"

request_object_type2:
type: object
properties:
simple_prop:
$ref: "#/components/schemas/a_string_prop"
# This prop has a ref to a shared enum type.
enum_prop:
a_prop:
$ref: "#/components/schemas/an_enum_type"
request_object_type2:

request_object_type3:
type: object
properties:
simple_prop:
$ref: "#/components/schemas/a_string_prop"
# Whereas this prop, also called `enum_prop`
# has an inline schema def.
enum_prop:
a_prop:
type: string
enum:
- d
- e
- f

responses:
response_object_type:
type: object
properties:
another_prop:
$ref: "#/components/schemas/a_string_prop"
description: The response will be a JSON object with a key called `action`.
content:
application/json:
schema:
type: object
properties:
another_prop:
$ref: "#/components/schemas/a_string_prop"

paths:
/v2/resource/someResource:
Expand All @@ -54,11 +81,7 @@ paths:
$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"
$ref: "#/components/responses/response_object_type"

/v2/resource/someOtherResource:
post:
Expand All @@ -70,8 +93,16 @@ paths:
$ref: "#/components/schemas/request_object_type2"
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"
$ref: "#/components/responses/response_object_type"

/v2/resource/lastResource:
post:
operationId: create_last_resource
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/request_object_type3"
responses:
"200":
$ref: "#/components/responses/response_object_type"

0 comments on commit 9e072b8

Please sign in to comment.