Azure / Azure/typespec-azure

When typespec-autorest emitter includes default values in PATCH APIs

Open
#1,586 3 comments 0 reactions 0 assignees View on GitHub
emitter:autorest feature
Dominant language
TypeScript
Stars
27
Forks
90
Avg merge
1d 22h
Merged PRs (30d)
156

Description

If all of the properties of a model are optional (`?`) and using `ArmResourcePatchSync`/`Async`, the emitter will not define and emit a new Update model to represent the PATCH operation. This results in what I think is improper spec (or, at least, [azure-openapi-validator](https://github.com/Azure/azure-openapi-validator) complains about it).

[Playground Link](https://cadlplayground.z22.web.core.windows.net/cadl-azure/?options=%7B%22linterRuleSet%22%3A%7B%22extends%22%3A%5B%22%40azure-tools%2Ftypespec-azure-rulesets%2Fresource-manager%22%5D%7D%7D&c=aW1wb3J0ICJAdHlwZXNwZWMvaHR0cCI7CtIZcmVzdNUZdmVyc2lvbmluZ8wfYXp1cmUtdG9vbHMvyCstxhVjb3Jl3yvIK3Jlc291cmNlLW1hbmFnZXIiOwoKdXNpbmcgVHlwZVNwZWMuSHR0cDvQFVJlc3TRFVbpAKnIG0HEZS5Db3JlzhJSx3dNxnY7CgovKiogQ29udG9zbyDIHiBQcm92aWRlciDmAJ5tZW50IEFQSS4gKi8KQGFybcggTmFtZXNwYWNlCkBzZXJ2aWNlKHsKICB0aXRsZTogIsdYyC5IdWJDbGllbnQiLAp9KQpA5wFgZWQo5wDBcykKbshSIE1pY3Jvc29mdC7SR%2B8AuEFQSSDHTXPkAKNlbnVtIMhUIOQAksQuMjAyMS0xMC0wMS1wcmV2aWV3yDXENCAgQHVzZURlcGVuZGVuY3ko9QEx6AFrcy52MV8wX1DGSF8xKcRAYXJtQ29tbW9u5AGZc8cq10jLKctUNcRIYPIAqWAsCn3mAPNB6AD16wEOIOgCTeQAyG1vZGVsIEVtcGxveWVlIGlzIFRyYWNrZWToAII8yBxQcm9wZXJ0aWVzPuUBJC4u6QCm5AHYUGFyYW1ldGVyyTE%2BO8Ym5gDEZFPmAfJJZGVudGl0ecdPeTvoAKvINyBwyWnyAKHHNsQc6AGoQWdlIG9mIGXIP%2BYBnXZpc2liaWxpdHkoImNyZWF0ZSIsICJyZWFkIuQBNWFnZT86IGludDMyID0gMjE7CsdOQ2l0ed9P009jaXR5Pzogc3Ry5AM8PSAiUmVkbW9uZOQDp8ZYVGhlIHN0YXR1c8RedGhlIGxhc3Qg5ADOYXRpb27lAyzPaclfcOQBvecDs1N0YXTkALzlAdHMFOkBScRrzDLFeOUBJGHpAfvFb0Bscm%2FEO3VzCnVu5ALY0VTlAWXmANYs7ADKyEcg5gEMIHJlcXVlc3QgaGFzIGJlZW4gYWNjZXB0ZWTEZyAgQccOOiAiyAsi1lBpxEDkALTpAMHIROwAnDogIswP2kx1cGRhdMRPxUNVxw46ICLIC8o76QTN6QDE5gDcZOcBmlN1Y2NlZWTlAMXJDNM%2FxTbkAU1mYWlsyT5GxQ06ICLGCdw4d2FzIGNhbmNlyj5Dxw%2FkBSTHC%2F8BQCBkZWxl6QGARMQN5gD5yAvkBVQKCmludGVyZuQFP0%2FoAolzIGV4dGVuZHP2BgMuyyl7fQrlBd%2FII8oby1noA7rmA7HlAZHlAKdByjhQYXRjaFN5bmPpBDss8wPxPuQC4Q%3D%3D&e=%40azure-tools%2Ftypespec-autorest)

In the above playground, the patch expects the `EmployeeUpdate` which contains `EmployeeProperties` and the `ManagedServiceIdentityUpdate`. However, the properties of `EmployeeProperties` are all create/read-only properties. Removing the nullability of one of the properties--say, age--will instead prompt the emittance of `EmployeeUpdate` containing `EmployeePropertiesUpdate`, which in turn is defined with no properties (as nothing from `EmployeeProperties` should be able to be updated.

Not sure if this improper definition by us, though. We're running into [RPC-Patch-V1-02](https://github.com/Azure/azure-openapi-validator/blob/main/docs/un-supported-patch-properties.md) and [RPC-Patch-V1-10](https://github.com/Azure/azure-openapi-validator/blob/main/docs/patch-body-parameters-schema.md) because of this.

Contributor guide

Open the contributing guide

Research direction

Start with the linked Playground reproduction and trace the ArmResourcePatchSync/Async emitter behavior for models whose properties are all optional or create/read-only. Compare the generated PATCH schemas with RPC-Patch-V1-02 and RPC-Patch-V1-10, and consider the issue done when the reproduction no longer triggers those validator complaints.

Written by the indexing model from the issue text.

Assessment

Tech stack
azure, openapi, typescript
Domain
api, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.