Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Document.sync() ignores Link fields #1105

Open
ADR-007 opened this issue Jan 6, 2025 · 2 comments
Open

[BUG] Document.sync() ignores Link fields #1105

ADR-007 opened this issue Jan 6, 2025 · 2 comments

Comments

@ADR-007
Copy link

ADR-007 commented Jan 6, 2025

Describe the bug
Document.sync() method doesn't sync Link fields.

To Reproduce

class A(Document):
    v: int = 1

class B(Document):
  a: Link[A]
  aa: list[Link[A]]

b = B(A(), [A()])
await b.save()

await B.update({}, {B.a: A(v=2), B.aa: [A(v=3), A(v=4)])

await b.sync()

assert b.a.v == 2 # fail
assert b.aa[0].v == 3 # fail

Expected behavior
Sync should refresh linked values as well.
It can be done by changing merge_models function,
Or by passing fetch_links, with_children, nesting_depth, nesting_depths_per_field fields to sync function that should be propagated to document = await self.find_one({"_id": self.id})

Additional context
This is because it uses merge_models function:
beanie/odm/utils/parsing.py

def merge_models(left: BaseModel, right: BaseModel) -> None:
    """
    Merge two models
    :param left: left model
    :param right: right model
    :return: None
    """
    from beanie.odm.fields import Link

    for k, right_value in right.__iter__():
        left_value = getattr(left, k)
        if isinstance(right_value, BaseModel) and isinstance(
            left_value, BaseModel
        ):
            if get_config_value(left_value, "frozen"):
                left.__setattr__(k, right_value)
            else:
                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:
                continue
            left.__setattr__(k, right_value)
        elif not isinstance(right_value, Link):
            left.__setattr__(k, right_value)
Copy link
Contributor

github-actions bot commented Feb 5, 2025

This issue is stale because it has been open 30 days with no activity.

@github-actions github-actions bot added the Stale label Feb 5, 2025
@martincpt
Copy link

That's right. Not only the sync but save, set, update and all methods relying on merge_models ingore Link fields.

I think #653 is related.

This part ignores the field if it's a Link:

elif not isinstance(right_value, Link):
    left.__setattr__(k, right_value)

I also wonder if it was done on purpose. Can anyone answer?

@github-actions github-actions bot removed the Stale label Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants