From 745b608a7883a27ccb1c4d1f7800fdcc9a7917b1 Mon Sep 17 00:00:00 2001
From: wu-hui <53845758+wu-hui@users.noreply.github.com>
Date: Thu, 26 Aug 2021 12:17:02 -0400
Subject: [PATCH] fix: Handles identical document ids from different
 collections. (#1599)

---
 dev/src/bundle.ts  |  6 ++---
 dev/test/bundle.ts | 60 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/dev/src/bundle.ts b/dev/src/bundle.ts
index 584a4c11c..86096d389 100644
--- a/dev/src/bundle.ts
+++ b/dev/src/bundle.ts
@@ -83,7 +83,7 @@ export class BundleBuilder {
   }
 
   private addBundledDocument(snap: DocumentSnapshot, queryName?: string): void {
-    const originalDocument = this.documents.get(snap.id);
+    const originalDocument = this.documents.get(snap.ref.path);
     const originalQueries = originalDocument?.metadata.queries;
 
     // Update with document built from `snap` because it is newer.
@@ -92,7 +92,7 @@ export class BundleBuilder {
       Timestamp.fromProto(originalDocument.metadata.readTime!) < snap.readTime
     ) {
       const docProto = snap.toDocumentProto();
-      this.documents.set(snap.id, {
+      this.documents.set(snap.ref.path, {
         document: snap.exists ? docProto : undefined,
         metadata: {
           name: docProto.name,
@@ -103,7 +103,7 @@ export class BundleBuilder {
     }
 
     // Update `queries` to include both original and `queryName`.
-    const newDocument = this.documents.get(snap.id)!;
+    const newDocument = this.documents.get(snap.ref.path)!;
     newDocument.metadata.queries = originalQueries || [];
     if (queryName) {
       newDocument.metadata.queries!.push(queryName);
diff --git a/dev/test/bundle.ts b/dev/test/bundle.ts
index 59617ce65..3a56584b9 100644
--- a/dev/test/bundle.ts
+++ b/dev/test/bundle.ts
@@ -302,6 +302,66 @@ describe('Bundle Builder', () => {
       true
     );
   });
+
+  it('handles identical document id from different collections', async () => {
+    const bundle = firestore.bundle(TEST_BUNDLE_ID);
+    const snap1 = firestore.snapshot_(
+      {
+        name: `${DATABASE_ROOT}/documents/collectionId_A/doc1`,
+        fields: {foo: {stringValue: 'value'}, bar: {integerValue: '42'}},
+        createTime: '1970-01-01T00:00:01.002Z',
+        updateTime: '1970-01-01T00:00:03.000004Z',
+      },
+      // This should be the bundle read time.
+      '2020-01-01T00:00:05.000000006Z',
+      'json'
+    );
+    // Same document id but different collection
+    const snap2 = firestore.snapshot_(
+      {
+        name: `${DATABASE_ROOT}/documents/collectionId_B/doc1`,
+        fields: {foo: {stringValue: 'value'}, bar: {integerValue: '-42'}},
+        createTime: '1970-01-01T00:00:01.002Z',
+        updateTime: '1970-01-01T00:00:03.000004Z',
+      },
+      '1970-01-01T00:00:05.000000006Z',
+      'json'
+    );
+
+    bundle.add(snap1);
+    bundle.add(snap2);
+    // Bundle is expected to be [bundleMeta, snap1Meta, snap1, snap2Meta, snap2] because `snap1` is newer.
+    const elements = await bundleToElementArray(bundle.build());
+    expect(elements.length).to.equal(5);
+
+    const meta = (elements[0] as IBundleElement).metadata;
+    verifyMetadata(
+      meta!,
+      // `snap1.readTime` is the bundle createTime, because it is larger than `snap2.readTime`.
+      snap1.readTime.toProto().timestampValue!,
+      2
+    );
+
+    // Verify doc1Meta and doc1Snap
+    let docMeta = (elements[1] as IBundleElement).documentMetadata;
+    let docSnap = (elements[2] as IBundleElement).document;
+    expect(docMeta).to.deep.equal({
+      name: snap1.toDocumentProto().name,
+      readTime: snap1.readTime.toProto().timestampValue,
+      exists: true,
+    });
+    expect(docSnap).to.deep.equal(snap1.toDocumentProto());
+
+    // Verify doc2Meta and doc2Snap
+    docMeta = (elements[3] as IBundleElement).documentMetadata;
+    docSnap = (elements[4] as IBundleElement).document;
+    expect(docMeta).to.deep.equal({
+      name: snap2.toDocumentProto().name,
+      readTime: snap2.readTime.toProto().timestampValue,
+      exists: true,
+    });
+    expect(docSnap).to.deep.equal(snap2.toDocumentProto());
+  });
 });
 
 describe('Bundle Builder using BigInt', () => {