Skip to content

Storage Blob generated TypeSpec issue tracker #34403

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

Open
alzimmermsft opened this issue May 1, 2025 · 2 comments
Open

Storage Blob generated TypeSpec issue tracker #34403

alzimmermsft opened this issue May 1, 2025 · 2 comments
Assignees
Labels
TypeSpec Authored with TypeSpec

Comments

@alzimmermsft
Copy link
Member

Creating this issue to track discrepancies found between Storage Blob generated using Swagger and TypeSpec. These could be issues with either the TypeSpec definition itself or with differences between Swagger and TypeSpec code generation for language code generators.

op StorageOperation is defined with RequestMediaType extends string = "application/xml" which flows through to many of the APIs extending it. This results in operations that don't have request bodies, such as op acquireLease is StorageOperation to generate with Content-Type: application/xml in the request. This operation doesn't have a body, and in Swagger code generation there isn't a Content-Type header sent to the service.

The TypeSpec includes auth ApiKeyAuth<ApiKeyLocation.header, "api-key">. Storage Blob doesn't have header api-key authentication, only SAS query parameter auth, shared key auth, and Managed Identity. This results in generated clients stating the service can be authorized with an api-key which isn't true.

op getProperties is StorageOperation is generated with Accept: application/octet-stream and Content-Type: application/json, neither of these can be true as the HTTP operation is a head operation which cannot have a request body nor response body.

OPINION: The namespace determines the name of the client generated. With Blobs being the top-level namespace for service methods, all service methods end up in a client called Blobs which differs greatly from Swagger where those APIs were in a Service client. The names for Blob clients are prepended with Container, ContainerBlob, ContainerAppendBlob, etc. Would be nice for the names to align more closely with what they were in Swagger to ease migration and reduce noise.

@alzimmermsft alzimmermsft added the TypeSpec Authored with TypeSpec label May 1, 2025
@catalinaperalta
Copy link
Member

catalinaperalta commented May 6, 2025

Starting to reply to some of these issues:

op StorageOperation is defined with RequestMediaType extends string = "application/xml" which flows through to many of the APIs extending it. This results in operations that don't have request bodies, such as op acquireLease is StorageOperation to generate with Content-Type: application/xml in the request. This operation doesn't have a body, and in Swagger code generation there isn't a Content-Type header sent to the service.

So, in the previous swagger the global consumes/produces what set to application/xml which is basically what we're doing through that template. I'm wondering if the emitter should handle these cases, where if it doesnt see a body then it doesnt set a content type? I'm not sure we need typespec to be smart enough to do that for us...

The TypeSpec includes auth ApiKeyAuth<ApiKeyLocation.header, "api-key">. Storage Blob doesn't have header api-key authentication, only SAS query parameter auth, shared key auth, and Managed Identity. This results in generated clients stating the service can be authorized with an api-key which isn't true.

Removed.

op getProperties is StorageOperation is generated with Accept: application/octet-stream and Content-Type: application/json, neither of these can be true as the HTTP operation is a head operation which cannot have a request body nor response body.

Is this the only operation that you're seeing return application/json? Also, for application/octet-stream we were just basing ourselves on the original getProperties method defined in the swagger, but I see your point. @vincenttran-msft / @jhendrixMSFT is it ok to remove application/octet-stream from the getProperties method?

OPINION: The namespace determines the name of the client generated. With Blobs being the top-level namespace for service methods, all service methods end up in a client called Blobs which differs greatly from Swagger where those APIs were in a Service client. The names for Blob clients are prepended with Container, ContainerBlob, ContainerAppendBlob, etc. Would be nice for the names to align more closely with what they were in Swagger to ease migration and reduce noise.

We've purposefully structured the namespaces in this spec to more accurately resemble the actual service structure, so I do expect we'll have some differences with how the original swagger was written. You can customize the client structure in client.tsp if you'd like to get something closer to what you were getting originally and scoping it down to "java" if it's very different from what other languages want. There is already some client configuration that might be worth trying to generate from to see what you get in java.

@jhendrixMSFT
Copy link
Member

the emitter should handle these cases, where if it doesnt see a body then it doesnt set a content type?

IMO no, this shouldn't be pushed to the emitters, as then each emitter is required to implement and maintain this magic. Ideally the tsp should be fixed to accurately model the operations.

is it ok to remove application/octet-stream from the getProperties method?

Yes. I'm surprised that tsp allows this. We should follow up on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TypeSpec Authored with TypeSpec
Projects
None yet
Development

No branches or pull requests

3 participants