Skip to content

Commit 9cda9c9

Browse files
author
Greg Soltis
authored
Keep track of number of queries in the query cache (#510)
* Adding Schema Migration * Pseudocode for Schema Migration * [AUTOMATED]: Prettier Code Styling * IndexedDb Schema Migration * Lint cleanup * Removing unused import * Removing user ID from instance row * [AUTOMATED]: Prettier Code Styling * Review comments * Lint fixes * Review * [AUTOMATED]: Prettier Code Styling * Fixing the tests * Closing the Database in the Schema tests * [AUTOMATED]: Prettier Code Styling * Changing test helper to close the DB * [AUTOMATED]: Prettier Code Styling * Making v2 the default version * [AUTOMATED]: Prettier Code Styling * Addressing comment * [AUTOMATED]: Prettier Code Styling * Renamed to ALL_STORES * Start work on adding query counts * Implement target count * [AUTOMATED]: Prettier Code Styling * Separate out add and update for the query cache * [AUTOMATED]: Prettier Code Styling * Comments and formatting * Comments and restructuring * [AUTOMATED]: Prettier Code Styling * Use SimpleDb, shorten iOS-y name * [AUTOMATED]: Prettier Code Styling * addTargetCount -> saveTargetCount * Fix lint warnings * Comment fixes * Rename test file * Add expectation for 0 targets if none were added * [AUTOMATED]: Prettier Code Styling * Switch to PersistenceTransaction for schema upgrade * Fix the other tests * [AUTOMATED]: Prettier Code Styling * Update comment * Add some asserts, revert to PersistencePromise * Add assert * [AUTOMATED]: Prettier Code Styling * helpers moved * Review feedback * Switch to just using fail() * Rename updateMetadata * Renaming and thread metadata through schema upgrade * Use the proper assert * Review feedback * [AUTOMATED]: Prettier Code Styling * Drop note re running time, initialize metadata to null * Fix tests that weren't calling start * [AUTOMATED]: Prettier Code Styling
1 parent e70ef37 commit 9cda9c9

19 files changed

+559
-146
lines changed

‎packages/database/test/order_by.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,9 @@ describe('.orderBy tests', function() {
363363
expect(addedPrevNames).to.deep.equal(expectedPrevNames);
364364
});
365365

366-
it('Removing default listener removes non-default listener that loads all data', function(done) {
366+
it('Removing default listener removes non-default listener that loads all data', function(
367+
done
368+
) {
367369
const ref = getRandomNode() as Reference;
368370

369371
const initial = { key: 'value' };

‎packages/database/test/query.test.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1971,7 +1971,9 @@ describe('Query Tests', function() {
19711971
expect(val).to.equal(2);
19721972
});
19731973

1974-
it('.startAt() with two arguments works properly (case 1169).', function(done) {
1974+
it('.startAt() with two arguments works properly (case 1169).', function(
1975+
done
1976+
) {
19751977
const ref = getRandomNode() as Reference;
19761978
const data = {
19771979
Walker: {
@@ -2108,7 +2110,9 @@ describe('Query Tests', function() {
21082110
});
21092111
});
21102112

2111-
it(".endAt(null, 'f').limitToLast(5) returns the right set of children.", function(done) {
2113+
it(".endAt(null, 'f').limitToLast(5) returns the right set of children.", function(
2114+
done
2115+
) {
21122116
const ref = getRandomNode() as Reference;
21132117
ref.set(
21142118
{ a: 'a', b: 'b', c: 'c', d: 'd', e: 'e', f: 'f', g: 'g', h: 'h' },
@@ -2130,7 +2134,9 @@ describe('Query Tests', function() {
21302134
);
21312135
});
21322136

2133-
it('complex update() at query root raises correct value event', function(done) {
2137+
it('complex update() at query root raises correct value event', function(
2138+
done
2139+
) {
21342140
const nodePair = getRandomNode(2);
21352141
const writer = nodePair[0];
21362142
const reader = nodePair[1];
@@ -2235,7 +2241,9 @@ describe('Query Tests', function() {
22352241
});
22362242
});
22372243

2238-
it('listen for child_added events with limit and different types fires properly', function(done) {
2244+
it('listen for child_added events with limit and different types fires properly', function(
2245+
done
2246+
) {
22392247
const nodePair = getRandomNode(2);
22402248
const writer = nodePair[0];
22412249
const reader = nodePair[1];
@@ -2277,7 +2285,9 @@ describe('Query Tests', function() {
22772285
});
22782286
});
22792287

2280-
it('listen for child_changed events with limit and different types fires properly', function(done) {
2288+
it('listen for child_changed events with limit and different types fires properly', function(
2289+
done
2290+
) {
22812291
const nodePair = getRandomNode(2);
22822292
const writer = nodePair[0];
22832293
const reader = nodePair[1];
@@ -2328,7 +2338,9 @@ describe('Query Tests', function() {
23282338
});
23292339
});
23302340

2331-
it('listen for child_remove events with limit and different types fires properly', function(done) {
2341+
it('listen for child_remove events with limit and different types fires properly', function(
2342+
done
2343+
) {
23322344
const nodePair = getRandomNode(2);
23332345
const writer = nodePair[0];
23342346
const reader = nodePair[1];
@@ -2430,7 +2442,9 @@ describe('Query Tests', function() {
24302442
);
24312443
});
24322444

2433-
it('listen for child_remove events when parent set to scalar', function(done) {
2445+
it('listen for child_remove events when parent set to scalar', function(
2446+
done
2447+
) {
24342448
const nodePair = getRandomNode(2);
24352449
const writer = nodePair[0];
24362450
const reader = nodePair[1];

‎packages/database/test/transaction.test.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ describe('Transaction Tests', function() {
8686
});
8787
});
8888

89-
it('Non-aborted transaction sets committed to true in callback.', function(done) {
89+
it('Non-aborted transaction sets committed to true in callback.', function(
90+
done
91+
) {
9092
const node = getRandomNode() as Reference;
9193

9294
node.transaction(
@@ -102,7 +104,9 @@ describe('Transaction Tests', function() {
102104
);
103105
});
104106

105-
it('Aborted transaction sets committed to false in callback.', function(done) {
107+
it('Aborted transaction sets committed to false in callback.', function(
108+
done
109+
) {
106110
const node = getRandomNode() as Reference;
107111

108112
node.transaction(
@@ -232,7 +236,9 @@ describe('Transaction Tests', function() {
232236
return ea.promise;
233237
});
234238

235-
it('Second transaction gets run immediately on previous output and only runs once.', function(done) {
239+
it('Second transaction gets run immediately on previous output and only runs once.', function(
240+
done
241+
) {
236242
const nodePair = getRandomNode(2) as Reference[];
237243
let firstRun = false,
238244
firstDone = false,
@@ -506,7 +512,9 @@ describe('Transaction Tests', function() {
506512
);
507513
});
508514

509-
it('Set should cancel already sent transactions that come back as datastale.', function(done) {
515+
it('Set should cancel already sent transactions that come back as datastale.', function(
516+
done
517+
) {
510518
const nodePair = getRandomNode(2) as Reference[];
511519
let transactionCalls = 0;
512520
nodePair[0].set(5, function() {
@@ -680,7 +688,9 @@ describe('Transaction Tests', function() {
680688
return Promise.all([tx1, tx2]);
681689
});
682690

683-
it('Doing set() in successful transaction callback works. Case 870.', function(done) {
691+
it('Doing set() in successful transaction callback works. Case 870.', function(
692+
done
693+
) {
684694
const node = getRandomNode() as Reference;
685695
let transactionCalled = false;
686696
let callbackCalled = false;
@@ -700,7 +710,9 @@ describe('Transaction Tests', function() {
700710
);
701711
});
702712

703-
it('Doing set() in aborted transaction callback works. Case 870.', function(done) {
713+
it('Doing set() in aborted transaction callback works. Case 870.', function(
714+
done
715+
) {
704716
const nodePair = getRandomNode(2) as Reference[],
705717
node1 = nodePair[0],
706718
node2 = nodePair[1];
@@ -1016,7 +1028,9 @@ describe('Transaction Tests', function() {
10161028
);
10171029
});
10181030

1019-
it('Transaction properly reverts data when you add a deeper listen.', function(done) {
1031+
it('Transaction properly reverts data when you add a deeper listen.', function(
1032+
done
1033+
) {
10201034
const refPair = getRandomNode(2) as Reference[],
10211035
ref1 = refPair[0],
10221036
ref2 = refPair[1];
@@ -1186,7 +1200,9 @@ describe('Transaction Tests', function() {
11861200
});
11871201
});
11881202

1189-
it("transaction() doesn't pick up cached data from previous once().", function(done) {
1203+
it("transaction() doesn't pick up cached data from previous once().", function(
1204+
done
1205+
) {
11901206
const refPair = getRandomNode(2) as Reference[];
11911207
const me = refPair[0],
11921208
other = refPair[1];
@@ -1213,7 +1229,9 @@ describe('Transaction Tests', function() {
12131229
});
12141230
});
12151231

1216-
it("transaction() doesn't pick up cached data from previous transaction.", function(done) {
1232+
it("transaction() doesn't pick up cached data from previous transaction.", function(
1233+
done
1234+
) {
12171235
const refPair = getRandomNode(2) as Reference[];
12181236
const me = refPair[0],
12191237
other = refPair[1];
@@ -1245,7 +1263,9 @@ describe('Transaction Tests', function() {
12451263
);
12461264
});
12471265

1248-
it('server values: local timestamp should eventually (but not immediately) match the server with txns', function(done) {
1266+
it('server values: local timestamp should eventually (but not immediately) match the server with txns', function(
1267+
done
1268+
) {
12491269
const refPair = getRandomNode(2) as Reference[],
12501270
writer = refPair[0],
12511271
reader = refPair[1],
@@ -1337,7 +1357,9 @@ describe('Transaction Tests', function() {
13371357
);
13381358
});
13391359

1340-
it("transaction() on queried location doesn't run initially on null (firebase-worker-queue depends on this).", function(done) {
1360+
it("transaction() on queried location doesn't run initially on null (firebase-worker-queue depends on this).", function(
1361+
done
1362+
) {
13411363
const ref = getRandomNode() as Reference;
13421364
ref.push({ a: 1, b: 2 }, function() {
13431365
ref
@@ -1415,7 +1437,9 @@ describe('Transaction Tests', function() {
14151437
);
14161438
});
14171439

1418-
it('transactions works with merges without the transaction path', function(done) {
1440+
it('transactions works with merges without the transaction path', function(
1441+
done
1442+
) {
14191443
const ref = getRandomNode() as Reference;
14201444

14211445
ref.update({ foo: 'bar' });

‎packages/firestore/src/local/indexeddb_persistence.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,13 @@ import { AutoId } from '../util/misc';
2525
import { IndexedDbMutationQueue } from './indexeddb_mutation_queue';
2626
import { IndexedDbQueryCache } from './indexeddb_query_cache';
2727
import { IndexedDbRemoteDocumentCache } from './indexeddb_remote_document_cache';
28-
import { ALL_STORES, DbOwner, DbOwnerKey } from './indexeddb_schema';
29-
import { createOrUpgradeDb, SCHEMA_VERSION } from './indexeddb_schema';
28+
import {
29+
ALL_STORES,
30+
createOrUpgradeDb,
31+
DbOwner,
32+
DbOwnerKey,
33+
SCHEMA_VERSION
34+
} from './indexeddb_schema';
3035
import { LocalSerializer } from './local_serializer';
3136
import { MutationQueue } from './mutation_queue';
3237
import { Persistence } from './persistence';

‎packages/firestore/src/local/indexeddb_query_cache.ts

Lines changed: 72 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,7 @@ import {
3131
DbTargetDocumentKey,
3232
DbTargetGlobal,
3333
DbTargetGlobalKey,
34-
DbTargetKey,
35-
DbTimestamp
34+
DbTargetKey
3635
} from './indexeddb_schema';
3736
import { LocalSerializer } from './local_serializer';
3837
import { PersistenceTransaction } from './persistence';
@@ -53,11 +52,7 @@ export class IndexedDbQueryCache implements QueryCache {
5352
/**
5453
* A cached copy of the metadata for the query cache.
5554
*/
56-
private metadata = new DbTargetGlobal(
57-
/*highestTargetId=*/ 0,
58-
/*lastListenSequenceNumber=*/ 0,
59-
SnapshotVersion.MIN.toTimestamp()
60-
);
55+
private metadata = null;
6156

6257
/** The garbage collector to notify about potential garbage keys. */
6358
private garbageCollector: GarbageCollector | null = null;
@@ -66,13 +61,15 @@ export class IndexedDbQueryCache implements QueryCache {
6661
return globalTargetStore(transaction)
6762
.get(DbTargetGlobal.key)
6863
.next(metadata => {
69-
if (metadata !== null) {
70-
this.metadata = metadata;
71-
const lastSavedVersion = metadata.lastRemoteSnapshotVersion;
72-
this.lastRemoteSnapshotVersion = SnapshotVersion.fromTimestamp(
73-
new Timestamp(lastSavedVersion.seconds, lastSavedVersion.nanos)
74-
);
75-
}
64+
assert(
65+
metadata !== null,
66+
'Missing metadata row that should be added by schema migration.'
67+
);
68+
this.metadata = metadata;
69+
const lastSavedVersion = metadata.lastRemoteSnapshotVersion;
70+
this.lastRemoteSnapshotVersion = SnapshotVersion.fromTimestamp(
71+
new Timestamp(lastSavedVersion.seconds, lastSavedVersion.nanos)
72+
);
7673
return PersistencePromise.resolve();
7774
});
7875
}
@@ -101,32 +98,75 @@ export class IndexedDbQueryCache implements QueryCache {
10198
transaction: PersistenceTransaction,
10299
queryData: QueryData
103100
): PersistencePromise<void> {
104-
const targetId = queryData.targetId;
105-
const addedQueryPromise = targetsStore(transaction).put(
106-
this.serializer.toDbTarget(queryData)
107-
);
108-
if (targetId > this.metadata.highestTargetId) {
109-
this.metadata.highestTargetId = targetId;
110-
return addedQueryPromise.next(() =>
111-
globalTargetStore(transaction).put(DbTargetGlobal.key, this.metadata)
112-
);
113-
} else {
114-
return addedQueryPromise;
115-
}
101+
return this.saveQueryData(transaction, queryData).next(() => {
102+
this.metadata.targetCount += 1;
103+
this.updateMetadataFromQueryData(queryData);
104+
return this.saveMetadata(transaction);
105+
});
116106
}
117107

118-
removeQueryData(
108+
updateQueryData(
119109
transaction: PersistenceTransaction,
120110
queryData: QueryData
121111
): PersistencePromise<void> {
122-
return this.removeMatchingKeysForTargetId(
123-
transaction,
124-
queryData.targetId
125-
).next(() => {
126-
targetsStore(transaction).delete(queryData.targetId);
112+
return this.saveQueryData(transaction, queryData).next(() => {
113+
if (this.updateMetadataFromQueryData(queryData)) {
114+
return this.saveMetadata(transaction);
115+
} else {
116+
return PersistencePromise.resolve();
117+
}
127118
});
128119
}
129120

121+
removeQueryData(
122+
transaction: PersistenceTransaction,
123+
queryData: QueryData
124+
): PersistencePromise<void> {
125+
assert(this.metadata.targetCount > 0, 'Removing from an empty query cache');
126+
return this.removeMatchingKeysForTargetId(transaction, queryData.targetId)
127+
.next(() => targetsStore(transaction).delete(queryData.targetId))
128+
.next(() => {
129+
this.metadata.targetCount -= 1;
130+
return this.saveMetadata(transaction);
131+
});
132+
}
133+
134+
private saveMetadata(
135+
transaction: PersistenceTransaction
136+
): PersistencePromise<void> {
137+
return globalTargetStore(transaction).put(
138+
DbTargetGlobal.key,
139+
this.metadata
140+
);
141+
}
142+
143+
private saveQueryData(
144+
transaction: PersistenceTransaction,
145+
queryData: QueryData
146+
): PersistencePromise<void> {
147+
return targetsStore(transaction).put(this.serializer.toDbTarget(queryData));
148+
}
149+
150+
/**
151+
* Updates the in-memory version of the metadata to account for values in the
152+
* given QueryData. Saving is done separately. Returns true if there were any
153+
* changes to the metadata.
154+
*/
155+
private updateMetadataFromQueryData(queryData: QueryData): boolean {
156+
let needsUpdate = false;
157+
if (queryData.targetId > this.metadata.highestTargetId) {
158+
this.metadata.highestTargetId = queryData.targetId;
159+
needsUpdate = true;
160+
}
161+
162+
// TODO(GC): add sequence number check
163+
return needsUpdate;
164+
}
165+
166+
get count(): number {
167+
return this.metadata.targetCount;
168+
}
169+
130170
getQueryData(
131171
transaction: PersistenceTransaction,
132172
query: Query

0 commit comments

Comments
 (0)