bugfix: Issue #455: StorageContext not saving data after deleting document/ref info #486
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -55,6 +55,11 @@ export class SimpleKVStore extends BaseKVStore { | |||
| ): Promise<boolean> { | |||
| if (key in this.data[collection]) { | |||
| delete this.data[collection][key]; | |||
|
|
|||
| if (this.persistPath) { | |||
There was a problem hiding this comment.
@1997roylee thanks for looking into #455!
but persisting the whole store can take a long time, so doing it for every change operation seems not to be feasible. That's why we're having the explicit persist method.
We're working on a fix for #455.
There was a problem hiding this comment.
It makes sense, but why is the 'persist' method checking if the instance is equal to 'BaseInMemoryKVStore'? I don't see any involvement of 'SimpleDocumentStore' with 'BaseInMemoryKVStore'.
I proposed to change the BaseInMemoryKVStore to SimpleKVStore.
Original code:
this.kvStore instanceof BaseInMemoryKVStoreNow:
this.kvStore instanceof SimpleKVStoreThere was a problem hiding this comment.
@1997roylee good catch, i think we don't need this instanceof check at all - @himself65 can you have a look into this while working on #487 ?
There was a problem hiding this comment.
yes I think instanceof is useless here. But it okay for now
There was a problem hiding this comment.
should I close this PR? Or I can help to fix the persist issue of another class.
Issue #455: StorageContext not saving data after deleting document/ref info
Changes Proposed:
Change the BaseInMemoryKVStore to SimpleKVStore for SimpleDocumentStore.
Original code:
Now: