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

Create methods to register all component objects #245

Open
lafrech opened this issue Jul 16, 2018 · 5 comments

Comments

@lafrech
Copy link
Member

commented Jul 16, 2018

Currently, definitions can be registered to be referenced to in the spec.

The spec allows to register other objects like parameters or responses.

OpenAPI 3: https://swagger.io/specification/#componentsObject

Some of them also exist in OpenAPI 2. Search for "Swagger Object" in https://swagger.io/specification/v2/.

We'd rather get this right with all those objects in mind rather than address them independently each time everyone needs one of them.

It should be easy to create methods to register each object type. Or should we create a single method and pass the type as argument (and deprecate definition)?

Then it's less obvious how to help the developer to use references. Referencing schema definition is automatic. Referencing responses probably wouldn't be, so the user would have to enter the reference response name. Ideally, we'd provide a function that at least takes care of the $ref syntax allowing the user to just enter the name of the ref.

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2018

For instance, 3556849 added the possibility to pass parameters a while ago.

We'd need to either do the same with each component type (response, security, etc.), or create a single method to manage them all. Since it's less than a ten of them, a method for each is reasonable. Besides, it makes it easier to do step-by-step.

Also, there should be hooks to allow plugins to provide helpers. For instance, there could be parameter helpers so that marshmallow schemas can be parsed when registered as parameters, response helpers, header helpers,...

Referencing could be done as was done for parameters: the developer passes the parameter name as a string and the clean_operation function blindly uses a ref when it encounters a string rather than a dict, assuming it has to be a reference: {'$ref': '#/parameters/' + x}, without any check. That's good enough for me. At least for a first step. This just takes a modification of clean_operation for each component type.

Overall, this issue is two-fold:

  • Add methods to register components
  • Add hooks for plugin helpers in those methods (including already existing add_parameter method)

@lafrech lafrech added the help wanted label Aug 22, 2018

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Aug 22, 2018

Currently, existing methods are

  • definition
  • add_parameter

It would be nice to have consistent naming.

Should it be

  • definition, parameter,...
  • add_definition, add_parameter,...
  • register_definition, register_parameter,...

Or a generic register_component method that accepts a type as first argument?

@lafrech lafrech added this to the 1.0 milestone Oct 6, 2018

@lafrech lafrech referenced this issue Oct 7, 2018
2 of 2 tasks complete
@lafrech

This comment has been minimized.

Copy link
Member Author

commented Oct 8, 2018

#305 proposes another approach: use a Components class to hold those components. I'm not convinced it is the best approach. I found in nicer when I did it, but I don't have a strong feeling about it.

From developer perspective, it allows to separate those methods rather than have an APISpec class with many methods. From user perspective, it is more verbose (needs to add components, no big deal...). It sticks closer to the OpenAPI spec (exposing the notion of components, which is internal and may change in later versions).

We have many possibilities:

Current (inconsistent):

spec.definition(...)
spec.add_parameter(...)

No prefix (not explicit enough IMHO):

spec.definition(...)
spec.parameter(...)

Add:

spec.add_definition(...)
spec.add_parameter(...)

Components + prefix (e.g. add):

spec.components.add_definition(...)
spec.components.add_parameter(...)

Definitions were renamed as schemas in v3, so we could use, schema, or definition, or both.

spec.components.add_schema(...)
spec.components.add_parameter(...)
# optional
spec.components.add_definition(...): return spec.components.add_schema(...)

I don't like the common method + string parameter approach as using a string is weird and I don't like the idea of having a slingle signature for methods with different parameters. But I like the consistency brought by the use of a common term (here, "components") in all methods.

spec.register_component('definition', ...)

It is this last approach that led me to adding a components class to use methods with some sort of components namespace:

spec.components.add_definition(...)

Overall, my best choices are "add_xxx" with or without components class.

spec.components.add_parameter(...)
spec.add_parameter(...)

I'm not sure there is an interest in separating components (definitions/schemas, parameters, responses) and other stuff (tags, paths,...).

@sloria, feedback welcome. This design choice is the only breaking part of the change. Once this is done, it does not block 1.0 anymore and other components can be added later in further non-breaking changes.

@sloria sloria removed the help wanted label Oct 24, 2018

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Nov 6, 2018

Removing 1.0 milestone as the breaking part was achieved in #305. We can now add other components using the same mechanism.

Anyone willing to contribute can easily add methods to register other component types. I don't think they'll need plugin helpers. Just pay attention to OpenAPI v2 vs. v3 differences.

(At this point, I'm tempted to add that help wanted label back, or ready to claim, or any label meaning it is easy and anyone is welcome to work on this.)

@lafrech lafrech removed this from the 1.0 milestone Nov 6, 2018

@sloria sloria added help wanted and removed feedback welcome labels Nov 6, 2018

@lafrech

This comment has been minimized.

Copy link
Member Author

commented Feb 1, 2019

List of components

  • schemas
  • parameters
  • securitySchemes
  • requestBodies
  • responses
  • headers
  • examples
  • links
  • callbacks

When adding a component adder method, it would be nice to also update build_reference and clean_operations to automatically build references in the spec.

(TODO: resolve headers refs in response in clean_operations.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.