Merge pull request #733 from petermax2/checkconf2

Checkconf (2nd attempt)

doc/tutorials/plugins.md

@@ -23,6 +23,7 @@ Additionally, there is one more function called
 [ELEKTRA_PLUGIN_EXPORT](http://doc.libelektra.org/api/current/html/group__plugin.html#gabe78724d2d477eef39997fd9b85bff16),
 where once again `Plugin` should be replaced with the name of the plug-in, this time in lower-case. So for my line plugin this function would be
 `ELEKTRA_PLUGIN_EXPORT(line)`.
+The developer may define elektraPluginCheckConf() if configuration validation at mount time is desired.
 
 The KDB relies on the first five functions for interacting with configuration files stored in the key database.
 Calls for kdbGet() and kdbClose() will call the functions elektraPluginGet() and elektraPluginClose() respectively for the
@@ -322,6 +323,38 @@ The `elektraPluginOpen` and `elektraPluginClose` functions are not commonly used
 reviewing. `elektraPluginOpen` function runs before `elektraPluginGet` and is useful to do initialization if necessary for the plug-in. On the other
 hand `elektraPluginClose` is run after other functions of the plug-in and can be useful for freeing up resources.
 
+### elektraPluginCheckConf ###
+
+The `elektraPluginCheckConf` function may be used for validation of the plugin configuration during mount time. The signature of the function is:
+
+	int elektraLineCheckConfig (Key * errorKey, KeySet * conf)
+
+The configuration of the plugin is provided as `conf`. The function may report an error or warnings using the `errorKey` and the return value.
+
+The following convention was established for the return value of `elektraPluginCheckConf`:
+
+- 0: The configuration was OK and has not been changed
+- 1: The configuration has been changed and now it is OK
+- -1: The configuration was not OK and could not be fixed. An error has to be set to errorKey.
+
+The following example demonstrates how to limit the length of the values within the plugin configuration to 3 charaters.
+
+	int elektraLineCheckConfig (Key * errorKey, KeySet * conf)
+	{
+		Key * cur;
+		ksRewind (conf);
+		while ((cur = ksNext (conf)) != 0)
+		{
+			const char * value = keyString (cur);
+			if (strlen (value) > 3)
+			{
+				ELEKTRA_SET_ERRORF (ELEKTRA_ERROR_VALUE_LENGTH, errorKey, "value %s is more than 3 characters long", value);
+				return -1; // The configuration was not OK and could not be fixed
+			}
+		}
+		return 0; // The configuration was OK and has not been changed
+	}
+
 ### ELEKTRA_PLUGIN_EXPORT ###
 
 The last function, one that is always needed in a plug-in, is `ELEKTRA_PLUGIN_EXPORT`. This functions is responsible for letting Elektra know that

src/libs/tools/include/backendbuilder.hpp

@@ -94,6 +94,9 @@ class BackendBuilder : public BackendInterface
 	void removeProvided (std::vector<std::string> & needs) const;
 	void removeMetadata (std::set<std::string> & needsMetadata) const;
 
+protected:
+	KeySet backendConf;
+
 public:
 	explicit BackendBuilder (BackendBuilderInit const & bbi = BackendBuilderInit ());
 
@@ -146,6 +149,9 @@ class BackendBuilder : public BackendInterface
 	void recommendPlugin (std::string provider);
 
 	void fillPlugins (BackendInterface & b) const;
+
+	void setBackendConfig (KeySet const & ks);
+	KeySet getBackendConfig ();
 };
 
 /**
@@ -167,8 +173,13 @@ class GlobalPluginsBuilder : public BackendBuilder
 class MountBackendBuilder : public MountBackendInterface, public BackendBuilder
 {
 	Key mountpoint;
-	KeySet backendConf;
+
+	/**
+	 * Contains the keys of system/elektra/mountpoints.
+	 * It is needed to detect if a mountpoint already exists.
+	 */
 	KeySet mountConf;
+
 	std::string configfile;
 
 public:
@@ -183,10 +194,6 @@ class MountBackendBuilder : public MountBackendInterface, public BackendBuilder
 	std::string getMountpoint () const;
 
 	void setBackendConfig (KeySet const & ks);
-	KeySet getBackendConfig ()
-	{
-		return backendConf;
-	}
 
 	void useConfigFile (std::string file);
 	std::string getConfigFile () const;

src/libs/tools/include/plugindatabase.hpp

