From ee506d24319265de840b2dc552f8f857b6b8402c Mon Sep 17 00:00:00 2001 From: Jonathan Eskew Date: Mon, 13 Jun 2022 12:34:10 -0400 Subject: [PATCH] Prevent ReadOnly/WriteOnly flag cascade on reusable object definitions --- .vscode/launch.json | 13 ++++++- src/autorest.bicep/src/type-generator.ts | 33 ++++++++++++++++-- .../basic/test.rp1/2021-10-31/types.json | 2 +- .../basic/test.rp1/2021-10-31/types.md | 19 ++++++++--- .../Test.Rp1/stable/2021-10-31/spec.json | 34 +++++++++++++++++-- 5 files changed, 89 insertions(+), 12 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 53a81b1cb9..995e2b47ba 100644 --- 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" } ] -} \ No newline at end of file +} diff --git a/src/autorest.bicep/src/type-generator.ts b/src/autorest.bicep/src/type-generator.ts index a81a8d9517..352108bf76 100755 --- 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 index a93599798d..44a851170a 100644 --- 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}}] \ No newline at end of file +[{"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}}] \ No newline at end of file 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 index 1990a56459..a1798eff48 100644 --- 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/src/autorest.bicep/test/integration/specs/basic/resource-manager/Test.Rp1/stable/2021-10-31/spec.json b/src/autorest.bicep/test/integration/specs/basic/resource-manager/Test.Rp1/stable/2021-10-31/spec.json index d4100a350f..5137754063 100644 --- a/src/autorest.bicep/test/integration/specs/basic/resource-manager/Test.Rp1/stable/2021-10-31/spec.json +++ b/src/autorest.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" },