Merge fef4ed0 into a722abc

Author: MarkDuckworth

packages/firestore/src/core/target.ts

@@ -732,7 +732,7 @@ export class CompositeFilter extends Filter {
   }
 
   matches(doc: Document): boolean {
-    if (this.isConjunction()) {
+    if (compositeFilterIsConjunction(this)) {
       // For conjunctions, all filters must match, so return false if any filter doesn't match.
       return this.filters.find(filter => !filter.matches(doc)) === undefined;
     } else {
@@ -780,10 +780,38 @@ export class CompositeFilter extends Filter {
 
     return null;
   }
+}
+
+export function compositeFilterIsConjunction(
+  compositeFilter: CompositeFilter
+): boolean {
+  return compositeFilter.op === CompositeOperator.AND;
+}
+
+/**
+ * Returns true if this filter is a conjunction of field filters only. Returns false otherwise.
+ */
+export function compositeFilterIsFlatConjunction(
+  compositeFilter: CompositeFilter
+): boolean {
+  return (
+    compositeFilterIsFlat(compositeFilter) &&
+    compositeFilterIsConjunction(compositeFilter)
+  );
+}
 
-  isConjunction(): boolean {
-    return this.op === CompositeOperator.AND;
+/**
+ * Returns true if this filter does not contain any composite filters. Returns false otherwise.
+ */
+export function compositeFilterIsFlat(
+  compositeFilter: CompositeFilter
+): boolean {
+  for (const filter of compositeFilter.filters) {
+    if (filter instanceof CompositeFilter) {
+      return false;
+    }
   }
+  return true;
 }
 
 export function canonifyFilter(filter: Filter): string {
@@ -811,18 +839,45 @@ export function canonifyFilter(filter: Filter): string {
 }
 
 export function filterEquals(f1: Filter, f2: Filter): boolean {
-  debugAssert(
-    f1 instanceof FieldFilter && f2 instanceof FieldFilter,
-    'Only FieldFilters can be compared'
-  );
+  if (f1 instanceof FieldFilter) {
+    return fieldFilterEquals(f1, f2);
+  } else if (f1 instanceof CompositeFilter) {
+    return compositeFilterEquals(f1, f2);
+  } else {
+    fail('Only FieldFilters and CompositeFilters can be compared');
+  }
+}
 
+export function fieldFilterEquals(f1: FieldFilter, f2: Filter): boolean {
   return (
+    f2 instanceof FieldFilter &&
     f1.op === f2.op &&
     f1.field.isEqual(f2.field) &&
     valueEquals(f1.value, f2.value)
   );
 }
 
+export function compositeFilterEquals(
+  f1: CompositeFilter,
+  f2: Filter
+): boolean {
+  if (
+    f2 instanceof CompositeFilter &&
+    f1.op === f2.op &&
+    f1.filters.length === f2.filters.length
+  ) {
+    const subFiltersMatch: boolean = f1.filters.reduce(
+      (result: boolean, f1Filter: Filter, index: number): boolean =>
+        result && filterEquals(f1Filter, f2.filters[index]),
+      true
+    );
+
+    return subFiltersMatch;
+  }
+
+  return false;
+}
+
 /** Returns a debug description for `filter`. */
 export function stringifyFilter(filter: Filter): string {
   debugAssert(

packages/firestore/src/remote/serializer.ts

@@ -31,8 +31,11 @@ import {
   Filter,
   targetIsDocumentTarget,
   Operator,
+  CompositeOperator,
   OrderBy,
-  Target
+  Target,
+  CompositeFilter,
+  compositeFilterIsFlatConjunction
 } from '../core/target';
 import { TargetId } from '../core/types';
 import { Timestamp } from '../lite-api/timestamp';
@@ -64,6 +67,7 @@ import { isNanValue, isNullValue } from '../model/values';
 import {
   ApiClientObjectMap as ProtoApiClientObjectMap,
   BatchGetDocumentsResponse as ProtoBatchGetDocumentsResponse,
+  CompositeFilterOp as ProtoCompositeFilterOp,
   Cursor as ProtoCursor,
   Document as ProtoDocument,
   DocumentMask as ProtoDocumentMask,
@@ -122,6 +126,14 @@ const OPERATORS = (() => {
   return ops;
 })();
 
+const COMPOSITE_OPERATORS = (() => {
+  const ops: { [op: string]: ProtoCompositeFilterOp } = {};
+  ops[CompositeOperator.AND] = 'AND';
+  // TODO(orquery) change 'OPERATOR_UNSPECIFIED' to 'OR' when the updated protos are published
+  ops[CompositeOperator.OR] = 'OPERATOR_UNSPECIFIED';
+  return ops;
+})();
+
 function assertPresent(value: unknown, description: string): asserts value {
   debugAssert(!isNullOrUndefined(value), description + ' is missing');
 }
@@ -827,7 +839,7 @@ export function toQueryTarget(
     result.structuredQuery!.from = [{ collectionId: path.lastSegment() }];
   }
 
-  const where = toFilter(target.filters);
+  const where = toFilters(target.filters);
   if (where) {
     result.structuredQuery!.where = where;
   }
@@ -873,7 +885,7 @@ export function convertQueryTargetToQuery(target: ProtoQueryTarget): Query {
 
   let filterBy: Filter[] = [];
   if (query.where) {
-    filterBy = fromFilter(query.where);
+    filterBy = fromFilters(query.where);
   }
 
   let orderBy: OrderBy[] = [];
@@ -972,34 +984,39 @@ export function toTarget(
   return result;
 }
 
-function toFilter(filters: Filter[]): ProtoFilter | undefined {
+function toFilters(filters: Filter[]): ProtoFilter | undefined {
   if (filters.length === 0) {
     return;
   }
-  const protos = filters.map(filter => {
-    debugAssert(
-      filter instanceof FieldFilter,
-      'Only FieldFilters are supported'
-    );
-    return toUnaryOrFieldFilter(filter);
-  });
-  if (protos.length === 1) {
-    return protos[0];
+
+  return toFilter(CompositeFilter.create(filters, CompositeOperator.AND));
+}
+
+function fromFilters(filter: ProtoFilter): Filter[] {
+  const result = fromFilter(filter);
+
+  // Instead of a singletonList containing AND(F1, F2, ...), we can return a list containing F1,
+  // F2, ...
+  // TODO(orquery): Once proper support for composite filters has been completed, we can remove
+  // this flattening from here.
+  if (
+    result instanceof CompositeFilter &&
+    compositeFilterIsFlatConjunction(result)
+  ) {
+    // Copy the readonly array into a mutable array
+    return Object.assign([], result.getFilters());
   }
-  return { compositeFilter: { op: 'AND', filters: protos } };
+
+  return [result];
 }
 
-function fromFilter(filter: ProtoFilter | undefined): Filter[] {
-  if (!filter) {
-    return [];
-  } else if (filter.unaryFilter !== undefined) {
-    return [fromUnaryFilter(filter)];
+function fromFilter(filter: ProtoFilter): Filter {
+  if (filter.unaryFilter !== undefined) {
+    return fromUnaryFilter(filter);
   } else if (filter.fieldFilter !== undefined) {
-    return [fromFieldFilter(filter)];
+    return fromFieldFilter(filter);
   } else if (filter.compositeFilter !== undefined) {
-    return filter.compositeFilter
-      .filters!.map(f => fromFilter(f))
-      .reduce((accum, current) => accum.concat(current));
+    return fromCompositeFilter(filter);
   } else {
     return fail('Unknown filter: ' + JSON.stringify(filter));
   }
@@ -1066,6 +1083,12 @@ export function toOperatorName(op: Operator): ProtoFieldFilterOp {
   return OPERATORS[op];
 }
 
+export function toCompositeOperatorName(
+  op: CompositeOperator
+): ProtoCompositeFilterOp {
+  return COMPOSITE_OPERATORS[op];
+}
+
 export function fromOperatorName(op: ProtoFieldFilterOp): Operator {
   switch (op) {
     case 'EQUAL':
@@ -1095,6 +1118,22 @@ export function fromOperatorName(op: ProtoFieldFilterOp): Operator {
   }
 }
 
+export function fromCompositeOperatorName(
+  op: ProtoCompositeFilterOp
+): CompositeOperator {
+  // TODO(orquery) support OR
+  switch (op) {
+    case 'AND':
+      return CompositeOperator.AND;
+    // TODO(orquery) update when OR operator is supported in ProtoCompositeFilterOp
+    //    OPERATOR_UNSPECIFIED should fail and OR should return OR
+    case 'OPERATOR_UNSPECIFIED':
+      return CompositeOperator.OR;
+    default:
+      return fail('Unknown operator');
+  }
+}
+
 export function toFieldPathReference(path: FieldPath): ProtoFieldReference {
   return { fieldPath: path.canonicalString() };
 }
@@ -1120,15 +1159,32 @@ export function fromPropertyOrder(orderBy: ProtoOrder): OrderBy {
   );
 }
 
-export function fromFieldFilter(filter: ProtoFilter): Filter {
-  return FieldFilter.create(
-    fromFieldPathReference(filter.fieldFilter!.field!),
-    fromOperatorName(filter.fieldFilter!.op!),
-    filter.fieldFilter!.value!
-  );
+// visible for testing
+export function toFilter(filter: Filter): ProtoFilter {
+  if (filter instanceof FieldFilter) {
+    return toUnaryOrFieldFilter(filter);
+  } else if (filter instanceof CompositeFilter) {
+    return toCompositeFilter(filter);
+  } else {
+    return fail('Unrecognized filter type ' + JSON.stringify(filter));
+  }
+}
+
+export function toCompositeFilter(filter: CompositeFilter): ProtoFilter {
+  const protos = filter.getFilters().map(filter => toFilter(filter));
+
+  if (protos.length === 1) {
+    return protos[0];
+  }
+
+  return {
+    compositeFilter: {
+      op: toCompositeOperatorName(filter.op),
+      filters: protos
+    }
+  };
 }
 
-// visible for testing
 export function toUnaryOrFieldFilter(filter: FieldFilter): ProtoFilter {
   if (filter.op === Operator.EQUAL) {
     if (isNanValue(filter.value)) {
@@ -1201,6 +1257,21 @@ export function fromUnaryFilter(filter: ProtoFilter): Filter {
   }
 }
 
+export function fromFieldFilter(filter: ProtoFilter): FieldFilter {
+  return FieldFilter.create(
+    fromFieldPathReference(filter.fieldFilter!.field!),
+    fromOperatorName(filter.fieldFilter!.op!),
+    filter.fieldFilter!.value!
+  );
+}
+
+export function fromCompositeFilter(filter: ProtoFilter): CompositeFilter {
+  return CompositeFilter.create(
+    filter.compositeFilter!.filters!.map(filter => fromFilter(filter)),
+    fromCompositeOperatorName(filter.compositeFilter!.op!)
+  );
+}
+
 export function toDocumentMask(fieldMask: FieldMask): ProtoDocumentMask {
   const canonicalFields: string[] = [];
   fieldMask.fields.forEach(field =>