Skip to content

Commit 4e3ebca

Browse files
author
Brian Chen
authored
Fix preconditions in transactions (#1980)
1 parent f635457 commit 4e3ebca

File tree

2 files changed

+437
-194
lines changed

2 files changed

+437
-194
lines changed

‎packages/firestore/src/core/transaction.ts

Lines changed: 89 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -42,33 +42,15 @@ export class Transaction {
4242
*/
4343
private lastWriteError: FirestoreError;
4444

45-
constructor(private datastore: Datastore) {}
46-
47-
private recordVersion(doc: MaybeDocument): void {
48-
let docVersion: SnapshotVersion;
49-
50-
if (doc instanceof Document) {
51-
docVersion = doc.version;
52-
} else if (doc instanceof NoDocument) {
53-
// For deleted docs, we must use baseVersion 0 when we overwrite them.
54-
docVersion = SnapshotVersion.forDeletedDoc();
55-
} else {
56-
throw fail('Document in a transaction was a ' + doc.constructor.name);
57-
}
45+
/**
46+
* Set of documents that have been written in the transaction.
47+
*
48+
* When there's more than one write to the same key in a transaction, any
49+
* writes after hte first are handled differently.
50+
*/
51+
private writtenDocs: Set<DocumentKey> = new Set();
5852

59-
const existingVersion = this.readVersions.get(doc.key);
60-
if (existingVersion !== null) {
61-
if (!docVersion.isEqual(existingVersion)) {
62-
// This transaction will fail no matter what.
63-
throw new FirestoreError(
64-
Code.ABORTED,
65-
'Document version changed between two reads.'
66-
);
67-
}
68-
} else {
69-
this.readVersions = this.readVersions.insert(doc.key, docVersion);
70-
}
71-
}
53+
constructor(private datastore: Datastore) {}
7254

7355
lookup(keys: DocumentKey[]): Promise<MaybeDocument[]> {
7456
this.ensureCommitNotCalled();
@@ -91,56 +73,9 @@ export class Transaction {
9173
});
9274
}
9375

94-
private write(mutations: Mutation[]): void {
95-
this.ensureCommitNotCalled();
96-
this.mutations = this.mutations.concat(mutations);
97-
}
98-
99-
/**
100-
* Returns the version of this document when it was read in this transaction,
101-
* as a precondition, or no precondition if it was not read.
102-
*/
103-
private precondition(key: DocumentKey): Precondition {
104-
const version = this.readVersions.get(key);
105-
if (version) {
106-
return Precondition.updateTime(version);
107-
} else {
108-
return Precondition.NONE;
109-
}
110-
}
111-
112-
/**
113-
* Returns the precondition for a document if the operation is an update.
114-
*/
115-
private preconditionForUpdate(key: DocumentKey): Precondition {
116-
const version = this.readVersions.get(key);
117-
if (version && version.isEqual(SnapshotVersion.forDeletedDoc())) {
118-
// The document doesn't exist, so fail the transaction.
119-
120-
// This has to be validated locally because you can't send a precondition
121-
// that a document does not exist without changing the semantics of the
122-
// backend write to be an insert. This is the reverse of what we want,
123-
// since we want to assert that the document doesn't exist but then send
124-
// the update and have it fail. Since we can't express that to the backend,
125-
// we have to validate locally.
126-
127-
// Note: this can change once we can send separate verify writes in the transaction.
128-
throw new FirestoreError(
129-
Code.INVALID_ARGUMENT,
130-
"Can't update a document that doesn't exist."
131-
);
132-
} else if (version) {
133-
// Document exists, base precondition on document update time.
134-
return Precondition.updateTime(version);
135-
} else {
136-
// Document was not read, so we just use the preconditions for a blind
137-
// update.
138-
return Precondition.exists(true);
139-
}
140-
}
141-
14276
set(key: DocumentKey, data: ParsedSetData): void {
14377
this.write(data.toMutations(key, this.precondition(key)));
78+
this.writtenDocs.add(key);
14479
}
14580

14681
update(key: DocumentKey, data: ParsedUpdateData): void {
@@ -149,16 +84,12 @@ export class Transaction {
14984
} catch (e) {
15085
this.lastWriteError = e;
15186
}
87+
this.writtenDocs.add(key);
15288
}
15389

15490
delete(key: DocumentKey): void {
15591
this.write([new DeleteMutation(key, this.precondition(key))]);
156-
// Since the delete will be applied before all following writes, we need to
157-
// ensure that the precondition for the next write will be exists: false.
158-
this.readVersions = this.readVersions.insert(
159-
key,
160-
SnapshotVersion.forDeletedDoc()
161-
);
92+
this.writtenDocs.add(key);
16293
}
16394

16495
commit(): Promise<void> {
@@ -183,6 +114,84 @@ export class Transaction {
183114
});
184115
}
185116

