fix: add BulkWriter integration tests, java backport changes, delete fix

dev/src/bulk-writer.ts

@@ -113,7 +113,7 @@ class BulkCommitBatch {
 
   /**
    * Adds a `delete` operation to the WriteBatch. Returns a promise that
-   * resolves with the result of the delete.
+   * resolves with the sentinel value (Timestamp(0)) for the delete operation.
    */
   delete<T>(
     documentRef: firestore.DocumentReference<T>,
@@ -343,8 +343,9 @@ export class BulkWriter {
    * @param {Timestamp=} precondition.lastUpdateTime If set, enforces that the
    * document was last updated at lastUpdateTime. Fails the batch if the
    * document doesn't exist or was last updated at a different time.
-   * @returns {Promise<WriteResult>} A promise that resolves with the result of
-   * the write. Throws an error if the write fails.
+   * @returns {Promise<WriteResult>} A promise that resolves with a sentinel
+   * Timestamp indicating that the delete was successful. Throws an error if
+   * the write fails.
    *
    * @example
    * let bulkWriter = firestore.bulkWriter();
@@ -353,7 +354,7 @@ export class BulkWriter {
    * bulkWriter
    *  .delete(documentRef)
    *  .then(result => {
-   *    console.log('Successfully deleted document at: ', result);
+   *    console.log('Successfully deleted document");
    *  })
    *  .catch(err => {
    *    console.log('Delete failed with: ', err);

dev/src/write-batch.ts

@@ -579,10 +579,18 @@ export class WriteBatch implements firestore.WriteBatch {
       const status = response.status[i];
       const error = new GoogleError(status.message || undefined);
       error.code = status.code as Status;
-      return new BatchWriteResult(
-        result.updateTime ? Timestamp.fromProto(result.updateTime) : null,
-        error
-      );
+
+      // Since delete operations currently do not have write times, use a
+      // sentinel Timestamp value.
+      // TODO(b/158502664): Use actual delete timestamp.
+      const isSuccessfulDelete =
+        result.updateTime === null && error.code === Status.OK;
+      const DELETE_TIMESTAMP_SENTINEL = Timestamp.fromMillis(0);
+      const updateTime =
+        error.code === Status.OK
+          ? Timestamp.fromProto(result.updateTime || DELETE_TIMESTAMP_SENTINEL)
+          : null;
+      return new BatchWriteResult(updateTime, error);
     });
   }
 

dev/system-test/firestore.ts

@@ -44,6 +44,7 @@ import {
   verifyInstance,
 } from '../test/util/helpers';
 import IBundleElement = firestore.IBundleElement;
+import {BulkWriter} from '../src/bulk-writer';
 
 use(chaiAsPromised);
 
@@ -2329,23 +2330,76 @@ describe('QuerySnapshot class', () => {
 describe('BulkWriter class', () => {
   let firestore: Firestore;
   let randomCol: CollectionReference;
+  let writer: BulkWriter;
 
   beforeEach(() => {
     firestore = new Firestore({});
+    writer = firestore._bulkWriter();
     randomCol = getTestRoot(firestore);
   });
 
   afterEach(() => verifyInstance(firestore));
 
-  // TODO(BulkWriter): Enable this test once protos are public.
-  it.skip('can terminate once BulkWriter is closed', async () => {
+  it('has create() method', async () => {
+    const ref = randomCol.doc('doc1');
+    const singleOp = writer.create(ref, {foo: 'bar'});
+    await writer.close();
+    const result = await ref.get();
+    expect(result.data()).to.deep.equal({foo: 'bar'});
+    const writeTime = (await singleOp).writeTime;
+    expect(writeTime).to.not.be.null;
+  });
+
+  it('has set() method', async () => {
+    const ref = randomCol.doc('doc1');
+    const singleOp = writer.set(ref, {foo: 'bar'});
+    await writer.close();
+    const result = await ref.get();
+    expect(result.data()).to.deep.equal({foo: 'bar'});
+    const writeTime = (await singleOp).writeTime;
+    expect(writeTime).to.not.be.null;
+  });
+
+  it('has update() method', async () => {
+    const ref = randomCol.doc('doc1');
+    await ref.set({foo: 'bar'});
+    const singleOp = writer.update(ref, {foo: 'bar2'});
+    await writer.close();
+    const result = await ref.get();
+    expect(result.data()).to.deep.equal({foo: 'bar2'});
+    const writeTime = (await singleOp).writeTime;
+    expect(writeTime).to.not.be.null;
+  });
+
+  it('has delete() method', async () => {
+    const ref = randomCol.doc('doc1');
+    await ref.set({foo: 'bar'});
+    const singleOp = writer.delete(ref);
+    await writer.close();
+    const result = await ref.get();
+    expect(result.exists).to.be.false;
+    // TODO(b/158502664): Remove this check once we can get write times.
+    const deleteResult = await singleOp;
+    expect(deleteResult.writeTime).to.deep.equal(new Timestamp(0, 0));
+  });
+
+  it('can terminate once BulkWriter is closed', async () => {
     const ref = randomCol.doc('doc1');
-    const writer = firestore._bulkWriter();
     writer.set(ref, {foo: 'bar'});
-    writer.set(ref, {foo: 'bar2'});
     await writer.close();
     return firestore.terminate();
   });
+
+  it('writes to the same document in order', async () => {
+    const ref = randomCol.doc('doc1');
+    await ref.set({foo: 'bar0'});
+    writer.set(ref, {foo: 'bar1'});
+    writer.set(ref, {foo: 'bar2'});
+    writer.set(ref, {foo: 'bar3'});
+    await writer.flush();
+    const res = await ref.get();
+    expect(res.data()).to.deep.equal({foo: 'bar3'});
+  });
 });
 
 describe('Client initialization', () => {

dev/test/bulk-writer.ts

@@ -97,21 +97,21 @@ describe('BulkWriter', () => {
     };
   }
 
-  function successResponse(seconds: number): api.IBatchWriteResponse {
+  function successResponse(updateTimeSeconds: number): api.IBatchWriteResponse {
     return {
       writeResults: [
         {
           updateTime: {
             nanos: 0,
-            seconds,
+            seconds: updateTimeSeconds,
           },
         },
       ],
       status: [{code: Status.OK}],
     };
   }
 
-  function failResponse(): api.IBatchWriteResponse {
+  function failedResponse(): api.IBatchWriteResponse {
     return {
       writeResults: [
         {
@@ -254,7 +254,7 @@ describe('BulkWriter', () => {
     const bulkWriter = await instantiateInstance([
       {
         request: createRequest([setOp('doc', 'bar')]),
-        response: failResponse(),
+        response: failedResponse(),
       },
     ]);
 
@@ -332,11 +332,11 @@ describe('BulkWriter', () => {
     const bulkWriter = await instantiateInstance([
       {
         request: createRequest([setOp('doc', 'bar')]),
-        response: successResponse(0),
+        response: successResponse(1),
       },
       {
         request: createRequest([updateOp('doc', 'bar1')]),
-        response: successResponse(1),
+        response: successResponse(2),
       },
     ]);
 
@@ -355,7 +355,7 @@ describe('BulkWriter', () => {
     const bulkWriter = await instantiateInstance([
       {
         request: createRequest([setOp('doc1', 'bar'), updateOp('doc2', 'bar')]),
-        response: mergeResponses([successResponse(0), successResponse(1)]),
+        response: mergeResponses([successResponse(1), successResponse(2)]),
       },
     ]);
 
@@ -404,7 +404,7 @@ describe('BulkWriter', () => {
     const bulkWriter = await instantiateInstance([
       {
         request: createRequest([setOp('doc', 'bar')]),
-        response: successResponse(0),
+        response: successResponse(1),
       },
       {
         request: createRequest([
@@ -447,9 +447,9 @@ describe('BulkWriter', () => {
           createOp('doc3', 'bar'),
         ]),
         response: mergeResponses([
-          successResponse(0),
           successResponse(1),
           successResponse(2),
+          successResponse(3),
         ]),
       },
       {