@@ -30,6 +30,9 @@ namespace tools
  */
 class PluginDatabase
 {
+protected:
+	typedef void (*func_t) ();
+
 public:
 	/**
 	 * @brief list all plugins
@@ -68,6 +71,16 @@ class PluginDatabase
 	 */
 	virtual std::string lookupInfo (PluginSpec const & whichplugin, std::string const & which) const = 0;
 
+	/**
+	 * @brief get exported plugin symbol
+	 *
+	 * @param whichplugin from which plugin?
+	 * @param which which symbol would you like to look up?
+	 *
+	 * @return the function pointer to the exported symbol or NULL if the symbol was not found
+	 */
+	virtual func_t getSymbol (PluginSpec const & whichplugin, std::string const & which) const = 0;
+
 	/**
 	 * @brief lookup which plugin handles meta data
 	 *
@@ -118,6 +131,7 @@ class ModulesPluginDatabase : public PluginDatabase
 	std::vector<std::string> listAllPlugins () const;
 	PluginDatabase::Status status (PluginSpec const & whichplugin) const;
 	std::string lookupInfo (PluginSpec const & spec, std::string const & which) const;
+	func_t getSymbol (PluginSpec const & whichplugin, std::string const & which) const;
 	PluginSpec lookupMetadata (std::string const & which) const;
 	PluginSpec lookupProvides (std::string const & provides) const;
 };
@@ -128,13 +142,20 @@ class ModulesPluginDatabase : public PluginDatabase
 class MockPluginDatabase : public ModulesPluginDatabase
 {
 public:
+	typedef int (*checkConfPtr) (ckdb::Key *, ckdb::KeySet *);
+
 	/// only data from here will be returned
 	/// @note that it is ordered by name, i.e., different ref-names cannot be distinguished
 	mutable std::unordered_map<PluginSpec, std::unordered_map<std::string, std::string>, PluginSpecHash, PluginSpecName> data;
 
 	std::vector<std::string> listAllPlugins () const;
 	PluginDatabase::Status status (PluginSpec const & whichplugin) const;
 	std::string lookupInfo (PluginSpec const & spec, std::string const & which) const;
+	func_t getSymbol (PluginSpec const & whichplugin, std::string const & which) const;
+	void setCheckconfFunction (checkConfPtr const newCheckconf);
+
+private:
+	checkConfPtr checkconf = NULL;
 };
 }
 }

src/libs/tools/include/pluginspec.hpp

@@ -51,6 +51,7 @@ class PluginSpec
 	void setName (std::string const & name);
 
 	void appendConfig (KeySet config);
+	void setConfig (KeySet config);
 
 	void validate (std::string const & str) const;
 

src/libs/tools/include/toolexcept.hpp

@@ -157,6 +157,35 @@ struct PluginAlreadyInserted : public PluginCheckException
 	std::string m_str;
 };
 
+struct PluginConfigInvalid : public PluginCheckException
+{
+	explicit PluginConfigInvalid (Key key) : m_key (key), m_str ()
+	{
+	}
+
+	explicit PluginConfigInvalid (std::string const & message) : m_str (message)
+	{
+	}
+
+	virtual const char * what () const throw () override
+	{
+		if (m_str.empty ())
+		{
+			std::stringstream ss;
+			ss << "The provided plugin configuration is not valid!\n";
+			ss << "Errors/Warnings during the check were:\n";
+			printError (ss, m_key);
+			printWarnings (ss, m_key);
+			m_str = ss.str ();
+		}
+		return m_str.c_str ();
+	}
+
+private:
+	Key m_key;
+	mutable std::string m_str;
+};
+
 struct BadPluginName : public PluginCheckException
 {
 	explicit BadPluginName (std::string name)

src/libs/tools/src/backendbuilder.cpp

@@ -384,12 +384,18 @@ void BackendBuilder::recommendPlugin (std::string name)
  * @pre Needs to be a unique new name (use refname if you want to add the same module multiple times)
  *
  * Will automatically resolve virtual plugins to actual plugins.
+ * 
+ * Also calls the checkconf function if provided by the plugin. The checkconf function has the 
+ * following signature: int checkconf (Key * errorKey, KeySet * config) and allows a plugin to 
+ * verify its configuration at mount time.
  *
  * @see resolveNeeds()
  * @param plugin
  */
 void BackendBuilder::addPlugin (PluginSpec const & plugin)
 {
+	typedef int (*checkConfPtr) (ckdb::Key *, ckdb::KeySet *);
+
 	for (auto & p : toAdd)
 	{
 		if (p.getFullName () == plugin.getFullName ())
@@ -409,6 +415,46 @@ void BackendBuilder::addPlugin (PluginSpec const & plugin)
 		newPlugin.appendConfig (provides.getConfig ());
 	}
 
+	// call plugin's checkconf function (if provided)
+	// this enables a plugin to verify its configuration at mount time
+	checkConfPtr checkConfFunction = reinterpret_cast<checkConfPtr> (pluginDatabase->getSymbol (newPlugin, "checkconf"));
+	if (checkConfFunction)
+	{
+		ckdb::Key * errorKey = ckdb::keyNew (0);
+
+		// merge plugin config and backend config together
+		ckdb::KeySet * pluginConfig = newPlugin.getConfig ().dup ();
+		ckdb::ksAppend (pluginConfig, backendConf.getKeySet ());
+
+		// call the plugin's checkconf function
+		int checkResult = checkConfFunction (errorKey, pluginConfig);
+		if (checkResult == -1)
+		{
+			ckdb::ksDel (pluginConfig);
+			throw PluginConfigInvalid (errorKey);
+		}
+		else if (checkResult == 1)
+		{
+			// separate plugin config from the backend config
+			ckdb::Key * backendParent = ckdb::keyNew ("system/", KEY_END);
+			ckdb::KeySet * newBackendConfig = ckdb::ksCut (pluginConfig, backendParent);
+
+			// take over the new configuration
+			KeySet modifiedPluginConfig = KeySet (pluginConfig);
+			KeySet modifiedBackendConfig = KeySet (newBackendConfig);
+
+			newPlugin.setConfig (modifiedPluginConfig);
+			setBackendConfig (modifiedBackendConfig);
+
+			ckdb::keyDel (backendParent);
+		}
+		else
+		{
+			ckdb::ksDel (pluginConfig);
+		}
+		ckdb::keyDel (errorKey);
+	}
+
 	toAdd.push_back (newPlugin);
 	sort ();
 }
@@ -428,6 +474,16 @@ void BackendBuilder::fillPlugins (BackendInterface & b) const
 	}
 }
 
