Replace usage of Set<DocumentKey> with Set<path> (#3652)

Author: schmidt-sebastian

packages/firestore/src/core/transaction.ts

@@ -33,7 +33,6 @@ import {
 import { fail, debugAssert } from '../util/assert';
 import { Code, FirestoreError } from '../util/error';
 import { SnapshotVersion } from './snapshot_version';
-import { ResourcePath } from '../model/path';
 
 /**
  * Internal transaction object responsible for accumulating the mutations to
@@ -57,7 +56,7 @@ export class Transaction {
    * When there's more than one write to the same key in a transaction, any
    * writes after the first are handled differently.
    */
-  private writtenDocs: Set<DocumentKey> = new Set();
+  private writtenDocs: Set</* path= */ string> = new Set();
 
   constructor(private datastore: Datastore) {}
 
@@ -83,7 +82,7 @@ export class Transaction {
 
   set(key: DocumentKey, data: ParsedSetData): void {
     this.write(data.toMutations(key, this.precondition(key)));
-    this.writtenDocs.add(key);
+    this.writtenDocs.add(key.toString());
   }
 
   update(key: DocumentKey, data: ParsedUpdateData): void {
@@ -92,12 +91,12 @@ export class Transaction {
     } catch (e) {
       this.lastWriteError = e;
     }
-    this.writtenDocs.add(key);
+    this.writtenDocs.add(key.toString());
   }
 
   delete(key: DocumentKey): void {
     this.write([new DeleteMutation(key, this.precondition(key))]);
-    this.writtenDocs.add(key);
+    this.writtenDocs.add(key.toString());
   }
 
   async commit(): Promise<void> {
@@ -114,7 +113,7 @@ export class Transaction {
     // For each document that was read but not written to, we want to perform
     // a `verify` operation.
     unwritten.forEach((_, path) => {
-      const key = new DocumentKey(ResourcePath.fromString(path));
+      const key = DocumentKey.fromPath(path);
       this.mutations.push(new VerifyMutation(key, this.precondition(key)));
     });
     await invokeCommitRpc(this.datastore, this.mutations);
@@ -153,7 +152,7 @@ export class Transaction {
    */
   private precondition(key: DocumentKey): Precondition {
     const version = this.readVersions.get(key.toString());
-    if (!this.writtenDocs.has(key) && version) {
+    if (!this.writtenDocs.has(key.toString()) && version) {
       return Precondition.updateTime(version);
     } else {
       return Precondition.none();
@@ -167,7 +166,7 @@ export class Transaction {
     const version = this.readVersions.get(key.toString());
     // 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 (!this.writtenDocs.has(key.toString()) && version) {
       if (version.isEqual(SnapshotVersion.min())) {
         // The document doesn't exist, so fail the transaction.
 

packages/firestore/src/local/memory_persistence.ts

@@ -191,15 +191,15 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
   /** Tracks all documents that are active in Query views. */
   private localViewReferences: ReferenceSet = new ReferenceSet();
   /** The list of documents that are potentially GCed after each transaction. */
-  private _orphanedDocuments: Set<DocumentKey> | null = null;
+  private _orphanedDocuments: Set</* path= */ string> | null = null;
 
   private constructor(private readonly persistence: MemoryPersistence) {}
 
   static factory(persistence: MemoryPersistence): MemoryEagerDelegate {
     return new MemoryEagerDelegate(persistence);
   }
 
-  private get orphanedDocuments(): Set<DocumentKey> {
+  private get orphanedDocuments(): Set<string> {
     if (!this._orphanedDocuments) {
       throw fail('orphanedDocuments is only valid during a transaction.');
     } else {
@@ -213,7 +213,7 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
     key: DocumentKey
   ): PersistencePromise<void> {
     this.localViewReferences.addReference(key, targetId);
-    this.orphanedDocuments.delete(key);
+    this.orphanedDocuments.delete(key.toString());
     return PersistencePromise.resolve();
   }
 
@@ -223,15 +223,15 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
     key: DocumentKey
   ): PersistencePromise<void> {
     this.localViewReferences.removeReference(key, targetId);
-    this.orphanedDocuments.add(key);
+    this.orphanedDocuments.add(key.toString());
     return PersistencePromise.resolve();
   }
 
   markPotentiallyOrphaned(
     txn: PersistenceTransaction,
     key: DocumentKey
   ): PersistencePromise<void> {
-    this.orphanedDocuments.add(key);
+    this.orphanedDocuments.add(key.toString());
     return PersistencePromise.resolve();
   }
 
@@ -242,18 +242,18 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
     const orphaned = this.localViewReferences.removeReferencesForId(
       targetData.targetId
     );
-    orphaned.forEach(key => this.orphanedDocuments.add(key));
+    orphaned.forEach(key => this.orphanedDocuments.add(key.toString()));
     const cache = this.persistence.getTargetCache();
     return cache
       .getMatchingKeysForTargetId(txn, targetData.targetId)
       .next(keys => {
-        keys.forEach(key => this.orphanedDocuments.add(key));
+        keys.forEach(key => this.orphanedDocuments.add(key.toString()));
       })
       .next(() => cache.removeTargetData(txn, targetData));
   }
 
   onTransactionStarted(): void {
-    this._orphanedDocuments = new Set<DocumentKey>();
+    this._orphanedDocuments = new Set<string>();
   }
 
   onTransactionCommitted(
@@ -264,7 +264,8 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
     const changeBuffer = cache.newChangeBuffer();
     return PersistencePromise.forEach(
       this.orphanedDocuments,
-      (key: DocumentKey) => {
+      (path: string) => {
+        const key = DocumentKey.fromPath(path);
         return this.isReferenced(txn, key).next(isReferenced => {
           if (!isReferenced) {
             changeBuffer.removeEntry(key);
@@ -283,9 +284,9 @@ export class MemoryEagerDelegate implements MemoryReferenceDelegate {
   ): PersistencePromise<void> {
     return this.isReferenced(txn, key).next(isReferenced => {
       if (isReferenced) {
-        this.orphanedDocuments.delete(key);
+        this.orphanedDocuments.delete(key.toString());
       } else {
-        this.orphanedDocuments.add(key);
+        this.orphanedDocuments.add(key.toString());
       }
     });
   }

packages/firestore/src/model/document_key.ts

@@ -28,6 +28,10 @@ export class DocumentKey {
     );
   }
 
+  static fromPath(path: string): DocumentKey {
+    return new DocumentKey(ResourcePath.fromString(path));
+  }
+
   static fromName(name: string): DocumentKey {
     return new DocumentKey(ResourcePath.fromString(name).popFirst(5));
   }