-
Notifications
You must be signed in to change notification settings - Fork 123
Checkconf (2nd attempt) #733
Changes from 12 commits
cf3b3b1
5858ed6
7fd6416
93e607c
034d202
c91f567
c18cb8e
6ad66dd
78cdd34
dff7a6e
5e146af
0caed1e
0639d04
6de7280
7bb49be
274a395
d15a8d9
7bf749b
e9ccac6
ac8c148
742cf1a
5d92d69
a904008
aed7053
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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,29 @@ 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 = nullptr; | ||
try | ||
{ | ||
checkConfFunction = reinterpret_cast<checkConfPtr> (pluginDatabase->getSymbol (newPlugin, "checkconf")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we can return 0 here instead of exception? |
||
} | ||
catch (...) | ||
{ | ||
// the checkconf operation is optional, so no worries if it was not found | ||
} | ||
|
||
if (checkConfFunction != nullptr) | ||
{ | ||
ckdb::Key * errorKey = ckdb::keyNew (0); | ||
int checkResult = checkConfFunction (errorKey, newPlugin.getConfig ().getKeySet ()); | ||
if (checkResult == -1) | ||
{ | ||
throw PluginConfigInvalid (errorKey); | ||
} | ||
ckdb::keyDel (errorKey); | ||
} | ||
|
||
toAdd.push_back (newPlugin); | ||
sort (); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -210,19 +210,16 @@ PluginDatabase::Status ModulesPluginDatabase::status (PluginSpec const & spec) c | |
|
||
std::string ModulesPluginDatabase::lookupInfo (PluginSpec const & spec, std::string const & which) const | ||
{ | ||
PluginPtr plugin; | ||
try | ||
{ | ||
plugin = impl->modules.load (spec.getName (), spec.getConfig ()); | ||
} | ||
catch (...) | ||
{ | ||
throw; | ||
} | ||
|
||
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 | ||
{ | ||
PluginPtr plugin = impl->modules.load (spec.getName (), spec.getConfig ()); | ||
return plugin->getSymbol (which); | ||
} | ||
|
||
PluginSpec ModulesPluginDatabase::lookupMetadata (std::string const & which) const | ||
{ | ||
std::vector<std::string> allPlugins = listAllPlugins (); | ||
|
@@ -367,5 +364,12 @@ std::string MockPluginDatabase::lookupInfo (PluginSpec const & spec, std::string | |
|
||
return ""; | ||
} | ||
|
||
PluginDatabase::func_t MockPluginDatabase::getSymbol (PluginSpec const & spec ELEKTRA_UNUSED, | ||
std::string const & which ELEKTRA_UNUSED) const | ||
{ | ||
// TODO implement mockup | ||
return NULL; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it looks pretty well! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment there is no use for the mocked method. @markus2330 any advice for a good implementation or should I just leave it as it is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For unit tests you should return some function as done for other mocks with |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@markus2330 @Namoshek @iandonnelly is this a viable example?