Nozbe / WatermelonDB

🍉 Reactive & asynchronous database for powerful React and React Native apps ⚡️
https://watermelondb.dev
MIT License
10.49k stars 589 forks source link

Using same Id in different tables doesn't work #1126

Open nmarchais opened 3 years ago

nmarchais commented 3 years ago

I have 2 SQL tables. A parent table and a child table. The parent table primary key matches the chlid table primary key. When I use WatermelonDB I first create a parent record and then I create a child record using the ID of the created parent. In the end, I get parent.id === child.id.

The issue is the following: the child record stays with the status "created" after calling watermelonDB synchronize(). If I call a second time synchronize() then the child record becomes "synced". The child record is sent to the server by the pushChanges() method during the first and the second synchronize.

The expected behaviour is that the child record should become "synced" after the first call to synchronize.

To be sure my server code is not responsible for the issue I deactivated completely. The push server method is empty. It just returns with an HTTP 200 status.

Here is a code snippet that demonstrates the issue: ` const activity = await database.write(async () => { return await database.collections.get(TableName.ACTIVITY).create(newActivity => { newActivity.beginDate = dayjs().toDate(); }); });

await database.write(async () => { return await database.collections.get(TableName.VISIT).create(record => { record._raw = sanitizedRaw({ id: activity.id, beginIdentificationMode: "XXX", beginIdentificationCode: "XXX", salesCounter: 1}, database.collections.get(TableName.VISIT).schema) }); `

The important part is "id: activity.id" when I create a Visit record. This is the cause of the issue. If I replace Visit id by a unique ID then everything works fine. I can't use the ID of a freshly created record to create another record.

Then when I call synchronize() activity record becomes "synced" and visit record stays with the status "created".

Does WatermelonDB expect to have unique IDs all tables? Or IDs have to be unique only inside a single table? Is there something I do wrong? Is it possible to make my use case work?

nmarchais commented 3 years ago

Could someone just tell me if WatermelonDB requires unique IDs for all tables? Or IDs have to be unique only inside a single table?

radex commented 3 years ago

no, IDs have to be unique per-table

nmarchais commented 3 years ago

Then, do you have an explanation why my use case doesn't work?

Le jeu. 12 août 2021 à 19:00, Radek Pietruszewski @.***> a écrit :

no, IDs have to be unique per-table

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Nozbe/WatermelonDB/issues/1126#issuecomment-897806499, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ65LCACWNXAN46IUS6M7TT4P4ZXANCNFSM5BXTREVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

radex commented 3 years ago

@nmarchais I don't know, I don't have time right now to debug such issues (unless there's a clear reproduction path to demonstrate a WatermelonDB bug) -- sorry, you have to analyze yourself what the app is doing to see why you're seeing the problem

nmarchais commented 3 years ago

@radex I created a minimalist project that reproduces the issue: https://github.com/nmarchais/wdb_uniqueID_bug I hope that will help. Tell me if you need anything else.

nmarchais commented 3 years ago

Here are how to use the app to reproduce the issue:

Press on "Create records" button

Press on "Synchronize" button

Press a second time on "Synchronize" button --> bug occurs --> activity or visit record previously created is pushed again for creation. Excepted behaviour: The second press on "Synchronize" should push nothing.

nmarchais commented 3 years ago

@radex I confirm the issue. After reading the source code I can explain the issue. WatermelonDB doesn't accept multiple records with the same id inside the same changeset. It means ids have to be unique inside the whole changeset across all the tables.

The problem comes from the method "unchangedRecordsForRaws". The method compares raw.id with recordCache.id. It should also check the table name to allow to have same ids across tables.

I forked your project to fix the issue. Here is the link to my pull request --> https://github.com/nmarchais/WatermelonDB/pull/1

Could you tell me if what I did is correct?

osvaldokalvaitir commented 2 years ago

@nmarchais did you manage to resolve otherwise? I saw that I can't use the 'hasUnsyncedChanges' function because there's always something to sync, however much I sync it doesn't change to 'synced'

nmarchais commented 2 years ago

Yes, I solved the issue thanks to my PR. You need to patch the src using my PR to solve the issue.

Le ven. 15 oct. 2021 à 17:41, Osvaldo Kalvaitir Filho < @.***> a écrit :

@nmarchais https://github.com/nmarchais did you manage to resolve otherwise? I saw that I can't use the 'hasUnsyncedChanges' function because there's always something to sync, however much I sync it doesn't change to 'synced'

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Nozbe/WatermelonDB/issues/1126#issuecomment-944402630, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQ65LGZSCEGASZSUGBWPZLUHBDUFANCNFSM5BXTREVQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- @+,

Nicolas

radex commented 2 years ago

@nmarchais You're right, sync was not aware of the case of non-globally-unique IDs. Your fix looks OK, but got out of date with my more recent changes. Sorry I was not able to react to your issue in a timely manner. I think I made a correct fix + tests here: https://github.com/Nozbe/WatermelonDB/pull/1198 . I'd appreciate if you could confirm that this change works for you