Cleaning up WriteResults

src/reference.js

@@ -352,8 +352,8 @@ class DocumentReference {
     return writeBatch
       .create(this, data)
       .commit()
-      .then(res => {
-        return Promise.resolve(res.writeResults[0]);
+      .then(writeResults => {
+        return Promise.resolve(writeResults[0]);
       });
   }
 
@@ -383,8 +383,8 @@ class DocumentReference {
     return writeBatch
       .delete(this, precondition)
       .commit()
-      .then(res => {
-        return Promise.resolve(res.writeResults[0]);
+      .then(writeResults => {
+        return Promise.resolve(writeResults[0]);
       });
   }
 
@@ -415,8 +415,8 @@ class DocumentReference {
     return writeBatch
       .set(this, data, options)
       .commit()
-      .then(res => {
-        return Promise.resolve(res.writeResults[0]);
+      .then(writeResults => {
+        return Promise.resolve(writeResults[0]);
       });
   }
 
@@ -455,8 +455,8 @@ class DocumentReference {
     return writeBatch.update
       .apply(writeBatch, [this, dataOrField].concat(preconditionOrValues))
       .commit()
-      .then(res => {
-        return Promise.resolve(res.writeResults[0]);
+      .then(writeResults => {
+        return Promise.resolve(writeResults[0]);
       });
   }
 

src/write-batch.js

@@ -16,6 +16,7 @@
 
 'use strict';
 
+const assert = require('assert');
 const is = require('is');
 
 /*!
@@ -449,12 +450,16 @@ class WriteBatch {
         });
     }
 
-    request.writes = this._writes;
+    // We create our own copy of this array here since we need to access it
+    // when processing the response.
+    let writeRequests = this._writes.slice();
+
+    request.writes = writeRequests;
 
     Firestore.log(
       'WriteBatch.commit',
       'Sending %d writes',
-      request.writes.length
+      writeRequests.length
     );
 
     if (explicitTransaction) {
@@ -464,22 +469,36 @@ class WriteBatch {
     return this._firestore
       .request(this._api.Firestore.commit.bind(this._api.Firestore), request)
       .then(resp => {
-        let commitTime = DocumentSnapshot.toISOTime(resp.commitTime);
-        let result = {
-          writeResults: [],
-        };
+        const commitTime = DocumentSnapshot.toISOTime(resp.commitTime);
+        const writeResults = [];
 
         if (resp.writeResults) {
-          for (let writeResult of resp.writeResults) {
-            result.writeResults.push(
-              new WriteResult(
-                DocumentSnapshot.toISOTime(writeResult.updateTime) || commitTime
-              )
-            );
+          assert(
+            writeRequests.length === resp.writeResults.length,
+            `Expected one write result per operation, but got ${resp
+              .writeResults
+              .length} results for ${writeRequests.length} operations.`
+          );
+
+          for (let i = 0; i < resp.writeResults.length; ++i) {
+            let writeRequest = writeRequests[i];
+            let writeResult = resp.writeResults[i];
+
+            // Don't return write results for document transforms, as the fact
+            // that we have to split one write operation into two distinct
+            // writes is an implementation detail.
+            if (writeRequest.update || writeRequest.delete) {
+              writeResults.push(
+                new WriteResult(
+                  DocumentSnapshot.toISOTime(writeResult.updateTime) ||
+                    commitTime
+                )
+              );
+            }
           }
         }
 
-        return result;
+        return writeResults;
       });
   }
 

system-test/firestore.js

@@ -1204,6 +1204,15 @@ describe('WriteBatch class', function() {
       });
   });
 
+  it('omits document transforms from write results', function() {
+    let batch = firestore.batch();
+    batch.set(randomCol.doc(), {foo: 'a'});
+    batch.set(randomCol.doc(), {foo: Firestore.FieldValue.serverTimestamp()});
+    return batch.commit().then(writeResults => {
+      assert.equal(writeResults.length, 2);
+    });
+  });
+
   it('enforces that updated document exists', function() {
     let ref = randomCol.doc();
     let batch = firestore.batch();
@@ -1218,17 +1227,6 @@ describe('WriteBatch class', function() {
       });
   });
 
-  // @todo: Uncomment when server supports verify()
-  // Throws 'java.lang.UnsupportedOperationException: not implemented yet'
-  // it('has verify() method', function() {
-  //   let ref = firestore.doc('col/doc');
-  //   let batch = firestore.batch();
-  //   batch.set(ref, {foo: 'a'});
-  //   return batch.commit().then(() => {
-  //     return firestore.batch().verify_(ref, {exists:true}).commit();
-  //   });
-  // });
-
   it('has delete() method', function() {
     let success;
 

test/document.js

@@ -206,6 +206,27 @@ const defaultWriteResult = {
   ],
 };
 
+const serverTimestampWriteResult = {
+  commitTime: {
+    nanos: 3,
+    seconds: 4,
+  },
+  writeResults: [
+    {
+      updateTime: {
+        nanos: 3,
+        seconds: 4,
+      },
+    },
+    {
+      updateTime: {
+        nanos: 3,
+        seconds: 4,
+      },
+    },
+  ],
+};
+
 describe('DocumentReference interface', function() {
   let firestore;
   let documentRef;
@@ -368,7 +389,7 @@ describe('serialize document', function() {
         )
       );
 
-      callback(null, defaultWriteResult);
+      callback(null, serverTimestampWriteResult);
     };
 
     return firestore.doc('collectionId/documentId').set({
@@ -993,7 +1014,7 @@ describe('create document', function() {
         )
       );
 
-      callback(null, defaultWriteResult);
+      callback(null, serverTimestampWriteResult);
     };
 
     return firestore.doc('collectionId/documentId').create({
@@ -1056,7 +1077,7 @@ describe('update document', function() {
           fieldTransform('a.b', 'REQUEST_TIME', 'c.d', 'REQUEST_TIME')
         )
       );
-      callback(null, defaultWriteResult);
+      callback(null, serverTimestampWriteResult);
     };
 
     return firestore.doc('collectionId/documentId').update(

test/transaction.js

@@ -49,16 +49,18 @@ function commit(transaction, writes, err) {
       nanos: 0,
       seconds: 0,
     },
-    writeResults: [
-      {
-        updateTime: {
-          nanos: 0,
-          seconds: 0,
-        },
-      },
-    ],
+    writeResults: [],
   };
 
+  for (let i = 0; i < proto.writes.length; ++i) {
+    response.writeResults.push({
+      updateTime: {
+        nanos: 0,
+        seconds: 0,
+      },
+    });
+  }
+
   return {
     type: 'commit',
     request: proto,

test/write-batch.js

@@ -157,6 +157,18 @@ describe('batch support', function() {
               name: documentName,
             },
           },
+          {
+            transform: {
+              document:
+                'projects/test-project/databases/(default)/documents/col/doc',
+              fieldTransforms: [
+                {
+                  fieldPath: 'foo',
+                  setToServerValue: 'REQUEST_TIME',
+                },
+              ],
+            },
+          },
           {
             currentDocument: {
               exists: true,
@@ -195,6 +207,13 @@ describe('batch support', function() {
               seconds: 0,
             },
           },
+          // This write result conforms to the DocumentTransform and won't be returned in the response.
+          {
+            updateTime: {
+              nanos: 1337,
+              seconds: 1337,
+            },
+          },
           {
             updateTime: {
               nanos: 1,
@@ -207,32 +226,30 @@ describe('batch support', function() {
               seconds: 2,
             },
           },
+          {
+            updateTime: {
+              nanos: 3,
+              seconds: 3,
+            },
+          },
         ],
       });
     };
 
     writeBatch = firestore.batch();
   });
 
-  function verifyResponse(resp) {
-    assert.equal(
-      resp.writeResults[0].writeTime,
-      '1970-01-01T00:00:00.000000000Z'
-    );
-    assert.equal(
-      resp.writeResults[1].writeTime,
-      '1970-01-01T00:00:01.000000001Z'
-    );
-    assert.equal(
-      resp.writeResults[2].writeTime,
-      '1970-01-01T00:00:02.000000002Z'
-    );
+  function verifyResponse(writeResults) {
+    assert.equal(writeResults[0].writeTime, '1970-01-01T00:00:00.000000000Z');
+    assert.equal(writeResults[1].writeTime, '1970-01-01T00:00:01.000000001Z');
+    assert.equal(writeResults[2].writeTime, '1970-01-01T00:00:02.000000002Z');
+    assert.equal(writeResults[3].writeTime, '1970-01-01T00:00:03.000000003Z');
   }
 
   it('accepts multiple operations', function() {
     let documentName = firestore.doc('col/doc');
 
-    writeBatch.set(documentName, {});
+    writeBatch.set(documentName, {foo: Firestore.FieldValue.serverTimestamp()});
     writeBatch.update(documentName, {});
     writeBatch.create(documentName, {});
     writeBatch.delete(documentName);
@@ -246,7 +263,7 @@ describe('batch support', function() {
     let documentName = firestore.doc('col/doc');
 
     return writeBatch
-      .set(documentName, {})
+      .set(documentName, {foo: Firestore.FieldValue.serverTimestamp()})
       .update(documentName, {})
       .create(documentName, {})
       .delete(documentName)