Simplify Mutations & small bug fixes (firebase#2577)

Author: schmidt-sebastian

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java

@@ -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
@@ -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
@@ -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);
  });
  }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/model/DatabaseId.java

@@ -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",

firebase-firestore/src/main/java/com/google/firebase/firestore/model/DocumentKey.java

@@ -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"),

firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java

@@ -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;
  }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/Mutation.java

@@ -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)));
@@ -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));
  }

firebase-firestore/src/main/java/com/google/firebase/firestore/model/mutation/MutationBatch.java

@@ -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).
@@ -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