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

ModelUtils.getUnusedSchemas reports simple model from array types as unused #252

Closed
jimschubert opened this issue Jun 8, 2018 · 5 comments
Milestone

Comments

@jimschubert
Copy link
Member

Description

Found in #251

openapi-generator version

3.0.0

OpenAPI declaration file content or url

https://github.com/OAI/OpenAPI-Specification/blob/master/examples/v3.0/petstore.yaml

Command line used for generation

n/a (not a generation bug)

Steps to reproduce

Discovered while testing #251.

On that branch:

mvn clean install
cd modules/openapi-generator-gradle-plugin/
./gradlew assemble publishToMavenLocal
./gradlew --build-file=samples/local-spec/build.gradle generateGoWithInvalidSpec
Related issues/PRs
Suggest a fix/enhancement

ModelUtils.getUnusedSchemas calls into visitOpenAPI, and visitOpenAPI needs to account for multiple levels of nested arrays, enums, map structures, and polymorphic types.

@jmini
Copy link
Member

jmini commented Jun 8, 2018

Thank you a lot!

ModelUtils.getUnusedSchemas calls into visitOpenAPI, and visitOpenAPI needs to account for multiple levels of nested arrays, enums, map structures, and polymorphic types.

This is absolutely correct, and we also needs to ensure that we do not create infinite loops

@jmini
Copy link
Member

jmini commented Jun 8, 2018

I have started something to fix it: #253

@jmini
Copy link
Member

jmini commented Jun 8, 2018

ModelUtils.getUnusedSchemas calls into visitOpenAPI, and visitOpenAPI needs to account for multiple levels of nested arrays, enums, map structures, and polymorphic types.

@jimschubert : what have you in mind with "enums"? I have looked at the spec, but I did not find where you can use/reference a Schema.

@jimschubert
Copy link
Member Author

@jmini here's an example of an OpenAPI 2.0 spec with a top-level enum of weekdays. https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/test/resources/integrationtests/csharp/general/enum-support-spec.json#L35

I only skimmed ModelUtils.getUnusedSchemas for a few seconds to present the list of what I felt might be missing. I noticed it was inspecting items, but it didn't look like it was evaluating nested children which might refer to top-level enums. Similar to logic that is there, I think it would have to be done for inputs, outputs, and other response models (like errors).

@jmini
Copy link
Member

jmini commented Jun 9, 2018

@jimschubert thank you a lot for your reply.

Enum models are exactly like other models in the definitions section (or schema and components/schema section in the OAS3 terminology), I have already an example in the Unit test:

SomeObj11:
type: string
enum:
- v1
- v2
default: v1

I am glad you gave this precision, because I was afraid to miss something.

Let me finish #253 and I hope I will have all cases.

@jmini jmini closed this as completed in #253 Jun 9, 2018
jmini added a commit that referenced this issue Jun 9, 2018
Fix #252

`ModelUtils.getUnusedSchema()` consider Schemas referenced in other Schemas. Implemented for:

* array
* object
* maps
* ComposedSchema
  - oneOf
  - anyOf
  - allOf
* not
@jmini jmini added this to the 3.0.1 milestone Jun 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants