openapi-generators / openapi-python-client

Generate modern Python clients from OpenAPI
MIT License
1.28k stars 197 forks source link

Schema processing fails when "allOf" references to nullable object #1066

Open sebastianheinig opened 3 months ago

sebastianheinig commented 3 months ago

Describe the bug When using a schema that contains an "allOf" reference to a nullable object, the generation fails with the message "Cannot take allOf a non-object".

OpenAPI Spec File Document to recreate:

openapi: 3.0.1
info:
  title: Title
  description: Description
  version: 1.0.0
servers:
  - url: /api
paths:
  /item:
    get:
      operationId: item
      responses:
        "200":
          description: Successful operation
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Item'
components:
  schemas:
    RelatedItemSuper:
      type: object
      nullable: true
      properties:
        propertyA:
          type: string
    RelatedItem:
      type: object
      allOf:
        - $ref: '#/components/schemas/RelatedItemSuper'
        - type: object
          properties:
            propertyB:
              type: string
    Item:
      type: object
      nullable: false
      properties:
        key:
          type: string
        relatedItem:
          $ref: '#/components/schemas/RelatedItem'

A real world example can be found here.

Desktop (please complete the following information):

Additional context Changing "nullable" to "false" on "RelatedItem" in the above example works but does not seem to be a suitable solution as the construct used is legitimate, or is it not?

eli-bl commented 2 weeks ago

I think I see how this is happening. There are two related issues here:

  1. The current allOf logic only works for object schemas, not for unions of "could be an object, could be something else" (in this case the something else is null). That's just a limitation of how it's implemented. I can sort of imagine a fix but not a very straightforward one.
  2. This spec is in kind of a gray area where it's technically valid but doesn't exactly match what you intended. I think that if you do an allOf with two schemas but only one of them is nullable, a strict OpenAPI validator would say that the result is not nullable. That is, a null value would not be a valid RelatedItem, because it would satisfy RelatedItemSuper but would not satisfy the non-nullable inline schema with propertyB. The fix for that would just be to make the inline schema nullable too; unfortunately it still wouldn't work with the current generator, due to the implementation issue I mentioned.