entity-manager: probe: Refactor matching
In a nutshell, rely on nlohmann's comparison operator to make the match
instead of rolling our own. A nice side effect is the expected match
behavior is well documented by nlohmann.
Code size and performance are unaffected (after removing dead code):
before: 25121752 bytes unstripped, 3202608 bytes stripped
after: 25146552 bytes unstripped, 3202616 bytes stripped
before: ~22 seconds for 5000 iterations of the tests
after: ~22 seconds for 5000 iterations of the tests
This refactor reduces the number of lines of code we need to maintain
and test and increases the number of lines of code used from the
extensively tested nlohmann json library.
The refactor simplifies the logic down to:
return probe == value;
for non-string probes. A string specialization is still required so we
can pattern match.
The refactor enables support for matching non-trivial json types such as
arrays and objects. New types need only be added to BasicVariantType.
There are several changed testcase expectations introduced by the
refactor. Each is discussed in detail below.
- stringRegexError
- boolStringError -> boolNeqString
- falseEmptyError -> falseNeqEmpty
- trueEmptyError -> trueNeqEmpty
- uintStringError -> uintNeqString
- uintEmptyError -> unitNeqEmpty
- intStringError -> intNeqString
- intEmptyError -> intNeqEmpty
- doubleStringError -> doubleNeqString
- doubleEmptyError -> doubleNeqEmpty
Configuration file validation of regular expression syntax or the
correct json type for a given dbus interface and property is better done
by our offline validation tools. Avoid runtime complexity attempting to
distinguish between user errors and probe statements that simply do not
match by treating them the same and always return "no-match" instead of
an invalid argument error for user error situations. stringRegexError
could be an exception - there is no additional runtime complexity
overhead associated with flagging it as a user error, but doing so would
result in inconsistent behavior, so return no-match for it as well.
- stringZeroEqFalse -> stringZeroNeqFalse
- stringOneEqTrue -> stringOneNeqTrue
- stringEqUnit8 -> stringNeqUint8
- stringEqUint32 -> stringNeqUint32
- stringEqInt32 -> stringNeqInt32
- stringRegexEqInt32 -> stringRegexNeqInt32
- stringEqDouble -> stringNeqDouble
Prior to this patch dbus properties are converted to strings in the
event of a string probe. This works around mistakes in configuration
files where the wrong json type is used but it can provide unexpected
results like the probe "1" matching a true and the probe "11" matching a
false, and the string "1*2" matching the number 112. Implementing the
expected behavior is possible but would add complexity. For these
reasons this refactor drops the string conversion and requires the user
to use the correct types in their configurations.
- falseEqUint32Zero -> falseNeqUint32Zero
- trueEqUint32Zero -> falseNeqUint32Zero
- trueEqDoubleOne -> trueNeqDoubleOne
- falseEqDoubleZero -> falseNeqDoubleZero
- uintEqTrue -> uintNeqTrue
- uintEqFalse -> uintNeqFalse
- unitEqDoubleRound -> uintNeqDouble
Prior to this patch dbus properties are coerced to unsigned integers in
the event of a boolean or unsigned integer probes. Like in the previous
section this can have unexpected results such as integral json types
matching floats with fractional components from dbus. Again,
implementing the expected behavior is possible but would add
coomplexity, and again these conversions are abandoned and instead
require the user to use the correct types in their configurations.
- intEqDoubleRound -> intNeqDoubleRoound
- doubleEqFalse -> doubleNeqFalse
- doubleEqTrue -> doubleNeqTrue
Like the previous two sections, dbus properties are coerced to integers
in the event of an integer probe, and dbus properties are coerced to
doubles in the event of a double probe, and can result in unexpected
behavior. Require the user to use the correct types in their
configurations.
Change-Id: I6ce7a9c4a80d9612b15cd5e7bb4647bd96852e48
Signed-off-by: Brad Bishop <bradleyb@fuzziesquirrel.com>
diff --git a/src/Utils.cpp b/src/Utils.cpp
index 27f1c15..d9ab4ed 100644
--- a/src/Utils.cpp
+++ b/src/Utils.cpp
@@ -392,8 +392,78 @@
return ret;
}
+/// \brief JSON/DBus matching Callable for std::variant (visitor)
+///
+/// Default match JSON/DBus match implementation
+/// \tparam T The concrete DBus value type from BasicVariantType
+template <typename T>
+struct MatchProbe
+{
+ /// \param probe the probe statement to match against
+ /// \param value the property value being matched to a probe
+ /// \return true if the dbusValue matched the probe otherwise false
+ static bool match(const nlohmann::json& probe, const T& value)
+ {
+ return probe == value;
+ }
+};
+
+/// \brief JSON/DBus matching Callable for std::variant (visitor)
+///
+/// std::string specialization of MatchProbe enabling regex matching
+template <>
+struct MatchProbe<std::string>
+{
+ /// \param probe the probe statement to match against
+ /// \param value the string value being matched to a probe
+ /// \return true if the dbusValue matched the probe otherwise false
+ static bool match(const nlohmann::json& probe, const std::string& value)
+ {
+ if (probe.is_string())
+ {
+ try
+ {
+ std::regex search(probe);
+ std::smatch regMatch;
+ return std::regex_search(value, regMatch, search);
+ }
+ catch (const std::regex_error&)
+ {
+ std::cerr << "Syntax error in regular expression: " << probe
+ << " will never match";
+ }
+ }
+
+ // Skip calling nlohmann here, since it will never match a non-string
+ // to a std::string
+ return false;
+ }
+};
+
+/// \brief Forwarding JSON/DBus matching Callable for std::variant (visitor)
+///
+/// Forward calls to the correct template instantiation of MatchProbe
+struct MatchProbeForwarder
+{
+ explicit MatchProbeForwarder(const nlohmann::json& probe) : probeRef(probe)
+ {}
+ const nlohmann::json& probeRef;
+
+ template <typename T>
+ bool operator()(const T& dbusValue) const
+ {
+ return MatchProbe<T>::match(probeRef, dbusValue);
+ }
+};
+
bool matchProbe(const nlohmann::json& probe, const BasicVariantType& dbusValue)
{
+ return std::visit(MatchProbeForwarder(probe), dbusValue);
+}
+
+bool matchProbeOld(const nlohmann::json& probe,
+ const BasicVariantType& dbusValue)
+{
bool deviceMatches = true;
switch (probe.type())
diff --git a/test/test_entity-manager.cpp b/test/test_entity-manager.cpp
index a5786e5..9826c04 100644
--- a/test/test_entity-manager.cpp
+++ b/test/test_entity-manager.cpp
@@ -3,7 +3,6 @@
#include <boost/container/flat_map.hpp>
#include <nlohmann/json.hpp>
-#include <regex>
#include <string>
#include <variant>
@@ -253,21 +252,21 @@
{
nlohmann::json j = R"("foo[")"_json;
BasicVariantType v = "foobar"s;
- EXPECT_THROW(matchProbe(j, v), std::regex_error);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringZeroEqFalse)
+TEST(MatchProbe, stringZeroNeqFalse)
{
nlohmann::json j = R"("0")"_json;
BasicVariantType v = false;
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringOneEqTrue)
+TEST(MatchProbe, stringOneNeqTrue)
{
nlohmann::json j = R"("1")"_json;
BasicVariantType v = true;
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, stringElevenNeqTrue)
@@ -298,11 +297,11 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringEqUint8)
+TEST(MatchProbe, stringNeqUint8)
{
nlohmann::json j = R"("255")"_json;
BasicVariantType v = uint8_t(255);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, stringNeqUint8Overflow)
@@ -326,39 +325,25 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringEqUint32)
+TEST(MatchProbe, stringNeqUint32)
{
nlohmann::json j = R"("11")"_json;
BasicVariantType v = uint32_t(11);
- EXPECT_TRUE(matchProbe(j, v));
-}
-
-TEST(MatchProbe, stringNeqUint32)
-{
- nlohmann::json j = R"("12")"_json;
- BasicVariantType v = uint32_t(11);
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringEqInt32)
-{
- nlohmann::json j = R"("-11")"_json;
- BasicVariantType v = int32_t(-11);
- EXPECT_TRUE(matchProbe(j, v));
-}
-
TEST(MatchProbe, stringNeqInt32)
{
- nlohmann::json j = R"("-12")"_json;
+ nlohmann::json j = R"("-11")"_json;
BasicVariantType v = int32_t(-11);
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringRegexEqInt32)
+TEST(MatchProbe, stringRegexNeqInt32)
{
nlohmann::json j = R"("1*4")"_json;
BasicVariantType v = int32_t(124);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, stringNeqUint64)
@@ -368,16 +353,9 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, stringEqDouble)
-{
- nlohmann::json j = R"("123.4")"_json;
- BasicVariantType v = double(123.4);
- EXPECT_TRUE(matchProbe(j, v));
-}
-
TEST(MatchProbe, stringNeqDouble)
{
- nlohmann::json j = R"("-123.4")"_json;
+ nlohmann::json j = R"("123.4")"_json;
BasicVariantType v = double(123.4);
EXPECT_FALSE(matchProbe(j, v));
}
@@ -389,11 +367,11 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, boolStringError)
+TEST(MatchProbe, boolNeqString)
{
nlohmann::json j = R"(false)"_json;
BasicVariantType v = "false"s;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, trueEqTrue)
@@ -438,11 +416,11 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, falseEqUint32Zero)
+TEST(MatchProbe, falseNeqUint32Zero)
{
nlohmann::json j = R"(false)"_json;
BasicVariantType v = uint32_t(0);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, trueNeqDoubleNegativeOne)
@@ -452,11 +430,11 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, trueEqDoubleOne)
+TEST(MatchProbe, trueNeqDoubleOne)
{
nlohmann::json j = R"(true)"_json;
BasicVariantType v = double(1.0);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, falseNeqDoubleOne)
@@ -466,55 +444,48 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, falseEqDoubleZero)
+TEST(MatchProbe, falseNeqDoubleZero)
{
nlohmann::json j = R"(false)"_json;
BasicVariantType v = double(0.0);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, falseEmptyError)
+TEST(MatchProbe, falseNeqEmpty)
{
nlohmann::json j = R"(false)"_json;
BasicVariantType v;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, trueEmptyError)
+TEST(MatchProbe, trueNeqEmpty)
{
nlohmann::json j = R"(true)"_json;
BasicVariantType v;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, uintStringError)
+TEST(MatchProbe, uintNeqString)
{
nlohmann::json j = R"(11)"_json;
BasicVariantType v = "11"s;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
-}
-
-TEST(MatchProbe, uintEqTrue)
-{
- nlohmann::json j = R"(1)"_json;
- BasicVariantType v = true;
- EXPECT_TRUE(matchProbe(j, v));
-}
-
-TEST(MatchProbe, uintEqFalse)
-{
- nlohmann::json j = R"(0)"_json;
- BasicVariantType v = false;
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, uintNeqTrue)
{
- nlohmann::json j = R"(11)"_json;
+ nlohmann::json j = R"(1)"_json;
BasicVariantType v = true;
EXPECT_FALSE(matchProbe(j, v));
}
+TEST(MatchProbe, uintNeqFalse)
+{
+ nlohmann::json j = R"(0)"_json;
+ BasicVariantType v = false;
+ EXPECT_FALSE(matchProbe(j, v));
+}
+
TEST(MatchProbe, uintEqUint8)
{
nlohmann::json j = R"(11)"_json;
@@ -550,25 +521,25 @@
EXPECT_TRUE(matchProbe(j, v));
}
-TEST(MatchProbe, uintEqDoubleRound)
+TEST(MatchProbe, uintNeqDouble)
{
nlohmann::json j = R"(11)"_json;
BasicVariantType v = double(11.7);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, uintEmptyError)
+TEST(MatchProbe, uintNeqEmpty)
{
nlohmann::json j = R"(11)"_json;
BasicVariantType v;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, intStringError)
+TEST(MatchProbe, intNeqString)
{
nlohmann::json j = R"(-11)"_json;
BasicVariantType v = "-11"s;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, intNeqTrue)
@@ -606,46 +577,39 @@
EXPECT_TRUE(matchProbe(j, v));
}
-TEST(MatchProbe, intEqDoubleRound)
+TEST(MatchProbe, intNeqDoubleRound)
{
nlohmann::json j = R"(-11)"_json;
BasicVariantType v = double(-11.7);
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, intEmptyError)
+TEST(MatchProbe, intNeqEmpty)
{
nlohmann::json j = R"(-11)"_json;
BasicVariantType v;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, doubleStringError)
+TEST(MatchProbe, doubleNeqString)
{
nlohmann::json j = R"(0.0)"_json;
BasicVariantType v = "0.0"s;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, doubleEqFalse)
+TEST(MatchProbe, doubleNeqFalse)
{
nlohmann::json j = R"(0.0)"_json;
BasicVariantType v = false;
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, doubleNeqTrue)
{
- nlohmann::json j = R"(1.1)"_json;
- BasicVariantType v = true;
- EXPECT_FALSE(matchProbe(j, v));
-}
-
-TEST(MatchProbe, doubleEqTrue)
-{
nlohmann::json j = R"(1.0)"_json;
BasicVariantType v = true;
- EXPECT_TRUE(matchProbe(j, v));
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, doubleEqInt32)
@@ -682,11 +646,11 @@
EXPECT_FALSE(matchProbe(j, v));
}
-TEST(MatchProbe, doubleEmptyError)
+TEST(MatchProbe, doubleNeqEmpty)
{
nlohmann::json j = R"(-11.0)"_json;
BasicVariantType v;
- EXPECT_THROW(matchProbe(j, v), std::invalid_argument);
+ EXPECT_FALSE(matchProbe(j, v));
}
TEST(MatchProbe, arrayNeqString)