+void BackendBuilder::setBackendConfig (KeySet const & ks)
+{
+	backendConf = ks;
+}
+
+KeySet BackendBuilder::getBackendConfig ()
+{
+	return backendConf;
+}
+
 GlobalPluginsBuilder::GlobalPluginsBuilder (BackendBuilderInit const & bbi) : BackendBuilder (bbi)
 {
 }
@@ -489,7 +545,7 @@ std::string MountBackendBuilder::getMountpoint () const
 
 void MountBackendBuilder::setBackendConfig (KeySet const & ks)
 {
-	backendConf = ks;
+	BackendBuilder::setBackendConfig (ks);
 }
 
 void MountBackendBuilder::useConfigFile (std::string file)

src/libs/tools/src/plugindatabase.cpp

@@ -210,17 +210,21 @@ PluginDatabase::Status ModulesPluginDatabase::status (PluginSpec const & spec) c
 
 std::string ModulesPluginDatabase::lookupInfo (PluginSpec const & spec, std::string const & which) const
 {
-	PluginPtr plugin;
+	PluginPtr plugin = impl->modules.load (spec.getName (), spec.getConfig ());
+	return plugin->lookupInfo (which);
+}
+
+PluginDatabase::func_t ModulesPluginDatabase::getSymbol (PluginSpec const & spec, std::string const & which) const
+{
 	try
 	{
-		plugin = impl->modules.load (spec.getName (), spec.getConfig ());
+		PluginPtr plugin = impl->modules.load (spec.getName (), spec.getConfig ());
+		return plugin->getSymbol (which);
 	}
 	catch (...)
 	{
-		throw;
+		return NULL;
 	}
-
-	return plugin->lookupInfo (which);
 }
 
 PluginSpec ModulesPluginDatabase::lookupMetadata (std::string const & which) const
@@ -367,5 +371,19 @@ std::string MockPluginDatabase::lookupInfo (PluginSpec const & spec, std::string
 
 	return "";
 }
+
+PluginDatabase::func_t MockPluginDatabase::getSymbol (PluginSpec const & spec ELEKTRA_UNUSED, std::string const & which) const
+{
+	if (which == "checkconf")
+	{
+		return reinterpret_cast<func_t> (checkconf);
+	}
+	return NULL;
+}
+
+void MockPluginDatabase::setCheckconfFunction (const MockPluginDatabase::checkConfPtr newCheckconf)
+{
+	checkconf = newCheckconf;
+}
 }
 }

src/libs/tools/src/pluginspec.cpp

@@ -195,6 +195,17 @@ void PluginSpec::appendConfig (KeySet c)
 	config.append (c);
 }
 
+/**
+ * @brief Set plugin config
+ *
+ * @param c new config to be used as plugin config
+ */
+void PluginSpec::setConfig (KeySet c)
+{
+	config.clear ();
+	config.append (c);
+}
+
 /**
  * @brief Check if str starts with a-z and then only has chars a-z, 0-9 or underscore (_)
  *