Remove TargetIdGenerator

Author: schmidt-sebastian

packages/firestore/src/core/sync_engine.ts

@@ -51,7 +51,6 @@ import { ListenSequence } from './listen_sequence';
 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 {
   BatchId,
@@ -161,7 +160,7 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
   };
   /** Stores user callbacks waiting for all pending writes to be acknowledged. */
   private pendingWritesCallbacks = new Map<BatchId, Array<Deferred<void>>>();
-  private limboTargetIdGenerator = TargetIdGenerator.forSyncEngine();
+  private nextLimboTargetId = 1;
 
   // The primary state is set to `true` or `false` immediately after Firestore
   // startup. In the interim, a client should only be considered primary if
@@ -770,7 +769,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
     const key = limboChange.key;
     if (!this.limboTargetsByKey.get(key)) {
       logDebug(LOG_TAG, 'New document in limbo: ' + key);
-      const limboTargetId = this.limboTargetIdGenerator.next();
+
+      // Target IDs in SyncEngine start at 1 and remain odd.
+      const limboTargetId = this.nextLimboTargetId;
+      this.nextLimboTargetId += 2;
+
       const query = Query.atPath(key.path);
       this.limboResolutionsByTarget[limboTargetId] = new LimboResolution(key);
       this.remoteStore.listen(

packages/firestore/src/core/target_id_generator.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.
@@ -23,7 +23,6 @@ import { DocumentKey } from '../model/document_key';
 import { assert } from '../util/assert';
 import { immediateSuccessor } from '../util/misc';
 
-import { TargetIdGenerator } from '../core/target_id_generator';
 import * as EncodedResourcePath from './encoded_resource_path';
 import {
   IndexedDbLruDelegate,
@@ -60,15 +59,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
-      );
+      // Target IDs in persistence start at zero and remain even.
+      metadata.highestTargetId += 2;
       return this.saveMetadata(transaction, metadata).next(
         () => metadata.highestTargetId
       );

packages/firestore/src/local/indexeddb_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.
@@ -16,7 +16,6 @@
  */
 
 import { SnapshotVersion } from '../core/snapshot_version';
-import { TargetIdGenerator } from '../core/target_id_generator';
 import { ListenSequenceNumber, TargetId } from '../core/types';
 import { DocumentKeySet } from '../model/collections';
 import { DocumentKey } from '../model/document_key';
@@ -52,8 +51,6 @@ export class MemoryTargetCache implements TargetCache {
 
   private targetCount = 0;
 
-  private targetIdGenerator = TargetIdGenerator.forTargetCache();
-
   constructor(private readonly persistence: MemoryPersistence) {}
 
   forEachTarget(
@@ -79,9 +76,9 @@ export class MemoryTargetCache implements TargetCache {
   allocateTargetId(
     transaction: PersistenceTransaction
   ): PersistencePromise<TargetId> {
-    const nextTargetId = this.targetIdGenerator.after(this.highestTargetId);
-    this.highestTargetId = nextTargetId;
-    return PersistencePromise.resolve(nextTargetId);
+    // Target IDs in persistence start at zero and remain even.
+    this.highestTargetId += 2;
+    return PersistencePromise.resolve(this.highestTargetId);
   }
 
   setTargetsMetadata(

packages/firestore/src/local/memory_target_cache.ts

@@ -17,7 +17,6 @@
 
 import { FieldFilter, Filter, Query } from '../../../src/core/query';
 import { Target } from '../../../src/core/target';
-import { TargetIdGenerator } from '../../../src/core/target_id_generator';
 import { TargetId } from '../../../src/core/types';
 import {
   Document,
@@ -82,8 +81,7 @@ export class ClientMemoryState {
   activeTargets: ActiveTargetMap = {};
   queryMapping = new ObjectMap<Target, TargetId>(t => t.canonicalId());
   limboMapping: LimboMap = {};
-
-  limboIdGenerator: TargetIdGenerator = TargetIdGenerator.forSyncEngine();
+  nextLimboTarget = 1;
 
   constructor() {
     this.reset();
@@ -94,7 +92,13 @@ export class ClientMemoryState {
     this.queryMapping = new ObjectMap<Target, TargetId>(t => t.canonicalId());
     this.limboMapping = {};
     this.activeTargets = {};
-    this.limboIdGenerator = TargetIdGenerator.forSyncEngine();
+    this.nextLimboTarget = 1;
+  }
+
+  nextLimboTargetId() {
+    const nextLimboTargetId = this.nextLimboTarget;
+    this.nextLimboTarget += 2;
+    return nextLimboTargetId;
   }
 
   /**
@@ -112,7 +116,7 @@ export class ClientMemoryState {
 class CachedTargetIdGenerator {
   // TODO(wuandy): rename this to targetMapping.
   private queryMapping = new ObjectMap<Target, TargetId>(t => t.canonicalId());
-  private targetIdGenerator = TargetIdGenerator.forTargetCache();
+  private nextTargetId = 2;
 
   /**
    * Returns a cached target ID for the provided Target, or a new ID if no
@@ -122,7 +126,8 @@ class CachedTargetIdGenerator {
     if (this.queryMapping.has(target)) {
       return this.queryMapping.get(target)!;
     }
-    const targetId = this.targetIdGenerator.next();
+    const targetId = this.nextTargetId;
+    this.nextTargetId += 2;
     this.queryMapping.set(target, targetId);
     return targetId;
   }
@@ -170,8 +175,8 @@ export class SpecBuilder {
     return this.currentClientState;
   }
 
-  private get limboIdGenerator(): TargetIdGenerator {
-    return this.clientState.limboIdGenerator;
+  private nextLimboTargetId(): TargetId {
+    return this.clientState.nextLimboTargetId();
   }
 
   private get queryMapping(): ObjectMap<Target, TargetId> {
@@ -450,7 +455,7 @@ export class SpecBuilder {
       const path = key.path.canonicalString();
       // Create limbo target ID mapping if it was not in limbo yet
       if (!objUtils.contains(this.limboMapping, path)) {
-        this.limboMapping[path] = this.limboIdGenerator.next();
+        this.limboMapping[path] = this.nextLimboTargetId();
       }
       // Limbo doc queries are always without resume token
       this.addQueryToActiveTargets(