Refactor notifyLocalViewChanges

Author: wu-hui

packages/firestore/src/local/local_store_impl.ts

@@ -773,7 +773,7 @@ function populateDocumentChangeBuffer(
 function shouldPersistTargetData(
   oldTargetData: TargetData,
   newTargetData: TargetData,
-  change: TargetChange
+  change: TargetChange | null
 ): boolean {
   // Always persist target data if we don't already have a resume token.
   if (oldTargetData.resumeToken.approximateByteSize() === 0) {
@@ -792,11 +792,21 @@ function shouldPersistTargetData(
     return true;
   }
 
+  // Update the target cache if sufficient time has passed since the last
+  // LastLimboFreeSnapshotVersion
+  const limboFreeTimeDelta =
+    newTargetData.lastLimboFreeSnapshotVersion.toMicroseconds() -
+    oldTargetData.lastLimboFreeSnapshotVersion.toMicroseconds();
+  if (limboFreeTimeDelta >= RESUME_TOKEN_MAX_AGE_MICROS) {
+    return true;
+  }
+
   // Otherwise if the only thing that has changed about a target is its resume
   // token it's not worth persisting. Note that the RemoteStore keeps an
   // in-memory view of the currently active targets which includes the current
   // resume token, so stream failure or user changes will still use an
   // up-to-date resume token regardless of what we do here.
+  if (change === null) return false;
   const changes =
     change.addedDocuments.size +
     change.modifiedDocuments.size +
@@ -828,17 +838,56 @@ export async function localStoreNotifyLocalViewChanges(
                   viewChange.targetId,
                   key
                 )
-            ).next(() =>
-              PersistencePromise.forEach(
-                viewChange.removedKeys,
-                (key: DocumentKey) =>
-                  localStoreImpl.persistence.referenceDelegate.removeReference(
-                    txn,
-                    viewChange.targetId,
-                    key
-                  )
+            )
+              .next(() =>
+                PersistencePromise.forEach(
+                  viewChange.removedKeys,
+                  (key: DocumentKey) =>
+                    localStoreImpl.persistence.referenceDelegate.removeReference(
+                      txn,
+                      viewChange.targetId,
+                      key
+                    )
+                )
               )
-            );
+              .next(() => {
+                const targetId = viewChange.targetId;
+
+                if (!viewChange.fromCache) {
+                  const targetData =
+                    localStoreImpl.targetDataByTarget.get(targetId);
+                  debugAssert(
+                    targetData !== null,
+                    `Can't set limbo-free snapshot version for unknown target: ${targetId}`
+                  );
+
+                  // Advance the last limbo free snapshot version
+                  const lastLimboFreeSnapshotVersion =
+                    targetData!.snapshotVersion;
+                  const updatedTargetData =
+                    targetData!.withLastLimboFreeSnapshotVersion(
+                      lastLimboFreeSnapshotVersion
+                    );
+                  localStoreImpl.targetDataByTarget =
+                    localStoreImpl.targetDataByTarget.insert(
+                      targetId,
+                      updatedTargetData
+                    );
+
+                  if (
+                    shouldPersistTargetData(
+                      targetData!,
+                      updatedTargetData,
+                      null
+                    )
+                  ) {
+                    localStoreImpl.targetCache.updateTargetData(
+                      txn,
+                      updatedTargetData
+                    );
+                  }
+                }
+              });
           }
         );
       }
@@ -854,26 +903,6 @@ export async function localStoreNotifyLocalViewChanges(
       throw e;
     }
   }
-
-  for (const viewChange of viewChanges) {
-    const targetId = viewChange.targetId;
-
-    if (!viewChange.fromCache) {
-      const targetData = localStoreImpl.targetDataByTarget.get(targetId);
-      debugAssert(
-        targetData !== null,
-        `Can't set limbo-free snapshot version for unknown target: ${targetId}`
-      );
-
-      // Advance the last limbo free snapshot version
-      const lastLimboFreeSnapshotVersion = targetData.snapshotVersion;
-      const updatedTargetData = targetData.withLastLimboFreeSnapshotVersion(
-        lastLimboFreeSnapshotVersion
-      );
-      localStoreImpl.targetDataByTarget =
-        localStoreImpl.targetDataByTarget.insert(targetId, updatedTargetData);
-    }
-  }
 }
 
 /**