Skip to content

Simplify Mutations #2577

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.

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 1 commit into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,7 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {
() -> {
// Load and apply all existing mutations. This lets us compute the current base state for
// all non-idempotent transforms before applying any additional user-provided writes.
ImmutableSortedMap<DocumentKey, Document> existingDocuments =
localDocuments.getDocuments(keys);
ImmutableSortedMap<DocumentKey, Document> documents = localDocuments.getDocuments(keys);

// For non-idempotent mutations (such as `FieldValue.increment()`), we record the base
// state in a separate patch mutation. This is later used to guarantee consistent values
Expand All @@ -226,7 +225,7 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {
List<Mutation> baseMutations = new ArrayList<>();
for (Mutation mutation : mutations) {
ObjectValue baseValue =
mutation.extractTransformBaseValue(existingDocuments.get(mutation.getKey()));
mutation.extractTransformBaseValue(documents.get(mutation.getKey()));
if (baseValue != null) {
// NOTE: The base state should only be applied if there's some existing
// document to override, so use a Precondition of exists=true
Expand All @@ -241,9 +240,8 @@ public LocalWriteResult writeLocally(List<Mutation> mutations) {

MutationBatch batch =
mutationQueue.addMutationBatch(localWriteTime, baseMutations, mutations);
ImmutableSortedMap<DocumentKey, Document> changedDocuments =
batch.applyToLocalDocumentSet(existingDocuments);
return new LocalWriteResult(batch.getBatchId(), changedDocuments);
batch.applyToLocalDocumentSet(documents);
return new LocalWriteResult(batch.getBatchId(), documents);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ private DatabaseId(String projectId, String databaseId) {
public static DatabaseId fromName(String name) {
ResourcePath resourceName = ResourcePath.fromString(name);
hardAssert(
resourceName.length() >= 3
resourceName.length() > 3
&& resourceName.getSegment(0).equals("projects")
&& resourceName.getSegment(2).equals("databases"),
"Tried to parse an invalid resource name: %s",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public static DocumentKey empty() {
public static DocumentKey fromName(String name) {
ResourcePath resourceName = ResourcePath.fromString(name);
hardAssert(
resourceName.length() >= 4
resourceName.length() > 4
&& resourceName.getSegment(0).equals("projects")
&& resourceName.getSegment(2).equals("databases")
&& resourceName.getSegment(4).equals("documents"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,12 +386,12 @@ private static void canonifyArray(StringBuilder builder, ArrayValue arrayValue)
builder.append("]");
}

/** Returns true if `value` is either a INTEGER_VALUE. */
/** Returns true if `value` is a INTEGER_VALUE. */
public static boolean isInteger(@Nullable Value value) {
return value != null && value.getValueTypeCase() == Value.ValueTypeCase.INTEGER_VALUE;
}

/** Returns true if `value` is either a DOUBLE_VALUE. */
/** Returns true if `value` is a DOUBLE_VALUE. */
public static boolean isDouble(@Nullable Value value) {
return value != null && value.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,7 @@ protected Map<FieldPath, Value> serverTransformResults(
for (int i = 0; i < serverTransformResults.size(); i++) {
FieldTransform fieldTransform = fieldTransforms.get(i);
TransformOperation transform = fieldTransform.getOperation();

Value previousValue = null;
if (mutableDocument.isFoundDocument()) {
previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
}

Value previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
transformResults.put(
fieldTransform.getFieldPath(),
transform.applyToRemoteDocument(previousValue, serverTransformResults.get(i)));
Expand All @@ -196,12 +191,7 @@ protected Map<FieldPath, Value> localTransformResults(
Map<FieldPath, Value> transformResults = new HashMap<>(fieldTransforms.size());
for (FieldTransform fieldTransform : fieldTransforms) {
TransformOperation transform = fieldTransform.getOperation();

Value previousValue = null;
if (mutableDocument.isFoundDocument()) {
previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
}

Value previousValue = mutableDocument.getField(fieldTransform.getFieldPath());
transformResults.put(
fieldTransform.getFieldPath(), transform.applyToLocalView(previousValue, localWriteTime));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ public void applyToLocalView(MutableDocument document) {
}

/** Computes the local view for all provided documents given the mutations in this batch. */
public ImmutableSortedMap<DocumentKey, Document> applyToLocalDocumentSet(
ImmutableSortedMap<DocumentKey, Document> documentMap) {
public void applyToLocalDocumentSet(ImmutableSortedMap<DocumentKey, Document> documentMap) {
// TODO(mrschmidt): This implementation is O(n^2). If we iterate through the mutations first
// (as done in `applyToLocalView(MutableDocument d)`), we can reduce the complexity to
// O(n).
Expand All @@ -131,9 +130,7 @@ public ImmutableSortedMap<DocumentKey, Document> applyToLocalDocumentSet(
if (!document.isValidDocument()) {
document.convertToNoDocument(SnapshotVersion.NONE);
}
documentMap = documentMap.insert(document.getKey(), document);
}
return documentMap;
}

@Override
Expand Down