Merge master into release

Author: google-oss-bot

.changeset/fast-moose-approve.md

@@ -0,0 +1,5 @@
+---
+'@firebase/app': patch
+---
+
+More safeguards to ensure that heartbeat objects queried from IndexedDB include a heartbeats field.

.changeset/olive-dogs-play.md

@@ -0,0 +1,7 @@
+---
+'@firebase/firestore': patch
+'firebase': patch
+---
+
+Fixed an issue in the local cache synchronization logic where all locally-cached documents that matched a resumed query would be unnecessarily re-downloaded; with the fix it now only downloads the documents that are known to be out-of-sync.
+

packages/app/src/heartbeatService.ts

@@ -90,6 +90,10 @@ export class HeartbeatServiceImpl implements HeartbeatService {
     const date = getUTCDateString();
     if (this._heartbeatsCache?.heartbeats == null) {
       this._heartbeatsCache = await this._heartbeatsCachePromise;
+      // If we failed to construct a heartbeats cache, then return immediately.
+      if (this._heartbeatsCache?.heartbeats == null) {
+        return;
+      }
     }
     // Do not store a heartbeat if one is already stored for this day
     // or if a header has already been sent today.
@@ -236,7 +240,11 @@ export class HeartbeatStorageImpl implements HeartbeatStorage {
       return { heartbeats: [] };
     } else {
       const idbHeartbeatObject = await readHeartbeatsFromIndexedDB(this.app);
-      return idbHeartbeatObject || { heartbeats: [] };
+      if (idbHeartbeatObject?.heartbeats) {
+        return idbHeartbeatObject;
+      } else {
+        return { heartbeats: [] };
+      }
     }
   }
   // overwrite the storage with the provided heartbeats

packages/auth/test/integration/webdriver/compat/firebaseui.test.ts

@@ -51,7 +51,11 @@ browserDescribe('WebDriver integration with FirebaseUI', driver => {
     expect(snap.uid).to.be.a('string');
   });
 
