From 99d60a6f87b70c942ac2bd9464cc6d64323f9dfb Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 23 Jan 2024 19:03:43 -0500 Subject: [PATCH] fix: Allow an explicit MustExist precondition for update (#1985) --- ....json => update-exists-false-precond.json} | 6 +-- .../update-exists-true-precond.json | 38 +++++++++++++++ ...=> update-paths-exists-false-precond.json} | 6 +-- .../update-paths-exists-true-precond.json | 47 +++++++++++++++++++ dev/src/write-batch.ts | 24 +++++----- dev/test/document.ts | 23 ++++++++- 6 files changed, 126 insertions(+), 18 deletions(-) rename dev/conformance/conformance-tests/{update-exists-precond.json => update-exists-false-precond.json} (70%) create mode 100644 dev/conformance/conformance-tests/update-exists-true-precond.json rename dev/conformance/conformance-tests/{update-paths-exists-precond.json => update-paths-exists-false-precond.json} (76%) create mode 100644 dev/conformance/conformance-tests/update-paths-exists-true-precond.json diff --git a/dev/conformance/conformance-tests/update-exists-precond.json b/dev/conformance/conformance-tests/update-exists-false-precond.json similarity index 70% rename from dev/conformance/conformance-tests/update-exists-precond.json rename to dev/conformance/conformance-tests/update-exists-false-precond.json index bdbe274b4..031fd1af3 100644 --- a/dev/conformance/conformance-tests/update-exists-precond.json +++ b/dev/conformance/conformance-tests/update-exists-false-precond.json @@ -1,12 +1,12 @@ { "tests": [ { - "description": "update: Exists precondition is invalid", - "comment": "The Update method does not support an explicit exists precondition.", + "description": "update: Exists=false precondition is invalid", + "comment": "The Update method does not support an explicit exists=false precondition.", "update": { "docRefPath": "projects/projectID/databases/(default)/documents/C/d", "precondition": { - "exists": true + "exists": false }, "jsonData": "{\"a\": 1}", "isError": true diff --git a/dev/conformance/conformance-tests/update-exists-true-precond.json b/dev/conformance/conformance-tests/update-exists-true-precond.json new file mode 100644 index 000000000..f1aa800e4 --- /dev/null +++ b/dev/conformance/conformance-tests/update-exists-true-precond.json @@ -0,0 +1,38 @@ +{ + "tests": [ + { + "description": "update: Exists=true precondition is valid", + "comment": "The Update method supports an explicit exists=true precondition.", + "update": { + "docRefPath": "projects/projectID/databases/(default)/documents/C/d", + "precondition": { + "exists": true + }, + "jsonData": "{\"a\": 1}", + "request": { + "database": "projects/projectID/databases/(default)", + "writes": [ + { + "update": { + "name": "projects/projectID/databases/(default)/documents/C/d", + "fields": { + "a": { + "integerValue": "1" + } + } + }, + "updateMask": { + "fieldPaths": [ + "a" + ] + }, + "currentDocument": { + "exists": true + } + } + ] + } + } + } + ] +} diff --git a/dev/conformance/conformance-tests/update-paths-exists-precond.json b/dev/conformance/conformance-tests/update-paths-exists-false-precond.json similarity index 76% rename from dev/conformance/conformance-tests/update-paths-exists-precond.json rename to dev/conformance/conformance-tests/update-paths-exists-false-precond.json index d495db033..f122b5307 100644 --- a/dev/conformance/conformance-tests/update-paths-exists-precond.json +++ b/dev/conformance/conformance-tests/update-paths-exists-false-precond.json @@ -1,12 +1,12 @@ { "tests": [ { - "description": "update-paths: Exists precondition is invalid", - "comment": "The Update method does not support an explicit exists precondition.", + "description": "update-paths: Exists=false precondition is invalid", + "comment": "The Update method does not support an explicit exists=false precondition.", "updatePaths": { "docRefPath": "projects/projectID/databases/(default)/documents/C/d", "precondition": { - "exists": true + "exists": false }, "fieldPaths": [ { diff --git a/dev/conformance/conformance-tests/update-paths-exists-true-precond.json b/dev/conformance/conformance-tests/update-paths-exists-true-precond.json new file mode 100644 index 000000000..39d61c99a --- /dev/null +++ b/dev/conformance/conformance-tests/update-paths-exists-true-precond.json @@ -0,0 +1,47 @@ +{ + "tests": [ + { + "description": "update-paths: Exists=true precondition is valid", + "comment": "The Update method supports an explicit exists=true precondition.", + "updatePaths": { + "docRefPath": "projects/projectID/databases/(default)/documents/C/d", + "precondition": { + "exists": true + }, + "fieldPaths": [ + { + "field": [ + "a" + ] + } + ], + "jsonValues": [ + "1" + ], + "request": { + "database": "projects/projectID/databases/(default)", + "writes": [ + { + "update": { + "name": "projects/projectID/databases/(default)/documents/C/d", + "fields": { + "a": { + "integerValue": "1" + } + } + }, + "updateMask": { + "fieldPaths": [ + "a" + ] + }, + "currentDocument": { + "exists": true + } + } + ] + } + } + } + ] +} diff --git a/dev/src/write-batch.ts b/dev/src/write-batch.ts index 33536a275..7cce55e04 100644 --- a/dev/src/write-batch.ts +++ b/dev/src/write-batch.ts @@ -660,12 +660,12 @@ export class WriteBatch implements firestore.WriteBatch { * @internal * @param arg The argument name or argument index (for varargs methods). * @param value The object to validate - * @param allowExists Whether to allow the 'exists' preconditions. + * @param options Options describing other things for this function to validate. */ function validatePrecondition( arg: string | number, value: unknown, - allowExists: boolean + options?: {allowedExistsValues?: boolean[]} ): void { if (typeof value !== 'object' || value === null) { throw new Error('Input is not an object.'); @@ -677,20 +677,22 @@ function validatePrecondition( if (precondition.exists !== undefined) { ++conditions; - if (!allowExists) { + if (typeof precondition.exists !== 'boolean') { throw new Error( `${invalidArgumentMessage( arg, 'precondition' - )} "exists" is not an allowed precondition.` + )} "exists" is not a boolean.'` ); } - if (typeof precondition.exists !== 'boolean') { + if ( + options?.allowedExistsValues && + options.allowedExistsValues.indexOf(precondition.exists) < 0 + ) { throw new Error( - `${invalidArgumentMessage( - arg, - 'precondition' - )} "exists" is not a boolean.'` + `${invalidArgumentMessage(arg, 'precondition')} ` + + `"exists" is not allowed to have the value ${precondition.exists} ` + + `(allowed values: ${options.allowedExistsValues.join(', ')})` ); } } @@ -733,7 +735,7 @@ function validateUpdatePrecondition( options?: RequiredArgumentOptions ): asserts value is {lastUpdateTime?: Timestamp} { if (!validateOptional(value, options)) { - validatePrecondition(arg, value, /* allowExists= */ false); + validatePrecondition(arg, value, {allowedExistsValues: [true]}); } } @@ -753,7 +755,7 @@ function validateDeletePrecondition( options?: RequiredArgumentOptions ): void { if (!validateOptional(value, options)) { - validatePrecondition(arg, value, /* allowExists= */ true); + validatePrecondition(arg, value); } } diff --git a/dev/test/document.ts b/dev/test/document.ts index f37bf0050..bc140aeef 100644 --- a/dev/test/document.ts +++ b/dev/test/document.ts @@ -1629,6 +1629,27 @@ describe('update document', () => { }); }); + it('allows explicitly specifying {exists:true} precondition', () => { + const overrides: ApiOverride = { + commit: request => { + requestEquals( + request, + update({ + document: document('documentId', 'foo', 'bar'), + mask: updateMask('foo'), + }) + ); + return response(writeResult(1)); + }, + }; + + return createInstance(overrides).then(firestore => { + return firestore + .doc('collectionId/documentId') + .update('foo', 'bar', {exists: true}); + }); + }); + it('returns update time', () => { const overrides: ApiOverride = { commit: request => { @@ -2069,7 +2090,7 @@ describe('update document', () => { it("doesn't accept argument after precondition", () => { expect(() => { firestore.doc('collectionId/documentId').update('foo', 'bar', { - exists: true, + exists: false, }); }).to.throw(INVALID_ARGUMENTS_TO_UPDATE);