Skip to content

Commit 2f6ff23

Browse files
wilhuffwilhuff
authored
Use new filesystem routines in FSTLevelDB (#1707)
* Convert between leveldb::Status and util::Status * Use new filesystem routines in FSTLevelDB Also rework -[FSTPersistence start] to return Status * Remove unused leveldb::Status to NSError conversions * Add GoogleTest as a dependency of Firestore_IntegrationTests_iOS
1 parent ea4c8c7 commit 2f6ff23

18 files changed

+228
-183
lines changed

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@
207207
B6FB468E208F9BAB00554BA2 /* executor_libdispatch_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */; };
208208
B6FB468F208F9BAE00554BA2 /* executor_std_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4687208F9B9100554BA2 /* executor_std_test.cc */; };
209209
B6FB4690208F9BB300554BA2 /* executor_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4688208F9B9100554BA2 /* executor_test.cc */; };
210+
BEE0294A23AB993E5DE0E946 /* leveldb_util_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 332485C4DCC6BA0DBB5E31B7 /* leveldb_util_test.cc */; };
210211
C1AA536F90A0A576CA2816EB /* Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = BB92EB03E3F92485023F64ED /* Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework */; };
211212
C482E724F4B10968417C3F78 /* Pods_Firestore_FuzzTests_iOS.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B79CA87A1A01FC5329031C9B /* Pods_Firestore_FuzzTests_iOS.framework */; };
212213
C80B10E79CDD7EF7843C321E /* type_traits_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2A0CF41BA5AED6049B0BEB2C /* type_traits_apple_test.mm */; };
@@ -293,6 +294,7 @@
293294
132E3BB3D5C42282B4ACFB20 /* FSTLevelDBBenchmarkTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FSTLevelDBBenchmarkTests.mm; sourceTree = "<group>"; };
294295
2A0CF41BA5AED6049B0BEB2C /* type_traits_apple_test.mm */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.objcpp; path = type_traits_apple_test.mm; sourceTree = "<group>"; };
295296
2B50B3A0DF77100EEE887891 /* Pods_Firestore_Tests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Tests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
297+
332485C4DCC6BA0DBB5E31B7 /* leveldb_util_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = leveldb_util_test.cc; sourceTree = "<group>"; };
296298
358C3B5FE573B1D60A4F7592 /* strerror_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = strerror_test.cc; sourceTree = "<group>"; };
297299
3B843E4A1F3930A400548890 /* remote_store_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = remote_store_spec_test.json; sourceTree = "<group>"; };
298300
3C81DE3772628FE297055662 /* Pods-Firestore_Example_iOS.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example_iOS.debug.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example_iOS/Pods-Firestore_Example_iOS.debug.xcconfig"; sourceTree = "<group>"; };
@@ -712,6 +714,7 @@
712714
isa = PBXGroup;
713715
children = (
714716
54995F6E205B6E12004EFFA0 /* leveldb_key_test.cc */,
717+
332485C4DCC6BA0DBB5E31B7 /* leveldb_util_test.cc */,
715718
F8043813A5D16963EC02B182 /* local_serializer_test.cc */,
716719
);
717720
path = local;
@@ -1669,10 +1672,14 @@
16691672
);
16701673
inputPaths = (
16711674
"${SRCROOT}/Pods/Target Support Files/Pods-Firestore_IntegrationTests_iOS/Pods-Firestore_IntegrationTests_iOS-frameworks.sh",
1675+
"${BUILT_PRODUCTS_DIR}/leveldb-library/leveldb.framework",
1676+
"${BUILT_PRODUCTS_DIR}/GoogleTest/GoogleTest.framework",
16721677
"${BUILT_PRODUCTS_DIR}/OCMock/OCMock.framework",
16731678
);
16741679
name = "[CP] Embed Pods Frameworks";
16751680
outputPaths = (
1681+
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/leveldb.framework",
1682+
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/GoogleTest.framework",
16761683
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/OCMock.framework",
16771684
);
16781685
runOnlyForDeploymentPostprocessing = 0;
@@ -1884,6 +1891,7 @@
18841891
54A0353520A3D8CB003E0143 /* iterator_adaptors_test.cc in Sources */,
18851892
618BBEAE20B89AAC00B5BCE7 /* latlng.pb.cc in Sources */,
18861893
54995F6F205B6E12004EFFA0 /* leveldb_key_test.cc in Sources */,
1894+
BEE0294A23AB993E5DE0E946 /* leveldb_util_test.cc in Sources */,
18871895
020AFD89BB40E5175838BB76 /* local_serializer_test.cc in Sources */,
18881896
54C2294F1FECABAE007D065B /* log_test.cc in Sources */,
18891897
618BBEA720B89AAC00B5BCE7 /* maybe_document.pb.cc in Sources */,

