[5096] Use DhcpConfigError only, add locations and a DB backend comment

src/lib/dhcpsrv/mysql_connection.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -28,7 +28,7 @@ const my_bool MLM_TRUE = 1;
 const int MLM_MYSQL_FETCH_SUCCESS = 0;
 const int MLM_MYSQL_FETCH_FAILURE = 1;
 
-const int MYSQL_DEFAULT_CONNECTION_TIMEOUT = 5;	// seconds
+const int MYSQL_DEFAULT_CONNECTION_TIMEOUT = 5; // seconds
 
 MySqlTransaction::MySqlTransaction(MySqlConnection& conn)
     : conn_(conn), committed_(false) {

src/lib/dhcpsrv/parsers/dbaccess_parser.cc

@@ -77,7 +77,7 @@ DbAccessParser::parse(CfgDbAccessPtr& cfg_db,
             }
         } catch (const isc::data::TypeError& ex) {
             // Append position of the element.
-            isc_throw(BadValue, "invalid value type specified for "
+            isc_throw(DhcpConfigError, "invalid value type specified for "
                       "parameter '" << param.first << "' ("
                       << param.second->getPosition() << ")");
         }
@@ -88,36 +88,44 @@ DbAccessParser::parse(CfgDbAccessPtr& cfg_db,
     // a. Check if the "type" keyword exists and thrown an exception if not.
     StringPairMap::const_iterator type_ptr = values_copy.find("type");
     if (type_ptr == values_copy.end()) {
-        isc_throw(TypeKeywordMissing, (type_ == LEASE_DB ? "lease" : "host")
+        isc_throw(DhcpConfigError, (type_ == LEASE_DB ? "lease" : "host")
                   << " database access parameters must "
                   "include the keyword 'type' to determine type of database "
                   "to be accessed (" << database_config->getPosition() << ")");
     }
 
     // b. Check if the 'type' keyword known and throw an exception if not.
+    //
+    // Please note when you add a new database backend you have to add
+    // the new type here and in server grammars.
     string dbtype = type_ptr->second;
     if ((dbtype != "memfile") &&
         (dbtype != "mysql") &&
         (dbtype != "postgresql") &&
         (dbtype != "cql")) {
-        isc_throw(BadValue, "unknown backend database type: " << dbtype
-                  << " (" << database_config->getPosition() << ")");
+        ConstElementPtr value = database_config->get("type");
+        isc_throw(DhcpConfigError, "unknown backend database type: " << dbtype
+                  << " (" << value->getPosition() << ")");
     }
 
     // c. Check that the lfc-interval is within a reasonable range.
     if ((lfc_interval < 0) ||
         (lfc_interval > std::numeric_limits<uint32_t>::max())) {
-        isc_throw(BadValue, "lfc-interval value: " << lfc_interval
+        ConstElementPtr value = database_config->get("lfc-interval");
+        isc_throw(DhcpConfigError, "lfc-interval value: " << lfc_interval
                   << " is out of range, expected value: 0.."
-                  << std::numeric_limits<uint32_t>::max());
+                  << std::numeric_limits<uint32_t>::max()
+                  << " (" << value->getPosition() << ")");
     }
 
     // d. Check that the timeout is within a reasonable range.
     if ((timeout < 0) ||
         (timeout > std::numeric_limits<uint32_t>::max())) {
-        isc_throw(BadValue, "timeout value: " << timeout
+        ConstElementPtr value = database_config->get("connect-timeout");
+        isc_throw(DhcpConfigError, "connect-timeout value: " << timeout
                   << " is out of range, expected value: 0.."
-                  << std::numeric_limits<uint32_t>::max());
+                  << std::numeric_limits<uint32_t>::max()
+                  << " (" << value->getPosition() << ")");
     }
 
     // 4. If all is OK, update the stored keyword/value pairs.  We do this by

src/lib/dhcpsrv/parsers/dbaccess_parser.h

@@ -17,16 +17,6 @@
 namespace isc {
 namespace dhcp {
 
-/// @brief Exception thrown when 'type' keyword is missing from string
-///
-/// This condition is checked, but should never occur because 'type' is marked
-/// as mandatory in the .spec file for the server.
-class TypeKeywordMissing : public isc::Exception {
-public:
-    TypeKeywordMissing(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
-};
-
 /// @brief Parse Lease Database Parameters
 ///
 /// This class is the parser for the lease database configuration.  This is a

src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc

@@ -8,6 +8,7 @@
 
 #include <cc/command_interpreter.h>
 #include <dhcpsrv/lease_mgr_factory.h>
+#include <dhcpsrv/parsers/dhcp_config_parser.h>
 #include <dhcpsrv/parsers/dbaccess_parser.h>
 #include <dhcpsrv/testutils/mysql_schema.h>
 #include <dhcpsrv/host_mgr.h>
@@ -332,7 +333,7 @@ TEST_F(DbAccessParserTest, negativeLFCInterval) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 // This test checks that the parser rejects the too large (greater than
@@ -348,7 +349,7 @@ TEST_F(DbAccessParserTest, largeLFCInterval) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 // This test checks that the parser accepts the valid value of the
@@ -382,7 +383,7 @@ TEST_F(DbAccessParserTest, negativeTimeout) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 // This test checks that the parser rejects a too large (greater than
@@ -398,7 +399,7 @@ TEST_F(DbAccessParserTest, largeTimeout) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 // Check that the parser works with a valid MySQL configuration
@@ -432,7 +433,7 @@ TEST_F(DbAccessParserTest, missingTypeKeyword) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), TypeKeywordMissing);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 // Check reconfiguration.  Checks that incremental changes applied to the
@@ -517,7 +518,7 @@ TEST_F(DbAccessParserTest, incrementalChanges) {
     json_elements = Element::fromJSON(json_config);
     EXPECT_TRUE(json_elements);
 
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
     checkAccessString("Incompatible incremental change", parser.getDbAccessParameters(),
                       config3);
 
@@ -592,7 +593,7 @@ TEST_F(DbAccessParserTest, invalidReadOnly) {
     EXPECT_TRUE(json_elements);
 
     TestDbAccessParser parser(DbAccessParser::LEASE_DB);
-    EXPECT_THROW(parser.parse(json_elements), BadValue);
+    EXPECT_THROW(parser.parse(json_elements), DhcpConfigError);
 }
 
 

src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -116,7 +116,7 @@ TEST(MySqlOpenTest, OpenDatabase) {
     // Check that lease manager open the database opens correctly with a longer
     // timeout.  If it fails, print the error message.
     try {
-	string connection_string = validMySQLConnectionString() + string(" ") +
+        string connection_string = validMySQLConnectionString() + string(" ") +
                                    string(VALID_TIMEOUT);
         LeaseMgrFactory::create(connection_string);
         EXPECT_NO_THROW((void) LeaseMgrFactory::instance());

src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -108,7 +108,7 @@ TEST(PgSqlOpenTest, OpenDatabase) {
     // Check that lease manager open the database opens correctly with a longer
     // timeout.  If it fails, print the error message.
     try {
-	string connection_string = validPgSQLConnectionString() + string(" ") +
+        string connection_string = validPgSQLConnectionString() + string(" ") +
                                    string(VALID_TIMEOUT);
         LeaseMgrFactory::create(connection_string);
         EXPECT_NO_THROW((void) LeaseMgrFactory::instance());