Repository: Scille/umongo (python)

Issue:
Trying to set a ReferenceField to None fails in _reference_io_validate
Using Python 3.8.6 and `motor_asyncio`, when trying to set a `ReferenceField` in a document to None, I get an `AttributeError` while committing.

The traceback looks something like this:

```
Traceback (most recent call last):
  ... various project-specific stuff ...
    await obj.commit()
  File "/opt/venv/lib/python3.7/site-packages/umongo/frameworks/motor_asyncio.py", line 161, in commit
    await self.io_validate(validate_all=io_validate_all)
  File "/opt/venv/lib/python3.7/site-packages/umongo/frameworks/motor_asyncio.py", line 253, in io_validate
    self.schema, self._data, partial=self._data.get_modified_fields())
  File "/opt/venv/lib/python3.7/site-packages/umongo/frameworks/motor_asyncio.py", line 335, in _io_validate_data_proxy
    raise res
  File "/usr/lib/python3.7/asyncio/tasks.py", line 223, in __step
    result = coro.send(None)
  File "/opt/venv/lib/python3.7/site-packages/umongo/frameworks/motor_asyncio.py", line 307, in _run_validators
    raise res
  File "/usr/lib/python3.7/asyncio/tasks.py", line 223, in __step
    result = coro.send(None)
  File "/opt/venv/lib/python3.7/site-packages/umongo/frameworks/motor_asyncio.py", line 341, in _reference_io_validate
    await value.fetch(no_data=True)
AttributeError: 'NoneType' object has no attribute 'fetch'
```

This appears to be because the `_reference_io_validate` function doesn't check if the value it's validating is not None, unlike (for example) the one for list fields:

```python
async def _reference_io_validate(field, value):
    await value.fetch(no_data=True)            
``` 
(from [`motor_asyncio.py`](https://github.com/Scille/umongo/blob/master/umongo/frameworks/motor_asyncio.py#L340) line 340)

Currently I'm working around this by doing the update manually (with `Impl.collection.update_one({"_id": obj.pk}, {"$unset": {"field": None}})`) but it'd much nicer if this worked natively.

Pull request:
Fix io_validation for None reference
Fixes #326.

Patch:
diff --git a/umongo/frameworks/motor_asyncio.py b/umongo/frameworks/motor_asyncio.py
index 976ee5d..ccb61a5 100644
--- a/umongo/frameworks/motor_asyncio.py
+++ b/umongo/frameworks/motor_asyncio.py
@@ -338,12 +338,16 @@ async def _io_validate_data_proxy(schema, data_proxy, partial=None):
 
 
 async def _reference_io_validate(field, value):
+    if value is None:
+        return
     await value.fetch(no_data=True)
 
 
 async def _list_io_validate(field, value):
+    if not value:
+        return
     validators = field.inner.io_validate
-    if not validators or not value:
+    if not validators:
         return
     tasks = [_run_validators(validators, field.inner, e) for e in value]
     results = await asyncio.gather(*tasks, return_exceptions=True)
@@ -358,6 +362,8 @@ async def _list_io_validate(field, value):
 
 
 async def _embedded_document_io_validate(field, value):
+    if not value:
+        return
     await _io_validate_data_proxy(value.schema, value._data)
 
 
diff --git a/umongo/frameworks/pymongo.py b/umongo/frameworks/pymongo.py
index 591a58d..aace5fe 100644
--- a/umongo/frameworks/pymongo.py
+++ b/umongo/frameworks/pymongo.py
@@ -272,10 +272,14 @@ def _io_validate_data_proxy(schema, data_proxy, partial=None):
 
 
 def _reference_io_validate(field, value):
+    if value is None:
+        return
     value.fetch(no_data=True)
 
 
 def _list_io_validate(field, value):
+    if not value:
+        return
     errors = {}
     validators = field.inner.io_validate
     if not validators:
@@ -290,6 +294,8 @@ def _list_io_validate(field, value):
 
 
 def _embedded_document_io_validate(field, value):
+    if not value:
+        return
     _io_validate_data_proxy(value.schema, value._data)
 
 
diff --git a/umongo/frameworks/txmongo.py b/umongo/frameworks/txmongo.py
index 7a965aa..408d173 100644
--- a/umongo/frameworks/txmongo.py
+++ b/umongo/frameworks/txmongo.py
@@ -273,14 +273,20 @@ def _io_validate_data_proxy(schema, data_proxy, partial=None):
         raise ma.ValidationError(errors)
 
 
+@inlineCallbacks
 def _reference_io_validate(field, value):
-    return value.fetch(no_data=True)
+    if value is None:
+        yield
+    else:
+        yield value.fetch(no_data=True)
 
 
 @inlineCallbacks
 def _list_io_validate(field, value):
+    if not value:
+        return
     validators = field.inner.io_validate
-    if not validators or not value:
+    if not validators:
         return
     errors = {}
     defers = []
@@ -294,6 +300,8 @@ def _list_io_validate(field, value):
 
 
 def _embedded_document_io_validate(field, value):
+    if not value:
+        return
     return _io_validate_data_proxy(value.schema, value._data)
 
 


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 11942e6..342c9ab 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -21,7 +21,7 @@ def classroom_model(instance):
     @instance.register
     class Course(Document):
         name = fields.StrField(required=True)
-        teacher = fields.ReferenceField(Teacher, required=True)
+        teacher = fields.ReferenceField(Teacher, required=True, allow_none=True)
 
     @instance.register
     class Student(Document):
diff --git a/tests/frameworks/test_motor_asyncio.py b/tests/frameworks/test_motor_asyncio.py
index 1400ebc..9a06fda 100644
--- a/tests/frameworks/test_motor_asyncio.py
+++ b/tests/frameworks/test_motor_asyncio.py
@@ -342,6 +342,11 @@ class TestMotorAsyncIO(BaseDBTest):
             with pytest.raises(ma.ValidationError) as exc:
                 await course.io_validate()
             assert exc.value.messages == {'teacher': ['Reference not found for document Teacher.']}
+            # Test setting to None / deleting
+            course.teacher = None
+            await course.io_validate()
+            del course.teacher
+            await course.io_validate()
 
         loop.run_until_complete(do_test())
 
@@ -474,12 +479,42 @@ class TestMotorAsyncIO(BaseDBTest):
 
             @instance.register
             class IOStudent(Student):
-                io_field = fields.ListField(fields.IntField(io_validate=io_validate))
+                io_field = fields.ListField(
+                    fields.IntField(io_validate=io_validate),
+                    allow_none=True
+                )
 
             student = IOStudent(name='Marty', io_field=values)
             await student.io_validate()
             assert set(called) == set(values)
 
+            student.io_field = None
+            await student.io_validate()
+            del student.io_field
+            await student.io_validate()
+
+        loop.run_until_complete(do_test())
+
+    def test_io_validate_embedded(self, loop, instance, classroom_model):
+        Student = classroom_model.Student
+
+        @instance.register
+        class EmbeddedDoc(EmbeddedDocument):
+            io_field = fields.IntField()
+
+        @instance.register
+        class IOStudent(Student):
+            embedded_io_field = fields.EmbeddedField(EmbeddedDoc, allow_none=True)
+
+        async def do_test():
+
+            student = IOStudent(name='Marty', embedded_io_field={'io_field': 12})
+            await student.io_validate()
+            student.embedded_io_field = None
+            await student.io_validate()
+            del student.embedded_io_field
+            await student.io_validate()
+
         loop.run_until_complete(do_test())
 
     def test_indexes(self, loop, instance):
diff --git a/tests/frameworks/test_pymongo.py b/tests/frameworks/test_pymongo.py
index 4af4436..100279d 100644
--- a/tests/frameworks/test_pymongo.py
+++ b/tests/frameworks/test_pymongo.py
@@ -245,6 +245,11 @@ class TestPymongo(BaseDBTest):
         with pytest.raises(ma.ValidationError) as exc:
             course.io_validate()
         assert exc.value.messages == {'teacher': ['Reference not found for document Teacher.']}
+        # Test setting to None / deleting
+        course.teacher = None
+        course.io_validate()
+        del course.teacher
+        course.io_validate()
 
     def test_io_validate(self, instance, classroom_model):
         Student = classroom_model.Student
@@ -331,12 +336,35 @@ class TestPymongo(BaseDBTest):
 
         @instance.register
         class IOStudent(Student):
-            io_field = fields.ListField(fields.IntField(io_validate=io_validate))
+            io_field = fields.ListField(fields.IntField(io_validate=io_validate), allow_none=True)
 
         student = IOStudent(name='Marty', io_field=values)
         student.io_validate()
         assert called == values
 
+        student.io_field = None
+        student.io_validate()
+        del student.io_field
+        student.io_validate()
+
+    def test_io_validate_embedded(self, instance, classroom_model):
+        Student = classroom_model.Student
+
+        @instance.register
+        class EmbeddedDoc(EmbeddedDocument):
+            io_field = fields.IntField()
+
+        @instance.register
+        class IOStudent(Student):
+            embedded_io_field = fields.EmbeddedField(EmbeddedDoc, allow_none=True)
+
+        student = IOStudent(name='Marty', embedded_io_field={'io_field': 12})
+        student.io_validate()
+        student.embedded_io_field = None
+        student.io_validate()
+        del student.embedded_io_field
+        student.io_validate()
+
     def test_indexes(self, instance):
 
         @instance.register
diff --git a/tests/frameworks/test_txmongo.py b/tests/frameworks/test_txmongo.py
index 6e11723..a829d44 100644
--- a/tests/frameworks/test_txmongo.py
+++ b/tests/frameworks/test_txmongo.py
@@ -294,6 +294,11 @@ class TestTxMongo(BaseDBTest):
         with pytest.raises(ma.ValidationError) as exc:
             yield course.io_validate()
         assert exc.value.messages == {'teacher': ['Reference not found for document Teacher.']}
+        # Test setting to None / deleting
+        course.teacher = None
+        yield course.io_validate()
+        del course.teacher
+        yield course.io_validate()
 
     @pytest_inlineCallbacks
     def test_io_validate(self, instance, classroom_model):
@@ -311,7 +316,7 @@ class TestTxMongo(BaseDBTest):
 
         @instance.register
         class IOStudent(Student):
-            io_field = fields.StrField(io_validate=io_validate)
+            io_field = fields.StrField(io_validate=io_validate, allow_none=True)
 
         student = IOStudent(name='Marty', io_field=io_field_value)
         assert not io_validate_called
@@ -319,6 +324,11 @@ class TestTxMongo(BaseDBTest):
         yield student.io_validate()
         assert io_validate_called
 
+        student.io_field = None
+        yield student.io_validate()
+        del student.io_field
+        yield student.io_validate()
+
     @pytest_inlineCallbacks
     def test_io_validate_error(self, instance, classroom_model):
         Student = classroom_model.Student
@@ -417,6 +427,25 @@ class TestTxMongo(BaseDBTest):
         yield student.io_validate()
         assert called == values
 
+    @pytest_inlineCallbacks
+    def test_io_validate_embedded(self, instance, classroom_model):
+        Student = classroom_model.Student
+
+        @instance.register
+        class EmbeddedDoc(EmbeddedDocument):
+            io_field = fields.IntField()
+
+        @instance.register
+        class IOStudent(Student):
+            embedded_io_field = fields.EmbeddedField(EmbeddedDoc, allow_none=True)
+
+        student = IOStudent(name='Marty', embedded_io_field={'io_field': 12})
+        yield student.io_validate()
+        student.embedded_io_field = None
+        yield student.io_validate()
+        del student.embedded_io_field
+        yield student.io_validate()
+
     @pytest_inlineCallbacks
     def test_indexes(self, instance):