From: Tomek Mrugalski Date: Mon, 3 Apr 2017 12:15:01 +0000 (+0200) Subject: [5132] Changes after review (comment #10): X-Git-Tag: trac5196_base~12^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a7eaf2f64fd23dd6626f80ab7afa519d1c69427;p=thirdparty%2Fkea.git [5132] Changes after review (comment #10): - Missing break added - LAST_IDENTIFIER_TYPE updated - auto now includes flex-id - HostTest::getIdentifier unit-test updated - bool_expr | string_expr removed - unit-tests now check that bool expressions are rejected for strings --- diff --git a/src/lib/dhcpsrv/host.cc b/src/lib/dhcpsrv/host.cc index 32b5e39bcd..702d37d9b8 100644 --- a/src/lib/dhcpsrv/host.cc +++ b/src/lib/dhcpsrv/host.cc @@ -207,6 +207,7 @@ Host::getIdentifierAsText(const IdentifierType& type, const uint8_t* value, break; case IDENT_FLEX: s << "flex-id"; + break; default: // This should never happen actually, unless we add new identifier // and forget to add a case for it above. diff --git a/src/lib/dhcpsrv/host.h b/src/lib/dhcpsrv/host.h index a89242a294..c3facde0e7 100644 --- a/src/lib/dhcpsrv/host.h +++ b/src/lib/dhcpsrv/host.h @@ -190,7 +190,7 @@ public: /// @brief Constant pointing to the last identifier of the /// @ref IdentifierType enumeration. - static const IdentifierType LAST_IDENTIFIER_TYPE = IDENT_CLIENT_ID; + static const IdentifierType LAST_IDENTIFIER_TYPE = IDENT_FLEX; /// @brief Constructor. /// diff --git a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc index f29ecd4173..12c6dbfe0b 100644 --- a/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_reservation_parser_unittest.cc @@ -1311,13 +1311,14 @@ TEST_F(HostReservationIdsParserTest, dhcp4AutoIdentifiers) { ConstCfgHostOperationsPtr cfg = CfgMgr::instance().getStagingCfg()-> getCfgHostOperations4(); const CfgHostOperations::IdentifierTypes& ids = cfg->getIdentifierTypes(); - ASSERT_EQ(4, ids.size()); + ASSERT_EQ(5, ids.size()); CfgHostOperations::IdentifierTypes::const_iterator id = ids.begin(); EXPECT_EQ(*id++, Host::IDENT_HWADDR); EXPECT_EQ(*id++, Host::IDENT_DUID); EXPECT_EQ(*id++, Host::IDENT_CIRCUIT_ID); EXPECT_EQ(*id++, Host::IDENT_CLIENT_ID); + EXPECT_EQ(*id++, Host::IDENT_FLEX); } // This test verifies that use of "auto" together with an explicit @@ -1354,11 +1355,12 @@ TEST_F(HostReservationIdsParserTest, dhcp6AutoIdentifiers) { ConstCfgHostOperationsPtr cfg = CfgMgr::instance().getStagingCfg()-> getCfgHostOperations6(); const CfgHostOperations::IdentifierTypes& ids = cfg->getIdentifierTypes(); - ASSERT_EQ(2, ids.size()); + ASSERT_EQ(3, ids.size()); CfgHostOperations::IdentifierTypes::const_iterator id = ids.begin(); EXPECT_EQ(*id++, Host::IDENT_HWADDR); EXPECT_EQ(*id++, Host::IDENT_DUID); + EXPECT_EQ(*id++, Host::IDENT_FLEX); } // This test verifies that use of "auto" together with an explicit diff --git a/src/lib/dhcpsrv/tests/host_unittest.cc b/src/lib/dhcpsrv/tests/host_unittest.cc index 53caef855b..2964f888cd 100644 --- a/src/lib/dhcpsrv/tests/host_unittest.cc +++ b/src/lib/dhcpsrv/tests/host_unittest.cc @@ -182,8 +182,9 @@ TEST_F(HostTest, getIdentifier) { EXPECT_EQ(Host::IDENT_DUID, Host::getIdentifierType("duid")); EXPECT_EQ(Host::IDENT_CIRCUIT_ID, Host::getIdentifierType("circuit-id")); EXPECT_EQ(Host::IDENT_CLIENT_ID, Host::getIdentifierType("client-id")); + EXPECT_EQ(Host::IDENT_FLEX, Host::getIdentifierType("flex-id")); - EXPECT_THROW(Host::getIdentifierType("unuspported"), isc::BadValue); + EXPECT_THROW(Host::getIdentifierType("unusupported"), isc::BadValue); } // This test verifies that it is possible to create a Host object diff --git a/src/lib/eval/parser.yy b/src/lib/eval/parser.yy index 9d31b01a67..26b58424b1 100644 --- a/src/lib/eval/parser.yy +++ b/src/lib/eval/parser.yy @@ -115,13 +115,7 @@ using namespace isc::eval; // ... that expects either TOPLEVEL_BOOL or TOPLEVEL_STRING. Depending on which // token appears first, it will determine what is allowed and what it not. start: TOPLEVEL_BOOL expression - | TOPLEVEL_STRING string_expression -; - -// string expression can be either a string (proper) or boolean (that is internally -// stored as "true" or "false") -string_expression: bool_expr - | string_expr + | TOPLEVEL_STRING string_expr ; // Expression can either be a single token or a (something == something) expression diff --git a/src/lib/eval/tests/evaluate_unittest.cc b/src/lib/eval/tests/evaluate_unittest.cc index b8a69d8180..37ade2798d 100644 --- a/src/lib/eval/tests/evaluate_unittest.cc +++ b/src/lib/eval/tests/evaluate_unittest.cc @@ -366,10 +366,11 @@ public: /// @param expr expression to be evaluated template void testExpressionNegative(const std::string& expr, - const Option::Universe& u = Option::V4) { + const Option::Universe& u = Option::V4, + EvalContext::ParserType type = EvalContext::PARSER_BOOL) { EvalContext eval(u); - EXPECT_THROW(eval.parseString(expr), ex) << "while parsing expression " + EXPECT_THROW(eval.parseString(expr, type), ex) << "while parsing expression " << expr; } }; @@ -477,10 +478,18 @@ TEST_F(ExpressionsTest, invalidIntegers) { // Tests whether expressions can be evaluated to a string. TEST_F(ExpressionsTest, evaluateString) { + // Check that content of the options is returned properly. testExpressionString(Option::V4, "option[100].hex", "hundred4"); testExpressionString(Option::V6, "option[100].hex", "hundred6"); + + // Check that content of non-existing option returns empty string. testExpressionString(Option::V4, "option[200].hex", ""); testExpressionString(Option::V6, "option[200].hex", ""); + + testExpressionNegative("pkt4.msgtype == 1", Option::V4, + EvalContext::PARSER_STRING); + testExpressionNegative("pkt6.msgtype == 1", Option::V6, + EvalContext::PARSER_STRING); } };