‎Firestore/Example/Podfile

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ target 'Firestore_Example_iOS' do
2121
target 'Firestore_Tests_iOS' do
2222
inherit! :search_paths
2323

24-
pod 'leveldb-library'
25-
pod 'OCMock'
2624
pod 'GoogleTest', :podspec => 'GoogleTest.podspec'
2725
pod 'ProtobufCpp', :podspec => 'ProtobufCpp.podspec'
26+
27+
pod 'OCMock'
28+
pod 'leveldb-library'
2829
end
2930

3031
target 'Firestore_Benchmarks_iOS' do
@@ -36,7 +37,10 @@ target 'Firestore_Example_iOS' do
3637
target 'Firestore_IntegrationTests_iOS' do
3738
inherit! :search_paths
3839

40+
pod 'GoogleTest', :podspec => 'GoogleTest.podspec'
41+
3942
pod 'OCMock'
43+
pod 'leveldb-library'
4044
end
4145

4246
target 'Firestore_SwiftTests_iOS' do

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
using firebase::firestore::model::TargetId;
5050
using firebase::firestore::testutil::Key;
5151
using firebase::firestore::util::OrderedCode;
52+
using firebase::firestore::util::Path;
5253
using leveldb::DB;
5354
using leveldb::Options;
5455
using leveldb::Status;
@@ -65,9 +66,9 @@ - (void)setUp {
6566
options.error_if_exists = true;
6667
options.create_if_missing = true;
6768

68-
NSString *dir = [FSTPersistenceTestHelpers levelDBDir];
69+
Path dir = [FSTPersistenceTestHelpers levelDBDir];
6970
DB *db;
70-
Status status = DB::Open(options, [dir UTF8String], &db);
71+
Status status = DB::Open(options, dir.ToUtf8String(), &db);
7172
XCTAssert(status.ok(), @"Failed to create db: %s", status.ToString().c_str());
7273
_db.reset(db);
7374
}

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

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

4041
NS_ASSUME_NONNULL_BEGIN
4142

