Merge 274c9f7 into 96076f0

Author: schmidt-sebastian

packages/firestore/exp/dependencies.json

@@ -31,6 +31,7 @@ import {
 import {
   Bound,
   Direction,
+  hasLimitToLast,
   LimitType,
   newQueryForCollectionGroup,
   newQueryForPath,
@@ -579,7 +580,7 @@ export function getDocs<T>(
         )
     );
 
-    if (queryImpl._query.hasLimitToLast()) {
+    if (hasLimitToLast(queryImpl._query)) {
       // Limit to last queries reverse the orderBy constraint that was
       // specified by the user. As such, we need to reverse the order of the
       // results to return the documents in the expected order.

packages/firestore/lite/dependencies.json

@@ -58,6 +58,9 @@ import {
   Direction,
   FieldFilter,
   Filter,
+  findFilterOperator,
+  getFirstOrderByField,
+  getInequalityFilterField,
   isCollectionGroupQuery,
   LimitType,
   newQueryComparator,
@@ -72,7 +75,8 @@ import {
   queryWithAddedOrderBy,
   queryWithEndAt,
   queryWithLimit,
-  queryWithStartAt
+  queryWithStartAt,
+  hasLimitToLast
 } from '../core/query';
 import { Transaction as InternalTransaction } from '../core/transaction';
 import { ChangeType, ViewSnapshot } from '../core/view_snapshot';
@@ -1784,7 +1788,7 @@ function validateNewFilter(query: InternalQuery, filter: Filter): void {
   debugAssert(filter instanceof FieldFilter, 'Only FieldFilters are supported');
 
   if (filter.isInequality()) {
-    const existingField = query.getInequalityFilterField();
+    const existingField = getInequalityFilterField(query);
     if (existingField !== null && !existingField.isEqual(filter.field)) {
       throw new FirestoreError(
         Code.INVALID_ARGUMENT,
@@ -1795,13 +1799,13 @@ function validateNewFilter(query: InternalQuery, filter: Filter): void {
       );
     }
 
-    const firstOrderByField = query.getFirstOrderByField();
+    const firstOrderByField = getFirstOrderByField(query);
     if (firstOrderByField !== null) {
       validateOrderByAndInequalityMatch(query, filter.field, firstOrderByField);
     }
   }
 
-  const conflictingOp = query.findFilterOperator(conflictingOps(filter.op));
+  const conflictingOp = findFilterOperator(query, conflictingOps(filter.op));
   if (conflictingOp !== null) {
     // Special case when it's a duplicate op to give a slightly clearer error message.
     if (conflictingOp === filter.op) {
@@ -1821,9 +1825,9 @@ function validateNewFilter(query: InternalQuery, filter: Filter): void {
 }
 
 function validateNewOrderBy(query: InternalQuery, orderBy: OrderBy): void {
-  if (query.getFirstOrderByField() === null) {
+  if (getFirstOrderByField(query) === null) {
     // This is the first order by. It must match any inequality.
-    const inequalityField = query.getInequalityFilterField();
+    const inequalityField = getInequalityFilterField(query);
     if (inequalityField !== null) {
       validateOrderByAndInequalityMatch(query, inequalityField, orderBy.field);
     }
@@ -1850,7 +1854,7 @@ function validateOrderByAndInequalityMatch(
 export function validateHasExplicitOrderByForLimitToLast(
   query: InternalQuery
 ): void {
-  if (query.hasLimitToLast() && query.explicitOrderBy.length === 0) {
+  if (hasLimitToLast(query) && query.explicitOrderBy.length === 0) {
     throw new FirestoreError(
       Code.UNIMPLEMENTED,
       'limitToLast() queries require specifying at least one orderBy() clause'

packages/firestore/lite/src/api/reference.ts

@@ -63,22 +63,6 @@ export interface Query {
   readonly limitType: LimitType;
   readonly startAt: Bound | null;
   readonly endAt: Bound | null;
-
-  hasLimitToFirst(): boolean;
-  hasLimitToLast(): boolean;
-  getFirstOrderByField(): FieldPath | null;
-  getInequalityFilterField(): FieldPath | null;
-  asCollectionQueryAtPath(path: ResourcePath): Query;
-
-  /**
-   * Returns true if this query does not specify any query constraints that
-   * could remove results.
-   */
-  matchesAllDocuments(): boolean;
-
-  // Checks if any of the provided Operators are included in the query and
-  // returns the first one that is, or null if none are.
-  findFilterOperator(operators: Operator[]): Operator | null;
 }
 
 /**
@@ -121,77 +105,6 @@ export class QueryImpl implements Query {
       );
     }
   }
-
-  /**
-   * Helper to convert a collection group query into a collection query at a
-   * specific path. This is used when executing collection group queries, since
-   * we have to split the query into a set of collection queries at multiple
-   * paths.
-   */
-  asCollectionQueryAtPath(path: ResourcePath): Query {
-    return new QueryImpl(
-      path,
-      /*collectionGroup=*/ null,
-      this.explicitOrderBy.slice(),
-      this.filters.slice(),
-      this.limit,
-      this.limitType,
-      this.startAt,
-      this.endAt
-    );
-  }
-
-  matchesAllDocuments(): boolean {
-    return (
-      this.filters.length === 0 &&
-      this.limit === null &&
-      this.startAt == null &&
-      this.endAt == null &&
-      (this.explicitOrderBy.length === 0 ||
-        (this.explicitOrderBy.length === 1 &&
-          this.explicitOrderBy[0].field.isKeyField()))
-    );
-  }
-
-  hasLimitToFirst(): boolean {
-    return !isNullOrUndefined(this.limit) && this.limitType === LimitType.First;
-  }
-
-  hasLimitToLast(): boolean {
-    return !isNullOrUndefined(this.limit) && this.limitType === LimitType.Last;
-  }
-
-  getFirstOrderByField(): FieldPath | null {
-    return this.explicitOrderBy.length > 0
-      ? this.explicitOrderBy[0].field
-      : null;
-  }
-
-  getInequalityFilterField(): FieldPath | null {
-    for (const filter of this.filters) {
-      debugAssert(
-        filter instanceof FieldFilter,
-        'Only FieldFilters are supported'
-      );
-      if (filter.isInequality()) {
-        return filter.field;
-      }
-    }
-    return null;
-  }
-
-  findFilterOperator(operators: Operator[]): Operator | null {
-    for (const filter of this.filters) {
-      debugAssert(
-        filter instanceof FieldFilter,
-        'Only FieldFilters are supported'
-      );
-      if (operators.indexOf(filter.op) >= 0) {
-        return filter.op;
-      }
-    }
-    return null;
-  }
 }
 
 /** Creates a new Query instance with the options provided. */
@@ -222,6 +135,91 @@ export function newQueryForPath(path: ResourcePath): Query {
   return new QueryImpl(path);
 }
 
+/**
+ * Helper to convert a collection group query into a collection query at a
+ * specific path. This is used when executing collection group queries, since
+ * we have to split the query into a set of collection queries at multiple
+ * paths.
+ */
+export function asCollectionQueryAtPath(
+  query: Query,
+  path: ResourcePath
+): Query {
+  return new QueryImpl(
+    path,
+    /*collectionGroup=*/ null,
+    query.explicitOrderBy.slice(),
+    query.filters.slice(),
+    query.limit,
+    query.limitType,
+    query.startAt,
+    query.endAt
+  );
+}
+
+/**
+ * Returns true if this query does not specify any query constraints that
+ * could remove results.
+ */
+export function matchesAllDocuments(query: Query): boolean {
+  return (
+    query.filters.length === 0 &&
+    query.limit === null &&
+    query.startAt == null &&
+    query.endAt == null &&
+    (query.explicitOrderBy.length === 0 ||
+      (query.explicitOrderBy.length === 1 &&
+        query.explicitOrderBy[0].field.isKeyField()))
+  );
+}
+
+export function hasLimitToFirst(query: Query): boolean {
+  return !isNullOrUndefined(query.limit) && query.limitType === LimitType.First;
+}
+
+export function hasLimitToLast(query: Query): boolean {
+  return !isNullOrUndefined(query.limit) && query.limitType === LimitType.Last;
+}
+
+export function getFirstOrderByField(query: Query): FieldPath | null {
+  return query.explicitOrderBy.length > 0
+    ? query.explicitOrderBy[0].field
+    : null;
+}
+
+export function getInequalityFilterField(query: Query): FieldPath | null {
+  for (const filter of query.filters) {
+    debugAssert(
+      filter instanceof FieldFilter,
+      'Only FieldFilters are supported'
+    );
+    if (filter.isInequality()) {
+      return filter.field;
+    }
+  }
+  return null;
+}
+
+/**
+ * Checks if any of the provided Operators are included in the query and
+ * returns the first one that is, or null if none are.
+ */
+export function findFilterOperator(
+  query: Query,
+  operators: Operator[]
+): Operator | null {
+  for (const filter of query.filters) {
+    debugAssert(
+      filter instanceof FieldFilter,
+      'Only FieldFilters are supported'
+    );
+    if (operators.indexOf(filter.op) >= 0) {
+      return filter.op;
+    }
+  }
+  return null;
+}
+
 /**
  * Creates a new Query for a collection group query that matches all documents
  * within the provided collection group.
@@ -260,8 +258,8 @@ export function queryOrderBy(query: Query): OrderBy[] {
   if (queryImpl.memoizedOrderBy === null) {
     queryImpl.memoizedOrderBy = [];
 
-    const inequalityField = queryImpl.getInequalityFilterField();
-    const firstOrderByField = queryImpl.getFirstOrderByField();
+    const inequalityField = getInequalityFilterField(queryImpl);
+    const firstOrderByField = getFirstOrderByField(queryImpl);
     if (inequalityField !== null && firstOrderByField === null) {
       // In order to implicitly add key ordering, we must also add the
       // inequality filter field for it to be a valid query.
@@ -355,10 +353,10 @@ export function queryToTarget(query: Query): Target {
 
 export function queryWithAddedFilter(query: Query, filter: Filter): Query {
   debugAssert(
-    query.getInequalityFilterField() == null ||
+    getInequalityFilterField(query) == null ||
       !(filter instanceof FieldFilter) ||
       !filter.isInequality() ||
-      filter.field.isEqual(query.getInequalityFilterField()!),
+      filter.field.isEqual(getInequalityFilterField(query)!),
     'Query must only have one inequality field.'
   );
 

packages/firestore/src/api/database.ts

@@ -27,7 +27,13 @@ import { DocumentSet } from '../model/document_set';
 import { TargetChange } from '../remote/remote_event';
 import { debugAssert, fail } from '../util/assert';
 
-import { newQueryComparator, Query, queryMatches } from './query';
+import {
+  hasLimitToFirst,
+  hasLimitToLast,
+  newQueryComparator,
+  Query,
+  queryMatches
+} from './query';
 import { OnlineState } from './types';
 import {
   ChangeType,
@@ -140,11 +146,11 @@ export class View {
     // Note that this should never get used in a refill (when previousChanges is
     // set), because there will only be adds -- no deletes or updates.
     const lastDocInLimit =
-      this.query.hasLimitToFirst() && oldDocumentSet.size === this.query.limit
+      hasLimitToFirst(this.query) && oldDocumentSet.size === this.query.limit
         ? oldDocumentSet.last()
         : null;
     const firstDocInLimit =
-      this.query.hasLimitToLast() && oldDocumentSet.size === this.query.limit
+      hasLimitToLast(this.query) && oldDocumentSet.size === this.query.limit
         ? oldDocumentSet.first()
         : null;
 
@@ -234,9 +240,9 @@ export class View {
     );
 
     // Drop documents out to meet limit/limitToLast requirement.
-    if (this.query.hasLimitToFirst() || this.query.hasLimitToLast()) {
+    if (hasLimitToFirst(this.query) || hasLimitToLast(this.query)) {
       while (newDocumentSet.size > this.query.limit!) {
-        const oldDoc = this.query.hasLimitToFirst()
+        const oldDoc = hasLimitToFirst(this.query)
           ? newDocumentSet.last()
           : newDocumentSet.first();
         newDocumentSet = newDocumentSet.delete(oldDoc!.key);

packages/firestore/src/core/query.ts

@@ -20,7 +20,10 @@ import { LocalDocumentsView } from './local_documents_view';
 import { PersistenceTransaction } from './persistence';
 import { PersistencePromise } from './persistence_promise';
 import {
+  hasLimitToFirst,
+  hasLimitToLast,
   LimitType,
+  matchesAllDocuments,
   newQueryComparator,
   Query,
   queryMatches,
@@ -78,7 +81,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
     // Queries that match all documents don't benefit from using
     // IndexFreeQueries. It is more efficient to scan all documents in a
     // collection, rather than to perform individual lookups.
-    if (query.matchesAllDocuments()) {
+    if (matchesAllDocuments(query)) {
       return this.executeFullCollectionScan(transaction, query);
     }
 
@@ -93,7 +96,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
         const previousResults = this.applyQuery(query, documents);
 
         if (
-          (query.hasLimitToFirst() || query.hasLimitToLast()) &&
+          (hasLimitToFirst(query) || hasLimitToLast(query)) &&
           this.needsRefill(
             query.limitType,
             previousResults,

packages/firestore/src/core/view.ts

@@ -16,6 +16,7 @@
  */
 
 import {
+  asCollectionQueryAtPath,
   isCollectionGroupQuery,
   isDocumentQuery,
   Query,
@@ -211,7 +212,8 @@ export class LocalDocumentsView {
         // Perform a collection query against each parent that contains the
         // collectionId and aggregate the results.
         return PersistencePromise.forEach(parents, (parent: ResourcePath) => {
-          const collectionQuery = query.asCollectionQueryAtPath(
+          const collectionQuery = asCollectionQueryAtPath(
+            query,
             parent.child(collectionId)
           );
           return this.getDocumentsMatchingCollectionQuery(