Skip to content

Commit 756da9c

Browse files
wilhuffwilhuff
authored
Add a schema migration that drops the query cache (#1558)
This is a force fix for potential existence filter mismatches caused by #1548 The essential problem is that when resuming a query, the server is allowed to forget deletes. If the number of incorrectly synthesized deletes matches the number of server-forgotten deletes then the existence filter can give a false positive, preventing the cache from self healing. Dropping the query cache clears any client-side resume token which prevents a false positive existence filter mismatch. Note that the remote document cache and mutation queues are unaffected so any cached documents that do exist will still work while offline firebase/firebase-js-sdk#1019
1 parent fe0952c commit 756da9c

File tree

6 files changed

+192
-106
lines changed

6 files changed

+192
-106
lines changed

‎Firestore/Example/Firestore.xcodeproj/project.pbxproj

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,24 +1431,6 @@
14311431
shellScript = "\"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_Tests_iOS/Pods-Firestore_Tests_iOS-frameworks.sh\"\n";
14321432
showEnvVarsInLog = 0;
14331433
};
1434-
5504F81EEBBEF943CA61D32C /* [CP] Check Pods Manifest.lock */ = {
1435-
isa = PBXShellScriptBuildPhase;
1436-
buildActionMask = 2147483647;
1437-
files = (
1438-
);
1439-
inputPaths = (
1440-
"${PODS_PODFILE_DIR_PATH}/Podfile.lock",
1441-
"${PODS_ROOT}/Manifest.lock",
1442-
);
1443-
name = "[CP] Check Pods Manifest.lock";
1444-
outputPaths = (
1445-
"$(DERIVED_FILE_DIR)/Pods-Firestore_Example_iOS-SwiftBuildTest-checkManifestLockResult.txt",
1446-
);
1447-
runOnlyForDeploymentPostprocessing = 0;
1448-
shellPath = /bin/sh;
1449-
shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n # print error to STDERR\n echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n";
1450-
showEnvVarsInLog = 0;
1451-
};
14521434
6E622C7A20F52C8300B7E93A /* Run Script */ = {
14531435
isa = PBXShellScriptBuildPhase;
14541436
buildActionMask = 12;
@@ -1637,8 +1619,8 @@
16371619
isa = PBXSourcesBuildPhase;
16381620
buildActionMask = 2147483647;
16391621
files = (
1640-
5495EB032040E90200EBA509 /* CodableGeoPointTests.swift in Sources */,
16411622
544A20EE20F6C10C004E52CD /* BasicCompileTests.swift in Sources */,
1623+
5495EB032040E90200EBA509 /* CodableGeoPointTests.swift in Sources */,
16421624
);
16431625
runOnlyForDeploymentPostprocessing = 0;
16441626
};

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

Lines changed: 119 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,29 @@
1818

1919
#include <memory>
2020
#include <string>
21+
#include <vector>
2122

2223
#import "Firestore/Protos/objc/firestore/local/Target.pbobjc.h"
2324
#import "Firestore/Source/Local/FSTLevelDB.h"
2425
#import "Firestore/Source/Local/FSTLevelDBKey.h"
2526
#import "Firestore/Source/Local/FSTLevelDBMigrations.h"
27+
#import "Firestore/Source/Local/FSTLevelDBMutationQueue.h"
2628
#import "Firestore/Source/Local/FSTLevelDBQueryCache.h"
2729

2830
#include "Firestore/core/src/firebase/firestore/util/ordered_code.h"
31+
#include "Firestore/core/src/firebase/firestore/util/status.h"
32+
#include "Firestore/core/test/firebase/firestore/testutil/testutil.h"
33+
#include "absl/strings/match.h"
2934
#include "leveldb/db.h"
3035

3136
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
3237

3338
NS_ASSUME_NONNULL_BEGIN
3439

40+
using firebase::firestore::FirestoreErrorCode;
3541
using firebase::firestore::local::LevelDbTransaction;
3642
using firebase::firestore::util::OrderedCode;
43+
using firebase::firestore::testutil::Key;
3744
using leveldb::DB;
3845
using leveldb::Options;
3946
using leveldb::Status;
@@ -64,54 +71,136 @@ - (void)tearDown {
6471
- (void)testAddsTargetGlobal {
6572
FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
6673
XCTAssertNil(metadata, @"Not expecting metadata yet, we should have an empty db");
67-
LevelDbTransaction transaction(_db.get(), "testAddsTargetGlobal");
68-
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
69-
transaction.Commit();
74+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get()];
75+
7076
metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
7177
XCTAssertNotNil(metadata, @"Migrations should have added the metadata");
7278
}
7379

7480
- (void)testSetsVersionNumber {
75-
LevelDbTransaction transaction(_db.get(), "testSetsVersionNumber");
76-
FSTLevelDBSchemaVersion initial =
77-
[FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
78-
XCTAssertEqual(0, initial, "No version should be equivalent to 0");
79-
80-
// Pick an arbitrary high migration number and migrate to it.
81-
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
82-
FSTLevelDBSchemaVersion actual = [FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
83-
XCTAssertGreaterThan(actual, 0, @"Expected to migrate to a schema version > 0");
81+
{
82+
LevelDbTransaction transaction(_db.get(), "testSetsVersionNumber before");
83+
FSTLevelDBSchemaVersion initial =
84+
[FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
85+
XCTAssertEqual(0, initial, "No version should be equivalent to 0");
86+
}
87+
88+
{
89+
// Pick an arbitrary high migration number and migrate to it.
90+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get()];
91+
92+
LevelDbTransaction transaction(_db.get(), "testSetsVersionNumber after");
93+
FSTLevelDBSchemaVersion actual =
94+
[FSTLevelDBMigrations schemaVersionWithTransaction:&transaction];
95+
XCTAssertGreaterThan(actual, 0, @"Expected to migrate to a schema version > 0");
96+
}
8497
}
8598

86-
- (void)testCountsQueries {
87-
NSUInteger expected = 50;
99+
#define ASSERT_NOT_FOUND(transaction, key) \
100+
do { \
101+
std::string unused_result; \
102+
Status status = transaction.Get(key, &unused_result); \
103+
XCTAssertTrue(status.IsNotFound()); \
104+
} while (0)
105+
106+
#define ASSERT_FOUND(transaction, key) \
107+
do { \
108+
std::string unused_result; \
109+
Status status = transaction.Get(key, &unused_result); \
110+
XCTAssertTrue(status.ok()); \
111+
} while (0)
112+
113+
- (void)testDropsTheQueryCache {
114+
NSString *userID = @"user";
115+
FSTBatchID batchID = 1;
116+
FSTTargetID targetID = 2;
117+
118+
FSTDocumentKey *key1 = Key("documents/1");
119+
FSTDocumentKey *key2 = Key("documents/2");
120+
121+
std::string targetKeys[] = {
122+
[FSTLevelDBTargetKey keyWithTargetID:targetID],
123+
[FSTLevelDBTargetDocumentKey keyWithTargetID:targetID documentKey:key1],
124+
[FSTLevelDBTargetDocumentKey keyWithTargetID:targetID documentKey:key2],
125+
[FSTLevelDBDocumentTargetKey keyWithDocumentKey:key1 targetID:targetID],
126+
[FSTLevelDBDocumentTargetKey keyWithDocumentKey:key2 targetID:targetID],
127+
[FSTLevelDBQueryTargetKey keyWithCanonicalID:"foo.bar.baz" targetID:targetID],
128+
};
129+
130+
// Keys that should not be modified by the dropping the query cache
131+
std::string preservedKeys[] = {
132+
[self dummyKeyForTable:"targetA"],
133+
[FSTLevelDBMutationQueueKey keyWithUserID:userID],
134+
[FSTLevelDBMutationKey keyWithUserID:userID batchID:batchID],
135+
};
136+
137+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get() upToVersion:2];
88138
{
89139
// Setup some targets to be counted in the migration.
90-
LevelDbTransaction transaction(_db.get(), "testCountsQueries setup");
91-
for (int i = 0; i < expected; i++) {
92-
std::string key = [FSTLevelDBTargetKey keyWithTargetID:i];
93-
transaction.Put(key, "dummy");
140+
LevelDbTransaction transaction(_db.get(), "testDropsTheQueryCache setup");
141+
for (const std::string &key : targetKeys) {
142+
transaction.Put(key, "target");
143+
}
144+
for (const std::string &key : preservedKeys) {
145+
transaction.Put(key, "preserved");
94146
}
95-
// Add a dummy entry after the targets to make sure the iteration is correctly bounded.
96-
// Use a table that would sort logically right after that table 'target'.
97-
std::string dummyKey;
98-
// Magic number that indicates a table name follows. Needed to mimic the prefix to the target
99-
// table.
100-
OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
101-
OrderedCode::WriteString(&dummyKey, "targetA");
102-
transaction.Put(dummyKey, "dummy");
103147
transaction.Commit();
104148
}
105149

150+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get() upToVersion:3];
106151
{
107-
LevelDbTransaction transaction(_db.get(), "testCountsQueries");
108-
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
109-
transaction.Commit();
152+
LevelDbTransaction transaction(_db.get(), "testDropsTheQueryCache");
153+
for (const std::string &key : targetKeys) {
154+
ASSERT_NOT_FOUND(transaction, key);
155+
}
156+
for (const std::string &key : preservedKeys) {
157+
ASSERT_FOUND(transaction, key);
158+
}
159+
110160
FSTPBTargetGlobal *metadata = [FSTLevelDBQueryCache readTargetMetadataFromDB:_db.get()];
111-
XCTAssertEqual(expected, metadata.targetCount, @"Failed to count all of the targets we added");
161+
XCTAssertNotNil(metadata, @"Metadata should have been added");
162+
XCTAssertEqual(metadata.targetCount, 0);
112163
}
113164
}
114165

166+
- (void)testDropsTheQueryCacheWithThousandsOfEntries {
167+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get() upToVersion:2];
168+
{
169+
// Setup some targets to be destroyed.
170+
LevelDbTransaction transaction(_db.get(), "testDropsTheQueryCacheWithThousandsOfEntries setup");
171+
for (int i = 0; i < 10000; ++i) {
172+
transaction.Put([FSTLevelDBTargetKey keyWithTargetID:i], "");
173+
}
174+
transaction.Commit();
175+
}
176+
177+
[FSTLevelDBMigrations runMigrationsWithDatabase:_db.get() upToVersion:3];
178+
{
179+
LevelDbTransaction transaction(_db.get(), "Verify");
180+
std::string prefix = [FSTLevelDBTargetKey keyPrefix];
181+
182+
auto it = transaction.NewIterator();
183+
std::vector<std::string> found_keys;
184+
for (it->Seek(prefix); it->Valid() && absl::StartsWith(it->key(), prefix); it->Next()) {
185+
found_keys.push_back(std::string{it->key()});
186+
}
187+
188+
XCTAssertEqual(found_keys, std::vector<std::string>{});
189+
}
190+
}
191+
192+
/**
193+
* Creates the name of a dummy entry to make sure the iteration is correctly bounded.
194+
*/
195+
- (std::string)dummyKeyForTable:(const char *)tableName {
196+
std::string dummyKey;
197+
// Magic number that indicates a table name follows. Needed to mimic the prefix to the target
198+
// table.
199+
OrderedCode::WriteSignedNumIncreasing(&dummyKey, 5);
200+
OrderedCode::WriteString(&dummyKey, tableName);
201+
return dummyKey;
202+
}
203+
115204
@end
116205

117206
NS_ASSUME_NONNULL_END

‎Firestore/Source/Local/FSTLevelDB.mm

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,9 +150,7 @@ - (BOOL)start:(NSError **)error {
150150
return NO;
151151
}
152152
_ptr.reset(database);
153-
LevelDbTransaction transaction(_ptr.get(), "Start LevelDB");
154-
[FSTLevelDBMigrations runMigrationsWithTransaction:&transaction];
155-
transaction.Commit();
153+
[FSTLevelDBMigrations runMigrationsWithDatabase:_ptr.get()];
156154
return YES;
157155
}
158156

‎Firestore/Source/Local/FSTLevelDBMigrations.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ typedef int32_t FSTLevelDBSchemaVersion;
3636
/**
3737
* Runs any migrations needed to bring the given database up to the current schema version
3838
*/
39-
+ (void)runMigrationsWithTransaction:(firebase::firestore::local::LevelDbTransaction *)transaction;
39+
+ (void)runMigrationsWithDatabase:(leveldb::DB *)database;
40+
41+
/**
42+
* Runs any migrations needed to bring the given database up to the given schema version
43+
*/
44+
+ (void)runMigrationsWithDatabase:(leveldb::DB *)database
45+
upToVersion:(FSTLevelDBSchemaVersion)version;
4046

4147
@end
4248

0 commit comments

Comments
 (0)