Skip to content

Commit 0725a5d

Browse files
wilhuffwilhuff
authored
Migrate FSTTargetID to C++ TargetId (#1697)
1 parent 270b1cd commit 0725a5d

38 files changed

+183
-140
lines changed

‎Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,14 @@
2828

2929
#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h"
3030
#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
31+
#include "Firestore/core/src/firebase/firestore/model/types.h"
3132

3233
NS_ASSUME_NONNULL_BEGIN
3334

3435
using firebase::firestore::local::LevelDbTargetDocumentKey;
3536
using firebase::firestore::local::LevelDbTransaction;
3637
using firebase::firestore::model::DatabaseId;
38+
using firebase::firestore::model::TargetId;
3739

3840
namespace {
3941

@@ -113,7 +115,7 @@ void FillDB() {
113115
protected:
114116
void WriteIndex(LevelDbTransaction &txn, FSTDocumentKey *docKey) {
115117
// Arbitrary target ID
116-
FSTTargetID targetID = 1;
118+
TargetId targetID = 1;
117119
txn.Put(LevelDbDocumentTargetKey::Key(docKey, targetID), emptyBuffer_);
118120
txn.Put(LevelDbTargetDocumentKey::Key(targetID, docKey), emptyBuffer_);
119121
}

‎Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,22 @@
3434
#include "Firestore/core/src/firebase/firestore/auth/user.h"
3535
#include "Firestore/core/src/firebase/firestore/model/document_key_set.h"
3636
#include "Firestore/core/src/firebase/firestore/model/precondition.h"
37+
#include "Firestore/core/src/firebase/firestore/model/types.h"
3738
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
3839
#include "absl/strings/str_cat.h"
3940

41+
namespace testutil = firebase::firestore::testutil;
4042
using firebase::firestore::auth::User;
4143
using firebase::firestore::model::DocumentKey;
4244
using firebase::firestore::model::DocumentKeyHash;
4345
using firebase::firestore::model::DocumentKeySet;
4446
using firebase::firestore::model::Precondition;
45-
namespace testutil = firebase::firestore::testutil;
47+
using firebase::firestore::model::TargetId;
4648

4749
NS_ASSUME_NONNULL_BEGIN
4850

4951
@implementation FSTLRUGarbageCollectorTests {
50-
FSTTargetID _previousTargetID;
52+
TargetId _previousTargetID;
5153
int _previousDocNum;
5254
FSTObjectValue *_testValue;
5355
FSTObjectValue *_bigObjectValue;
@@ -121,7 +123,7 @@ - (int)removeOrphanedDocumentsThroughSequenceNumber:(FSTListenSequenceNumber)seq
121123
}
122124

123125
- (FSTQueryData *)nextTestQuery {
124-
FSTTargetID targetID = ++_previousTargetID;
126+
TargetId targetID = ++_previousTargetID;
125127
FSTListenSequenceNumber listenSequenceNumber = _persistence.currentSequenceNumber;
126128
FSTQuery *query = FSTTestQuery(absl::StrCat("path", targetID));
127129
return [[FSTQueryData alloc] initWithQuery:query
@@ -175,11 +177,11 @@ - (void)markDocumentEligibleForGCInTransaction:(const DocumentKey &)docKey {
175177
[_persistence.referenceDelegate removeMutationReference:docKey];
176178
}
177179

178-
- (void)addDocument:(const DocumentKey &)docKey toTarget:(FSTTargetID)targetId {
180+
- (void)addDocument:(const DocumentKey &)docKey toTarget:(TargetId)targetId {
179181
[_queryCache addMatchingKeys:DocumentKeySet{docKey} forTargetID:targetId];
180182
}
181183

182-
- (void)removeDocument:(const DocumentKey &)docKey fromTarget:(FSTTargetID)targetId {
184+
- (void)removeDocument:(const DocumentKey &)docKey fromTarget:(TargetId)targetId {
183185
[_queryCache removeMatchingKeys:DocumentKeySet{docKey} forTargetID:targetId];
184186
}
185187

@@ -639,4 +641,4 @@ - (void)testRemoveTargetsThenGC {
639641
}
640642
@end
641643

642-
NS_ASSUME_NONNULL_END
644+
NS_ASSUME_NONNULL_END

‎Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,9 @@
4545
using firebase::firestore::local::LevelDbTargetDocumentKey;
4646
using firebase::firestore::local::LevelDbTargetKey;
4747
using firebase::firestore::local::LevelDbTransaction;
48-
using firebase::firestore::util::OrderedCode;
48+
using firebase::firestore::model::TargetId;
4949
using firebase::firestore::testutil::Key;
50+
using firebase::firestore::util::OrderedCode;
5051
using leveldb::DB;
5152
using leveldb::Options;
5253
using leveldb::Status;
@@ -119,7 +120,7 @@ - (void)testSetsVersionNumber {
119120
- (void)testDropsTheQueryCache {
120121
std::string userID{"user"};
121122
FSTBatchID batchID = 1;
122-
FSTTargetID targetID = 2;
123+
TargetId targetID = 2;
123124

124125
FSTDocumentKey *key1 = Key("documents/1");
125126
FSTDocumentKey *key2 = Key("documents/2");

‎Firestore/Example/Tests/Local/FSTLevelDBQueryCacheTests.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
using firebase::firestore::model::DatabaseId;
3535
using firebase::firestore::model::ResourcePath;
3636
using firebase::firestore::model::SnapshotVersion;
37+
using firebase::firestore::model::TargetId;
3738

3839
NS_ASSUME_NONNULL_BEGIN
3940

@@ -75,7 +76,7 @@ - (void)testMetadataPersistedAcrossRestarts {
7576
XCTAssertEqual(versionZero, queryCache.lastRemoteSnapshotVersion);
7677

7778
FSTListenSequenceNumber minimumSequenceNumber = 1234;
78-
FSTTargetID lastTargetId = 5;
79+
TargetId lastTargetId = 5;
7980
SnapshotVersion lastVersion(Timestamp(1, 2));
8081

8182
db1.run("add query data", [&]() {

‎Firestore/Example/Tests/Local/FSTLocalSerializerTests.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
using firebase::firestore::model::FieldMask;
5151
using firebase::firestore::model::Precondition;
5252
using firebase::firestore::model::SnapshotVersion;
53+
using firebase::firestore::model::TargetId;
5354

5455
NS_ASSUME_NONNULL_BEGIN
5556

@@ -161,7 +162,7 @@ - (void)testEncodesDeletedDocumentAsMaybeDocument {
161162

162163
- (void)testEncodesQueryData {
163164
FSTQuery *query = FSTTestQuery("room");
164-
FSTTargetID targetID = 42;
165+
TargetId targetID = 42;
165166
SnapshotVersion version = testutil::Version(1039);
166167
NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(1039);
167168

‎Firestore/Example/Tests/Local/FSTLocalStoreTests.mm

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
using firebase::firestore::auth::User;
4747
using firebase::firestore::model::SnapshotVersion;
4848
using firebase::firestore::model::DocumentKeySet;
49+
using firebase::firestore::model::TargetId;
4950

5051
NS_ASSUME_NONNULL_BEGIN
5152

@@ -56,7 +57,7 @@ @interface FSTLocalStoreTests ()
5657

5758
@property(nonatomic, strong, readonly) NSMutableArray<FSTMutationBatch *> *batches;
5859
@property(nonatomic, strong, readwrite, nullable) FSTMaybeDocumentDictionary *lastChanges;
59-
@property(nonatomic, assign, readwrite) FSTTargetID lastTargetID;
60+
@property(nonatomic, assign, readwrite) TargetId lastTargetID;
6061

6162
@end
6263

@@ -144,7 +145,7 @@ - (void)rejectMutation {
144145
self.lastChanges = [self.localStore rejectBatchID:batch.batchID];
145146
}
146147

147-
- (FSTTargetID)allocateQuery:(FSTQuery *)query {
148+
- (TargetId)allocateQuery:(FSTQuery *)query {
148149
FSTQueryData *queryData = [self.localStore allocateQuery:query];
149150
self.lastTargetID = queryData.targetID;
150151
return queryData.targetID;
@@ -240,7 +241,7 @@ - (void)testHandlesSetMutationThenDocument {
240241
FSTAssertContains(FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, YES));
241242

242243
FSTQuery *query = FSTTestQuery("foo");
243-
FSTTargetID targetID = [self allocateQuery:query];
244+
TargetId targetID = [self allocateQuery:query];
244245

245246
[self
246247
applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 2, @{@"it" : @"changed"}, NO),
@@ -254,7 +255,7 @@ - (void)testHandlesAckThenRejectThenRemoteEvent {
254255

255256
// Start a query that requires acks to be held.
256257
FSTQuery *query = FSTTestQuery("foo");
257-
FSTTargetID targetID = [self allocateQuery:query];
258+
TargetId targetID = [self allocateQuery:query];
258259

259260
[self writeMutation:FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"})];
260261
FSTAssertChanged(@[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, YES) ]);
@@ -285,7 +286,7 @@ - (void)testHandlesDeletedDocumentThenSetMutationThenAck {
285286
if ([self isTestBaseClass]) return;
286287

287288
FSTQuery *query = FSTTestQuery("foo");
288-
FSTTargetID targetID = [self allocateQuery:query];
289+
TargetId targetID = [self allocateQuery:query];
289290

290291
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDeletedDoc("foo/bar", 2), @[ @(targetID) ],
291292
@[])];
@@ -318,7 +319,7 @@ - (void)testHandlesSetMutationThenDeletedDocument {
318319
if ([self isTestBaseClass]) return;
319320

320321
FSTQuery *query = FSTTestQuery("foo");
321-
FSTTargetID targetID = [self allocateQuery:query];
322+
TargetId targetID = [self allocateQuery:query];
322323

323324
[self writeMutation:FSTTestSetMutation(@"foo/bar", @{@"foo" : @"bar"})];
324325
FSTAssertChanged(@[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, YES) ]);
@@ -334,7 +335,7 @@ - (void)testHandlesDocumentThenSetMutationThenAckThenDocument {
334335

335336
// Start a query that requires acks to be held.
336337
FSTQuery *query = FSTTestQuery("foo");
337-
FSTTargetID targetID = [self allocateQuery:query];
338+
TargetId targetID = [self allocateQuery:query];
338339

339340
[self applyRemoteEvent:FSTTestAddedRemoteEvent(FSTTestDoc("foo/bar", 2, @{@"it" : @"base"}, NO),
340341
@[ @(targetID) ])];
@@ -377,7 +378,7 @@ - (void)testHandlesPatchMutationThenDocumentThenAck {
377378
FSTAssertNotContains(@"foo/bar");
378379

379380
FSTQuery *query = FSTTestQuery("foo");
380-
FSTTargetID targetID = [self allocateQuery:query];
381+
TargetId targetID = [self allocateQuery:query];
381382

382383
[self applyRemoteEvent:FSTTestAddedRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"it" : @"base"}, NO),
383384
@[ @(targetID) ])];
@@ -410,7 +411,7 @@ - (void)testHandlesPatchMutationThenAckThenDocument {
410411
FSTAssertNotContains(@"foo/bar");
411412

412413
FSTQuery *query = FSTTestQuery("foo");
413-
FSTTargetID targetID = [self allocateQuery:query];
414+
TargetId targetID = [self allocateQuery:query];
414415

415416
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"it" : @"base"}, NO),
416417
@[ @(targetID) ], @[])];
@@ -437,7 +438,7 @@ - (void)testHandlesDocumentThenDeleteMutationThenAck {
437438
if ([self isTestBaseClass]) return;
438439

439440
FSTQuery *query = FSTTestQuery("foo");
440-
FSTTargetID targetID = [self allocateQuery:query];
441+
TargetId targetID = [self allocateQuery:query];
441442

442443
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"it" : @"base"}, NO),
443444
@[ @(targetID) ], @[])];
@@ -463,7 +464,7 @@ - (void)testHandlesDeleteMutationThenDocumentThenAck {
463464
if ([self isTestBaseClass]) return;
464465

465466
FSTQuery *query = FSTTestQuery("foo");
466-
FSTTargetID targetID = [self allocateQuery:query];
467+
TargetId targetID = [self allocateQuery:query];
467468

468469
[self writeMutation:FSTTestDeleteMutation(@"foo/bar")];
469470
FSTAssertRemoved(@[ @"foo/bar" ]);
@@ -491,7 +492,7 @@ - (void)testHandlesDocumentThenDeletedDocumentThenDocument {
491492
if ([self isTestBaseClass]) return;
492493

493494
FSTQuery *query = FSTTestQuery("foo");
494-
FSTTargetID targetID = [self allocateQuery:query];
495+
TargetId targetID = [self allocateQuery:query];
495496

496497
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"it" : @"base"}, NO),
497498
@[ @(targetID) ], @[])];
@@ -524,7 +525,7 @@ - (void)testHandlesSetMutationThenPatchMutationThenDocumentThenAckThenAck {
524525
FSTAssertContains(FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, YES));
525526

526527
FSTQuery *query = FSTTestQuery("foo");
527-
FSTTargetID targetID = [self allocateQuery:query];
528+
TargetId targetID = [self allocateQuery:query];
528529

529530
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"it" : @"base"}, NO),
530531
@[ @(targetID) ], @[])];
@@ -631,7 +632,7 @@ - (void)testCollectsGarbageAfterChangeBatch {
631632
if (![self gcIsEager]) return;
632633

633634
FSTQuery *query = FSTTestQuery("foo");
634-
FSTTargetID targetID = [self allocateQuery:query];
635+
TargetId targetID = [self allocateQuery:query];
635636

636637
[self applyRemoteEvent:FSTTestAddedRemoteEvent(FSTTestDoc("foo/bar", 2, @{@"foo" : @"bar"}, NO),
637638
@[ @(targetID) ])];
@@ -648,7 +649,7 @@ - (void)testCollectsGarbageAfterAcknowledgedMutation {
648649
if (![self gcIsEager]) return;
649650

650651
FSTQuery *query = FSTTestQuery("foo");
651-
FSTTargetID targetID = [self allocateQuery:query];
652+
TargetId targetID = [self allocateQuery:query];
652653

653654
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 0, @{@"foo" : @"old"}, NO),
654655
@[ @(targetID) ], @[])];
@@ -683,7 +684,7 @@ - (void)testCollectsGarbageAfterRejectedMutation {
683684
if (![self gcIsEager]) return;
684685

685686
FSTQuery *query = FSTTestQuery("foo");
686-
FSTTargetID targetID = [self allocateQuery:query];
687+
TargetId targetID = [self allocateQuery:query];
687688

688689
[self applyRemoteEvent:FSTTestUpdateRemoteEvent(FSTTestDoc("foo/bar", 0, @{@"foo" : @"old"}, NO),
689690
@[ @(targetID) ], @[])];
@@ -718,7 +719,7 @@ - (void)testPinsDocumentsInTheLocalView {
718719
if (![self gcIsEager]) return;
719720

720721
FSTQuery *query = FSTTestQuery("foo");
721-
FSTTargetID targetID = [self allocateQuery:query];
722+
TargetId targetID = [self allocateQuery:query];
722723

723724
[self applyRemoteEvent:FSTTestAddedRemoteEvent(FSTTestDoc("foo/bar", 1, @{@"foo" : @"bar"}, NO),
724725
@[ @(targetID) ])];
@@ -749,7 +750,7 @@ - (void)testThrowsAwayDocumentsWithUnknownTargetIDsImmediately {
749750
if ([self isTestBaseClass]) return;
750751
if (![self gcIsEager]) return;
751752

752-
FSTTargetID targetID = 321;
753+
TargetId targetID = 321;
753754
[self applyRemoteEvent:FSTTestUpdateRemoteEventWithLimboTargets(FSTTestDoc("foo/bar", 1, @{}, NO),
754755
@[], @[], @[ @(targetID) ])];
755756

‎Firestore/Example/Tests/Local/FSTQueryCacheTests.mm

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,16 @@
3131

3232
namespace testutil = firebase::firestore::testutil;
3333
using firebase::firestore::model::DocumentKey;
34-
using firebase::firestore::model::SnapshotVersion;
3534
using firebase::firestore::model::DocumentKeySet;
35+
using firebase::firestore::model::SnapshotVersion;
36+
using firebase::firestore::model::TargetId;
3637

3738
NS_ASSUME_NONNULL_BEGIN
3839

3940
@implementation FSTQueryCacheTests {
4041
FSTQuery *_queryRooms;
4142
FSTListenSequenceNumber _previousSequenceNumber;
42-
FSTTargetID _previousTargetID;
43+
TargetId _previousTargetID;
4344
FSTTestSnapshotVersion _previousSnapshotVersion;
4445
}
4546

@@ -363,7 +364,7 @@ - (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query {
363364
}
364365

365366
- (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query
366-
targetID:(FSTTargetID)targetID
367+
targetID:(TargetId)targetID
367368
listenSequenceNumber:(FSTListenSequenceNumber)sequenceNumber
368369
version:(FSTTestSnapshotVersion)version {
369370
NSData *resumeToken = FSTTestResumeTokenFromSnapshotVersion(version);
@@ -375,12 +376,12 @@ - (FSTQueryData *)queryDataWithQuery:(FSTQuery *)query
375376
resumeToken:resumeToken];
376377
}
377378

378-
- (void)addMatchingKey:(const DocumentKey &)key forTargetID:(FSTTargetID)targetID {
379+
- (void)addMatchingKey:(const DocumentKey &)key forTargetID:(TargetId)targetID {
379380
DocumentKeySet keys{key};
380381
[self.queryCache addMatchingKeys:keys forTargetID:targetID];
381382
}
382383

383-
- (void)removeMatchingKey:(const DocumentKey &)key forTargetID:(FSTTargetID)targetID {
384+
- (void)removeMatchingKey:(const DocumentKey &)key forTargetID:(TargetId)targetID {
384385
DocumentKeySet keys{key};
385386
[self.queryCache removeMatchingKeys:keys forTargetID:targetID];
386387
}

‎Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
using firebase::firestore::core::DatabaseInfo;
3737
using firebase::firestore::model::DatabaseId;
3838
using firebase::firestore::model::SnapshotVersion;
39+
using firebase::firestore::model::TargetId;
3940

4041
@class GRPCProtoCall;
4142

@@ -126,7 +127,7 @@ - (void)watchQuery:(FSTQueryData *)query {
126127
self.activeTargets[@(query.targetID)] = sentQueryData;
127128
}
128129

129-
- (void)unwatchTargetID:(FSTTargetID)targetID {
130+
- (void)unwatchTargetID:(TargetId)targetID {
130131
LOG_DEBUG("unwatchTargetID: %s", targetID);
131132
[self.activeTargets removeObjectForKey:@(targetID)];
132133
}

‎Firestore/Example/Tests/SpecTests/FSTSpecTests.mm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,9 @@ - (FSTDocumentViewChange *)parseChange:(NSArray *)change ofType:(FSTDocumentView
185185

186186
- (void)doListen:(NSArray *)listenSpec {
187187
FSTQuery *query = [self parseQuery:listenSpec[1]];
188-
FSTTargetID actualID = [self.driver addUserListenerWithQuery:query];
188+
TargetId actualID = [self.driver addUserListenerWithQuery:query];
189189

190-
FSTTargetID expectedID = [listenSpec[0] intValue];
190+
TargetId expectedID = [listenSpec[0] intValue];
191191
XCTAssertEqual(actualID, expectedID, @"targetID assigned to listen");
192192
}
193193

@@ -579,7 +579,7 @@ - (void)validateStateExpectations:(nullable NSDictionary *)expected {
579579
[expected[@"activeTargets"] enumerateKeysAndObjectsUsingBlock:^(NSString *targetIDString,
580580
NSDictionary *queryData,
581581
BOOL *stop) {
582-
FSTTargetID targetID = [targetIDString intValue];
582+
TargetId targetID = [targetIDString intValue];
583583
FSTQuery *query = [self parseQuery:queryData[@"query"]];
584584
NSData *resumeToken = [queryData[@"resumeToken"] dataUsingEncoding:NSUTF8StringEncoding];
585585
// TODO(mcg): populate the purpose of the target once it's possible to encode that in the

0 commit comments

Comments
 (0)