Skip to content

Commit

Permalink
Convert ConfigService paths to absolute on demand
Browse files Browse the repository at this point in the history 
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 mantidproject#12425
  • Loading branch information
@martyngigg
martyngigg committed Aug 6, 2019
1 parent 6344128 commit 4e9fa68
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 142 deletions.
15 changes: 3 additions & 12 deletions Framework/Kernel/inc/MantidKernel/ConfigService.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class MANTID_KERNEL_DLL ConfigServiceImpl final {
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
Expand Down Expand Up @@ -257,9 +257,6 @@ class MANTID_KERNEL_DLL ConfigServiceImpl final {

/// 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;
Expand Down Expand Up @@ -292,22 +289,16 @@ class MANTID_KERNEL_DLL ConfigServiceImpl final {
/// 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
std::string m_PropertyString;
std::string m_propertyString;
/// The filename of the Mantid properties file
const std::string m_properties_file_name;
/// The filename of the Mantid user properties file
const std::string m_user_properties_file_name;
/// Store a list of data search paths
std::vector<std::string> m_DataSearchDirs;
std::vector<std::string> m_dataSearchDirs;
/// Store a list of instrument directory paths
std::vector<std::string> m_instrumentDirs;
/// The list of available facilities
Expand Down
160 changes: 51 additions & 109 deletions Framework/Kernel/src/ConfigService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -157,16 +166,17 @@ template <typename T> class ConfigServiceImpl::WrappedObject : public T {
/// 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
m_user_properties_file_name("Mantid-mpi.user.properties"),
#else
m_user_properties_file_name("Mantid.user.properties"),
#endif
m_dataSearchDirs(), m_instrumentDirs(), m_proxyInfo(), m_isProxySet(false) {
m_dataSearchDirs(), m_instrumentDirs(), m_proxyInfo(),
m_isProxySet(false) {
// getting at system details
m_pSysConfig =
std::make_unique<WrappedObject<Poco::Util::SystemConfiguration>>();
Expand All @@ -179,25 +189,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;
Expand Down Expand Up @@ -388,7 +379,7 @@ void ConfigServiceImpl::loadConfig(const std::string &filename,

if (!append) {
// remove the previous property string
m_PropertyString = "";
m_propertyString = "";
m_changed_keys.clear();
}

Expand All @@ -411,10 +402,10 @@ void ConfigServiceImpl::loadConfig(const std::string &filename,
temp = checkForBadConfigOptions(filename, temp);

// store the property string
if ((append) && (!m_PropertyString.empty())) {
m_PropertyString = m_PropertyString + "\n" + temp;
if ((append) && (!m_propertyString.empty())) {
m_propertyString = m_propertyString + "\n" + temp;
} else {
m_PropertyString = temp;
m_propertyString = temp;
}
} catch (std::exception &e) {
// there was a problem loading the file - it probably is not there
Expand All @@ -425,7 +416,7 @@ void ConfigServiceImpl::loadConfig(const std::string &filename,
}

// use the cached property string to initialise the POCO property file
std::istringstream istr(m_PropertyString);
std::istringstream istr(m_propertyString);
m_pConf =
std::make_unique<WrappedObject<Poco::Util::PropertyFileConfiguration>>(
istr);
Expand Down Expand Up @@ -469,30 +460,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
Expand Down Expand Up @@ -550,25 +517,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.
Expand All @@ -582,11 +530,11 @@ 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();
m_dataSearchDirs.clear();
} else {
m_DataSearchDirs = splitPath(paths);
m_dataSearchDirs = splitPath(paths);
}
}

Expand All @@ -603,9 +551,9 @@ bool ConfigServiceImpl::isInDataSearchList(const std::string &path) const {
replace(correctedPath.begin(), correctedPath.end(), '\\', '/');

auto it =
std::find_if(m_DataSearchDirs.cbegin(), m_DataSearchDirs.cend(),
std::find_if(m_dataSearchDirs.cbegin(), m_dataSearchDirs.cend(),
std::bind2nd(std::equal_to<std::string>(), correctedPath));
return (it != m_DataSearchDirs.end());
return (it != m_dataSearchDirs.end());
}

/**
Expand Down Expand Up @@ -745,9 +693,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();
Expand Down Expand Up @@ -869,21 +814,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"
Expand Down Expand Up @@ -1026,12 +970,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") {
Expand All @@ -1040,8 +982,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);
}
Expand Down Expand Up @@ -1524,7 +1464,7 @@ std::string ConfigServiceImpl::getUserPropertiesDir() const {
* @returns A vector of strings containing the defined search directories
*/
const std::vector<std::string> &ConfigServiceImpl::getDataSearchDirs() const {
return m_DataSearchDirs;
return m_dataSearchDirs;
}

/**
Expand Down Expand Up @@ -1566,8 +1506,8 @@ void ConfigServiceImpl::appendDataSearchSubDir(const std::string &subdir) {
return;
}

auto newDataDirs = m_DataSearchDirs;
for (const auto &path : m_DataSearchDirs) {
auto newDataDirs = m_dataSearchDirs;
for (const auto &path : m_dataSearchDirs) {
Poco::Path newDirPath;
try {
newDirPath = Poco::Path(path);
Expand Down Expand Up @@ -1601,8 +1541,10 @@ void ConfigServiceImpl::appendDataSearchDir(const std::string &path) {
return;
}
if (!isInDataSearchList(dirPath.toString())) {
auto newSearchString = Strings::join(std::begin(m_DataSearchDirs),
std::end(m_DataSearchDirs), ";");
auto newSearchString = Strings::join(std::begin(m_dataSearchDirs),
std::end(m_dataSearchDirs), ";");

newSearchString.append(";").append(path);
setString("datasearch.directories", newSearchString);
}
}
Expand All @@ -1613,7 +1555,7 @@ void ConfigServiceImpl::appendDataSearchDir(const std::string &path) {
*/
void ConfigServiceImpl::setInstrumentDirectories(
const std::vector<std::string> &directories) {
m_InstrumentDirs = directories;
m_instrumentDirs = directories;
}

/**
Expand All @@ -1622,23 +1564,23 @@ void ConfigServiceImpl::setInstrumentDirectories(
*/
const std::vector<std::string> &
ConfigServiceImpl::getInstrumentDirectories() const {
return m_InstrumentDirs;
return m_instrumentDirs;
}

/**
* Return the base search directories for XML instrument definition files (IDFs)
* @returns a last entry of getInstrumentDirectories
*/
const std::string ConfigServiceImpl::getInstrumentDirectory() const {
return m_InstrumentDirs.back();
return m_instrumentDirs.back();
}
/**
* Return the search directory for vtp files
* @returns a path
*/
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());
Expand All @@ -1660,28 +1602,28 @@ const std::string ConfigServiceImpl::getVTPFileDirectory() {
* - The install directory/instrument
*/
void ConfigServiceImpl::cacheInstrumentPaths() {
m_InstrumentDirs.clear();
m_instrumentDirs.clear();

Poco::Path path(getAppDataDir());
path.makeDirectory();
path.pushDirectory("instrument");
const std::string appdatadir = path.toString();
addDirectoryifExists(appdatadir, m_InstrumentDirs);
addDirectoryifExists(appdatadir, m_instrumentDirs);

#ifndef _WIN32
addDirectoryifExists("/etc/mantid/instrument", m_InstrumentDirs);
addDirectoryifExists("/etc/mantid/instrument", m_instrumentDirs);
#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
// directory of the executable, not the current working directory.
directoryName =
Poco::Path(getPropertiesDir()).resolve("../instrument").toString();
}
addDirectoryifExists(directoryName, m_InstrumentDirs);
addDirectoryifExists(directoryName, m_instrumentDirs);
}

/**
Expand Down

0 comments on commit 4e9fa68

Please sign in to comment.