Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

Commit 99d60a6

Browse files
dconeybedconeybe
authored
fix: Allow an explicit MustExist precondition for update (#1985)
1 parent cd6bc86 commit 99d60a6

6 files changed

Lines changed: 126 additions & 18 deletions

File tree

dev/conformance/conformance-tests/update-exists-precond.json renamed to dev/conformance/conformance-tests/update-exists-false-precond.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
{
22
"tests": [
33
{
4-
"description": "update: Exists precondition is invalid",
5-
"comment": "The Update method does not support an explicit exists precondition.",
4+
"description": "update: Exists=false precondition is invalid",
5+
"comment": "The Update method does not support an explicit exists=false precondition.",
66
"update": {
77
"docRefPath": "projects/projectID/databases/(default)/documents/C/d",
88
"precondition": {
9-
"exists": true
9+
"exists": false
1010
},
1111
"jsonData": "{\"a\": 1}",
1212
"isError": true

‎dev/conformance/conformance-tests/update-exists-true-precond.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"tests": [
3+
{
4+
"description": "update: Exists=true precondition is valid",
5+
"comment": "The Update method supports an explicit exists=true precondition.",
6+
"update": {
7+
"docRefPath": "projects/projectID/databases/(default)/documents/C/d",
8+
"precondition": {
9+
"exists": true
10+
},
11+
"jsonData": "{\"a\": 1}",
12+
"request": {
13+
"database": "projects/projectID/databases/(default)",
14+
"writes": [
15+
{
16+
"update": {
17+
"name": "projects/projectID/databases/(default)/documents/C/d",
18+
"fields": {
19+
"a": {
20+
"integerValue": "1"
21+
}
22+
}
23+
},
24+
"updateMask": {
25+
"fieldPaths": [
26+
"a"
27+
]
28+
},
29+
"currentDocument": {
30+
"exists": true
31+
}
32+
}
33+
]
34+
}
35+
}
36+
}
37+
]
38+
}

dev/conformance/conformance-tests/update-paths-exists-precond.json renamed to dev/conformance/conformance-tests/update-paths-exists-false-precond.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
{
22
"tests": [
33
{
4-
"description": "update-paths: Exists precondition is invalid",
5-
"comment": "The Update method does not support an explicit exists precondition.",
4+
"description": "update-paths: Exists=false precondition is invalid",
5+
"comment": "The Update method does not support an explicit exists=false precondition.",
66
"updatePaths": {
77
"docRefPath": "projects/projectID/databases/(default)/documents/C/d",
88
"precondition": {
9-
"exists": true
9+
"exists": false
1010
},
1111
"fieldPaths": [
1212
{

‎dev/conformance/conformance-tests/update-paths-exists-true-precond.json‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
{
2+
"tests": [
3+
{
4+
"description": "update-paths: Exists=true precondition is valid",
5+
"comment": "The Update method supports an explicit exists=true precondition.",
6+
"updatePaths": {
7+
"docRefPath": "projects/projectID/databases/(default)/documents/C/d",
8+
"precondition": {
9+
"exists": true
10+
},
11+
"fieldPaths": [
12+
{
13+
"field": [
14+
"a"
15+
]
16+
}
17+
],
18+
"jsonValues": [
19+
"1"
20+
],
21+
"request": {
22+
"database": "projects/projectID/databases/(default)",
23+
"writes": [
24+
{
25+
"update": {
26+
"name": "projects/projectID/databases/(default)/documents/C/d",
27+
"fields": {
28+
"a": {
29+
"integerValue": "1"
30+
}
31+
}
32+
},
33+
"updateMask": {
34+
"fieldPaths": [
35+
"a"
36+
]
37+
},
38+
"currentDocument": {
39+
"exists": true
40+
}
41+
}
42+
]
43+
}
44+
}
45+
}
46+
]
47+
}

‎dev/src/write-batch.ts‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -660,12 +660,12 @@ export class WriteBatch implements firestore.WriteBatch {
660660
* @internal
661661
* @param arg The argument name or argument index (for varargs methods).
662662
* @param value The object to validate
663-
* @param allowExists Whether to allow the 'exists' preconditions.
663+
* @param options Options describing other things for this function to validate.
664664
*/
665665
function validatePrecondition(
666666
arg: string | number,
667667
value: unknown,
668-
allowExists: boolean
668+
options?: {allowedExistsValues?: boolean[]}
669669
): void {
670670
if (typeof value !== 'object' || value === null) {
671671
throw new Error('Input is not an object.');
@@ -677,20 +677,22 @@ function validatePrecondition(
677677

678678
if (precondition.exists !== undefined) {
679679
++conditions;
680-
if (!allowExists) {
680+
if (typeof precondition.exists !== 'boolean') {
681681
throw new Error(
682682
`${invalidArgumentMessage(
683683
arg,
684684
'precondition'
685-
)} "exists" is not an allowed precondition.`
685+
)} "exists" is not a boolean.'`
686686
);
687687
}
688-
if (typeof precondition.exists !== 'boolean') {
688+
if (
689+
options?.allowedExistsValues &&
690+
options.allowedExistsValues.indexOf(precondition.exists) < 0
691+
) {
689692
throw new Error(
690-
`${invalidArgumentMessage(
691-
arg,
692-
'precondition'
693-
)} "exists" is not a boolean.'`
693+
`${invalidArgumentMessage(arg, 'precondition')} ` +
694+
`"exists" is not allowed to have the value ${precondition.exists} ` +
695+
`(allowed values: ${options.allowedExistsValues.join(', ')})`
694696
);
695697
}
696698
}
@@ -733,7 +735,7 @@ function validateUpdatePrecondition(
733735
options?: RequiredArgumentOptions
734736
): asserts value is {lastUpdateTime?: Timestamp} {
735737
if (!validateOptional(value, options)) {
736-
validatePrecondition(arg, value, /* allowExists= */ false);
738+
validatePrecondition(arg, value, {allowedExistsValues: [true]});
737739
}
738740
}
739741

@@ -753,7 +755,7 @@ function validateDeletePrecondition(
753755
options?: RequiredArgumentOptions
754756
): void {
755757
if (!validateOptional(value, options)) {
756-
validatePrecondition(arg, value, /* allowExists= */ true);
758+
validatePrecondition(arg, value);
757759
}
758760
}
759761

‎dev/test/document.ts‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1629,6 +1629,27 @@ describe('update document', () => {
16291629
});
16301630
});
16311631

1632+
it('allows explicitly specifying {exists:true} precondition', () => {
1633+
const overrides: ApiOverride = {
1634+
commit: request => {
1635+
requestEquals(
1636+
request,
1637+
update({
1638+
document: document('documentId', 'foo', 'bar'),
1639+
mask: updateMask('foo'),
1640+
})
1641+
);
1642+
return response(writeResult(1));
1643+
},
1644+
};
1645+
1646+
return createInstance(overrides).then(firestore => {
1647+
return firestore
1648+
.doc('collectionId/documentId')
1649+
.update('foo', 'bar', {exists: true});
1650+
});
1651+
});
1652+
16321653
it('returns update time', () => {
16331654
const overrides: ApiOverride = {
16341655
commit: request => {
@@ -2069,7 +2090,7 @@ describe('update document', () => {
20692090
it("doesn't accept argument after precondition", () => {
20702091
expect(() => {
20712092
firestore.doc('collectionId/documentId').update('foo', 'bar', {
2072-
exists: true,
2093+
exists: false,
20732094
});
20742095
}).to.throw(INVALID_ARGUMENTS_TO_UPDATE);
20752096

0 commit comments

Comments
 (0)