Remove TargetIdGenerator (#2818)

Author: schmidt-sebastian

packages/firestore/src/core/target_id_generator.ts

@@ -15,83 +15,43 @@
  * limitations under the License.
  */
 
-import { assert } from '../util/assert';
 import { TargetId } from './types';
 
-const RESERVED_BITS = 1;
-
-const enum GeneratorIds {
-  QueryCache = 0, // The target IDs for user-issued queries are even (end in 0).
-  SyncEngine = 1 // The target IDs for limbo detection are odd (end in 1).
-}
+/** Offset to ensure non-overlapping target ids. */
+const OFFSET = 2;
 
 /**
  * Generates monotonically increasing target IDs for sending targets to the
  * watch stream.
  *
- * The client constructs two generators, one for the query cache (via
- * forQueryCache()), and one for limbo documents (via forSyncEngine()). These
- * two generators produce non-overlapping IDs (by using even and odd IDs
+ * The client constructs two generators, one for the target cache, and one for
+ * for the sync engine (to generate limbo documents targets). These
+ * generators produce non-overlapping IDs (by using even and odd IDs
  * respectively).
  *
  * By separating the target ID space, the query cache can generate target IDs
  * that persist across client restarts, while sync engine can independently
  * generate in-memory target IDs that are transient and can be reused after a
  * restart.
  */
-// TODO(mrschmidt): Explore removing this class in favor of generating these IDs
-// directly in SyncEngine and LocalStore.
 export class TargetIdGenerator {
-  // Initialized in the constructor via call to seek().
-  private nextId!: TargetId;
-
-  /**
-   * Instantiates a new TargetIdGenerator. If a seed is provided, the generator
-   * will use the seed value as the next target ID.
-   */
-  constructor(private generatorId: number, seed?: number) {
-    assert(
-      (generatorId & RESERVED_BITS) === generatorId,
-      `Generator ID ${generatorId} contains more than ${RESERVED_BITS} reserved bits`
-    );
-    this.seek(seed !== undefined ? seed : this.generatorId);
-  }
+  constructor(private lastId: number) {}
 
   next(): TargetId {
-    const nextId = this.nextId;
-    this.nextId += 1 << RESERVED_BITS;
-    return nextId;
-  }
-
-  /**
-   * Returns the ID that follows the given ID. Subsequent calls to `next()`
-   * use the newly returned target ID as their base.
-   */
-  // PORTING NOTE: Multi-tab only.
-  after(targetId: TargetId): TargetId {
-    this.seek(targetId + (1 << RESERVED_BITS));
-    return this.next();
-  }
-
-  private seek(targetId: TargetId): void {
-    assert(
-      (targetId & RESERVED_BITS) === this.generatorId,
-      'Cannot supply target ID from different generator ID'
-    );
-    this.nextId = targetId;
+    this.lastId += OFFSET;
+    return this.lastId;
   }
 
   static forTargetCache(): TargetIdGenerator {
-    // We seed the query cache generator to return '2' as its first ID, as there
-    // is no differentiation in the protocol layer between an unset number and
-    // the number '0'. If we were to sent a target with target ID '0', the
-    // backend would consider it unset and replace it with its own ID.
-    const targetIdGenerator = new TargetIdGenerator(GeneratorIds.QueryCache, 2);
-    return targetIdGenerator;
+    // The target cache generator must return '2' in its first call to `next()`
+    // as there is no differentiation in the protocol layer between an unset
+    // number and the number '0'. If we were to sent a target with target ID
+    // '0', the backend would consider it unset and replace it with its own ID.
+    return new TargetIdGenerator(2 - OFFSET);
   }
 
   static forSyncEngine(): TargetIdGenerator {
     // Sync engine assigns target IDs for limbo document detection.
-    return new TargetIdGenerator(GeneratorIds.SyncEngine);
+    return new TargetIdGenerator(1 - OFFSET);
   }
 }

packages/firestore/src/local/indexeddb_target_cache.ts

@@ -62,15 +62,12 @@ export class IndexedDbTargetCache implements TargetCache {
   // to IndexedDb whenever we need to read metadata. We can revisit if it turns
   // out to have a meaningful performance impact.
 
-  private targetIdGenerator = TargetIdGenerator.forTargetCache();
-
   allocateTargetId(
     transaction: PersistenceTransaction
   ): PersistencePromise<TargetId> {
     return this.retrieveMetadata(transaction).next(metadata => {
-      metadata.highestTargetId = this.targetIdGenerator.after(
-        metadata.highestTargetId
-      );
+      const targetIdGenerator = new TargetIdGenerator(metadata.highestTargetId);
+      metadata.highestTargetId = targetIdGenerator.next();
       return this.saveMetadata(transaction, metadata).next(
         () => metadata.highestTargetId
       );

packages/firestore/src/local/local_serializer.ts

@@ -170,7 +170,7 @@ export class LocalSerializer {
       mutations
     );
   }
-  
+
   /** Decodes a DbTarget into TargetData */
   fromDbTarget(dbTarget: DbTarget): TargetData {
     const version = this.fromDbTimestamp(dbTarget.readTime);

packages/firestore/src/local/memory_target_cache.ts

@@ -1,6 +1,6 @@
 /**
  * @license
- * Copyright 2017 Google Inc.
+ * Copyright 2017 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -79,9 +79,8 @@ export class MemoryTargetCache implements TargetCache {
   allocateTargetId(
     transaction: PersistenceTransaction
   ): PersistencePromise<TargetId> {
-    const nextTargetId = this.targetIdGenerator.after(this.highestTargetId);
-    this.highestTargetId = nextTargetId;
-    return PersistencePromise.resolve(nextTargetId);
+    this.highestTargetId = this.targetIdGenerator.next();
+    return PersistencePromise.resolve(this.highestTargetId);
   }
 
   setTargetsMetadata(
@@ -102,6 +101,7 @@ export class MemoryTargetCache implements TargetCache {
     this.targets.set(targetData.target, targetData);
     const targetId = targetData.targetId;
     if (targetId > this.highestTargetId) {
+      this.targetIdGenerator = new TargetIdGenerator(targetId);
       this.highestTargetId = targetId;
     }
     if (targetData.sequenceNumber > this.highestSequenceNumber) {