Skip to content
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

chore: remove internal WriteOp #1067

Merged
merged 4 commits into from
May 1, 2020
Merged

chore: remove internal WriteOp #1067

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
40cc1bd
chore: remove internal WriteOp
schmidt-sebastian May 1, 2020
5fdbc8c
Merge
schmidt-sebastian May 1, 2020
3e2d0e0
s/update/write
schmidt-sebastian May 1, 2020
19a6ac1
Add comment
schmidt-sebastian May 1, 2020
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
76 changes: 24 additions & 52 deletions dev/src/write-batch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,7 @@ export class BatchWriteResult {
) {}
}

/** Helper type to manage the list of writes in a WriteBatch. */
// TODO(mrschmidt): Replace with api.IWrite
interface WriteOp {
write: api.IWrite;
precondition?: api.IPrecondition | null;
}

export type PendingWriteOp = () => WriteOp;
export type PendingWriteOp = () => api.IWrite;

/**
* A Firestore WriteBatch that can be used to atomically commit multiple write
Expand Down Expand Up @@ -210,11 +203,8 @@ export class WriteBatch {
if (!transform.isEmpty) {
write.updateTransforms = transform.toProto(this._serializer);
}

return {
write,
precondition: precondition.toProto(),
};
write.currentDocument = precondition.toProto();
return write;
};

this._ops.push(op);
Expand Down Expand Up @@ -257,12 +247,11 @@ export class WriteBatch {
const conditions = new Precondition(precondition);

const op: PendingWriteOp = () => {
return {
write: {
delete: documentRef.formattedName,
},
precondition: conditions.toProto(),
};
const write: api.IWrite = {delete: documentRef.formattedName};
if (!conditions.isEmpty) {
write.currentDocument = conditions.toProto();
}
return write;
};

this._ops.push(op);
Expand Down Expand Up @@ -337,18 +326,14 @@ export class WriteBatch {
documentMask = DocumentMask.fromObject(firestoreData);
}

const write = document.toProto();
const update = document.toProto();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why this variable is called update, but the other variables are write? They all have type api.IWrite. Consider using one or the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, no reason. Updated to write.

if (!transform.isEmpty) {
write.updateTransforms = transform.toProto(this._serializer);
update.updateTransforms = transform.toProto(this._serializer);
}

if (mergePaths || mergeLeaves) {
write.updateMask = documentMask!.toProto();
update.updateMask = documentMask!.toProto();
}

return {
write,
};
return update;
};

this._ops.push(op);
Expand Down Expand Up @@ -495,14 +480,12 @@ export class WriteBatch {
const op: PendingWriteOp = () => {
const document = DocumentSnapshot.fromUpdateMap(documentRef, updateMap);
const write = document.toProto();
write!.updateMask = documentMask.toProto();
write.updateMask = documentMask.toProto();
if (!transform.isEmpty) {
write!.updateTransforms = transform.toProto(this._serializer);
write.updateTransforms = transform.toProto(this._serializer);
}
return {
write,
precondition: precondition.toProto(),
};
write.currentDocument = precondition.toProto();
return write;
};

this._ops.push(op);
Expand Down Expand Up @@ -546,15 +529,10 @@ export class WriteBatch {
await this._firestore.initializeIfNeeded(tag);

const database = this._firestore.formattedName;
const request: api.IBatchWriteRequest = {database, writes: []};
const writes = this._ops.map(op => op());

for (const req of writes) {
if (req.precondition) {
req.write!.currentDocument = req.precondition;
}
request.writes!.push(req.write);
}
const request: api.IBatchWriteRequest = {
database,
writes: this._ops.map(op => op()),
};

const response = await this._firestore.request<
api.IBatchWriteRequest,
Expand Down Expand Up @@ -610,16 +588,10 @@ export class WriteBatch {
});
}

const request: api.ICommitRequest = {database, writes: []};
const writes = this._ops.map(op => op());

for (const req of writes) {
if (req.precondition) {
req.write!.currentDocument = req.precondition;
}

request.writes!.push(req.write);
}
const request: api.ICommitRequest = {
database,
writes: this._ops.map(op => op()),
};

logger(
'WriteBatch.commit',
Expand Down