Skip to content

Commit 068ca6f

Browse files
schmidt-sebastianschmidt-sebastian
authored
DeferredValue changes for ServerValue.increment() (#1415)
1 parent 9e69d52 commit 068ca6f

File tree

7 files changed

+169
-49
lines changed

7 files changed

+169
-49
lines changed

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

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,7 +2747,18 @@ public void onComplete(DatabaseError error, boolean committed, DataSnapshot curr
27472747
}
27482748

27492749
@Test
2750-
public void testServerIncrementOverwritesExistingData()
2750+
public void testServerIncrementOverwritesExistingDataOnline()
2751+
throws DatabaseException, TimeoutException, InterruptedException {
2752+
serverIncrementOverwritesExistingData(/* online= */ true);
2753+
}
2754+
2755+
@Test
2756+
public void testServerIncrementOverwritesExistingDataOffline()
2757+
throws DatabaseException, TimeoutException, InterruptedException {
2758+
serverIncrementOverwritesExistingData(/* online= */ false);
2759+
}
2760+
2761+
public void serverIncrementOverwritesExistingData(boolean online)
27512762
throws DatabaseException, TimeoutException, InterruptedException {
27522763
DatabaseConfig cfg = IntegrationTestHelpers.newTestConfig();
27532764
DatabaseReference ref = IntegrationTestHelpers.rootWithConfig(cfg);
@@ -2756,7 +2767,9 @@ public void testServerIncrementOverwritesExistingData()
27562767
List<Object> expectedValues = new ArrayList<>();
27572768

27582769
// Going offline ensures that local events get queued up before server events
2759-
IntegrationTestHelpers.goOffline(cfg);
2770+
if (!online) {
2771+
IntegrationTestHelpers.goOffline(cfg);
2772+
}
27602773

27612774
// Phaser is the closest built-in to a bidrectional latch. We could use a semaphore with a fixed
27622775
// 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) {}
28082821
assertEquals(expectedValues, foundValues);
28092822
} finally {
28102823
ref.removeEventListener(listener);
2811-
IntegrationTestHelpers.goOnline(cfg);
2824+
if (!online) {
2825+
IntegrationTestHelpers.goOnline(cfg);
2826+
}
28122827
}
28132828
}
28142829

28152830
@Test
2816-
public void testServerIncrementPriority()
2831+
public void testServerIncrementPriorityOnline()
2832+
throws DatabaseException, TimeoutException, InterruptedException {
2833+
serverIncrementPriority(/* online= */ true);
2834+
}
2835+
2836+
@Test
2837+
public void testServerIncrementPriorityOffline()
2838+
throws DatabaseException, TimeoutException, InterruptedException {
2839+
serverIncrementPriority(/* online= */ false);
2840+
}
2841+
2842+
public void serverIncrementPriority(boolean online)
28172843
throws DatabaseException, TimeoutException, InterruptedException {
28182844
DatabaseConfig cfg = IntegrationTestHelpers.newTestConfig();
28192845
DatabaseReference ref = IntegrationTestHelpers.rootWithConfig(cfg);
@@ -2822,7 +2848,9 @@ public void testServerIncrementPriority()
28222848
List<Object> expectedPriorities = new ArrayList<>();
28232849

28242850
// Going offline ensures that local events get queued up before server events
2825-
IntegrationTestHelpers.goOffline(cfg);
2851+
if (!online) {
2852+
IntegrationTestHelpers.goOffline(cfg);
2853+
}
28262854

28272855
// Phaser is the closest built-in to a bidrectional latch. We could use a semaphore with a fixed
28282856
// 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) {}
28592887
assertEquals(expectedPriorities, foundPriorities);
28602888
} finally {
28612889
ref.removeEventListener(listener);
2862-
IntegrationTestHelpers.goOnline(cfg);
2890+
if (!online) {
2891+
IntegrationTestHelpers.goOnline(cfg);
2892+
}
28632893
}
28642894
}
28652895

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public class ServerValue {
4545
* @param delta the amount to modify the current value atomically.
4646
* @return a placeholder value for modifying data atomically server-side.
4747
*/
48+
@NonNull
4849
static final Object increment(long delta) {
4950
return createParameterizedServerValuePlaceholder(ServerValues.NAME_OP_INCREMENT, delta);
5051
}
@@ -61,6 +62,7 @@ static final Object increment(long delta) {
6162
* @param delta the amount to modify the current value atomically.
6263
* @return a placeholder value for modifying data atomically server-side.
6364
*/
65+
@NonNull
6466
static final Object increment(double delta) {
6567
return createParameterizedServerValuePlaceholder(ServerValues.NAME_OP_INCREMENT, delta);
6668
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -227,14 +227,14 @@ public void onRequestResult(String optErrorCode, String optErrorMessage) {
227227
}
228228
lastWriteId = write.getWriteId();
229229
nextWriteId = write.getWriteId() + 1;
230-
Node existing = serverSyncTree.calcCompleteEventCache(write.getPath(), new ArrayList<>());
231230
if (write.isOverwrite()) {
232231
if (operationLogger.logsDebug()) {
233232
operationLogger.debug("Restoring overwrite with id " + write.getWriteId());
234233
}
235234
connection.put(write.getPath().asList(), write.getOverwrite().getValue(true), onComplete);
236235
Node resolved =
237-
ServerValues.resolveDeferredValueSnapshot(write.getOverwrite(), existing, serverValues);
236+
ServerValues.resolveDeferredValueSnapshot(
237+
write.getOverwrite(), serverSyncTree, write.getPath(), serverValues);
238238
serverSyncTree.applyUserOverwrite(
239239
write.getPath(),
240240
write.getOverwrite(),
@@ -248,7 +248,8 @@ public void onRequestResult(String optErrorCode, String optErrorMessage) {
248248
}
249249
connection.merge(write.getPath().asList(), write.getMerge().getValue(true), onComplete);
250250
CompoundWrite resolved =
251-
ServerValues.resolveDeferredValueMerge(write.getMerge(), existing, serverValues);
251+
ServerValues.resolveDeferredValueMerge(
252+
write.getMerge(), serverSyncTree, write.getPath(), serverValues);
252253
serverSyncTree.applyUserMerge(
253254
write.getPath(), write.getMerge(), resolved, write.getWriteId(), /*persist=*/ false);
254255
}
@@ -483,9 +484,8 @@ public void updateChildren(
483484

484485
// Start with our existing data and merge each child into it.
485486
Map<String, Object> serverValues = ServerValues.generateServerValues(serverClock);
486-
Node existing = serverSyncTree.calcCompleteEventCache(path, new ArrayList<>());
487487
CompoundWrite resolved =
488-
ServerValues.resolveDeferredValueMerge(updates, existing, serverValues);
488+
ServerValues.resolveDeferredValueMerge(updates, serverSyncTree, path, serverValues);
489489

490490
final long writeId = this.getNextWriteId();
491491
List<? extends Event> events =

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

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414

1515
package com.google.firebase.database.core;
1616

17+
import com.google.firebase.database.core.ValueProvider.DeferredValueProvider;
18+
import com.google.firebase.database.core.ValueProvider.ExistingValueProvider;
1719
import com.google.firebase.database.core.utilities.Clock;
20+
import com.google.firebase.database.core.utilities.Utilities;
1821
import com.google.firebase.database.snapshot.ChildKey;
1922
import com.google.firebase.database.snapshot.ChildrenNode;
2023
import com.google.firebase.database.snapshot.Node;
@@ -34,8 +37,8 @@ public static Map<String, Object> generateServerValues(Clock clock) {
3437
return values;
3538
}
3639

37-
public static Object resolveDeferredValue(
38-
Object value, Node existing, Map<String, Object> serverValues) {
40+
public static Object resolveDeferredLeafValue(
41+
Object value, ValueProvider existing, Map<String, Object> serverValues) {
3942
if (!(value instanceof Map)) {
4043
return value;
4144
}
@@ -47,7 +50,7 @@ public static Object resolveDeferredValue(
4750
Object op = mapValue.get(NAME_SUBKEY_SERVERVALUE);
4851
Object res = null;
4952
if (op instanceof String) {
50-
res = resolveScalarDeferredValue((String) op, existing, serverValues);
53+
res = resolveScalarDeferredValue((String) op, serverValues);
5154
} else if (op instanceof Map) {
5255
res = resolveComplexDeferredValue((Map) op, existing, serverValues);
5356
}
@@ -57,16 +60,15 @@ public static Object resolveDeferredValue(
5760
return res;
5861
}
5962

60-
static Object resolveScalarDeferredValue(
61-
String op, Node existing, Map<String, Object> serverValues) {
63+
static Object resolveScalarDeferredValue(String op, Map<String, Object> serverValues) {
6264
if (NAME_OP_TIMESTAMP.equals(op) && serverValues.containsKey(op)) {
6365
return serverValues.get(op);
6466
}
6567
return null;
6668
}
6769

6870
static Object resolveComplexDeferredValue(
69-
Map<String, Object> op, Node existing, Map<String, Object> serverValues) {
71+
Map<String, Object> op, ValueProvider existing, Map<String, Object> serverValues) {
7072
// Only supported complex op so far
7173
if (!op.containsKey(NAME_OP_INCREMENT)) {
7274
return null;
@@ -80,11 +82,12 @@ static Object resolveComplexDeferredValue(
8082
Number increment = (Number) incrObject;
8183

8284
// Incrementing a non-number sets the value to the incremented amount
83-
if (!(existing.isLeafNode() && existing.getValue() instanceof Number)) {
85+
Node existingNode = existing.node();
86+
if (!(existingNode.isLeafNode() && existingNode.getValue() instanceof Number)) {
8487
return increment;
8588
}
8689

87-
Number existingVal = (Number) existing.getValue();
90+
Number existingVal = (Number) existingNode.getValue();
8891
if (canBeRepresentedAsLong(increment) && canBeRepresentedAsLong(existingVal)) {
8992
long x = increment.longValue();
9093
long y = existingVal.longValue();
@@ -99,32 +102,29 @@ static Object resolveComplexDeferredValue(
99102
return increment.doubleValue() + existingVal.doubleValue();
100103
}
101104

102-
public static SparseSnapshotTree resolveDeferredValueTree(
103-
SparseSnapshotTree tree, Node existing, final Map<String, Object> serverValues) {
104-
final SparseSnapshotTree resolvedTree = new SparseSnapshotTree();
105-
tree.forEachTree(
106-
new Path(""),
107-
new SparseSnapshotTree.SparseSnapshotTreeVisitor() {
108-
@Override
109-
public void visitTree(Path prefixPath, Node tree) {
110-
resolvedTree.remember(
111-
prefixPath,
112-
resolveDeferredValueSnapshot(tree, existing.getChild(prefixPath), serverValues));
113-
}
114-
});
115-
return resolvedTree;
105+
public static Node resolveDeferredValueSnapshot(
106+
Node data, Node existing, final Map<String, Object> serverValues) {
107+
return resolveDeferredValueSnapshot(data, new ExistingValueProvider(existing), serverValues);
116108
}
117109

118110
public static Node resolveDeferredValueSnapshot(
119-
Node data, Node existing, final Map<String, Object> serverValues) {
120-
Object priorityVal =
121-
resolveDeferredValue(data.getPriority().getValue(), existing.getPriority(), serverValues);
122-
Node priority = PriorityUtilities.parsePriority(priorityVal);
111+
Node data, SyncTree syncTree, Path path, final Map<String, Object> serverValues) {
112+
return resolveDeferredValueSnapshot(
113+
data, new DeferredValueProvider(syncTree, path), serverValues);
114+
}
123115

116+
private static Node resolveDeferredValueSnapshot(
117+
Node data, ValueProvider existing, final Map<String, Object> serverValues) {
118+
Object rawPriority = data.getPriority().getValue();
119+
Object priority =
120+
resolveDeferredLeafValue(
121+
rawPriority,
122+
existing.getImmediateChild(ChildKey.fromString(".priority")),
123+
serverValues);
124124
if (data.isLeafNode()) {
125-
Object value = resolveDeferredValue(data.getValue(), existing, serverValues);
126-
if (!value.equals(data.getValue()) || !priority.equals(data.getPriority())) {
127-
return NodeUtilities.NodeFromJSON(value, priority);
125+
Object value = resolveDeferredLeafValue(data.getValue(), existing, serverValues);
126+
if (!value.equals(data.getValue()) || !Utilities.equals(priority, rawPriority)) {
127+
return NodeUtilities.NodeFromJSON(value, PriorityUtilities.parsePriority(priority));
128128
}
129129
return data;
130130
} else if (data.isEmpty()) {
@@ -145,22 +145,22 @@ public void visitChild(ChildKey name, Node child) {
145145
}
146146
});
147147
if (!holder.getRootNode().getPriority().equals(priority)) {
148-
return holder.getRootNode().updatePriority(priority);
148+
return holder.getRootNode().updatePriority(PriorityUtilities.parsePriority(priority));
149149
} else {
150150
return holder.getRootNode();
151151
}
152152
}
153153
}
154154

155155
public static CompoundWrite resolveDeferredValueMerge(
156-
CompoundWrite merge, Node existing, final Map<String, Object> serverValues) {
156+
CompoundWrite merge, SyncTree syncTree, Path path, final Map<String, Object> serverValues) {
157157
CompoundWrite write = CompoundWrite.emptyWrite();
158158
for (Map.Entry<Path, Node> entry : merge) {
159+
ValueProvider deferredValue = new DeferredValueProvider(syncTree, path.child(entry.getKey()));
159160
write =
160161
write.addWrite(
161162
entry.getKey(),
162-
resolveDeferredValueSnapshot(
163-
entry.getValue(), existing.getChild(entry.getKey()), serverValues));
163+
resolveDeferredValueSnapshot(entry.getValue(), deferredValue, serverValues));
164164
}
165165
return write;
166166
}

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,19 +242,16 @@ public List<? extends Event> call() {
242242
boolean needToReevaluate = pendingWriteTree.removeWrite(writeId);
243243
if (write.isVisible()) {
244244
if (!revert) {
245-
ArrayList<Long> excludeThis = new ArrayList<>();
246-
excludeThis.add(write.getWriteId());
247-
Node existing = calcCompleteEventCache(write.getPath(), excludeThis);
248245
Map<String, Object> serverValues = ServerValues.generateServerValues(serverClock);
249246
if (write.isOverwrite()) {
250247
Node resolvedNode =
251248
ServerValues.resolveDeferredValueSnapshot(
252-
write.getOverwrite(), existing, serverValues);
249+
write.getOverwrite(), SyncTree.this, write.getPath(), serverValues);
253250
persistenceManager.applyUserWriteToServerCache(write.getPath(), resolvedNode);
254251
} else {
255252
CompoundWrite resolvedMerge =
256253
ServerValues.resolveDeferredValueMerge(
257-
write.getMerge(), existing, serverValues);
254+
write.getMerge(), SyncTree.this, write.getPath(), serverValues);
258255
persistenceManager.applyUserWriteToServerCache(write.getPath(), resolvedMerge);
259256
}
260257
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright 2020 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.database.core;
16+
17+
import com.google.firebase.database.snapshot.ChildKey;
18+
import com.google.firebase.database.snapshot.Node;
19+
import java.util.ArrayList;
20+
21+
/**
22+
* A ValueProvider defers the calculation of a Node's value until needed.
23+
*
24+
* <p>It's critical for performance that we do not calculate actual values from a SyncTree unless
25+
* and until the value is needed. Because we expose both a SyncTree and Node version of deferred
26+
* value resolution, we ned a wrapper class that will let us share code {@see
27+
* https://github.com/firebase/firebase-js-sdk/issues/2487}.
28+
*/
29+
abstract class ValueProvider {
30+
public abstract ValueProvider getImmediateChild(ChildKey childKey);
31+
32+
public abstract Node node();
33+
34+
/**
35+
* An ExistingValueProvider implements the ValueProvider interface for a Node whose value is
36+
* known.
37+
*/
38+
public static class ExistingValueProvider extends ValueProvider {
39+
private final Node node;
40+
41+
ExistingValueProvider(Node node) {
42+
this.node = node;
43+
}
44+
45+
@Override
46+
public ValueProvider getImmediateChild(ChildKey childKey) {
47+
Node child = node.getImmediateChild(childKey);
48+
return new ExistingValueProvider(child);
49+
}
50+
51+
@Override
52+
public Node node() {
53+
return node;
54+
}
55+
}
56+
57+
/** A DeferredValueProvider computes the value of a Node only when {@link #node()} is invoked. */
58+
public static class DeferredValueProvider extends ValueProvider {
59+
60+
private final SyncTree syncTree;
61+
private final Path path;
62+
63+
DeferredValueProvider(SyncTree syncTree, Path path) {
64+
this.syncTree = syncTree;
65+
this.path = path;
66+
}
67+
68+
@Override
69+
public ValueProvider getImmediateChild(ChildKey childKey) {
70+
Path child = path.child(childKey);
71+
return new DeferredValueProvider(syncTree, child);
72+
}
73+
74+
@Override
75+
public Node node() {
76+
return syncTree.calcCompleteEventCache(path, new ArrayList<>());
77+
}
78+
}
79+
}

0 commit comments

Comments
 (0)