Merge remote-tracking branch 'isc-kea/master' into cassandra_improvem…

…ents

configure.ac

@@ -1623,9 +1623,9 @@ if test "$CQL_CPPFLAGS" != "" ; then
 cat >> config.report << END
 
 Cassandra CQL:
-  CQL_VERSION:                  ${CQL_VERSION}
-  CQL_CPPFLAGS:                 ${CQL_CPPFLAGS}
-  CQL_LIBS:                     ${CQL_LIBS}
+  CQL_VERSION:     ${CQL_VERSION}
+  CQL_CPPFLAGS:    ${CQL_CPPFLAGS}
+  CQL_LIBS:        ${CQL_LIBS}
 END
 else
 cat >> config.report << END

src/lib/dhcpsrv/cql_host_data_source.cc

@@ -1339,7 +1339,7 @@ class CqlHostDataSourceImpl {
     /// See @ref CqlHostDataSource::add() for parameter details.
     ///
     /// @param host host to be added or deleted
-    /// @param insert insert or delete the host
+    /// @param insert insert (true) or delete (false) the host
     virtual bool insertOrDelete(const HostPtr& host, bool insert);
 
     /// @brief Implementation of @ref CqlHostDataSource::get4()
@@ -1974,7 +1974,8 @@ CqlHostDataSourceImpl::insertOrDeleteHostWithOptions(bool insert,
                 }
                 option_found = true;
                 /// @todo: Assign actual value to subnet id.
-                result = insertOrDeleteHost(insert, host, OptionalValue<SubnetID>(), reservation, space, option);
+                result = insertOrDeleteHost(insert, host, OptionalValue<SubnetID>(), reservation,
+                                            space, option);
             }
         }
     }
