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

Deep update components instead of overwriting components for OpenAPI 3 #222

Merged
merged 2 commits into from
Jun 9, 2018

Conversation

fMeow
Copy link
Contributor

@fMeow fMeow commented Jun 1, 2018

In my case, I would like to specify securitySchemes in components
object. However, currently there is no predefined function to meet my
need. So I would like to specify components object when initialize
APISpec object.

doc_string = """
components:
  securitySchemes:
    bearerAuth:
      type: http
      scheme: bearer
      bearerFormat: JWT
"""
setting = yaml.load(doc_string)
spec = APISpec(
    plugins=(
        'apispec.ext.marshmallow',
    ),
    openapi_version='3.0.1',
    **settings
)

The document says that the keyword arguments are optional top-level
keys. And I have managed to add optional top-level keys like server
objects.

However, the components objects I specified is shadowing components that is generated by apispec code. Relevant code locates in to_dict function in apispec.core:

...
elif self.openapi_version.version[0] == 3:
    ...
    components = {
        'schemas': self._definitions,
        'parameters': self._parameters,
    }
ret.update(self.options)

I think it better to update component objects instead of
overwriting component objects. In this way, we can take advantages of both
custom settings and apispec functions.

Furthermore, I am rewriting relevant code to deep update components objects. In other words, the definitions defined by APISpec are merged with given schemas defined in top-level components objects. So do parameters in components objects.

@sloria
Copy link
Member

sloria commented Jun 1, 2018

I think the behavior here is correct. @Guoli-Lyu Would you mind adding tests, please?


ret.update(self.options)
Copy link
Member

Choose a reason for hiding this comment

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

ret.update(self.options) can be kept factorized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I explicitly pass components in options, this line of code would shadow ret['components'] that is generated by codes above. And that is why I have to differentiate the behaviors between OpenAPI 2 and 3.

@fMeow
Copy link
Contributor Author

fMeow commented Jun 6, 2018

Tests is added.

And I feel like adding an example for this case as it demonstrates how to adopt specification on security and to add arbitrary top level fields.

import yaml
from apispec import APISpec
from apispec.utils import validate_swagger

OPENAPI_SPEC = """
openapi: 3.0.1
info:
  description: Server API document
  title: Server API
  version: 1.0.0
servers:
- url: http://localhost:{port}/
  description: The development API server
  variables:
    port:
      enum:
      - '3000'
      - '8888'
      default: '3000'
components:
  securitySchemes:
    bearerAuth:
      type: http
      scheme: bearer
      bearerFormat: JWT
"""

settings = yaml.load(OPENAPI_SPEC)

# retrieve version, title and openapi or swagger version
version = str(settings.pop('openapi', None) or settings.pop('swagger', None))
title = settings['info'].pop('title')
spec_version = settings['info'].pop('version')

spec = APISpec(
    title=title,
    version=spec_version,
    plugins=(
        'apispec.ext.marshmallow',
    ),
    openapi_version=version,
    **settings
)

validate_swagger(spec)

I don't know where to put this example, maybe in docs. Though I prefer to create an example folder and put examples in it.

@sloria
Copy link
Member

sloria commented Jun 9, 2018

@Guoli-Lyu Good idea on documentation. I'd prefer examples to go in the docs, as they're more discoverable there. I think your example could go in the "Special Topics" section of the documentation.

@sloria
Copy link
Member

sloria commented Jun 9, 2018

Let's get this merged. I'll write up some docs for this when I have time.

@sloria sloria merged commit f0f7fbc into marshmallow-code:dev Jun 9, 2018
sloria added a commit that referenced this pull request Jun 9, 2018
@lafrech
Copy link
Member

lafrech commented Jul 16, 2018

For the record, there was an issue in this change as it popped stuff our of components which itself was not copied. Only options was copied. So to_dict couldn't be called twice. I fixed that by using deepcopy when rebasing the Plugin interface PR. Might be overkill, but no big deal.

This change makes sense, however, this specific use case (defining security components) should be addressed by creating a component registrar (#245).

@lafrech
Copy link
Member

lafrech commented Aug 22, 2018

Also for the record, we could do the same when using OpenAPI v2: merge definitions and parameters passed as **options with those from self._definitions and self._parameters. Currently, this works only for OpenAPI v3. Not a big deal, though.

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.

3 participants