From b12dce77734befaec0063deb92944c8a07589589 Mon Sep 17 00:00:00 2001 From: wezrule Date: Mon, 6 May 2019 13:43:27 +0100 Subject: [PATCH 1/3] Backup config files if upgrading --- nano/lib/jsonconfig.hpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/nano/lib/jsonconfig.hpp b/nano/lib/jsonconfig.hpp index e5ed4fdcdd..ff695b9cc9 100644 --- a/nano/lib/jsonconfig.hpp +++ b/nano/lib/jsonconfig.hpp @@ -99,6 +99,8 @@ class jsonconfig : public nano::error_aware<> *error = object.deserialize_json (updated, *this); if (!*error && updated) { + // Before updating the config file during an upgrade make a backup first + create_backup_file (path_a); stream.open (path_a.string (), std::ios_base::out | std::ios_base::trunc); try { @@ -146,6 +148,22 @@ class jsonconfig : public nano::error_aware<> stream_a.open (path_a); } + /** Takes a filepath, appends '_backup_' to the end (but before any extension) and saves that file in the same directory */ + void create_backup_file (boost::filesystem::path const & filepath_a) + { + auto extension = filepath_a.extension (); + auto filename_without_extension = filepath_a.filename ().replace_extension (""); + auto orig_filepath = filepath_a; + auto & backup_path = orig_filepath.remove_filename (); + auto backup_filename = filename_without_extension; + backup_filename += "_backup_"; + backup_filename += std::to_string (std::chrono::system_clock::now ().time_since_epoch ().count ()); + backup_filename += extension; + auto backup_filepath = backup_path / backup_filename; + + boost::filesystem::copy_file (filepath_a, backup_filepath); + } + /** Returns the boost property node managed by this instance */ boost::property_tree::ptree const & get_tree () { From d71e51f0fde525533428cf5f155aa416aa05a028 Mon Sep 17 00:00:00 2001 From: wezrule Date: Tue, 7 May 2019 09:36:27 +0100 Subject: [PATCH 2/3] Add test --- nano/core_test/node.cpp | 51 +++++++++++++++++++++++++++++++++++ nano/lib/jsonconfig.hpp | 9 ++++--- nano/node/node_rpc_config.cpp | 2 +- 3 files changed, 58 insertions(+), 4 deletions(-) diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index 28cc9df512..a044093df9 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -863,6 +863,57 @@ TEST (json, upgrade_from_existing) ASSERT_EQ ("changed", object1.text); } +/** Test that backups are made only when there is an upgrade */ +TEST (json, backup) +{ + auto dir (nano::unique_path ()); + namespace fs = boost::filesystem; + fs::create_directory (dir); + auto path = dir / dir.leaf (); + + // Create json file + nano::jsonconfig json; + json_upgrade_test object1; + ASSERT_FALSE (json.read_and_update (object1, path)); + ASSERT_EQ ("created", object1.text); + + /** Returns 'dir' if backup file cannot be found */ + // clang-format off + auto get_backup_path = [&dir]() { + for (fs::directory_iterator itr (dir); itr != fs::directory_iterator (); ++itr) + { + if (itr->path ().filename ().string ().find ("_backup_") != std::string::npos) + { + return itr->path (); + } + } + return dir; + }; + // clang-format on + + auto get_file_count = [&dir] () { + return std::count_if (boost::filesystem::directory_iterator (dir), boost::filesystem::directory_iterator (), static_cast (boost::filesystem::is_regular_file)); + }; + + // There should only be the original file in this directory + ASSERT_EQ (get_file_count (), 1); + ASSERT_EQ (get_backup_path (), dir); + + // Upgrade, check that there is a backup which matches the first object + ASSERT_FALSE (json.read_and_update (object1, path)); + ASSERT_EQ (get_file_count (), 2); + ASSERT_NE (get_backup_path (), path); + + // Check there is a backup which has the same contents as the original file + nano::jsonconfig json1; + ASSERT_FALSE (json1.read (get_backup_path ())); + ASSERT_EQ (json1.get ("thing"), "created"); + + // Try and upgrade an already upgraded file, should not create any backups + ASSERT_FALSE (json.read_and_update (object1, path)); + ASSERT_EQ (get_file_count (), 2); +} + TEST (node, fork_publish) { std::weak_ptr node0; diff --git a/nano/lib/jsonconfig.hpp b/nano/lib/jsonconfig.hpp index ff695b9cc9..93a82be7f2 100644 --- a/nano/lib/jsonconfig.hpp +++ b/nano/lib/jsonconfig.hpp @@ -60,7 +60,6 @@ class jsonconfig : public nano::error_aware<> * Reads a json object from the stream * @return nano::error&, including a descriptive error message if the config file is malformed. */ - template nano::error & read (boost::filesystem::path const & path_a) { std::fstream stream; @@ -91,7 +90,8 @@ class jsonconfig : public nano::error_aware<> template nano::error & read_and_update (T & object, boost::filesystem::path const & path_a) { - read (path_a); + auto file_exists (boost::filesystem::exists (path_a)); + read (path_a); if (!*error) { std::fstream stream; @@ -100,7 +100,10 @@ class jsonconfig : public nano::error_aware<> if (!*error && updated) { // Before updating the config file during an upgrade make a backup first - create_backup_file (path_a); + if (file_exists) + { + create_backup_file (path_a); + } stream.open (path_a.string (), std::ios_base::out | std::ios_base::trunc); try { diff --git a/nano/node/node_rpc_config.cpp b/nano/node/node_rpc_config.cpp index 82dff55b67..3dca26bf65 100644 --- a/nano/node/node_rpc_config.cpp +++ b/nano/node/node_rpc_config.cpp @@ -69,7 +69,7 @@ void nano::node_rpc_config::migrate (nano::jsonconfig & json, boost::filesystem: { nano::jsonconfig rpc_json; auto rpc_config_path = nano::get_rpc_config_path (data_path); - auto rpc_error (rpc_json.read (rpc_config_path)); + auto rpc_error (rpc_json.read (rpc_config_path)); if (rpc_error || rpc_json.empty ()) { // Migrate RPC info across From 43c124e404ff574a67868b20a7fe6a7147ef0c20 Mon Sep 17 00:00:00 2001 From: wezrule Date: Tue, 7 May 2019 09:45:41 +0100 Subject: [PATCH 3/3] Formatting --- nano/core_test/node.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index a044093df9..fa2cad1591 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -889,11 +889,11 @@ TEST (json, backup) } return dir; }; - // clang-format on - auto get_file_count = [&dir] () { + auto get_file_count = [&dir]() { return std::count_if (boost::filesystem::directory_iterator (dir), boost::filesystem::directory_iterator (), static_cast (boost::filesystem::is_regular_file)); }; + // clang-format on // There should only be the original file in this directory ASSERT_EQ (get_file_count (), 1);