DeferredValue changes for ServerValue.increment()

firebase-database/src/androidTest/java/com/google/firebase/database/DataTest.java

@@ -2747,7 +2747,18 @@ public void onComplete(DatabaseError error, boolean committed, DataSnapshot curr
   }
 
   @Test
-  public void testServerIncrementOverwritesExistingData()
+  public void testServerIncrementOverwritesExistingDataOnline()
+      throws DatabaseException, TimeoutException, InterruptedException {
+    serverIncrementOverwritesExistingData(/* online= */ true);
+  }
+
+  @Test
+  public void testServerIncrementOverwritesExistingDataOffline()
+      throws DatabaseException, TimeoutException, InterruptedException {
+    serverIncrementOverwritesExistingData(/* online= */ false);
+  }
+
+  public void serverIncrementOverwritesExistingData(boolean online)
       throws DatabaseException, TimeoutException, InterruptedException {
     DatabaseConfig cfg = IntegrationTestHelpers.newTestConfig();
     DatabaseReference ref = IntegrationTestHelpers.rootWithConfig(cfg);
@@ -2756,7 +2767,9 @@ public void testServerIncrementOverwritesExistingData()
     List<Object> expectedValues = new ArrayList<>();
 
     // Going offline ensures that local events get queued up before server events
-    IntegrationTestHelpers.goOffline(cfg);
+    if (!online) {
+      IntegrationTestHelpers.goOffline(cfg);
+    }
 
     // Phaser is the closest built-in to a bidrectional latch. We could use a semaphore with a fixed
     // number of permits, but the test would be fragile since the permit count isn't closely related
@@ -2808,12 +2821,25 @@ public void onCancelled(DatabaseError error) {}
       assertEquals(expectedValues, foundValues);
     } finally {
       ref.removeEventListener(listener);
-      IntegrationTestHelpers.goOnline(cfg);
+      if (!online) {
+        IntegrationTestHelpers.goOnline(cfg);
+      }
     }
   }
 
   @Test
-  public void testServerIncrementPriority()
+  public void testServerIncrementPriorityOnline()
+      throws DatabaseException, TimeoutException, InterruptedException {
+    serverIncrementPriority(/* online= */ true);
+  }
+
+  @Test
+  public void testServerIncrementPriorityOffline()
+      throws DatabaseException, TimeoutException, InterruptedException {
+    serverIncrementPriority(/* online= */ false);
+  }
+
+  public void serverIncrementPriority(boolean online)
       throws DatabaseException, TimeoutException, InterruptedException {
     DatabaseConfig cfg = IntegrationTestHelpers.newTestConfig();
     DatabaseReference ref = IntegrationTestHelpers.rootWithConfig(cfg);
@@ -2822,7 +2848,9 @@ public void testServerIncrementPriority()
     List<Object> expectedPriorities = new ArrayList<>();
 
     // Going offline ensures that local events get queued up before server events
-    IntegrationTestHelpers.goOffline(cfg);
+    if (!online) {
+      IntegrationTestHelpers.goOffline(cfg);
+    }
 
     // Phaser is the closest built-in to a bidrectional latch. We could use a semaphore with a fixed
     // number of permits, but the test would be fragile since the permit count isn't closely related
@@ -2859,7 +2887,9 @@ public void onCancelled(DatabaseError error) {}
       assertEquals(expectedPriorities, foundPriorities);
     } finally {
       ref.removeEventListener(listener);
-      IntegrationTestHelpers.goOnline(cfg);
+      if (!online) {
+        IntegrationTestHelpers.goOnline(cfg);
+      }
     }
   }
 

firebase-database/src/main/java/com/google/firebase/database/ServerValue.java

@@ -45,6 +45,7 @@ public class ServerValue {
    * @param delta the amount to modify the current value atomically.
    * @return a placeholder value for modifying data atomically server-side.
    */
+  @NonNull
   static final Object increment(long delta) {
     return createParameterizedServerValuePlaceholder(ServerValues.NAME_OP_INCREMENT, delta);
   }
@@ -61,6 +62,7 @@ static final Object increment(long delta) {
    * @param delta the amount to modify the current value atomically.
    * @return a placeholder value for modifying data atomically server-side.
    */
+  @NonNull
   static final Object increment(double delta) {
     return createParameterizedServerValuePlaceholder(ServerValues.NAME_OP_INCREMENT, delta);
   }

firebase-database/src/main/java/com/google/firebase/database/core/Repo.java

@@ -227,14 +227,14 @@ public void onRequestResult(String optErrorCode, String optErrorMessage) {
       }
       lastWriteId = write.getWriteId();
       nextWriteId = write.getWriteId() + 1;
-      Node existing = serverSyncTree.calcCompleteEventCache(write.getPath(), new ArrayList<>());
       if (write.isOverwrite()) {
         if (operationLogger.logsDebug()) {
           operationLogger.debug("Restoring overwrite with id " + write.getWriteId());
         }
         connection.put(write.getPath().asList(), write.getOverwrite().getValue(true), onComplete);
         Node resolved =
-            ServerValues.resolveDeferredValueSnapshot(write.getOverwrite(), existing, serverValues);
+            ServerValues.resolveDeferredValueSnapshot(
+                write.getOverwrite(), serverSyncTree, write.getPath(), serverValues);
         serverSyncTree.applyUserOverwrite(
             write.getPath(),
             write.getOverwrite(),
@@ -248,7 +248,8 @@ public void onRequestResult(String optErrorCode, String optErrorMessage) {
         }
         connection.merge(write.getPath().asList(), write.getMerge().getValue(true), onComplete);
         CompoundWrite resolved =
-            ServerValues.resolveDeferredValueMerge(write.getMerge(), existing, serverValues);
+            ServerValues.resolveDeferredValueMerge(
+                write.getMerge(), serverSyncTree, write.getPath(), serverValues);
         serverSyncTree.applyUserMerge(
             write.getPath(), write.getMerge(), resolved, write.getWriteId(), /*persist=*/ false);
       }
@@ -483,9 +484,8 @@ public void updateChildren(
 
     // Start with our existing data and merge each child into it.
     Map<String, Object> serverValues = ServerValues.generateServerValues(serverClock);
-    Node existing = serverSyncTree.calcCompleteEventCache(path, new ArrayList<>());
     CompoundWrite resolved =
-        ServerValues.resolveDeferredValueMerge(updates, existing, serverValues);
+        ServerValues.resolveDeferredValueMerge(updates, serverSyncTree, path, serverValues);
 
     final long writeId = this.getNextWriteId();
     List<? extends Event> events =

firebase-database/src/main/java/com/google/firebase/database/core/ServerValues.java

@@ -14,7 +14,10 @@
 
 package com.google.firebase.database.core;
 
+import com.google.firebase.database.core.ValueProvider.DeferredValueProvider;
+import com.google.firebase.database.core.ValueProvider.ExistingValueProvider;
 import com.google.firebase.database.core.utilities.Clock;
+import com.google.firebase.database.core.utilities.Utilities;
 import com.google.firebase.database.snapshot.ChildKey;
 import com.google.firebase.database.snapshot.ChildrenNode;
 import com.google.firebase.database.snapshot.Node;
@@ -34,8 +37,8 @@ public static Map<String, Object> generateServerValues(Clock clock) {
     return values;
   }
 
-  public static Object resolveDeferredValue(
-      Object value, Node existing, Map<String, Object> serverValues) {
+  public static Object resolveDeferredLeafValue(
+      Object value, ValueProvider existing, Map<String, Object> serverValues) {
     if (!(value instanceof Map)) {
       return value;
     }
@@ -47,7 +50,7 @@ public static Object resolveDeferredValue(
     Object op = mapValue.get(NAME_SUBKEY_SERVERVALUE);
     Object res = null;
     if (op instanceof String) {
-      res = resolveScalarDeferredValue((String) op, existing, serverValues);
+      res = resolveScalarDeferredValue((String) op, serverValues);
     } else if (op instanceof Map) {
       res = resolveComplexDeferredValue((Map) op, existing, serverValues);
     }
@@ -57,16 +60,15 @@ public static Object resolveDeferredValue(
     return res;
   }
 
-  static Object resolveScalarDeferredValue(
-      String op, Node existing, Map<String, Object> serverValues) {
+  static Object resolveScalarDeferredValue(String op, Map<String, Object> serverValues) {
     if (NAME_OP_TIMESTAMP.equals(op) && serverValues.containsKey(op)) {
       return serverValues.get(op);
     }
     return null;
   }
 
   static Object resolveComplexDeferredValue(
-      Map<String, Object> op, Node existing, Map<String, Object> serverValues) {
+      Map<String, Object> op, ValueProvider existing, Map<String, Object> serverValues) {
     // Only supported complex op so far
     if (!op.containsKey(NAME_OP_INCREMENT)) {
       return null;
@@ -80,11 +82,12 @@ static Object resolveComplexDeferredValue(
     Number increment = (Number) incrObject;
 
     // Incrementing a non-number sets the value to the incremented amount
-    if (!(existing.isLeafNode() && existing.getValue() instanceof Number)) {
+    Node existingNode = existing.node();
+    if (!(existingNode.isLeafNode() && existingNode.getValue() instanceof Number)) {
       return increment;
     }
 
-    Number existingVal = (Number) existing.getValue();
+    Number existingVal = (Number) existingNode.getValue();
     if (canBeRepresentedAsLong(increment) && canBeRepresentedAsLong(existingVal)) {
       long x = increment.longValue();
       long y = existingVal.longValue();
@@ -99,32 +102,29 @@ static Object resolveComplexDeferredValue(
     return increment.doubleValue() + existingVal.doubleValue();
   }
 
-  public static SparseSnapshotTree resolveDeferredValueTree(
-      SparseSnapshotTree tree, Node existing, final Map<String, Object> serverValues) {
-    final SparseSnapshotTree resolvedTree = new SparseSnapshotTree();
-    tree.forEachTree(
-        new Path(""),
-        new SparseSnapshotTree.SparseSnapshotTreeVisitor() {
-          @Override
-          public void visitTree(Path prefixPath, Node tree) {
-            resolvedTree.remember(
-                prefixPath,
-                resolveDeferredValueSnapshot(tree, existing.getChild(prefixPath), serverValues));
-          }
-        });
-    return resolvedTree;
+  public static Node resolveDeferredValueSnapshot(
+      Node data, Node existing, final Map<String, Object> serverValues) {
+    return resolveDeferredValueSnapshot(data, new ExistingValueProvider(existing), serverValues);
   }
 
   public static Node resolveDeferredValueSnapshot(
-      Node data, Node existing, final Map<String, Object> serverValues) {
-    Object priorityVal =
-        resolveDeferredValue(data.getPriority().getValue(), existing.getPriority(), serverValues);
-    Node priority = PriorityUtilities.parsePriority(priorityVal);
+      Node data, SyncTree syncTree, Path path, final Map<String, Object> serverValues) {
+    return resolveDeferredValueSnapshot(
+        data, new DeferredValueProvider(syncTree, path), serverValues);
+  }
 
+  private static Node resolveDeferredValueSnapshot(
+      Node data, ValueProvider existing, final Map<String, Object> serverValues) {
+    Object rawPriority = data.getPriority().getValue();
+    Object priority =
+        resolveDeferredLeafValue(
+            rawPriority,
+            existing.getImmediateChild(ChildKey.fromString(".priority")),
+            serverValues);
     if (data.isLeafNode()) {
-      Object value = resolveDeferredValue(data.getValue(), existing, serverValues);
-      if (!value.equals(data.getValue()) || !priority.equals(data.getPriority())) {
-        return NodeUtilities.NodeFromJSON(value, priority);
+      Object value = resolveDeferredLeafValue(data.getValue(), existing, serverValues);
+      if (!value.equals(data.getValue()) || !Utilities.equals(priority, rawPriority)) {
+        return NodeUtilities.NodeFromJSON(value, PriorityUtilities.parsePriority(priority));
       }
       return data;
     } else if (data.isEmpty()) {
@@ -145,22 +145,22 @@ public void visitChild(ChildKey name, Node child) {
             }
           });
       if (!holder.getRootNode().getPriority().equals(priority)) {
-        return holder.getRootNode().updatePriority(priority);
+        return holder.getRootNode().updatePriority(PriorityUtilities.parsePriority(priority));
       } else {
         return holder.getRootNode();
       }
     }
   }
 
   public static CompoundWrite resolveDeferredValueMerge(
-      CompoundWrite merge, Node existing, final Map<String, Object> serverValues) {
+      CompoundWrite merge, SyncTree syncTree, Path path, final Map<String, Object> serverValues) {
     CompoundWrite write = CompoundWrite.emptyWrite();
     for (Map.Entry<Path, Node> entry : merge) {
+      ValueProvider deferredValue = new DeferredValueProvider(syncTree, path.child(entry.getKey()));
       write =
           write.addWrite(
               entry.getKey(),
-              resolveDeferredValueSnapshot(
-                  entry.getValue(), existing.getChild(entry.getKey()), serverValues));
+              resolveDeferredValueSnapshot(entry.getValue(), deferredValue, serverValues));
     }
     return write;
   }

firebase-database/src/main/java/com/google/firebase/database/core/SyncTree.java

@@ -242,19 +242,16 @@ public List<? extends Event> call() {
             boolean needToReevaluate = pendingWriteTree.removeWrite(writeId);
             if (write.isVisible()) {
               if (!revert) {
-                ArrayList<Long> excludeThis = new ArrayList<>();
-                excludeThis.add(write.getWriteId());
-                Node existing = calcCompleteEventCache(write.getPath(), excludeThis);
                 Map<String, Object> serverValues = ServerValues.generateServerValues(serverClock);
                 if (write.isOverwrite()) {
                   Node resolvedNode =
                       ServerValues.resolveDeferredValueSnapshot(
-                          write.getOverwrite(), existing, serverValues);
+                          write.getOverwrite(), SyncTree.this, write.getPath(), serverValues);
                   persistenceManager.applyUserWriteToServerCache(write.getPath(), resolvedNode);
                 } else {
                   CompoundWrite resolvedMerge =
                       ServerValues.resolveDeferredValueMerge(
-                          write.getMerge(), existing, serverValues);
+                          write.getMerge(), SyncTree.this, write.getPath(), serverValues);
                   persistenceManager.applyUserWriteToServerCache(write.getPath(), resolvedMerge);
                 }
               }

firebase-database/src/main/java/com/google/firebase/database/core/ValueProvider.java

@@ -0,0 +1,79 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.firebase.database.core;
+
+import com.google.firebase.database.snapshot.ChildKey;
+import com.google.firebase.database.snapshot.Node;
+import java.util.ArrayList;
+
+/**
+ * A ValueProvider defers the calculation of a Node's value until needed.
+ *
+ * <p>It's critical for performance that we do not calculate actual values from a SyncTree unless
+ * and until the value is needed. Because we expose both a SyncTree and Node version of deferred
+ * value resolution, we ned a wrapper class that will let us share code {@see
+ * https://github.com/firebase/firebase-js-sdk/issues/2487}.
+ */
+abstract class ValueProvider {
+  public abstract ValueProvider getImmediateChild(ChildKey childKey);
+
+  public abstract Node node();
+
+  /**
+   * An ExistingValueProvider implements the ValueProvider interface for a Node whose value is
+   * known.
+   */
+  public static class ExistingValueProvider extends ValueProvider {
+    private final Node node;
+
+    ExistingValueProvider(Node node) {
+      this.node = node;
+    }
+
+    @Override
+    public ValueProvider getImmediateChild(ChildKey childKey) {
+      Node child = node.getImmediateChild(childKey);
+      return new ExistingValueProvider(child);
+    }
+
+    @Override
+    public Node node() {
+      return node;
+    }
+  }
+
+  /** A DeferredValueProvider computes the value of a Node only when {@link #node()} is invoked. */
+  public static class DeferredValueProvider extends ValueProvider {
+
+    private final SyncTree syncTree;
+    private final Path path;
+
+    DeferredValueProvider(SyncTree syncTree, Path path) {
+      this.syncTree = syncTree;
+      this.path = path;
+    }
+
+    @Override
+    public ValueProvider getImmediateChild(ChildKey childKey) {
+      Path child = path.child(childKey);
+      return new DeferredValueProvider(syncTree, child);
+    }
+
+    @Override
+    public Node node() {
+      return syncTree.calcCompleteEventCache(path, new ArrayList<>());
+    }
+  }
+}