-
Notifications
You must be signed in to change notification settings - Fork 614
Remove local references from reference delegate #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, you will be adding the test for this in an upcoming PR?
@@ -558,7 +558,13 @@ public void releaseQuery(Query query) { | |||
queryCache.updateQueryData(queryData); | |||
} | |||
|
|||
localViewReferences.removeReferencesForId(queryData.getTargetId()); | |||
// We remove both locally edited documents and documents that are associated via the query | |||
// cache. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add more to this comment. Something like: "We may have determined that a document is associated with a target, but if that hasn't been confirmed by the backend, the query cache won't know to remove the reference for those documents. Make sure we remove them here."
Or some variation on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the comment to:
// References for documents sent via Watch are automatically removed when we delete a
// query's target data from the reference delegate. Since this does not remove references
// for locally mutated documents, we have to remove the target associations for these
// documents manually.
c7d7a95
to
5e1f03c
Compare
FYI: I also added a unit test. |
Fixes issue with GC that prevented local document removal for documents that had pending mutations.
Also renames applyBatchResult to applyWriteToRemoteDocuments to match Web.