Backup config files when upgrading (#1959)

* Backup config files if upgrading

* Add test

* Formatting

nano/core_test/node.cpp

@@ -890,6 +890,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;
+	};
+
+	auto get_file_count = [&dir]() {
+		return std::count_if (boost::filesystem::directory_iterator (dir), boost::filesystem::directory_iterator (), static_cast<bool (*) (const boost::filesystem::path &)> (boost::filesystem::is_regular_file));
+	};
+	// clang-format on
+
+	// 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<std::string> ("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<nano::node> node0;

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 <typename T>
 	nano::error & read (boost::filesystem::path const & path_a)
 	{
 		std::fstream stream;
@@ -91,14 +90,20 @@ class jsonconfig : public nano::error_aware<>
 	template <typename T>
 	nano::error & read_and_update (T & object, boost::filesystem::path const & path_a)
 	{
-		read<T> (path_a);
+		auto file_exists (boost::filesystem::exists (path_a));
+		read (path_a);
 		if (!*error)
 		{
 			std::fstream stream;
 			auto updated (false);
 			*error = object.deserialize_json (updated, *this);
 			if (!*error && updated)
 			{
+				// Before updating the config file during an upgrade make a backup first
+				if (file_exists)
+				{
+					create_backup_file (path_a);
+				}
 				stream.open (path_a.string (), std::ios_base::out | std::ios_base::trunc);
 				try
 				{
@@ -146,6 +151,22 @@ class jsonconfig : public nano::error_aware<>
 		stream_a.open (path_a);
 	}
 
+	/** Takes a filepath, appends '_backup_<timestamp>' 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 ()
 	{

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<nano::rpc_config> (rpc_config_path));
+	auto rpc_error (rpc_json.read (rpc_config_path));
 	if (rpc_error || rpc_json.empty ())
 	{
 		// Migrate RPC info across