Add some more spec tests

Author: dconeybe

packages/firestore/src/core/sync_engine_impl.ts

@@ -929,9 +929,13 @@ function trackLimboChange(
   limboChange: AddedLimboDocument
 ): void {
   const key = limboChange.key;
-  if (!syncEngineImpl.activeLimboTargetsByKey.get(key)) {
+  const keyString = key.path.canonicalString();
+  if (
+    !syncEngineImpl.activeLimboTargetsByKey.get(key) &&
+    !syncEngineImpl.enqueuedLimboResolutions.has(keyString)
+  ) {
     logDebug(LOG_TAG, 'New document in limbo: ' + key);
-    syncEngineImpl.enqueuedLimboResolutions.add(key.path.canonicalString());
+    syncEngineImpl.enqueuedLimboResolutions.add(keyString);
     pumpEnqueuedLimboResolutions(syncEngineImpl);
   }
 }

packages/firestore/test/unit/specs/limbo_spec.test.ts

@@ -924,7 +924,7 @@ describeSpec('Limbo Documents:', [], () => {
   );
 
   specTest(
-    'Enqueued limbo resolutions should be removed when query listen stops',
+    'A limbo resolution for a document should be removed from the queue when the last query listen stops',
     [],
     () => {
       const doc1 = doc('collection1/doc', 1000, { key: 1 });
@@ -934,38 +934,136 @@ describeSpec('Limbo Documents:', [], () => {
       const doc2 = doc('collection2/doc', 1000, { key: 2 });
       const query2 = query('collection2');
       const filteredQuery2 = query('collection2', filter('key', '==', 2));
+      const filteredQuery3 = query('collection2', filter('key', '>=', 2));
 
       return (
         spec()
           .withGCEnabled(false)
           .withMaxConcurrentLimboResolutions(1)
 
           // Max out the number of active limbo resolutions.
-          .userListens(filteredQuery1)
-          .watchAcksFull(filteredQuery1, 1000, doc1)
-          .expectEvents(filteredQuery1, { added: [doc1] })
-          .userUnlistens(filteredQuery1)
           .userListens(query1)
-          .expectEvents(query1, { added: [doc1], fromCache: true })
-          .watchAcksFull(query1, 1001)
+          .watchAcksFull(query1, 1000, doc1)
+          .expectEvents(query1, { added: [doc1] })
+          .userUnlistens(query1)
+          .userListens(filteredQuery1)
+          .expectEvents(filteredQuery1, { added: [doc1], fromCache: true })
+          .watchAcksFull(filteredQuery1, 1001)
           .expectLimboDocs(doc1.key)
 
-          // Enqueue a limbo resolution from another query.
+          // Enqueue a limbo resolution for doc2.
+          .userListens(query2)
+          .watchAcksFull(query2, 1002, doc2)
+          .expectEvents(query2, { added: [doc2] })
+          .userUnlistens(query2)
           .userListens(filteredQuery2)
-          .watchAcksFull(filteredQuery2, 1002, doc2)
-          .expectEvents(filteredQuery2, { added: [doc2] })
+          .expectEvents(filteredQuery2, { added: [doc2], fromCache: true })
+          .watchAcksFull(filteredQuery2, 1003)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs(doc2.key)
+
+          // Start another query that puts doc2 into limbo again.
+          .userListens(filteredQuery3)
+          .expectEvents(filteredQuery3, { added: [doc2], fromCache: true })
+          .watchAcksFull(filteredQuery3, 1004)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs(doc2.key)
+
+          // Stop one of the queries that enqueued a limbo resolution for doc2;
+          // verify that doc2 is not removed from the limbo resolution queue.
+          .userUnlistens(filteredQuery3)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs(doc2.key)
+
+          // Stop the other query that enqueued a limbo resolution for doc2;
+          // verify that doc2 *is* removed from the limbo resolution queue.
           .userUnlistens(filteredQuery2)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs()
+      );
+    }
+  );
+
+  specTest(
+    'A limbo resolution for a document should not be started if one is already active',
+    [],
+    () => {
+      const doc1 = doc('collection/doc', 1000, { key: 1 });
+      const query1 = query('collection');
+      const filteredQuery1 = query('collection', filter('key', '==', 1));
+      const filteredQuery2 = query('collection', filter('key', '>=', 1));
+
+      return (
+        spec()
+          .withGCEnabled(false)
+
+          // Start a limbo resolution listen for a document.
+          .userListens(query1)
+          .watchAcksFull(query1, 1000, doc1)
+          .expectEvents(query1, { added: [doc1] })
+          .userUnlistens(query1)
+          .userListens(filteredQuery1)
+          .expectEvents(filteredQuery1, { added: [doc1], fromCache: true })
+          .watchAcksFull(filteredQuery1, 1001)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs()
+
+          // Put the same document into limbo in a different query.
+          .userListens(filteredQuery2)
+          .expectEvents(filteredQuery2, { added: [doc1], fromCache: true })
+          .watchAcksFull(filteredQuery2, 1002)
+          .expectLimboDocs(doc1.key)
+          .expectEnqueuedLimboDocs()
+      );
+    }
+  );
+
+  specTest(
+    'A limbo resolution for a document should not be enqueued if one is already enqueued',
+    [],
+    () => {
+      const doc1 = doc('collection1/doc1', 1000, { key: 1 });
+      const query1 = query('collection1');
+      const filteredQuery1 = query('collection1', filter('key', '==', 1));
+      const doc2 = doc('collection2/doc2', 1000, { key: 2 });
+      const query2 = query('collection2');
+      const filteredQuery2 = query('collection2', filter('key', '==', 2));
+      const filteredQuery3 = query('collection2', filter('key', '>=', 2));
+
+      return (
+        spec()
+          .withGCEnabled(false)
+          .withMaxConcurrentLimboResolutions(1)
+
+          // Max out the number of active limbo resolutions.
+          .userListens(query1)
+          .watchAcksFull(query1, 1000, doc1)
+          .expectEvents(query1, { added: [doc1] })
+          .userUnlistens(query1)
+          .userListens(filteredQuery1)
+          .expectEvents(filteredQuery1, { added: [doc1], fromCache: true })
+          .watchAcksFull(filteredQuery1, 1001)
+          .expectLimboDocs(doc1.key)
+
+          // Start a limbo resolution listen for a different document.
           .userListens(query2)
-          .expectEvents(query2, { added: [doc2], fromCache: true })
-          .watchAcksFull(query2, 1003)
+          .watchAcksFull(query2, 1002, doc2)
+          .expectEvents(query2, { added: [doc2] })
+          .userUnlistens(query2)
+          .userListens(filteredQuery2)
+          .expectEvents(filteredQuery2, { added: [doc2], fromCache: true })
+          .watchAcksFull(filteredQuery2, 1003)
           .expectLimboDocs(doc1.key)
           .expectEnqueuedLimboDocs(doc2.key)
 
-          // Stop the other query and ensure that the enqueued limbo resolution
-          // is removed.
-          .userUnlistens(query2)
+          // Put the the "different" document into limbo in a different query
+          // and verify that it's not added to the limbo resolution queue a
+          // second time.
+          .userListens(filteredQuery3)
+          .expectEvents(filteredQuery3, { added: [doc2], fromCache: true })
+          .watchAcksFull(filteredQuery3, 1004)
           .expectLimboDocs(doc1.key)
-          .expectEnqueuedLimboDocs()
+          .expectEnqueuedLimboDocs(doc2.key)
       );
     }
   );