OpenAPITools / openapi-generator

OpenAPI Generator allows generation of API client libraries (SDK generation), server stubs, documentation and configuration automatically given an OpenAPI Spec (v2, v3)
https://openapi-generator.tech
Apache License 2.0
21.32k stars 6.45k forks source link

[BUG] Java/Spring generator wrongly emits warnings related to oneOf discriminator #17893

Open s-jepsen opened 7 months ago

s-jepsen commented 7 months ago

Bug Report Checklist

Description

Java/Spring generator wrongly emits warnings related to oneOf discriminator: [WARNING] 'getPets_request' defines discriminator 'pet_type', but the referenced schema 'Cat' is incorrect. pet_type is missing from the schema, define it as required and type string [WARNING] 'getPets_request' defines discriminator 'pet_type', but the referenced schema 'Dog' is incorrect. pet_type is missing from the schema, define it as required and type string

Expected output: Valid generated pojo models No misleading warnings related to oneOf 'pet_type' discriminator.

Actual output: Valid generated pojo models. Misleading warnings related to oneOf 'pet_type' discriminator.

openapi-generator version

7.3.0

OpenAPI declaration file content or url
openapi: 3.0.3
servers:
  - url: http://localhost/api
    description: Local environment
info:
  description: API
  version: 0.0.0
  title: Api
  x-audience: business-unit-internal
  x-api-id: a8fb5db3-49ef-41c0-a12d-b670cbf64943
  contact:
    name: Unknown
    url: http://unknown.api
    email: unknown@address.unk
paths:
  /pets:
    get:
      operationId: getPets
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/Cat'
                - $ref: '#/components/schemas/Dog'
              discriminator:
                propertyName: pet_type
      responses:
        '200':
          description: Updated

components:
  schemas:
    Pet:
      type: object
      required:
        - petType
      properties:
        petType:
          type: string
      discriminator:
        propertyName: petType
    Cat:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Cat`
          properties:
            name:
              type: string
    Dog:
      allOf:
        - $ref: '#/components/schemas/Pet'
        - type: object
          # all other properties specific to a `Dog`
          properties:
            bark:
              type: string
Generation Details
Steps to reproduce

A Maven project to reproduce the issue is attached openapi-one-of-warning-bug.zip

Related issues/PRs
Suggest a fix
jpfinne commented 6 months ago

This is simpler:

  /pets:
    get:
      operationId: getPets
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/Pet'
s-jepsen commented 6 months ago

It might be - but the structure i'm using is taken from the official documentation.

jpfinne commented 6 months ago

I've looked in the code. DefaultCodeGen.discriminatorFound() has a bug.

It visits recursively all the children to find the discriminator. To avoid infinite loops, there is a safegard. It is badly implemented.

  private CodegenProperty discriminatorFound(String composedSchemaName, Schema sc, String discPropName, Set<String> visitedSchemas) {
   if (visitedSchemas.contains(composedSchemaName)) { // recursive schema definition found
      return null;
    } else {
       visitedSchemas.add(composedSchemaName);
    }

          ...

    for (Object allOf : composedSchema.getOneOf()) {
       // this call will always return null because the first argument is composedSchemaName instead of ((Schema)oneOf).getName()
       // warning (Schema)oneOf).getName() can be null so the safeguard needs to take it into account
       CodegenProperty thisCp = discriminatorFound(composedSchemaName, (Schema) oneOf, discPropName, visitedSchemas);
                    ...
     }

     // same for allOf and anyOf

I'll try to create a PR

jpfinne commented 6 months ago

I think #15298 fix this issue