Semantic violation: Certain OData functions have path parameters referenced in query

[peombwa]

Certain OData functions have path parameters referenced in query instead of path. This results in generation error since code generators cannot find the specified path parameters.

Current Representation

'/users/{user-id}/joinedTeams/{team-id}/primaryChannel/microsoft.graph.doesUserHaveAccess(userId=''{userId}'',tenantId=''{tenantId}'',userPrincipalName=''{userPrincipalName}'')':
  get:
    tags:
      - users.Functions
    summary: Invoke function doesUserHaveAccess
    operationId: users.joinedTeams.primaryChannel_doesUserHaveAccess
    parameters:
      - name: user-id
        in: path
        description: 'key: id of user'
        required: true
        schema:
          type: string
        x-ms-docs-key-type: user
      - name: team-id
        in: path
        description: 'key: id of team'
        required: true
        schema:
          type: string
        x-ms-docs-key-type: team
      - name: userId
        in: query
        description: 'Usage: userId=''{userId}'''
        schema:
          type: string
          nullable: true
      - name: tenantId
        in: query
        description: 'Usage: tenantId=''{tenantId}'''
        schema:
          type: string
          nullable: true
      - name: userPrincipalName
        in: query
        description: 'Usage: userPrincipalName=''{userPrincipalName}'''
        schema:
          type: string
          nullable: true
    responses:
      '200': ...
      default:
        $ref: '#/components/responses/error'
    x-ms-docs-operation-type: function

Ideal Representation

'/users/{user-id}/joinedTeams/{team-id}/primaryChannel/microsoft.graph.doesUserHaveAccess(userId=''{userId}'',tenantId=''{tenantId}'',userPrincipalName=''{userPrincipalName}'')':
  get:
    tags:
      - users.Functions
    summary: Invoke function doesUserHaveAccess
    operationId: users.joinedTeams.primaryChannel_doesUserHaveAccess
    parameters:
      - name: user-id
        in: path
        description: 'key: id of user'
        required: true
        schema:
          type: string
        x-ms-docs-key-type: user
      - name: team-id
        in: path
        description: 'key: id of team'
        required: true
        schema:
          type: string
        x-ms-docs-key-type: team
      - name: userId
+         in: path
-         in: query
        description: 'Usage: userId=''{userId}'''
        schema:
          type: string
          nullable: true
      - name: tenantId
+         in: path
-         in: query
        description: 'Usage: tenantId=''{tenantId}'''
        schema:
          type: string
          nullable: true
      - name: userPrincipalName
+         in: path
-         in: query
        description: 'Usage: userPrincipalName=''{userPrincipalName}'''
        schema:
          type: string
          nullable: true
    responses:
      '200': ...
      default:
        $ref: '#/components/responses/error'
    x-ms-docs-operation-type: function

[darrelmiller]

@irvinesunday Do you know why we are generating the above path? JoinedTeams is non-contained and yet we are exploding the navigation property primaryChannel.

@peombwa I am fairly confident that this path does not work. You can't even readbykey on the JoinedTeams so there is no way to get at this function via a path that has user-id in it.

[baywet]

still the parameters should be in path and not in query, this is something we're running into in Kiota based SDK as well.

[irvinesunday]

Since having optional parameters defined either in: path and in: query are both acceptable (see this related issue), I suggest we introduce a convert setting, OptionalParametersInPath and have its default to false. Then we can use this setting in DevX API and Hidi to have our generation append optional parameters to in: path by setting the convert setting value to true.
@peombwa @baywet

[baywet]

I don't think this is a functional preference at this point. The query parameter should be described as in path. It could be described as in query if it were aliased, but we've already made changes to have it in path instead.
If we were to provide a setting it should be between this (corrected with in path) and the aliased approach. Not just for query and path for this format as it is invalid.

[irvinesunday]

Implicit parameter aliases are permitted as well, where the query parameter can be described in the query, acting as the implicit parameter alias.

doc ref:

[irvinesunday]

@irvinesunday Do you know why we are generating the above path? JoinedTeams is non-contained and yet we are exploding the navigation property primaryChannel.

@peombwa I am fairly confident that this path does not work. You can't even readbykey on the JoinedTeams so there is no way to get at this function via a path that has user-id in it.

@darrelmiller we do not generate this path in our latest version of the conversion lib.

[baywet]

@irvinesunday sure but in that case the parameters should not be in the path anymore for the URI template. The concern is that currently the URL template is invalid.
Let's just fix it to have query changed to path in the parameter description, which was the original ask of this issue. And defer additional feature works (option to have it aliased or implicitly aliased) to another issue

[irvinesunday]

@irvinesunday sure but in that case the parameters should not be in the path anymore for the URI template. The concern is that currently the URL template is invalid. Let's just fix it to have query changed to path in the parameter description, which was the original ask of this issue. And defer additional feature works (option to have it aliased or implicitly aliased) to another issue

My concern here is that this will be a breaking change, with regards to this issue which requested that optional parameters be defined in: query which was resolved by this PR