Remove PluginMetadata::operator== and != for PluginMetadata

Unlike the other metadata classes, they were only comparing plugin names
not full object equality. Checking full object equality isn't very
useful, so instead the operators have been replaced by a new
NameMatches() method that takes a non-regex plugin name string.
This commit is contained in:
Oliver Hamlet
2020-07-11 19:14:06 +01:00
parent c9ce0f96d0
commit c74f2f4978
5 changed files with 53 additions and 78 deletions
+10 -12
View File
@@ -236,20 +236,18 @@ public:
LOOT_API bool IsRegexPlugin() const;
/**
* Check if two PluginMetadata objects are equal by comparing their name
* values.
* @returns True if the plugin names are case-insensitively equal, false
* otherwise.
* Check if the given plugin name matches this plugin metadata object's
* name field.
*
* If the name field is a regular expression, the given plugin name will be
* matched against it, otherwise the strings will be compared
* case-insensitively. The given plugin name must be literal, i.e. not a
* regular expression.
* @returns True if the given plugin name matches this metadata's plugin
* name, false otherwise.
*/
LOOT_API bool operator==(const PluginMetadata& rhs) const;
LOOT_API bool NameMatches(const std::string& pluginName) const;
/**
* Check if two PluginMetadata objects are not equal by comparing their name
* values.
* @returns True if the plugin names are not case-insensitively equal, false
* otherwise.
*/
LOOT_API bool operator!=(const PluginMetadata& rhs) const;
private:
std::string name_;
std::optional<std::string> group_;
+6 -13
View File
@@ -199,20 +199,13 @@ bool PluginMetadata::IsRegexPlugin() const {
return strpbrk(name_.c_str(), ":\\*?|") != nullptr;
}
bool PluginMetadata::operator==(const PluginMetadata& rhs) const {
if (IsRegexPlugin() == rhs.IsRegexPlugin()) {
return CompareFilenames(name_, rhs.name_) == 0;
bool PluginMetadata::NameMatches(const std::string& pluginName) const {
if (IsRegexPlugin()) {
return std::regex_match(
pluginName,
std::regex(name_, std::regex::ECMAScript | std::regex::icase));
}
if (IsRegexPlugin())
return regex_match(rhs.GetName(),
regex(name_, regex::ECMAScript | regex::icase));
else
return regex_match(name_,
regex(rhs.GetName(), regex::ECMAScript | regex::icase));
}
bool PluginMetadata::operator!=(const PluginMetadata& rhs) const {
return !(*this == rhs);
return CompareFilenames(name_, pluginName) == 0;
}
}
+14 -9
View File
@@ -60,7 +60,7 @@ void MetadataList::Load(const std::filesystem::path& filepath) {
PluginMetadata plugin(node.as<PluginMetadata>());
if (plugin.IsRegexPlugin())
regexPlugins_.push_back(plugin);
else if (!plugins_.insert(plugin).second)
else if (!plugins_.emplace(Filename(plugin.GetName()), plugin).second)
throw FileAccessError("More than one entry exists for plugin \"" +
plugin.GetName() + "\"");
}
@@ -158,7 +158,9 @@ void MetadataList::Clear() {
std::vector<PluginMetadata> MetadataList::Plugins() const {
std::vector<PluginMetadata> plugins;
plugins.reserve(plugins_.size() + regexPlugins_.size());
plugins.insert(plugins.end(), plugins_.begin(), plugins_.end());
for (const auto& pluginPair : plugins_) {
plugins.push_back(pluginPair.second);
}
plugins.insert(plugins.end(), regexPlugins_.begin(), regexPlugins_.end());
return plugins;
@@ -198,17 +200,20 @@ std::optional<PluginMetadata> MetadataList::FindPlugin(
const std::string& pluginName) const {
PluginMetadata match(pluginName);
auto it = plugins_.find(match);
auto it = plugins_.find(Filename(pluginName));
if (it != plugins_.end())
match = *it;
match = it->second;
// Now we want to also match possibly multiple regex entries.
auto regIt = find(regexPlugins_.begin(), regexPlugins_.end(), match);
auto nameMatches = [&](const PluginMetadata& pluginMetadata) {
return pluginMetadata.NameMatches(pluginName);
};
auto regIt = find_if(regexPlugins_.begin(), regexPlugins_.end(), nameMatches);
while (regIt != regexPlugins_.end()) {
match.MergeMetadata(*regIt);
regIt = find(++regIt, regexPlugins_.end(), match);
regIt = find_if(++regIt, regexPlugins_.end(), nameMatches);
}
if (match.HasNameOnly()) {
@@ -222,7 +227,7 @@ void MetadataList::AddPlugin(const PluginMetadata& plugin) {
if (plugin.IsRegexPlugin())
regexPlugins_.push_back(plugin);
else {
if (!plugins_.insert(plugin).second)
if (!plugins_.emplace(Filename(plugin.GetName()), plugin).second)
throw std::invalid_argument(
"Cannot add \"" + plugin.GetName() +
"\" to the metadata list as another entry already exists.");
@@ -232,7 +237,7 @@ void MetadataList::AddPlugin(const PluginMetadata& plugin) {
// Doesn't erase matching regex entries, because they might also
// be required for other plugins.
void MetadataList::ErasePlugin(const std::string& pluginName) {
auto it = plugins_.find(PluginMetadata(pluginName));
auto it = plugins_.find(Filename(pluginName));
if (it != plugins_.end()) {
plugins_.erase(it);
@@ -251,7 +256,7 @@ void MetadataList::EvalAllConditions(ConditionEvaluator& conditionEvaluator) {
plugins_.clear();
for (const auto& plugin : unevaluatedPlugins_) {
plugins_.insert(conditionEvaluator.EvaluateAll(plugin));
plugins_.emplace(plugin.first, conditionEvaluator.EvaluateAll(plugin.second));
}
if (unevaluatedRegexPlugins_.empty())
+8 -16
View File
@@ -37,20 +37,12 @@
#include "loot/metadata/plugin_metadata.h"
namespace std {
/**
* A specialisation of std::hash for loot::PluginMetadata.
*/
template<>
struct hash<loot::PluginMetadata> {
/**
* Calculate a hash value for an object of a class that implements
* loot::PluginMetadata.
* @return The hash generated from the plugin's normalized filename.
*/
size_t operator()(const loot::PluginMetadata& plugin) const {
return hash<string>()(loot::NormalizeFilename(plugin.GetName()));
}
};
template<>
struct hash<loot::Filename> {
size_t operator()(const loot::Filename& filename) const {
return hash<string>()(loot::NormalizeFilename(std::string(filename)));
}
};
}
namespace loot {
@@ -83,11 +75,11 @@ public:
protected:
std::vector<Group> groups_;
std::vector<std::string> bashTags_;
std::unordered_set<PluginMetadata> plugins_;
std::unordered_map<Filename, PluginMetadata> plugins_;
std::vector<PluginMetadata> regexPlugins_;
std::vector<Message> messages_;
std::unordered_set<PluginMetadata> unevaluatedPlugins_;
std::unordered_map<Filename, PluginMetadata> unevaluatedPlugins_;
std::vector<PluginMetadata> unevaluatedRegexPlugins_;
std::vector<Message> unevaluatedMessages_;
};
@@ -68,40 +68,26 @@ TEST_P(
}
TEST_P(PluginMetadataTest,
equalityOperatorShouldUseCaseInsensitiveNameComparisonForNonRegexNames) {
PluginMetadata plugin1(blankEsm);
PluginMetadata plugin2(boost::to_lower_copy(blankEsm));
EXPECT_TRUE(plugin1 == plugin2);
nameMatchesShouldUseCaseInsensitiveNameComparisonForNonRegexNames) {
PluginMetadata plugin(blankEsm);
plugin1 = PluginMetadata(blankEsm);
plugin2 = PluginMetadata(blankDifferentEsm);
EXPECT_FALSE(plugin1 == plugin2);
EXPECT_TRUE(plugin.NameMatches(boost::to_lower_copy(blankEsm)));
EXPECT_FALSE(plugin.NameMatches(blankDifferentEsm));
}
TEST_P(PluginMetadataTest,
equalityOperatorShouldUseCaseInsensitiveNameComparisonForTwoRegexNames) {
PluginMetadata plugin1("Blan.\\.esm");
PluginMetadata plugin2("blan.\\.esm");
EXPECT_TRUE(plugin1 == plugin2);
EXPECT_TRUE(plugin2 == plugin1);
nameMatchesShouldTreatGivenPluginNameStringsAsLiterals) {
PluginMetadata plugin(blankEsm);
std::string regex = "blan.\\.esm";
plugin1 = PluginMetadata("Blan(k|p).esm");
plugin2 = PluginMetadata("Blan.\\.esm");
EXPECT_FALSE(plugin1 == plugin2);
EXPECT_FALSE(plugin2 == plugin1);
EXPECT_FALSE(plugin.NameMatches(regex));
}
TEST_P(PluginMetadataTest,
equalityOperatorShouldUseRegexMatchingForARegexNameAndANonRegexName) {
PluginMetadata plugin1("Blank.esm");
PluginMetadata plugin2("Blan.\\.esm");
EXPECT_TRUE(plugin1 == plugin2);
EXPECT_TRUE(plugin2 == plugin1);
TEST_P(PluginMetadataTest, nameMatchesShouldUseCaseInsensitiveRegexMatchingForARegexName) {
PluginMetadata plugin("Blan.\\.esm");
plugin1 = PluginMetadata("Blan.esm");
plugin2 = PluginMetadata("Blan.\\.esm");
EXPECT_FALSE(plugin1 == plugin2);
EXPECT_FALSE(plugin2 == plugin1);
EXPECT_TRUE(plugin.NameMatches(boost::to_lower_copy(blankEsm)));
EXPECT_FALSE(plugin.NameMatches(blankDifferentEsm));
}
TEST_P(PluginMetadataTest, mergeMetadataShouldNotChangeName) {
@@ -114,7 +100,7 @@ TEST_P(PluginMetadataTest, mergeMetadataShouldNotChangeName) {
}
TEST_P(PluginMetadataTest,
mergeMetadataShouldNotUseMergedGroupIfItAndCurrentGroupAreBothExplicit) {
mergeMetadataShouldNotUseMergedGroupIfItAndCurrentGroupAreBothExplicit) {
PluginMetadata plugin1;
PluginMetadata plugin2;
@@ -886,7 +872,8 @@ TEST_P(PluginMetadataTest, decodingFromYamlShouldStoreAllGivenData) {
EXPECT_EQ("Blank.esp", plugin.GetName());
EXPECT_EQ(std::vector<File>({File("Blank.esm")}), plugin.GetLoadAfterFiles());
EXPECT_EQ(std::vector<File>({File("Blank.esm")}), plugin.GetRequirements());
EXPECT_EQ(std::vector<File>({File("Blank.esm")}), plugin.GetIncompatibilities());
EXPECT_EQ(std::vector<File>({File("Blank.esm")}),
plugin.GetIncompatibilities());
EXPECT_EQ(std::vector<Message>({Message(MessageType::say, "content")}),
plugin.GetMessages());
EXPECT_EQ(std::vector<Tag>({Tag("Relev")}), plugin.GetTags());