merge_models does not merge list even if it does not include Link

[roman-right]

Discussed in https://github.com/roman-right/beanie/discussions/652

^{Originally posted by **tomohirohiratsuka** August 10, 2023} Hi, I'm wondering this is expected behavior or not. When I update document with `update` method, it returns updated document only when the updated property is not list. I found that inside update method it merging via `parsing.merge_models`. `merge_models` looks like care about if the list includes Link or not but it does not merge even when no Links are found in the list. Is this expected behavior? I'd appreciate it, if anyone answer. Thank you. ```python:documents.py @wrap_with_actions(EventTypes.UPDATE) @save_state_after async def update( self, *args, ignore_revision: bool = False, session: Optional[ClientSession] = None, bulk_writer: Optional[BulkWriter] = None, skip_actions: Optional[List[Union[ActionDirections, str]]] = None, skip_sync: Optional[bool] = None, **pymongo_kwargs, ) -> DocType: """ Partially update the document in the database :param args: *Union[dict, Mapping] - the modifications to apply. :param session: ClientSession - pymongo session. :param ignore_revision: bool - force update. Will update even if revision id is not the same, as stored :param bulk_writer: "BulkWriter" - Beanie bulk writer :param pymongo_kwargs: pymongo native parameters for update operation :return: None """ arguments = list(args) if skip_sync is not None: raise DeprecationWarning( "skip_sync parameter is not supported. The document get synced always using atomic operation." ) use_revision_id = self.get_settings().use_revision if self.id is not None: find_query: Dict[str, Any] = {"_id": self.id} else: find_query = {"_id": PydanticObjectId()} if use_revision_id and not ignore_revision: find_query["revision_id"] = self._previous_revision_id if use_revision_id: arguments.append(SetRevisionId(self.revision_id)) try: result = await self.find_one(find_query).update( *arguments, session=session, response_type=UpdateResponse.NEW_DOCUMENT, bulk_writer=bulk_writer, **pymongo_kwargs, ) except DuplicateKeyError: raise RevisionIdWasChanged if bulk_writer is None: if use_revision_id and not ignore_revision and result is None: raise RevisionIdWasChanged merge_models(self, result) # this update self return self ``` ```python:parsing.py def merge_models(left: BaseModel, right: BaseModel) -> None: from beanie.odm.fields import Link if hasattr(left, "_previous_revision_id") and hasattr( right, "_previous_revision_id" ): left._previous_revision_id = right._previous_revision_id # type: ignore for k, right_value in right.__iter__(): left_value = left.__getattribute__(k) if isinstance(right_value, BaseModel) and isinstance( left_value, BaseModel ): merge_models(left_value, right_value) continue if isinstance(right_value, list): links_found = False for i in right_value: if isinstance(i, Link): links_found = True break if links_found: # why no links case is not handled? continue elif not isinstance(right_value, Link): left.__setattr__(k, right_value) ```

[roman-right]

Hey @tomohirohiratsuka Sorry for the delay.

I see the problem, but I'd like to have a use case, when it appears. Could you please give me a one-module example for it? Thank you

[IterableTrucks]

import asyncio
from beanie import init_beanie, Document

class Directory(Document):
    name: str
    file_names: list[str]

async def main():
    await init_beanie(connection_string="mongodb://localhost:27017/test", document_models=[Directory])
    files = ['file-1', 'file-2', 'file-3']
    directory = await Directory(name='dir-1', file_names=files).insert()
    new_files = ['file-1']
    new_directory = await directory.set(dict(file_names=new_files))
    assert new_directory.file_names == new_files
asyncio.run(main())

Here is the code that can reproduce this bug

[tomohirohiratsuka]

Hey @tomohirohiratsuka Sorry for the delay.

I see the problem, but I'd like to have a use case, when it appears. Could you please give me a one-module example for it? Thank you

@roman-right Hi, Thank you for your response. I've made a simple reproduction repo. https://github.com/tomohirohiratsuka/beanie-reproduction-653 Please take a look at it.

[roman-right]

Hey @tomohirohiratsuka It looks like this case is solved in the 1.22 version. Could you please check?

[tomohirohiratsuka]

@roman-right Hi, thank you for versioning up. I've confirmed that it's fixed on v1.22.1. https://github.com/tomohirohiratsuka/beanie-reproduction-653

Since this is solved, I've also closed my PR. Thank you.

[martincpt]

I encountered an issue when attempting to update a document that includes a Link. I executed the update using the set method, and the changes appeared in the database immediately. However, the object returned by merge_models does not reflect the new link. Is this expected behavior?

model.set( {'my_link': {'id': '66d9b36467d68baa0b96849b', 'collection': 'MyLink'})

[martincpt]

After reviewing the code of merge_models, I discovered the reason I'm not getting the expected result. In the final part of the code, it checks the type of the field and skips it if it’s a Link.

    # starting from this line
    elif not isinstance(right_value, Link):
            left.__setattr__(k, right_value)

Can I ask why this is necessary?