Merge f80f1f2 into 480d7d5

Author: ehsannas

.changeset/grumpy-bees-shake.md

@@ -0,0 +1,5 @@
+---
+"@firebase/firestore": patch
+---
+
+Fixed stack overflow caused by deeply nested server timestamps.

packages/firestore/src/model/server_timestamps.ts

@@ -73,6 +73,17 @@ export function serverTimestamp(
     }
   };
 
+  // We should avoid storing deeply nested server timestamp map values
+  // because we never use the intermediate "previous values".
+  // For example:
+  // previous: 42L, add: t1, result: t1 -> 42L
+  // previous: t1,  add: t2, result: t2 -> 42L (NOT t2 -> t1 -> 42L)
+  // previous: t2,  add: t3, result: t3 -> 42L (NOT t3 -> t2 -> t1 -> 42L)
+  // `getPreviousValue` recursively traverses server timestamps to find the
+  // least recent Value.
+  if (previousValue && isServerTimestamp(previousValue)) {
+    previousValue = getPreviousValue(previousValue);
+  }
   if (previousValue) {
     mapValue.fields![PREVIOUS_VALUE_KEY] = previousValue;
   }

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

@@ -61,16 +61,22 @@ import {
   DocumentMap
 } from '../../../src/model/collections';
 import { Document } from '../../../src/model/document';
+import { FieldMask } from '../../../src/model/field_mask';
 import {
+  FieldTransform,
   Mutation,
   MutationResult,
   MutationType,
+  PatchMutation,
   Precondition
 } from '../../../src/model/mutation';
 import {
   MutationBatch,
   MutationBatchResult
 } from '../../../src/model/mutation_batch';
+import { ObjectValue } from '../../../src/model/object_value';
+import { serverTimestamp } from '../../../src/model/server_timestamps';
+import { ServerTimestampTransform } from '../../../src/model/transform_operation';
 import { BundleMetadata as ProtoBundleMetadata } from '../../../src/protos/firestore_bundle_proto';
 import * as api from '../../../src/protos/firestore_proto_api';
 import { RemoteEvent } from '../../../src/remote/remote_event';
@@ -94,6 +100,7 @@ import {
   docUpdateRemoteEvent,
   existenceFilterEvent,
   expectEqual,
+  field,
   filter,
   key,
   localViewChanges,
@@ -2268,6 +2275,36 @@ function genericLocalStoreTests(
       .finish();
   });
 
+  it('deeply nested server timestamps do not cause stack overflow', async () => {
+    const timestamp = Timestamp.now();
+    const initialServerTimestamp = serverTimestamp(timestamp, null);
+    const value: ObjectValue = ObjectValue.empty();
+    value.set(
+      field('timestamp'),
+      serverTimestamp(timestamp, initialServerTimestamp)
+    );
+
+    const mutations: PatchMutation[] = [];
+    for (let i = 0; i < 100; ++i) {
+      mutations.push(
+        new PatchMutation(
+          key('foo/bar'),
+          value,
+          new FieldMask([field('timestamp')]),
+          Precondition.none(),
+          [
+            new FieldTransform(
+              field('timestamp'),
+              new ServerTimestampTransform()
+            )
+          ]
+        )
+      );
+    }
+    await expect(expectLocalStore().afterMutations(mutations).finish()).to.not
+      .be.eventually.rejected;
+  });
+
   it('uses target mapping to execute queries', () => {
     if (gcIsEager) {
       return;