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

@api.expect and @api.response decorators don't consistently handle documentation of pre-defined Model from string name #56

Open
p-lucero opened this issue Feb 13, 2020 · 2 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@p-lucero
Copy link

Code

I'm trying to organize my code so that model definitions are shared, in order to reduce clutter, and so that pure API logic ends up in its own files. Here's the structure that I've got right now.

In app.py (the entry point):

from api import api
from flask import Flask

app = Flask(__name__)
api.init_app(app)

In api/__init__.py:

from flask_restx import Api

from .demo import api as demo_ns
from .models import register_models

api = Api()
register_models(api)
api.add_namespace(demo_ns)

In api/models.py:

from flask_restx import fields

models = {
    'My Cool Model': {'my_field': fields.String(description='A cool field!')}
}


def register_models(api):
    for model_name, model_data in models.items():
        api.model(name=model_name, model=model_data)

In api/demo.py:

from flask import json
from flask_restx import Namespace, Resource, Model

api = Namespace('demo', description='Demo and testing endpoints')


@api.route('/hello')
class HelloWorld(Resource):
    @api.expect(Model('My Cool Model'), validate=True)
    @api.response(200, description='Success', model=Model('My Cool Model'))
    def post(self):
        return json.jsonify({'my_field': 'hello!'})

Repro Steps (if applicable)

  1. Run the above code.
  2. Note that the generated documentation does not have a definition for the data returned on a 200 response, but it does have a definition for the data expected in the request.
  3. Change the model parameter in the api.response decorator to [Model('My Cool Model')].
  4. Reload the documentation.
  5. Note that the generated documentation now specifies that a list of objects are returned on a 200 response.

Expected Behavior

Constructing a model object should work the same in both places; if constructing an already-defined model object works for the api.expect decorator, it should also work for the api.response decorator, and vice versa.

Actual Behavior

With the above code, the model will properly render in the documentation for the api.expect decorator, but it will not render for the api.response decorator. However, changing the model parameter in the api.response decorator from Model('My Cool Model') to [Model('My Cool Model')] (i.e. encapsulating it in a list) causes it to render. This is fine for when I want my defined API to produce a list of objects, but not so good for when I want my defined API to produce a single object.

Environment

  • Python version
    Running the python3.7-slim docker image, exact version is below:
Python 3.7.5 (default, Nov 15 2019, 02:40:28) 
[GCC 8.3.0] on linux
  • Flask version: 1.1.1
  • Flask-RESTX version: 0.1.1
  • Other installed Flask extensions: N/A

Additional Context

My current workaround is to include a make_model function in the api/models.py file:

def make_model(api, model_name):
    return api.model(model_name, models[model_name])

This function can then be imported and called in api/demo.py as follows, and produces the expected result in the documentation:

from .models import make_model
# snip
@api.response(200, description='Success', model=make_model(api, 'My Cool Model'))
@p-lucero p-lucero added the bug Something isn't working label Feb 13, 2020
@j5awry
Copy link
Contributor

j5awry commented Apr 5, 2020

I was able to reproduce the issue. The two paths generate their docs in different manners, apparently.

api.expect takes a model as its first argument, and the looks up registered model in the api (here's a traceback of me forcing a failure -- not elegant, but what came to mind to show the path without doing pdb)

  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/api.py", line 538, in __schema__
    self._schema = Swagger(self).as_dict()
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 239, in as_dict
    serialized = self.serialize_resource(
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 433, in serialize_resource
    doc = self.extract_resource_doc(resource, url, route_doc=route_doc)
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 338, in extract_resource_doc
    method_params = self.expected_params(method_doc)
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 392, in expected_params
    "schema": self.serialize_schema(expect),
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 632, in serialize_schema
    self.register_model(model)
  File "/home/jchittum/dev01/python-restx/flask-restx/flask_restx/swagger.py", line 653, in register_model
    raise ValueError("Model {0} not registered".format(name))
ValueError: Model Something Different not registered

however api.response, when generating docs, does not do the same lookup path. a couple things to understand that code path

  1. you're not using the Namespace object you've instantiated. Instead, everything is going through the default Namespace object
  2. running api.response is really Namespace.response (for looking at the code)
  3. this adds to the Namespace doc
    def response(self, code, description, model=None, **kwargs):
        """
        A decorator to specify one of the expected responses

        :param int code: the HTTP status code
        :param str description: a small description about the response
        :param ModelBase model: an optional response model

        """
        return self.doc(responses={str(code): (description, model, kwargs)})
  1. Since it's not doing lookup, what it does is just add the model. With this code, Model("My Cool Model) is actually an "empty" Model object. As such, it doesn't add anything to the doc when it tries to look up Model._schema

the easiest workaround is to always reference your namespace APIs, Keeping your code exactly the same, set up your route as follows:

@api.route('/hello')
class HelloWorld(Resource):
    @api.expect(api.models['My Cool Model'], validate=True)
    @api.response(200, description='Success', model=api.models['My Cool Model'])
    def post(self):
        return json.jsonify({'my_field': 'hello!'})

It's definitely strange that the different types of documentation can take different paths to completion. I'll write a separate comment specifying the root cause. However, please use the above workaround so you don't have to have an extra method.

Also, this will work per Namespace. Just reference Namespace.models

@j5awry
Copy link
Contributor

j5awry commented Apr 5, 2020

Summary -- Namespace.expect and other marshalling style methods do Model lookup in swagger.py. However, other documentation style methods (Namespace.doc, Namespace.response) generate schema and pass those on.

I suggest the following:

  1. Minimally update documentation to explain the different paths
  2. Next Step (for review by team) Look into having only Namespace.doc take a separate path, and allow Namespace.response to do lookup like the marshalling methods

@j5awry j5awry added documentation Improvements or additions to documentation enhancement New feature or request and removed bug Something isn't working labels Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants