[2.8/3] Add limitToLast implementation and tests. (#2296)

Author: wu-hui

packages/firebase/index.d.ts

@@ -8118,14 +8118,26 @@ declare namespace firebase.firestore {
     ): Query;
 
     /**
-     * Creates and returns a new Query where the results are limited to the
-     * specified number of documents.
+     * Creates and returns a new Query that only returns the first matching
+     * documents.
      *
      * @param limit The maximum number of items to return.
      * @return The created Query.
      */
     limit(limit: number): Query;
 
+    /**
+     * Creates and returns a new Query that only returns the last matching
+     * documents.
+     *
+     * You must specify at least one `orderBy` clause for `limitToLast` queries,
+     * otherwise an exception will be thrown thrown during execution.
+     *
+     * @param limit The maximum number of items to return.
+     * @return The created Query.
+     */
+    limitToLast(limit: number): Query;
+
     /**
      * Creates and returns a new Query that starts at the provided document
      * (inclusive). The starting position is relative to the order of the query.

packages/firestore-types/index.d.ts

@@ -1089,14 +1089,26 @@ export class Query {
   ): Query;
 
   /**
-   * Creates and returns a new Query that's additionally limited to only
-   * return up to the specified number of documents.
+   * Creates and returns a new Query that only returns the first matching
+   * documents.
    *
    * @param limit The maximum number of items to return.
    * @return The created Query.
    */
   limit(limit: number): Query;
 
+  /**
+   * Creates and returns a new Query that only returns the last matching
+   * documents.
+   *
+   * Queries with `limitToLast` must have at least one `orderBy` clause on
+   * one of the document fields, or an Exception will throw during execution.
+   *
+   * @param limit The maximum number of items to return.
+   * @return The created Query.
+   */
+  limitToLast(limit: number): Query;
+
   /**
    * Creates and returns a new Query that starts at the provided document
    * (inclusive). The starting position is relative to the order of the query.

packages/firestore/src/api/database.ts

@@ -71,6 +71,7 @@ import {
   validateOptionalArgType,
   validateOptionalArrayElements,
   validateOptionNames,
+  validatePositiveNumber,
   validateStringEnum,
   valueDescription
 } from '../util/input_validation';
@@ -1542,14 +1543,15 @@ export class Query implements firestore.Query {
   limit(n: number): firestore.Query {
     validateExactNumberOfArgs('Query.limit', arguments, 1);
     validateArgType('Query.limit', 'number', 1, n);
-    if (n <= 0) {
-      throw new FirestoreError(
-        Code.INVALID_ARGUMENT,
-        `Invalid Query. Query limit (${n}) is invalid. Limit must be ` +
-          'positive.'
-      );
-    }
-    return new Query(this._query.withLimit(n), this.firestore);
+    validatePositiveNumber('Query.limit', 1, n);
+    return new Query(this._query.withLimitToFirst(n), this.firestore);
+  }
+
+  limitToLast(n: number): firestore.Query {
+    validateExactNumberOfArgs('Query.limitToLast', arguments, 1);
+    validateArgType('Query.limitToLast', 'number', 1, n);
+    validatePositiveNumber('Query.limitToLast', 1, n);
+    return new Query(this._query.withLimitToLast(n), this.firestore);
   }
 
   startAt(
@@ -1829,6 +1831,7 @@ export class Query implements firestore.Query {
         complete: args[currArg + 2] as CompleteFn
       };
     }
+    this.validateHasExplicitOrderByForLimitToLast(this._query);
     return this.onSnapshotInternal(options, observer);
   }
 
@@ -1864,9 +1867,19 @@ export class Query implements firestore.Query {
     };
   }
 
+  private validateHasExplicitOrderByForLimitToLast(query: InternalQuery): void {
+    if (query.hasLimitToLast() && query.explicitOrderBy.length === 0) {
+      throw new FirestoreError(
+        Code.UNIMPLEMENTED,
+        'limitToLast() queries require specifying at least one orderBy() clause'
+      );
+    }
+  }
+
   get(options?: firestore.GetOptions): Promise<firestore.QuerySnapshot> {
     validateBetweenNumberOfArgs('Query.get', arguments, 0, 1);
     validateGetOptions('Query.get', options);
+    this.validateHasExplicitOrderByForLimitToLast(this._query);
     return new Promise(
       (resolve: Resolver<firestore.QuerySnapshot>, reject: Rejecter) => {
         if (options && options.source === 'cache') {

packages/firestore/src/core/query.ts

@@ -30,6 +30,11 @@ import { Code, FirestoreError } from '../util/error';
 import { isNullOrUndefined } from '../util/types';
 import { Target } from './target';
 
+export enum LimitType {
+  First = 'F',
+  Last = 'L'
+}
+
 /**
  * Query encapsulates all the query attributes we support in the SDK. It can
  * be run against the LocalStore, as well as be converted to a `Target` to
@@ -55,6 +60,7 @@ export class Query {
     readonly explicitOrderBy: OrderBy[] = [],
     readonly filters: Filter[] = [],
     readonly limit: number | null = null,
+    readonly limitType: LimitType = LimitType.First,
     readonly startAt: Bound | null = null,
     readonly endAt: Bound | null = null
   ) {
@@ -133,6 +139,7 @@ export class Query {
       this.explicitOrderBy.slice(),
       newFilters,
       this.limit,
+      this.limitType,
       this.startAt,
       this.endAt
     );
@@ -148,18 +155,33 @@ export class Query {
       newOrderBy,
       this.filters.slice(),
       this.limit,
+      this.limitType,
+      this.startAt,
+      this.endAt
+    );
+  }
+
+  withLimitToFirst(limit: number | null): Query {
+    return new Query(
+      this.path,
+      this.collectionGroup,
+      this.explicitOrderBy.slice(),
+      this.filters.slice(),
+      limit,
+      LimitType.First,
       this.startAt,
       this.endAt
     );
   }
 
-  withLimit(limit: number | null): Query {
+  withLimitToLast(limit: number | null): Query {
     return new Query(
       this.path,
       this.collectionGroup,
       this.explicitOrderBy.slice(),
       this.filters.slice(),
       limit,
+      LimitType.Last,
       this.startAt,
       this.endAt
     );
@@ -172,6 +194,7 @@ export class Query {
       this.explicitOrderBy.slice(),
       this.filters.slice(),
       this.limit,
+      this.limitType,
       bound,
       this.endAt
     );
@@ -184,6 +207,7 @@ export class Query {
       this.explicitOrderBy.slice(),
       this.filters.slice(),
       this.limit,
+      this.limitType,
       this.startAt,
       bound
     );
@@ -202,6 +226,7 @@ export class Query {
       this.explicitOrderBy.slice(),
       this.filters.slice(),
       this.limit,
+      this.limitType,
       this.startAt,
       this.endAt
     );
@@ -227,15 +252,20 @@ export class Query {
   // example, use as a dictionary key, but the implementation is subject to
   // collisions. Make it collision-free.
   canonicalId(): string {
-    return this.toTarget().canonicalId();
+    return `${this.toTarget().canonicalId()}|lt:${this.limitType}`;
   }
 
   toString(): string {
-    return `Query(target=${this.toTarget().toString()})`;
+    return `Query(target=${this.toTarget().toString()}; limitType=${
+      this.limitType
+    })`;
   }
 
   isEqual(other: Query): boolean {
-    return this.toTarget().isEqual(other.toTarget());
+    return (
+      this.toTarget().isEqual(other.toTarget()) &&
+      this.limitType === other.limitType
+    );
   }
 
   docComparator(d1: Document, d2: Document): number {
@@ -264,8 +294,12 @@ export class Query {
     );
   }
 
-  hasLimit(): boolean {
-    return !isNullOrUndefined(this.limit);
+  hasLimitToFirst(): boolean {
+    return !isNullOrUndefined(this.limit) && this.limitType === LimitType.First;
+  }
+
+  hasLimitToLast(): boolean {
+    return !isNullOrUndefined(this.limit) && this.limitType === LimitType.Last;
   }
 
   getFirstOrderByField(): FieldPath | null {
@@ -304,17 +338,52 @@ export class Query {
     return this.collectionGroup !== null;
   }
 
+  /**
+   * Converts this `Query` instance to it's corresponding `Target`
+   * representation.
+   */
   toTarget(): Target {
     if (!this.target) {
-      this.target = new Target(
-        this.path,
-        this.collectionGroup,
-        this.orderBy,
-        this.filters,
-        this.limit,
-        this.startAt,
-        this.endAt
-      );
+      if (this.limitType === LimitType.First) {
+        this.target = new Target(
+          this.path,
+          this.collectionGroup,
+          this.orderBy,
+          this.filters,
+          this.limit,
+          this.startAt,
+          this.endAt
+        );
+      } else {
+        // Flip the orderBy directions since we want the last results
+        const orderBys = [] as OrderBy[];
+        for (const orderBy of this.orderBy) {
+          const dir =
+            orderBy.dir === Direction.DESCENDING
+              ? Direction.ASCENDING
+              : Direction.DESCENDING;
+          orderBys.push(new OrderBy(orderBy.field, dir));
+        }
+
+        // We need to swap the cursors to match the now-flipped query ordering.
+        const startAt = this.endAt
+          ? new Bound(this.endAt.position, !this.endAt.before)
+          : null;
+        const endAt = this.startAt
+          ? new Bound(this.startAt.position, !this.startAt.before)
+          : null;
+
+        // Now return as a LimitType.First query.
+        this.target = new Target(
+          this.path,
+          this.collectionGroup,
+          orderBys,
+          this.filters,
+          this.limit,
+          startAt,
+          endAt
+        );
+      }
     }
     return this.target!;
   }

packages/firestore/src/core/sync_engine.ts

@@ -49,8 +49,9 @@ import {
 import * as objUtils from '../util/obj';
 import { SortedSet } from '../util/sorted_set';
 import { ListenSequence } from './listen_sequence';
-import { Query } from './query';
+import { Query, LimitType } from './query';
 import { SnapshotVersion } from './snapshot_version';
+import { Target } from './target';
 import { TargetIdGenerator } from './target_id_generator';
 import { Transaction } from './transaction';
 import {
@@ -216,7 +217,8 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
       this.sharedClientState.addLocalQueryTarget(targetId);
       viewSnapshot = queryView.view.computeInitialSnapshot();
     } else {
-      const queryData = await this.localStore.allocateQuery(query);
+      const queryData = await this.localStore.allocateTarget(query.toTarget());
+
       const status = this.sharedClientState.addLocalQueryTarget(
         queryData.targetId
       );
@@ -305,8 +307,18 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
     const queryView = this.queryViewsByQuery.get(query)!;
     assert(!!queryView, 'Trying to unlisten on query not found:' + query);
 
-    // TODO(wuandy): Note this does not handle the case where multiple queries
-    // map to one target, and user request to unlisten on of the queries.
+    // If it's not the only query mapped to the target, only clean up
+    // query view and keep the target active.
+    const queries = this.queriesByTarget[queryView.targetId];
+    if (queries.length > 1) {
+      this.queriesByTarget[queryView.targetId] = queries.filter(
+        q => !q.isEqual(query)
+      );
+      this.queryViewsByQuery.delete(query);
+      return;
+    }
+
+    // No other queries are mapped to the target, clean up the query and the target.
     if (this.isPrimary) {
       // We need to remove the local query target first to allow us to verify
       // whether any other client is still interested in this target.
@@ -983,7 +995,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
         assert(!!target, `Target for id ${targetId} not found`);
         queryData = await this.localStore.allocateTarget(target!);
         await this.initializeViewAndComputeSnapshot(
-          target!.toTargetQuery(),
+          this.synthesizeTargetToQuery(target!),
           targetId,
           /*current=*/ false
         );
@@ -996,6 +1008,30 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
     return activeQueries;
   }
 
+  /**
+   * Creates a `Query` object from the specified `Target`. There is no way to
+   * obtain the original `Query`, so we synthesize a `Query` from the `Target`
+   * object.
+   *
+   * The synthesized result might be different from the original `Query`, but
+   * since the synthesized `Query` should return the same results as the
+   * original one (only the presentation of results might differ), and this is
+   * only used for multi-tab, the potential difference will not cause issues.
+   */
+  // PORTING NOTE: Multi-tab only
+  private synthesizeTargetToQuery(target: Target): Query {
+    return new Query(
+      target.path,
+      target.collectionGroup,
+      target.orderBy,
+      target.filters,
+      target.limit,
+      LimitType.First,
+      target.startAt,
+      target.endAt
+    );
+  }
+
   // PORTING NOTE: Multi-tab only
   getActiveClients(): Promise<ClientId[]> {
     return this.localStore.getActiveClients();
@@ -1061,7 +1097,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
       assert(!!target, `Query data for active target ${targetId} not found`);
       const queryData = await this.localStore.allocateTarget(target!);
       await this.initializeViewAndComputeSnapshot(
-        target!.toTargetQuery(),
+        this.synthesizeTargetToQuery(target!),
         queryData.targetId,
         /*current=*/ false
       );