Treat oneOf without discriminator in resource inputs as allOf (#160)

* Treat oneOf without discriminator in resource inputs as allOf

* Update enum collisions test

* Fix lint error

* Add test for resource oneOf without discriminator

pkg/openapi.go

@@ -357,11 +357,17 @@ func (o *OpenAPIContext) GatherResourcesFromAPI(csharpNamespaces map[string]stri
 					}
 				} else {
 					resourceName := getResourceTitleFromOperationID(pathItem.Delete.OperationID, http.MethodDelete, o.OperationIdsHaveTypeSpecNamespace)
+					if !strings.HasSuffix(resourceName, "Kubernetes") {
+						resourceName = strings.TrimSuffix(resourceName, "s")
+					}
 					typeToken := fmt.Sprintf("%s:%s:%s", o.Pkg.Name, module, resourceName)
 					setDeleteOperationMapping(typeToken)
 				}
 			} else {
 				resourceName := getResourceTitleFromOperationID(pathItem.Delete.OperationID, http.MethodDelete, o.OperationIdsHaveTypeSpecNamespace)
+				if !strings.HasSuffix(resourceName, "Kubernetes") {
+					resourceName = strings.TrimSuffix(resourceName, "s")
+				}
 				typeToken := fmt.Sprintf("%s:%s:%s", o.Pkg.Name, module, resourceName)
 				setDeleteOperationMapping(typeToken)
 			}
@@ -419,6 +425,9 @@ func (o *OpenAPIContext) GatherResourcesFromAPI(csharpNamespaces map[string]stri
 		}
 
 		resourceName := getResourceTitleFromOperationID(pathItem.Post.OperationID, http.MethodPost, o.OperationIdsHaveTypeSpecNamespace)
+		if !strings.HasSuffix(resourceName, "Kubernetes") {
+			resourceName = strings.TrimSuffix(resourceName, "s")
+		}
 
 		resourceRequestType := jsonReq.Schema.Value
 		parameters := pathItem.Parameters
@@ -663,6 +672,15 @@ func (o *OpenAPIContext) gatherResource(
 		return nil
 	}
 
+	if len(resourceRequestType.OneOf) > 0 {
+		glog.Infof("OneOf definition missing discriminator. Will treat it as AllOf for resource %s. All input properties will be optional.", resourceName)
+		schemaRefs := resourceRequestType.OneOf
+		for _, schemaRef := range schemaRefs {
+			schemaRef.Value.Required = nil
+		}
+		resourceRequestType.AllOf = schemaRefs
+	}
+
 	resourceTypeToken, err := o.gatherResourceProperties(resourceName, resourceRequestType, resourceResponseType, apiPath, module)
 
 	if err != nil {
@@ -759,6 +777,7 @@ func (o *OpenAPIContext) gatherResourceProperties(resourceName string, requestBo
 				// TODO: Should we add these properties to the required outputs as well?
 			}
 		}
+
 		for propName, prop := range responseBodySchema.Properties {
 			var propSpec pschema.PropertySpec
 
@@ -1425,26 +1444,33 @@ 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 {
-			// If 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.
 			if !strings.HasPrefix(typName, ctx.resourceName) {
-				typName = ctx.resourceName + enumName
-				tok = fmt.Sprintf("%s:%s:%s", ctx.pkg.Name, ctx.mod, typName)
-				referencedTypeName = fmt.Sprintf("#/types/%s", tok)
-			} else {
-				msg := fmt.Sprintf("duplicate enum %q: %+v vs. %+v", tok, enumSpec.Enum, other.Enum)
-				return nil, &duplicateEnumError{msg: msg}
+				// 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.
+				return ctx.genEnumType(ctx.resourceName+enumName, propSchema)
 			}
+
+			// 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}
 		}
 
 		return &pschema.TypeSpec{

pkg/prefix_enum_type_on_collision_test.go

@@ -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)
 }

pkg/resource_inputs_oneof_no_discriminator_test.go

@@ -0,0 +1,41 @@
+package pkg
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestResourceInputsWithOneOfNoDiscriminator tests if
+// resource request bodies with oneOf definitions that
+// don't define a discriminator are treated as allOf
+// with none of the inputs being required.
+func TestResourceInputsWithOneOfNoDiscriminator(t *testing.T) {
+	mustReadTestOpenAPIDoc(t, filepath.Join("testdata", "resource_inputs_oneof_no_discriminator_openapi.yml"))
+
+	openAPICtx := &OpenAPIContext{
+		Doc: *testOpenAPIDoc,
+		Pkg: &testPulumiPkg,
+	}
+
+	csharpNamespaces := map[string]string{
+		"": "Provider",
+	}
+
+	_, _, err := openAPICtx.GatherResourcesFromAPI(csharpNamespaces)
+	assert.Nil(t, err)
+
+	resourceSpec, ok := testPulumiPkg.Resources["fake-package:resource/v2:Resource"]
+	assert.Truef(t, ok, "Expected to find a resource called Resource: %v", testPulumiPkg.Resources)
+
+	// The resource should have all props from all types that was in the oneOf
+	// definition.
+	assert.Equal(t, "string", resourceSpec.InputProperties["simpleProp"].Type)
+	assert.Equal(t, "string", resourceSpec.InputProperties["anotherProp"].Type)
+	// None of them should be required because the API does not
+	// describe a discriminator. It's up to the user to know
+	// which set of inputs are required based on the "type"
+	// of request they would like to send to the API.
+	assert.Empty(t, resourceSpec.Required)
+}

pkg/testdata/prefix_enum_type_on_collision_openapi.yml

@@ -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:
@@ -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:
@@ -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"

pkg/testdata/resource_inputs_oneof_no_discriminator_openapi.yml

@@ -0,0 +1,54 @@
+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"
+      required:
+        - simple_prop
+
+    request_object_type2:
+      type: object
+      properties:
+        another_prop:
+          $ref: "#/components/schemas/a_string_prop"
+      required:
+        - another_prop
+
+  responses:
+    response_object_type:
+      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:
+    post:
+      operationId: create_resource
+      requestBody:
+        content:
+          application/json:
+            schema:
+              oneOf:
+                - $ref: "#/components/schemas/request_object_type"
+                - $ref: "#/components/schemas/request_object_type2"
+      responses:
+        "200":
+          $ref: "#/components/responses/response_object_type"