117+
private recordVersion(doc: MaybeDocument): void {
118+
let docVersion: SnapshotVersion;
119+
120+
if (doc instanceof Document) {
121+
docVersion = doc.version;
122+
} else if (doc instanceof NoDocument) {
123+
// For deleted docs, we must use baseVersion 0 when we overwrite them.
124+
docVersion = SnapshotVersion.forDeletedDoc();
125+
} else {
126+
throw fail('Document in a transaction was a ' + doc.constructor.name);
127+
}
128+
129+
const existingVersion = this.readVersions.get(doc.key);
130+
if (existingVersion !== null) {
131+
if (!docVersion.isEqual(existingVersion)) {
132+
// This transaction will fail no matter what.
133+
throw new FirestoreError(
134+
Code.ABORTED,
135+
'Document version changed between two reads.'
136+
);
137+
}
138+
} else {
139+
this.readVersions = this.readVersions.insert(doc.key, docVersion);
140+
}
141+
}
142+
143+
/**
144+
* Returns the version of this document when it was read in this transaction,
145+
* as a precondition, or no precondition if it was not read.
146+
*/
147+
private precondition(key: DocumentKey): Precondition {
148+
const version = this.readVersions.get(key);
149+
if (!this.writtenDocs.has(key) && version) {
150+
return Precondition.updateTime(version);
151+
} else {
152+
return Precondition.NONE;
153+
}
154+
}
155+
156+
/**
157+
* Returns the precondition for a document if the operation is an update.
158+
*/
159+
private preconditionForUpdate(key: DocumentKey): Precondition {
160+
const version = this.readVersions.get(key);
161+
// The first time a document is written, we want to take into account the
162+
// read time and existence
163+
if (!this.writtenDocs.has(key) && version) {
164+
if (version.isEqual(SnapshotVersion.forDeletedDoc())) {
165+
// The document doesn't exist, so fail the transaction.
166+
167+
// This has to be validated locally because you can't send a
168+
// precondition that a document does not exist without changing the
169+
// semantics of the backend write to be an insert. This is the reverse
170+
// of what we want, since we want to assert that the document doesn't
171+
// exist but then send the update and have it fail. Since we can't
172+
// express that to the backend, we have to validate locally.
173+
174+
// Note: this can change once we can send separate verify writes in the
175+
// transaction.
176+
throw new FirestoreError(
177+
Code.INVALID_ARGUMENT,
178+
"Can't update a document that doesn't exist."
179+
);
180+
}
181+
// Document exists, base precondition on document update time.
182+
return Precondition.updateTime(version);
183+
} else {
184+
// Document was not read, so we just use the preconditions for a blind
185+
// update.
186+
return Precondition.exists(true);
187+
}
188+
}
189+
190+
private write(mutations: Mutation[]): void {
191+
this.ensureCommitNotCalled();
192+
this.mutations = this.mutations.concat(mutations);
193+
}
194+
186195
private ensureCommitNotCalled(): void {
187196
assert(
188197
!this.committed,

0 commit comments

Comments
 (0)