Azure / autorest.java

Extension for AutoRest (https://github.com/Azure/autorest) that generates Java code
MIT License
33 stars 82 forks source link

MPG, common model `ManagedServiceIdentity` defers between `typespec-azure` and `azure-rest-api-specs` #2549

Open XiaofeiCao opened 9 months ago

XiaofeiCao commented 9 months ago

Network analytics: https://github.com/Azure/azure-rest-api-specs/blob/f776434f63fb6505926273db8d4f9a93b75ee4a1/specification/networkanalytics/NetworkAnalytics.Management/main.tsp#L147

@doc("The data product resource.")
@added(Versions.v2023_11_15)
model DataProduct is TrackedResource<DataProductProperties> {
  @doc("The data product resource name")
  @key("dataProductName")
  @segment("dataProducts")
  @path
  @pattern("^[a-z][a-z0-9]*$")
  @minLength(3)
  @maxLength(63)
  name: string;

  ...ManagedServiceIdentity;
}

After spreading, it's a model decorated with @armCommonDefinition: https://github.com/Azure/typespec-azure/blob/ebfe63960277356c611f15b2404a0ae6f2d9e6ed/packages/typespec-azure-resource-manager/lib/arm.foundations.tsp#L363-L377

@armCommonDefinition(
  "ManagedServiceIdentity",
  {
    version: Azure.ResourceManager.CommonTypes.Versions.v4,
    isDefault: true,
  },
  "managedidentity.json"
)
@armCommonDefinition(
  "ManagedServiceIdentity",
  Azure.ResourceManager.CommonTypes.Versions.v5,
  "managedidentity.json"
)
@doc("The properties of the managed service identities assigned to this resource.")
model ManagedIdentityProperties {
  @doc("The Active Directory tenant id of the principal.")
  @visibility("read")
  tenantId?: string;

  @doc("The active directory identifier of this principal.")
  @visibility("read")
  principalId?: string;

  @doc("The type of managed identity assigned to this resource.")
  type: ManagedIdentityType;

  @doc("The identities assigned to this resource by the user.")
  userAssignedIdentities?: Record<UserAssignedIdentity>;
}

typespec-autorest will generate it to refer to the common model: https://github.com/Azure/azure-rest-api-specs/blob/f776434f63fb6505926273db8d4f9a93b75ee4a1/specification/networkanalytics/resource-manager/Microsoft.NetworkAnalytics/stable/2023-11-15/networkanalytics.json#L1617

"identity": {
  "$ref": "../../../../../common-types/resource-management/v4/managedidentity.json#/definitions/ManagedServiceIdentity",
  "description": "The managed service identities assigned to this resource."
}

Their name and properties defer:

  1. model name: ManagedServiceIdentity(swagger) vs ManagedIdentityProperties(tsp). ManagedIdentityProperties has @armCommonDefinition, maybe it can be solved by getting the decorator value.
  2. property type:ManagedServiceIdentityType(swagger) vs ManagedIdentityType(tsp) ManagedIdentityType doesn't have @armCommonDefinition on it.. This one's trickier.

Potential solution

  1. We ask typespec-azure-resource-manager to fix the mismatched model property(ManagedServiceIdentityType instead of ManagedIdentityType). Reason: Why define the model and its properties if it's properties are wrong? Con: Each version(e.g. v4, v5) may have common models different from each other.
  2. We replace the model with the common referenced one(like m4 did). Reason: Totally align with current swagger. Con: Needs more effort. Do we want to implement it ourselves? Or do we do it in TCGC?
  3. Hard code the property names, make it predefined models in codegen. Reason: Easiest way. Con: same as 1.
XiaofeiCao commented 5 months ago

Current discrepancy: https://github.com/Azure/autorest.java/issues/2766