From bf531216569f1891cc66f29796f765a9f9d24f82 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 5 Dec 2015 15:59:17 +0000 Subject: [PATCH] Refactor plugin version extraction Into Version constructor. This will allow version extraction to be tested independently of plugin data loading for #510. --- src/backend/helpers/version.cpp | 69 +++++++++++++++++++++++- src/backend/metadata/condition_grammar.h | 2 +- src/backend/plugin/plugin.cpp | 68 ----------------------- src/backend/plugin/plugin.h | 2 +- src/gui/handler.cpp | 3 +- src/tests/backend/game/test_game.h | 44 +++++++-------- src/tests/backend/plugin/test_plugin.h | 8 +-- 7 files changed, 97 insertions(+), 99 deletions(-) diff --git a/src/backend/helpers/version.cpp b/src/backend/helpers/version.cpp index 1740fc72..1f1c9ce2 100644 --- a/src/backend/helpers/version.cpp +++ b/src/backend/helpers/version.cpp @@ -25,6 +25,9 @@ #include "helpers.h" #include "version.h" +#include + +#include #include #ifdef _WIN32 @@ -40,10 +43,72 @@ namespace loot { using namespace std; + /// REGEX expression definition + /// Each expression is composed of three parts: + /// 1. The marker string "version", "ver", "rev", "v" or "r" + /// 2. The version string itself. + + const char* regex1 = + "^(?:\\bversion\\b[ ]*(?:[:.\\-]?)|\\brevision\\b(?:[:.\\-]?))[ ]*" + "((?:alpha|beta|test|debug)?\\s*[-0-9a-zA-Z._+]+\\s*(?:alpha|beta|test|debug)?\\s*(?:[0-9]*))$" + ; + + const char* regex2 = + "(?:\\bversion\\b(?:[ :]?)|\\brevision\\b(?:[:.\\-]?))[ ]*" + "([0-9][-0-9a-zA-Z._]+\\+?)" + ; + + const char* regex3 = + "(?:\\bver(?:[:.]?)|\\brev(?:[:.]?))\\s*" + "([0-9][-0-9a-zA-Z._]*\\+?)" + ; + + // Matches "Updated: " for the Bashed patch + const char* regex4 = + "(?:Updated:)\\s*" + "([-0-9aAmMpP/ :]+)$" + ; + + // Matches isolated versions as last resort + const char* regex5 = + "(?:(?:\\bv|\\br)(?:\\s?)(?:[-.:])?(?:\\s*))" + "((?:(?:\\balpha\\b)?|(?:\\bbeta\\b)?)\\s*[0-9]+([-._]*(?!esp|esm)[0-9a-zA-Z]+)*\\+?)" + ; + + // Matches isolated versions as last resort + const char* regex6 = + "((?:(?:\\balpha\\b)?|(?:\\bbeta\\b)?)\\s*\\b[0-9][-0-9a-zA-Z._]*\\+?)$" + ; + + const char* regex7 = + "(^\\bmark\\b\\s*\\b[IVX0-9][-0-9a-zA-Z._+]*\\s*(?:alpha|beta|test|debug)?\\s*(?:[0-9]*)?)$" + ; + + /// Array used to try each of the expressions defined above using + /// an iteration for each of them. + const vector version_checks({ + regex(regex1, regex::ECMAScript | regex::icase), + regex(regex2, regex::ECMAScript | regex::icase), + regex(regex3, regex::ECMAScript | regex::icase), + regex(regex4, regex::ECMAScript | regex::icase), + regex(regex5, regex::ECMAScript | regex::icase), //This incorrectly identifies "OBSE v19" where 19 is any integer. + //regex(regex6, regex::ECMAScript | regex::icase), //This is responsible for metallicow's false positive. + regex(regex7, regex::ECMAScript | regex::icase) + }); + Version::Version() {} - Version::Version(const std::string& ver) - : verString(ver) {} + Version::Version(const std::string& ver) : verString(ver) { + for (size_t i = 0; i < version_checks.size(); ++i) { + smatch what; + if (regex_search(verString, what, version_checks[i])) { + //Use the first sub-expression match. + verString = string(what[1].first, what[1].second); + boost::trim(verString); + break; + } + } + } Version::Version(const boost::filesystem::path& file) { #ifdef _WIN32 diff --git a/src/backend/metadata/condition_grammar.h b/src/backend/metadata/condition_grammar.h index e3b622d4..fc6c74c4 100644 --- a/src/backend/metadata/condition_grammar.h +++ b/src/backend/metadata/condition_grammar.h @@ -328,7 +328,7 @@ namespace loot { trueVersion = Version(boost::filesystem::absolute("LOOT.exe")); else if (Plugin::IsValid(file, *_game)) { Plugin plugin(*_game, file, true); - trueVersion = Version(plugin.Version()); + trueVersion = Version(plugin.getDescription()); } else trueVersion = Version(_game->DataPath() / file); diff --git a/src/backend/plugin/plugin.cpp b/src/backend/plugin/plugin.cpp index 8e9726e1..c6915020 100644 --- a/src/backend/plugin/plugin.cpp +++ b/src/backend/plugin/plugin.cpp @@ -37,59 +37,6 @@ using namespace std; using libespm::FormId; namespace loot { - /// REGEX expression definition - /// Each expression is composed of three parts: - /// 1. The marker string "version", "ver", "rev", "v" or "r" - /// 2. The version string itself. - - const char* regex1 = - "^(?:\\bversion\\b[ ]*(?:[:.\\-]?)|\\brevision\\b(?:[:.\\-]?))[ ]*" - "((?:alpha|beta|test|debug)?\\s*[-0-9a-zA-Z._+]+\\s*(?:alpha|beta|test|debug)?\\s*(?:[0-9]*))$" - ; - - const char* regex2 = - "(?:\\bversion\\b(?:[ :]?)|\\brevision\\b(?:[:.\\-]?))[ ]*" - "([0-9][-0-9a-zA-Z._]+\\+?)" - ; - - const char* regex3 = - "(?:\\bver(?:[:.]?)|\\brev(?:[:.]?))\\s*" - "([0-9][-0-9a-zA-Z._]*\\+?)" - ; - - // Matches "Updated: " for the Bashed patch - const char* regex4 = - "(?:Updated:)\\s*" - "([-0-9aAmMpP/ :]+)$" - ; - - // Matches isolated versions as last resort - const char* regex5 = - "(?:(?:\\bv|\\br)(?:\\s?)(?:[-.:])?(?:\\s*))" - "((?:(?:\\balpha\\b)?|(?:\\bbeta\\b)?)\\s*[0-9]+([-._]*(?!esp|esm)[0-9a-zA-Z]+)*\\+?)" - ; - - // Matches isolated versions as last resort - const char* regex6 = - "((?:(?:\\balpha\\b)?|(?:\\bbeta\\b)?)\\s*\\b[0-9][-0-9a-zA-Z._]*\\+?)$" - ; - - const char* regex7 = - "(^\\bmark\\b\\s*\\b[IVX0-9][-0-9a-zA-Z._+]*\\s*(?:alpha|beta|test|debug)?\\s*(?:[0-9]*)?)$" - ; - - /// Array used to try each of the expressions defined above using - /// an iteration for each of them. - const vector version_checks({ - regex(regex1, regex::ECMAScript | regex::icase), - regex(regex2, regex::ECMAScript | regex::icase), - regex(regex3, regex::ECMAScript | regex::icase), - regex(regex4, regex::ECMAScript | regex::icase), - regex(regex5, regex::ECMAScript | regex::icase), //This incorrectly identifies "OBSE v19" where 19 is any integer. - //regex(regex6, regex::ECMAScript | regex::icase), //This is responsible for metallicow's false positive. - regex(regex7, regex::ECMAScript | regex::icase) - }); - // TODO: Remove the name-only constructor. Plugin::Plugin(const std::string& n) : PluginMetadata(n), @@ -131,17 +78,6 @@ namespace loot { //Also read Bash Tags applied and version string in description. string text = getDescription(); - BOOST_LOG_TRIVIAL(trace) << Name() << ": " << "Attempting to read the version from the description."; - for (size_t i = 0; i < version_checks.size(); ++i) { - smatch what; - if (regex_search(text, what, version_checks[i])) { - //Use the first sub-expression match. - version = string(what[1].first, what[1].second); - boost::trim(version); - BOOST_LOG_TRIVIAL(info) << Name() << ": " << "Extracted version \"" << version << "\" using regex " << i + 1; - break; - } - } BOOST_LOG_TRIVIAL(trace) << Name() << ": " << "Attempting to extract Bash Tags from the description."; size_t pos1 = text.find("{{BASH:"); if (pos1 != string::npos && pos1 + 7 != text.length()) { @@ -261,10 +197,6 @@ namespace loot { return game.IsPluginActive(Name()); } - std::string Plugin::Version() const { - return version; - } - uint32_t Plugin::Crc() const { return crc; } diff --git a/src/backend/plugin/plugin.h b/src/backend/plugin/plugin.h index 11b7ab4b..e673b793 100644 --- a/src/backend/plugin/plugin.h +++ b/src/backend/plugin/plugin.h @@ -44,12 +44,12 @@ namespace loot { Plugin(const std::string& name); Plugin(Game& game, const std::string& name, const bool headerOnly); + using libespm::Plugin::getDescription; using libespm::Plugin::getFormIds; using libespm::Plugin::getMasters; using libespm::Plugin::isMasterFile; bool IsEmpty() const; - std::string Version() const; uint32_t Crc() const; size_t NumOverrideFormIDs() const; diff --git a/src/gui/handler.cpp b/src/gui/handler.cpp index 29520284..99ffcc5b 100644 --- a/src/gui/handler.cpp +++ b/src/gui/handler.cpp @@ -32,6 +32,7 @@ #include "../backend/plugin_sorter.h" #include "../backend/helpers/helpers.h" #include "../backend/helpers/json.h" +#include "../backend/helpers/version.h" #include #include @@ -745,7 +746,7 @@ namespace loot { pluginNode["isMaster"] = plugin.isMasterFile(); pluginNode["loadsBSA"] = plugin.LoadsBSA(); pluginNode["crc"] = IntToHexString(plugin.Crc()); - pluginNode["version"] = plugin.Version(); + pluginNode["version"] = Version(plugin.getDescription()).AsString(); if (!mlistPlugin.HasNameOnly()) { // Now add the masterlist metadata to the pluginNode. diff --git a/src/tests/backend/game/test_game.h b/src/tests/backend/game/test_game.h index bc0146b5..e1daf1d6 100644 --- a/src/tests/backend/game/test_game.h +++ b/src/tests/backend/game/test_game.h @@ -288,7 +288,7 @@ TEST_F(Game, LoadPlugins) { libespm::FormId("Skyrim.esm", std::vector(), 0xCF9), }), plugin.getFormIds()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0x187BE342, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -310,7 +310,7 @@ TEST_F(Game, LoadPlugins) { libespm::FormId("Blank.esm", std::vector(), 0xCF9), }), plugin.getFormIds()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0x187BE342, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -331,7 +331,7 @@ TEST_F(Game, LoadPlugins) { libespm::FormId("Blank - Different.esm", std::vector(), 0xCF7), }), plugin.getFormIds()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0x64B9F757, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -353,7 +353,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0xB2D4119E, plugin.Crc()); EXPECT_EQ(4, plugin.NumOverrideFormIDs()); @@ -374,7 +374,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank - Different.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0xAADF6710, plugin.Crc()); EXPECT_EQ(4, plugin.NumOverrideFormIDs()); @@ -392,7 +392,7 @@ TEST_F(Game, LoadPlugins) { libespm::FormId("Blank.esp", std::vector(), 0xCF1), }), plugin.getFormIds()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("\xE2\x82\xAC\xC6\x92\xC5\xA0", plugin.getDescription()); EXPECT_EQ(0x24F0E2A1, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -409,7 +409,7 @@ TEST_F(Game, LoadPlugins) { libespm::FormId("Blank - Different.esp", std::vector(), 0xCEF), }), plugin.getFormIds()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0xD4C9B7AE, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -427,7 +427,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0x832152DC, plugin.Crc()); EXPECT_EQ(2, plugin.NumOverrideFormIDs()); @@ -444,7 +444,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank - Different.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0x3AD17683, plugin.Crc()); EXPECT_EQ(2, plugin.NumOverrideFormIDs()); @@ -460,7 +460,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank.esp" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0x28EF26DB, plugin.Crc()); EXPECT_EQ(1, plugin.NumOverrideFormIDs()); @@ -475,7 +475,7 @@ TEST_F(Game, LoadPlugins) { EXPECT_EQ(std::vector({ "Blank - Different.esp" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0xEB47BE63, plugin.Crc()); EXPECT_EQ(1, plugin.NumOverrideFormIDs()); } @@ -495,7 +495,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_TRUE(plugin.isMasterFile()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -506,7 +506,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_TRUE(plugin.isMasterFile()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -517,7 +517,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_TRUE(plugin.isMasterFile()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -530,7 +530,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -543,7 +543,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank - Different.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -554,7 +554,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_FALSE(plugin.isMasterFile()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("\xE2\x82\xAC\xC6\x92\xC5\xA0", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -565,7 +565,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_FALSE(plugin.isMasterFile()); EXPECT_TRUE(plugin.getFormIds().empty()); EXPECT_TRUE(plugin.getMasters().empty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -578,7 +578,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -591,7 +591,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank - Different.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -604,7 +604,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank.esp" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); @@ -617,7 +617,7 @@ TEST_F(Game, LoadPlugins_HeadersOnly) { EXPECT_EQ(std::vector({ "Blank - Different.esp" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); EXPECT_EQ(0, plugin.NumOverrideFormIDs()); } diff --git a/src/tests/backend/plugin/test_plugin.h b/src/tests/backend/plugin/test_plugin.h index f18b5f8e..45fac1ef 100644 --- a/src/tests/backend/plugin/test_plugin.h +++ b/src/tests/backend/plugin/test_plugin.h @@ -37,7 +37,7 @@ TEST_F(Plugin, ConstructorsAndDataAccess) { EXPECT_TRUE(plugin.getMasters().empty()); EXPECT_FALSE(plugin.isMasterFile()); EXPECT_TRUE(plugin.IsEmpty()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); loot::Game game(loot::Game::tes5); @@ -50,7 +50,7 @@ TEST_F(Plugin, ConstructorsAndDataAccess) { EXPECT_TRUE(plugin.getMasters().empty()); EXPECT_TRUE(plugin.isMasterFile()); EXPECT_FALSE(plugin.IsEmpty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0, plugin.Crc()); plugin = loot::Plugin(game, "Blank.esm", false); @@ -70,7 +70,7 @@ TEST_F(Plugin, ConstructorsAndDataAccess) { EXPECT_TRUE(plugin.getMasters().empty()); EXPECT_TRUE(plugin.isMasterFile()); EXPECT_FALSE(plugin.IsEmpty()); - EXPECT_EQ("5.0", plugin.Version()); + EXPECT_EQ("v5.0", plugin.getDescription()); EXPECT_EQ(0x187BE342, plugin.Crc()); plugin = loot::Plugin(game, "Blank - Master Dependent.esp", false); @@ -86,7 +86,7 @@ TEST_F(Plugin, ConstructorsAndDataAccess) { EXPECT_EQ(std::vector({ "Blank.esm" }), plugin.getMasters()); - EXPECT_EQ("", plugin.Version()); + EXPECT_EQ("", plugin.getDescription()); EXPECT_EQ(0x832152DC, plugin.Crc()); }