microsoft / kiota

OpenAPI based HTTP Client code generator
https://aka.ms/kiota/docs
MIT License
3.02k stars 210 forks source link

ID parameters in Open API that do not conform to `type-id` format generate broken request builders #1052

Closed jasonjoh closed 2 years ago

jasonjoh commented 2 years ago

An Open API path like /ToDoItems/{id} will result in an extra model class being generated named With, and a WithRequestBuilder being generated to handle GET (for a single item), DELETE, and PATCH. The PATCH method (PatchAsync) doesn't behave as expected - you cannot pass an instance of the ToDoItem class to it, it requires an instance of the With class.

I was able to work around this by changing my Open API description's path to /ToDoItems/{ToDoItem-id}.

image

var createdItem = await client.ToDoItems.PostAsync(newItem);
Console.WriteLine($"Item created with ID: {createdItem.Id}");

// Update an item
var updateItem = new ToDoItem
{
    IsComplete = true
};

// This doesn't compile
await client.ToDoItems[$"createdItem.Id"].PatchAsync(updateItem);

image

Repro OpenAPI

openapi: 3.0.1
info:
  title: OData Service for namespace Default
  description: This OData service is located at https://localhost:7206/
  version: 1.0.1
servers:
  - url: https://localhost:7206/
paths:
  /ToDoItems:
    get:
      tags:
        - ToDoItems.ToDoItem
      summary: Get entities from ToDoItems
      operationId: ToDoItems.ToDoItem.ListToDoItem
      parameters:
        - $ref: '#/components/parameters/top'
        - $ref: '#/components/parameters/skip'
        - $ref: '#/components/parameters/search'
        - $ref: '#/components/parameters/filter'
        - $ref: '#/components/parameters/count'
        - name: $orderby
          in: query
          description: Order items by property values
          style: form
          explode: false
          schema:
            uniqueItems: true
            type: array
            items:
              enum:
                - id
                - id desc
                - name
                - name desc
                - isComplete
                - isComplete desc
                - type
                - type desc
                - priority
                - priority desc
                - dueDate
                - dueDate desc
              type: string
        - name: $select
          in: query
          description: Select properties to be returned
          style: form
          explode: false
          schema:
            uniqueItems: true
            type: array
            items:
              enum:
                - id
                - name
                - isComplete
                - type
                - priority
                - dueDate
              type: string
        - name: $expand
          in: query
          description: Expand related entities
          style: form
          explode: false
          schema:
            uniqueItems: true
            type: array
            items:
              enum:
                - '*'
              type: string
      responses:
        '200':
          description: Retrieved entities
          content:
            application/json:
              schema:
                title: Collection of ToDoItem
                type: object
                properties:
                  value:
                    type: array
                    items:
                      $ref: '#/components/schemas/ToDoApi.Models.ToDoItem'
        default:
          $ref: '#/components/responses/error'
    post:
      tags:
        - ToDoItems.ToDoItem
      summary: Add new entity to ToDoItems
      operationId: ToDoItems.ToDoItem.CreateToDoItem
      requestBody:
        description: New entity
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/ToDoApi.Models.ToDoItem'
        required: true
      responses:
        '201':
          description: Created entity
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ToDoApi.Models.ToDoItem'
        default:
          $ref: '#/components/responses/error'
      x-ms-docs-operation-type: operation
  /ToDoItems/{id}:
    get:
      tags:
        - ToDoItems.ToDoItem
      summary: Get entity from ToDoItems by key
      operationId: ToDoItems.ToDoItem.GetToDoItem
      parameters:
        - name: id
          in: path
          description: 'key: id of ToDoItem'
          required: true
          schema:
            type: string
          x-ms-docs-key-type: ToDoItem
        - name: $select
          in: query
          description: Select properties to be returned
          style: form
          explode: false
          schema:
            uniqueItems: true
            type: array
            items:
              enum:
                - id
                - name
                - isComplete
                - type
                - priority
                - dueDate
              type: string
        - name: $expand
          in: query
          description: Expand related entities
          style: form
          explode: false
          schema:
            uniqueItems: true
            type: array
            items:
              enum:
                - '*'
              type: string
      responses:
        '200':
          description: Retrieved entity
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ToDoApi.Models.ToDoItem'
        default:
          $ref: '#/components/responses/error'
      x-ms-docs-operation-type: operation
    patch:
      tags:
        - ToDoItems.ToDoItem
      summary: Update entity in ToDoItems
      operationId: ToDoItems.ToDoItem.UpdateToDoItem
      parameters:
        - name: id
          in: path
          description: 'key: id of ToDoItem'
          required: true
          schema:
            type: string
          x-ms-docs-key-type: ToDoItem
      requestBody:
        description: New property values
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/ToDoApi.Models.ToDoItem'
        required: true
      responses:
        '204':
          description: Success
        default:
          $ref: '#/components/responses/error'
      x-ms-docs-operation-type: operation
    delete:
      tags:
        - ToDoItems.ToDoItem
      summary: Delete entity from ToDoItems
      operationId: ToDoItems.ToDoItem.DeleteToDoItem
      parameters:
        - name: id
          in: path
          description: 'key: id of ToDoItem'
          required: true
          schema:
            type: string
          x-ms-docs-key-type: ToDoItem
        - name: If-Match
          in: header
          description: ETag
          schema:
            type: string
      responses:
        '204':
          description: Success
        default:
          $ref: '#/components/responses/error'
      x-ms-docs-operation-type: operation
