Repository: Scille/umongo
Pull request #107 (2017-05-05 09:50:23)

Rework default/missing attributes behavior
see #36 

coveralls: 
[![Coverage Status](https://coveralls.io/builds/11385918/badge)](https://coveralls.io/builds/11385918)

Coverage decreased (-0.07%) to 95.678% when pulling **273d8f326a3b118c5ce4a0d04ccdb36c03e727a2 on rework-default** into **fd3490b7c0fd7a22fbfed95a6619806aa43469bc on master**.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/11386182/badge)](https://coveralls.io/builds/11386182)

Coverage decreased (-0.07%) to 95.68% when pulling **a89be53881a60570e2ee07c55a5130d56befcce3 on rework-default** into **fd3490b7c0fd7a22fbfed95a6619806aa43469bc on master**.


Patch:
diff --git a/.travis.yml b/.travis.yml
index 0738e93..8ba1e79 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -30,9 +30,9 @@ deploy:
   distributions: sdist bdist_wheel
   user: touilleMan
   password:
-    secure: cQN5qW1k5kMQprmgIWiqjXyvH5SdZCs93a1f5NVquo/NsxFvwvtm6J3Zmp8NaC4tvRzSpyolZyW+5o0ISXBMiLPpv4cqGHdSaKfzWRWtTUy+JdQjcZxcPi7vD7E+HJha1+7gjpoX8qXD8NyOlWr4VFhQxWd77cFA19tgIvhbDCMN1BadaCObNDI/D8jomVygGH/G5bk4QQc9+xl7JSbBzGcncEjUXa83WSFgXXuQtnFchArR12YQQSVN4Y1Rnm215haatJ511oGEby3PAkUw4kaScbW/lo8w2hAY/p9YRAV9+5SXASDCdta53opawoX9rFfGCiZhuil5XJCxz29IqLD5TtcWOyzte45Soz7VYf7wWdmokMCxZO3lMv6IH275VdZOYrDRrRAM9pO0HxPF5K6hZxi4tZgSsFgdV9t4qfjpQp0cazseCpiCSRtWbNWYXkrdm+LWnbIkC1Oz2np4Ehw+rEpRHZyd31ENyhKhG1n6GNuZrtTjIeRYnTolV5mOudZqvib8egqtck4ehWrA4rgJtYwzoLKEI2OtRYxjLaLrOKYn5rGrDM1ayeFQA13NCJ5LqCv6qFtswYVA96bKbB66HAygySeQZr/lYlgzAbmLi/bVV7JO+auG79RWyyr3n0Yz9rmovNuWEBCbAei3ghL8sRx2a9ifEveXVh3ZK0U=
+    secure: N5oj/7qsMQykhuhGE9uXEEyhZXDzRceBV047Z1iFxZKPv2hzimrTqY0Zb87hIeSDCmQEowtVsSD40vSHJ9bqdOKmiCYKRLwgAHDs5RepAtszZmCLNWQS/oiwIXrGzg3kqitxvaym1p4aK4ykUUQo8mBsii8WKxefa5esAZG49ovOEPKaywtAiC7De48ChYoxUnJhcKVDxUTBntORsP81F7gmibbZVeB97nVOzmjWnwlfVrEZ83Oq/rGpbPiK/ydYKonrmDVfrHGuhNS41ZpXqR0UlDSa7OWuw/dB5jZQDUtw57la/7FLuBtoGP5EbOUsWiy6+Kc3pEWrx2EupLKILzVezL27frTW41Y7mkhxkS2gu72BlEIdzUqUG+1/MVWzFbidn7auQ7DZeWH6/rFER/PmCzg7Mfkkomgu/Avqcu+sS4ZevVQfN67Lawk8rHvD8g79xSopfr4W55enss4QA5Kf2ICH3mjBuP9PubgJplHyqHf2hvtGc0ycvVxogzk3Ss7fIxg1UuonuW01Se28Fh06cMYaH0qjs06QDisv9qxrM1TTK81yRbzPqAyjxkqpXLLh8pM5BehkBwel7uVT4obDwvgk/mOpC4XKNSLUa6TjSApGe1QorA3E6kD+6Cr/4YLEcFYUlUtF6JFkT3KkqNlYaDNEM5RhRyeeaUYuIEU=
   on:
     tags: true
     repo: Scille/umongo
     python: 3.6
-  skip_existing: true
+  skip_existing: True
diff --git a/HISTORY.rst b/HISTORY.rst
index a12ea56..1b69669 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -2,6 +2,13 @@
 History
 =======
 
+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.
+
 1.2.0 (2019-02-08)
 ------------------
 
diff --git a/docs/userguide.rst b/docs/userguide.rst
index 2211456..f49de4c 100644
--- a/docs/userguide.rst
+++ b/docs/userguide.rst
@@ -55,13 +55,13 @@ First let's define a document with few :mod:`umongo.fields`
     @instance.register
     class Dog(Document):
         name = fields.StrField(required=True)
-        breed = fields.StrField(missing="Mongrel")
+        breed = fields.StrField(default="Mongrel")
         birthday = fields.DateTimeField()
 
 First don't pay attention to the ``@instance.register``, this is for later ;-)
 
 Note that each field can be customized with special attributes like
-``required`` (which is pretty self-explanatory) or ``missing`` (if the
+``required`` (which is pretty self-explanatory) or ``default`` (if the
 field is missing during unserialization it will take this value).
 
 Now we can play back and forth between OO and client worlds
@@ -327,7 +327,7 @@ and you're good to go:
     >>> @instance.register
     ... class Dog(Document):
     ...     name = fields.StrField(attribute='_id')
-    ...     breed = fields.StrField(missing="Mongrel")
+    ...     breed = fields.StrField(default="Mongrel")
 
 Of course the way you'll be calling methods will differ:
 
@@ -554,7 +554,7 @@ For more simple usecases we could use the
     >>> @instance.register
     ... class Dog(Document):
     ...     name = fields.StrField(required=True)
-    ...     breed = fields.StrField(missing="Mongrel")
+    ...     breed = fields.StrField(default="Mongrel")
     ...     birthday = fields.DateTimeField()
     ...     @post_dump
     ...     def customize_dump(self, data):
@@ -591,8 +591,8 @@ from scratch... almost:
 
 .. note:: A custom marshmallow schema :class:`umongo.marshmallow_bonus.SchemaFromUmongo`
     can be used instead of regular :class:`marshmallow.Schema` to benefit a tighter
-    integration with umongo (unknown field checking and access to missing fields
-    instead of serializing them as `None`)
+    integration with umongo (unknown field checking and field with missing value
+    actually return the ``missing`` singleton instead of serializing it as `None`)
 
 This time we directly convert umongo schema's fields into there marshmallow
 equivalent with ``as_marshmallow_field``. Now we can build our ducks easily:
@@ -611,6 +611,37 @@ equivalent with ``as_marshmallow_field``. Now we can build our ducks easily:
         # Error handling
         ...
 
+One final thought: field's ``missing`` and ``default`` attributes are not handled the same in
+marshmallow and umongo.
+
+In marshmallow ``default`` contains the value to use during serialization
+(i.e. calling ``schema.dump(doc)``) and ``missing`` the value for deserialization.
+ where the field is
+
+In umongo however there is only a ``default`` attribute which will be used when
+creating (or loading from user world) a document where this field is missing.
+This is because you don't need to control how umongo will store the document in
+mongo world.
+
+So when you use ``as_marshmallow_field``, the resulting marshmallow field's
+``missing``&``default`` will be by default both infered from the umongo's
+``default`` field. You can want to overwrite this behavior by using
+``marshmallow_missing``/``marshmallow_default`` attributes:
+
+.. code-block:: python
+
+    @instance.register
+    class Employee(Document):
+        name = fields.StrField(default='John Doe')
+        birthday = fields.DateTimeField(marshmallow_missing='2000-01-01T00:00:00Z')
+        # You can use `missing` singleton to overwrite `default` field inference
+        skill = fields.StrField(default='Dummy', marshmallow_default=missing)
+
+    ret = Employee.schema.as_marshmallow_schema()().load({})
+    assert ret.data == {'name': 'John Doe', 'birthday': datetime(2000, 1, 1, 0, 0, tzinfo=tzutc()), 'skill': 'Dummy'}
+    ret = Employee.schema.as_marshmallow_schema()().dump({})
+    assert ret.data == {'name': 'John Doe', 'birthday': '2000-01-01T00:00:00+00:00'}  # Note `skill` hasn't been serialized
+
 
 Field validate & io_validate
 ============================
diff --git a/umongo/__init__.py b/umongo/__init__.py
index b356f50..de23ff1 100644
--- a/umongo/__init__.py
+++ b/umongo/__init__.py
@@ -1,3 +1,4 @@
+from marshmallow import missing
 from .instance import Instance
 from .frameworks import (
     PyMongoInstance,
@@ -38,6 +39,8 @@ __author__ = 'Emmanuel Leblond'
 __email__ = 'emmanuel.leblond@gmail.com'
 __version__ = '1.2.0'
 __all__ = (
+    'missing',
+
     'Instance',
     'PyMongoInstance',
     'TxMongoInstance',
diff --git a/umongo/abstract.py b/umongo/abstract.py
index e313abf..a8a1096 100644
--- a/umongo/abstract.py
+++ b/umongo/abstract.py
@@ -110,6 +110,13 @@ class BaseField(ma_fields.Field):
     }
 
     def __init__(self, *args, io_validate=None, unique=False, instance=None, **kwargs):
+        if 'missing' in kwargs:
+            raise RuntimeError("uMongo doesn't use `missing` argument, use `default` "
+                "instead and `marshmallow_missing`/`marshmallow_default` "
+                "to tell `as_marshmallow_field` to use a custom value when "
+                "generating pure Marshmallow field.")
+        if 'default' in kwargs:
+            kwargs['missing'] = kwargs['default']
         super().__init__(*args, **kwargs)
         # Overwrite error_messages to handle i18n translation
         self.error_messages = I18nErrorDict(self.error_messages)
@@ -121,12 +128,21 @@ 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}, '
                 'validate={self.validate}, required={self.required}, '
                 'load_only={self.load_only}, dump_only={self.dump_only}, '
-                'missing={self.missing}, allow_none={self.allow_none}, '
+                'marshmallow_missing={self.marshmallow_missing}, '
+                'marshmallow_default={self.marshmallow_default}, '
+                'allow_none={self.allow_none}, '
                 'error_messages={self.error_messages}, '
                 'io_validate={self.io_validate}, '
                 'io_validate_recursive={self.io_validate_recursive}, '
@@ -198,11 +214,17 @@ class BaseField(ma_fields.Field):
 
     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')}
+                  for field in ('load_from', 'validate', 'required',
+                                'allow_none', 'load_only',
+                                'dump_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
         params.update(self.metadata)
         return params
 
diff --git a/umongo/data_proxy.py b/umongo/data_proxy.py
index c49fb1f..230e628 100644
--- a/umongo/data_proxy.py
+++ b/umongo/data_proxy.py
@@ -141,11 +141,8 @@ class BaseDataProxy:
         return name, field
 
     def get(self, name, to_raise=KeyError):
-        name, field = self._get_field(name, to_raise)
-        value = self._data[name]
-        if value is missing and field.default is not missing:
-            return field.default
-        return value
+        name, _ = self._get_field(name, to_raise)
+        return self._data[name]
 
     def set(self, name, value, to_raise=KeyError):
         name, field = self._get_field(name, to_raise)
@@ -158,8 +155,9 @@ class BaseDataProxy:
         self._mark_as_modified(name)
 
     def delete(self, name, to_raise=KeyError):
-        name, _ = self._get_field(name, to_raise)
-        self._data[name] = missing
+        name, field = self._get_field(name, to_raise)
+        default = field.default
+        self._data[name] = default() if callable(default) else default
         self._mark_as_modified(name)
 
     def __repr__(self):
diff --git a/umongo/fields.py b/umongo/fields.py
index f85bd36..78caaed 100644
--- a/umongo/fields.py
+++ b/umongo/fields.py
@@ -54,8 +54,7 @@ __all__ = (
 class DictField(BaseField, ma_fields.Dict):
 
     def __init__(self, *args, **kwargs):
-        kwargs.setdefault('default', {})
-        kwargs.setdefault('missing', Dict)
+        kwargs.setdefault('default', Dict)
         super().__init__(*args, **kwargs)
 
     def _deserialize(self, value, attr, data):
@@ -82,8 +81,7 @@ class DictField(BaseField, ma_fields.Dict):
 class ListField(BaseField, ma_fields.List):
 
     def __init__(self, *args, **kwargs):
-        kwargs.setdefault('default', [])
-        kwargs.setdefault('missing', lambda: List(self.container))
+        kwargs.setdefault('default', lambda: List(self.container))
         super().__init__(*args, **kwargs)
 
     def _deserialize(self, value, attr, data):
diff --git a/umongo/schema.py b/umongo/schema.py
index 311cf6c..27684a8 100644
--- a/umongo/schema.py
+++ b/umongo/schema.py
@@ -35,7 +35,7 @@ def on_need_add_id_field(bases, fields):
 
 def add_child_field(name, fields):
     from .fields import StrField
-    fields['cls'] = StrField(attribute='_cls', missing=name, dump_only=True)
+    fields['cls'] = StrField(attribute='_cls', default=name, dump_only=True)
 
 
 class Schema(BaseSchema):


Test patch:
diff --git a/tests/test_data_proxy.py b/tests/test_data_proxy.py
index 37f1ec9..c80c4e2 100644
--- a/tests/test_data_proxy.py
+++ b/tests/test_data_proxy.py
@@ -1,5 +1,6 @@
-from marshmallow import ValidationError, missing
 import pytest
+from bson import ObjectId
+from marshmallow import ValidationError, missing
 
 from umongo.data_proxy import data_proxy_factory, BaseDataProxy, BaseNonStrictDataProxy
 from umongo import EmbeddedSchema, fields, EmbeddedDocument, validate, exceptions
@@ -264,19 +265,37 @@ class TestDataProxy(BaseTest):
         assert d._data['in_mongo_b'] is missing
 
     def test_default(self):
+        default_value = ObjectId('507f1f77bcf86cd799439011')
+        default_callable = lambda: ObjectId('507f1f77bcf86cd799439012')
 
         class MySchema(EmbeddedSchema):
-            with_default = fields.StrField(default='default_value')
-            with_missing = fields.StrField(missing='missing_value')
+            no_default = fields.ObjectIdField()
+            with_default = fields.ObjectIdField(default=default_value)
+            with_callable_default = fields.ObjectIdField(default=default_callable)
 
         MyDataProxy = data_proxy_factory('My', MySchema())
         d = MyDataProxy(data={})
-        assert d._data['with_default'] is missing
-        assert d._data['with_missing'] is 'missing_value'
-        assert d.get('with_default') == 'default_value'
-        assert d.get('with_missing') == 'missing_value'
-        assert d.to_mongo() == {'with_missing': 'missing_value'}
-        assert d.dump() == {'with_default': 'default_value', 'with_missing': 'missing_value'}
+        assert d._data['no_default'] is missing
+        assert d._data['with_default'] == default_value
+        assert d._data['with_callable_default'] == default_callable()
+        assert d.get('no_default') is missing
+        assert d.get('with_default') == default_value
+        assert d.get('with_callable_default') == default_callable()
+        assert d.to_mongo() == {
+            'with_default': default_value,
+            'with_callable_default': default_callable(),
+        }
+        assert d.dump() == {
+            'with_default': str(default_value),
+            'with_callable_default': str(default_callable()),
+        }
+
+        d.delete('with_default')
+        assert d._data['with_default'] == default_value
+        assert d.get('with_default') == default_value
+        d.delete('with_callable_default')
+        assert d._data['with_callable_default'] == default_callable()
+        assert d.get('with_callable_default') == default_callable()
 
     def test_validate(self):
 
@@ -296,7 +315,6 @@ class TestDataProxy(BaseTest):
 
         class MySchema(EmbeddedSchema):
             with_default = fields.StrField(default='default_value')
-            with_missing = fields.StrField(missing='missing_value')
             normal = fields.StrField()
             loaded = fields.StrField()
             loaded_but_empty = fields.StrField()
@@ -306,7 +324,7 @@ class TestDataProxy(BaseTest):
         d = MyDataProxy()
         d.from_mongo({'loaded': "foo", 'loaded_but_empty': missing}, partial=True)
         assert d.partial is True
-        for field in ('with_default', 'with_missing', 'normal'):
+        for field in ('with_default', 'normal'):
             with pytest.raises(exceptions.FieldNotLoadedError):
                 d.get(field)
             with pytest.raises(exceptions.FieldNotLoadedError):
@@ -325,7 +343,7 @@ class TestDataProxy(BaseTest):
         d = MyDataProxy()
         d.load({'loaded': "foo", 'loaded_but_empty': missing}, partial=True)
         assert d.partial is True
-        for field in ('with_default', 'with_missing', 'normal'):
+        for field in ('with_default', 'normal'):
             with pytest.raises(exceptions.FieldNotLoadedError):
                 d.get(field)
             with pytest.raises(exceptions.FieldNotLoadedError):
@@ -345,7 +363,6 @@ class TestDataProxy(BaseTest):
         d.from_mongo({'loaded': "foo", 'loaded_but_empty': missing})
         assert d.partial is False
         assert d.get('with_default') == 'default_value'
-        assert d.get('with_missing') == 'missing_value'
         assert d.get('normal') is missing
         assert d.get('loaded') == "foo"
         assert d.get('loaded_but_empty') == missing
@@ -355,7 +372,6 @@ class TestDataProxy(BaseTest):
         assert d.partial is False
         assert d.partial is False
         assert d.get('with_default') == 'default_value'
-        assert d.get('with_missing') == 'missing_value'
         assert d.get('normal') is missing
         assert d.get('loaded') == "foo"
         assert d.get('loaded_but_empty') == missing
@@ -376,8 +392,8 @@ class TestDataProxy(BaseTest):
         # Partial, then update turns it into not partial
         d = MyDataProxy()
         d.from_mongo({'loaded': "foo", 'loaded_but_empty': missing}, partial=True)
-        assert len(d.not_loaded_fields) == 4
-        d.update({'with_default': 'test', 'with_missing': 'test', 'normal_with_attribute': 'foo'})
+        assert len(d.not_loaded_fields) == 3
+        d.update({'with_default': 'test', 'normal_with_attribute': 'foo'})
         assert len(d.not_loaded_fields) == 1
         assert d.partial is True
         d.update({'normal': 'test'})
diff --git a/tests/test_document.py b/tests/test_document.py
index 784cd34..1e898f3 100644
--- a/tests/test_document.py
+++ b/tests/test_document.py
@@ -406,7 +406,7 @@ class TestDocument(BaseTest):
 
         @self.instance.register
         class Parent(Document):
-            id = fields.ObjectIdField(attribute='_id', missing=ObjectId)
+            id = fields.ObjectIdField(attribute='_id', default=ObjectId)
             name = fields.StrField()
 
         john = Parent(name='John Doe')
diff --git a/tests/test_inheritance.py b/tests/test_inheritance.py
index 8a3d3ea..ef3dabf 100644
--- a/tests/test_inheritance.py
+++ b/tests/test_inheritance.py
@@ -64,7 +64,7 @@ class TestInheritance(BaseTest):
 
         @self.instance.register
         class AbstractDoc(Document):
-            abs_field = fields.StrField(missing='from abstract')
+            abs_field = fields.StrField(default='from abstract')
 
             class Meta:
                 abstract = True
diff --git a/tests/test_marshmallow.py b/tests/test_marshmallow.py
index ae83d52..919e8a8 100644
--- a/tests/test_marshmallow.py
+++ b/tests/test_marshmallow.py
@@ -3,7 +3,7 @@ from datetime import datetime
 from bson import ObjectId
 import marshmallow
 
-from umongo import Document, EmbeddedDocument, fields, set_gettext, validate
+from umongo import Document, EmbeddedDocument, fields, set_gettext, validate, missing
 from umongo import marshmallow_bonus as ma_bonus_fields
 from umongo.abstract import BaseField, BaseSchema
 from umongo.marshmallow_bonus import (
@@ -167,7 +167,7 @@ class TestMarshmallow(BaseTest):
         class Vehicle(Document):
             brand = fields.StrField(description='Manufacturer name')
             category = fields.StrField(required=True)
-            nb_wheels = fields.IntField(missing=4)
+            nb_wheels = fields.IntField(default=4)
 
         ma_schema_cls = Vehicle.schema.as_marshmallow_schema()
         schema = ma_schema_cls()
@@ -207,7 +207,7 @@ class TestMarshmallow(BaseTest):
 
         ret = schema.dump({'is_left_handed': True})
         assert not ret.errors
-        assert ret.data == {'name': '1337', 'is_left_handed': True}
+        assert ret.data == {'name': '1337', 'is_left_handed': True, 'cls': 'AdvancedUser'}
 
     def test_to_mongo(self):
         @self.instance.register
@@ -252,8 +252,34 @@ class TestMarshmallow(BaseTest):
         assert ret.data == {'name': 'John'}
 
     def test_missing_accessor(self):
-        # TODO
-        pass
+
+        @self.instance.register
+        class WithDefault(Document):
+            with_umongo_default = fields.StrictDateTimeField(default=datetime(1999, 1, 1))
+            with_marshmallow_missing = fields.StrictDateTimeField(marshmallow_missing='2000-01-01T00:00:00+00:00')
+            with_marshmallow_default = fields.StrictDateTimeField(marshmallow_default='2001-01-01T00:00:00+00:00')
+            with_marshmallow_and_umongo = fields.StrictDateTimeField(
+                default=datetime(1999, 1, 1), marshmallow_missing='2000-01-01T00:00:00+00:00',
+                marshmallow_default='2001-01-01T00:00:00+00:00')
+            with_force_missing = fields.StrictDateTimeField(
+                default=datetime(2001, 1, 1), marshmallow_missing=missing, marshmallow_default=missing)
+            with_nothing = fields.StrField()
+
+        ma_schema = WithDefault.schema.as_marshmallow_schema()()
+        ret = ma_schema.dump({})
+        assert not ret.errors
+        assert ret.data == {
+            'with_umongo_default': '1999-01-01T00:00:00+00:00',
+            'with_marshmallow_default': '2001-01-01T00:00:00+00:00',
+            'with_marshmallow_and_umongo': '2001-01-01T00:00:00+00:00',
+        }
+        ret = ma_schema.load({})
+        assert not ret.errors
+        assert ret.data == {
+            'with_umongo_default': datetime(1999, 1, 1),
+            'with_marshmallow_missing': datetime(2000, 1, 1),
+            'with_marshmallow_and_umongo': datetime(2000, 1, 1),
+        }
 
     def test_nested_field(self):
         @self.instance.register