From: Francis Dupont Date: Fri, 20 Jan 2017 13:52:54 +0000 (+0100) Subject: [5096] Use DhcpConfigError only, add locations and a DB backend comment X-Git-Tag: trac5119_base~4^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e74dccaf1149841a460cd9b92948598d266d06e5;p=thirdparty%2Fkea.git [5096] Use DhcpConfigError only, add locations and a DB backend comment --- diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 06162bdf1e..5d5f44ef0b 100644 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/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) { diff --git a/src/lib/dhcpsrv/parsers/dbaccess_parser.cc b/src/lib/dhcpsrv/parsers/dbaccess_parser.cc index 18769f9c10..dd51a9c3eb 100644 --- a/src/lib/dhcpsrv/parsers/dbaccess_parser.cc +++ b/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::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::max()); + << std::numeric_limits::max() + << " (" << value->getPosition() << ")"); } // d. Check that the timeout is within a reasonable range. if ((timeout < 0) || (timeout > std::numeric_limits::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::max()); + << std::numeric_limits::max() + << " (" << value->getPosition() << ")"); } // 4. If all is OK, update the stored keyword/value pairs. We do this by diff --git a/src/lib/dhcpsrv/parsers/dbaccess_parser.h b/src/lib/dhcpsrv/parsers/dbaccess_parser.h index ed894d32d3..8cb716d7d2 100644 --- a/src/lib/dhcpsrv/parsers/dbaccess_parser.h +++ b/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 diff --git a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc index 674b78cc39..5fa1081dcc 100644 --- a/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc +++ b/src/lib/dhcpsrv/tests/dbaccess_parser_unittest.cc @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -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); } diff --git a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc index b5433ab332..4502bb23c7 100644 --- a/src/lib/dhcpsrv/tests/mysql_lease_mgr_unittest.cc +++ b/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()); diff --git a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc index 0a629d5579..d5627cdb80 100644 --- a/src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc +++ b/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());