diff --git a/dev/src/bulk-writer.ts b/dev/src/bulk-writer.ts index 7013a334d..cdad89544 100644 --- a/dev/src/bulk-writer.ts +++ b/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( documentRef: firestore.DocumentReference, @@ -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} A promise that resolves with the result of - * the write. Throws an error if the write fails. + * @returns {Promise} 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); diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index c775e336c..b37bcf157 100644 --- a/dev/src/write-batch.ts +++ b/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); }); } diff --git a/dev/system-test/firestore.ts b/dev/system-test/firestore.ts index 9fbb9c619..047a028b2 100644 --- a/dev/system-test/firestore.ts +++ b/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', () => { diff --git a/dev/test/bulk-writer.ts b/dev/test/bulk-writer.ts index 3ec73b73f..338e69382 100644 --- a/dev/test/bulk-writer.ts +++ b/dev/test/bulk-writer.ts @@ -97,13 +97,13 @@ describe('BulkWriter', () => { }; } - function successResponse(seconds: number): api.IBatchWriteResponse { + function successResponse(updateTimeSeconds: number): api.IBatchWriteResponse { return { writeResults: [ { updateTime: { nanos: 0, - seconds, + seconds: updateTimeSeconds, }, }, ], @@ -111,7 +111,7 @@ describe('BulkWriter', () => { }; } - 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), ]), }, {