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.

diff --git a/src/lib/config.cc b/src/lib/config.cc
index 419436995dc829337bf59c0cf81a604c01741603..5496c0b3c788c9161e59eecc2bb6004adfacc190 100644 (file)
--- 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 48dec27d76543e9b13566ae1f614f3f9c75d62c8..0e6a05ac04e384e420ecbc2d900ba3415e989cda 100644 (file)
--- 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 << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
          << "<Config>\n"
          << "<Foo></Foo>\n"
+         << extra_line << "\n"
          << "</Config>\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 << "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+         << "<Config>\n"
+         << "<Link>" << path(version / "actual.xml").string() << "</Link>\n"
+         << "</Config>\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"));
 }