Skip to content

Commit e4dcc96

Browse files
wilhuffwilhuff
authored
Eliminate Objective-C BOOL in Firestore/core (#2574)
* Add objc::EqualTo and objc::Hash These make it possible to use std::unordered_map with Objective-C object keys. * Use std::unordered_map in MemoryQueryCache * Fix objective-C library detection This ensures that .mm files are compiled with -fobjc-arc (among other things). * Remove BOOL in FSTDocument to eliminate static_cast<bool>
1 parent 714bb34 commit e4dcc96

18 files changed

+221
-105
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ - (void)testRemoveQueriesUpThroughSequenceNumber {
398398
XCTAssertEqual(10, removed);
399399
// Make sure we removed the even targets with targetID <= 20.
400400
_persistence.run("verify remaining targets are > 20 or odd", [&]() {
401-
_queryCache->EnumerateTargets(^(FSTQueryData *queryData, BOOL *stop) {
401+
_queryCache->EnumerateTargets([&](FSTQueryData *queryData) {
402402
XCTAssertTrue(queryData.targetID > 20 || queryData.targetID % 2 == 1);
403403
});
404404
});

‎Firestore/Source/Core/FSTFirestoreClient.mm

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,9 @@ - (void)getDocumentFromLocalCache:(const DocumentReference &)doc
336336

337337
if ([maybeDoc isKindOfClass:[FSTDocument class]]) {
338338
FSTDocument *document = (FSTDocument *)maybeDoc;
339-
maybe_snapshot =
340-
DocumentSnapshot{doc.firestore(), doc.key(), document,
341-
/*from_cache=*/true,
342-
/*has_pending_writes=*/static_cast<bool>(document.hasLocalMutations)};
339+
maybe_snapshot = DocumentSnapshot{doc.firestore(), doc.key(), document,
340+
/*from_cache=*/true,
341+
/*has_pending_writes=*/document.hasLocalMutations};
343342
} else if ([maybeDoc isKindOfClass:[FSTDeletedDocument class]]) {
344343
maybe_snapshot = DocumentSnapshot{doc.firestore(), doc.key(), nil,
345344
/*from_cache=*/true,

‎Firestore/Source/Local/FSTLRUGarbageCollector.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121

2222
#import "FIRFirestoreSettings.h"
2323
#import "Firestore/Source/Local/FSTQueryData.h"
24+
25+
#include "Firestore/core/src/firebase/firestore/local/query_cache.h"
2426
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
2527
#include "Firestore/core/src/firebase/firestore/model/types.h"
2628

@@ -79,15 +81,13 @@ struct LruResults {
7981
* Enumerates all the targets that the delegate is aware of. This is typically all of the targets in
8082
* an FSTQueryCache.
8183
*/
82-
- (void)enumerateTargetsUsingBlock:(void (^)(FSTQueryData *queryData, BOOL *stop))block;
84+
- (void)enumerateTargetsUsingCallback:(const firebase::firestore::local::TargetCallback &)callback;
8385

8486
/**
8587
* Enumerates all of the outstanding mutations.
8688
*/
87-
- (void)enumerateMutationsUsingBlock:
88-
(void (^)(const firebase::firestore::model::DocumentKey &key,
89-
firebase::firestore::model::ListenSequenceNumber sequenceNumber,
90-
BOOL *stop))block;
89+
- (void)enumerateMutationsUsingCallback:
90+
(const firebase::firestore::local::OrphanedDocumentCallback &)callback;
9191

9292
/**
9393
* Removes all unreferenced documents from the cache that have a sequence number less than or equal

‎Firestore/Source/Local/FSTLRUGarbageCollector.mm

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -159,14 +159,13 @@ - (ListenSequenceNumber)sequenceNumberForQueryCount:(NSUInteger)queryCount {
159159
return kFSTListenSequenceNumberInvalid;
160160
}
161161
RollingSequenceNumberBuffer buffer(queryCount);
162-
// Pointer is necessary to access stack-allocated buffer from a block.
163-
RollingSequenceNumberBuffer *ptr_to_buffer = &buffer;
164-
[_delegate enumerateTargetsUsingBlock:^(FSTQueryData *queryData, BOOL *stop) {
165-
ptr_to_buffer->AddElement(queryData.sequenceNumber);
162+
163+
[_delegate enumerateTargetsUsingCallback:[&buffer](FSTQueryData *queryData) {
164+
buffer.AddElement(queryData.sequenceNumber);
166165
}];
167-
[_delegate enumerateMutationsUsingBlock:^(const DocumentKey &docKey,
168-
ListenSequenceNumber sequenceNumber, BOOL *stop) {
169-
ptr_to_buffer->AddElement(sequenceNumber);
166+
[_delegate enumerateMutationsUsingCallback:[&buffer](const DocumentKey &docKey,
167+
ListenSequenceNumber sequenceNumber) {
168+
buffer.AddElement(sequenceNumber);
170169
}];
171170
return buffer.max_value();
172171
}

‎Firestore/Source/Local/FSTLevelDB.mm

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,10 @@
7373
using firebase::firestore::local::LevelDbTransaction;
7474
using firebase::firestore::local::ListenSequence;
7575
using firebase::firestore::local::LruParams;
76+
using firebase::firestore::local::OrphanedDocumentCallback;
7677
using firebase::firestore::local::ReferenceSet;
7778
using firebase::firestore::local::RemoteDocumentCache;
79+
using firebase::firestore::local::TargetCallback;
7880
using firebase::firestore::model::DatabaseId;
7981
using firebase::firestore::model::DocumentKey;
8082
using firebase::firestore::model::ListenSequenceNumber;
@@ -210,19 +212,18 @@ - (BOOL)isPinned:(const DocumentKey &)docKey {
210212
return NO;
211213
}
212214

213-
- (void)enumerateTargetsUsingBlock:(void (^)(FSTQueryData *queryData, BOOL *stop))block {
214-
_db.queryCache->EnumerateTargets(block);
215+
- (void)enumerateTargetsUsingCallback:(const TargetCallback &)callback {
216+
_db.queryCache->EnumerateTargets(callback);
215217
}
216218

217-
- (void)enumerateMutationsUsingBlock:
218-
(void (^)(const DocumentKey &key, ListenSequenceNumber sequenceNumber, BOOL *stop))block {
219-
_db.queryCache->EnumerateOrphanedDocuments(block);
219+
- (void)enumerateMutationsUsingCallback:(const OrphanedDocumentCallback &)callback {
220+
_db.queryCache->EnumerateOrphanedDocuments(callback);
220221
}
221222

222223
- (int)removeOrphanedDocumentsThroughSequenceNumber:(ListenSequenceNumber)upperBound {
223-
__block int count = 0;
224+
int count = 0;
224225
_db.queryCache->EnumerateOrphanedDocuments(
225-
^(const DocumentKey &docKey, ListenSequenceNumber sequenceNumber, BOOL *stop) {
226+
[&count, self, upperBound](const DocumentKey &docKey, ListenSequenceNumber sequenceNumber) {
226227
if (sequenceNumber <= upperBound) {
227228
if (![self isPinned:docKey]) {
228229
count++;
@@ -245,9 +246,9 @@ - (int)removeTargetsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
245246
}
246247

247248
- (size_t)sequenceNumberCount {
248-
__block size_t totalCount = _db.queryCache->size();
249-
[self enumerateMutationsUsingBlock:^(const DocumentKey &key, ListenSequenceNumber sequenceNumber,
250-
BOOL *stop) {
249+
size_t totalCount = _db.queryCache->size();
250+
[self enumerateMutationsUsingCallback:[&totalCount](const DocumentKey &key,
251+
ListenSequenceNumber sequenceNumber) {
251252
totalCount++;
252253
}];
253254
return totalCount;

‎Firestore/Source/Local/FSTMemoryPersistence.mm

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
using firebase::firestore::local::MemoryQueryCache;
4343
using firebase::firestore::local::MemoryRemoteDocumentCache;
4444
using firebase::firestore::local::ReferenceSet;
45+
using firebase::firestore::local::TargetCallback;
4546
using firebase::firestore::model::DocumentKey;
4647
using firebase::firestore::model::DocumentKeyHash;
4748
using firebase::firestore::model::ListenSequenceNumber;
@@ -234,20 +235,19 @@ - (void)commitTransaction {
234235
_currentSequenceNumber = kFSTListenSequenceNumberInvalid;
235236
}
236237

237-
- (void)enumerateTargetsUsingBlock:(void (^)(FSTQueryData *queryData, BOOL *stop))block {
238-
return _persistence.queryCache->EnumerateTargets(block);
238+
- (void)enumerateTargetsUsingCallback:(const TargetCallback &)callback {
239+
return _persistence.queryCache->EnumerateTargets(callback);
239240
}
240241

241-
- (void)enumerateMutationsUsingBlock:
242-
(void (^)(const DocumentKey &key, ListenSequenceNumber sequenceNumber, BOOL *stop))block {
243-
BOOL stop = NO;
242+
- (void)enumerateMutationsUsingCallback:
243+
(const firebase::firestore::local::OrphanedDocumentCallback &)callback {
244244
for (const auto &entry : _sequenceNumbers) {
245245
ListenSequenceNumber sequenceNumber = entry.second;
246246
const DocumentKey &key = entry.first;
247247
// Pass in the exact sequence number as the upper bound so we know it won't be pinned by being
248248
// too recent.
249249
if (![self isPinnedAtSequenceNumber:sequenceNumber document:key]) {
250-
block(key, sequenceNumber, &stop);
250+
callback(key, sequenceNumber);
251251
}
252252
}
253253
}
@@ -259,9 +259,9 @@ - (int)removeTargetsThroughSequenceNumber:(ListenSequenceNumber)sequenceNumber
259259
}
260260

261261
- (size_t)sequenceNumberCount {
262-
__block size_t totalCount = _persistence.queryCache->size();
263-
[self enumerateMutationsUsingBlock:^(const DocumentKey &key, ListenSequenceNumber sequenceNumber,
264-
BOOL *stop) {
262+
size_t totalCount = _persistence.queryCache->size();
263+
[self enumerateMutationsUsingCallback:[&totalCount](const DocumentKey &key,
264+
ListenSequenceNumber sequenceNumber) {
265265
totalCount++;
266266
}];
267267
return totalCount;

‎Firestore/Source/Model/FSTDocument.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ typedef NS_ENUM(NSInteger, FSTDocumentState) {
4848
/**
4949
* Whether this document has a local mutation applied that has not yet been acknowledged by Watch.
5050
*/
51-
- (BOOL)hasPendingWrites;
51+
- (bool)hasPendingWrites;
5252

5353
@end
5454

@@ -65,8 +65,8 @@ typedef NS_ENUM(NSInteger, FSTDocumentState) {
6565
proto:(GCFSDocument *)proto;
6666

6767
- (nullable FSTFieldValue *)fieldForPath:(const firebase::firestore::model::FieldPath &)path;
68-
- (BOOL)hasLocalMutations;
69-
- (BOOL)hasCommittedMutations;
68+
- (bool)hasLocalMutations;
69+
- (bool)hasCommittedMutations;
7070

7171
@property(nonatomic, strong, readonly) FSTObjectValue *data;
7272

@@ -81,9 +81,9 @@ typedef NS_ENUM(NSInteger, FSTDocumentState) {
8181
@interface FSTDeletedDocument : FSTMaybeDocument
8282
+ (instancetype)documentWithKey:(firebase::firestore::model::DocumentKey)key
8383
version:(firebase::firestore::model::SnapshotVersion)version
84-
hasCommittedMutations:(BOOL)committedMutations;
84+
hasCommittedMutations:(bool)committedMutations;
8585

86-
- (BOOL)hasCommittedMutations;
86+
- (bool)hasCommittedMutations;
8787

8888
@end
8989

‎Firestore/Source/Model/FSTDocument.mm

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ - (instancetype)initWithKey:(DocumentKey)key version:(SnapshotVersion)version {
5555
return self;
5656
}
5757

58-
- (BOOL)hasPendingWrites {
58+
- (bool)hasPendingWrites {
5959
@throw FSTAbstractMethodException(); // NOLINT
6060
}
6161

@@ -127,15 +127,15 @@ - (instancetype)initWithData:(FSTObjectValue *)data
127127
return self;
128128
}
129129

130-
- (BOOL)hasLocalMutations {
130+
- (bool)hasLocalMutations {
131131
return _documentState == FSTDocumentStateLocalMutations;
132132
}
133133

134-
- (BOOL)hasCommittedMutations {
134+
- (bool)hasCommittedMutations {
135135
return _documentState == FSTDocumentStateCommittedMutations;
136136
}
137137

138-
- (BOOL)hasPendingWrites {
138+
- (bool)hasPendingWrites {
139139
return self.hasLocalMutations || self.hasCommittedMutations;
140140
}
141141

@@ -174,12 +174,12 @@ - (nullable FSTFieldValue *)fieldForPath:(const FieldPath &)path {
174174
@end
175175

176176
@implementation FSTDeletedDocument {
177-
BOOL _hasCommittedMutations;
177+
bool _hasCommittedMutations;
178178
}
179179

180180
+ (instancetype)documentWithKey:(DocumentKey)key
181181
version:(SnapshotVersion)version
182-
hasCommittedMutations:(BOOL)committedMutations {
182+
hasCommittedMutations:(bool)committedMutations {
183183
FSTDeletedDocument *deletedDocument = [[FSTDeletedDocument alloc] initWithKey:std::move(key)
184184
version:std::move(version)];
185185

@@ -190,11 +190,11 @@ + (instancetype)documentWithKey:(DocumentKey)key
190190
return deletedDocument;
191191
}
192192

193-
- (BOOL)hasCommittedMutations {
193+
- (bool)hasCommittedMutations {
194194
return _hasCommittedMutations;
195195
}
196196

197-
- (BOOL)hasPendingWrites {
197+
- (bool)hasPendingWrites {
198198
return self.hasCommittedMutations;
199199
}
200200

@@ -233,8 +233,8 @@ + (instancetype)documentWithKey:(DocumentKey)key version:(SnapshotVersion)versio
233233
return [[FSTUnknownDocument alloc] initWithKey:std::move(key) version:std::move(version)];
234234
}
235235

236-
- (BOOL)hasPendingWrites {
237-
return YES;
236+
- (bool)hasPendingWrites {
237+
return true;
238238
}
239239

240240
- (BOOL)isEqual:(id)other {

‎Firestore/core/src/firebase/firestore/local/CMakeLists.txt

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,18 @@ if(HAVE_LEVELDB)
1616
cc_library(
1717
firebase_firestore_local_persistence_leveldb
1818
SOURCES
19+
leveldb_index_manager.h
20+
#leveldb_index_manager.mm
1921
leveldb_key.cc
2022
leveldb_key.h
21-
#leveldb_index_manager.mm
2223
leveldb_migrations.cc
2324
leveldb_migrations.h
25+
leveldb_mutation_queue.h
26+
#leveldb_mutation_queue.mm
27+
leveldb_query_cache.h
28+
#leveldb_query_cache.mm
29+
leveldb_remote_document_cache.h
30+
#leveldb_remote_document_cache.mm
2431
leveldb_transaction.cc
2532
leveldb_transaction.h
2633
leveldb_util.cc
@@ -49,13 +56,25 @@ cc_library(
4956
SOURCES
5057
document_reference.h
5158
document_reference.cc
59+
index_manager.h
60+
listen_sequence.h
5261
local_serializer.h
5362
local_serializer.cc
5463
memory_index_manager.cc
64+
memory_index_manager.h
65+
memory_mutation_queue.h
66+
#memory_mutation_queue.mm
67+
memory_query_cache.h
68+
#memory_query_cache.mm
69+
memory_remote_document_cache.h
70+
#memory_remote_document_cache.mm
71+
mutation_queue.h
72+
query_cache.h
5573
query_data.cc
5674
query_data.h
5775
reference_set.cc
5876
reference_set.h
77+
remote_document_cache.h
5978
DEPENDS
6079
# TODO(b/111328563) Force nanopb first to work around ODR violations
6180
protobuf-nanopb

‎Firestore/core/src/firebase/firestore/local/leveldb_query_cache.h

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,6 @@ namespace local {
4747
/** Cached Queries backed by LevelDB. */
4848
class LevelDbQueryCache : public QueryCache {
4949
public:
50-
/** Enumerator callback type for orphaned documents */
51-
typedef void (^OrphanedDocumentEnumerator)(const model::DocumentKey&,
52-
model::ListenSequenceNumber,
53-
BOOL*);
54-
5550
/**
5651
* Retrieves the global singleton metadata row from the given database, if it
5752
* exists.
@@ -75,7 +70,7 @@ class LevelDbQueryCache : public QueryCache {
7570

7671
FSTQueryData* _Nullable GetTarget(FSTQuery* query) override;
7772

78-
void EnumerateTargets(TargetEnumerator block) override;
73+
void EnumerateTargets(const TargetCallback& callback) override;
7974

8075
int RemoveTargets(model::ListenSequenceNumber upper_bound,
8176
const std::unordered_map<model::TargetId, FSTQueryData*>&
@@ -123,7 +118,7 @@ class LevelDbQueryCache : public QueryCache {
123118
// Non-interface methods
124119
void Start();
125120

126-
void EnumerateOrphanedDocuments(OrphanedDocumentEnumerator block);
121+
void EnumerateOrphanedDocuments(const OrphanedDocumentCallback& callback);
127122

128123
private:
129124
void Save(FSTQueryData* query_data);

0 commit comments

Comments
 (0)