DELETE requests are returning Content-Type: text/html, this is breaking some clients that expect application/json, even though there is no actual content, simply the type in the header broke a lookup. Looks like this was introduced here, as text/html is the Flask response default mimetype.

Rolling that back here and adding a test.

https://github.com/plangrid/flask-rebar/blob/master/setup.py#L19 specifies marshmallow>=2.13. Right now this is picking up Marshmallow 2, but as soon as Marshmallow 3 final comes out, this will start picking up v3, which has several backwards-incompatible changes:

https://marshmallow.readthedocs.io/en/3.0/changelog.html

Are any updates necessary before then?

I just discovered flask-rebar and I think it could very well become the new standard for REST with flask. The documentation is very well made, but a more complete "real-life" example/template would help a lot to get started. I am creating one for my project that i could share, but I don't know all the best practices for the framework. Keep up the good work, cheers!

https://flask-rebar.readthedocs.io/en/latest/quickstart/basics.html#marshaling currently documents the following: "This means the if response_body_schema is None, the return value must be a return value that Flask supports, e.g. a string or a Flask.Response object.

However, Flask-Rebar currently fails to interoperate properly with a Flask.Response object, as demonstrated by the included test. Without this bugfix to rebar.py, the included test fails with KeyError: 200. This is because the code currently passes rv to _unpack_view_func_return_value, which always returns a 200 status when rv is not a tuple, not realizing that a Flask.Response instance can get passed through as rv which can have a non-200 .status_code (and also custom .headers as well).

Since this came up in the discussion, would it be a good idea to just jump into the deep and make add a test to ensure pep8 compliance going forward? Do people have a strong opinion about pep8 settings or should we just go with the vanilla version for now?

Thoughts?

Corresponds to Issue #121

• Extended deprecated_parameters decorator to allow a coercion function to convert between old and new styles of parameters.
• Adds support for handlers to have multiple Authenticators.
• Adds support for registries to have multiple Authenticators as their default authentication.
• Should mark authenticator parameters as deprecated using the deprecation utils

JIRA Tickets | --------------| BP-763|

Today we discussed creating our own class of errors because we wanted to have uniforms errors with a machine parsable type. Though we could add it just fine with the additional_data field, it is a bit painful to enforce it on every error thrown. The problem is that we can't currently modify the default errors that Flask-Rebar raise in the case of an invalid schema for example. I don't have a clear answer as to how we should handle this, but I figured I could ask if someone had an idea as to how we could do this.

One annoying thing about marshmallow is the fast that you get a dictionary instead of an object with attributes (I am bit jealous of pydantic). We started using marshmallow-objects in our project. Since we already define some custom schemas, it would be very valuable to either use this lib or make something similar. Especially since we are going toward a v2 with some breaking changes anyway.

I believe obj.fields.items(): does not consistently return objects in the same order. When committing the generated swagger.json file, it causes some redundant diffs in the required fields array

e.g: https://github.com/plangrid/informant/pull/419

Add registry.add_supplemental_schemas function to export supplemental schemas to swagger, in addition to those used by the API handlers.

In my app, I have a handler:

@registry.handles(
    marshal_schema={201: FooSchema}
)
def create_foo():
    pass

With:

class FooSchema(Schema):
    template = fields.String()
    vars = fields.Dict()

Elsewhere, I validate the vars against the template according to various template schemas:

class Template_1_Schema(Schema):
    var_1 = fields.String()
    var_2 = fields.String()

class Template_2_Schema(Schema):
    var_3 = fields.Boolean()
    var_4 = fields.Integer()

I would like to export these template schemas in the swagger definition, so others can know what to expect to send to my handler in the vars dictionary param.

This PR introduces a registry function add_supplemental_schemas to support this. The function can accept a single schema, or for convenience, a directory containing all the supplemental schemas.

cc: @barakalon

If you try to yaml.dump a rebar-generated swagger object, you end up with yaml that looks like this:

!!python/object/apply:collections.OrderedDict
- - - consumes
    - [application/json]
  - - definitions
    - !!python/object/apply:collections.OrderedDict
      - - - Error
          - !!python/object/apply:collections.OrderedDict
            - - - properties
                - !!python/object/apply:collections.OrderedDict
                  - - - errors
                      - !!python/object/apply:collections.OrderedDict
                        - - [type, object]
                    - - message
                      - !!python/object/apply:collections.OrderedDict
                        - - [type, string]
              - - required
                - [message]
              - [title, Error]
              - [type, object]
...

It looks like this is due to the current behavior of unconditionally converting dicts to OrderedDicts.

This PR allows passing order_dicts=False to swagger_generator.generate(...) to suppress the conversion.

This may also be desired by users who want to avoid the extra work if they're using a Python implementation where dicts preserve insertion order already (e.g. PyPy or CPython >= 3.6).

Flask has a signal got_request_exception intended to signify that an unhandled exception occurred in a handler. This signal is triggered by the default error handler, ref: https://github.com/pallets/flask/blob/0d8c8ba71bc6362e6ea9af08146dc97e1a0a8abc/src/flask/app.py#L1675. This means that to Flask, adding error handlers is equivalent to catching an exception; their documentation attempts to explains this, but it can be easy to miss the implication that the exception signal is not sent if a registered error handler matches the exception.

Existing instrumentation/observability tooling uses this signal to detect when an unexpected error has occurred in a service.

Rebar currently registers a generic Exception error handler, ref: https://github.com/plangrid/flask-rebar/blob/63922a17379a5b5a99c7422b7f7b881f8c08eb13/flask_rebar/rebar.py#L818-L827.

This causes tools/plugins that rely on the got_request_exception signal to fail to work with a Rebar service.

Rebar should either

1. Switch the generic error handler to register for InternalServerError; meaning that the rebar handler would only run after the default flask handler had run and sent the got_request_exception signal.
2. Send the got_request_exception signal as part of it's generic exception handling.

I am getting a RecursionError trying to generate swagger with this schema:

class ZipStructureRowSchema(Schema):
    name = fields.String(required=True)
    sub_rows = fields.List(
        fields.Nested("ZipStructureRowSchema"), load_from="subRows", dump_to="subRows"
    )


class ZipPreviewResponseSchema(Schema):
    preview = fields.List(fields.Nested(ZipStructureRowSchema))

As far as I can tell, this is valid according to Marshmallow. Thank you!

I have a custom authenticator that works, but I am trying to get the swagger docs to work as well but I keep getting the above exception.

Here is my custom authenticator:

class CustomAuthenticator(authenticators.Authenticator):
    def authenticate(self):
        if current_user and current_user.is_authenticated:
            pass
        elif current_app.config["AUTH_OFF"]:
            try:
                login_user(default_user)
            except AttributeError:
                raise ValueError("Trying to log into user that doesn't exist!")

This part I took directly from the flask_rebar docs:

class SwaggerAuthConverter(AuthenticatorConverter):
    AUTHENTICATOR_TYPE = CustomAuthenticator

    def get_security_schemes(self, obj, context):
        return {
            obj.name: {sw.type_: sw.api_key, sw.in_: sw.header, sw.name: obj.header}
        }

    def get_security_requirements(self, obj, context):
        return [{obj.name: []}]


custom_auth_registry = AuthenticatorConverterRegistry()
custom_auth_registry.register_type(SwaggerAuthConverter())

Finally, this is where I set up the app:

rebar = Rebar()
swagger_generator = SwaggerV2Generator(authenticator_converter_registry=custom_auth_registry)
registry = rebar.create_handler_registry(swagger_generator=swagger_generator)
registry.set_default_authenticator(CustomAuthenticator)

Here is the full backtrace:

127.0.0.1 - - [04/Nov/2021 09:33:13] "GET /swagger HTTP/1.0" 500 -
Traceback (most recent call last):
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 2464, in __call__
    return self.wsgi_app(environ, start_response)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 2450, in wsgi_app
    response = self.handle_exception(e)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 1867, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 1952, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 1822, in handle_user_exception
    return handler(e)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask_rebar/rebar.py", line 831, in handle_generic_error
    raise error
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 1950, in full_dispatch_request
    rv = self.dispatch_request()
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask/app.py", line 1936, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask_rebar/rebar.py", line 612, in get_swagger
    registry=self, host=request.host_url.rstrip("/")
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask_rebar/swagger_generation/swagger_generator_v2.py", line 100, in generate_swagger
    return self.generate(registry=registry, host=host)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask_rebar/swagger_generation/swagger_generator_v2.py", line 131, in generate
    self.authenticator_converter.get_security_schemes(authenticator)
  File "/home/canopy/canopy/venv/lib/python3.6/site-packages/flask_rebar/swagger_generation/authenticator_to_swagger.py", line 168, in get_security_schemes
    return self.get_security_schemes_legacy(registry=authenticator)
AttributeError: 'AuthenticatorConverterRegistry' object has no attribute 'get_security_schemes_legacy'

