Skip to content

Mess with paths #93

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

Closed
dbazhal opened this issue Sep 6, 2018 · 8 comments
Closed

Mess with paths #93

dbazhal opened this issue Sep 6, 2018 · 8 comments

Comments

@dbazhal
Copy link

dbazhal commented Sep 6, 2018

If you have url pattern in flask, you'll never get your paths validated.

import yaml
from flask import Flask, request
from openapi_core import create_spec
from openapi_core.validation.request.validators import RequestValidator
from openapi_core.wrappers.flask import FlaskOpenAPIRequest

spec = """openapi: "3.0.0"
info:
    version: 1.0.0
    title: test
servers:
- url: http://127.0.0.1:8000
components:
    parameters:
        some_param:
            name: some
            in: path
            required: true
            schema:
                type: string
paths:
    /{some}/path/:
        parameters:
        - $ref: '#/components/parameters/some_param'
        post:
            summary: anything
"""
TEST_SPEC = create_spec(spec)

FLASK_APP = Flask(__name__)

@FLASK_APP.route('/<some>/path', methods=['POST'])
def duh(some: str):
    validator = RequestValidator(TEST_SPEC)
    result = validator.validate(FlaskOpenAPIRequest(request))
    print(result.errors)

Will always get an error

[InvalidOperation('Unknown operation path /<some>/path with method post')]

Because flask url pattern syntax and openapi pattern syntax do not match.
An error hides here https://github.com/p1c2u/openapi-core/blob/master/openapi_core/validation/request/validators.py#L24

  1. path_pattern is not transformed to openapi format
  2. request.path_pattern is not used
@GlennS
Copy link

GlennS commented Sep 25, 2018

I find that it works if you leave out the type.

So instead of /some/url/<string:myParameter>/ you can write /some/url/<myParameter>/.

@dbazhal
Copy link
Author

dbazhal commented Sep 25, 2018

@GlennS I'm confused a little - can't find any type notation in my paths definitions. Could you please explain what do you mean by that?

@GlennS
Copy link

GlennS commented Oct 11, 2018

I meant in the Flask route itself.

So, instead of the usual:
@FLASK_APP.route('/<string:some>/path', methods=['POST'])

Use:
@FLASK_APP.route('/<some>/path', methods=['POST'])

@stephenfin
Copy link
Contributor

I've a similar issue with Django, which up until 2.0 only supported regex-style URL templates. Since 2.0, it supports and recommends URLs closer to what flask uses, albeit with optional type annotations (or path converters in Django lingo).

I have worked around this locally by hacking together something that will find and extract the regex used to resolve the provided path, then convert that regex to the template string style that openapi-core expects. However, this is hacky, relies on Django internals that are subject to change, and tightly couples how URLs are defined in my urls.py with how they're defined in the spec (i.e. my named groups in the regex must have the same name as the parameters in the spec, even though they're just placeholders/tokens).

I think it would be a good idea if openapi-core converted each URL template from the spec to a basic regex and stored these in openapi_core.schemas.specs.models.Spec.paths. This would allow us to deprecate the openapi_core.wrappers.base.BaseOpenAPIRequest.path_pattern property, as that would no longer be necessary. I'm happy to work on this but I'd like to know that it will be looked at before I do 😄 As such, does this sound somewhat reasonable?

@stephenfin
Copy link
Contributor

As an aside, this sounds like a dupe of #35 and should probably be deprecated in favour of that.

@p1c2u
Copy link
Collaborator

p1c2u commented Nov 15, 2018

@stephenfin yup. Closing it.Duplicate of #35.

@p1c2u p1c2u closed this as completed Nov 15, 2018
@ghost
Copy link

ghost commented Nov 15, 2018

I like that idea as we can couple the schema properties with the parameter place holder:
type: number -> /[0-9\.,+-]+/
type: string -> [^/]+
type: string with pattern -> copy the pattern

etc.
But you still need to query Django's resolver and somehow match it to ascertain if the URL exists?

@stephenfin
Copy link
Contributor

Hmm, depends on how you want to do the testing. I personally have a plethora of tests already in place and I was simply using the Response object (which contains the request) returned by Django REST Framework's APIClient to test things. Since I've already made the request with APIClient, I have triggered the resolver and returned the appropriate HTTP code. Maybe that's not what's expected here though?

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

No branches or pull requests

4 participants