@@ -66,7 +67,7 @@ - (void)testMetadataPersistedAcrossRestarts {
6667
[self.persistence shutdown];
6768
self.persistence = nil;
6869

69-
NSString *dir = [FSTPersistenceTestHelpers levelDBDir];
70+
Path dir = [FSTPersistenceTestHelpers levelDBDir];
7071

7172
FSTLevelDB *db1 = [FSTPersistenceTestHelpers levelDBPersistenceWithDir:dir];
7273
FSTLevelDBQueryCache *queryCache = [db1 queryCache];

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,14 @@
3434

3535
NS_ASSUME_NONNULL_BEGIN
3636

37+
using firebase::firestore::local::LevelDbMutationKey;
38+
using firebase::firestore::local::LevelDbTransaction;
39+
using firebase::firestore::util::Path;
3740
using leveldb::DB;
3841
using leveldb::Options;
3942
using leveldb::ReadOptions;
40-
using leveldb::WriteOptions;
4143
using leveldb::Status;
42-
using firebase::firestore::local::LevelDbMutationKey;
43-
using firebase::firestore::local::LevelDbTransaction;
44+
using leveldb::WriteOptions;
4445

4546
@interface FSTLevelDBTransactionTests : XCTestCase
4647
@end
@@ -54,9 +55,9 @@ - (void)setUp {
5455
options.error_if_exists = true;
5556
options.create_if_missing = true;
5657

57-
NSString *dir = [FSTPersistenceTestHelpers levelDBDir];
58+
Path dir = [FSTPersistenceTestHelpers levelDBDir];
5859
DB *db;
59-
Status status = DB::Open(options, [dir UTF8String], &db);
60+
Status status = DB::Open(options, dir.ToUtf8String(), &db);
6061
XCTAssert(status.ok(), @"Failed to create db: %s", status.ToString().c_str());
6162
_db.reset(db);
6263
}

‎Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
#import <Foundation/Foundation.h>
1818

19+
#include "Firestore/core/src/firebase/firestore/util/path.h"
20+
1921
@class FSTLevelDB;
2022
@class FSTMemoryPersistence;
2123

@@ -27,7 +29,7 @@ NS_ASSUME_NONNULL_BEGIN
2729
* @return The directory where a leveldb instance can store data files. Any files that existed
2830
* there will be deleted first.
2931
*/
30-
+ (NSString *)levelDBDir;
32+
+ (firebase::firestore::util::Path)levelDBDir;
3133

3234
/**
3335
* Creates and starts a new FSTLevelDB instance for testing, destroying any previous contents
@@ -44,7 +46,7 @@ NS_ASSUME_NONNULL_BEGIN
4446
* present in the given directory. As a consequence, the resulting databse is not guaranteed
4547
* to be empty.
4648
*/
47-
+ (FSTLevelDB *)levelDBPersistenceWithDir:(NSString *)dir;
49+
+ (FSTLevelDB *)levelDBPersistenceWithDir:(firebase::firestore::util::Path)dir;
4850

4951
/** Creates and starts a new FSTMemoryPersistence instance for testing. */
5052
+ (FSTMemoryPersistence *)eagerGCMemoryPersistence;

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
#import "Firestore/Example/Tests/Local/FSTPersistenceTestHelpers.h"
1818

19+
#include <utility>
20+
1921
#import "Firestore/Source/Local/FSTLevelDB.h"
2022
#import "Firestore/Source/Local/FSTLocalSerializer.h"
2123
#import "Firestore/Source/Local/FSTMemoryPersistence.h"
@@ -30,12 +32,13 @@
3032
namespace util = firebase::firestore::util;
3133
using firebase::firestore::model::DatabaseId;
3234
using firebase::firestore::util::Path;
35+
using firebase::firestore::util::Status;
3336

3437
NS_ASSUME_NONNULL_BEGIN
3538

3639
@implementation FSTPersistenceTestHelpers
3740

38-
+ (NSString *)levelDBDir {
41+
+ (Path)levelDBDir {
3942
Path dir = util::TempDir().AppendUtf8("FSTPersistenceTestHelpers");
4043

4144
// Delete the directory first to ensure isolation between runs.
@@ -46,22 +49,21 @@ + (NSString *)levelDBDir {
4649
format:@"Failed to clean up leveldb path %s: %s", dir.c_str(), status.ToString().c_str()];
4750
}
4851

49-
return dir.ToNSString();
52+
return dir;
5053
}
5154

52-
+ (FSTLevelDB *)levelDBPersistenceWithDir:(NSString *)dir {
55+
+ (FSTLevelDB *)levelDBPersistenceWithDir:(Path)dir {
5356
// This owns the DatabaseIds since we do not have FirestoreClient instance to own them.
5457
static DatabaseId database_id{"p", "d"};
5558

5659
FSTSerializerBeta *remoteSerializer = [[FSTSerializerBeta alloc] initWithDatabaseID:&database_id];
5760
FSTLocalSerializer *serializer =
5861
[[FSTLocalSerializer alloc] initWithRemoteSerializer:remoteSerializer];
59-
FSTLevelDB *db = [[FSTLevelDB alloc] initWithDirectory:dir serializer:serializer];
60-
NSError *error;
61-
BOOL success = [db start:&error];
62-
if (!success) {
62+
FSTLevelDB *db = [[FSTLevelDB alloc] initWithDirectory:std::move(dir) serializer:serializer];
63+
Status status = [db start];
64+
if (!status.ok()) {
6365
[NSException raise:NSInternalInconsistencyException
64-
format:@"Failed to create leveldb path %@: %@", dir, error];
66+
format:@"Failed to start leveldb persistence: %s", status.ToString().c_str()];
6567
}
6668

6769
return db;
@@ -72,24 +74,22 @@ + (FSTLevelDB *)levelDBPersistence {
7274
}
7375

7476
+ (FSTMemoryPersistence *)eagerGCMemoryPersistence {
75-
NSError *error;
7677
FSTMemoryPersistence *persistence = [FSTMemoryPersistence persistenceWithEagerGC];
77-
BOOL success = [persistence start:&error];
78-
if (!success) {
78+
Status status = [persistence start];
79+
if (!status.ok()) {
7980
[NSException raise:NSInternalInconsistencyException
80-
format:@"Failed to start memory persistence: %@", error];
81+
format:@"Failed to start memory persistence: %s", status.ToString().c_str()];
8182
}
8283

8384
return persistence;
8485
}
8586

8687
+ (FSTMemoryPersistence *)lruMemoryPersistence {
87-
NSError *error;
8888
FSTMemoryPersistence *persistence = [FSTMemoryPersistence persistenceWithLRUGC];
89-
BOOL success = [persistence start:&error];
90-
if (!success) {
89+
Status status = [persistence start];
90+
if (!status.ok()) {
9191
[NSException raise:NSInternalInconsistencyException
92-
format:@"Failed to start memory persistence: %@", error];
92+
format:@"Failed to start memory persistence: %s", status.ToString().c_str()];
9393
}
9494

9595
return persistence;

‎Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,10 @@
3434
#include "Firestore/core/src/firebase/firestore/auth/empty_credentials_provider.h"
3535
#include "Firestore/core/src/firebase/firestore/model/database_id.h"
3636
#include "Firestore/core/src/firebase/firestore/util/autoid.h"
37+
#include "Firestore/core/src/firebase/firestore/util/filesystem.h"
38+
#include "Firestore/core/src/firebase/firestore/util/path.h"
3739
#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
40+
#include "Firestore/core/test/firebase/firestore/util/status_test_util.h"
3841
#include "absl/memory/memory.h"
3942

4043
#import "Firestore/Source/API/FIRFirestore+Internal.h"
@@ -49,6 +52,8 @@
4952
using firebase::firestore::auth::EmptyCredentialsProvider;
5053
using firebase::firestore::model::DatabaseId;
5154
using firebase::firestore::util::CreateAutoId;
55+
using firebase::firestore::util::Path;
56+
using firebase::firestore::util::Status;
5257

5358
NS_ASSUME_NONNULL_BEGIN
5459

@@ -86,14 +91,9 @@ - (void)tearDown {
8691
}
8792

8893
- (void)clearPersistence {
89-
NSString *levelDBDir = [FSTLevelDB documentsDirectory];
90-
NSError *error;
91-
if (![[NSFileManager defaultManager] removeItemAtPath:levelDBDir error:&error]) {
92-
// file not found is okay.
93-
XCTAssertTrue(
94-
[error.domain isEqualToString:NSCocoaErrorDomain] && error.code == NSFileNoSuchFileError,
95-
@"Failed to clear LevelDB Persistence: %@", error);
96-
}
94+
Path levelDBDir = [FSTLevelDB documentsDirectory];
95+
Status status = util::RecursivelyDelete(levelDBDir);
96+
ASSERT_OK(status);
9797
}
9898

9999
- (FIRFirestore *)firestore {

‎Firestore/Source/Core/FSTFirestoreClient.mm

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@
5757
using firebase::firestore::model::DatabaseId;
5858
using firebase::firestore::model::DocumentKeySet;
5959
using firebase::firestore::model::OnlineState;
60+
using firebase::firestore::util::Path;
61+
using firebase::firestore::util::Status;
6062
using firebase::firestore::util::internal::Executor;
6163

6264
NS_ASSUME_NONNULL_BEGIN
@@ -165,26 +167,27 @@ - (void)initializeWithUser:(const User &)user usePersistence:(BOOL)usePersistenc
165167
// external write/listen operations could get queued to run before that subsequent work
166168
// completes.
167169
if (usePersistence) {
168-
NSString *dir = [FSTLevelDB storageDirectoryForDatabaseInfo:*self.databaseInfo
169-
documentsDirectory:[FSTLevelDB documentsDirectory]];
170+
Path dir = [FSTLevelDB storageDirectoryForDatabaseInfo:*self.databaseInfo
171+
documentsDirectory:[FSTLevelDB documentsDirectory]];
170172

171173
FSTSerializerBeta *remoteSerializer =
172174
[[FSTSerializerBeta alloc] initWithDatabaseID:&self.databaseInfo->database_id()];
173175
FSTLocalSerializer *serializer =
174176
[[FSTLocalSerializer alloc] initWithRemoteSerializer:remoteSerializer];
175177

176-
_persistence = [[FSTLevelDB alloc] initWithDirectory:dir serializer:serializer];
178+
_persistence = [[FSTLevelDB alloc] initWithDirectory:std::move(dir) serializer:serializer];
177179
} else {
178180
_persistence = [FSTMemoryPersistence persistenceWithEagerGC];
179181
}
180182

181-
NSError *error;
182-
if (![_persistence start:&error]) {
183+
Status status = [_persistence start];
184+
if (!status.ok()) {
183185
// If local storage fails to start then just throw up our hands: the error is unrecoverable.
184186
// There's nothing an end-user can do and nearly all failures indicate the developer is doing
185187
// something grossly wrong so we should stop them cold in their tracks with a failure they
186188
// can't ignore.
187-
[NSException raise:NSInternalInconsistencyException format:@"Failed to open DB: %@", error];
189+
[NSException raise:NSInternalInconsistencyException
190+
format:@"Failed to open DB: %s", status.ToString().c_str()];
188191
}
189192

190193
_localStore = [[FSTLocalStore alloc] initWithPersistence:_persistence initialUser:user];

0 commit comments

Comments
 (0)