Skip to content

Merging Master into Multi-Tab #1045

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 25 commits into from
Jul 25, 2018

Conversation

schmidt-sebastian
Copy link
Contributor

Hey there! So you want to contribute to a Firebase SDK?
Before you file this pull request, please read these guidelines:

Discussion

  • Read the contribution guidelines (CONTRIBUTING.md).
  • If this has been discussed in an issue, make sure to link to the issue here.
    If not, go file an issue about this before creating a pull request to discuss.

Testing

  • Make sure all existing tests in the repository pass after your change.
  • If you fixed a bug or added a feature, add a new test to cover your code.

API Changes

  • At this time we cannot accept changes that affect the public API. If you'd like to help
    us make Firebase APIs better, please propose your change in an issue so that we
    can discuss it together.

Josh Crowther and others added 25 commits July 9, 2018 11:41
* Embed metadata directly into the RPC call

* [AUTOMATED]: Prettier Code Styling

* Use ...args

* [AUTOMATED]: Prettier Code Styling

* Minimize diff

* Add the OAuth assertion back in
* initial database code

* test setup

* database tests

* auditTrail and database tests

* [AUTOMATED]: Prettier Code Styling

* [AUTOMATED]: License Headers

* Josh's comments. Database docs

* [AUTOMATED]: Prettier Code Styling

* Firestore docs

* auth docs

* declaration fixes

* switch to peerDeps

* [AUTOMATED]: Prettier Code Styling

* test config
Also remove test code that was combining multiple array contains queries since those were disallowed in 04c9c3a.
This is a refactoring to unify filter creation across platforms.
* Enable firestore sdk to talk to emulator

* [AUTOMATED]: Prettier Code Styling

* Revert firestore sdk changes

* [AUTOMATED]: Prettier Code Styling

* Revert credentials.ts

* Cleanup

* [AUTOMATED]: Prettier Code Styling

* Set webSafe=false

* Combine initializeTestApp and initializeFirestoreTestApp

* [AUTOMATED]: Prettier Code Styling

* Cleanup

* [AUTOMATED]: Prettier Code Styling

* Update major version since this is a breaking change that will cause the testing sdk to no longer work with old versions of the RTDB emulator

* Completely remove admin sdk

* Change version back to 0.1.0
…ufactured deletes. (#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.targetContainsDocument() 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, targetContainsDocument() returned false 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 true from
targetContainsDocument().
…tes in it (#1015)

* Write a spec test for the busted cache

* Modify spec test to demonstrate deletedDoc issue. (#1017)

* Allow updates for targets where the document is modified
While porting I noticed this was slightly wrong. targetContainsDocument() is the method in WatchChangeAggregator. The SyncEngine method I meant to reference is getRemoteKeysForTarget().
* Add a schema migration that drops the query cache

This is a force fix for potential existence filter mismatches caused by
firebase/firebase-ios-sdk#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.

* Implement review feedback
Ensure the v3 migration unconditionally creates the TargetGlobal row. Remove the no-longer-necessary v2 schema migration.
* Fix an errant any usage

* [AUTOMATED]: Prettier Code Styling
This makes it line up with it's own docs, and also the other platforms.
* Add iat to fake access token payload

* [AUTOMATED]: Prettier Code Styling

* Simpler tests

* [AUTOMATED]: Prettier Code Styling

* Do not clobber iat
@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.

@schmidt-sebastian schmidt-sebastian merged commit 59a7208 into firestore-multi-tab Jul 25, 2018
@schmidt-sebastian schmidt-sebastian deleted the firestore-dirtymastermerge branch August 3, 2018 17:15
@firebase firebase locked and limited conversation to collaborators Oct 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.