Skip to content

Commit 673b085

Browse files
var-constvar-const
authored
C++: replace FSTMaybeDocumentDictionary with a C++ equivalent (#2139)
Also eliminate most usages of `FSTDocumentKey`, remove most methods from the Objective-C class and make it just a wrapper over `DocumentKey`. The only usage that cannot be directly replaced by C++ `DocumentKey` is in `FSTFieldValue`.
1 parent 4dac884 commit 673b085

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+530
-546
lines changed

‎Firestore/Example/Benchmarks/FSTLevelDBBenchmarkTests.mm

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,23 @@
2222

2323
#import "Firestore/Source/Local/FSTLevelDB.h"
2424
#import "Firestore/Source/Local/FSTLocalSerializer.h"
25-
#import "Firestore/Source/Model/FSTDocumentKey.h"
2625
#import "Firestore/Source/Remote/FSTSerializerBeta.h"
2726

2827
#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h"
2928
#include "Firestore/core/src/firebase/firestore/local/leveldb_transaction.h"
29+
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
3030
#include "Firestore/core/src/firebase/firestore/model/types.h"
31+
#include "Firestore/core/src/firebase/firestore/util/string_format.h"
3132

3233
NS_ASSUME_NONNULL_BEGIN
3334

35+
using firebase::firestore::local::LevelDbRemoteDocumentKey;
3436
using firebase::firestore::local::LevelDbTargetDocumentKey;
3537
using firebase::firestore::local::LevelDbTransaction;
3638
using firebase::firestore::model::DatabaseId;
39+
using firebase::firestore::model::DocumentKey;
3740
using firebase::firestore::model::TargetId;
41+
using firebase::firestore::util::StringFormat;
3842

3943
namespace {
4044

@@ -100,9 +104,8 @@ void FillDB() {
100104
LevelDbTransaction txn(db_.ptr, "benchmark");
101105

102106
for (int i = 0; i < numDocuments_; i++) {
103-
FSTDocumentKey *docKey =
104-
[FSTDocumentKey keyWithPathString:[NSString stringWithFormat:@"docs/doc_%i", i]];
105-
std::string docKeyString = [FSTLevelDBRemoteDocumentKey keyWithDocumentKey:docKey];
107+
auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i));
108+
std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey);
106109
txn.Put(docKeyString, DocumentData());
107110
WriteIndex(txn, docKey);
108111
}
@@ -112,7 +115,7 @@ void FillDB() {
112115
}
113116

