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

Semantics of validation from path has changed #411

Closed
dairiki opened this issue Oct 26, 2016 · 3 comments
Closed

Semantics of validation from path has changed #411

dairiki opened this issue Oct 26, 2016 · 3 comments

Comments

@dairiki
Copy link
Collaborator

dairiki commented Oct 26, 2016

In Cornice < 2, when a schema item had location=path, it was extracted/validated from request.matchdict. This allowed for code like

class MySchema(colander.Schema):
    item_id = colander.SchemaNode(
        colander.Integer(),
        location='path')

@resource(
    path='/items/{item_id}.json',
    schema=MySchema)
class ItemResource(object):
    def __init__(self, request):
        self.request = request

    def get(self):
        item_id = request.validated['item_id']
        return {'id': item_id}

In Cornice >= 2, I would naively translate the above to

class PathSchema(colander.Schema):
    item_id = colander.SchemaNode(colander.Integer())

class MySchema(colander.Schema):
    path = PathSchema()

@resource(
    path='/items/{item_id}.json',
    schema=MySchema,
    validators=(colander_validator,))
class ItemResource(object):
    def __init__(self, request):
        self.request = request

    def get(self):
        item_id = request.validated['path']['item_id']
        return {'id': item_id}

This does not, however, seem to work. Currently, extract_cstruct puts request.path (a string) into cstruct['path'] (and does not appear to make request.matchdict available at all.)

I would suggest that extract_cstruct should be modified to make request.matchdict available in the cstruct; either as cstruct['path'] (replacing request.path) or as cstruct['matchdict'].

Am I missing something?

@leplatrem
Copy link
Contributor

Your feedback seems totally valid!

In Cornice 1.2, we used request.matchdict
https://github.com/Cornices/cornice/blob/1.2.1/cornice/util.py#L187

I'm open to both approaches: request['path'] as before, or in a new key 'matchdict'. The first one would just be a simple bug fix, the other one is a new feature with the required details in the upgrading docs.

Would you be willing to open a pull-request with the small fix please?

@dairiki
Copy link
Collaborator Author

dairiki commented Oct 26, 2016

Yes, I can come up with a pull request.

(Upon further consideration, I think my first approach is the right one. Otherwise the semantics of 'path' change for error reporting as well.)

@Natim
Copy link
Contributor

Natim commented Oct 26, 2016

However I thought the change for the error reporting part was done on purpose.

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

3 participants