fix: collectionReference.add() validation (#925)

googleapis · Feb 18, 2020 · 19c2c75 · 19c2c75
1 parent d36f509
commit 19c2c75
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 6 deletions.
diff --git a/dev/src/reference.ts b/dev/src/reference.ts
@@ -2301,7 +2301,8 @@ export class CollectionReference<T = DocumentData> extends Query<T> {
    * });
    */
   add(data: T): Promise<DocumentReference<T>> {
-    validateDocumentData('data', data, /*allowDeletes=*/ false);
+    const firestoreData = this._queryOptions.converter.toFirestore(data);
+    validateDocumentData('data', firestoreData, /*allowDeletes=*/ false);
 
     const documentRef = this.doc();
     return documentRef.create(data).then(() => documentRef);

diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts
@@ -182,11 +182,11 @@ export class WriteBatch {
    */
   create<T>(documentRef: DocumentReference<T>, data: T): WriteBatch {
     validateDocumentReference('documentRef', documentRef);
-    validateDocumentData('data', data, /* allowDeletes= */ false);
+    const firestoreData = documentRef._converter.toFirestore(data);
+    validateDocumentData('data', firestoreData, /* allowDeletes= */ false);
 
     this.verifyNotCommitted();
 
-    const firestoreData = documentRef._converter.toFirestore(data);
     const transform = DocumentTransform.fromObject(documentRef, firestoreData);
     transform.validate();
 

diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts
@@ -222,11 +222,10 @@ describe('CollectionReference class', () => {
   });
 
   it('supports withConverter()', async () => {
-    const ref = firestore
+    const ref = await firestore
       .collection('col')
       .withConverter(postConverter)
-      .doc('doc');
-    await ref.set(new Post('post', 'author'));
+      .add(new Post('post', 'author'));
     const postData = await ref.get();
     const post = postData.data();
     expect(post).to.not.be.undefined;

diff --git a/dev/test/collection.ts b/dev/test/collection.ts
@@ -211,6 +211,66 @@ describe('Collection interface', () => {
     });
   });
 
+  it('for CollectionReference.withConverter().add()', async () => {
+    let doc = document('dummy');
+    const overrides: ApiOverride = {
+      commit: request => {
+        // Extract the auto-generated document ID.
+        const docId = request.writes![0].update!.name!;
+        const docIdSplit = docId.split('/');
+        doc = document(
+          docIdSplit[docIdSplit.length - 1],
+          'author',
+          'author',
+          'title',
+          'post'
+        );
+        expect(request).to.deep.equal({
+          database: DATABASE_ROOT,
+          writes: [
+            {
+              update: {
+                fields: {
+                  author: {
+                    stringValue: 'author',
+                  },
+                  title: {
+                    stringValue: 'post',
+                  },
+                },
+                name: docId,
+              },
+              currentDocument: {
+                exists: false,
+              },
+            },
+          ],
+        });
+
+        return response(writeResult(1));
+      },
+      batchGetDocuments: () => {
+        const stream = through2.obj();
+        setImmediate(() => {
+          stream.push({found: doc, readTime: {seconds: 5, nanos: 6}});
+          stream.push(null);
+        });
+        return stream;
+      },
+    };
+
+    return createInstance(overrides).then(async firestore => {
+      const docRef = await firestore
+        .collection('collectionId')
+        .withConverter(postConverter)
+        .add(new Post('post', 'author'));
+      const postData = await docRef.get();
+      const post = postData.data();
+      expect(post).to.not.be.undefined;
+      expect(post!.toString()).to.equal('post, by author');
+    });
+  });
+
   it('drops the converter when calling CollectionReference<T>.parent()', () => {
     return createInstance().then(async firestore => {
       const postsCollection = firestore