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

validate_json_api_content_type helper has confusing name vs expected use #18

Open
sergiofenoll opened this issue Nov 10, 2023 · 0 comments

Comments

@sergiofenoll
Copy link

sergiofenoll commented Nov 10, 2023

Context

The validate_json_api_content_type helper checks whether the Content-Type header of a passed in request is JSON:API and otherwise returns an error response object.

Because it just returns an error object that the user needs to handle manually (as opposed to the automatic halting of the request handling as implemented in the ruby-template) its use is a bit confusing.

from helpers import validate_json_api_content_type
from flask import request

@app.route("/")
def index():
    validate_json_api_content_type(request)
    return "Hello from the mu-python-template!"

Given the above code and based on the function name and its docstring one might assume that the route handler would automatically return an error if the request didn't contain JSON:API, but in fact it does nothing. The proper code would look something like this:

from helpers import validate_json_api_content_type
from flask import request

@app.route("/")
def index():
    maybe_error = validate_json_api_content_type(request)
    if maybe_error:
        return maybe_error
    return "Hello from the mu-python-template!"

Possible solutions

Make validate_json_api_content_type raise an exception instead of returning an error object. Users of the template can then either manually catch and handle the raised exception as they deem fit, or add an error handler to their service that automatically handles all such errors:

# In template: helpers.py
class InvalidJSONAPIContentTypeException(Exception):
    """Request does not contain valid JSON:API"""

def validate_json_api_content_type(request):
    """Validate whether the request contains the JSONAPI content-type header (application/vnd.api+json). Raises an exception of type InvalidJSONAPIContentTypeException otherwise"""
    if "application/vnd.api+json" not in request.content_type:
        raise InvalidJSONAPIContentTypeException("Content-Type must be application/vnd.api+json instead of " + request.content_type)

# In service: web.py
from helpers import validate_json_api_content_type, InvalidJSONAPIContentTypeException
from flask import request

@app.route("/")
def index():
    try:
        validate_json_api_content_type(request)
    except InvalidJSONAPIContentTypeException as e:
        return error(str(e), 400)
    return "Hello from the mu-python-template!"

# Or a service-level error handler, which would make the above try-except unnecessary

@app.errorhandler(InvalidJSONAPIContentTypeException)
def handle_invalid_json_api_content_type(e):
    return error(str(e), 400)

Alternatively, there might be a way to make the validate_json_api_content_type abort the request using some Flask function, emulating the behaviour of the ruby-template.

The helper can also be renamed and its documentation can be clarified to make its usage more obvious.

EDIT: the reasoning for opening an issue that I've found one place in Kaleidos code where someone wrote validate_json_api_content_type(request) and it effectively did nothing, making me assume they got confused about what the function actually did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant