Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Property added denoted in CommonTypes.v4 didn't get parsed in model using CommonTypes.v5 #1992

Open
4 tasks done
AllyW opened this issue Dec 17, 2024 · 4 comments
Open
4 tasks done
Labels
bug Something isn't working cli/psh Issues for Azure CLI/PSH features needs-area

Comments

@AllyW
Copy link
Member

AllyW commented Dec 17, 2024

Describe the bug

The target tsp is from MongoCluster: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/mongocluster/DocumentDB.MongoCluster.Management/main.tsp#L23

When we try to access the properties from metadata of @typespec/http, we cannot find the property groupId added in v4, while this mongoCluster is using commonTypes.v5.

The lost property isgroupId here:

Reproduction

Our emitter is typespec-aaz here: https://github.com/Azure/aaz-dev-tools/tree/dev/src/typespec-aaz
Please clone the emitter above, link it to local npm env and then provide the tspconfig.yaml as following:

Image

And then, run tsp compile ${your_workspaceFolder}/specification/mongocluster/DocumentDB.MongoCluster.Management/main.tsp -- config yourconfig.yaml and check the output file ./tsp-output/@azure-tools/typespec-aaz/resources_operations.json .

we can check properties of privateEndpointConnections and you'll find privateEndpoint, privateLinkServiceConnectionState and other stuff, except groupIds.

Emitter code line for parsing properties of common types PrivateEndpointConnectionProperties is here: https://github.com/Azure/aaz-dev-tools/blob/dev/src/typespec-aaz/src/convertor.ts#L655. We use metadata from @typespec/http and check all its properties for our aaz model generation.

Checklist

  • Follow our Code of Conduct
  • Check that this issue is about the Azure libraries for typespec. For bug in the typespec language or core libraries file it in the TypeSpec repo
  • Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
  • The provided reproduction is a minimal reproducible example of the bug.
@AllyW AllyW added bug Something isn't working cli/psh Issues for Azure CLI/PSH features labels Dec 17, 2024
@allenjzhang
Copy link
Member

allenjzhang commented Dec 17, 2024

I tried with OpenAPI3 emitter and it correctly filtered out the groupId. Probably missed some processing logic in your emitter. Adding @chrisradek to take a look your emitter code.

Playground links:

V3
V5

@chrisradek
Copy link
Member

I haven't been able to get the emitter to actually emit anything except an empty array following the instructions (I am using wsl if that matters), so can't dig too deeply.

Since the openapi3 emitter is handling this properly, I can point out where projections between the 2 emitters are handled differently.

In the openapi3 emitter, we get the projected service:

const document = await getOpenApiFromVersion(
  projectedServiceNs === projectedProgram.getGlobalNamespaceType()
    ? { type: projectedProgram.getGlobalNamespaceType() }
    : getService(program, projectedServiceNs)!,
  record.version,
);

https://github.com/microsoft/typespec/blob/f76f3182addf6c271e41087ee8fa3f8440af3bcb/packages/openapi3/src/openapi.ts#L505-L510

and then pass that service directly into getHttpService:

const httpService = ignoreDiagnostics(getHttpService(program, service.type));

https://github.com/microsoft/typespec/blob/f76f3182addf6c271e41087ee8fa3f8440af3bcb/packages/openapi3/src/openapi.ts#L632

In your emitter, it looks like you store the original service:

const aazContext: AAZEmitterContext = {
  program: projectedProgram,
  service: service,
  sdkContext: sdkContext,
  apiVersion: apiVersion,
  tracer,
};

https://github.com/Azure/aaz-dev-tools/blob/4de9f1448c931cc691b6edeecfb8ddfdce79c7fb/src/typespec-aaz/src/emit.ts#L115-L121

Though, that service isn't actually used. You do however call getAllHttpServices and grab the 1st one returned:

const services = ignoreDiagnostics(getAllHttpServices(context.program));

https://github.com/Azure/aaz-dev-tools/blob/4de9f1448c931cc691b6edeecfb8ddfdce79c7fb/src/typespec-aaz/src/emit.ts#L138

You are using the projectProgram when making that call, but I wonder if you'd have better results passing in the projected service directly.

Again - I'd give it a try but I can't get your repro steps to emit anything locally :-(

@AllyW
Copy link
Member Author

AllyW commented Dec 18, 2024

Yes I was thinking maybe we didn't use the proper function for parsing models from commonTypes or missed the version somewhere. However, for inline model properties from target tsp file, the versioning part can be taken care of by the compiler by default, only commonTypes properties has this kind of versioning lost issue.

As @allenjzhang and @chrisradek mentioned, will try the one from openapi3 later.

@AllyW
Copy link
Member Author

AllyW commented Dec 18, 2024

During my development period, there often occurs to me that, is there a dev doc or function specification that emitter developer can use when they try to fetch and parse different parts of different models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli/psh Issues for Azure CLI/PSH features needs-area
Projects
None yet
Development

No branches or pull requests

3 participants