realm / realm-js

Realm is a mobile database: an alternative to SQLite & key-value stores
https://realm.io
Apache License 2.0
5.74k stars 565 forks source link

Intentional change to `realm.create()` when `UpdateMode` not specified? #4328

Open liamjones opened 2 years ago

liamjones commented 2 years ago

Description

I'm testing the pre-release branch with our existing app. We had some code creating a bunch of nested items with UUID IDs for the primary keys in a loop which now throws an error but worked in 10.20.0 and earlier.

function loadThings(importModel: ImportModel[]) {
    realm.write(() => {
        for (const item of importModel) {

            const savedRealmThing = realm.create<RealmThing>(RealmThing.schema.name, createRealmThing(item))

            const aliases = item.aliases ? item.aliases : []
            const thingNames = [...aliases, item.name]
            thingNames.forEach((name) => {
                const alias = {
                    id: uuid.v4(),
                    thing: savedRealmThing,
                    name: name,
                    schema: latestRealmSchema.realmAlias,
                }
                realm.create<RealmAlias>(RealmAlias.schema.name, alias)
            })
        }
    })
    return importModel.length
}

When it gets to realm.create<RealmAlias>(RealmAlias.schema.name, alias) it dies, apparently because it's trying to recreate the nested savedRealmThing and the object already exists. Changing that line to realm.create<RealmAlias>(RealmAlias.schema.name, alias, Realm.UpdateMode.Modified) fixes the issue.

It seems like the existing behaviour for realm.create() with no UpdateMode was Modified but is now Never, was this a deliberate change?

Stacktrace & log output

Error is: `Exception in HostFunction: Attempting to create an object of type 'RealmThing' with an existing primary key value ''0c5f1ca1-1e08-48a2-a75b-e5a302db67d5''.`

Can you reproduce a bug?

Yes, always

Reproduction Steps

See example code in description

Version

hermes (10.20.0-beta.1)

What SDK flavour are you using?

Local Database only

Are you using encryption?

No, not using encryption

Platform OS and version(s)

RN 0.66.3 using JSC (not Hermes!) - iPhone 13 Simulator w/ iOS 15.2, Android AVD Emulator running Android 9.0

tomduncalf commented 2 years ago

Hey @liamjones, that's strange! We've not deliberately changed this behaviour, and I just created a test app using 10.20.0-beta.1 using a similar schema to yours (if I understand it correctly) and I don't see this issue.

You can see my sample app at https://github.com/tomduncalf/issue-4328-updatemode – I just added a TaskAlias schema to our sample todo list app, which references an existing task, and then at startup I create a task and a few aliases pointing to it, as you can see in this commit.

Perhaps there are some other details of what your app is doing that are causing this – would you be able to take a look at my sample app and see if you have any ideas what might be different? You can also try out my app to check you don't see the same issue there (clone it and npm install && npx pod-install && npm run ios and you should see some task aliases in the logs).

If you're able to share your source code (in public or private) I'd be happy to take a look.

haswalt commented 2 years ago

I'm not sure I can provide a simple reproduction at the moment but in updating our project to use the latest pre-release we're seeing the same result. Using a model with a to-many relationship (with no inverse) results in:

Error: Exception in HostFunction: Attempting to create an object of type '[REDACTED]' with an existing primary key value '[REDACTED]''

Nothing else has been changed other than the version updated.

liamjones commented 2 years ago

I've finally had a chance to try our app again against the latest beta (beta.2) - this is no longer happening for us. 😕

That's good but I'm somewhat confused... I guess it must have been something within our codebase causing it because even going back to beta.1 with the current HEAD commit of our app no longer has the issue? It's weird though, the area of code that was blowing up before hasn't been touched in months.

If I remember correctly, doesn't the realm package download a binary during install? Is it possible that's now pulling a newer version even if I install beta.1 and that's why things are now fixed?

Not sure if I should keep this open or close it? I no longer have the issue but @haswalt said they'd experienced it too?

An aside; while the code is now working, it does run almost twice as slow as it used to, are the betas known to be slower than the non-beta builds right now?

10.20.0-beta.2:
Loaded 6066 Products in 13.883 seconds
Loaded 6066 Products in 14.27 seconds
Loaded 6066 Products in 13.969 seconds

10.20.0-beta.1:
Loaded 6066 Products in 13.923 seconds
Loaded 6066 Products in 13.829 seconds
Loaded 6066 Products in 13.909 seconds

10.13.0:
Loaded 6066 Products in 7.503 seconds
Loaded 6066 Products in 7.485 seconds
Loaded 6066 Products in 7.504 seconds
haswalt commented 2 years ago

I’ve been testing on a freshly re-installed node_modules (didn’t clean npm cache) and I’m seeing the same.

I haven’t noticed the performance hit as I’m not loading large collections HOWEVER I am seeing issues with subscriptions not triggering callbacks when objects are created / deleted so our reactive UI isn’t working any more.

In my case I have a react component creating objects abs another component deleting them on click. A third component has a collection listener to update the ui based on create and delete events but only about 1 on 3 trigger the listener. Not consistent either, sometimes it’s immediate, sometimes there is a large delay and sometimes the listener doesn’t trigger at all.

kneth commented 2 years ago

@haswalt Do you use 10.13.0 or 10.20.0-beta.2?

haswalt commented 2 years ago

@kneth 10.20.0-beta.1

tomduncalf commented 2 years ago

@haswalt Are you able to test with 10.20.0-beta.2 please?

haswalt commented 2 years ago

@tomduncalf initial testing with 10.20.0-beta.2 seems to fix the subscriptions. Will do further testing to confirm UpdateMode and report back.

tomduncalf commented 2 years ago

Great, thanks

haswalt commented 2 years ago

@tomduncalf can confirm that UpdateMode is no longer explicitely required and subscriptions seem to be triggering correctly using the latest beta. I don't have any tests setup for large datasets so cannot confirm @liamjones performance issue

tomduncalf commented 2 years ago

Thanks @haswalt, I'm happy to hear the problem is solved for you!

liamjones commented 2 years ago

Okay, sounds like this one can be closed then. @tomduncalf @kneth shall I raise a separate ticket for the perf issue or is that expected while we're in beta and will improve before release?

tomduncalf commented 2 years ago

@liamjones If you could raise a separate ticket so that we know for sure that we are tracking it, that would be great. Thanks!

liamjones commented 2 years ago

Hi @tomduncalf I'm reopening this one since I've just hit it again in beta 3 while making a minimal repro for #4443.

Please see the minimal repro email sent in to realm-help@mongodb.com titled "Private repro for realm-js issues #4443 & #4328".

You'll need to edit one line to reproduce the issue (since it's setup for the performance stuff OOTB), see the email body for details.

tomduncalf commented 2 years ago

Thanks @liamjones, we've received your reproduction and I was able to reproduce this issue. We'll look into it – we might not get to it this week but we will update you here.