[#2736] Addressed review comments

Changes to be committed:
	modified:   ../../../../ChangeLog
	modified:   ../../../../doc/sphinx/arm/classify.rst
	modified:   ../parsers/client_class_def_parser.cc
	modified:   client_class_def_parser_unittest.cc

Author: tmarkwalder

ChangeLog

@@ -1,9 +1,9 @@
 2304.	[func]		tmark
-	Both kea-dhcp4 and kea-dhcp6 will now emit a warning
-	log message when classes are configured with both
-	``only-in-additional-list`` true and parameter(s)
-	that normally impact lease lifetimes (e.g. 'valid-
-	lifetime', 'preferred-lifetime`).
+	Both kea-dhcp4 and kea-dhcp6 servers will now
+	log a warning message when they detect classes that
+	configure lease life time parameters (e.g. 'valid-lifetime',
+	'preferred-lifetime') while also setting 
+	'only-in-addditiional-list' to true.
 	(Gitlab #2736)
 
 2303.	[bug]		tmark

doc/sphinx/arm/classify.rst

@@ -1170,8 +1170,8 @@ Class Priority
 ==============
 
 Client classes in Kea follow the order in which they are specified in the
-configuration (vs. alphabetical order). Additional classes are ordered by 
-pool, subnet, and then shared-network and within each scope by the order in 
+configuration (vs. alphabetical order). Additional classes are ordered by
+pool, subnet, and then shared-network and within each scope by the order in
 which they appear in ``evaluate-additional-classes``.
 
 When determining which client-class information (comprised of
@@ -1189,7 +1189,7 @@ reservation.
 
 On the other hand, lease lifetimes and DHCPv4 field values defined at class
 scope override any values defined globally, in a subnet scope, or in a
-shared-network scope.  
+shared-network scope.
 
 .. note::
    Because additional evaluation occurs after lease assignment, parameters

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -297,7 +297,7 @@ ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
          !preferred_lft.unspecified() ||
          !offer_lft.unspecified())) {
         LOG_WARN(dhcpsrv_logger, DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES)
-                 .arg(name);
+            .arg(name);
     }
 
     // Add the client class definition

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -103,7 +103,6 @@ class ExpressionParserTest : public ::testing::Test {
 };
 
 /// @brief Test fixture class for @c ClientClassDefParser.
-//class ClientClassDefParserTest : public ::testing::Test {
 class ClientClassDefParserTest : public LogContentTest {
 protected:
 
@@ -2261,11 +2260,12 @@ TEST_F(ClientClassDefParserTest, addtionalWithLifetimes4) {
         // If we expect the warning log to be emitted the occurrences
         // in the log file should bump by 1.
         if (scenario.should_log_) {
-            // Veriy we emitted another instance of the log message.
             ++exp_log_count;
-            ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"),
-                      exp_log_count);
         }
+
+        // Veriy we have the expected count of log messages.
+        ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"), 
+                            exp_log_count);
     }
 }
 
@@ -2335,11 +2335,12 @@ TEST_F(ClientClassDefParserTest, addtionalWithLifetimes6) {
         // If we expect the warning log to be emitted the occurrences
         // in the log file should bump by 1.
         if (scenario.should_log_) {
-            // Veriy we emitted another instance of the log message.
             ++exp_log_count;
-            ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"),
-                      exp_log_count);
         }
+
+        // Veriy we have the expected count of log messages.
+        ASSERT_EQ(countFile("DHCPSRV_CLASS_WITH_ADDTIONAL_AND_LIFETIMES"),
+                            exp_log_count);
     }
 }