114117
protected:
115-
void WriteIndex(LevelDbTransaction &txn, FSTDocumentKey *docKey) {
118+
void WriteIndex(LevelDbTransaction &txn, const DocumentKey &docKey) {
116119
// Arbitrary target ID
117120
TargetId targetID = 1;
118121
txn.Put(LevelDbDocumentTargetKey::Key(docKey, targetID), emptyBuffer_);
@@ -136,10 +139,9 @@ void WriteIndex(LevelDbTransaction &txn, FSTDocumentKey *docKey) {
136139
for (const auto &_ : state) {
137140
LevelDbTransaction txn(db_.ptr, "benchmark");
138141
for (int i = 0; i < docsToUpdate; i++) {
139-
FSTDocumentKey *docKey =
140-
[FSTDocumentKey keyWithPathString:[NSString stringWithFormat:@"docs/doc_%i", i]];
142+
auto docKey = DocumentKey::FromPathString(StringFormat("docs/doc_%i", i));
141143
if (writeIndexes) WriteIndex(txn, docKey);
142-
std::string docKeyString = [FSTLevelDBRemoteDocumentKey keyWithDocumentKey:docKey];
144+
std::string docKeyString = LevelDbRemoteDocumentKey::Key(docKey);
143145
txn.Put(docKeyString, documentUpdate);
144146
}
145147
txn.Commit();

‎Firestore/Example/Tests/API/FSTAPIHelpers.mm

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#import "Firestore/Source/Core/FSTQuery.h"
3232
#import "Firestore/Source/Core/FSTViewSnapshot.h"
3333
#import "Firestore/Source/Model/FSTDocument.h"
34-
#import "Firestore/Source/Model/FSTDocumentKey.h"
3534
#import "Firestore/Source/Model/FSTDocumentSet.h"
3635

3736
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"

‎Firestore/Example/Tests/Core/FSTQueryTests.mm

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
#import "Firestore/Source/API/FIRFirestore+Internal.h"
2222
#import "Firestore/Source/Model/FSTDocument.h"
23-
#import "Firestore/Source/Model/FSTDocumentKey.h"
2423

2524
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
2625

‎Firestore/Example/Tests/Core/FSTViewTests.mm

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#import "Firestore/Source/Core/FSTQuery.h"
2323
#import "Firestore/Source/Core/FSTViewSnapshot.h"
2424
#import "Firestore/Source/Model/FSTDocument.h"
25-
#import "Firestore/Source/Model/FSTDocumentKey.h"
2625
#import "Firestore/Source/Model/FSTDocumentSet.h"
2726
#import "Firestore/Source/Model/FSTFieldValue.h"
2827
#import "Firestore/Source/Remote/FSTRemoteEvent.h"

‎Firestore/Example/Tests/Integration/FSTDatastoreTests.mm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
#import "Firestore/Source/Core/FSTFirestoreClient.h"
2727
#import "Firestore/Source/Core/FSTQuery.h"
2828
#import "Firestore/Source/Local/FSTQueryData.h"
29-
#import "Firestore/Source/Model/FSTDocumentKey.h"
3029
#import "Firestore/Source/Model/FSTFieldValue.h"
3130
#import "Firestore/Source/Model/FSTMutation.h"
3231
#import "Firestore/Source/Model/FSTMutationBatch.h"
@@ -39,6 +38,7 @@
3938
#include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h"
4039
#include "Firestore/core/src/firebase/firestore/core/database_info.h"
4140
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
41+
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
4242
#include "Firestore/core/src/firebase/firestore/model/precondition.h"
4343
#include "Firestore/core/src/firebase/firestore/util/async_queue.h"
4444
#include "Firestore/core/src/firebase/firestore/util/executor_libdispatch.h"
@@ -51,6 +51,7 @@
5151
using firebase::firestore::core::DatabaseInfo;
5252
using firebase::firestore::model::BatchId;
5353
using firebase::firestore::model::DatabaseId;
54+
using firebase::firestore::model::DocumentKey;
5455
using firebase::firestore::model::DocumentKeySet;
5556
using firebase::firestore::model::Precondition;
5657
using firebase::firestore::model::TargetId;
@@ -243,7 +244,7 @@ - (void)awaitExpectations {
243244

244245
- (FSTSetMutation *)setMutation {
245246
return [[FSTSetMutation alloc]
246-
initWithKey:[FSTDocumentKey keyWithPathString:@"rooms/eros"]
247+
initWithKey:DocumentKey::FromPathString("rooms/eros")
247248
value:[[FSTObjectValue alloc]
248249
initWithDictionary:@{@"name" : [FSTStringValue stringValue:@"Eros"]}]
249250
precondition:Precondition::None()];

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,8 @@ - (void)testDropsTheQueryCache {
124124
BatchId batchID = 1;
125125
TargetId targetID = 2;
126126

127-
FSTDocumentKey *key1 = Key("documents/1");
128-
FSTDocumentKey *key2 = Key("documents/2");
127+
DocumentKey key1 = Key("documents/1");
128+
DocumentKey key2 = Key("documents/2");
129129

130130
std::string targetKeys[] = {
131131
LevelDbTargetKey::Key(targetID),

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#import "Firestore/Source/Core/FSTQuery.h"
3232
#import "Firestore/Source/Local/FSTQueryData.h"
3333
#import "Firestore/Source/Model/FSTDocument.h"
34-
#import "Firestore/Source/Model/FSTDocumentKey.h"
3534
#import "Firestore/Source/Model/FSTFieldValue.h"
3635
#import "Firestore/Source/Model/FSTMutation.h"
3736
#import "Firestore/Source/Model/FSTMutationBatch.h"

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

Lines changed: 49 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
#import "Firestore/Source/Local/FSTQueryCache.h"
2626
#import "Firestore/Source/Local/FSTQueryData.h"
2727
#import "Firestore/Source/Model/FSTDocument.h"
28-
#import "Firestore/Source/Model/FSTDocumentKey.h"
2928
#import "Firestore/Source/Model/FSTDocumentSet.h"
3029
#import "Firestore/Source/Model/FSTMutation.h"
3130
#import "Firestore/Source/Model/FSTMutationBatch.h"
@@ -40,15 +39,27 @@
4039
#import "Firestore/third_party/Immutable/Tests/FSTImmutableSortedSet+Testing.h"
4140

4241
#include "Firestore/core/src/firebase/firestore/auth/user.h"
42+
#include "Firestore/core/src/firebase/firestore/model/document_map.h"
4343
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
4444

4545
namespace testutil = firebase::firestore::testutil;
4646
using firebase::firestore::auth::User;
47+
using firebase::firestore::model::DocumentKey;
4748
using firebase::firestore::model::DocumentKeySet;
4849
using firebase::firestore::model::ListenSequenceNumber;
50+
using firebase::firestore::model::DocumentMap;
51+
using firebase::firestore::model::MaybeDocumentMap;
4952
using firebase::firestore::model::SnapshotVersion;
5053
using firebase::firestore::model::TargetId;
5154

55+
static NSArray<FSTDocument *> *docMapToArray(const DocumentMap &docs) {
56+
NSMutableArray<FSTDocument *> *result = [NSMutableArray array];
57+
for (const auto &kv : docs.underlying_map()) {
58+
[result addObject:static_cast<FSTDocument *>(kv.second)];
59+
}
60+
return result;
61+
}
62+
5263
NS_ASSUME_NONNULL_BEGIN
5364

5465
@interface FSTLocalStoreTests ()
@@ -57,12 +68,13 @@ @interface FSTLocalStoreTests ()
5768
@property(nonatomic, strong, readwrite) FSTLocalStore *localStore;
5869

5970
@property(nonatomic, strong, readonly) NSMutableArray<FSTMutationBatch *> *batches;
60-
@property(nonatomic, strong, readwrite, nullable) FSTMaybeDocumentDictionary *lastChanges;
6171
@property(nonatomic, assign, readwrite) TargetId lastTargetID;
6272

6373
@end
6474

65-
@implementation FSTLocalStoreTests
75+
@implementation FSTLocalStoreTests {
76+
MaybeDocumentMap _lastChanges;
77+
}
6678

6779
- (void)setUp {
6880
[super setUp];
@@ -78,7 +90,6 @@ - (void)setUp {
7890
[self.localStore start];
7991

8092
_batches = [NSMutableArray array];
81-
_lastChanges = nil;
8293
_lastTargetID = 0;
8394
}
8495

@@ -115,11 +126,11 @@ - (void)writeMutations:(NSArray<FSTMutation *> *)mutations {
115126
[self.batches addObject:[[FSTMutationBatch alloc] initWithBatchID:result.batchID
116127
localWriteTime:[FIRTimestamp timestamp]
117128
mutations:mutations]];
118-
self.lastChanges = result.changes;
129+
_lastChanges = result.changes;
119130
}
120131

121132
- (void)applyRemoteEvent:(FSTRemoteEvent *)event {
122-
self.lastChanges = [self.localStore applyRemoteEvent:event];
133+
_lastChanges = [self.localStore applyRemoteEvent:event];
123134
}
124135

125136
- (void)notifyLocalViewChanges:(FSTLocalViewChanges *)changes {
@@ -137,13 +148,13 @@ - (void)acknowledgeMutationWithVersion:(FSTTestSnapshotVersion)documentVersion {
137148
commitVersion:version
138149
mutationResults:@[ mutationResult ]
139150
streamToken:nil];
140-
self.lastChanges = [self.localStore acknowledgeBatchWithResult:result];
151+
_lastChanges = [self.localStore acknowledgeBatchWithResult:result];
141152
}
142153

143154
- (void)rejectMutation {
144155
FSTMutationBatch *batch = [self.batches firstObject];
145156
[self.batches removeObjectAtIndex:0];
146-
self.lastChanges = [self.localStore rejectBatchID:batch.batchID];
157+
_lastChanges = [self.localStore rejectBatchID:batch.batchID];
147158
}
148159

149160
- (TargetId)allocateQuery:(FSTQuery *)query {
@@ -159,34 +170,31 @@ - (TargetId)allocateQuery:(FSTQuery *)query {
159170
} while (0)
160171

161172
/** Asserts that a the lastChanges contain the docs in the given array. */
162-
#define FSTAssertChanged(documents) \
163-
XCTAssertNotNil(self.lastChanges); \
164-
do { \
165-
FSTMaybeDocumentDictionary *actual = self.lastChanges; \
166-
NSArray<FSTMaybeDocument *> *expected = (documents); \
167-
XCTAssertEqual(actual.count, expected.count); \
168-
NSEnumerator<FSTMaybeDocument *> *enumerator = expected.objectEnumerator; \
169-
[actual enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey * key, FSTMaybeDocument * value, \
170-
BOOL * stop) { \
171-
XCTAssertEqualObjects(value, [enumerator nextObject]); \
172-
}]; \
173-
self.lastChanges = nil; \
173+
#define FSTAssertChanged(documents) \
174+
do { \
175+
NSArray<FSTMaybeDocument *> *expected = (documents); \
176+
XCTAssertEqual(_lastChanges.size(), expected.count); \
177+
NSEnumerator<FSTMaybeDocument *> *enumerator = expected.objectEnumerator; \
178+
for (const auto &kv : _lastChanges) { \
179+
FSTMaybeDocument *value = kv.second; \
180+
XCTAssertEqualObjects(value, [enumerator nextObject]); \
181+
} \
182+
_lastChanges = MaybeDocumentMap{}; \
174183
} while (0)
175184

176185
/** Asserts that the given keys were removed. */
177-
#define FSTAssertRemoved(keyPaths) \
178-
XCTAssertNotNil(self.lastChanges); \
179-
do { \
180-
FSTMaybeDocumentDictionary *actual = self.lastChanges; \
181-
XCTAssertEqual(actual.count, keyPaths.count); \
182-
NSEnumerator<NSString *> *keyPathEnumerator = keyPaths.objectEnumerator; \
183-
[actual enumerateKeysAndObjectsUsingBlock:^(FSTDocumentKey * actualKey, \
184-
FSTMaybeDocument * value, BOOL * stop) { \
185-
FSTDocumentKey *expectedKey = FSTTestDocKey([keyPathEnumerator nextObject]); \
186-
XCTAssertEqualObjects(actualKey, expectedKey); \
187-
XCTAssertTrue([value isKindOfClass:[FSTDeletedDocument class]]); \
188-
}]; \
189-
self.lastChanges = nil; \
186+
#define FSTAssertRemoved(keyPaths) \
187+
do { \
188+
XCTAssertEqual(_lastChanges.size(), keyPaths.count); \
189+
NSEnumerator<NSString *> *keyPathEnumerator = keyPaths.objectEnumerator; \
190+
for (const auto &kv : _lastChanges) { \
191+
const DocumentKey &actualKey = kv.first; \
192+
FSTMaybeDocument *value = kv.second; \
193+
DocumentKey expectedKey = FSTTestDocKey([keyPathEnumerator nextObject]); \
194+
XCTAssertEqual(actualKey, expectedKey); \
195+
XCTAssertTrue([value isKindOfClass:[FSTDeletedDocument class]]); \
196+
} \
197+
_lastChanges = MaybeDocumentMap{}; \
190198
} while (0)
191199

192200
/** Asserts that the given local store contains the given document. */
@@ -200,7 +208,7 @@ - (TargetId)allocateQuery:(FSTQuery *)query {
200208
/** Asserts that the given local store does not contain the given document. */
201209
#define FSTAssertNotContains(keyPathString) \
202210
do { \
203-
FSTDocumentKey *key = FSTTestDocKey(keyPathString); \
211+
DocumentKey key = FSTTestDocKey(keyPathString); \
204212
FSTMaybeDocument *actual = [self.localStore readDocument:key]; \
205213
XCTAssertNil(actual); \
206214
} while (0)
@@ -834,9 +842,9 @@ - (void)testCanExecuteDocumentQueries {
834842
FSTTestSetMutation(@"foo/bar/Foo/Bar", @{@"Foo" : @"Bar"})
835843
]];
836844
FSTQuery *query = FSTTestQuery("foo/bar");
837-
FSTDocumentDictionary *docs = [self.localStore executeQuery:query];
838-
XCTAssertEqualObjects([docs values], @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"},
839-
FSTDocumentStateLocalMutations) ]);
845+
DocumentMap docs = [self.localStore executeQuery:query];
846+
XCTAssertEqualObjects(docMapToArray(docs), @[ FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"},
847+
FSTDocumentStateLocalMutations) ]);
840848
}
841849

842850
- (void)testCanExecuteCollectionQueries {
@@ -850,9 +858,9 @@ - (void)testCanExecuteCollectionQueries {
850858
FSTTestSetMutation(@"fooo/blah", @{@"fooo" : @"blah"})
851859
]];
852860
FSTQuery *query = FSTTestQuery("foo");
853-
FSTDocumentDictionary *docs = [self.localStore executeQuery:query];
861+
DocumentMap docs = [self.localStore executeQuery:query];
854862
XCTAssertEqualObjects(
855-
[docs values], (@[
863+
docMapToArray(docs), (@[
856864
FSTTestDoc("foo/bar", 0, @{@"foo" : @"bar"}, FSTDocumentStateLocalMutations),
857865
FSTTestDoc("foo/baz", 0, @{@"foo" : @"baz"}, FSTDocumentStateLocalMutations)
858866
]));
@@ -874,8 +882,8 @@ - (void)testCanExecuteMixedCollectionQueries {
874882

875883
[self.localStore locallyWriteMutations:@[ FSTTestSetMutation(@"foo/bonk", @{@"a" : @"b"}) ]];
876884

877-
FSTDocumentDictionary *docs = [self.localStore executeQuery:query];
878-
XCTAssertEqualObjects([docs values], (@[
885+
DocumentMap docs = [self.localStore executeQuery:query];
886+
XCTAssertEqualObjects(docMapToArray(docs), (@[
879887
FSTTestDoc("foo/bar", 20, @{@"a" : @"b"}, FSTDocumentStateSynced),
880888
FSTTestDoc("foo/baz", 10, @{@"a" : @"b"}, FSTDocumentStateSynced),
881889
FSTTestDoc("foo/bonk", 0, @{@"a" : @"b"}, FSTDocumentStateLocalMutations)

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919
#import <XCTest/XCTest.h>
2020

2121
#import "Firestore/Example/Tests/Util/FSTHelpers.h"
22-
#import "Firestore/Source/Model/FSTDocumentKey.h"
22+
23+
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
24+
25+
using firebase::firestore::model::DocumentKey;
2326

2427
NS_ASSUME_NONNULL_BEGIN
2528

@@ -29,7 +32,7 @@ @interface FSTReferenceSetTests : XCTestCase
2932
@implementation FSTReferenceSetTests
3033

3134
- (void)testAddOrRemoveReferences {
32-
FSTDocumentKey *key = FSTTestDocKey(@"foo/bar");
35+
DocumentKey key = FSTTestDocKey(@"foo/bar");
3336

3437
FSTReferenceSet *referenceSet = [[FSTReferenceSet alloc] init];
3538
XCTAssertTrue([referenceSet isEmpty]);
@@ -54,9 +57,9 @@ - (void)testAddOrRemoveReferences {
5457
}
5558

5659
- (void)testRemoveAllReferencesForTargetID {
57-
FSTDocumentKey *key1 = FSTTestDocKey(@"foo/bar");
58-
FSTDocumentKey *key2 = FSTTestDocKey(@"foo/baz");
59-
FSTDocumentKey *key3 = FSTTestDocKey(@"foo/blah");
60+
DocumentKey key1 = FSTTestDocKey(@"foo/bar");
61+
DocumentKey key2 = FSTTestDocKey(@"foo/baz");
62+
DocumentKey key3 = FSTTestDocKey(@"foo/blah");
6063
FSTReferenceSet *referenceSet = [[FSTReferenceSet alloc] init];
6164

6265
[referenceSet addReferenceToKey:key1 forID:1];

0 commit comments

Comments
 (0)