@@ -2082,17 +2083,14 @@ CqlHostDataSourceImpl::insertOrDeleteHost(bool insert,
 
     try {
         if (insert) {
-            host_exchange->createBindForMutation(
-                host, subnet_id, reservation, option_space, option_descriptor,
-                CqlHostExchange::INSERT_HOST, assigned_values);
+            host_exchange->createBindForMutation(host, subnet_id, reservation, option_space,
+                option_descriptor, CqlHostExchange::INSERT_HOST, assigned_values);
 
 
             host_exchange->executeMutation(dbconn_, assigned_values, CqlHostExchange::INSERT_HOST);
         } else {
-            host_exchange->createBindForDelete(
-                host, subnet_id, reservation, option_space, option_descriptor,
-                CqlHostExchange::DELETE_HOST, assigned_values);
-
+            host_exchange->createBindForDelete(host, subnet_id, reservation, option_space,
+                option_descriptor, CqlHostExchange::DELETE_HOST, assigned_values);
 
             host_exchange->executeMutation(dbconn_, assigned_values, CqlHostExchange::DELETE_HOST);
         }
@@ -2155,17 +2153,17 @@ CqlHostDataSource::del(const SubnetID& subnet_id, const asiolink::IOAddress& add
 bool
 CqlHostDataSource::del4(const SubnetID& subnet_id, const Host::IdentifierType& identifier_type,
                         const uint8_t* identifier_begin, const size_t identifier_len) {
-    HostPtr host = boost::const_pointer_cast<Host>(impl_->get4(
-            subnet_id, identifier_type, identifier_begin, identifier_len));
+    HostPtr host = boost::const_pointer_cast<Host>(impl_->get4(subnet_id, identifier_type,
+                                                               identifier_begin, identifier_len));
 
     return (impl_->insertOrDelete(host, false));
 }
 
 bool
 CqlHostDataSource::del6(const SubnetID& subnet_id, const Host::IdentifierType& identifier_type,
                         const uint8_t* identifier_begin, const size_t identifier_len) {
-    HostPtr host = boost::const_pointer_cast<Host>(impl_->get6(
-            subnet_id, identifier_type, identifier_begin, identifier_len));
+    HostPtr host = boost::const_pointer_cast<Host>(impl_->get6(subnet_id, identifier_type,
+                                                               identifier_begin, identifier_len));
 
     return (impl_->insertOrDelete(host, false));
 }

src/lib/dhcpsrv/cql_host_data_source.h

@@ -314,7 +314,9 @@ class CqlHostDataSource : public BaseHostDataSource {
 
     /// @brief Returns a collection of all the hosts.
     ///
-    /// This method may return multiple @ref Host objects.
+    /// This method may return multiple @ref Host objects. It is only used in
+    /// testing. It may be very slow when running on a production server with
+    /// lots of hosts.
     ///
     /// @return Collection of const @ref Host objects.
     virtual ConstHostCollection

src/lib/dhcpsrv/tests/cql_host_data_source_unittest.cc

@@ -281,6 +281,7 @@ TEST(CqlConnection, checkTimeConversion) {
 }
 
 // This test verifies that database backend can operate in Read-Only mode.
+// We currently don't test Cassandra in read-only mode.
 TEST_F(CqlHostDataSourceTest, DISABLED_testReadOnlyDatabase) {
     testReadOnlyDatabase(CQL_VALID_TYPE);
 }

src/lib/dhcpsrv/tests/cql_lease_mgr_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2012-2018 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2016-2018 Internet Systems Consortium, Inc. ("ISC")
 // Copyright (C) 2015-2017 Deutsche Telekom AG.
 //
 // Authors: Razvan Becheriu <razvan.becheriu@qualitance.com>

src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc

@@ -1970,6 +1970,7 @@ GenericLeaseMgrTest::testDeleteExpiredReclaimedLeases4() {
             EXPECT_FALSE(lease) << "The following lease should have been"
                 " deleted: " << leases[i]->toText();
             ++should_delete_num;
+
         } else {
             // If the lease is not reclaimed or it has expired less than
             // 15 seconds ago, the lease should still be there.

src/lib/dhcpsrv/tests/pgsql_host_data_source_unittest.cc

@@ -225,6 +225,36 @@ TEST(PgSqlHostDataSource, OpenDatabase) {
     destroyPgSQLSchema();
 }
 
+/// @brief Flag used to detect calls to db_lost_callback function
+bool callback_called = false;
+
+/// @brief Callback function used in open database testing
+bool db_lost_callback(ReconnectCtlPtr /* db_conn_retry */) {
+    return (callback_called = true);
+}
+
+/// @brief Make sure open failures do NOT invoke db lost callback
+/// The db lost callback should only be invoked after succesfully
+/// opening the DB and then subsequently losing it. Failing to
+/// open should be handled directly by the application layer.
+/// There is simply no good way to break the connection in a
+/// unit test environment.  So testing the callback invocation
+/// in a unit test is next to impossible. That has to be done
+/// as a system test.
+TEST(PgSqlHostDataSource, NoCallbackOnOpenFail) {
+    // Schema needs to be created for the test to work.
+    destroyPgSQLSchema();
+    createPgSQLSchema();
+
+    callback_called = false;
+    EXPECT_THROW(HostDataSourceFactory::create(connectionString(
+        PGSQL_VALID_TYPE, VALID_NAME, INVALID_HOST, VALID_USER, VALID_PASSWORD),
+        db_lost_callback), DbOpenError);
+
+    EXPECT_FALSE(callback_called);
+    destroyPgSQLSchema();
+}
+
 // This test verifies that database backend can operate in Read-Only mode.
 TEST_F(PgSqlHostDataSourceTest, testReadOnlyDatabase) {
     testReadOnlyDatabase(PGSQL_VALID_TYPE);

src/lib/dhcpsrv/tests/pgsql_lease_mgr_unittest.cc

@@ -473,12 +473,12 @@ TEST_F(PgSqlLeaseMgrTest, nullDuid) {
     testNullDuid();
 }
 
-/// @brief Tests whether memfile can store and retrieve hardware addresses
+/// @brief Tests whether Postgres can store and retrieve hardware addresses
 TEST_F(PgSqlLeaseMgrTest, testLease6Mac) {
     testLease6MAC();
 }
 
-/// @brief Tests whether memfile can store and retrieve hardware addresses
+/// @brief Tests whether Postgres can store and retrieve hardware addresses
 TEST_F(PgSqlLeaseMgrTest, testLease6HWTypeAndSource) {
     testLease6HWTypeAndSource();
 }