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] Wrong expression used to detect required properties #13548

Closed
5 of 6 tasks
meyer-r opened this issue Sep 29, 2022 · 5 comments · Fixed by #13630
Closed
5 of 6 tasks

[BUG] Wrong expression used to detect required properties #13548

meyer-r opened this issue Sep 29, 2022 · 5 comments · Fixed by #13630

Comments

@meyer-r
Copy link

meyer-r commented Sep 29, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

Some mustache templates use the expressions "{{#isRequired}}" and "{{^isRequired}}" to differentiate between required
and optional properties. But "isRequired" is not available as a property (at least it's not contained in the debug
model file when the generator is run with the "debugModels" option).
It looks like the "required" property should be used instead.

The consequence is that "{{#isRequired}}" always evaluates to false, and "{{^isRequired}}" always evaluates to true.
So code to be generated for required properties is never generated, and all properties are treated as non-required.

This mainly affects the "java" generators, but also some other languages.

openapi-generator version

v6.1.0 (also tested with latest master)

OpenAPI declaration file content or url
openapi: 3.0.1
info:
  title: openapi-generator java isRequired bug test case
  version: 1.0.0
paths:
  /some-service:
    get:
      operationId: someService
      responses:
        200:
          description: response
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/SomeSchema'
components:
  schemas:
    SomeSchema:
      required:
      - required_property
      type: object
      properties:
        required_property:
          $ref: '#/components/schemas/ReferencedSchema'
        optional_property:
          $ref: '#/components/schemas/ReferencedSchema'
    ReferencedSchema:
      type: object
      properties:
        some_property:
          type: string
Generation Details

openapi-generator-cli generate -i ./openapi.yaml -g java -o ./generated/java/

Steps to reproduce

When the openapi file above is generated with "java", the "SomeSchema" class contains the following code in the
"validateJsonObject" method:

  // validate the optional field `required_property`
  if (jsonObj.get("required_property") != null && !jsonObj.get("required_property").isJsonNull()) {
    ReferencedSchema.validateJsonObject(jsonObj.getAsJsonObject("required_property"));
  }
  // validate the optional field `optional_property`
  if (jsonObj.get("optional_property") != null && !jsonObj.get("optional_property").isJsonNull()) {
    ReferencedSchema.validateJsonObject(jsonObj.getAsJsonObject("optional_property"));
  }

Both the required and optional property are handled the same, and the generated comment even states that
"required_property" is recognized as an optional field.

Expected output for "required_property" would be according to the template code in the "{{#isRequired}}" condition:

  // validate the required field `required_property`
  ReferencedSchema.validateJsonObject(jsonObj.getAsJsonObject("required_property"));
Suggest a fix

Replace all references to "isRequired" with "required".

@wing328
Copy link
Member

wing328 commented Oct 7, 2022

thanks for reporting the issue, I've filed #13630 to fix it.

(will file another for jersey2 later)

@wing328
Copy link
Member

wing328 commented Oct 7, 2022

@meyer-r can you please pull the latest master to give it another try?

@wing328
Copy link
Member

wing328 commented Oct 9, 2022

FYI. #13646 fixed the jersey2, jersey3 templates.

@meyer-r
Copy link
Author

meyer-r commented Oct 14, 2022

@meyer-r can you please pull the latest master to give it another try?

I checked latest master and the issue is fixed there, thanks!

@wing328
Copy link
Member

wing328 commented Oct 14, 2022

thanks for the confirmation

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