[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

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.

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

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

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

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
+     | TOPLEVEL_STRING string_expr
-;
-
-// string expression can be either a string (proper) or boolean (that is internally
-// stored as "true" or "false")
-string_expression: bool_expr
-      | string_expr
 ;
 
 // Expression can either be a single token or a (something == something) expression

src/lib/eval/tests/evaluate_unittest.cc

@@ -366,10 +366,11 @@ public:
     /// @param expr expression to be evaluated
     template<typename ex>
     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<EvalParseError>("pkt4.msgtype == 1", Option::V4,
+                                           EvalContext::PARSER_STRING);
+    testExpressionNegative<EvalParseError>("pkt6.msgtype == 1", Option::V6,
+                                           EvalContext::PARSER_STRING);
 }
 
 };