Skip to content

Commit e1c3ebf

Browse files
committed
Make a Filesystem class that can be subclassed for testing.
This change does not yet change all the classes that use the filesystem to take Filesystem as a dependency. Those changes will come in follow-up PRs. This change is motivated by feedback in #4609. There I had created a complicated interface to LevelDbOpener just so that different migration scenarios could be tested, when in reality, I just needed to be able to plumb through different values for the result of AppDataDir and related functions.
1 parent ff56753 commit e1c3ebf

File tree

12 files changed

+347
-363
lines changed

12 files changed

+347
-363
lines changed

Firestore/Example/Tests/Util/FSTIntegrationTestCase.mm

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
using firebase::firestore::remote::GrpcConnection;
6868
using firebase::firestore::util::AsyncQueue;
6969
using firebase::firestore::util::CreateAutoId;
70+
using firebase::firestore::util::Filesystem;
7071
using firebase::firestore::util::Path;
7172
using firebase::firestore::util::Status;
7273
using firebase::firestore::util::StatusOr;
@@ -142,6 +143,7 @@ - (void)tearDown {
142143
* with each other.
143144
*/
144145
- (void)clearPersistenceOnce {
146+
auto fs = Filesystem::Default();
145147
static bool clearedPersistence = false;
146148

147149
@synchronized([FSTIntegrationTestCase class]) {
@@ -150,7 +152,7 @@ - (void)clearPersistenceOnce {
150152
ASSERT_OK(maybe_dir);
151153

152154
Path levelDBDir = maybe_dir.ValueOrDie();
153-
Status status = util::RecursivelyDelete(levelDBDir);
155+
Status status = fs->RecursivelyRemove(levelDBDir);
154156
ASSERT_OK(status);
155157

156158
clearedPersistence = true;

Firestore/core/src/firebase/firestore/local/leveldb_persistence.cc

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ namespace {
4343
using auth::User;
4444
using leveldb::DB;
4545
using model::ListenSequenceNumber;
46+
using util::Filesystem;
4647
using util::Path;
4748
using util::Status;
4849
using util::StatusOr;
@@ -75,10 +76,11 @@ std::set<std::string> CollectUserSet(LevelDbTransaction* transaction) {
7576

7677
util::StatusOr<std::unique_ptr<LevelDbPersistence>> LevelDbPersistence::Create(
7778
util::Path dir, LocalSerializer serializer, const LruParams& lru_params) {
79+
auto fs = Filesystem::Default();
7880
Status status = EnsureDirectory(dir);
7981
if (!status.ok()) return status;
8082

81-
status = util::ExcludeFromBackups(dir);
83+
status = fs->ExcludeFromBackups(dir);
8284
if (!status.ok()) return status;
8385

8486
StatusOr<std::unique_ptr<DB>> created = OpenDb(dir);
@@ -123,7 +125,8 @@ LevelDbPersistence::LevelDbPersistence(std::unique_ptr<leveldb::DB> db,
123125
// MARK: - Storage location
124126

125127
StatusOr<Path> LevelDbPersistence::AppDataDirectory() {
126-
return util::AppDataDir(kReservedPathComponent);
128+
auto fs = Filesystem::Default();
129+
return fs->AppDataDir(kReservedPathComponent);
127130
}
128131

129132
util::Path LevelDbPersistence::StorageDirectory(
@@ -149,7 +152,8 @@ util::Path LevelDbPersistence::StorageDirectory(
149152
// MARK: - Startup
150153

151154
Status LevelDbPersistence::EnsureDirectory(const Path& dir) {
152-
Status status = util::RecursivelyCreateDir(dir);
155+
auto fs = Filesystem::Default();
156+
Status status = fs->RecursivelyCreateDir(dir);
153157
if (!status.ok()) {
154158
return Status{Error::Internal, "Failed to create persistence directory"}
155159
.CausedBy(status);
@@ -191,14 +195,17 @@ util::Status LevelDbPersistence::ClearPersistence(
191195
Path leveldb_dir =
192196
StorageDirectory(database_info, maybe_data_dir.ValueOrDie());
193197
LOG_DEBUG("Clearing persistence for path: %s", leveldb_dir.ToUtf8String());
194-
return util::RecursivelyDelete(leveldb_dir);
198+
auto fs = Filesystem::Default();
199+
return fs->RecursivelyRemove(leveldb_dir);
195200
}
196201

197202
int64_t LevelDbPersistence::CalculateByteSize() {
203+
auto fs = Filesystem::Default();
204+
198205
int64_t count = 0;
199206
auto iter = util::DirectoryIterator::Create(directory_);
200207
for (; iter->Valid(); iter->Next()) {
201-
int64_t file_size = util::FileSize(iter->file()).ValueOrDie();
208+
int64_t file_size = fs->FileSize(iter->file()).ValueOrDie();
202209
count += file_size;
203210
}
204211

Firestore/core/src/firebase/firestore/remote/grpc_connection.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ namespace remote {
4141
using auth::Token;
4242
using core::DatabaseInfo;
4343
using model::DatabaseId;
44+
using util::Filesystem;
4445
using util::Path;
4546
using util::Status;
4647
using util::StatusOr;
@@ -207,9 +208,10 @@ std::shared_ptr<grpc::Channel> GrpcConnection::CreateChannel() const {
207208
}
208209

209210
// For tests only
211+
auto fs = Filesystem::Default();
210212
args.SetSslTargetNameOverride(host_config->target_name);
211213
Path path = host_config->certificate_path;
212-
StatusOr<std::string> test_certificate = ReadFile(path);
214+
StatusOr<std::string> test_certificate = fs->ReadFile(path);
213215
HARD_ASSERT(test_certificate.ok(),
214216
StringFormat("Unable to open root certificates at file path %s",
215217
path.ToUtf8String())

Firestore/core/src/firebase/firestore/util/CMakeLists.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ cc_library(
120120
filesystem.h
121121
filesystem_apple.mm
122122
filesystem_common.cc
123-
filesystem_detail.h
124123
filesystem_posix.cc
125124
path.cc
126125
path.h
@@ -135,7 +134,6 @@ cc_library(
135134
SOURCES
136135
filesystem.h
137136
filesystem_common.cc
138-
filesystem_detail.h
139137
filesystem_posix.cc
140138
path.cc
141139
path.h
@@ -150,7 +148,6 @@ cc_library(
150148
SOURCES
151149
filesystem.h
152150
filesystem_common.cc
153-
filesystem_detail.h
154151
filesystem_win.cc
155152
path.cc
156153
path.h

Firestore/core/src/firebase/firestore/util/filesystem.h

Lines changed: 123 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -28,88 +28,138 @@ namespace firebase {
2828
namespace firestore {
2929
namespace util {
3030

31-
// High-level routines for the manipulating the filesystem. All filesystems
32-
// are required to implement these routines.
33-
3431
/**
35-
* Answers the question "is this path a directory? The path is not required to
36-
* have a trailing slash.
37-
*
38-
* Typical return codes include:
39-
* * Ok - The path exists and is a directory.
40-
* * FailedPrecondition - Some component of the path is not a directory. This
41-
* does not necessarily imply that the path exists and is a file.
42-
* * NotFound - The path does not exist
43-
* * PermissionDenied - Insufficient permissions to access the path.
32+
* A high-level interface describing
4433
*/
45-
Status IsDirectory(const Path& path);
34+
class Filesystem {
35+
public:
36+
Filesystem(const Filesystem&) = delete;
4637

47-
/**
48-
* Recursively creates all the directories in the path name if they don't
49-
* exist.
50-
*
51-
* @return Ok if the directory was created or already existed.
52-
*/
53-
Status RecursivelyCreateDir(const Path& path);
38+
/**
39+
* Returns a singleton default filesystem implementation for the current
40+
* operating system.
41+
*/
42+
static Filesystem* Default();
5443

55-
/**
56-
* Recursively deletes the contents of the given pathname. If the pathname is
57-
* a file, deletes just that file. The the pathname is a directory, deletes
58-
* everything within the directory.
59-
*
60-
* @return Ok if the directory was deleted or did not exist.
61-
*/
62-
Status RecursivelyDelete(const Path& path);
44+
/**
45+
* Returns a system-defined best directory in which to create application
46+
* data. Values vary wildly across platforms. They include:
47+
*
48+
* * iOS: $container/Documents/$app_name
49+
* * Linux: $HOME/.local/share/$app_name
50+
* * macOS: $HOME/.$app_name
51+
* * Other UNIX: $HOME/.$app_name
52+
* * tvOS: $HOME/Library/Caches/$app_name
53+
* * Windows: %USERPROFILE%/AppData/Local
54+
*
55+
* Note: the returned path is just where the system thinks the application
56+
* data should be stored, but AppDataDir does not actually guarantee that this
57+
* path exists.
58+
*
59+
* @param app_name The name of the application.
60+
*/
61+
virtual StatusOr<Path> AppDataDir(absl::string_view app_name);
6362

64-
/**
65-
* Marks the given directory as excluded from platform-specific backup schemes
66-
* like iCloud backup.
67-
*/
68-
Status ExcludeFromBackups(const Path& dir);
63+
/**
64+
* Returns system-defined best directory in which to create temporary files.
65+
* Typical return values are like `/tmp` on UNIX systems. Clients should
66+
* create randomly named directories or files within this location to avoid
67+
* collisions. Absent any changes that might affect the underlying calls, the
68+
* value returned from TempDir will be stable over time.
69+
*
70+
* Note: the returned path is just where the system thinks temporary files
71+
* should be stored, but TempDir does not actually guarantee that this path
72+
* exists.
73+
*/
74+
virtual Path TempDir();
6975

70-
/**
71-
* Returns a system-defined best directory in which to create application data.
72-
* Values vary wildly across platforms. They include:
73-
*
74-
* * iOS: $container/Documents/$app_name
75-
* * Linux: $HOME/.local/share/$app_name
76-
* * macOS: $HOME/.$app_name
77-
* * Other UNIX: $HOME/.$app_name
78-
* * tvOS: $HOME/Library/Caches/$app_name
79-
* * Windows: %USERPROFILE%/AppData/Local
80-
*
81-
* Note: the returned path is just where the system thinks the application data
82-
* should be stored, but AppDataDir does not actually guarantee that this path
83-
* exists.
84-
*
85-
* @param app_name The name of the application.
86-
*/
87-
StatusOr<Path> AppDataDir(absl::string_view app_name);
76+
/**
77+
* Answers the question "is this path a directory? The path is not required to
78+
* have a trailing slash.
79+
*
80+
* Typical return codes include:
81+
* * Ok - The path exists and is a directory.
82+
* * FailedPrecondition - Some component of the path is not a directory.
83+
* This does not necessarily imply that the path exists and is a file.
84+
* * NotFound - The path does not exist
85+
* * PermissionDenied - Insufficient permissions to access the path.
86+
*/
87+
virtual Status IsDirectory(const Path& path);
8888

89-
/**
90-
* Returns system-defined best directory in which to create temporary files.
91-
* Typical return values are like `/tmp` on UNIX systems. Clients should create
92-
* randomly named directories or files within this location to avoid collisions.
93-
* Absent any changes that might affect the underlying calls, the value returned
94-
* from TempDir will be stable over time.
95-
*
96-
* Note: the returned path is just where the system thinks temporary files
97-
* should be stored, but TempDir does not actually guarantee that this path
98-
* exists.
99-
*/
100-
Path TempDir();
89+
/**
90+
* On success, returns the size in bytes of the file specified by
91+
* `path`.
92+
*/
93+
virtual StatusOr<int64_t> FileSize(const Path& path);
10194

102-
/**
103-
* On success, returns the size in bytes of the file specified by
104-
* `path`.
105-
*/
106-
StatusOr<int64_t> FileSize(const Path& path);
95+
/**
96+
* Recursively creates all the directories in the path name if they don't
97+
* exist.
98+
*
99+
* @return Ok if the directory was created or already existed.
100+
*/
101+
virtual Status RecursivelyCreateDir(const Path& path);
107102

108-
/**
109-
* On success, opens the file at the given `path` and returns its contents as
110-
* a string.
111-
*/
112-
StatusOr<std::string> ReadFile(const Path& path);
103+
/**
104+
* Recursively deletes the contents of the given pathname. If the pathname is
105+
* a file, deletes just that file. The the pathname is a directory, deletes
106+
* everything within the directory.
107+
*
108+
* @return Ok if the directory was deleted or did not exist.
109+
*/
110+
virtual Status RecursivelyRemove(const Path& path);
111+
112+
/**
113+
* Creates the given directory. The immediate parent directory must already
114+
* exist and not already be a file.
115+
*
116+
* @return Ok if the directory was created or already existed. On some systems
117+
* this may also return Ok if a regular file exists at the given path.
118+
*/
119+
virtual Status CreateDir(const Path& path);
120+
121+
/**
122+
* Deletes the given directory if it exists.
123+
*
124+
* @return Ok if the directory was deleted or did not exist. Returns a
125+
* system-defined error if the path is not a directory or the directory is
126+
* non-empty.
127+
*/
128+
virtual Status RemoveDir(const Path& path);
129+
130+
/**
131+
* Deletes the given file if it exists.
132+
*
133+
* @return Ok if the file was deleted or did not exist. Returns a
134+
* system-defined error if the path exists but is not a regular file.
135+
*/
136+
virtual Status RemoveFile(const Path& path);
137+
138+
/**
139+
* Recursively deletes the contents of the given pathname that is known to be
140+
* a directory.
141+
*
142+
* @return Ok if the directory was deleted or did not exist. Returns a
143+
* system-defined error if the path exists but is not a directory.
144+
*
145+
*/
146+
virtual Status RecursivelyRemoveDir(const Path& path);
147+
148+
/**
149+
* Marks the given directory as excluded from platform-specific backup schemes
150+
* like iCloud backup.
151+
*/
152+
virtual Status ExcludeFromBackups(const Path& dir);
153+
154+
/**
155+
* On success, opens the file at the given `path` and returns its contents as
156+
* a string.
157+
*/
158+
virtual StatusOr<std::string> ReadFile(const Path& path);
159+
160+
protected:
161+
Filesystem() = default;
162+
};
113163

114164
/**
115165
* Implements an iterator over the contents of a directory. Initializes to the

Firestore/core/src/firebase/firestore/util/filesystem_apple.mm

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,7 @@
2727
namespace firestore {
2828
namespace util {
2929

30-
Status ExcludeFromBackups(const Path& dir) {
31-
NSURL* dir_url = [NSURL fileURLWithPath:dir.ToNSString()];
32-
NSError* error = nil;
33-
if (![dir_url setResourceValue:@YES
34-
forKey:NSURLIsExcludedFromBackupKey
35-
error:&error]) {
36-
return Status{
37-
Error::Internal,
38-
"Failed to mark persistence directory as excluded from backups"}
39-
.CausedBy(Status::FromNSError(error));
40-
}
41-
42-
return Status::OK();
43-
}
44-
45-
StatusOr<Path> AppDataDir(absl::string_view app_name) {
30+
StatusOr<Path> Filesystem::AppDataDir(absl::string_view app_name) {
4631
#if TARGET_OS_IOS
4732
NSArray<NSString*>* directories = NSSearchPathForDirectoriesInDomains(
4833
NSDocumentDirectory, NSUserDomainMask, YES);
@@ -62,7 +47,7 @@ Status ExcludeFromBackups(const Path& dir) {
6247
#endif
6348
}
6449

65-
Path TempDir() {
50+
Path Filesystem::TempDir() {
6651
const char* env_tmpdir = getenv("TMPDIR");
6752
if (env_tmpdir) {
6853
return Path::FromUtf8(env_tmpdir);
@@ -76,6 +61,21 @@ Path TempDir() {
7661
return Path::FromUtf8("/tmp");
7762
}
7863

64+
Status Filesystem::ExcludeFromBackups(const Path& dir) {
65+
NSURL* dir_url = [NSURL fileURLWithPath:dir.ToNSString()];
66+
NSError* error = nil;
67+
if (![dir_url setResourceValue:@YES
68+
forKey:NSURLIsExcludedFromBackupKey
69+
error:&error]) {
70+
return Status{
71+
Error::Internal,
72+
"Failed to mark persistence directory as excluded from backups"}
73+
.CausedBy(Status::FromNSError(error));
74+
}
75+
76+
return Status::OK();
77+
}
78+
7979
} // namespace util
8080
} // namespace firestore
8181
} // namespace firebase

0 commit comments

Comments
 (0)