Custom error message for unknown when dump only

[choco]

I'm just getting started with marshmallow so if there's a more elegant way to solve the issue please let me know. I have the following simple model and schema.

class Instance(db.Model):
    __tablename__ = 'instances'

    id = db.Column(db.Integer, primary_key=True)
    hostname = db.Column(db.Unicode, index=True, unique=True, nullable=False)
    port = db.Column(db.Integer, nullable=False)

    def __repr__(self):
        return '<Instance: {}>'.format(self.hostname)

class InstanceSchema(Schema):
    id = fields.Integer(dump_only=True)
    hostname = fields.String(
        required=True,
        error_messages={'required': {'message': 'Hostname required', 'code': 400}}
    )
    port = fields.Integer(
        required=True,
        error_messages={'required': {'message': 'Port required', 'code': 400}}
    )
    @validates_schema
    def at_least_one(self, data):
        if self.partial and not len(data.keys()):
            raise ValidationError('Provide at least one valid key')

As you can see id is dump_only. If I try to load a simple dict (in the real application this would be data of a PATCH request)

try:
    result = InstanceSchema(partial=True).load({'id': 4})
    print(result)
except ValidationError as err:
    print(err.messages)

I'll get that id is an unknown field. So I have to manually check for it to provide a better error message like

    if "id" in err.messages:
        err.messages["id"] = "id is readonly"

It would be cool if it was possible to have a custom error message like hostname or port like

id = fields.Integer(dump_only=True,
    error_messages={'dump_only': {'message': 'id is readonly', 'code': 400}}
)

[deckar01]

This seems like it would be consistent with the other field validation error messages since the dump_only option can be declared when creating a field instance.

We may also want to consider using a more specific default error message for dump only errors since unknown isn't exactly true if the consumer is aware of the schema output.

This should be a lot easier to implement now that the marshalling logic is in the schema. Previously the marshaller only had access to a filtered set of fields.

[choco]

I'm not familiar with your codebase, but something like this seems to work

diff --git a/marshmallow/fields.py b/marshmallow/fields.py
index 01cff48..29f7831 100644
--- a/marshmallow/fields.py
+++ b/marshmallow/fields.py
@@ -122,6 +122,7 @@ class Field(FieldABC):
         'required': 'Missing data for required field.',
         'null': 'Field may not be null.',
         'validator_failed': 'Invalid value.',
+        'dump_only': 'Field may not be deserialized.',
     }
 
     def __init__(
diff --git a/marshmallow/schema.py b/marshmallow/schema.py
index 3079a82..56432b5 100644
--- a/marshmallow/schema.py
+++ b/marshmallow/schema.py
@@ -623,8 +623,6 @@ class BaseSchema(base.SchemaABC):
         else:
             partial_is_collection = is_collection(partial)
             for attr_name, field_obj in iteritems(fields_dict):
-                if field_obj.dump_only:
-                    continue
                 field_name = attr_name
                 if field_obj.data_key:
                     field_name = field_obj.data_key
@@ -636,6 +634,13 @@ class BaseSchema(base.SchemaABC):
                         (partial_is_collection and attr_name in partial)
                     ):
                         continue
+                elif field_obj.dump_only:
+                    try:
+                        field_obj.fail('dump_only')
+                    except ValidationError as err:
+                        error_store.store_error(err.messages, field_name, index=index)
+                    continue
+
                 d_kwargs = {}
                 if isinstance(field_obj, Nested):
                     # Allow partial loading of nested schemas.
@@ -665,7 +670,6 @@ class BaseSchema(base.SchemaABC):
                 fields = {
                     field_obj.data_key or field_name
                     for field_name, field_obj in fields_dict.items()
-                    if not field_obj.dump_only
                 }
                 for key in set(data) - fields:
                     value = data[key]

But it's not really clean since it's basically try catching a function that we know will fail.... We could access the errors_messages dict in the field directly but then you would need to duplicate the key check in the fail function, also I don't know what kind of separation you may want to maintain.

EDIT: oops, updated code since it didn't even check if dump_only field had a value

[andrewwhitehead]

I don't think I understand the behaviour of dump_only, shouldn't it simply ignore these fields when loading a serialized value? A minimal example:

from marshmallow import Schema, fields

class S(Schema):
	test = fields.Constant("test", dump_only=True)

dumped = S().dump({})
print("Serialized value:", dumped)  # produces {'test': 'test'}

loaded = S().load(dumped)  # raises a ValidationError, why?

[deckar01]

@cywolf unknown defaults to RAISE. Since this field is not allowed to be loaded, it raises an error if it is provided. The "unknown" message is an artifact of the implementation, which this issue is discussing. Raising errors when unexpected fields are encountered helps developer catch bugs instead of discarding the data and hiding the bugs. If you don't want this protection, you can set unknown to IGNORE. See #875 for more info.

[Morikko]

I would even suggest that unknown=RAISE option with dump-only fields would deserve a default message like {'my_dump_only_field': ['Read-Only field.']} instead of the generic {'my_dump_only_field': ['Unknown field.']}.

The reason is that this field is known by the schema writer. Also it could help the users to better understand why they could see the value once dumped/returned but they could not update it (especially through an API).