Skip to content

Fix preconditions in transactions #1980

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 13, 2019
Merged

Fix preconditions in transactions #1980

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ca2eef9
Implement general and exact precondition checking, reorder public/pri…
Jul 11, 2019
84278c8
resolve gil's first pass
Jul 11, 2019
4b062d4
Add builder and runner for tests to increase readability
Jul 12, 2019
6d56343
[AUTOMATED]: Prettier Code Styling
Jul 12, 2019
afdf402
resolve 2nd round of comments
Jul 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 89 additions & 80 deletions packages/firestore/src/core/transaction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,33 +42,15 @@ export class Transaction {
*/
private lastWriteError: FirestoreError;

constructor(private datastore: Datastore) {}

private recordVersion(doc: MaybeDocument): void {
let docVersion: SnapshotVersion;

if (doc instanceof Document) {
docVersion = doc.version;
} else if (doc instanceof NoDocument) {
// For deleted docs, we must use baseVersion 0 when we overwrite them.
docVersion = SnapshotVersion.forDeletedDoc();
} else {
throw fail('Document in a transaction was a ' + doc.constructor.name);
}
/**
* Set of documents that have been written in the transaction.
*
* When there's more than one write to the same key in a transaction, any
* writes after hte first are handled differently.
*/
private writtenDocs: Set<DocumentKey> = new Set();

const existingVersion = this.readVersions.get(doc.key);
if (existingVersion !== null) {
if (!docVersion.isEqual(existingVersion)) {
// This transaction will fail no matter what.
throw new FirestoreError(
Code.ABORTED,
'Document version changed between two reads.'
);
}
} else {
this.readVersions = this.readVersions.insert(doc.key, docVersion);
}
}
constructor(private datastore: Datastore) {}

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

private write(mutations: Mutation[]): void {
this.ensureCommitNotCalled();
this.mutations = this.mutations.concat(mutations);
}

/**
* Returns the version of this document when it was read in this transaction,
* as a precondition, or no precondition if it was not read.
*/
private precondition(key: DocumentKey): Precondition {
const version = this.readVersions.get(key);
if (version) {
return Precondition.updateTime(version);
} else {
return Precondition.NONE;
}
}

/**
* Returns the precondition for a document if the operation is an update.
*/
private preconditionForUpdate(key: DocumentKey): Precondition {
const version = this.readVersions.get(key);
if (version && version.isEqual(SnapshotVersion.forDeletedDoc())) {
// The document doesn't exist, so fail the transaction.

// This has to be validated locally because you can't send a precondition
// that a document does not exist without changing the semantics of the
// backend write to be an insert. This is the reverse of what we want,
// since we want to assert that the document doesn't exist but then send
// the update and have it fail. Since we can't express that to the backend,
// we have to validate locally.

// Note: this can change once we can send separate verify writes in the transaction.
throw new FirestoreError(
Code.INVALID_ARGUMENT,
"Can't update a document that doesn't exist."
);
} else if (version) {
// Document exists, base precondition on document update time.
return Precondition.updateTime(version);
} else {
// Document was not read, so we just use the preconditions for a blind
// update.
return Precondition.exists(true);
}
}

set(key: DocumentKey, data: ParsedSetData): void {
this.write(data.toMutations(key, this.precondition(key)));
this.writtenDocs.add(key);
}

update(key: DocumentKey, data: ParsedUpdateData): void {
Expand All @@ -149,16 +84,12 @@ export class Transaction {
} catch (e) {
this.lastWriteError = e;
}
this.writtenDocs.add(key);
}

delete(key: DocumentKey): void {
this.write([new DeleteMutation(key, this.precondition(key))]);
// Since the delete will be applied before all following writes, we need to
// ensure that the precondition for the next write will be exists: false.
this.readVersions = this.readVersions.insert(
key,
SnapshotVersion.forDeletedDoc()
);
this.writtenDocs.add(key);
}

commit(): Promise<void> {
Expand All @@ -183,6 +114,84 @@ export class Transaction {
});
}

private recordVersion(doc: MaybeDocument): void {
let docVersion: SnapshotVersion;

if (doc instanceof Document) {
docVersion = doc.version;
} else if (doc instanceof NoDocument) {
// For deleted docs, we must use baseVersion 0 when we overwrite them.
docVersion = SnapshotVersion.forDeletedDoc();
} else {
throw fail('Document in a transaction was a ' + doc.constructor.name);
}

const existingVersion = this.readVersions.get(doc.key);
if (existingVersion !== null) {
if (!docVersion.isEqual(existingVersion)) {
// This transaction will fail no matter what.
throw new FirestoreError(
Code.ABORTED,
'Document version changed between two reads.'
);
}
} else {
this.readVersions = this.readVersions.insert(doc.key, docVersion);
}
}

/**
* Returns the version of this document when it was read in this transaction,
* as a precondition, or no precondition if it was not read.
*/
private precondition(key: DocumentKey): Precondition {
const version = this.readVersions.get(key);
if (!this.writtenDocs.has(key) && version) {
return Precondition.updateTime(version);
} else {
return Precondition.NONE;
}
}

/**
* Returns the precondition for a document if the operation is an update.
*/
private preconditionForUpdate(key: DocumentKey): Precondition {
const version = this.readVersions.get(key);
// The first time a document is written, we want to take into account the
// read time and existence
if (!this.writtenDocs.has(key) && version) {
if (version.isEqual(SnapshotVersion.forDeletedDoc())) {
// The document doesn't exist, so fail the transaction.

// This has to be validated locally because you can't send a
// precondition that a document does not exist without changing the
// semantics of the backend write to be an insert. This is the reverse
// of what we want, since we want to assert that the document doesn't
// exist but then send the update and have it fail. Since we can't
// express that to the backend, we have to validate locally.

// Note: this can change once we can send separate verify writes in the
// transaction.
throw new FirestoreError(
Code.INVALID_ARGUMENT,
"Can't update a document that doesn't exist."
);
}
// Document exists, base precondition on document update time.
return Precondition.updateTime(version);
} else {
// Document was not read, so we just use the preconditions for a blind
// update.
return Precondition.exists(true);
}
}

private write(mutations: Mutation[]): void {
this.ensureCommitNotCalled();
this.mutations = this.mutations.concat(mutations);
}

private ensureCommitNotCalled(): void {
assert(
!this.committed,
Expand Down
Loading