Skip to content

Merge Firebase 5.4.1 to master #1567

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 10 commits into from
Jul 20, 2018
Merged

Merge Firebase 5.4.1 to master #1567

merged 10 commits into from
Jul 20, 2018

Conversation

wilhuff
Copy link
Contributor

@wilhuff wilhuff commented Jul 20, 2018

No description provided.

paulb777 and others added 10 commits July 19, 2018 07:02
Replace fallthrough comments with `ABSL_FALLTHROUGH_INTENDED`, allowing
clean building with -Wimplicit-fallthrough.
…ufactured deletes. (#1556)

[Port of firebase/firebase-js-sdk#1014]

This fixes an issue occurring when a limbo target receives a documentUpdate,
then a global snapshot, and then a CURRENT. Because there was a global
snapshot before the CURRENT, WatchChangeAggregator has no pending document
updates and calls SyncEngine.remoteKeysForTarget to see if we previously got any
document from the backend for the target. See:
https://github.com/firebase/firebase-js-sdk/blob/6905339235ad801291edc696dd75a08e80647f5b/packages/firestore/src/remote/watch_change.ts#L422

Prior to this change, remoteKeysForTarget returned empty because
it relies on our Views to track the contents of the target, and we don't
have Views for limbo targets. Thus WatchChangeAggregator incorrectly
manufactures a NoDocument document update which deletes data from our
cache.

The fix is to have SyncEngine track the fact that we did indeed get
a document for the limbo resolution and return it from
remoteKeysForTarget.
…tes in it (#1557)

* Update spec tests from the js-sdk

* Allow remote updates from watch to heal a cache with synthesized deletes in it

Port of firebase/firebase-js-sdk#1015

Addresses #1548
This is a force fix for potential existence filter mismatches caused by
#1548

The essential problem is that when resuming a query, the server is
allowed to forget deletes. If the number of incorrectly synthesized
deletes matches the number of server-forgotten deletes then the
existence filter can give a false positive, preventing the cache from
self healing.

Dropping the query cache clears any client-side resume token which
prevents a false positive existence filter mismatch.

Note that the remote document cache and mutation queues are unaffected
so any cached documents that do exist will still work while offline

firebase/firebase-js-sdk#1019
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

LGTM to merge on travis green.

Remember to merge - not squash.

Thanks!

@paulb777
Copy link
Member

CLA is good based on #1562 (comment).

@paulb777 paulb777 merged commit 3ccb15e into master Jul 20, 2018
@paulb777 paulb777 deleted the wilhuff/merge-5.4.1 branch July 20, 2018 17:52
@firebase firebase locked and limited conversation to collaborators Oct 30, 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.

6 participants