Renames and comments

Author: schmidt-sebastian

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

@@ -18,7 +18,7 @@
 import com.google.firestore.v1.Value;
 import java.util.Comparator;
 
-public class MutableDocument implements Cloneable {
+public final class MutableDocument implements Cloneable {
   private static final Comparator<MutableDocument> KEY_COMPARATOR =
       (left, right) -> left.getKey().compareTo(right.getKey());
 
@@ -27,81 +27,99 @@ public static Comparator<MutableDocument> keyComparator() {
     return KEY_COMPARATOR;
   }
 
-  private enum Type {
+  private enum DocumentType {
+    /**
+     * Represents the initial state of a MutableDocument when only the document key is known.
+     * Invalid documents transition to other states as mutations are applied. If a document remain
+     * invalids after applying mutations, it should be discarded.
+     */
     INVALID,
+    /**
+     * Represents a document in Firestore with a key, version, data and whether the data has local
+     * mutations applied to it.
+     */
     FOUND_DOCUMENT,
+    /** Represents that no documents exists for the key at the given version. */
     NO_DOCUMENT,
+    /**
+     * Represents an existing document whose data is unknown (e.g. a document that was updated
+     * without a known base document).
+     */
     UNKNOWN_DOCUMENT;
   }
 
+  /** Describes the `hasPendingWrites` state of a document. */
+  private enum DocumentState {
+    /** Local mutations applied via the mutation queue. Document is potentially inconsistent. */
+    LOCAL_MUTATIONS,
+    /** Mutations applied based on a write acknowledgment. Document is potentially inconsistent. */
+    COMMITTED_MUTATIONS,
+    /** No mutations applied. Document was sent to us by Watch. */
+    SYNCED
+  }
+
   private final DocumentKey key;
-  private Type type;
+  private DocumentType documentType;
   private SnapshotVersion version;
   private ObjectValue value;
-  boolean hasLocalMutations;
-  boolean hasCommittedMutations;
+  private DocumentState documentState;
 
   public MutableDocument(DocumentKey key) {
-    this.key = key;
-    this.version = SnapshotVersion.NONE;
-    this.type = Type.INVALID;
-    this.value = new ObjectValue();
+    this(key, DocumentType.INVALID, SnapshotVersion.NONE, new ObjectValue(), DocumentState.SYNCED);
   }
 
   private MutableDocument(
       DocumentKey key,
-      Type type,
+      DocumentType documentType,
       SnapshotVersion version,
       ObjectValue value,
-      boolean hasLocalMutations,
-      boolean hasCommittedMutations) {
+      DocumentState documentState) {
     this.key = key;
     this.version = version;
-    this.type = type;
-    this.hasLocalMutations = hasLocalMutations;
-    this.hasCommittedMutations = hasCommittedMutations;
+    this.documentType = documentType;
+    this.documentState = documentState;
     this.value = value;
   }
 
-  /** Changes the document type to FOUND_DOCUMENT and sets the given version and data. */
+  /**
+   * Changes the document type to indicate that it exists and that its version and data are known.
+   */
   public MutableDocument setFoundDocument(SnapshotVersion version, ObjectValue value) {
     this.version = version;
-    this.type = Type.FOUND_DOCUMENT;
+    this.documentType = DocumentType.FOUND_DOCUMENT;
     this.value = value;
-    this.hasLocalMutations = false;
-    this.hasCommittedMutations = false;
+    this.documentState = DocumentState.SYNCED;
     return this;
   }
 
-  /** Changes the document type to NO_DOCUMENT and sets the given version. */
+  /** Changes the document type to indicate that it doesn't exist at the given version. */
   public MutableDocument setNoDocument(SnapshotVersion version) {
     this.version = version;
-    this.type = Type.NO_DOCUMENT;
+    this.documentType = DocumentType.NO_DOCUMENT;
     this.value = new ObjectValue();
-    this.hasLocalMutations = false;
-    this.hasCommittedMutations = false;
+    this.documentState = DocumentState.SYNCED;
     return this;
   }
 
-  /** Changes the document type to UNKNOWN_DOCUMENT and sets the given version. */
+  /**
+   * Changes the document type to indicate that it exists at a given version but that is data is not
+   * known (e.g. a document that was updated without a known base document).
+   */
   public MutableDocument setUnknownDocument(SnapshotVersion version) {
     this.version = version;
-    this.type = Type.UNKNOWN_DOCUMENT;
+    this.documentType = DocumentType.UNKNOWN_DOCUMENT;
     this.value = new ObjectValue();
-    this.hasLocalMutations = false;
-    this.hasCommittedMutations = true;
+    this.documentState = DocumentState.COMMITTED_MUTATIONS;
     return this;
   }
 
   public MutableDocument setCommittedMutations() {
-    this.hasLocalMutations = false;
-    this.hasCommittedMutations = true;
+    this.documentState = DocumentState.COMMITTED_MUTATIONS;
     return this;
   }
 
   public MutableDocument setLocalMutations() {
-    this.hasLocalMutations = true;
-    this.hasCommittedMutations = true;
+    this.documentState = DocumentState.LOCAL_MUTATIONS;
     return this;
   }
 
@@ -120,12 +138,12 @@ public SnapshotVersion getVersion() {
 
   /** Returns whether local mutations were applied via the mutation queue. */
   public boolean hasLocalMutations() {
-    return hasLocalMutations;
+    return documentState.equals(DocumentState.LOCAL_MUTATIONS);
   }
 
   /** Returns whether mutations were applied based on a write acknowledgment. */
   public boolean hasCommittedMutations() {
-    return hasCommittedMutations;
+    return documentState.equals(DocumentState.COMMITTED_MUTATIONS);
   }
 
   /**
@@ -143,27 +161,33 @@ public Value getField(FieldPath field) {
     return getData().get(field);
   }
 
+  /**
+   * Returns whether this document is valid (i.e. it is an entry in the RemoteDocumentCache, was
+   * created by a mutation or read from the backend).
+   */
   public boolean isValidDocument() {
-    return !type.equals(Type.INVALID);
+    return !documentType.equals(DocumentType.INVALID);
   }
 
+  /** Returns whether the document exists and its data is known at the current version. */
   public boolean isFoundDocument() {
-    return type.equals(Type.FOUND_DOCUMENT);
+    return documentType.equals(DocumentType.FOUND_DOCUMENT);
   }
 
+  /** Returns whether the document is known to not exist at the current version. */
   public boolean isNoDocument() {
-    return type.equals(Type.NO_DOCUMENT);
+    return documentType.equals(DocumentType.NO_DOCUMENT);
   }
 
+  /** Returns whether the document exists and its data is unknown at the current version. */
   public boolean isUnknownDocument() {
-    return type.equals(Type.UNKNOWN_DOCUMENT);
+    return documentType.equals(DocumentType.UNKNOWN_DOCUMENT);
   }
 
   @Override
   @NonNull
   public MutableDocument clone() {
-    return new MutableDocument(
-        key, type, version, value.clone(), hasLocalMutations, hasCommittedMutations);
+    return new MutableDocument(key, documentType, version, value.clone(), documentState);
   }
 
   @Override
@@ -173,11 +197,9 @@ public boolean equals(Object o) {
 
     MutableDocument document = (MutableDocument) o;
 
-    if (hasLocalMutations != document.hasLocalMutations) return false;
-    if (hasCommittedMutations != document.hasCommittedMutations) return false;
     if (!key.equals(document.key)) return false;
     if (!version.equals(document.version)) return false;
-    if (type != document.type) return false;
+    if (documentType != document.documentType) return false;
     return value.equals(document.value);
   }
 
@@ -194,11 +216,9 @@ public String toString() {
         + ", version="
         + version
         + ", type="
-        + type
-        + ", hasLocalMutations="
-        + hasLocalMutations
-        + ", hasCommittedMutations="
-        + hasCommittedMutations
+        + documentType
+        + ", documentState="
+        + documentState
         + ", value="
         + value
         + '}';

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

@@ -27,17 +27,17 @@
 import java.util.Set;
 
 /** A structured object value stored in Firestore. */
-public class ObjectValue implements Cloneable {
+public final class ObjectValue implements Cloneable {
   /**
-   * The immutable Value proto for this object. Local mutations are stored in `overlayMap` and and
-   * only applied when `memoize` is invoked.
+   * The immutable Value proto for this object. Local mutations are stored in `overlayMap` and only
+   * applied when {@link #buildProto()} is invoked.
    */
   private Value partialValue;
 
   /**
-   * A nested map that contains the accumulated changes that haven't yet been applied to
-   * `partialValue`. Values can either be `Value` protos, `Map<String, Object>` values (to represent
-   * additional nesting) or `null` (to represent field deletes).
+   * A nested map that contains the accumulated changes that haven't yet been applied to {@link
+   * #partialValue}. Values can either be {@link Value} protos, {@code Map<String, Object>} values
+   * (to represent additional nesting) or {@code null} (to represent field deletes).
    */
   private Map<String, Object> overlayMap = new HashMap<>();
 
@@ -61,12 +61,12 @@ public ObjectValue() {
   }
 
   public Map<String, Value> getFieldsMap() {
-    return getProto().getMapValue().getFieldsMap();
+    return buildProto().getMapValue().getFieldsMap();
   }
 
   /** Recursively extracts the FieldPaths that are set in this ObjectValue. */
   public FieldMask getFieldMask() {
-    return extractFieldMask(getProto().getMapValue());
+    return extractFieldMask(buildProto().getMapValue());
   }
 
   private FieldMask extractFieldMask(MapValue value) {
@@ -99,11 +99,11 @@ private FieldMask extractFieldMask(MapValue value) {
    * @return The value at the path or null if it doesn't exist.
    */
   public @Nullable Value get(FieldPath fieldPath) {
-    return get(getProto(), fieldPath);
+    return extractNestedValue(buildProto(), fieldPath);
   }
 
   @Nullable
-  Value get(Value value, FieldPath fieldPath) {
+  private Value extractNestedValue(Value value, FieldPath fieldPath) {
     if (fieldPath.isEmpty()) {
       return value;
     } else {
@@ -117,9 +117,18 @@ Value get(Value value, FieldPath fieldPath) {
     }
   }
 
-  /** Returns the Protobuf that backs this ObjectValue. */
-  public Value getProto() {
-    memoize();
+  /**
+   * Returns the Protobuf that backs this ObjectValue.
+   *
+   * <p>This method applies any outstanding modifications and memoizes the result. Further
+   * invocations are based on this memoized result.
+   */
+  private Value buildProto() {
+    MapValue mergedResult = applyOverlay(FieldPath.EMPTY_PATH, overlayMap);
+    if (mergedResult != null) {
+      partialValue = Value.newBuilder().setMapValue(mergedResult).build();
+      overlayMap.clear();
+    }
     return partialValue;
   }
 
@@ -145,7 +154,7 @@ public void set(FieldPath path, Value value) {
     setOverlay(path, value);
   }
 
-  public void set(Map<FieldPath, Value> data) {
+  public void setAll(Map<FieldPath, Value> data) {
     for (Map.Entry<FieldPath, Value> entry : data.entrySet()) {
       FieldPath path = entry.getKey();
       if (entry.getValue() == null) {
@@ -156,7 +165,9 @@ public void set(Map<FieldPath, Value> data) {
     }
   }
 
-  /** Adds `value` to the overlay map at `path`. Creates nested map entries if needed. */
+  /**
+   * Adds {@code value} to the overlay map at {@code path}. Creates nested map entries if needed.
+   */
   private void setOverlay(FieldPath path, @Nullable Value value) {
     Map<String, Object> currentLevel = overlayMap;
 
@@ -185,30 +196,21 @@ private void setOverlay(FieldPath path, @Nullable Value value) {
     currentLevel.put(path.getLastSegment(), value);
   }
 
-  /** Returns an ObjectValue with all mutations applied. */
-  void memoize() {
-    MapValue mergedResult = applyOverlay(FieldPath.EMPTY_PATH, overlayMap);
-    if (mergedResult != null) {
-      partialValue = Value.newBuilder().setMapValue(mergedResult).build();
-      overlayMap.clear();
-    }
-  }
-
   /**
-   * Applies any overlays from `currentOverlays` that exist at `currentPath` and returns the merged
-   * data at `currentPath` (or null if there were no changes).
+   * Applies any overlays from {@code currentOverlays} that exist at `currentPath` and returns the
+   * merged data at {@code currentPath} (or {@code null} if there were no changes).
    *
-   * @param currentPath The path at the current nesting level. Can be set toFieldValue.EMPTY_PATH to
-   *     represent the root.
-   * @param currentOverlays The overlays at the current nesting level in the same format as
-   *     `overlayMap`.
+   * @param currentPath The path at the current nesting level. Can be set to {@code
+   *     FieldValue.EMPTY_PATH} to represent the root.
+   * @param currentOverlays The overlays at the current nesting level in the same format as {@code
+   *     overlayMap}.
    * @return The merged data at `currentPath` or null if no modifications were applied.
    */
   private @Nullable MapValue applyOverlay(
       FieldPath currentPath, Map<String, Object> currentOverlays) {
     boolean modified = false;
 
-    @Nullable Value existingValue = get(partialValue, currentPath);
+    @Nullable Value existingValue = extractNestedValue(partialValue, currentPath);
     MapValue.Builder resultAtPath =
         Values.isMapValue(existingValue)
             // If there is already data at the current path, base our modifications on top
@@ -246,24 +248,24 @@ public boolean equals(Object o) {
     if (this == o) {
       return true;
     } else if (o instanceof ObjectValue) {
-      return Values.equals(getProto(), ((ObjectValue) o).getProto());
+      return Values.equals(buildProto(), ((ObjectValue) o).buildProto());
     }
     return false;
   }
 
   @Override
   public int hashCode() {
-    return getProto().hashCode();
+    return buildProto().hashCode();
   }
 
   @Override
   @NonNull
   public String toString() {
-    return "ObjectValue{" + "internalValue=" + getProto() + '}';
+    return "ObjectValue{" + "internalValue=" + buildProto() + '}';
   }
 
   @NonNull
   public ObjectValue clone() {
-    return new ObjectValue(getProto());
+    return new ObjectValue(buildProto());
   }
 }