components:
  schemas:
    ToDoApi.Models.ToDoItem:
      title: ToDoItem
      type: object
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
          nullable: true
        isComplete:
          type: boolean
        type:
          type: string
          nullable: true
        priority:
          maximum: 2147483647
          minimum: -2147483648
          type: integer
          format: int32
        dueDate:
          pattern: '^[0-9]{4,}-(0[1-9]|1[012])-(0[1-9]|[12][0-9]|3[01])T([01][0-9]|2[0-3]):[0-5][0-9]:[0-5][0-9]([.][0-9]{1,12})?(Z|[+-][0-9][0-9]:[0-9][0-9])$'
          type: string
          format: date-time
    odata.error:
      required:
        - error
      type: object
      properties:
        error:
          $ref: '#/components/schemas/odata.error.main'
    odata.error.main:
      required:
        - code
        - message
      type: object
      properties:
        code:
          type: string
        message:
          type: string
        target:
          type: string
        details:
          type: array
          items:
            $ref: '#/components/schemas/odata.error.detail'
        innererror:
          type: object
          description: The structure of this object is service-specific
    odata.error.detail:
      required:
        - code
        - message
      type: object
      properties:
        code:
          type: string
        message:
          type: string
        target:
          type: string
  responses:
    error:
      description: error
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/odata.error'
  parameters:
    top:
      name: $top
      in: query
      description: Show only the first n items
      schema:
        minimum: 0
        type: integer
      example: 50
    skip:
      name: $skip
      in: query
      description: Skip the first n items
      schema:
        minimum: 0
        type: integer
    count:
      name: $count
      in: query
      description: Include count of items
      schema:
        type: boolean
    filter:
      name: $filter
      in: query
      description: Filter items by property values
      schema:
        type: string
    search:
      name: $search
      in: query
      description: Search items by search phrases
      schema:
        type: string
  examples:
    ToDoApi.Models.ToDoItem:
      value:
        dueDate: '0001-01-01T00:00:00.0000000+00:00'
        id: 0
        isComplete: true
        name: String
        priority: 0
        type: String
tags:
  - name: ToDoItems.ToDoItem
    x-ms-docs-toc-type: page
baywet commented 2 years ago

Thanks for reporting that @jasonjoh, this is most likely caused by the parameters cleanup regex which is failing to recognize it's an indexable collection, falling back to a path parameter, and trying to generate the naming convention without being able to extract the parameters names https://github.com/microsoft/kiota/blob/8654a5ee338b72373fb20c798371cffb64a0146b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs#L30

calebkiage commented 2 years ago

The same issue affects CLI when generating the request builders for the API endpoint /users/{user-id}/licenseDetails/{licenseDetails-id}. image In this instance, there are 2 classes with the same name. When generating CLI code, this situation causes a stack overflow error. For the .NET sdks, there doesn't seem to be any runtime error, but the operations under the Item.LicenseDetailsRequestBuilder are unreachable via the fluent API. image The above image shows the cause of the stack overflow on line 29.

baywet commented 2 years ago

update: I've authored #1455 to address this issue. As per your comment @calebkiage I think there are two bugs at play here:

I think the second bug should be handled separately for the CLI. Checkout the dotnet generation code which has added logic to circumvent that case.