check_unknown_fields raises ValidationError if passed a dump_only field even if value is 'missing'

[lafrech]

I'm using webargs to parse API requests using the Schema I get from the Document and I have an issue with dump_only fields.

Consider a document/resource user with dump_only id field.

When sending a request like this:

http POST http://127.0.0.1:5000/user/ name=Test

webargs searches for all attributes in the query locations and original_data in check_unknown_fields is:

{'id': <marshmallow.missing>, 'name': 'Test'}

Then, since id is not in loadable_fields, check_unknown_fields raises an error.

    @validates_schema(pass_original=True)
    def check_unknown_fields(self, data, original_data):
        loadable_fields = [k for k, v in self.fields.items() if not v.dump_only]
        for key in original_data:
            if key not in loadable_fields:
                raise ValidationError(_('Unknown field name {field}.').format(field=key))

This happens when I'm reusing the Schema in webargs, not on typical umongo usage like in the Flask example.

Possible fixes/workarounds:

Stick to the version of check_unknown_fields suggested in Marshmallow's documentation:

    @validates_schema(pass_original=True)
    def check_unknown_fields(self, data, original_data):
        for key in original_data:
            if key not in self.fields:
                raise ValidationError('Unknown field name {}'.format(key))

Doing this removes the error but also won't raise any error for

http POST http://127.0.0.1:5000/user/ name=Test id=whatever

If we want to get an error for this, then another fix could be

    @validates_schema(pass_original=True)
    def check_unknown_fields(self, data, original_data):
        for key, value in original_data.items():
            if key not in self.fields:
                raise ValidationError(_('Unknown field name {field}.').format(field=key))
            if self.fields[key].dump_only and value is not missing:
                raise ValidationError(_('Field {field} is dump_only.').format(field=key))

which also raises a different message for dump_only and unknown fields.

Another fix would be to stop webargs from parsing dump_only fields in the first place so that they don't appear in original_data. After all, I don't see the point in adding those fields since they are ignored in the next step.

It would happen here:

            for argname, field_obj in iteritems(argdict):
                argname = field_obj.load_from or argname
                parsed_value = self.parse_arg(argname, field_obj, req, locations)
                parsed[argname] = parsed_value

This could be changed into

            for argname, field_obj in iteritems(argdict):
                if field_obj.dump_only:
                    continue
                argname = field_obj.load_from or argname
                parsed_value = self.parse_arg(argname, field_obj, req, locations)
                parsed[argname] = parsed_value

But this would also silently mask cases where data is provided to a dump_only field:

http POST http://127.0.0.1:5000/user/ name=Test id=whatever

so maybe this is not any better.

Besides, I'm not familiar enough with webargs' insides to be confident with this change.

Anyway, if we want to raise an error if a dump_only field receives a value, we should make sure it is not "missing".

What do you think about the value is not missing test alternative?

[touilleMan]

implemented in 857d730