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/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)