Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Generation fails "the allOf schemas must use $ref" #7262

Closed
5 of 6 tasks
felschr opened this issue Aug 20, 2020 · 1 comment · Fixed by #7328
Closed
5 of 6 tasks

[BUG] Generation fails "the allOf schemas must use $ref" #7262

felschr opened this issue Aug 20, 2020 · 1 comment · Fixed by #7328

Comments

@felschr
Copy link

felschr commented Aug 20, 2020

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix (example)
Description

After upgrading from v4.2.2 to v5 (d3017ff) to get the changes from #6789 I'm getting a new error now that wasn't present before:

Exception in thread "main" java.lang.RuntimeException: Could not process model 'APIModelActivitiesActivity'.Please make sure that your schema is correct!
        at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:476)
        at org.openapitools.codegen.DefaultGenerator.generate(DefaultGenerator.java:851)
        at org.openapitools.codegen.cmd.Generate.execute(Generate.java:432)
        at org.openapitools.codegen.cmd.OpenApiGeneratorCommand.run(OpenApiGeneratorCommand.java:32)
        at org.openapitools.codegen.OpenAPIGenerator.main(OpenAPIGenerator.java:66)
Caused by: java.lang.RuntimeException: Invalid inline schema defined in allOf in 'UserSleep'. Per the OpenApi spec, for this case when a composed schema defines a discriminator, the allOf schemas must use $ref. Change this inline definition to a $ref definition
        at org.openapitools.codegen.DefaultCodegen.getAllOfDescendants(DefaultCodegen.java:2825)
        at org.openapitools.codegen.DefaultCodegen.createDiscriminator(DefaultCodegen.java:2882)
        at org.openapitools.codegen.DefaultCodegen.fromModel(DefaultCodegen.java:2243)
        at org.openapitools.codegen.languages.HaskellServantCodegen.fromModel(HaskellServantCodegen.java:635)
        at org.openapitools.codegen.DefaultGenerator.processModels(DefaultGenerator.java:1156)
        at org.openapitools.codegen.DefaultGenerator.generateModels(DefaultGenerator.java:471)
        ... 4 more

The UserSleep schema looks like this:

      "UserSleep": {
        "allOf": [
          {
            "$ref": "#/components/schemas/UserTimeBase"
          },
          {
            "type": "object",
            "additionalProperties": false
          }
        ]
      }

I don't see any mention of this being invalid in the official OpenAPI specification.

openapi-generator version

d3017ff (v5)
It was working in v4.2.2.
I haven't tested any other versions.

Generation Details

This issue happens for all generators.

Steps to reproduce

Minimum config to reproduce the issue:

openapi: "3.0.1"
info:
  version: 1.0.0
  title: allOf discriminator issue
paths:
  /sleep:
    patch:
      requestBody:
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/UserSleep'
      responses:
        '200':
          description: Updated
components:
  schemas:
    UserSleep:
      allOf:
        - $ref: '#/components/schemas/UserTimeBase'
        - type: object
          # properties:
          #   break:
          #     type: string
          #     nullable: true
          additionalProperties: false
    UserTimeBase:
      required:
      - "$type"
      type: object
      properties:
        "$type":
          type: string
        start:
          type: string
          nullable: true
        end:
          type: string
          nullable: true
      additionalProperties: false
      discriminator:
        propertyName: "$type"

While creating this minimum config I noticed that when I add in properties (the commented block) to the inline schema in allOf the generation will work.

Suggest a fix

Using discriminators in allOf schemas should work no matter if it's using $ref or inline schemas and no matter if it includes properties or not.

@spacether
Copy link
Contributor

spacether commented Aug 31, 2020

@felschr we have three issues at play here.
@sebastien-rosset

  1. It does look like there is a bug in openapi generator where it incorrectly hoisted your discriminator from UserTimeBase into UserSleep.
  2. Your spec incorrectly sets additionalProperties to false when additional properties exist in the javascript object. Per the json schema spec (which the openapi spec uses) If "additionalProperties" is false, validation succeeds only if the instance is an object and all properties on the instance were covered by "properties" and/or "patternProperties".With the definition that you have in the sample spec (keeping the inline property commented out) deserializing {} and {"$type": "abc"} will fail for reasons I explain below.
  3. The example that you gave is lacking discriminated schemas that UserTimeBase will discriminate to. Should only UserSleep exist in UserTimeBase's discriminator map?

Let's dig into point two a little more
We can send the following payloads:

  1. {"$type": "abc"} this is the required UserTimeBase property only
  2. {"$type": "abc", "break": "def"} this is the required UserTimeBase param + the optional property in the inline schema

With additionalProperties false for the inline schema that has break and for UserTimeBase.
For payload 1, validation in UserTimeBase succeeds. Validation in the inline schema fails because the payload {"$type": "abc"} is what we attempt to validate in both schemas. For the inline schema, "$type" is unrecognized and is an additional property. additionalProperties is set to false in the inline schema so validation fails. It must be set to true.
The same explanation applies to payload2. When validating the payload, "break" is an additionalProperty in UserTimeBase and "$type" is an additional property in the inline schema.

What about when additionalProperties is true? (This is what we need)
For payload1, validation succeeds for UserTimeBase and the inline schema.
For payload2, validation succeeds for UserTimeBase and the inline schema. When validating the payload, "break" is an additionalProperty in UserTimeBase and "$type" is an additional property in the inline schema. The additional properties are now allowed.

I will look into this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants