Skip to content

Conversation

@angelos3lex
Copy link
Contributor

@angelos3lex angelos3lex commented Feb 19, 2025

#6889 issue, failing test ticket

If you remove lines 664-666, (the part with updating the managed object):

this.realm.write(() => {
    this.realm.create(PersonWithId, john, UpdateMode.All);
});

It works well. So the problem is when you update a managed object, it somehow loses the collection.

Update: Pushed a commit that fixes the issue.

@cla-bot
Copy link

cla-bot bot commented Feb 19, 2025

Realm welcomes all contributions! The only requirement we have is that, like many other projects, we need to have a Contributor License Agreement (CLA) in place before we can accept any external code. Our own CLA is a modified version of the Apache Software Foundation’s CLA. Our records show that CLA has not been signed by @angelos3lex. Please submit your CLA electronically using our Google form so we can accept your submissions. After signing the CLA you can recheck this PR with a @cla-bot check comment. The GitHub usernames you file there will need to match that of your Pull Requests. If you have any questions or cannot file the CLA electronically, make a comment here and we will be happy to help you out.

@angelos3lex
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label Feb 19, 2025
@cla-bot
Copy link

cla-bot bot commented Feb 19, 2025

The cla-bot has been summoned, and re-checked this pull request!

@angelos3lex angelos3lex changed the title adds failing test case for issue #6889 Adds failing test case for issue #6889 Feb 19, 2025
@angelos3lex angelos3lex changed the title Adds failing test case for issue #6889 Adds failing test case for issue #6889 + fix Mar 11, 2025
if (!john) throw new Error("Object not found");

this.realm.write(() => {
this.realm.create(PersonWithId, john, UpdateMode.All);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree this should be a possible invocation of the API, can you help me understand the use-case for it? When would you ever call the create and pass an existing object directly? What would be the purpose of that?

I'm asking because I suspect there might be other (less synthetic) ways of hitting the same (or similar) bug, which won't be tested here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kraenhansen In our case, we get the child object through db, inside a write transaction we change a field of it, and we then want to trigger an update on the parent. So we query the parent, and perform a change in a field of it, then call realm.create, which ends up loosing subLists that we didn't even change in any transaction.

We need to update the whole object because that's required by our design, changing a child must trigger a whole update on a parent so that the parent updates some localUpdatedOn etc things we need.

In the example above, I made no change for simplicity, but we could for example change:

this.realm.write(() => {
    john.friends[0].age = 20;
    this.realm.create(PersonWithId, john, UpdateMode.All);
});

which would result in loosing the friends sublist.

I understand that we are not required to perform realm.create since we already change (inside a write transaction) the fields we desire, but due to our architecture, we reuse some functions that work either for managed or unmanaged objects, an then the problem arises when we do it for managed objects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand this correctly? john might or might not be managed and to keep things simply, you pass it to realm.create in any case?

We need to update the whole object because that's required by our design, changing a child must trigger a whole update on a parent

You might want to benefit from passing a second "keypaths" argument to the addListener method to have changes to friends fire events on the "parent":

const Realm = require("realm");
const realm = new Realm({ schema: [{ name: "Person", properties: { name: "string", friends: "Person[]" } }] });
const alice = realm.write(() => realm.create("Person", { name: "Alice", friends: [{ name: "Bob" }] }));

alice.addListener(() => {
  console.log("Alice changed!", alice);
}, "friends.*");

setTimeout(() => {
  console.log("Update!");
  realm.write(() => {
    alice.friends[0].name = "Charlie";
  });
}, 1000);

yields

Alice changed! Person {
  name: 'Alice',
  friends: Realm.List [ Person { name: 'Bob', friends: [Realm.List] } ]
}
Update!
Alice changed! Person {
  name: 'Alice',
  friends: Realm.List [ Person { name: 'Charlie', friends: [Realm.List] } ]
}

assert.inTransaction(realm);
assert.iterable(values);

const valuesCopiedRef = Array.from(values);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Array.from will eagerly consume the values iterator, which might be okay, but depending on the size of the collection, it could cause unnecessary memory pressure. I suggest using a snapshot instead.

@kraenhansen
Copy link
Member

Tests (finally) passed over in #6985, making this PR is good to merge 🎉 Thanks a lot for your patience on this one.

@kraenhansen kraenhansen merged commit 192bccf into realm:main Apr 2, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants