Prevent ReadOnly/WriteOnly flag cascade on reusable object definitions

Azure · Jun 13, 2022 · ee506d2 · ee506d2
1 parent 6bc73cd
commit ee506d2
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 12 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -35,6 +35,17 @@
             "cwd": "${workspaceFolder}/src/generator",
             "preLaunchTask": "build autorest.bicep"
         },
+        {
+            "name": "Test autorest.bicep",
+            "type": "node",
+            "request": "launch",
+            "runtimeExecutable": "npm",
+            "runtimeArgs": [
+                "test"
+            ],
+            "cwd": "${workspaceFolder}/src/autorest.bicep",
+            "preLaunchTask": "build autorest.bicep"
+        },
         {
             "type": "node",
             "request": "attach",
@@ -50,4 +61,4 @@
             "description": "Pick a specific base path to generate types for"
         }
     ]
-}
+}
diff --git a/src/autorest.bicep/src/type-generator.ts b/src/autorest.bicep/src/type-generator.ts
@@ -417,9 +417,29 @@ export function generateTypes(host: AutorestExtensionHost, definition: ProviderD
     }
   }
 
+  function getObjectName(putSchema: ObjectSchema | undefined, getSchema: ObjectSchema | undefined) {
+    const putName = putSchema ? getSerializedName(putSchema) : undefined;
+    const getName = getSchema ? getSerializedName(getSchema) : undefined;
+
+    if (putSchema) {
+      if (getSchema) {
+        if (putName !== getName) {
+          return {
+            syntheticObject: true,
+            definitionName: `${putName}Or${getName}`,
+          };
+        }
+      }
+
+      return {syntheticObject: false, definitionName: putName};
+    }
+
+    return {syntheticObject: false, definitionName: getName};
+  }
+
   function parseObjectType(putSchema: ObjectSchema | undefined, getSchema: ObjectSchema | undefined, includeBaseProperties: boolean) {
     const combinedSchema = combineAndThrowIfNull(putSchema, getSchema);
-    const definitionName = getSerializedName(combinedSchema);
+    const {syntheticObject, definitionName} = getObjectName(putSchema, getSchema);
 
     if (includeBaseProperties && namedDefinitions[definitionName]) {
       // if we're building a discriminated subtype, we're going to be missing the base properties
@@ -444,7 +464,14 @@ export function generateTypes(host: AutorestExtensionHost, definition: ProviderD
       namedDefinitions[definitionName] = definition;
     }
 
-    for (const { propertyName, putProperty, getProperty } of getObjectTypeProperties(putSchema, getSchema, includeBaseProperties)) {
+    // Only make a distinction between what's defined on PUT vs GET if we're dealing with a synthetic object.
+    // If the schema on both PUT and GET is the same named object (or if one of the two is undefined),
+    // use the combined schema as both GET and PUT schemata to prevent ReadOnly/WriteOnly flags from trickling down
+    // to object properties (which is problematic if shapes are reused across resources)
+    const schemaForPut = syntheticObject ? putSchema : combinedSchema;
+    const schemaForGet = syntheticObject ? getSchema : combinedSchema;
+
+    for (const { propertyName, putProperty, getProperty } of getObjectTypeProperties(schemaForPut, schemaForGet, includeBaseProperties)) {
       const propertyDefinition = parseType(putProperty?.schema, getProperty?.schema);
       if (propertyDefinition) {
         const description = (putProperty?.schema ?? getProperty?.schema)?.language.default?.description;
@@ -456,7 +483,7 @@ export function generateTypes(host: AutorestExtensionHost, definition: ProviderD
     if (combinedSchema.discriminator) {
       const discriminatedObjectType = factory.lookupType(definition) as DiscriminatedObjectType;
 
-      handlePolymorphicType(discriminatedObjectType, putSchema, getSchema);
+      handlePolymorphicType(discriminatedObjectType, schemaForPut, schemaForGet);
     }
 
     return definition;

diff --git a/src/autorest.bicep/test/integration/generated/basic/test.rp1/2021-10-31/types.json b/src/autorest.bicep/test/integration/generated/basic/test.rp1/2021-10-31/types.json
@@ -1 +1 @@
-[{"1":{"Kind":1}},{"1":{"Kind":2}},{"1":{"Kind":3}},{"1":{"Kind":4}},{"1":{"Kind":5}},{"1":{"Kind":6}},{"1":{"Kind":7}},{"1":{"Kind":8}},{"6":{"Value":"Test.Rp1/testType1"}},{"6":{"Value":"2021-10-31"}},{"2":{"Name":"Test.Rp1/testType1","Properties":{"id":{"Type":4,"Flags":10,"Description":"The resource id"},"name":{"Type":4,"Flags":9,"Description":"The resource name"},"type":{"Type":8,"Flags":10,"Description":"The resource type"},"apiVersion":{"Type":9,"Flags":10,"Description":"The resource api version"},"properties":{"Type":11,"Flags":0},"tags":{"Type":20,"Flags":0,"Description":"Resource tags."},"location":{"Type":4,"Flags":1,"Description":"The geo-location where the resource lives"},"systemData":{"Type":21,"Flags":2,"Description":"Metadata pertaining to creation and last modification of the resource."}}}},{"2":{"Name":"TestType1Properties","Properties":{"basicString":{"Type":4,"Flags":0,"Description":"Description for a basic string property."},"stringEnum":{"Type":14,"Flags":0,"Description":"Description for a basic enum property."},"skuTier":{"Type":19,"Flags":0,"Description":"This field is required to be implemented by the Resource Provider if the service has more than one tier, but is not required on a PUT."}}}},{"6":{"Value":"Foo"}},{"6":{"Value":"Bar"}},{"5":{"Elements":[12,13,4]}},{"6":{"Value":"Free"}},{"6":{"Value":"Basic"}},{"6":{"Value":"Standard"}},{"6":{"Value":"Premium"}},{"5":{"Elements":[15,16,17,18]}},{"2":{"Name":"TrackedResourceTags","Properties":{},"AdditionalProperties":4}},{"2":{"Name":"SystemData","Properties":{"createdBy":{"Type":4,"Flags":0,"Description":"The identity that created the resource."},"createdByType":{"Type":26,"Flags":0,"Description":"The type of identity that created the resource."},"createdAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource creation (UTC)."},"lastModifiedBy":{"Type":4,"Flags":0,"Description":"The identity that last modified the resource."},"lastModifiedByType":{"Type":31,"Flags":0,"Description":"The type of identity that created the resource."},"lastModifiedAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource last modification (UTC)"}}}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[22,23,24,25,4]}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[27,28,29,30,4]}},{"4":{"Name":"Test.Rp1/testType1@2021-10-31","ScopeType":8,"Body":10}},{"2":{"Name":"FoosRequest","Properties":{"someString":{"Type":4,"Flags":5,"Description":"The foo request string"}}}},{"2":{"Name":"FoosResponse","Properties":{"someString":{"Type":4,"Flags":2,"Description":"The foo response string"}}}},{"8":{"Name":"listFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":34,"Input":33}},{"3":{"ItemType":34}},{"8":{"Name":"listArrayOfFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":36}}]
+[{"1":{"Kind":1}},{"1":{"Kind":2}},{"1":{"Kind":3}},{"1":{"Kind":4}},{"1":{"Kind":5}},{"1":{"Kind":6}},{"1":{"Kind":7}},{"1":{"Kind":8}},{"6":{"Value":"Test.Rp1/testType1"}},{"6":{"Value":"2021-10-31"}},{"2":{"Name":"Test.Rp1/testType1","Properties":{"id":{"Type":4,"Flags":10,"Description":"The resource id"},"name":{"Type":4,"Flags":9,"Description":"The resource name"},"type":{"Type":8,"Flags":10,"Description":"The resource type"},"apiVersion":{"Type":9,"Flags":10,"Description":"The resource api version"},"properties":{"Type":11,"Flags":0},"tags":{"Type":21,"Flags":0,"Description":"Resource tags."},"location":{"Type":4,"Flags":1,"Description":"The geo-location where the resource lives"},"systemData":{"Type":22,"Flags":2,"Description":"Metadata pertaining to creation and last modification of the resource."}}}},{"2":{"Name":"TestType1CreateOrUpdatePropertiesOrTestType1Properties","Properties":{"basicString":{"Type":4,"Flags":0,"Description":"Description for a basic string property."},"stringEnum":{"Type":14,"Flags":0,"Description":"Description for a basic enum property."},"skuTier":{"Type":19,"Flags":0,"Description":"This field is required to be implemented by the Resource Provider if the service has more than one tier, but is not required on a PUT."},"locationData":{"Type":20,"Flags":2,"Description":"Metadata pertaining to the geographic location of the resource."}}}},{"6":{"Value":"Foo"}},{"6":{"Value":"Bar"}},{"5":{"Elements":[12,13,4]}},{"6":{"Value":"Free"}},{"6":{"Value":"Basic"}},{"6":{"Value":"Standard"}},{"6":{"Value":"Premium"}},{"5":{"Elements":[15,16,17,18]}},{"2":{"Name":"LocationData","Properties":{"name":{"Type":4,"Flags":1,"Description":"A canonical name for the geographic or physical location."},"city":{"Type":4,"Flags":0,"Description":"The city or locality where the resource is located."},"district":{"Type":4,"Flags":0,"Description":"The district, state, or province where the resource is located."},"countryOrRegion":{"Type":4,"Flags":0,"Description":"The country or region where the resource is located"}}}},{"2":{"Name":"TrackedResourceTags","Properties":{},"AdditionalProperties":4}},{"2":{"Name":"SystemData","Properties":{"createdBy":{"Type":4,"Flags":0,"Description":"The identity that created the resource."},"createdByType":{"Type":27,"Flags":0,"Description":"The type of identity that created the resource."},"createdAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource creation (UTC)."},"lastModifiedBy":{"Type":4,"Flags":0,"Description":"The identity that last modified the resource."},"lastModifiedByType":{"Type":32,"Flags":0,"Description":"The type of identity that created the resource."},"lastModifiedAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource last modification (UTC)"}}}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[23,24,25,26,4]}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[28,29,30,31,4]}},{"4":{"Name":"Test.Rp1/testType1@2021-10-31","ScopeType":8,"Body":10}},{"2":{"Name":"FoosRequest","Properties":{"someString":{"Type":4,"Flags":1,"Description":"The foo request string"},"locationData":{"Type":20,"Flags":0,"Description":"Metadata pertaining to the geographic location of the resource."}}}},{"2":{"Name":"FoosResponse","Properties":{"someString":{"Type":4,"Flags":0,"Description":"The foo response string"}}}},{"8":{"Name":"listFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":35,"Input":34}},{"3":{"ItemType":35}},{"8":{"Name":"listArrayOfFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":37}}]
diff --git a/src/autorest.bicep/test/integration/generated/basic/test.rp1/2021-10-31/types.md b/src/autorest.bicep/test/integration/generated/basic/test.rp1/2021-10-31/types.md
@@ -7,7 +7,7 @@
 * **id**: string (ReadOnly, DeployTimeConstant): The resource id
 * **location**: string (Required): The geo-location where the resource lives
 * **name**: string (Required, DeployTimeConstant): The resource name
-* **properties**: [TestType1Properties](#testtype1properties)
+* **properties**: [TestType1CreateOrUpdatePropertiesOrTestType1Properties](#testtype1createorupdatepropertiesortesttype1properties)
 * **systemData**: [SystemData](#systemdata) (ReadOnly): Metadata pertaining to creation and last modification of the resource.
 * **tags**: [TrackedResourceTags](#trackedresourcetags): Resource tags.
 * **type**: 'Test.Rp1/testType1' (ReadOnly, DeployTimeConstant): The resource type
@@ -25,15 +25,23 @@
 
 ## FoosRequest
 ### Properties
-* **someString**: string (Required, WriteOnly): The foo request string
+* **locationData**: [LocationData](#locationdata): Metadata pertaining to the geographic location of the resource.
+* **someString**: string (Required): The foo request string
 
 ## FoosResponse
 ### Properties
-* **someString**: string (ReadOnly): The foo response string
+* **someString**: string: The foo response string
 
 ## FoosResponse
 ### Properties
-* **someString**: string (ReadOnly): The foo response string
+* **someString**: string: The foo response string
+
+## LocationData
+### Properties
+* **city**: string: The city or locality where the resource is located.
+* **countryOrRegion**: string: The country or region where the resource is located
+* **district**: string: The district, state, or province where the resource is located.
+* **name**: string (Required): A canonical name for the geographic or physical location.
 
 ## SystemData
 ### Properties
@@ -44,9 +52,10 @@
 * **lastModifiedBy**: string: The identity that last modified the resource.
 * **lastModifiedByType**: 'Application' | 'Key' | 'ManagedIdentity' | 'User' | string: The type of identity that created the resource.
 
-## TestType1Properties
+## TestType1CreateOrUpdatePropertiesOrTestType1Properties
 ### Properties
 * **basicString**: string: Description for a basic string property.
+* **locationData**: [LocationData](#locationdata) (ReadOnly): Metadata pertaining to the geographic location of the resource.
 * **skuTier**: 'Basic' | 'Free' | 'Premium' | 'Standard': This field is required to be implemented by the Resource Provider if the service has more than one tier, but is not required on a PUT.
 * **stringEnum**: 'Bar' | 'Foo' | string: Description for a basic enum property.
 

diff --git a/....bicep/test/integration/specs/basic/resource-manager/Test.Rp1/stable/2021-10-31/spec.json b/....bicep/test/integration/specs/basic/resource-manager/Test.Rp1/stable/2021-10-31/spec.json
@@ -43,7 +43,22 @@
       "type": "object",
       "description": "The testType1 resource."
     },
-    "TestType1Properties": {
+    "TestType1Input": {
+      "allOf": [
+        {
+          "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/TrackedResource"
+        }
+      ],
+      "type": "object",
+      "properties": {
+        "properties": {
+          "$ref": "#/definitions/TestType1CreateOrUpdateProperties",
+          "description": "The resource properties.",
+          "x-ms-client-flatten": true
+        }
+      }
+    },
+    "TestType1CreateOrUpdateProperties": {
       "properties": {
         "basicString": {
           "type": "string",
@@ -77,6 +92,18 @@
         }
       }
     },
+    "TestType1Properties": {
+      "allOf": [
+        {
+          "$ref": "#/definitions/TestType1CreateOrUpdateProperties"
+        }
+      ],
+      "properties": {
+        "locationData": {
+          "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/locationData"
+        }
+      }
+    },
     "FoosResponse": {
       "properties": {
         "someString": {
@@ -90,6 +117,9 @@
         "someString": {
           "type": "string",
           "description": "The foo request string"
+        },
+        "locationData": {
+          "$ref": "../../../../../common-types/resource-management/v3/types.json#/definitions/locationData"
         }
       },
       "required": [
@@ -200,7 +230,7 @@
             "in": "body",
             "required": true,
             "schema": {
-              "$ref": "#/definitions/TestType1"
+              "$ref": "#/definitions/TestType1Input"
             },
             "description": "The request parameters"
           },
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		[{"1":{"Kind":1}},{"1":{"Kind":2}},{"1":{"Kind":3}},{"1":{"Kind":4}},{"1":{"Kind":5}},{"1":{"Kind":6}},{"1":{"Kind":7}},{"1":{"Kind":8}},{"6":{"Value":"Test.Rp1/testType1"}},{"6":{"Value":"2021-10-31"}},{"2":{"Name":"Test.Rp1/testType1","Properties":{"id":{"Type":4,"Flags":10,"Description":"The resource id"},"name":{"Type":4,"Flags":9,"Description":"The resource name"},"type":{"Type":8,"Flags":10,"Description":"The resource type"},"apiVersion":{"Type":9,"Flags":10,"Description":"The resource api version"},"properties":{"Type":11,"Flags":0},"tags":{"Type":20,"Flags":0,"Description":"Resource tags."},"location":{"Type":4,"Flags":1,"Description":"The geo-location where the resource lives"},"systemData":{"Type":21,"Flags":2,"Description":"Metadata pertaining to creation and last modification of the resource."}}}},{"2":{"Name":"TestType1Properties","Properties":{"basicString":{"Type":4,"Flags":0,"Description":"Description for a basic string property."},"stringEnum":{"Type":14,"Flags":0,"Description":"Description for a basic enum property."},"skuTier":{"Type":19,"Flags":0,"Description":"This field is required to be implemented by the Resource Provider if the service has more than one tier, but is not required on a PUT."}}}},{"6":{"Value":"Foo"}},{"6":{"Value":"Bar"}},{"5":{"Elements":[12,13,4]}},{"6":{"Value":"Free"}},{"6":{"Value":"Basic"}},{"6":{"Value":"Standard"}},{"6":{"Value":"Premium"}},{"5":{"Elements":[15,16,17,18]}},{"2":{"Name":"TrackedResourceTags","Properties":{},"AdditionalProperties":4}},{"2":{"Name":"SystemData","Properties":{"createdBy":{"Type":4,"Flags":0,"Description":"The identity that created the resource."},"createdByType":{"Type":26,"Flags":0,"Description":"The type of identity that created the resource."},"createdAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource creation (UTC)."},"lastModifiedBy":{"Type":4,"Flags":0,"Description":"The identity that last modified the resource."},"lastModifiedByType":{"Type":31,"Flags":0,"Description":"The type of identity that created the resource."},"lastModifiedAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource last modification (UTC)"}}}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[22,23,24,25,4]}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[27,28,29,30,4]}},{"4":{"Name":"Test.Rp1/testType1@2021-10-31","ScopeType":8,"Body":10}},{"2":{"Name":"FoosRequest","Properties":{"someString":{"Type":4,"Flags":5,"Description":"The foo request string"}}}},{"2":{"Name":"FoosResponse","Properties":{"someString":{"Type":4,"Flags":2,"Description":"The foo response string"}}}},{"8":{"Name":"listFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":34,"Input":33}},{"3":{"ItemType":34}},{"8":{"Name":"listArrayOfFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":36}}]
		[{"1":{"Kind":1}},{"1":{"Kind":2}},{"1":{"Kind":3}},{"1":{"Kind":4}},{"1":{"Kind":5}},{"1":{"Kind":6}},{"1":{"Kind":7}},{"1":{"Kind":8}},{"6":{"Value":"Test.Rp1/testType1"}},{"6":{"Value":"2021-10-31"}},{"2":{"Name":"Test.Rp1/testType1","Properties":{"id":{"Type":4,"Flags":10,"Description":"The resource id"},"name":{"Type":4,"Flags":9,"Description":"The resource name"},"type":{"Type":8,"Flags":10,"Description":"The resource type"},"apiVersion":{"Type":9,"Flags":10,"Description":"The resource api version"},"properties":{"Type":11,"Flags":0},"tags":{"Type":21,"Flags":0,"Description":"Resource tags."},"location":{"Type":4,"Flags":1,"Description":"The geo-location where the resource lives"},"systemData":{"Type":22,"Flags":2,"Description":"Metadata pertaining to creation and last modification of the resource."}}}},{"2":{"Name":"TestType1CreateOrUpdatePropertiesOrTestType1Properties","Properties":{"basicString":{"Type":4,"Flags":0,"Description":"Description for a basic string property."},"stringEnum":{"Type":14,"Flags":0,"Description":"Description for a basic enum property."},"skuTier":{"Type":19,"Flags":0,"Description":"This field is required to be implemented by the Resource Provider if the service has more than one tier, but is not required on a PUT."},"locationData":{"Type":20,"Flags":2,"Description":"Metadata pertaining to the geographic location of the resource."}}}},{"6":{"Value":"Foo"}},{"6":{"Value":"Bar"}},{"5":{"Elements":[12,13,4]}},{"6":{"Value":"Free"}},{"6":{"Value":"Basic"}},{"6":{"Value":"Standard"}},{"6":{"Value":"Premium"}},{"5":{"Elements":[15,16,17,18]}},{"2":{"Name":"LocationData","Properties":{"name":{"Type":4,"Flags":1,"Description":"A canonical name for the geographic or physical location."},"city":{"Type":4,"Flags":0,"Description":"The city or locality where the resource is located."},"district":{"Type":4,"Flags":0,"Description":"The district, state, or province where the resource is located."},"countryOrRegion":{"Type":4,"Flags":0,"Description":"The country or region where the resource is located"}}}},{"2":{"Name":"TrackedResourceTags","Properties":{},"AdditionalProperties":4}},{"2":{"Name":"SystemData","Properties":{"createdBy":{"Type":4,"Flags":0,"Description":"The identity that created the resource."},"createdByType":{"Type":27,"Flags":0,"Description":"The type of identity that created the resource."},"createdAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource creation (UTC)."},"lastModifiedBy":{"Type":4,"Flags":0,"Description":"The identity that last modified the resource."},"lastModifiedByType":{"Type":32,"Flags":0,"Description":"The type of identity that created the resource."},"lastModifiedAt":{"Type":4,"Flags":0,"Description":"The timestamp of resource last modification (UTC)"}}}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[23,24,25,26,4]}},{"6":{"Value":"User"}},{"6":{"Value":"Application"}},{"6":{"Value":"ManagedIdentity"}},{"6":{"Value":"Key"}},{"5":{"Elements":[28,29,30,31,4]}},{"4":{"Name":"Test.Rp1/testType1@2021-10-31","ScopeType":8,"Body":10}},{"2":{"Name":"FoosRequest","Properties":{"someString":{"Type":4,"Flags":1,"Description":"The foo request string"},"locationData":{"Type":20,"Flags":0,"Description":"Metadata pertaining to the geographic location of the resource."}}}},{"2":{"Name":"FoosResponse","Properties":{"someString":{"Type":4,"Flags":0,"Description":"The foo response string"}}}},{"8":{"Name":"listFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":35,"Input":34}},{"3":{"ItemType":35}},{"8":{"Name":"listArrayOfFoos","ResourceType":"Test.Rp1/testType1","ApiVersion":"2021-10-31","Output":37}}]