Support values of inner docs given as AttrDict instances (#3080)

* Support values of inner docs given as AttrDict instances

* one more unit test

Author: miguelgrinberg

elasticsearch/dsl/field.py

@@ -572,7 +572,11 @@ def _serialize(
         if isinstance(data, collections.abc.Mapping):
             return data
 
-        return data.to_dict(skip_empty=skip_empty)
+        try:
+            return data.to_dict(skip_empty=skip_empty)
+        except TypeError:
+            # this would only happen if an AttrDict was given instead of an InnerDoc
+            return data.to_dict()
 
     def clean(self, data: Any) -> Any:
         data = super().clean(data)

test_elasticsearch/test_dsl/test_field.py

@@ -24,7 +24,14 @@
 from dateutil import tz
 
 from elasticsearch import dsl
-from elasticsearch.dsl import InnerDoc, Range, ValidationException, field
+from elasticsearch.dsl import (
+    AttrDict,
+    AttrList,
+    InnerDoc,
+    Range,
+    ValidationException,
+    field,
+)
 
 
 def test_date_range_deserialization() -> None:
@@ -235,6 +242,33 @@ class Inner(InnerDoc):
         field.Object(doc_class=Inner, dynamic=False)
 
 
+def test_dynamic_object() -> None:
+    f = field.Object(dynamic=True)
+    assert f.deserialize({"a": "b"}).to_dict() == {"a": "b"}
+    assert f.deserialize(AttrDict({"a": "b"})).to_dict() == {"a": "b"}
+    assert f.serialize({"a": "b"}) == {"a": "b"}
+    assert f.serialize(AttrDict({"a": "b"})) == {"a": "b"}
+
+
+def test_dynamic_nested() -> None:
+    f = field.Nested(dynamic=True)
+    assert f.deserialize([{"a": "b"}, {"c": "d"}]) == [{"a": "b"}, {"c": "d"}]
+    assert f.deserialize([AttrDict({"a": "b"}), {"c": "d"}]) == [
+        {"a": "b"},
+        {"c": "d"},
+    ]
+    assert f.deserialize(AttrList([AttrDict({"a": "b"}), {"c": "d"}])) == [
+        {"a": "b"},
+        {"c": "d"},
+    ]
+    assert f.serialize([{"a": "b"}, {"c": "d"}]) == [{"a": "b"}, {"c": "d"}]
+    assert f.serialize([AttrDict({"a": "b"}), {"c": "d"}]) == [{"a": "b"}, {"c": "d"}]
+    assert f.serialize(AttrList([AttrDict({"a": "b"}), {"c": "d"}])) == [
+        {"a": "b"},
+        {"c": "d"},
+    ]
+
+
 def test_all_fields_exported() -> None:
     """Make sure that all the generated field classes are exported at the top-level"""
     fields = [

test_elasticsearch/test_dsl/test_integration/_async/test_document.py

@@ -33,6 +33,7 @@
 from elasticsearch.dsl import (
     AsyncDocument,
     AsyncSearch,
+    AttrDict,
     Binary,
     Boolean,
     Date,
@@ -627,13 +628,17 @@ async def test_can_save_to_different_index(
 
 
 @pytest.mark.asyncio
+@pytest.mark.parametrize("validate", (True, False))
 async def test_save_without_skip_empty_will_include_empty_fields(
     async_write_client: AsyncElasticsearch,
+    validate: bool,
 ) -> None:
     test_repo = Repository(
         field_1=[], field_2=None, field_3={}, owner={"name": None}, meta={"id": 42}
     )
-    assert await test_repo.save(index="test-document", skip_empty=False)
+    assert await test_repo.save(
+        index="test-document", skip_empty=False, validate=validate
+    )
 
     assert_doc_equals(
         {
@@ -650,6 +655,23 @@ async def test_save_without_skip_empty_will_include_empty_fields(
         await async_write_client.get(index="test-document", id=42),
     )
 
+    test_repo = Repository(owner=AttrDict({"name": None}), meta={"id": 43})
+    assert await test_repo.save(
+        index="test-document", skip_empty=False, validate=validate
+    )
+
+    assert_doc_equals(
+        {
+            "found": True,
+            "_index": "test-document",
+            "_id": "43",
+            "_source": {
+                "owner": {"name": None},
+            },
+        },
+        await async_write_client.get(index="test-document", id=43),
+    )
+
 
 @pytest.mark.asyncio
 async def test_delete(async_write_client: AsyncElasticsearch) -> None:

test_elasticsearch/test_dsl/test_integration/_sync/test_document.py

@@ -31,6 +31,7 @@
 
 from elasticsearch import ConflictError, Elasticsearch, NotFoundError
 from elasticsearch.dsl import (
+    AttrDict,
     Binary,
     Boolean,
     Date,
@@ -621,13 +622,15 @@ def test_can_save_to_different_index(
 
 
 @pytest.mark.sync
+@pytest.mark.parametrize("validate", (True, False))
 def test_save_without_skip_empty_will_include_empty_fields(
     write_client: Elasticsearch,
+    validate: bool,
 ) -> None:
     test_repo = Repository(
         field_1=[], field_2=None, field_3={}, owner={"name": None}, meta={"id": 42}
     )
-    assert test_repo.save(index="test-document", skip_empty=False)
+    assert test_repo.save(index="test-document", skip_empty=False, validate=validate)
 
     assert_doc_equals(
         {
@@ -644,6 +647,21 @@ def test_save_without_skip_empty_will_include_empty_fields(
         write_client.get(index="test-document", id=42),
     )
 
+    test_repo = Repository(owner=AttrDict({"name": None}), meta={"id": 43})
+    assert test_repo.save(index="test-document", skip_empty=False, validate=validate)
+
+    assert_doc_equals(
+        {
+            "found": True,
+            "_index": "test-document",
+            "_id": "43",
+            "_source": {
+                "owner": {"name": None},
+            },
+        },
+        write_client.get(index="test-document", id=43),
+    )
+
 
 @pytest.mark.sync
 def test_delete(write_client: Elasticsearch) -> None:

utils/templates/field.py.tpl

@@ -334,7 +334,11 @@ class {{ k.name }}({{ k.parent }}):
         if isinstance(data, collections.abc.Mapping):
             return data
 
-        return data.to_dict(skip_empty=skip_empty)
+        try:
+            return data.to_dict(skip_empty=skip_empty)
+        except TypeError:
+            # this would only happen if an AttrDict was given instead of an InnerDoc
+            return data.to_dict()
 
     def clean(self, data: Any) -> Any:
         data = super().clean(data)