marshmallow-code / apispec

A pluggable API specification generator. Currently supports the OpenAPI Specification (f.k.a. the Swagger specification)..
https://apispec.readthedocs.io/
MIT License
1.17k stars 175 forks source link

Create methods to register all component objects #245

Open lafrech opened 6 years ago

lafrech commented 6 years ago

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 commented 6 years ago

For instance, https://github.com/marshmallow-code/apispec/commit/3556849f815d42e28f84ee60cc518af00c988cc1 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:

lafrech commented 6 years ago

Currently, existing methods are

It would be nice to have consistent naming.

Should it be

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

lafrech commented 5 years ago

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.

lafrech commented 5 years ago

Removing 1.0 milestone as the breaking part was achieved in https://github.com/marshmallow-code/apispec/pull/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 commented 5 years ago

List of components

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