Remove "cyclic" references (#2969)

Author: schmidt-sebastian

packages/firestore/src/api/database.ts

@@ -69,7 +69,7 @@ import {
   validateStringEnum,
   valueDescription
 } from '../util/input_validation';
-import { logError, setLogLevel, LogLevel, getLogLevel } from '../util/log';
+import { getLogLevel, logError, LogLevel, setLogLevel } from '../util/log';
 import { AutoId } from '../util/misc';
 import { Deferred, Rejecter, Resolver } from '../util/promise';
 import { FieldPath as ExternalFieldPath } from './field_path';
@@ -836,7 +836,7 @@ export class WriteBatch implements firestore.WriteBatch {
             convertedValue
           );
     this._mutations = this._mutations.concat(
-      parsed.toMutations(ref._key, Precondition.NONE)
+      parsed.toMutations(ref._key, Precondition.none())
     );
     return this;
   }
@@ -906,7 +906,7 @@ export class WriteBatch implements firestore.WriteBatch {
       this._firestore
     );
     this._mutations = this._mutations.concat(
-      new DeleteMutation(ref._key, Precondition.NONE)
+      new DeleteMutation(ref._key, Precondition.none())
     );
     return this;
   }
@@ -1031,7 +1031,7 @@ export class DocumentReference<T = firestore.DocumentData>
           )
         : this.firestore._dataReader.parseSetData(functionName, convertedValue);
     return this._firestoreClient.write(
-      parsed.toMutations(this._key, Precondition.NONE)
+      parsed.toMutations(this._key, Precondition.none())
     );
   }
 
@@ -1075,7 +1075,7 @@ export class DocumentReference<T = firestore.DocumentData>
   delete(): Promise<void> {
     validateExactNumberOfArgs('DocumentReference.delete', arguments, 0);
     return this._firestoreClient.write([
-      new DeleteMutation(this._key, Precondition.NONE)
+      new DeleteMutation(this._key, Precondition.none())
     ]);
   }
 
@@ -1447,32 +1447,31 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
 
     // Enumerated from the WhereFilterOp type in index.d.ts.
     const whereFilterOpEnums = [
-      '<',
-      '<=',
-      '==',
-      '>=',
-      '>',
-      'array-contains',
-      'in',
-      'array-contains-any'
+      Operator.LESS_THAN,
+      Operator.LESS_THAN_OR_EQUAL,
+      Operator.EQUAL,
+      Operator.GREATER_THAN_OR_EQUAL,
+      Operator.GREATER_THAN,
+      Operator.ARRAY_CONTAINS,
+      Operator.IN,
+      Operator.ARRAY_CONTAINS_ANY
     ];
-    validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);
+    const op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);
 
     let fieldValue: api.Value;
     const fieldPath = fieldPathFromArgument('Query.where', field);
