Swagger emitter makes up new name for the same model with different schemas

[pshao25]

Background:
We have some customers that already have their typespec. However, our generators still have some limitations so that we cannot support typespec input. So these customers have to generate swagger first from swagger emitter and then call our generator with swagger input. We expect when our generator is ready to accept typespec input, their original typespec generates same SDK, which means the swagger generated from typespec should be equivalent.

Problem:
There are several scenarios under which the model names in the generated swagger are different from original typespec:

1. When model has visibility decorator. playground
2. Models in patch operations. playground You could see the body type changed to AssetEndpointProfileUpdate as well as its nested models.

Expected behavior:
From this additional trial of "a model with all the optional properties in a patch operation", which don't have any made-up name, I could understand the swagger emitter produces new name because of there are more than one schema needed for the same model. But IMO we should keep the name unchanged and figure out another way to handle the different representation of the same model.

[markcowl]

IMO, this is by design. ArmResourcePatch creates a set of schema that are compatible with current RPC guidelines on PATCH. Many legacy APIs used older guidelines, or predated any real enforcement on how PATCH worked. Additionally, there are no controls over how types created from metadata are named (for which there is an existing design issue).

For ARM brownfield APIs, though, most APIs should be using ArmCustomPatch, which allows you to specify your own request body schema, here is an example playground showing this . You may need to change the parameterVisibility for the operation if there are required properties in your PATCH schema.

[XiaofeiCao]

Hi @markcowl ,

If I'm understanding correctly, there's a new guideline of the PATCH model schema name right? May I ask which guideline are you referring to?

On the other hand, if we were to follow this new guideline for all SDKs, I think we should enforce this in some common places, e.g. compiler or TCGC, instead of this implict behavior that exists only in typespec-autorest :(

[allenjzhang]

This problem is not new. Prior to TypeSpec, service teams all have different approaches in swagger, some use the root model while some use update model for Patch. I assume you reproduce that faithfully at that time as well, right?

[tadelesh]

let's use playground as an example, there are two kinds of model separation logics:

1. separate by template, in the example, for ArmResourcePatchSync , we will have a AssetEndpointProfileUpdate model created. this is defined in template and for any emitter, we could get the model from compiler

2. separate by emitter logic, in the example, autorest emitter will generate UserAuthenticationUpdate since `UserAuthentication has required properties that could not be used in update operation. this is implemented by different emitters themselves.

i believe we do not discuss them separately in the meeting. imo, for 1, we should leave as it is. nothing should do for emitter or tcgc. for 2, if we could have consensus across all languages' sdk that we need do it, we could opt it in tcgc, otherwise i don't like such implicitly behavior. and since you all think it is a client behavior, why we do this in openapi and autorest emitter, if not, why http library do it for all emitter?

[microsoft-github-policy-service]

Hello @pshao25,
This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[markcowl]

Clowing this issue in favor of #859