Repository: Scille/umongo (python)

Issue:
Rework  _extract_marshmallow_field_params
I think we should rework `_extract_marshmallow_field_params` to avoid copying `default`, `missing`, `load_from`, `dump_to` and perhaps other attributes.

Current version:

```py
    def _extract_marshmallow_field_params(self, mongo_world):
        params = {field: getattr(self, field)
                  for field in ('default', 'load_from', 'validate',
                                'required', 'allow_none', 'load_only',
                                'dump_only', 'missing', 'error_messages')}
        if mongo_world and self.attribute:
            params['attribute'] = self.attribute
        params.update(self.metadata)
        return params
```

This is partly addressed in https://github.com/Scille/umongo/pull/107 (only `default` and `missing` are dealt with).

Copying `load_from` seems like a mistake to me because it is used to map an OO world name to a Mongo name, so using it to map an outside world name to an OO world name is bound to fail. If we want to keep flexibility, we could do the same as in #107: add `marshmallow_load_from` attribute, maybe we could even blindly pass all attributes prefixed with `marshmallow_`.

We should finalize #107 and complete it to remove other attributes, at least `load_from` and `dump_to`, maybe others (feedback welcome).

Not sure when I'll get the time to do that.

FWIW, here's the monkeypatch I'm using in my code for now.

```py
def _extract_marshmallow_field_params(self, mongo_world):
    params = {field: getattr(self, field)
              for field in ('validate', 'required', 'allow_none',
                            'load_only', 'dump_only', 'error_messages')}
    if mongo_world and self.attribute:
        params['attribute'] = self.attribute
    params.update(self.metadata)
    return params
BaseField._extract_marshmallow_field_params = _extract_marshmallow_field_params
```


Pull request:
Rework extract marshmallow field params
Closes #168.

Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 1b69669..d10faee 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -7,7 +7,14 @@ dev
 * ``missing`` attribute is no longer used in umongo fields, only ``default`` is
   used. ``marshmallow_missing`` and ``marshmallow_default`` attribute can be
   used to overwrite the value to use in the pure marshmallow field returned
-  by `as_marshmallow_field` method.
+  by ``as_marshmallow_field`` method.
+* ``as_marshmallow_field`` does not pass ``load_from``, ``dump_to`` and
+  ``attribute`` to the pure marshmallow field anymore. It only passes
+  ``validate``, ``required``, ``allow_none``, ``dump_only``, ``load_only`` and
+  ``error_messages``, as well as ``default`` and ``missing`` values inferred
+  from umongo's ``default``. Parameters prefixed with ``marshmallow_`` in the
+  umongo field are passed to the pure marshmallow field and override their
+  non-prefixed counterpart.
 
 1.2.0 (2019-02-08)
 ------------------
diff --git a/umongo/abstract.py b/umongo/abstract.py
index a8a1096..0778836 100644
--- a/umongo/abstract.py
+++ b/umongo/abstract.py
@@ -117,7 +117,29 @@ class BaseField(ma_fields.Field):
                 "generating pure Marshmallow field.")
         if 'default' in kwargs:
             kwargs['missing'] = kwargs['default']
+
+        # Store attributes prefixed with marshmallow_ to use them when
+        # creating pure marshmallow Schema
+        for attribute in (
+            'load_from', 'dump_to', 'attribute',
+            'validate', 'required', 'allow_none',
+            'load_only', 'dump_only', 'error_messages'
+        ):
+            attribute = 'marshmallow_' + attribute
+            if attribute in kwargs:
+                setattr(self, attribute, kwargs.pop(attribute))
+
+        # Infer from "default" parameter a default value for
+        # marshmallow_default and marshmallow_missing
+        def serialize_default():
+            val = self.default() if callable(self.default) else self.default
+            return self.serialize('foo', {'foo': val})
+
+        self.marshmallow_missing = kwargs.pop('marshmallow_missing', serialize_default)
+        self.marshmallow_default = kwargs.pop('marshmallow_default', serialize_default)
+
         super().__init__(*args, **kwargs)
+
         # Overwrite error_messages to handle i18n translation
         self.error_messages = I18nErrorDict(self.error_messages)
         # `io_validate` will be run after `io_validate_resursive`
@@ -128,13 +150,6 @@ class BaseField(ma_fields.Field):
         self.unique = unique
         self.instance = instance
 
-        def serialize_default():
-            val = self.default() if callable(self.default) else self.default
-            return self.serialize('foo', {'foo': val})
-
-        self.marshmallow_missing = kwargs.get('marshmallow_missing', serialize_default)
-        self.marshmallow_default = kwargs.get('marshmallow_default', serialize_default)
-
     def __repr__(self):
         return ('<fields.{ClassName}(default={self.default!r}, '
                 'attribute={self.attribute!r}, '
@@ -213,18 +228,26 @@ class BaseField(ma_fields.Field):
         return {self.attribute or key: query}
 
     def _extract_marshmallow_field_params(self, mongo_world):
-        params = {field: getattr(self, field)
-                  for field in ('load_from', 'validate', 'required',
-                                'allow_none', 'load_only',
-                                'dump_only', 'error_messages')}
+        params = {
+            attribute: getattr(self, attribute)
+            for attribute in (
+                'validate', 'required', 'allow_none',
+                'dump_only', 'load_only', 'error_messages'
+            )
+        }
         if mongo_world and self.attribute:
             params['attribute'] = self.attribute
-        # Marshmallow doesn't serialize default value when doing `dump`,
-        # hence we use the serialized value..
-        params['default'] = self.marshmallow_default
-        # ...on the other hand, marshmallow does use unserialize on the
-        # missing value when doing `load`, hence also use the serialized value
-        params['missing'] = self.marshmallow_missing
+
+        # Override uMongo attributes with marshmallow_ prefixed attributes
+        for attribute in (
+            'default', 'missing', 'load_from', 'dump_to', 'attribute',
+            'validate', 'required', 'allow_none',
+            'load_only', 'dump_only', 'error_messages'
+        ):
+            ma_attribute = 'marshmallow_' + attribute
+            if hasattr(self, ma_attribute):
+                params[attribute] = getattr(self, ma_attribute)
+
         params.update(self.metadata)
         return params
 


Test patch:
diff --git a/tests/test_marshmallow.py b/tests/test_marshmallow.py
index 919e8a8..f58aba4 100644
--- a/tests/test_marshmallow.py
+++ b/tests/test_marshmallow.py
@@ -133,6 +133,59 @@ class TestMarshmallow(BaseTest):
         assert ma_custom_base_schema.Meta.exclude == ('content',)
         assert ma_custom_base_schema.Meta.dump_only == ('id',)
 
+    def test_as_marshmallow_field_pass_params(self):
+        @self.instance.register
+        class MyDoc(Document):
+            lf = fields.IntField(marshmallow_load_from='lflf')
+            dt = fields.IntField(marshmallow_dump_to='dtdt')
+            at = fields.IntField(marshmallow_attribute='atat')
+            re = fields.IntField(marshmallow_required=True)
+            an = fields.IntField(marshmallow_allow_none=True)
+            lo = fields.IntField(marshmallow_load_only=True)
+            do = fields.IntField(marshmallow_dump_only=True)
+            va = fields.IntField(marshmallow_validate=validate.Range(min=0))
+            em = fields.IntField(marshmallow_error_messages={'invalid': 'Wrong'})
+
+        MyMaDoc = MyDoc.schema.as_marshmallow_schema()
+
+        assert MyMaDoc().fields['lf'].load_from == 'lflf'
+        assert MyMaDoc().fields['dt'].dump_to == 'dtdt'
+        assert MyMaDoc().fields['at'].attribute == 'atat'
+        assert MyMaDoc().fields['re'].required is True
+        assert MyMaDoc().fields['an'].allow_none is True
+        assert MyMaDoc().fields['lo'].load_only is True
+        assert MyMaDoc().fields['do'].dump_only is True
+        _, err = MyMaDoc().load({'va': -1})
+        assert 'va' in err
+        assert MyMaDoc().fields['em'].error_messages['invalid'] == 'Wrong'
+
+    def test_as_marshmallow_field_infer_missing_default(self):
+        @self.instance.register
+        class MyDoc(Document):
+            de = fields.IntField(default=42)
+            mm = fields.IntField(marshmallow_missing=12)
+            md = fields.IntField(marshmallow_default=12)
+            mmd = fields.IntField(default=42, marshmallow_missing=12)
+            mdd = fields.IntField(default=42, marshmallow_default=12)
+
+        MyMaDoc = MyDoc.schema.as_marshmallow_schema()
+
+        data, _ = MyMaDoc().load({})
+        assert data == {
+            'de': 42,
+            'mm': 12,
+            'mmd': 12,
+            'mdd': 42,
+        }
+
+        data, _ = MyMaDoc().dump({})
+        assert data == {
+            'de': 42,
+            'md': 12,
+            'mmd': 42,
+            'mdd': 12,
+        }
+
     def test_as_marshmallow_schema_cache(self):
         ma_schema_cls = self.User.schema.as_marshmallow_schema()