-    const operator = Operator.fromString(opStr);
     if (fieldPath.isKeyField()) {
       if (
-        operator === Operator.ARRAY_CONTAINS ||
-        operator === Operator.ARRAY_CONTAINS_ANY
+        op === Operator.ARRAY_CONTAINS ||
+        op === Operator.ARRAY_CONTAINS_ANY
       ) {
         throw new FirestoreError(
           Code.INVALID_ARGUMENT,
-          `Invalid Query. You can't perform '${operator.toString()}' ` +
+          `Invalid Query. You can't perform '${op}' ` +
             'queries on FieldPath.documentId().'
         );
-      } else if (operator === Operator.IN) {
-        this.validateDisjunctiveFilterElements(value, operator);
+      } else if (op === Operator.IN) {
+        this.validateDisjunctiveFilterElements(value, op);
         const referenceList: api.Value[] = [];
         for (const arrayValue of value as api.Value[]) {
           referenceList.push(this.parseDocumentIdValue(arrayValue));
@@ -1482,20 +1481,17 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
         fieldValue = this.parseDocumentIdValue(value);
       }
     } else {
-      if (
-        operator === Operator.IN ||
-        operator === Operator.ARRAY_CONTAINS_ANY
-      ) {
-        this.validateDisjunctiveFilterElements(value, operator);
+      if (op === Operator.IN || op === Operator.ARRAY_CONTAINS_ANY) {
+        this.validateDisjunctiveFilterElements(value, op);
       }
       fieldValue = this.firestore._dataReader.parseQueryValue(
         'Query.where',
         value,
         // We only allow nested arrays for IN queries.
-        /** allowArrays = */ operator === Operator.IN ? true : false
+        /** allowArrays = */ op === Operator.IN
       );
     }
-    const filter = FieldFilter.create(fieldPath, operator, fieldValue);
+    const filter = FieldFilter.create(fieldPath, op, fieldValue);
     this.validateNewFilter(filter);
     return new Query(
       this._query.addFilter(filter),

packages/firestore/src/core/query.ts

@@ -80,28 +80,27 @@ export class Query {
 
   get orderBy(): OrderBy[] {
     if (this.memoizedOrderBy === null) {
+      this.memoizedOrderBy = [];
+
       const inequalityField = this.getInequalityFilterField();
       const firstOrderByField = this.getFirstOrderByField();
       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.
         // Note that the default inequality field and key ordering is ascending.
-        if (inequalityField.isKeyField()) {
-          this.memoizedOrderBy = [KEY_ORDERING_ASC];
-        } else {
-          this.memoizedOrderBy = [
-            new OrderBy(inequalityField),
-            KEY_ORDERING_ASC
-          ];
+        if (!inequalityField.isKeyField()) {
+          this.memoizedOrderBy.push(new OrderBy(inequalityField));
         }
+        this.memoizedOrderBy.push(
+          new OrderBy(FieldPath.keyField(), Direction.ASCENDING)
+        );
       } else {
         debugAssert(
           inequalityField === null ||
             (firstOrderByField !== null &&
               inequalityField.isEqual(firstOrderByField)),
           'First orderBy should match inequality field.'
         );
-        this.memoizedOrderBy = [];
         let foundKeyOrdering = false;
         for (const orderBy of this.explicitOrderBy) {
           this.memoizedOrderBy.push(orderBy);
@@ -117,9 +116,7 @@ export class Query {
               ? this.explicitOrderBy[this.explicitOrderBy.length - 1].dir
               : Direction.ASCENDING;
           this.memoizedOrderBy.push(
-            lastDirection === Direction.ASCENDING
-              ? KEY_ORDERING_ASC
-              : KEY_ORDERING_DESC
+            new OrderBy(FieldPath.keyField(), lastDirection)
           );
         }
       }
@@ -468,48 +465,15 @@ export abstract class Filter {
   abstract isEqual(filter: Filter): boolean;
 }
 
-export class Operator {
-  static LESS_THAN = new Operator('<');
-  static LESS_THAN_OR_EQUAL = new Operator('<=');
-  static EQUAL = new Operator('==');
-  static GREATER_THAN = new Operator('>');
-  static GREATER_THAN_OR_EQUAL = new Operator('>=');
-  static ARRAY_CONTAINS = new Operator('array-contains');
-  static IN = new Operator('in');
-  static ARRAY_CONTAINS_ANY = new Operator('array-contains-any');
-
-  static fromString(op: string): Operator {
-    switch (op) {
-      case '<':
-        return Operator.LESS_THAN;
-      case '<=':
-        return Operator.LESS_THAN_OR_EQUAL;
-      case '==':
-        return Operator.EQUAL;
-      case '>=':
-        return Operator.GREATER_THAN_OR_EQUAL;
-      case '>':
-        return Operator.GREATER_THAN;
-      case 'array-contains':
-        return Operator.ARRAY_CONTAINS;
-      case 'in':
-        return Operator.IN;
-      case 'array-contains-any':
-        return Operator.ARRAY_CONTAINS_ANY;
-      default:
-        return fail('Unknown FieldFilter operator: ' + op);
-    }
-  }
-
-  constructor(public name: string) {}
-
-  toString(): string {
-    return this.name;
-  }
-
-  isEqual(other: Operator): boolean {
-    return this.name === other.name;
-  }
+export const enum Operator {
+  LESS_THAN = '<',
+  LESS_THAN_OR_EQUAL = '<=',
+  EQUAL = '==',
+  GREATER_THAN = '>',
+  GREATER_THAN_OR_EQUAL = '>=',
+  ARRAY_CONTAINS = 'array-contains',
+  IN = 'in',
+  ARRAY_CONTAINS_ANY = 'array-contains-any'
 }
 
 export class FieldFilter extends Filter {
@@ -635,7 +599,7 @@ export class FieldFilter extends Filter {
   isEqual(other: Filter): boolean {
     if (other instanceof FieldFilter) {
       return (
-        this.op.isEqual(other.op) &&
+        this.op === other.op &&
         this.field.isEqual(other.field) &&
         valueEquals(this.value, other.value)
       );
@@ -737,15 +701,9 @@ export class ArrayContainsAnyFilter extends FieldFilter {
 /**
  * The direction of sorting in an order by.
  */
-export class Direction {
-  static ASCENDING = new Direction('asc');
-  static DESCENDING = new Direction('desc');
-
-  private constructor(public name: string) {}
-
-  toString(): string {
-    return this.name;
-  }
+export const enum Direction {
+  ASCENDING = 'asc',
+  DESCENDING = 'desc'
 }
 
 /**
@@ -875,9 +833,3 @@ export class OrderBy {
     return this.dir === other.dir && this.field.isEqual(other.field);
   }
 }
-
-const KEY_ORDERING_ASC = new OrderBy(FieldPath.keyField(), Direction.ASCENDING);
-const KEY_ORDERING_DESC = new OrderBy(
-  FieldPath.keyField(),
-  Direction.DESCENDING
-);

packages/firestore/src/core/snapshot_version.ts

@@ -22,14 +22,12 @@ import { Timestamp } from '../api/timestamp';
  * timestamp, such as update_time or read_time.
  */
 export class SnapshotVersion {
-  static readonly MIN = new SnapshotVersion(new Timestamp(0, 0));
-
   static fromTimestamp(value: Timestamp): SnapshotVersion {
     return new SnapshotVersion(value);
   }
 
-  static forDeletedDoc(): SnapshotVersion {
-    return SnapshotVersion.MIN;
+  static min(): SnapshotVersion {
+    return new SnapshotVersion(new Timestamp(0, 0));
   }
 
   private constructor(private timestamp: Timestamp) {}

packages/firestore/src/core/sync_engine.ts

@@ -33,7 +33,7 @@ import {
 import { MaybeDocument, NoDocument } from '../model/document';
 import { DocumentKey } from '../model/document_key';
 import { Mutation } from '../model/mutation';
-import { MutationBatchResult, BATCHID_UNKNOWN } from '../model/mutation_batch';
+import { BATCHID_UNKNOWN, MutationBatchResult } from '../model/mutation_batch';
 import { RemoteEvent, TargetChange } from '../remote/remote_event';
 import { RemoteStore } from '../remote/remote_store';
 import { RemoteSyncer } from '../remote/remote_syncer';
@@ -52,7 +52,7 @@ import {
 } from '../local/shared_client_state_syncer';
 import { SortedSet } from '../util/sorted_set';
 import { ListenSequence } from './listen_sequence';
-import { Query, LimitType } from './query';
+import { LimitType, Query } from './query';
 import { SnapshotVersion } from './snapshot_version';
 import { Target } from './target';
 import { TargetIdGenerator } from './target_id_generator';
@@ -504,11 +504,11 @@ export class SyncEngine implements RemoteSyncer {
       );
       documentUpdates = documentUpdates.insert(
         limboKey,
-        new NoDocument(limboKey, SnapshotVersion.forDeletedDoc())
+        new NoDocument(limboKey, SnapshotVersion.min())
       );
       const resolvedLimboDocuments = documentKeySet().add(limboKey);
       const event = new RemoteEvent(
-        SnapshotVersion.MIN,
+        SnapshotVersion.min(),
         /* targetChanges= */ new Map<TargetId, TargetChange>(),
         /* targetMismatches= */ new SortedSet<TargetId>(primitiveComparator),
         documentUpdates,

packages/firestore/src/core/transaction.ts

@@ -17,7 +17,7 @@
 
 import { ParsedSetData, ParsedUpdateData } from '../api/user_data_reader';
 import { documentVersionMap } from '../model/collections';
-import { Document, NoDocument, MaybeDocument } from '../model/document';
+import { Document, MaybeDocument, NoDocument } from '../model/document';
 
 import { DocumentKey } from '../model/document_key';
 import {
@@ -27,7 +27,7 @@ import {
   VerifyMutation
 } from '../model/mutation';
 import { Datastore } from '../remote/datastore';
-import { fail, debugAssert } from '../util/assert';
+import { debugAssert, fail } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
 import { SnapshotVersion } from './snapshot_version';
 
@@ -123,7 +123,7 @@ export class Transaction {
       docVersion = doc.version;
     } else if (doc instanceof NoDocument) {
       // For deleted docs, we must use baseVersion 0 when we overwrite them.
-      docVersion = SnapshotVersion.forDeletedDoc();
+      docVersion = SnapshotVersion.min();
     } else {
       throw fail('Document in a transaction was a ' + doc.constructor.name);
     }
@@ -151,7 +151,7 @@ export class Transaction {
     if (!this.writtenDocs.has(key) && version) {
       return Precondition.updateTime(version);
     } else {
-      return Precondition.NONE;
+      return Precondition.none();
     }
   }
 
@@ -163,7 +163,7 @@ export class Transaction {
     // The first time a document is written, we want to take into account the
     // read time and existence
     if (!this.writtenDocs.has(key) && version) {
-      if (version.isEqual(SnapshotVersion.forDeletedDoc())) {
+      if (version.isEqual(SnapshotVersion.min())) {
         // The document doesn't exist, so fail the transaction.
 
         // This has to be validated locally because you can't send a

packages/firestore/src/local/index_free_query_engine.ts

@@ -78,7 +78,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
 
     // Queries that have never seen a snapshot without limbo free documents
     // should also be run as a full collection scan.
-    if (lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.MIN)) {
+    if (lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.min())) {
       return this.executeFullCollectionScan(transaction, query);
     }
 
@@ -204,7 +204,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
     return this.localDocumentsView!.getDocumentsMatchingQuery(
       transaction,
       query,
-      SnapshotVersion.MIN
+      SnapshotVersion.min()
     );
   }
 }