Currently (technically, when 2.0.1 is merged and released) it is possible to opt in to response validation at the schema level by inclusion of `RequireOnDumpMixin. While this works, a more natural fit would be to allow this to be directly specified in the schema definition itself by including a "custom class Meta option" as described here: https://marshmallow.readthedocs.io/en/stable/extending.html

https://swagger.io/docs/specification/paths-and-operations/#operationid

operationId is an optional unique string used to identify an operation. If provided, these IDs must be unique among all operations described in your API.

This is important because things downstream of the spec (such as generated clients) may use the operationId (e.g. to code-generate methods) in such a way that depends on unique operationIds.

However, it looks like Rebar's OpenAPI spec generation uses the function name to generate operationIds, which need not be unique:

# my_rebar_registry.py 
from flask_rebar import Rebar

rebar = Rebar()
registry = rebar.create_handler_registry(prefix="/api")

# handlers1.py 
from my_rebar_registry import registry

@registry.handles(
    rule="/foo",
)
def foo():
    return "foo"

# handlers2.py 
from my_rebar_registry import registry

@registry.handles(
    rule="/bar",
)
def foo():
    return "bar"

# my_api_spec.py
import json

from my_rebar_registry import registry
import handlers1
import handlers2

if __name__ == "__main__":
    print(json.dumps(registry.swagger_generator.generate_swagger(registry), indent=2))

> python -m my_api_spec | grep operationId  # thither be duplicates
        "operationId": "foo",
        "operationId": "foo",

{
  "consumes": [
    "application/json"
  ],
  "definitions": {
    "Error": {
      "properties": {
        "errors": {
          "type": "object"
        },
        "message": {
          "type": "string"
        }
      },
      "required": [
        "message"
      ],
      "title": "Error",
      "type": "object"
    }
  },
  "host": "localhost",
  "info": {
    "description": "",
    "title": "My API",
    "version": "1.0.0"
  },
  "paths": {
    "/api/bar": {
      "get": {
        "operationId": "foo",
        "responses": {
          "default": {
            "description": "Error",
            "schema": {
              "$ref": "#/definitions/Error"
            }
          }
        }
      }
    },
    "/api/foo": {
      "get": {
        "operationId": "foo",
        "responses": {
          "default": {
            "description": "Error",
            "schema": {
              "$ref": "#/definitions/Error"
            }
          }
        }
      }
    }
  },
  "produces": [
    "application/json"
  ],
  "schemes": [],
  "securityDefinitions": {},
  "swagger": "2.0"
}

This may not be a terribly big deal in practice, since the moment you try to use such a registry with a Flask app, Flask's own duplication checking (of endpoint names) prevents you from doing so:

# app.py 
from flask import Flask

from my_rebar_registry import rebar
import handlers1
import handlers2

app = Flask("app")
rebar.init_app(app)  # kaboom

> python -m app
Traceback (most recent call last):
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/[email protected]/3.9.6/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/jab/tmp/rebarv2/app.py", line 9, in <module>
    rebar.init_app(app)  # kaboom
  File "/Users/jab/tmp/rebarv2/.venv/lib/python3.9/site-packages/flask_rebar/rebar.py", line 784, in init_app
    registry.register(app=app)
  File "/Users/jab/tmp/rebarv2/.venv/lib/python3.9/site-packages/flask_rebar/rebar.py", line 554, in register
    self._register_routes(app=app)
  File "/Users/jab/tmp/rebarv2/.venv/lib/python3.9/site-packages/flask_rebar/rebar.py", line 576, in _register_routes
    app.add_url_rule(
  File "/Users/jab/tmp/rebarv2/.venv/lib/python3.9/site-packages/flask/app.py", line 98, in wrapper_func
    return f(self, *args, **kwargs)
  File "/Users/jab/tmp/rebarv2/.venv/lib/python3.9/site-packages/flask/app.py", line 1282, in add_url_rule
    raise AssertionError(
AssertionError: View function mapping is overwriting an existing endpoint function: api.foo

However, it still seems preferable to make Rebar's OpenAPI spec generation smart enough to not generate invalid specs in this way. I think Rebar should instead raise an error telling the user to provide a unique function name to avoid generating duplicate operationIds, or perhaps better yet, the @registry.handles() decorators (and so forth) could accept an operation_id param that allows the user to explicitly set the associated operationId that gets generated into the spec, making the operationId no longer tied to the Python function name 1-to-1. (A non-option, IMO, would be for Rebar to silently append some number to deduplicate what would otherwise be a duplicate operationId, which I've seen some Swagger tooling do, and it causes all kinds of madness.)