-  it('allows google redirect sign in', async () => {
+  it('allows google redirect sign in', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     const page = await startUi();
     await page.clickGoogleSignIn();
     const widget = new IdPPage(driver.webDriver);

packages/auth/test/integration/webdriver/redirect.test.ts

@@ -48,7 +48,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     await driver.start('chrome');
   });
 
-  it('allows users to sign in', async () => {
+  it('allows users to sign in', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     await driver.callNoWait(RedirectFunction.IDP_REDIRECT);
     const widget = new IdPPage(driver.webDriver);
 
@@ -80,6 +84,10 @@ browserDescribe('WebDriver redirect IdP test', driver => {
 
   // Redirect works with middleware for now
   it('is blocked by middleware', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     if (driver.isCompatLayer()) {
       console.warn('Skipping middleware tests in compat');
       this.skip();
@@ -106,7 +114,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(await driver.getUserSnapshot()).to.be.null;
   });
 
-  it('can link with another account account', async () => {
+  it('can link with another account account', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     // First, sign in anonymously
     const { user: anonUser }: UserCredential = await driver.call(
       AnonFunction.SIGN_IN_ANONYMOUSLY
@@ -128,7 +140,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(user.email).to.eq('bob@bob.test');
   });
 
-  it('can be converted to a credential', async () => {
+  it('can be converted to a credential', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     // Start with redirect
     await driver.callNoWait(RedirectFunction.IDP_REDIRECT);
     const widget = new IdPPage(driver.webDriver);
@@ -156,7 +172,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(second.providerData).to.eql(first.providerData);
   });
 
-  it('handles account exists different credential errors', async () => {
+  it('handles account exists different credential errors', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     // Start with redirect and a verified account
     await driver.callNoWait(RedirectFunction.IDP_REDIRECT);
     const widget = new IdPPage(driver.webDriver);
@@ -191,7 +211,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     ]);
   });
 
-  it('does not auto-upgrade anon accounts', async () => {
+  it('does not auto-upgrade anon accounts', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     const { user: anonUser }: UserCredential = await driver.call(
       AnonFunction.SIGN_IN_ANONYMOUSLY
     );
@@ -208,7 +232,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(curUser.uid).not.to.eq(anonUser.uid);
   });
 
-  it('linking with anonymous user upgrades account', async () => {
+  it('linking with anonymous user upgrades account', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     const { user: anonUser }: UserCredential = await driver.call(
       AnonFunction.SIGN_IN_ANONYMOUSLY
     );
@@ -226,7 +254,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(curUser.isAnonymous).to.be.false;
   });
 
-  it('is possible to link with different email', async () => {
+  it('is possible to link with different email', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     const { user: emailUser }: UserCredential = await driver.call(
       EmailFunction.CREATE_USER,
       'user@test.test'
@@ -249,7 +281,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
     expect(curUser.providerData.length).to.eq(2);
   });
 
-  it('is possible to link with the same email', async () => {
+  it('is possible to link with the same email', async function () {
+    // Test is ignored for now as it fails.
+    // TODO: Investigate and unskip the test.
+    this.skip();
+
     const { user: emailUser }: UserCredential = await driver.call(
       EmailFunction.CREATE_USER,
       'same@test.test'
@@ -291,7 +327,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
       await driver.call(CoreFunction.SIGN_OUT);
     });
 
-    it('a user can sign in again', async () => {
+    it('a user can sign in again', async function () {
+      // Test is ignored for now as it fails.
+      // TODO: Investigate and unskip the test.
+      this.skip();
+
       // Sign in using pre-poulated user
       await driver.callNoWait(RedirectFunction.IDP_REDIRECT);
 
@@ -307,7 +347,11 @@ browserDescribe('WebDriver redirect IdP test', driver => {
       expect(user.email).to.eq(user1.email);
     });
 
-    it('reauthenticate works for the correct user', async () => {
+    it('reauthenticate works for the correct user', async function () {
+      // Test is ignored for now as it fails.
+      // TODO: Investigate and unskip the test.
+      this.skip();
+
       // Sign in using pre-poulated user
       await driver.callNoWait(RedirectFunction.IDP_REDIRECT);
 

packages/firestore/src/core/firestore_client.ts

@@ -729,7 +729,7 @@ async function executeQueryFromCache(
     const viewDocChanges = view.computeDocChanges(queryResult.documents);
     const viewChange = view.applyChanges(
       viewDocChanges,
-      /* updateLimboDocuments= */ false
+      /* limboResolutionEnabled= */ false
     );
     result.resolve(viewChange.snapshot!);
   } catch (e) {

packages/firestore/src/core/sync_engine_impl.ts

@@ -368,7 +368,7 @@ async function initializeViewAndComputeSnapshot(
     );
   const viewChange = view.applyChanges(
     viewDocChanges,
-    /* updateLimboDocuments= */ syncEngineImpl.isPrimaryClient,
+    /* limboResolutionEnabled= */ syncEngineImpl.isPrimaryClient,
     synthesizedTargetChange
   );
   updateTrackedLimbos(syncEngineImpl, targetId, viewChange.limboChanges);
@@ -1081,10 +1081,13 @@ async function applyDocChanges(
 
   const targetChange =
     remoteEvent && remoteEvent.targetChanges.get(queryView.targetId);
+  const targetIsPendingReset =
+    remoteEvent && remoteEvent.targetMismatches.get(queryView.targetId) != null;
   const viewChange = queryView.view.applyChanges(
     viewDocChanges,
-    /* updateLimboDocuments= */ syncEngineImpl.isPrimaryClient,
-    targetChange
+    /* limboResolutionEnabled= */ syncEngineImpl.isPrimaryClient,
+    targetChange,
+    targetIsPendingReset
   );
   updateTrackedLimbos(
     syncEngineImpl,

packages/firestore/src/core/view.ts

@@ -271,17 +271,21 @@ export class View {
    * Updates the view with the given ViewDocumentChanges and optionally updates
    * limbo docs and sync state from the provided target change.
    * @param docChanges - The set of changes to make to the view's docs.
-   * @param updateLimboDocuments - Whether to update limbo documents based on
+   * @param limboResolutionEnabled - Whether to update limbo documents based on
    *        this change.
    * @param targetChange - A target change to apply for computing limbo docs and
    *        sync state.
+   * @param targetIsPendingReset - Whether the target is pending to reset due to
+   *        existence filter mismatch. If not explicitly specified, it is treated
+   *        equivalently to `false`.
    * @returns A new ViewChange with the given docs, changes, and sync state.
    */
   // PORTING NOTE: The iOS/Android clients always compute limbo document changes.
   applyChanges(
     docChanges: ViewDocumentChanges,
-    updateLimboDocuments: boolean,
-    targetChange?: TargetChange
+    limboResolutionEnabled: boolean,
+    targetChange?: TargetChange,
+    targetIsPendingReset?: boolean
   ): ViewChange {
     debugAssert(
       !docChanges.needsRefill,
@@ -300,10 +304,18 @@ export class View {
     });
 
     this.applyTargetChange(targetChange);
-    const limboChanges = updateLimboDocuments
-      ? this.updateLimboDocuments()
-      : [];
-    const synced = this.limboDocuments.size === 0 && this.current;
+
+    targetIsPendingReset = targetIsPendingReset ?? false;
+    const limboChanges =
+      limboResolutionEnabled && !targetIsPendingReset
+        ? this.updateLimboDocuments()
+        : [];
+
+    // We are at synced state if there is no limbo docs are waiting to be resolved, view is current
+    // with the backend, and the query is not pending to reset due to existence filter mismatch.
+    const synced =
+      this.limboDocuments.size === 0 && this.current && !targetIsPendingReset;
+
     const newSyncState = synced ? SyncState.Synced : SyncState.Local;
     const syncStateChanged = newSyncState !== this.syncState;
     this.syncState = newSyncState;
@@ -350,7 +362,7 @@ export class View {
           mutatedKeys: this.mutatedKeys,
           needsRefill: false
         },
-        /* updateLimboDocuments= */ false
+        /* limboResolutionEnabled= */ false
       );
     } else {
       // No effect, just return a no-op ViewChange.
@@ -458,7 +470,7 @@ export class View {
     this._syncedDocuments = queryResult.remoteKeys;
     this.limboDocuments = documentKeySet();
     const docChanges = this.computeDocChanges(queryResult.documents);
-    return this.applyChanges(docChanges, /*updateLimboDocuments=*/ true);
+    return this.applyChanges(docChanges, /* limboResolutionEnabled= */ true);
   }
 
   /**

packages/firestore/test/unit/core/view.test.ts

@@ -304,19 +304,19 @@ describe('View', () => {
     let changes = view.computeDocChanges(documentUpdates(doc1));
     let viewChange = view.applyChanges(
       changes,
-      /* updateLimboDocuments= */ true
+      /* limboResolutionEnabled= */ true
     );
     expect(viewChange.snapshot!.fromCache).to.be.true;
 
     // Add doc2 to generate a snapshot. Doc1 is still missing.
     changes = view.computeDocChanges(documentUpdates(doc2));
-    viewChange = view.applyChanges(changes, /* updateLimboDocuments= */ true);
+    viewChange = view.applyChanges(changes, /* limboResolutionEnabled= */ true);
     expect(viewChange.snapshot!.fromCache).to.be.true;
 
     // Add doc2 to the backend's result set.
     viewChange = view.applyChanges(
       changes,
-      /* updateLimboDocuments= */ true,
+      /* limboResolutionEnabled= */ true,
       updateMapping(version(0), [doc2], [], [], /* current= */ true)
     );
     // We are CURRENT but doc1 is in limbo.
@@ -325,7 +325,7 @@ describe('View', () => {
     // Add doc1 to the backend's result set.
     viewChange = view.applyChanges(
       changes,
-      /* updateLimboDocuments= */ true,
+      /* limboResolutionEnabled= */ true,
       updateMapping(version(0), [doc1], [], [], /* current= */ true)
     );
     expect(viewChange.snapshot!.fromCache).to.be.false;

packages/firestore/test/unit/local/query_engine.test.ts

@@ -253,7 +253,7 @@ function genericQueryEngineTest(
               const viewDocChanges = view.computeDocChanges(docs);
               return view.applyChanges(
                 viewDocChanges,
-                /*updateLimboDocuments=*/ true
+                /* limboResolutionEnabled= */ true
               ).snapshot!.docs;
             });
         });