Scille / umongo

sync/async MongoDB ODM, yes.
MIT License
446 stars 63 forks source link

DictFields and EmbeddedFields aren't created with default values #373

Closed tiltowait closed 2 years ago

tiltowait commented 2 years ago

When creating a new document, fields in embedded documents, as well as top-level DictFields, aren't receiving their default values. Instead, they receive the values of whatever the last-committed document had. The complete program below demonstrates the issue.

import asyncio

from motor.motor_asyncio import AsyncIOMotorClient
from umongo import Document, EmbeddedDocument, fields
from umongo.frameworks import MotorAsyncIOInstance

_mongo_client = AsyncIOMotorClient()
_db = _mongo_client.umongo_test
instance = MotorAsyncIOInstance(_db)

@instance.register
class EmbeddedWithDict(EmbeddedDocument):
    """
    This embedded doc has two fields in it. When a document is initialized, it
    should have default values: an empty dict and an empty string. However,
    this is only the case for the first initialization; subsequent
    initializations have whatever the last document's values were.
    """

    embedded_dict = fields.DictField(default={})
    embedded_str = fields.StrField(default="")

@instance.register
class MainDoc(Document):
    """
    This document defines three keys: a name, a dictionary, and an embedded
    document with a dictionary. The first document is correctly generated;
    subsequent documents, however, do not use default values for the
    dictionary nor the embedded doc. Only the name and the age fields are
    correctly assigned.
    """

    name = fields.StrField(required=True)
    age = fields.IntField(default=0)
    dictionary = fields.DictField(default={})
    embedded = fields.EmbeddedField(EmbeddedWithDict, default=EmbeddedWithDict())

    class Meta:
        collection_name = "test_stale_embedded_dict"

async def main():
    """Create and commit a pair of objects."""
    first = MainDoc(name="First Document")
    first.age = 10
    first.dictionary["value"] = 3
    first.embedded.embedded_dict["embedded"] = "dictionary"
    first.embedded.embedded_str = "test"
    await first.commit()

    # When creating this second document, it is expected that the top-level
    # dictionary, the embedded dictionary, and the embedded string will all
    # have default values ({}, {}, and "", respectively).

    second = MainDoc(name="Second Document")
    await second.commit()

    # These tests pass
    assert second.name == "Second Document"
    assert second.age == 0

    # These tests fail
    assert not second.dictionary, "Top-level dictionary isn't empty"
    assert not second.embedded.embedded_str, "Embedded string isn't empty"
    assert not second.embedded.embedded_dict, "Embedded dictionary isn't empty"

if __name__ == "__main__":
    loop = asyncio.get_event_loop()
    loop.run_until_complete(main())
whophil commented 2 years ago

This appears to be the same (or similar) issue to https://github.com/Scille/umongo/issues/371

For fields with default values, try to use the following

    dictionary = fields.DictField(default=dict)

Instead of

    dictionary = fields.DictField(default={})
tiltowait commented 2 years ago

default=dict fixes the top-level dictionary test. It does not, however, fix the embedded document tests. To be clear, the following is still initialized with incorrect values on subsequent document creation:

@instance.register
class EmbeddedWithDict(EmbeddedDocument):
    embedded_dict = fields.DictField(default=dict)
    embedded_str = fields.StrField(default=str)
whophil commented 2 years ago

How about changing this:

    embedded = fields.EmbeddedField(EmbeddedWithDict, default=EmbeddedWithDict())

to

    embedded = fields.EmbeddedField(EmbeddedWithDict, default=EmbeddedWithDict)
tiltowait commented 2 years ago

That works! (And as soon as you posted it, it was a total facepalm moment.)