Support schemas in callbacks #544
Conversation
Callback definitions are iterated and checked for schemas. This is implemented by running operation_helper on all events defined in a callback definition. These events may contain request and response definitions similar to regular operation definitions.
There was a problem hiding this comment.
Thank you @kortsi for this contribution.
See my comment inline.
The tests seem comprehensive. And kinda massive. I guess the massiveness is due to black formatting the dicts like this. Perhaps they'd look better if the callback was declared as a local variable first. That's just cosmetic anyway. I can rework this later if I'm not happy with it.
Also, since all tests are a copy of a schema v3 test, I'm wondering if there could be room for factorization.
| @@ -196,6 +196,9 @@ def operation_helper(self, operations, **kwargs): | |||
| operation["parameters"] | |||
| ) | |||
| if self.openapi_version.major >= 3: | |||
| for callback in operation.get("callbacks", {}).values(): | |||
| for event in callback.values(): | |||
There was a problem hiding this comment.
callback may be a reference. Let's not assume it is a callback object dict.
Right now we don't offer the possibility to define callback components and we don't resolve the strings as reference either (see #245 (comment)). But still, a user could pass {$ref: "MyCallback"}, I suppose.
Also I try to stick to OAS naming. I might have used path rather than event, but maybe event is better. Just wondering out loud. No big deal anyway.
There was a problem hiding this comment.
Actually, we don't support {$ref: "MyCallback"} in other places, so let's not support it here. However, we could expect ref as strings, even if we don't support them yet as per #245. So let's check callback is actually a dict.
if self.openapi_version.major >= 3:
for callback in operation.get("callbacks", {}).values():
if isinstance(callback, dict):
for path in callback.values():
There was a problem hiding this comment.
It would be convenient if this code block were implemented in a separate method so that support for callbacks can be documented similar to the other OpenAPI objects that are parsed.
There was a problem hiding this comment.
@Bangertm do you mean add callback_helper to the plugin interface and have it called from APISpec.path?
There was a problem hiding this comment.
No just putting that code block in a method called something like resolve_callback and then having a docstring similar to the ones in SchemaResolver. Then there is some user documentation on the fact that users can put a Marshmallow schema in a callback.
Ideally this method would be part of SchemaResolver too, but given the recursion it can't. You were probably right about making SchemaResolver it a mixin.
|
I implemented a "resolve_callback" method with documentation as proposed, made sure only dict callback objects are processed, and refactored tests a bit to make them a little less massive. I then merged the latest dev on top. Is there still something left to do to get this ready? |
There was a problem hiding this comment.
I'd prefer the resolve_callback method to be in the schema_resolver module so that all of the code for looking for schemas in OpenAPI object is in the same place. Other than that this looks great.
This is now done. I also had to move the code from |
| https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.2.md#callbackObject`_. | ||
|
|
||
| This is done recursively, so it it is possible to define callbacks in your callbacks. | ||
|
|
Callback definitions are iterated and checked for schemas. This is
implemented by running operation_helper on all events defined in a
callback definition. These events may contain request and response
definitions similar to regular operation definitions.