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

Added flag to avoid aggregation of anyOf/allOf/oneOf schemas while using resolveFully() #568

Merged
merged 4 commits into from
Nov 17, 2017

Conversation

slinkydeveloper
Copy link
Contributor

Some users don't want to aggregate anyOf/allOf/oneOf schemas but simply wants all refs solved.
This solution adds a flag inside the ResolverFully and when schemas are processed It checks this flag. In case user don't want to aggregate, It generates a new list of subschemas solved and saves it to the composedSchema object.

I've also added a flag in ParseOptions that OpenAPIV3Parser reads during readContents() and I've wrote a test. For default the parser aggregate, as is now

@slinkydeveloper
Copy link
Contributor Author

@webron can you accept this pr? I need that for some bugs inside vertx-web (vert-x3/vertx-web#733 and vert-x3/vertx-web#728)

@vinscom
Copy link

vinscom commented Nov 15, 2017

There are too many diff. just because of space. Can you please just have your changes visible as part of diff.

@slinkydeveloper
Copy link
Contributor Author

slinkydeveloper commented Nov 15, 2017

I've reverted back some whitespaces removed automatically by my ide. Major changes involve in particular:

Copy link
Contributor

@gracekarina gracekarina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! @slinkydeveloper, I think it looks good, what got me confused for a while was the test, because it checks that the $ref are resolved in the Components->Schemas when aggregateCombinators: false, and not resolved when aggregateCombinators: true, but I think we could add to the test what happens with the PathItem that uses the AllOfSchema, e.g the withInvalidComposedModel

    post:
      operationId: withInvalidComposedModel
      x-swagger-router-controller: TestController
      requestBody:
        content:
          "application/json":
            schema:
              "$ref": "#/components/schemas/ExtendedAddress"
        required: false
      responses:
        '200':
          description: success!

The test should show that when aggregateCombinators is false the properties inside the AllOfSchema (in the pathItem) remain separated. Right now the test is showing that the schema inside components is being resolved or not.
Thanks for this interesting option to the parser!

@slinkydeveloper
Copy link
Contributor Author

I've modified a bit the test. Now it checks also the path @gracekarina suggested 😄


assertEquals(allOf.getAllOf().size(), 2);

assertTrue(allOf.getAllOf().get(0).getProperties().containsKey("street"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be schema.getAllOf...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ops, brutal copy paste 😳

@gracekarina
Copy link
Contributor

Thanks @slinkydeveloper

@gracekarina gracekarina merged commit efad139 into swagger-api:2.0 Nov 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants