Skip to content

Commit f59bc07

Browse files
schmidt-sebastianinlined
authored andcommitted
Revert "Revert: Add Database.Servervalue.increment(x) (#2500)" (#2505)
This reverts commit 8504da9.
1 parent ff7be4b commit f59bc07

File tree

6 files changed

+188
-17
lines changed

6 files changed

+188
-17
lines changed

packages/database/src/api/Database.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ export class Database implements FirebaseService {
3939
static readonly ServerValue = {
4040
TIMESTAMP: {
4141
'.sv': 'timestamp'
42+
},
43+
_increment: (x: number) => {
44+
return {
45+
'.sv': {
46+
'increment': x
47+
}
48+
};
4249
}
4350
};
4451

packages/database/src/core/Repo.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,8 +313,10 @@ export class Repo {
313313
// (b) store unresolved paths on JSON parse
314314
const serverValues = this.generateServerValues();
315315
const newNodeUnresolved = nodeFromJSON(newVal, newPriority);
316+
const existing = this.serverSyncTree_.calcCompleteEventCache(path);
316317
const newNode = resolveDeferredValueSnapshot(
317318
newNodeUnresolved,
319+
existing,
318320
serverValues
319321
);
320322

@@ -365,6 +367,7 @@ export class Repo {
365367
const newNodeUnresolved = nodeFromJSON(changedValue);
366368
changedChildren[changedKey] = resolveDeferredValueSnapshot(
367369
newNodeUnresolved,
370+
this.serverSyncTree_.calcCompleteEventCache(path),
368371
serverValues
369372
);
370373
});
@@ -419,6 +422,7 @@ export class Repo {
419422
const serverValues = this.generateServerValues();
420423
const resolvedOnDisconnectTree = resolveDeferredValueTree(
421424
this.onDisconnect_,
425+
this.serverSyncTree_,
422426
serverValues
423427
);
424428
let events: Event[] = [];

packages/database/src/core/Repo_transaction.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ Repo.prototype.startTransaction = function(
248248
const newNodeUnresolved = nodeFromJSON(newVal, priorityForNode);
249249
const newNode = resolveDeferredValueSnapshot(
250250
newNodeUnresolved,
251+
currentState,
251252
serverValues
252253
);
253254
transaction.currentOutputSnapshotRaw = newNodeUnresolved;
@@ -533,6 +534,7 @@ Repo.prototype.startTransaction = function(
533534
const serverValues = this.generateServerValues();
534535
const newNodeResolved = resolveDeferredValueSnapshot(
535536
newDataNode,
537+
currentNode,
536538
serverValues
537539
);
538540

packages/database/src/core/SyncTree.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -474,18 +474,17 @@ export class SyncTree {
474474
}
475475

476476
/**
477-
* Returns a complete cache, if we have one, of the data at a particular path. The location must have a listener above
478-
* it, but as this is only used by transaction code, that should always be the case anyways.
477+
* Returns a complete cache, if we have one, of the data at a particular path. If the location does not have a
478+
* listener above it, we will get a false "null". This shouldn't be a problem because transactions will always
479+
* have a listener above, and atomic operations would correctly show a jitter of <increment value> ->
480+
* <incremented total> as the write is applied locally and then acknowledged at the server.
479481
*
480482
* Note: this method will *include* hidden writes from transaction with applyLocally set to false.
481483
*
482484
* @param path The path to the data we want
483485
* @param writeIdsToExclude A specific set to be excluded
484486
*/
485-
calcCompleteEventCache(
486-
path: Path,
487-
writeIdsToExclude?: number[]
488-
): Node | null {
487+
calcCompleteEventCache(path: Path, writeIdsToExclude?: number[]): Node {
489488
const includeHiddenSets = true;
490489
const writeTree = this.pendingWriteTree_;
491490
const serverCache = this.syncPointTree_.findOnPath(

packages/database/src/core/util/ServerValues.ts

Lines changed: 74 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { nodeFromJSON } from '../snap/nodeFromJSON';
2323
import { PRIORITY_INDEX } from '../snap/indexes/PriorityIndex';
2424
import { Node } from '../snap/Node';
2525
import { ChildrenNode } from '../snap/ChildrenNode';
26+
import { SyncTree } from '../SyncTree';
2627
import { Indexable } from './misc';
2728

2829
/**
@@ -48,17 +49,63 @@ export const generateWithValues = function(
4849
* @return {!(string|number|boolean)}
4950
*/
5051
export const resolveDeferredValue = function(
51-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
52-
value: { [k: string]: any } | string | number | boolean,
53-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
54-
serverValues: { [k: string]: any }
52+
value: { [k: string]: unknown } | string | number | boolean,
53+
existing: Node,
54+
serverValues: { [k: string]: unknown }
5555
): string | number | boolean {
5656
if (!value || typeof value !== 'object') {
5757
return value as string | number | boolean;
58+
}
59+
assert('.sv' in value, 'Unexpected leaf node or priority contents');
60+
61+
if (typeof value['.sv'] === 'string') {
62+
return resolveScalarDeferredValue(value['.sv'], existing, serverValues);
63+
} else if (typeof value['.sv'] === 'object') {
64+
return resolveComplexDeferredValue(value['.sv'], existing, serverValues);
5865
} else {
59-
assert('.sv' in value, 'Unexpected leaf node or priority contents');
60-
return serverValues[value['.sv']];
66+
assert(false, 'Unexpected server value: ' + JSON.stringify(value, null, 2));
67+
}
68+
};
69+
70+
const resolveScalarDeferredValue = function(
71+
op: string,
72+
existing: Node,
73+
serverValues: { [k: string]: unknown }
74+
): string | number | boolean {
75+
switch (op) {
76+
case 'timestamp':
77+
return serverValues['timestamp'] as string | number | boolean;
78+
default:
79+
assert(false, 'Unexpected server value: ' + op);
80+
}
81+
};
82+
83+
const resolveComplexDeferredValue = function(
84+
op: object,
85+
existing: Node,
86+
unused: { [k: string]: unknown }
87+
): string | number | boolean {
88+
if (!op.hasOwnProperty('increment')) {
89+
assert(false, 'Unexpected server value: ' + JSON.stringify(op, null, 2));
90+
}
91+
const delta = op['increment'];
92+
if (typeof delta !== 'number') {
93+
assert(false, 'Unexpected increment value: ' + delta);
6194
}
95+
96+
// Incrementing a non-number sets the value to the incremented amount
97+
if (!existing.isLeafNode()) {
98+
return delta;
99+
}
100+
101+
const leaf = existing as LeafNode;
102+
const existingVal = leaf.getValue();
103+
if (typeof existingVal !== 'number') {
104+
return delta;
105+
}
106+
107+
// No need to do over/underflow arithmetic here because JS only handles floats under the covers
108+
return existingVal + delta;
62109
};
63110

64111
/**
@@ -70,13 +117,19 @@ export const resolveDeferredValue = function(
70117
*/
71118
export const resolveDeferredValueTree = function(
72119
tree: SparseSnapshotTree,
73-
serverValues: object
120+
syncTree: SyncTree,
121+
serverValues: Indexable
74122
): SparseSnapshotTree {
75123
const resolvedTree = new SparseSnapshotTree();
76124
tree.forEachTree(new Path(''), (path, node) => {
125+
const existing = syncTree.calcCompleteEventCache(path);
126+
assert(
127+
existing !== null && typeof existing !== 'undefined',
128+
'Expected ChildrenNode.EMPTY_NODE for nulls'
129+
);
77130
resolvedTree.remember(
78131
path,
79-
resolveDeferredValueSnapshot(node, serverValues)
132+
resolveDeferredValueSnapshot(node, existing, serverValues)
80133
);
81134
});
82135
return resolvedTree;
@@ -92,20 +145,29 @@ export const resolveDeferredValueTree = function(
92145
*/
93146
export const resolveDeferredValueSnapshot = function(
94147
node: Node,
95-
serverValues: object
148+
existing: Node,
149+
serverValues: Indexable
96150
): Node {
97151
const rawPri = node.getPriority().val() as
98152
| Indexable
99153
| boolean
100154
| null
101155
| number
102156
| string;
103-
const priority = resolveDeferredValue(rawPri, serverValues);
157+
const priority = resolveDeferredValue(
158+
rawPri,
159+
existing.getPriority(),
160+
serverValues
161+
);
104162
let newNode: Node;
105163

106164
if (node.isLeafNode()) {
107165
const leafNode = node as LeafNode;
108-
const value = resolveDeferredValue(leafNode.getValue(), serverValues);
166+
const value = resolveDeferredValue(
167+
leafNode.getValue(),
168+
existing,
169+
serverValues
170+
);
109171
if (
110172
value !== leafNode.getValue() ||
111173
priority !== leafNode.getPriority().val()
@@ -123,6 +185,7 @@ export const resolveDeferredValueSnapshot = function(
123185
childrenNode.forEachChild(PRIORITY_INDEX, (childName, childNode) => {
124186
const newChildNode = resolveDeferredValueSnapshot(
125187
childNode,
188+
existing.getImmediateChild(childName),
126189
serverValues
127190
);
128191
if (newChildNode !== childNode) {
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/**
2+
* @license
3+
* Copyright 2019 Google Inc.
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
import { expect } from 'chai';
19+
import { getFreshRepoFromReference, getRandomNode } from './helpers/util';
20+
import { Database } from '../src/api/Database';
21+
import { Reference } from '../src/api/Reference';
22+
import { nodeFromJSON } from '../src/core/snap/nodeFromJSON';
23+
24+
describe('ServerValue tests', () => {
25+
it('resolves timestamps locally', async () => {
26+
const node = getRandomNode() as Reference;
27+
const start = Date.now();
28+
const values: number[] = [];
29+
node.on('value', snap => {
30+
expect(typeof snap.val()).to.equal('number');
31+
values.push(snap.val() as number);
32+
});
33+
await node.set(Database.ServerValue.TIMESTAMP);
34+
node.off('value');
35+
36+
// By the time the write is acknowledged, we should have a local and
37+
// server version of the timestamp.
38+
expect(values.length).to.equal(2);
39+
values.forEach(serverTime => {
40+
const delta = Math.abs(serverTime - start);
41+
expect(delta).to.be.lessThan(1000);
42+
});
43+
});
44+
45+
it('handles increments without listeners', () => {
46+
// Ensure that increments don't explode when the SyncTree must return a null
47+
// node (i.e. ChildrenNode.EMPTY_NODE) because there is not yet any synced
48+
// data.
49+
// TODO(b/146657568): Remove getFreshRepoFromReference() call and goOffline()
50+
// once we have emulator support. We can also await the set() call.
51+
const node = getFreshRepoFromReference(getRandomNode()) as Reference;
52+
node.database.goOffline();
53+
54+
const addOne = Database.ServerValue._increment(1);
55+
56+
node.set(addOne);
57+
});
58+
59+
it('handles increments locally', async () => {
60+
// TODO(b/146657568): Remove getFreshRepoFromReference() call and goOffline()
61+
// once we have emulator support. We can also await the set() calls.
62+
const node = getFreshRepoFromReference(getRandomNode()) as Reference;
63+
node.database.goOffline();
64+
65+
const addOne = Database.ServerValue._increment(1);
66+
67+
const values: any = [];
68+
const expected: any = [];
69+
node.on('value', snap => values.push(snap.val()));
70+
71+
// null -> increment(x) = x
72+
node.set(addOne);
73+
expected.push(1);
74+
75+
// x -> increment(y) = x + y
76+
node.set(5);
77+
node.set(addOne);
78+
expected.push(5);
79+
expected.push(6);
80+
81+
// str -> increment(x) = x
82+
node.set('hello');
83+
node.set(addOne);
84+
expected.push('hello');
85+
expected.push(1);
86+
87+
// obj -> increment(x) = x
88+
node.set({ 'hello': 'world' });
89+
node.set(addOne);
90+
expected.push({ 'hello': 'world' });
91+
expected.push(1);
92+
93+
node.off('value');
94+
expect(values).to.deep.equal(expected);
95+
});
96+
});

0 commit comments

Comments
 (0)