LimitToLast: back port some changes from android (#2382)

* LimitToLast: back port some changes from android

* [AUTOMATED]: Prettier Code Styling

* Address comments

Author: wu-hui

packages/firestore/src/core/query.ts

@@ -48,7 +48,7 @@ export class Query {
   private memoizedOrderBy: OrderBy[] | null = null;
 
   // The corresponding `Target` of this `Query` instance.
-  private memorizedTarget: Target | null = null;
+  private memoizedTarget: Target | null = null;
 
   /**
    * Initializes a Query with a path and optional additional query constraints.
@@ -343,9 +343,9 @@ export class Query {
    * representation.
    */
   toTarget(): Target {
-    if (!this.memorizedTarget) {
+    if (!this.memoizedTarget) {
       if (this.limitType === LimitType.First) {
-        this.memorizedTarget = new Target(
+        this.memoizedTarget = new Target(
           this.path,
           this.collectionGroup,
           this.orderBy,
@@ -374,7 +374,7 @@ export class Query {
           : null;
 
         // Now return as a LimitType.First query.
-        this.memorizedTarget = new Target(
+        this.memoizedTarget = new Target(
           this.path,
           this.collectionGroup,
           orderBys,
@@ -385,7 +385,7 @@ export class Query {
         );
       }
     }
-    return this.memorizedTarget!;
+    return this.memoizedTarget!;
   }
 
   private matchesPathAndCollectionGroup(doc: Document): boolean {

packages/firestore/src/local/local_store.ts

@@ -890,9 +890,10 @@ export class LocalStore {
     keepPersistedTargetData: boolean
   ): Promise<void> {
     const targetData = this.targetDataByTarget.get(targetId);
-    if (!targetData) {
-      return Promise.resolve();
-    }
+    assert(
+      targetData !== null,
+      `Tried to release nonexistent target: ${targetId}`
+    );
 
     const mode = keepPersistedTargetData
       ? 'readwrite-idempotent'
@@ -917,15 +918,15 @@ export class LocalStore {
           return PersistencePromise.forEach(removed, (key: DocumentKey) =>
             this.persistence.referenceDelegate.removeReference(txn, key)
           ).next(() => {
-            this.persistence.referenceDelegate.removeTarget(txn, targetData);
+            this.persistence.referenceDelegate.removeTarget(txn, targetData!);
           });
         } else {
           return PersistencePromise.resolve();
         }
       })
       .then(() => {
         this.targetDataByTarget = this.targetDataByTarget.remove(targetId);
-        this.targetIdByTarget.delete(targetData.target);
+        this.targetIdByTarget.delete(targetData!.target);
       });
   }
 

packages/firestore/src/remote/remote_event.ts

@@ -126,12 +126,10 @@ export class TargetChange {
   ) {}
 
   /**
-   * HACK: Views require TargetChanges in order to determine whether the view is
-   * CURRENT, but secondary tabs don't receive remote events. So this method is
-   * used to create a synthesized TargetChanges that can be used to apply a
-   * CURRENT status change to a View, for queries executed in a different tab.
+   * This method is used to create a synthesized TargetChanges that can be used to
+   * apply a CURRENT status change to a View (for queries executed in a different
+   * tab) or for new queries (to raise snapshots with correct CURRENT status).
    */
-  // PORTING NOTE: Multi-tab only
   static createSynthesizedTargetChangeForCurrentChange(
     targetId: TargetId,
     current: boolean

packages/firestore/src/remote/remote_store.ts

@@ -263,9 +263,10 @@ export class RemoteStore implements TargetMetadataProvider {
    * not being listened to.
    */
   unlisten(targetId: TargetId): void {
-    if (!objUtils.contains(this.listenTargets, targetId)) {
-      return;
-    }
+    assert(
+      objUtils.contains(this.listenTargets, targetId),
+      `unlisten called on target no currently watched: ${targetId}`
+    );
 
     delete this.listenTargets[targetId];
     if (this.watchStream.isOpen()) {

packages/firestore/test/integration/api/query.test.ts

@@ -143,144 +143,65 @@ apiDescribe('Queries', (persistence: boolean) => {
   // Since limitToLast() queries are sent to the backend with a modified
   // orderBy() clause, they can map to the same target representation as
   // limit() query, even if both queries appear separate to the user.
-  it('can listen to mirror queries', () => {
+  it('can listen/unlisten/relisten to mirror queries', () => {
     const testDocs = {
       a: { k: 'a', sort: 0 },
       b: { k: 'b', sort: 1 },
       c: { k: 'c', sort: 1 },
       d: { k: 'd', sort: 2 }
     };
     return withTestCollection(persistence, testDocs, async collection => {
+      // Setup `limit` query
       const storeLimitEvent = new EventsAccumulator<firestore.QuerySnapshot>();
-      collection
+      let limitUnlisten = collection
         .orderBy('sort', 'asc')
         .limit(2)
         .onSnapshot(storeLimitEvent.storeEvent);
 
+      // Setup mirroring `limitToLast` query
       const storeLimitToLastEvent = new EventsAccumulator<
         firestore.QuerySnapshot
       >();
-      collection
+      let limitToLastUnlisten = collection
         .orderBy('sort', 'desc')
         .limitToLast(2)
         .onSnapshot(storeLimitToLastEvent.storeEvent);
 
+      // Verify both queries get expected results.
       let snapshot = await storeLimitEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'a', sort: 0 },
         { k: 'b', sort: 1 }
       ]);
-
       snapshot = await storeLimitToLastEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'b', sort: 1 },
         { k: 'a', sort: 0 }
       ]);
 
-      await collection.add({ k: 'e', sort: -1 });
-      snapshot = await storeLimitEvent.awaitEvent();
-      expect(toDataArray(snapshot)).to.deep.equal([
-        { k: 'e', sort: -1 },
-        { k: 'a', sort: 0 }
-      ]);
-
-      snapshot = await storeLimitToLastEvent.awaitEvent();
-      expect(toDataArray(snapshot)).to.deep.equal([
-        { k: 'a', sort: 0 },
-        { k: 'e', sort: -1 }
-      ]);
-    });
-  });
-
-  it('can unlisten from mirror queries', () => {
-    const testDocs = {
-      a: { k: 'a', sort: 0 },
-      b: { k: 'b', sort: 1 },
-      c: { k: 'c', sort: 1 },
-      d: { k: 'd', sort: 2 }
-    };
-    return withTestCollection(persistence, testDocs, async collection => {
-      const storeLimitEvent = new EventsAccumulator<firestore.QuerySnapshot>();
-      const limitUnlisten = collection
-        .orderBy('sort', 'asc')
-        .limit(2)
-        .onSnapshot(storeLimitEvent.storeEvent);
-
-      const storeLimitToLastEvent = new EventsAccumulator<
-        firestore.QuerySnapshot
-      >();
-      const limitToLastUnlisten = collection
-        .orderBy('sort', 'desc')
-        .limitToLast(2)
-        .onSnapshot(storeLimitToLastEvent.storeEvent);
-
-      await storeLimitEvent.awaitEvent();
-      await storeLimitToLastEvent.awaitEvent();
-
-      limitUnlisten();
-
-      await collection.add({ k: 'e', sort: -1 });
-      await storeLimitEvent.assertNoAdditionalEvents();
-      const snapshot = await storeLimitToLastEvent.awaitEvent();
-      // Check the limitToLast query still functions after the mirroring
-      // limit query is un-listened.
-      expect(toDataArray(snapshot)).to.deep.equal([
-        { k: 'a', sort: 0 },
-        { k: 'e', sort: -1 }
-      ]);
-
-      limitToLastUnlisten();
-
-      await collection.add({ k: 'f', sort: -1 });
-      await storeLimitToLastEvent.assertNoAdditionalEvents();
-    });
-  });
-
-  it('can relisten to mirror queries', () => {
-    const testDocs = {
-      a: { k: 'a', sort: 0 },
-      b: { k: 'b', sort: 1 },
-      c: { k: 'c', sort: 1 },
-      d: { k: 'd', sort: 2 }
-    };
-    return withTestCollection(persistence, testDocs, async collection => {
-      const storeLimitEvent = new EventsAccumulator<firestore.QuerySnapshot>();
-      let limitUnlisten = collection
-        .orderBy('sort', 'asc')
-        .limit(2)
-        .onSnapshot(storeLimitEvent.storeEvent);
-
-      const storeLimitToLastEvent = new EventsAccumulator<
-        firestore.QuerySnapshot
-      >();
-      let limitToLastUnlisten = collection
-        .orderBy('sort', 'desc')
-        .limitToLast(2)
-        .onSnapshot(storeLimitToLastEvent.storeEvent);
-
-      await storeLimitEvent.awaitEvent();
-      await storeLimitToLastEvent.awaitEvent();
-
       // Unlisten then relisten limit query.
       limitUnlisten();
       limitUnlisten = collection
         .orderBy('sort', 'asc')
         .limit(2)
         .onSnapshot(storeLimitEvent.storeEvent);
-      let snapshot = await storeLimitEvent.awaitEvent();
+
+      // Verify `limit` query still works.
+      snapshot = await storeLimitEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'a', sort: 0 },
         { k: 'b', sort: 1 }
       ]);
 
+      // Add a document that would change the result set.
       await collection.add({ k: 'e', sort: -1 });
-      // Verify limit query results.
+
+      // Verify both queries get expected results.
       snapshot = await storeLimitEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'e', sort: -1 },
         { k: 'a', sort: 0 }
       ]);
-      // Verify limitToLast query results.
       snapshot = await storeLimitToLastEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'a', sort: 0 },
@@ -295,14 +216,12 @@ apiDescribe('Queries', (persistence: boolean) => {
         .limitToLast(2)
         .onSnapshot(storeLimitToLastEvent.storeEvent);
 
+      // Verify both queries get expected results.
       snapshot = await storeLimitEvent.awaitEvent();
-      // Checking limit query is still functioning when the mirroring
-      // limitToLast query is un-listened.
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'a', sort: -2 },
         { k: 'e', sort: -1 }
       ]);
-
       snapshot = await storeLimitToLastEvent.awaitEvent();
       expect(toDataArray(snapshot)).to.deep.equal([
         { k: 'e', sort: -1 },