Commit 77385d00 authored by Gigg, Martyn Anthony's avatar Gigg, Martyn Anthony Committed by Conor Finn
Browse files

Convert ConfigService paths to absolute on demand

If the key contains the string "director" it is assumed to point
to a directory and if it its value is relatiove then the path
is converted to an absolute path. This reduces the workload on
startup while keeping the caching of important keys
such as the datasearch paths and instrument directories for performance.
Refs #12425
parent 0e2df044
......@@ -119,7 +119,7 @@ public:
void saveConfig(const std::string &filename) const;
/// Searches for a configuration property
std::string getString(const std::string &keyName,
bool use_cache = true) const;
bool pathAbsolute = true) const;
/// Searches for a key in the configuration property
std::vector<std::string> getKeys(const std::string &keyName) const;
/// Returns a list of all full keys in the config
......@@ -255,9 +255,6 @@ private:
/// Writes out a fresh user properties file
void createUserPropertiesFile() const;
/// Convert any relative paths to absolute ones and store them locally so that
/// if the working directory is altered the paths will not be affected
void convertRelativeToAbsolute();
/// Make a relative path or a list of relative paths into an absolute one.
std::string makeAbsolute(const std::string &dir,
const std::string &key) const;
......@@ -288,12 +285,6 @@ private:
/// A set of property keys that have been changed
mutable std::set<std::string> m_changed_keys;
/// A map storing string/key pairs where the string denotes a path
/// that could be relative in the user properties file
/// The boolean indicates whether the path needs to exist or not
std::map<std::string, bool> m_ConfigPaths;
/// Local storage for the relative path key/values that have been changed
std::map<std::string, std::string> m_AbsolutePaths;
/// The directory that is considered to be the base directory
std::string m_strBaseDir;
/// The configuration properties in string format
......
......@@ -84,6 +84,15 @@ namespace { // anonymous namespace for some utility functions
/// static Logger object
Logger g_log("ConfigService");
/// Identifier for a key that will contain a path. This is defined
/// so as to catch both the word directory and directories in a key name.
/// Any keys found containing this string are assumed to contain paths
/// and if their values are relative paths then when fetched they
/// are transformed to absolute paths using the application directory
/// as a base. This is necessary to avoid problems with the interpretation of
/// a relative path if current directory s changed at runtime.
std::string DIRECTORY_KEY_MAGIC{"director"};
/**
* Split the supplied string on semicolons.
*
......@@ -118,8 +127,8 @@ std::vector<std::string> splitPath(const std::string &path) {
/// Private constructor for singleton class
ConfigServiceImpl::ConfigServiceImpl()
: m_pConf(nullptr), m_pSysConfig(nullptr), m_changed_keys(),
m_ConfigPaths(), m_AbsolutePaths(), m_strBaseDir(""),
m_PropertyString(""), m_properties_file_name("Mantid.properties"),
m_strBaseDir(""), m_PropertyString(""),
m_properties_file_name("Mantid.properties"),
#ifdef MPI_BUILD
// Use a different user properties file for an mpi-enabled build to avoid
// confusion if both are used on the same file system
......@@ -140,25 +149,6 @@ ConfigServiceImpl::ConfigServiceImpl()
setBaseDirectory();
// Fill the list of possible relative path keys that may require conversion to
// absolute paths
m_ConfigPaths.emplace("mantidqt.python_interfaces_directory", true);
m_ConfigPaths.emplace("framework.plugins.directory", true);
m_ConfigPaths.emplace("pvplugins.directory", false);
m_ConfigPaths.emplace("mantidqt.plugins.directory", false);
m_ConfigPaths.emplace("instrumentDefinition.directory", true);
m_ConfigPaths.emplace("instrumentDefinition.vtpDirectory", true);
m_ConfigPaths.emplace("groupingFiles.directory", true);
m_ConfigPaths.emplace("maskFiles.directory", true);
m_ConfigPaths.emplace("colormaps.directory", true);
m_ConfigPaths.emplace("requiredpythonscript.directories", true);
m_ConfigPaths.emplace("pythonscripts.directory", true);
m_ConfigPaths.emplace("pythonscripts.directories", true);
m_ConfigPaths.emplace("python.plugins.directories", true);
m_ConfigPaths.emplace("user.python.plugins.directories", true);
m_ConfigPaths.emplace("datasearch.directories", true);
m_ConfigPaths.emplace("icatDownload.directory", true);
// attempt to load the default properties file that resides in the directory
// of the executable
std::string propertiesFilesList;
......@@ -432,30 +422,6 @@ void ConfigServiceImpl::configureLogging() {
}
}
/**
* Searches the stored list for keys that have been loaded from the config file
* and may contain
* relative paths. Any it find are converted to absolute paths and stored
* separately
*/
void ConfigServiceImpl::convertRelativeToAbsolute() {
if (m_ConfigPaths.empty())
return;
m_AbsolutePaths.clear();
std::map<std::string, bool>::const_iterator send = m_ConfigPaths.end();
for (std::map<std::string, bool>::const_iterator sitr = m_ConfigPaths.begin();
sitr != send; ++sitr) {
std::string key = sitr->first;
if (!m_pConf->hasProperty(key))
continue;
std::string value(m_pConf->getString(key));
value = makeAbsolute(value, key);
m_AbsolutePaths.emplace(key, value);
}
}
/**
* Make a relative path or a list of relative paths into an absolute one.
* @param dir :: The directory to convert
......@@ -513,25 +479,6 @@ std::string ConfigServiceImpl::makeAbsolute(const std::string &dir,
}
converted = Poco::Path(converted).makeDirectory().toString();
// C++ doesn't have a const version of operator[] for maps so I can't call
// that here
auto it = m_ConfigPaths.find(key);
bool required = false;
if (it != m_ConfigPaths.end()) {
required = it->second;
}
try {
if (required && !Poco::File(converted).exists()) {
g_log.debug() << "Required properties path \"" << converted
<< "\" in the \"" << key << "\" variable does not exist.\n";
converted = "";
}
} catch (Poco::FileException &) {
g_log.debug() << "Required properties path \"" << converted
<< "\" in the \"" << key << "\" variable does not exist.\n";
converted = "";
}
// Backward slashes cannot be allowed to go into our properties file
// Note this is a temporary fix for ticket #2445.
// Ticket #2460 prompts a review of our path handling in the config service.
......@@ -545,7 +492,7 @@ std::string ConfigServiceImpl::makeAbsolute(const std::string &dir,
* The value of the key should be a semi-colon separated list of directories
*/
void ConfigServiceImpl::cacheDataSearchPaths() {
std::string paths = getString("datasearch.directories");
std::string paths = getString("datasearch.directories", true);
if (paths.empty()) {
m_DataSearchDirs.clear();
} else {
......@@ -707,9 +654,6 @@ void ConfigServiceImpl::updateConfig(const std::string &filename,
if (update_caches) {
// Only configure logging once
configureLogging();
// Ensure that any relative paths given in the configuration file are
// relative to the correct directory
convertRelativeToAbsolute();
// Configure search paths into a specially saved store as they will be used
// frequently
cacheDataSearchPaths();
......@@ -831,21 +775,20 @@ void ConfigServiceImpl::saveConfig(const std::string &filename) const {
*
* @param keyName :: The case sensitive name of the property that you need the
*value of.
* @param use_cache :: If true, the local cache of directory names is queried
*first.
* @param pathAbsolute :: If true then any key that looks like it contains
* a path has this path converted to an absolute path.
* @returns The string value of the property, or an empty string if the key
*cannot be found
*/
std::string ConfigServiceImpl::getString(const std::string &keyName,
bool use_cache) const {
if (use_cache) {
auto mitr = m_AbsolutePaths.find(keyName);
if (mitr != m_AbsolutePaths.end()) {
return (*mitr).second;
}
}
bool pathAbsolute) const {
if (m_pConf->hasProperty(keyName)) {
return m_pConf->getString(keyName);
std::string value = m_pConf->getString(keyName);
if (pathAbsolute &&
keyName.rfind(DIRECTORY_KEY_MAGIC) != std::string::npos) {
value = makeAbsolute(value, keyName);
}
return value;
}
g_log.debug() << "Unable to find " << keyName << " in the properties file"
......@@ -988,12 +931,10 @@ void ConfigServiceImpl::setString(const std::string &key,
if (value == old)
return;
// Ensure we keep a correct full path
std::map<std::string, bool>::const_iterator itr = m_ConfigPaths.find(key);
if (itr != m_ConfigPaths.end()) {
m_AbsolutePaths[key] = makeAbsolute(value, key);
}
// Update the internal value
m_pConf->setString(key, value);
// Cache things that are accessed frequently
if (key == "datasearch.directories") {
cacheDataSearchPaths();
} else if (key == "instrumentDefinition.directory") {
......@@ -1002,8 +943,6 @@ void ConfigServiceImpl::setString(const std::string &key,
appendDataSearchDir(value);
}
m_pConf->setString(key, value);
m_notificationCenter.postNotification(new ValueChanged(key, value, old));
m_changed_keys.insert(key);
}
......@@ -1565,6 +1504,7 @@ void ConfigServiceImpl::appendDataSearchDir(const std::string &path) {
if (!isInDataSearchList(dirPath.toString())) {
auto newSearchString = Strings::join(std::begin(m_DataSearchDirs),
std::end(m_DataSearchDirs), ";");
newSearchString.append(path);
setString("datasearch.directories", newSearchString);
}
}
......@@ -1600,7 +1540,7 @@ const std::string ConfigServiceImpl::getInstrumentDirectory() const {
*/
const std::string ConfigServiceImpl::getVTPFileDirectory() {
// Determine the search directory for XML instrument definition files (IDFs)
std::string directoryName = getString("instrumentDefinition.vtpDirectory");
std::string directoryName = getString("instrumentDefinition.vtp.directory");
if (directoryName.empty()) {
Poco::Path path(getAppDataDir());
......@@ -1635,7 +1575,7 @@ void ConfigServiceImpl::cacheInstrumentPaths() {
#endif
// Determine the search directory for XML instrument definition files (IDFs)
std::string directoryName = getString("instrumentDefinition.directory");
std::string directoryName = getString("instrumentDefinition.directory", true);
if (directoryName.empty()) {
// This is the assumed deployment directory for IDFs, where we need to be
// relative to the
......
......@@ -350,10 +350,32 @@ public:
TS_ASSERT_EQUALS(noseCountString, "");
}
void TestRelativeToAbsolute() {
// std::string path =
// ConfigService::Instance().getString("defaultsave.directory");
// TS_ASSERT( Poco::Path(path).isAbsolute() );
void testRelativeToAbsoluteForKeysWithCorrectIdentifier() {
auto &cfg = ConfigService::Instance();
cfg.setString("mantid.test.directory", "..");
cfg.setString("mantid.test.directories", "..");
TS_ASSERT(Poco::Path(cfg.getString("mantid.test.directory")).isAbsolute());
TS_ASSERT(
Poco::Path(cfg.getString("mantid.test.directories")).isAbsolute());
}
void testNoRelativeToAbsoluteForKeysWithoutCorrectIdentifier() {
auto &cfg = ConfigService::Instance();
cfg.setString("mantid.test.direc", "..");
TS_ASSERT(Poco::Path(cfg.getString("mantid.test.direc")).isRelative());
}
void testNoRelativeToAbsoluteForKeysOnRequest() {
auto &cfg = ConfigService::Instance();
cfg.setString("mantid.test.directory", "..");
cfg.setString("mantid.test.directories", "..");
TS_ASSERT(
Poco::Path(cfg.getString("mantid.test.directory", false)).isRelative());
TS_ASSERT(Poco::Path(cfg.getString("mantid.test.directories", false))
.isRelative());
}
void testAppendProperties() {
......@@ -646,8 +668,6 @@ public:
// Returns all *root* keys, i.e. unique keys left of the first period
std::vector<std::string> keyVector = ConfigService::Instance().getKeys("");
// The 4 unique in the file and the ConfigService always sets a
// datasearch.directories key on creation
TS_ASSERT_EQUALS(keyVector.size(), 5);
}
......
......@@ -122,11 +122,12 @@ void export_ConfigService() {
"default.instrument is returned",
(arg("self"), arg("instrumentName") = boost::python::object()))
[return_value_policy<copy_const_reference>()])
.def(
"getString", &ConfigServiceImpl::getString,
getStringOverload("Returns the named key's value. If use_cache = "
"true [default] then relative paths->absolute",
(arg("self"), arg("key"), arg("use_cache") = true)))
.def("getString", &ConfigServiceImpl::getString,
getString_Overload(
"Returns the named key's value. If use_cache = "
"true [default] then relative paths->absolute",
(arg("self"), arg("key"), arg("pathAbsolute") = true)))
.def("setString", &ConfigServiceImpl::setString,
(arg("self"), arg("key"), arg("value")),
"Set the given property name. "
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment