Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[MPG] Empty models in tsp, what should we generate #2779

Closed
XiaofeiCao opened this issue May 22, 2024 · 4 comments · Fixed by #2859
Closed

[MPG] Empty models in tsp, what should we generate #2779

XiaofeiCao opened this issue May 22, 2024 · 4 comments · Fixed by #2859
Assignees
Labels
Mgmt This issue is related to a management-plane library.

Comments

@XiaofeiCao
Copy link
Contributor

XiaofeiCao commented May 22, 2024

tsp: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/devopsinfrastructure/Microsoft.DevOpsInfrastructure/main.tsp#L314-L315

converted swagger: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/devopsinfrastructure/resource-manager/Microsoft.DevOpsInfrastructure/preview/2024-04-04-preview/devopsinfrastructure.json#L1511-L1514

TCGC issue: Azure/typespec-azure#846
copied from the issue:

Swagger m4 TypeSpec TCGC
{"type": "object"} anyObject {} anonymous model with no properties
{"type": "object"} anyObject model EmptyModel {} named model with no properties
{} any unknown any

Considerations:
We probably need to check what code-model would it be, if it is

  1. {} from tsp
  2. model Model {} from tsp
    Is it only anonymous model vs named model? Do we want to generate them both as empty class (DPG would do this)?

And can we distinguish this "empty model" from typespec, from e.g. "type": "object" from Swagger?

@XiaofeiCao XiaofeiCao added the Mgmt This issue is related to a management-plane library. label May 22, 2024
@XiaofeiCao XiaofeiCao self-assigned this May 22, 2024
@XiaofeiCao
Copy link
Contributor Author

XiaofeiCao commented Jul 11, 2024

Is it only anonymous model vs named model?

From code-model point of view, it is. The generated code-model is the same. Though TCGC's result contains isGeneratedName for SdkModelType:
Screenshot 2024-07-11 at 18 31 49

And can we distinguish this "empty model" from typespec, from e.g. "type": "object" from Swagger?

I'm not sure if TCGC has a way to represent anyObject in M4. If it has, we can modify our old M4 logic to detect anyObject(instead of treating it as AnySchema). If not, guess we can't.

Likely they will use {} to represent anyObject: Azure/typespec-azure#846 (comment). This case, we probably want to generate empty model for named model, but not for anonymous model {}. Need to confirm with TCGC.

@XiaofeiCao
Copy link
Contributor Author

Discussion here: Azure/typespec-azure#846 (comment)

  1. Empty model in TypeSpec means model without properties, not anyObject.
  2. For brownfields, by default we convert anyObject as TypeSpec (anonymous)empty model. If service really wants anyObject, TypeSpec's equivalence is Record<unknown>.

@weidongxu-microsoft Guess for TypeSpec greenfield in MPG, we treat anonymous/named empty model as empty class. While for brownfields(since by default they convert anyObject to anonymous empty model), we treat them as Object to avoid breaking change?

@weidongxu-microsoft
Copy link
Member

I think for tsp, empty model generate empty class.

Compatibility with brownfield is another topic. And we may not need exact same result (one reason is that m4 don't return empty model in the first place, and we likely won't solve that).

@XiaofeiCao
Copy link
Contributor Author

Makes sense.
Guess we first extend ObjectMapper in TypeSpec:

return !JavaSettings.getInstance().isDataPlaneClient()

And do a case by case for brownfields in the future.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Mgmt This issue is related to a management-plane library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants