Azure / typespec-azure

About TypeSpec Azure Libraries
https://azure.github.io/typespec-azure/
MIT License
15 stars 42 forks source link

SubscriptionIdParameter is defined as string resulting in generated ProviderHub controller actions to define it as string #637

Open abatishchev opened 7 months ago

abatishchev commented 7 months ago

Clear and concise description of the problem

The ProviderHub template generates the controller actions simply as {{decl operation}}. What results in an action like this:

public async Task<IActionResult> BeginEvacuateAsync(string subscriptionId, string resourceGroupName, string buildingName)

Notice that parameter subscriptionId is defined as a string. However, in Azure it's always a Guid. I think it'd be safe to assume and rely on it to be always a Guid.

The root cause of this is the definition of a common types parameter SubscriptionIdParameter:

https://github.com/Azure/typespec-azure-pr/blob/65444497eab4163997fee25fc49b9359fb9fe8dc/packages/typespec-azure-resource-manager/lib/parameters.tsp#L36-L48

which is defined as string.

Proposal: define it as Azure.Core.uuid instead. However, this change is likely would have widespread consequence, so it should be analyzed and designed carefully.

Checklist

allenjzhang commented 6 days ago

Since Common-Types v4, subscriptionId type has been changed to uuid. So if the ProviderHub codegen does not translate it into Guid for C#, that would be a bug.

To support older version of spec on common-type v3 or prior, it is possible to always apply an augment decorator in ProviderHub library with "@@format(Azure.ResourceManager.CommonType.SubscriptionIdParameter.subscriptionId, "uuid").

abatishchev commented 6 days ago

Hi @allenjzhang, I don't see the type declared in the common types being translated to TypeSpec. Instead, it re-defines it here:

https://github.com/Azure/typespec-azure-pr/blob/65444497eab4163997fee25fc49b9359fb9fe8dc/packages/typespec-azure-resource-manager/lib/parameters.tsp#L36-L48

And re-defines it as string, hence the generated code gets System.String too.

abatishchev commented 6 days ago

Sorry! The previous link points to the previous version.

Here's the current one: https://github.com/Azure/typespec-azure-pr/blob/d4c0e71f059d7636a2e7e0d0c24193a51ead702d/packages/typespec-azure-resource-manager/lib/parameters.tsp#L19

Do you know where's CommonTypes.SubscriptionIdParameter defined?

allenjzhang commented 6 days ago

https://github.com/Azure/typespec-azure/blob/main/packages/typespec-azure-resource-manager/lib/common-types/types.tsp#L513

abatishchev commented 6 days ago

Got it. Thanks!

Then I think I need to update to the latest TypeSpec (once it's published in a couple of weeks, hopefully) and the generated code would get System.Guid instead.