pubkey / rxdb

A fast, local first, reactive Database for JavaScript Applications https://rxdb.info/
https://rxdb.info/
Apache License 2.0
20.95k stars 1.02k forks source link

RxDB does not respond to collection.remove() changes. #5721

Open AleksandrMatuka opened 4 months ago

AleksandrMatuka commented 4 months ago

Version: 13 - 15 environment:: chrome

Issue: When attempting to remove all data from a collection using collection.remove(), the data remains in other tabs until they are reloaded. I suspect this issue might also be related to the fact that we cannot listen to the collection.remove(), operations.

Expected Outcome:

The collection.remove()method should be listened to in other tabs as well as through hooks similar to all other operations - bulkRemove, update, create operations.

Step to reproduce: 1)Open the application in 2 tabs. 2)Clear the collection using the remove() method. 3)Verify that there is no data left in the collection on the current page (specifically not in the indexedDB storage but in the RxDB instance itself). 4)Open another tab and ensure that the data is still available (possibly because RxDB does not react to collection remove operations unlike other operations).

What do I mean when comparing with other operations? We have a set of hooks that allow us to listen to various database operations:

preInsert
postInsert
preSave
postSave
preRemove
postRemove
postCreate

So, the operationcollection.remove() cannot be listened to in this way. Although it may have nothing to do with the described problem.

pubkey commented 4 months ago

Yes this feature is missing, you can do a pull request. I think the best way to implement would be to add RxCollection.remove$ and then observe the internal store of the RxDatabase for the event where the metadata of the collection gets removed. Then you know you have to trigger the .remove$ observable.

phal0r commented 4 months ago

@pubkey Instead of implementing this on our own, would it be an option to fund this feature?

Our goal is to have a bulletproof logout, which cleans all data in the browser (when using persistent storages like indexeddb). We encountered different problems on this matter:

We are experimenting with cleaning up the data with bulkRemove/cleanup, so the events are send to all tabs and data is purged. For the interrupted logout we are setting a flag, which indicates, that the logout/cleanup was not finished and will be resumed on next load, but we think, that it is more working around the problem and probably leading to other problems in the future.

So the requirement would be to have a bulletproof way to reset the data to an empty state as fast as possible, informing all open tabs and considering conditions like reloads. Are you open to discuss a funded solution?

pubkey commented 3 months ago

No sorry I do not have time for that. Implementing the collection.remove$ observable isn't that hard, so just make a PR if you need that.

@phal0r some of these things you describe look like bugs, so make a PR with a test case then I will fix that.

Connum commented 3 months ago

There is also the postRemoveRxCollection hook that might help? By the way, there already is an RxCollection.remove$ observable, but it only gets triggered on single document deletion.

pubkey commented 3 months ago

@Connum ah I forgot. So RxCollection.remove$ already fires on document-delete events. So we should use something like RxCollection.collectionRemoved$ or so.

AleksandrMatuka commented 3 months ago

@Connum You probably mean just postRemove, or do we have some hook that wasn't mentioned in the documentation?

Unfortunately, it only tracks the removal of single documents, not the entire collection.

So, in this case, it will work with collection.find().remove(), but not with collection.remove().

So I associated the fact that data in other tabs doesn't update specifically with the lack of tracking the event of deleting the entire collection.

@pubkey I'll see if I can implement this myself; but I'm not very proficient in the concept of reactivity and I'm not familiar with the rxdb codebase at the moment

Connum commented 3 months ago

You probably mean just postRemove, or do we have some hook that wasn't mentioned in the documentation?

No, postRemoveRxCollection is what I found in the code base, and it looks like it's in the right spot, but I haven't tried it myself.

phal0r commented 3 months ago

@pubkey We give it a go and try to add the observable. It may not be much work, but we still need to understand the structure to a certain degree.

Apart from that, we sometimes face the problem, that the remove() is interrupted by closing the browser early. This leads to incomplete data, i.e. records are deleted, but replication checkpoints not and the new replication cycle won't fetch the data again, leaving the user with an empty collection. Do you have something in mind (or already in place) to flag, if the removal process was interrupted?

pubkey commented 3 months ago

@phal0r the replication protocol was constructed in a way that should not leave a broken state no matter when the browser/javascirpt process crashes. After restarting the replication should continue and at one point have the exact same state on both sides again.

If you can reproduce your describe behavior in a test case, I can fix it.

stale[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. If you still have a problem, make a PR with a test case or to prove that you have tried to fix the problem. Notice that only bugs in the rxdb premium plugins are ensured to be fixed by the maintainer. Everything else is expected to be fixed by the community, likely you must fix it by yourself.

phal0r commented 3 months ago

@pubkey First to clarify, what I was talking about: We don't have problems with the replication itself. We have problems, with logging users out and deleting RxDB. Sometimes the deletion is interrupted due to page reload and leaves the database in this incomplete state (data is delete, some metadata like replication checkpoints remains). This process must be guarded and continued on reload, if it is detected, that the deletion did not complete.

Regarding the broadcasting of the removal of collections, we had some discussions and roamed through the code. We cannot use the existing changeStream as it is meant purely for documents. It seems to be unreasonable high effort (and also makes not much sense) to create a stream for the removal of collections as this only happens very rarely, so currently we go with a plugin to use the mentioned hooks and broadcast the removal to other tabs to purge the cache. It's the best approach we came up with until now :)

pubkey commented 3 months ago

I think a better approach would be to listen for the internal storage instance of the database. When a collection is removed, its metadata will be deleted. This deletion can be observed accross tabs.

phal0r commented 3 months ago

Thanks for the input. @AleksandrMatuka let's discuss this.

AleksandrMatuka commented 3 months ago

I think a better approach would be to listen for the internal storage instance of the database. When a collection is removed, its metadata will be deleted. This deletion can be observed accross tabs.

@pubkey This could have been a solution if we could distinguish the type of operation inside the db subscription, whether it's a creation, update, or deletion. But unfortunately, it always shows as UPDATE, and there's nothing we can latch onto to use it for clearing and interpret it as a deletion operation.

So, when subscribing to the database, we do get a trigger fired for every collection deletion, but we can't distinguish the type of operation inside because the entity doesn't allow us to do that.

db.$.subscribe((entity) => {
  // filter all metadocs and remove collections
})

While it is possible to come up with some workaround here, it seems that using broadcast is still an option

stale[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. If you still have a problem, make a PR with a test case or to prove that you have tried to fix the problem. Notice that only bugs in the rxdb premium plugins are ensured to be fixed by the maintainer. Everything else is expected to be fixed by the community, likely you must fix it by yourself.

phal0r commented 2 months ago

We would appreciate some feedback on the aformentioned problem. If we were to use the changestream, how do we detect deleted collections, if the docs are not marked as deleted?

pubkey commented 2 months ago

The meta docs of the collection should be marked as deleted and therefore you should be able to detect that a collection was removed.

stale[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. If you still have a problem, make a PR with a test case or to prove that you have tried to fix the problem. Notice that only bugs in the rxdb premium plugins are ensured to be fixed by the maintainer. Everything else is expected to be fixed by the community, likely you must fix it by yourself.

phal0r commented 1 month ago

still relevant

stale[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. Please update it or it may be closed to keep our repository organized. The best way is to add some more information or make a pull request with a test case. Also you might get help in fixing it at the RxDB Community Chat If you know you will continue working on this, just write any message to the issue (like "ping") to remove the stale tag.

pubkey commented 1 month ago

Should be fixed here: https://github.com/pubkey/rxdb/pull/6013