Azure / autorest.powershell

AutoRest PowerShell Generator
MIT License
113 stars 86 forks source link

incorrect parsing and singularization of paths when generating subjects from OData #775

Open markwahl-msft opened 3 years ago

markwahl-msft commented 3 years ago

The Azure AD feature Terms of Use has as one of its resource surfaced in OData, agreement. It appears in CSDL as

<EntityType Name="identityGovernance">
        <NavigationProperty Name="termsOfUse" Type="graph.termsOfUseContainer" ContainsTarget="true" />
</EntityType>
<EntityType Name="termsOfUseContainer" BaseType="graph.entity">
        <NavigationProperty Name="agreements" Type="Collection(graph.agreement)" ContainsTarget="true" />
</EntityType>

and YAML as

    microsoft.graph.identityGovernance:
      title: identityGovernance
      type: object
      properties:
        termsOfUse:
          $ref: '#/components/schemas/microsoft.graph.termsOfUseContainer'

with operations like

  /identityGovernance/termsOfUse/agreements:
    get:
      tags:
        - identityGovernance.termsOfUseContainer
      summary: Get agreements from identityGovernance
      operationId: identityGovernance.termsOfUse_ListAgreements

I would expect that by default cmdlets generated for this to be named like Get-MgIdentityGovernanceTermsOfUseAgreement. This is because there is a Collection() type of a singular agreement that has a plural agreements, so it makes sense for that to be represented in PSh cmdlet and type names as Agreement. Note there is no term, 'terms' or termofuse in this service's endpoint.

However it appears autorest.powershell is incorrectly attempting to singularize inside of the token termsOfUse as well, rather than treating termsOfUse as a token. As a result when generating PSh from this, I see generated cmdlet names like Get-MgIdentityGovernanceTerm and Get-MgIdentityGovernanceTermOfUseAgreement. For example, GetMgIdentityGovernanceTerm_Get.cs has

    /// [DETAILS]
    /// verb: Get
    /// subjectPrefix:
    /// subject: IdentityGovernanceTerm
    /// variant: Get

and GetMgIdentityGovernanceTermOfUseAgreement_Get.cs

   /// [DETAILS]
    /// verb: Get
    /// subjectPrefix:
    /// subject: IdentityGovernanceTermOfUseAgreement
    /// variant: Get

It is incorrect for autorest.powershell to

since 'term' or 'terms' are not entities, types or paths in the service.

peombwa commented 3 years ago

+1 to this. Also, it would be nice for AutoREST.PowerShell to expose EnglishPluralizationService as a directive or config that customers can use to include and exclude plurals. Essentially something like this but as a directive/config: https://github.com/Azure/autorest.powershell/blob/67d99227e4cb8b03ca3168731bcbe23ae14d35e5/powershell/internal/name-inferrer.ts#L12