Add datastore Clear/Reset method

And don't automatically reset on Init error.
Psiphon-Inc · Sep 9, 2019 · 97004b4 · 97004b4
1 parent aac31b8
commit 97004b4
Show file tree

Hide file tree

Showing 11 changed files with 244 additions and 81 deletions.
diff --git a/datastore.cpp b/datastore.cpp
@@ -30,56 +30,78 @@ using namespace error;
 
 
 Datastore::Datastore()
-        : json_(json::object()) {
+        : initialized_(false), json_(json::object()), paused_(false) {
 }
 
-Error Datastore::Init(const char* file_root, const char* suffix) {
+static string FilePath(const string& file_root, const string& suffix) {
+    return file_root + "/psicashdatastore" + suffix;
+}
+
+Error Datastore::Init(const string& file_root, const string& suffix) {
     SYNCHRONIZE(mutex_);
-    file_path_ = string(file_root) + "/psicashdatastore" + (suffix ? suffix : "");
-    return PassError(FileLoad());
+    file_path_ = FilePath(file_root, suffix);
+    if (auto err = FileLoad(file_path_)) {
+        return PassError(err);
+    }
+    initialized_ = true;
+    return error::nullerr;
 }
 
-void Datastore::Clear() {
+#define MUST_BE_INITIALIZED     if (!initialized_) { return MakeCriticalError("must only be called on an initialized datastore"); }
+
+Error Datastore::Clear(const string& file_path) {
     SYNCHRONIZE(mutex_);
     json_ = json::object();
-    FileStore();
+    return PassError(FileStore(file_path));
+}
+
+Error Datastore::Clear(const string& file_root, const string& suffix) {
+    return PassError(Clear(FilePath(file_root, suffix)));
+}
+
+Error Datastore::Clear() {
+    SYNCHRONIZE(mutex_);
+    MUST_BE_INITIALIZED;
+    return PassError(Clear(file_path_));
 }
 
 void Datastore::PauseWrites() {
     SYNCHRONIZE(mutex_);
     paused_ = true;
 }
 
-error::Error Datastore::UnpauseWrites() {
+Error Datastore::UnpauseWrites() {
     SYNCHRONIZE(mutex_);
+    MUST_BE_INITIALIZED;
     if (!paused_) {
         return nullerr;
     }
     paused_ = false;
-    return FileStore();
+    return FileStore(file_path_);
 }
 
 Error Datastore::Set(const json& in) {
     SYNCHRONIZE(mutex_);
+    MUST_BE_INITIALIZED;
     json_.update(in);
-    return PassError(FileStore());
+    return PassError(FileStore(file_path_));
 }
 
-Error Datastore::FileLoad() {
+Error Datastore::FileLoad(const string& file_path) {
     SYNCHRONIZE(mutex_);
 
     json_ = json::object();
 
     ifstream f;
-    f.open(file_path_, ios::binary);
+    f.open(file_path, ios::binary);
 
     // Figuring out the cause of an open-file problem (i.e., file doesn't exist vs. filesystem is
     // broken) is annoying difficult to do robustly and in a cross-platform manner.
     // It seems like these state achieve approximately what we want.
     // For details see: https://en.cppreference.com/w/cpp/io/ios_base/iostate
     if (f.fail()) {
         // File probably doesn't exist. Check that we can write here.
-        return WrapError(FileStore(), "f.fail and FileStore failed");
+        return WrapError(FileStore(file_path), "f.fail and FileStore failed");
     } else if (!f.good()) {
         return MakeCriticalError(utils::Stringer("not f.good; errno=", errno));
     }
@@ -94,15 +116,15 @@ Error Datastore::FileLoad() {
     return nullerr;
 }
 
-Error Datastore::FileStore() {
+Error Datastore::FileStore(const string& file_path) {
     SYNCHRONIZE(mutex_);
 
     if (paused_) {
         return nullerr;
     }
 
     ofstream f;
-    f.open(file_path_, ios::trunc | ios::binary);
+    f.open(file_path, ios::trunc | ios::binary);
     if (!f.is_open()) {
         return MakeCriticalError(utils::Stringer("not f.is_open; errno=", errno));
     }

diff --git a/datastore.hpp b/datastore.hpp
@@ -37,7 +37,8 @@ class Datastore {
 public:
     enum DatastoreGetError {
         kNotFound = 1,
-        kTypeMismatch
+        kTypeMismatch,
+        kDatastoreUninitialized
     };
 
 public:
@@ -47,11 +48,17 @@ class Datastore {
     /// The fileRoot directory must already exist.
     /// suffix should be used to disambiguate different datastores. Optional (can be null).
     /// Returns false if there's an unrecoverable error (such as an inability to use the filesystem).
-    error::Error Init(const char* file_root, const char* suffix);
+    error::Error Init(const std::string& file_root, const std::string& suffix);
 
     /// Clears the in-memory structure and the persistent file.
-    /// Primarily intended for debugging purposes.
-    void Clear();
+    /// Calling this does not change the initialized state. If the datastore was already
+    /// initialized with a different file_root+suffix, then the result is undefined.
+    error::Error Clear(const std::string& file_root, const std::string& suffix);
+
+    /// Clears the in-memory structure and the persistent file.
+    /// Calling this does not change the initialized state.
+    /// Init() must have already been called, successfully.
+    error::Error Clear();
 
     /// Stops writing of updates to disk until UnpauseWrites is called.
     void PauseWrites();
@@ -66,6 +73,9 @@ class Datastore {
             // "control reached end of non-void function without returning a value".
             T val;
             SYNCHRONIZE_BLOCK(mutex_) {
+                if (!initialized_) {
+                    return nonstd::make_unexpected(kDatastoreUninitialized);
+                }
                 if (json_.find(key) == json_.end()) {
                     return nonstd::make_unexpected(kNotFound);
                 }
@@ -92,11 +102,15 @@ class Datastore {
     error::Error Set(const json& in);
 
 protected:
-    error::Error FileLoad();
-    error::Error FileStore();
+    /// Helper for the public Clear methods
+    error::Error Clear(const std::string& file_path);
+
+    error::Error FileLoad(const std::string& file_path);
+    error::Error FileStore(const std::string& file_path);
 
 private:
     mutable std::recursive_mutex mutex_;
+    bool initialized_;
     std::string file_path_;
     json json_;
     bool paused_;