Putting values in `request.validated` before calling colander_body_validator
Opened this issue · 3 comments
Let say we have a service:
service = Service(name='service', path='/service')
class ColSchema(MappingSchema):
field = SchemaNode(String())
def custom_validator(request, **kwargs):
# do some validation and put something in request.validated
request.validated['something'] = 'something'
@service.post(schema=ColSchema(), validators=(custom_validator, colander_body_validator))
def service_handler(request):
request.validated['something'] # <---- raises KeyError
return {}
Getting something
from request.validated raises KeyError because of the line https://github.com/Cornices/cornice/blob/master/cornice/validators/_colander.py#L71
which overwrites request.validated with request.validated['body'].
Fix for that would be just replacing line 71 in the cornice/validators/_colander.py:
request.validated = request.validated.get(location, {})
with
validated_location = request.validated.get(location, {})
request.validated.update(validated_location)
request.validated.pop(location, None)
but ... there is something that this change breaks.
What if someone sets schema which inherits from SequenceSchema rather than MappingSchema? Current code just rewrites request.validated (which is instantiated as dict in pyramidhook.py wrap_request()) with a list. I'm not sure if this should work like that or I'm just missing something?
Since we have that here:
cornice/cornice/pyramidhook.py
Lines 148 to 149 in c8d3ad6
I would assume that request.validated
should be a dict
.
Even the colander code should assume to find a dict there, no?
We can also explicitly document that a SequenceSchema cannot be used.
I like your idea of not replacing the existing stuff, please go on ;)
That's OK for me :)
But ... there is a test:
cornice/tests/test_validation.py
Lines 419 to 426 in c8d3ad6
which relay on that behavior. I'm not sure it should because of this default value (empty dict) which would suggest that it should stay dict.
I can remove that test or change it to MappingSchema
.
This change would potentially break compatibility. If that's ok I can prepere PR with a solution.
Thing is that if someone would want to use SequenceSchema
he can, but it needs to be used with colander_validator
and than it will be deserialized to request.validated['body']
and request.validated
will still remain a dict
.
This change would potentially break compatibility. If that's ok I can prepere PR with a solution.
Yes, we can bump a major version, no problem.
Thing is that if someone would want to use SequenceSchema he can, but it needs to be used with colander_validator and than it will be deserialized to request.validated['body'] and request.validated will still remain a dict.
Makes sense! Yes!