Skip to content

Don't leak orphaned docs when deleting #32

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

Merged
merged 2 commits into from
Sep 20, 2018
Merged

Conversation

gsoltis
Copy link
Contributor

@gsoltis gsoltis commented Sep 19, 2018

Previously, we weren't deleting our tracking of orphaned documents when we deleted the doc itself. I moved the deletion into the reference delegate, which is probably a better home anyways, and delete from the orphaned doc map at the same time so we no longer leak.

Caught while porting to web.

}
docs = updated;
return count;
ImmutableSortedMap<DocumentKey, MaybeDocument> getDocuments() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Rename to getAllDocuments() to match getAllDocumentsMatchingQuery().

Feel free to disagree :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other one should change to getDocumentsMatchingQuery().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine by me. Can you make this change (if you want, in a follow up?). getDocumentsMatchingQuery() is already the name we use on Web.

@ashwinraghav
Copy link
Contributor

/retest

@gsoltis
Copy link
Contributor Author

gsoltis commented Sep 20, 2018

/test smoke-tests-experimental

@gsoltis
Copy link
Contributor Author

gsoltis commented Sep 20, 2018

/test connected-check

@gsoltis
Copy link
Contributor Author

gsoltis commented Sep 20, 2018

/test connected-check

@gsoltis
Copy link
Contributor Author

gsoltis commented Sep 20, 2018

/test smoke-tests-experimental

@gsoltis gsoltis merged commit ff6ae3f into master Sep 20, 2018
@gsoltis gsoltis deleted the gsoltis/dont_leak_docs branch September 20, 2018 21:46
@firebase firebase locked and limited conversation to collaborators Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants