Skip to content

Commit 900ca3b

Browse files
author
Greg Soltis
authored
Implement QueryCache interface and port QueryCache tests (#2209)
* Implement QueryCache interface and port tests * Port production usages of QueryCache (#2211) * Remove FSTQueryCache and implementations * Switch size() to size_t
1 parent 466880b commit 900ca3b

22 files changed

+410
-761
lines changed

Firestore/Example/Tests/Local/FSTLRUGarbageCollectorTests.mm

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@
2525
#import "Firestore/Source/Local/FSTLRUGarbageCollector.h"
2626
#import "Firestore/Source/Local/FSTMutationQueue.h"
2727
#import "Firestore/Source/Local/FSTPersistence.h"
28-
#import "Firestore/Source/Local/FSTQueryCache.h"
2928
#import "Firestore/Source/Model/FSTDocument.h"
3029
#import "Firestore/Source/Model/FSTFieldValue.h"
3130
#import "Firestore/Source/Model/FSTMutation.h"
3231
#import "Firestore/Source/Util/FSTClasses.h"
3332
#include "Firestore/core/src/firebase/firestore/auth/user.h"
33+
#include "Firestore/core/src/firebase/firestore/local/query_cache.h"
3434
#include "Firestore/core/src/firebase/firestore/local/reference_set.h"
3535
#include "Firestore/core/src/firebase/firestore/local/remote_document_cache.h"
3636
#include "Firestore/core/src/firebase/firestore/model/document_key_set.h"
@@ -43,6 +43,7 @@
4343
using firebase::firestore::auth::User;
4444
using firebase::firestore::local::LruParams;
4545
using firebase::firestore::local::LruResults;
46+
using firebase::firestore::local::QueryCache;
4647
using firebase::firestore::local::ReferenceSet;
4748
using firebase::firestore::local::RemoteDocumentCache;
4849
using firebase::firestore::model::DocumentKey;
@@ -60,7 +61,7 @@ @implementation FSTLRUGarbageCollectorTests {
6061
FSTObjectValue *_testValue;
6162
FSTObjectValue *_bigObjectValue;
6263
id<FSTPersistence> _persistence;
63-
id<FSTQueryCache> _queryCache;
64+
QueryCache *_queryCache;
6465
RemoteDocumentCache *_documentCache;
6566
id<FSTMutationQueue> _mutationQueue;
6667
id<FSTLRUDelegate> _lruDelegate;
@@ -155,7 +156,7 @@ - (FSTQueryData *)nextTestQuery {
155156

156157
- (FSTQueryData *)addNextQueryInTransaction {
157158
FSTQueryData *queryData = [self nextTestQuery];
158-
[_queryCache addQueryData:queryData];
159+
_queryCache->AddTarget(queryData);
159160
return queryData;
160161
}
161162

@@ -165,7 +166,7 @@ - (void)updateTargetInTransaction:(FSTQueryData *)queryData {
165166
[queryData queryDataByReplacingSnapshotVersion:queryData.snapshotVersion
166167
resumeToken:token
167168
sequenceNumber:_persistence.currentSequenceNumber];
168-
[_queryCache updateQueryData:updated];
169+
_queryCache->UpdateTarget(updated);
169170
}
170171

171172
- (FSTQueryData *)addNextQuery {
@@ -199,11 +200,11 @@ - (void)markDocumentEligibleForGCInTransaction:(const DocumentKey &)docKey {
199200
}
200201

201202
- (void)addDocument:(const DocumentKey &)docKey toTarget:(TargetId)targetId {
202-
[_queryCache addMatchingKeys:DocumentKeySet{docKey} forTargetID:targetId];
203+
_queryCache->AddMatchingKeys(DocumentKeySet{docKey}, targetId);
203204
}
204205

205206
- (void)removeDocument:(const DocumentKey &)docKey fromTarget:(TargetId)targetId {
206-
[_queryCache removeMatchingKeys:DocumentKeySet{docKey} forTargetID:targetId];
207+
_queryCache->RemoveMatchingKeys(DocumentKeySet{docKey}, targetId);
207208
}
208209

209210
/**
@@ -392,9 +393,9 @@ - (void)testRemoveQueriesUpThroughSequenceNumber {
392393
XCTAssertEqual(10, removed);
393394
// Make sure we removed the even targets with targetID <= 20.
394395
_persistence.run("verify remaining targets are > 20 or odd", [&]() {
395-
[_queryCache enumerateTargetsUsingBlock:^(FSTQueryData *queryData, BOOL *stop) {
396+
_queryCache->EnumerateTargets(^(FSTQueryData *queryData, BOOL *stop) {
396397
XCTAssertTrue(queryData.targetID > 20 || queryData.targetID % 2 == 1);
397-
}];
398+
});
398399
});
399400
[_persistence shutdown];
400401
}
@@ -467,7 +468,7 @@ - (void)testRemoveOrphanedDocuments {
467468
_persistence.run("verify", [&]() {
468469
for (const DocumentKey &key : toBeRemoved) {
469470
XCTAssertNil(_documentCache->Get(key));
470-
XCTAssertFalse([_queryCache containsKey:key]);
471+
XCTAssertFalse(_queryCache->Contains(key));
471472
}
472473
for (const DocumentKey &key : expectedRetained) {
473474
XCTAssertNotNil(_documentCache->Get(key), @"Missing document %s", key.ToString().c_str());
@@ -649,7 +650,7 @@ - (void)testRemoveTargetsThenGC {
649650
for (const DocumentKey &key : expectedRemoved) {
650651
XCTAssertNil(_documentCache->Get(key), @"Did not expect to find %s in document cache",
651652
key.ToString().c_str());
652-
XCTAssertFalse([_queryCache containsKey:key], @"Did not expect to find %s in queryCache",
653+
XCTAssertFalse(_queryCache->Contains(key), @"Did not expect to find %s in queryCache",
653654
key.ToString().c_str());
654655
[self expectSentinelRemoved:key];
655656
}

Firestore/Example/Tests/Local/FSTLevelDBMigrationsTests.mm

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@
2424
#import "Firestore/Protos/objc/firestore/local/Target.pbobjc.h"
2525
#import "Firestore/Source/Local/FSTLevelDB.h"
2626
#import "Firestore/Source/Local/FSTLevelDBMutationQueue.h"
27-
#import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
2827

2928
#include "Firestore/core/src/firebase/firestore/local/leveldb_key.h"
3029
#include "Firestore/core/src/firebase/firestore/local/leveldb_migrations.h"
30+
#include "Firestore/core/src/firebase/firestore/local/leveldb_query_cache.h"
3131
#include "Firestore/core/src/firebase/firestore/util/ordered_code.h"
3232
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
3333
#include "absl/strings/match.h"
@@ -43,6 +43,7 @@
4343
using firebase::firestore::local::LevelDbMigrations;
4444
using firebase::firestore::local::LevelDbMutationKey;
4545
using firebase::firestore::local::LevelDbMutationQueueKey;
46+
using firebase::firestore::local::LevelDbQueryCache;
4647
using firebase::firestore::local::LevelDbQueryTargetKey;
4748
using firebase::firestore::local::LevelDbRemoteDocumentKey;
4849
using firebase::firestore::local::LevelDbTargetDocumentKey;
@@ -86,11 +87,11 @@ - (void)tearDown {
8687
}
8788

8889
- (void)testAddsTargetGlobal {
89-
FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
90+
FSTPBTargetGlobal *metadata = LevelDbQueryCache::ReadMetadata(_db.get());
9091
XCTAssertNil(metadata, @"Not expecting metadata yet, we should have an empty db");
9192
LevelDbMigrations::RunMigrations(_db.get());
9293

93-
metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
94+
metadata = LevelDbQueryCache::ReadMetadata(_db.get());
9495
XCTAssertNotNil(metadata, @"Migrations should have added the metadata");
9596
}
9697

@@ -166,7 +167,7 @@ - (void)testDropsTheQueryCache {
166167
ASSERT_FOUND(transaction, key);
167168
}
168169

169-
FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
170+
FSTPBTargetGlobal *metadata = LevelDbQueryCache::ReadMetadata(_db.get());
170171
XCTAssertNotNil(metadata, @"Metadata should have been added");
171172
XCTAssertEqual(metadata.targetCount, 0);
172173
}
@@ -209,7 +210,7 @@ - (void)testAddsSentinelRows {
209210
LevelDbTransaction transaction(_db.get(), "Setup");
210211

211212
// Set up target global
212-
FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
213+
FSTPBTargetGlobal *metadata = LevelDbQueryCache::ReadMetadata(_db.get());
213214
// Expect that documents missing a row will get the new number
214215
metadata.highestListenSequenceNumber = new_sequence_number;
215216
transaction.Put(LevelDbTargetGlobalKey::Key(), metadata);

Firestore/Example/Tests/Local/FSTLevelDBQueryCacheTests.mm

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,28 @@
1414
* limitations under the License.
1515
*/
1616

17-
#import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
18-
1917
#import "Firestore/Source/Core/FSTQuery.h"
2018
#import "Firestore/Source/Local/FSTLevelDB.h"
21-
#import "Firestore/Source/Local/FSTLocalSerializer.h"
2219
#import "Firestore/Source/Local/FSTQueryData.h"
23-
#import "Firestore/Source/Remote/FSTSerializerBeta.h"
2420

2521
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
2622
#import "Firestore/Example/Tests/Local/FSTQueryCacheTests.h"
2723

2824
#include "Firestore/core/include/firebase/firestore/timestamp.h"
25+
#include "Firestore/core/src/firebase/firestore/local/leveldb_query_cache.h"
2926
#include "Firestore/core/src/firebase/firestore/local/reference_set.h"
3027
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
28+
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
3129
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
3230
#include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
31+
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
3332

33+
namespace testutil = firebase::firestore::testutil;
3434
using firebase::Timestamp;
35+
using firebase::firestore::local::LevelDbQueryCache;
3536
using firebase::firestore::local::ReferenceSet;
3637
using firebase::firestore::model::DatabaseId;
38+
using firebase::firestore::model::DocumentKey;
3739
using firebase::firestore::model::ListenSequenceNumber;
3840
using firebase::firestore::model::ResourcePath;
3941
using firebase::firestore::model::SnapshotVersion;
@@ -54,11 +56,15 @@ @implementation FSTLevelDBQueryCacheTests {
5456
ReferenceSet _additionalReferences;
5557
}
5658

59+
- (LevelDbQueryCache *)getCache:(id<FSTPersistence>)persistence {
60+
return static_cast<LevelDbQueryCache *>(persistence.queryCache);
61+
}
62+
5763
- (void)setUp {
5864
[super setUp];
5965

6066
self.persistence = [FSTPersistenceTestHelpers levelDBPersistence];
61-
self.queryCache = [self.persistence queryCache];
67+
self.queryCache = [self getCache:self.persistence];
6268
[self.persistence.referenceDelegate addInMemoryPins:&_additionalReferences];
6369
}
6470

@@ -75,12 +81,12 @@ - (void)testMetadataPersistedAcrossRestarts {
7581
Path dir = [FSTPersistenceTestHelpers levelDBDir];
7682

7783
FSTLevelDB *db1 = [FSTPersistenceTestHelpers levelDBPersistenceWithDir:dir];
78-
FSTLevelDBQueryCache *queryCache = [db1 queryCache];
84+
LevelDbQueryCache *queryCache = [self getCache:db1];
7985

80-
XCTAssertEqual(0, queryCache.highestListenSequenceNumber);
81-
XCTAssertEqual(0, queryCache.highestTargetID);
86+
XCTAssertEqual(0, queryCache->highest_listen_sequence_number());
87+
XCTAssertEqual(0, queryCache->highest_target_id());
8288
SnapshotVersion versionZero;
83-
XCTAssertEqual(versionZero, queryCache.lastRemoteSnapshotVersion);
89+
XCTAssertEqual(versionZero, queryCache->GetLastRemoteSnapshotVersion());
8490

8591
ListenSequenceNumber minimumSequenceNumber = 1234;
8692
TargetId lastTargetId = 5;
@@ -92,8 +98,8 @@ - (void)testMetadataPersistedAcrossRestarts {
9298
targetID:lastTargetId
9399
listenSequenceNumber:minimumSequenceNumber
94100
purpose:FSTQueryPurposeListen];
95-
[queryCache addQueryData:queryData];
96-
[queryCache setLastRemoteSnapshotVersion:lastVersion];
101+
queryCache->AddTarget(queryData);
102+
queryCache->SetLastRemoteSnapshotVersion(lastVersion);
97103
});
98104

99105
[db1 shutdown];
@@ -106,14 +112,40 @@ - (void)testMetadataPersistedAcrossRestarts {
106112
XCTAssertGreaterThan(db2.currentSequenceNumber, minimumSequenceNumber);
107113
});
108114

109-
FSTLevelDBQueryCache *queryCache2 = [db2 queryCache];
110-
XCTAssertEqual(lastTargetId, queryCache2.highestTargetID);
111-
XCTAssertEqual(lastVersion, queryCache2.lastRemoteSnapshotVersion);
115+
LevelDbQueryCache *queryCache2 = [self getCache:db2];
116+
XCTAssertEqual(lastTargetId, queryCache2->highest_target_id());
117+
XCTAssertEqual(lastVersion, queryCache2->GetLastRemoteSnapshotVersion());
112118

113119
[db2 shutdown];
114120
db2 = nil;
115121
}
116122

123+
- (void)testRemoveMatchingKeysForTargetID {
124+
self.persistence.run("testRemoveMatchingKeysForTargetID", [&]() {
125+
DocumentKey key1 = testutil::Key("foo/bar");
126+
DocumentKey key2 = testutil::Key("foo/baz");
127+
DocumentKey key3 = testutil::Key("foo/blah");
128+
129+
LevelDbQueryCache *cache = [self getCache:self.persistence];
130+
[self addMatchingKey:key1 forTargetID:1];
131+
[self addMatchingKey:key2 forTargetID:1];
132+
[self addMatchingKey:key3 forTargetID:2];
133+
XCTAssertTrue(cache->Contains(key1));
134+
XCTAssertTrue(cache->Contains(key2));
135+
XCTAssertTrue(cache->Contains(key3));
136+
137+
cache->RemoveAllKeysForTarget(1);
138+
XCTAssertFalse(self.queryCache->Contains(key1));
139+
XCTAssertFalse(self.queryCache->Contains(key2));
140+
XCTAssertTrue(self.queryCache->Contains(key3));
141+
142+
cache->RemoveAllKeysForTarget(2);
143+
XCTAssertFalse(self.queryCache->Contains(key1));
144+
XCTAssertFalse(self.queryCache->Contains(key2));
145+
XCTAssertFalse(self.queryCache->Contains(key3));
146+
});
147+
}
148+
117149
@end
118150

119151
NS_ASSUME_NONNULL_END

Firestore/Example/Tests/Local/FSTLocalStoreTests.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/Local/FSTLocalWriteResult.h"
2424
#import "Firestore/Source/Local/FSTPersistence.h"
25-
#import "Firestore/Source/Local/FSTQueryCache.h"
2625
#import "Firestore/Source/Local/FSTQueryData.h"
2726
#import "Firestore/Source/Model/FSTDocument.h"
2827
#import "Firestore/Source/Model/FSTDocumentSet.h"

Firestore/Example/Tests/Local/FSTMemoryQueryCacheTests.mm

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
* limitations under the License.
1515
*/
1616

17-
#import "Firestore/Source/Local/FSTMemoryQueryCache.h"
18-
1917
#import "Firestore/Source/Local/FSTMemoryPersistence.h"
2018

2119
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
@@ -43,7 +41,7 @@ - (void)setUp {
4341
[super setUp];
4442

4543
self.persistence = [FSTPersistenceTestHelpers eagerGCMemoryPersistence];
46-
self.queryCache = [self.persistence queryCache];
44+
self.queryCache = self.persistence.queryCache;
4745
[self.persistence.referenceDelegate addInMemoryPins:&_additionalReferences];
4846
}
4947

Firestore/Example/Tests/Local/FSTQueryCacheTests.h

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,33 @@
1414
* limitations under the License.
1515
*/
1616

17-
#import "Firestore/Source/Local/FSTQueryCache.h"
18-
1917
#import <XCTest/XCTest.h>
2018

19+
#include "Firestore/core/src/firebase/firestore/local/query_cache.h"
20+
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
21+
#include "Firestore/core/src/firebase/firestore/model/types.h"
22+
2123
@protocol FSTPersistence;
2224

2325
NS_ASSUME_NONNULL_BEGIN
2426

2527
/**
26-
* These are tests for any implementation of the FSTQueryCache protocol.
28+
* These are tests for any implementation of the QueryCache interface.
2729
*
28-
* To test a specific implementation of FSTQueryCache:
30+
* To test a specific implementation of QueryCache:
2931
*
3032
* + Subclass FSTQueryCacheTests
3133
* + override -setUp, assigning to queryCache and persistence
3234
* + override -tearDown, cleaning up queryCache and persistence
3335
*/
3436
@interface FSTQueryCacheTests : XCTestCase
3537

38+
/** Helper method to add a single document key to target association */
39+
- (void)addMatchingKey:(const firebase::firestore::model::DocumentKey&)key
40+
forTargetID:(firebase::firestore::model::TargetId)targetID;
41+
3642
/** The implementation of the query cache to test. */
37-
@property(nonatomic, strong, nullable) id<FSTQueryCache> queryCache;
43+
@property(nonatomic, nullable) firebase::firestore::local::QueryCache* queryCache;
3844

3945
/**
4046
* The persistence implementation to use while testing the queryCache (e.g. for committing write

0 commit comments

Comments
 (0)