From b502b5723f4516bfd33c17a6b71a72831933d212 Mon Sep 17 00:00:00 2001 From: Carl Hetherington Date: Tue, 21 Dec 2021 02:35:55 +0100 Subject: [PATCH] Tidy up backing up of config files, improve the tests a little and fix it for the case when the user has specified their own config file path. --- src/lib/config.cc | 36 +++++++++++++++++---- test/config_test.cc | 77 ++++++++++++++++++++++++++++++++++++++------- 2 files changed, 95 insertions(+), 18 deletions(-) diff --git a/src/lib/config.cc b/src/lib/config.cc index 419436995..5496c0b3c 100644 --- a/src/lib/config.cc +++ b/src/lib/config.cc @@ -125,6 +125,9 @@ Config::set_defaults () #ifdef DCPOMATIC_WINDOWS _win32_console = false; #endif + /* At the moment we don't write these files anywhere new after a version change, so they will be read from + * ~/.config/dcpomatic2 (or equivalent) and written back there. + */ _cinemas_file = read_path ("cinemas.xml"); _dkdm_recipients_file = read_path ("dkdm_recipients.xml"); _show_hints_before_make_dcp = true; @@ -212,16 +215,37 @@ Config::create_certificate_chain () void Config::backup () { - /* Make a copy of the configuration */ - try { + using namespace boost::filesystem; + + auto copy_adding_number = [](path const& path_to_copy) { + + auto add_number = [](path const& path, int number) { + return String::compose("%1.%2", path, number); + }; + int n = 1; - while (n < 100 && boost::filesystem::exists(write_path(String::compose("config.xml.%1", n)))) { + while (n < 100 && exists(add_number(path_to_copy, n))) { ++n; } + copy_file(path_to_copy, add_number(path_to_copy, n)); + }; - boost::filesystem::copy_file(read_path("config.xml"), write_path(String::compose("config.xml.%1", n))); - boost::filesystem::copy_file(read_path("cinemas.xml"), write_path(String::compose("cinemas.xml.%1", n))); - boost::filesystem::copy_file(read_path("dkdm_recipients.xml"), write_path(String::compose("dkdm_recipients.xml.%1", n))); + /* Make a backup copy of any config.xml, cinemas.xml, dkdm_recipients.xml that we might be about + * to write over. This is more intended for the situation where we have a corrupted config.xml, + * and decide to overwrite it with a new one (possibly losing important details in the corrupted + * file). But we might as well back up the other files while we're about it. + */ + try { + /* This uses the State::write_path stuff so, e.g. for a current version 2.16 we might copy + * ~/.config/dcpomatic2/2.16/config.xml to ~/.config/dcpomatic2/2.16/config.xml.1 + */ + copy_adding_number (config_write_file()); + + /* These do not use State::write_path, so whatever path is in the Config we will copy + * adding a number. + */ + copy_adding_number (_cinemas_file); + copy_adding_number (_dkdm_recipients_file); } catch (...) {} } diff --git a/test/config_test.cc b/test/config_test.cc index 48dec27d7..0e6a05ac0 100644 --- a/test/config_test.cc +++ b/test/config_test.cc @@ -26,22 +26,31 @@ using std::ofstream; +using std::string; +using boost::optional; -static void -rewrite_bad_config () +static string +rewrite_bad_config (string filename, string extra_line) { + using namespace boost::filesystem; + + auto base = path("build/test/bad_config/2.16"); + auto file = base / filename; + boost::system::error_code ec; - boost::filesystem::remove ("build/test/bad_config/2.16/config.xml", ec); + remove (file, ec); - Config::override_path = "build/test/bad_config"; - boost::filesystem::create_directories ("build/test/bad_config/2.16"); - ofstream f ("build/test/bad_config/2.16/config.xml"); + boost::filesystem::create_directories (base); + std::ofstream f (file.string().c_str()); f << "\n" << "\n" << "\n" + << extra_line << "\n" << "\n"; f.close (); + + return dcp::file_to_string (file); } @@ -50,46 +59,90 @@ BOOST_AUTO_TEST_CASE (config_backup_test) ConfigRestorer cr; Config::override_path = "build/test/bad_config"; - Config::drop(); - boost::filesystem::remove_all ("build/test/bad_config"); - rewrite_bad_config(); + /* Write an invalid config file to config.xml */ + auto const first_write_xml = rewrite_bad_config("config.xml", "first write"); + /* Load the config; this should fail, causing the bad config to be copied to config.xml.1 + * and a new config.xml created in its place. + */ Config::instance(); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.1") == first_write_xml); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); - rewrite_bad_config(); + auto const second_write_xml = rewrite_bad_config("config.xml", "second write"); Config::instance(); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.1") == first_write_xml); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.2") == second_write_xml); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); - rewrite_bad_config(); + auto const third_write_xml = rewrite_bad_config("config.xml", "third write"); Config::instance(); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.1") == first_write_xml); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.2") == second_write_xml); BOOST_CHECK ( boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.3") == third_write_xml); BOOST_CHECK (!boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); Config::drop(); - rewrite_bad_config(); + auto const fourth_write_xml = rewrite_bad_config("config.xml", "fourth write"); Config::instance(); BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.1")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.1") == first_write_xml); BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.2")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.2") == second_write_xml); BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.3")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.3") == third_write_xml); BOOST_CHECK (boost::filesystem::exists("build/test/bad_config/2.16/config.xml.4")); + BOOST_CHECK (dcp::file_to_string("build/test/bad_config/2.16/config.xml.4") == fourth_write_xml); +} + + +BOOST_AUTO_TEST_CASE (config_backup_with_link_test) +{ + using namespace boost::filesystem; + + ConfigRestorer cr; + + auto base = path("build/test/bad_config"); + auto version = base / "2.16"; + + Config::override_path = base; + Config::drop(); + + boost::filesystem::remove_all (base); + + boost::filesystem::create_directories (version); + std::ofstream f (path(version / "config.xml").string().c_str()); + f << "\n" + << "\n" + << "" << path(version / "actual.xml").string() << "\n" + << "\n"; + f.close (); + + Config::drop (); + /* Cause actual.xml to be backed up */ + rewrite_bad_config ("actual.xml", "first write"); + Config::instance (); + + /* Make sure actual.xml was backed up to the right place */ + BOOST_CHECK (boost::filesystem::exists(version / "actual.xml.1")); } -- 2.30.2