[4489] Addressed review comments.

The only review item not addressed with this commit is the
implementation of unit test that operates on the read only
database, i.e. the database containing tables on which the
given user only has SELECT privileges.

diff --git a/src/lib/dhcpsrv/database_connection.cc b/src/lib/dhcpsrv/database_connection.cc
index 983a752d96b60d25eb142c0b514e6a3f4711aadd..0aa886fa889f62a5da6942d7b78b5e0fdeb03028 100644 (file)
--- a/src/lib/dhcpsrv/database_connection.cc
+++ b/src/lib/dhcpsrv/database_connection.cc
@@ -1,10 +1,11 @@
-// Copyright (C) 2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2016 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
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 #include <dhcpsrv/database_connection.h>
+#include <dhcpsrv/db_exceptions.h>
 #include <dhcpsrv/dhcpsrv_log.h>
 #include <exceptions/exceptions.h>
 
@@ -86,5 +87,24 @@ DatabaseConnection::redactedAccessString(const ParameterMap& parameters) {
     return (access);
 }
 
+bool
+DatabaseConnection::configuredReadOnly() const {
+    std::string readonly_value = "false";
+    try {
+        readonly_value = getParameter("readonly");
+        boost::algorithm::to_lower(readonly_value);
+    } catch (...) {
+        // Parameter "readonly" hasn't been specified so we simply use
+        // the default value of "false".
+    }
+
+    if ((readonly_value != "false") && (readonly_value != "true")) {
+        isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value
+                  << "' specified for boolean parameter 'readonly'");
+    }
+
+    return (readonly_value == "true");
+}
+
 };
 };

diff --git a/src/lib/dhcpsrv/database_connection.h b/src/lib/dhcpsrv/database_connection.h
index c18c16f28cdc7b73615c462689ceaa80d720aff0..e6a902babff6dcb3380c932d54cf5e78dd97165e 100644 (file)
--- a/src/lib/dhcpsrv/database_connection.h
+++ b/src/lib/dhcpsrv/database_connection.h
@@ -121,6 +121,14 @@ public:
     /// @return Redacted database access string.
     static std::string redactedAccessString(const ParameterMap& parameters);
 
+    /// @brief Convenience method checking if database should be opened with
+    /// read only access.
+    ///
+    /// @return true if "readonly" parameter is specified and set to true;
+    /// false if "readonly" parameter is not specified or it is specified
+    /// and set to false.
+    bool configuredReadOnly() const;
+
 private:
 
     /// @brief List of parameters passed in dbconfig

diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes
index 7bf0ba22ad8f5432a67d7dbf1b0db4467fd7238f..bb8abaee1f95ff2aa13de3ad14134ba165686a3e 100644 (file)
--- a/src/lib/dhcpsrv/dhcpsrv_messages.mes
+++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes
@@ -551,7 +551,7 @@ connection including database name and username needed to access it
 A debug message issued when the server is about to obtain schema version
 information from the MySQL hosts database.
 
-% DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database operates in read-only mode
+% DHCPSRV_MYSQL_HOST_DB_READONLY MySQL host database opened for read access only
 This informational message is issued when the user has configured the PostgreSQL
 database in read-only mode. Kea will not be able to insert or modify
 host reservations but will be able to retrieve existing ones and
@@ -702,7 +702,7 @@ V6) is about to open a PostgreSQL hosts database.  The parameters of the
 connection including database name and username needed to access it
 (but not the password if any) are logged.
 
-% DHCPSRV_PGSQL_HOST_DB_READONLY PostgreSQL host database operates in read-only mode
+% DHCPSRV_PGSQL_HOST_DB_READONLY PostgreSQL host database opened for read access only
 This informational message is issued when the user has configured the PostgreSQL
 database in read-only mode. Kea will not be able to insert or modify
 host reservations but will be able to retrieve existing ones and

diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc
index b12bdd5e193d6f5b9121102c54d0813b3310b7e0..9569b4e8d3f1ac8021806ef72335a021be1a7094 100644 (file)
--- a/src/lib/dhcpsrv/mysql_connection.cc
+++ b/src/lib/dhcpsrv/mysql_connection.cc
@@ -214,12 +214,15 @@ MySqlConnection::prepareStatement(uint32_t index, const char* text) {
 
 void
 MySqlConnection::prepareStatements(const TaggedStatement* start_statement,
-                                   const TaggedStatement* end_statement,
-                                   size_t num_statements) {
-    // Allocate space for all statements
-    statements_.resize(num_statements, NULL);
+                                   const TaggedStatement* end_statement) {
+    size_t num_statements = std::distance(start_statement, end_statement);
+    if (num_statements == 0) {
+        return;
+    }
 
-    text_statements_.resize(num_statements, std::string(""));
+    // Expand vectors of allocated statements to hold additional statements.
+    statements_.resize(statements_.size() + num_statements, NULL);
+    text_statements_.resize(text_statements_.size() + num_statements, std::string(""));
 
     // Created the MySQL prepared statements for each DML statement.
     for (const TaggedStatement* tagged_statement = start_statement;

diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h
index 06fb34d4663bf3be2b192879145d87c3f3660d3e..89ad0ec59371bf4f9064596fee22d3a6d8fdc2d7 100644 (file)
--- a/src/lib/dhcpsrv/mysql_connection.h
+++ b/src/lib/dhcpsrv/mysql_connection.h
@@ -241,15 +241,13 @@ public:
     /// Creates the prepared statements for all of the SQL statements used
     /// by the MySQL backend.
     /// @param tagged_statements an array of statements to be compiled
-    /// @param num_statements number of statements in tagged_statements
     ///
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
     /// @throw isc::InvalidParameter 'index' is not valid for the vector.  This
     ///        represents an internal error within the code.
     void prepareStatements(const TaggedStatement* start_statement,
-                           const TaggedStatement* end_statement,
-                           size_t num_statements);
+                           const TaggedStatement* end_statement);
 
     /// @brief Clears prepared statements and text statements.
     void clearStatements();

diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc
index 96ef41f9f0a24e38f98196e33e589938f8fe8ef9..becad67abef62a97f6fd03ca88f578ba21bdd8da 100644 (file)
--- a/src/lib/dhcpsrv/mysql_host_data_source.cc
+++ b/src/lib/dhcpsrv/mysql_host_data_source.cc
@@ -849,7 +849,7 @@ private:
         size_t start_column_;
 
         /// @brief Option id.
-        uint64_t option_id_;
+        uint32_t option_id_;
 
         /// @brief Option code.
         uint16_t code_;
@@ -919,7 +919,7 @@ private:
         //@}
 
         /// @brief Option id for last processed row.
-        uint64_t most_recent_option_id_;
+        uint32_t most_recent_option_id_;
     };
 
     /// @brief Pointer to the @ref OptionProcessor class.
@@ -1116,7 +1116,7 @@ public:
     /// @brief Returns last fetched reservation id.
     ///
     /// @return Reservation id or 0 if no reservation data is fetched.
-    uint64_t getReservationId() const {
+    uint32_t getReservationId() const {
         if (reserv_type_null_ == MLM_FALSE) {
             return (reservation_id_);
         }
@@ -1254,7 +1254,7 @@ public:
 private:
 
     /// @brief IPv6 reservation id.
-    uint64_t reservation_id_;
+    uint32_t reservation_id_;
 
     /// @brief IPv6 reservation type.
     uint8_t reserv_type_;
@@ -1297,7 +1297,7 @@ private:
     //@}
 
     /// @brief Reservation id for last processed row.
-    uint64_t most_recent_reservation_id_;
+    uint32_t most_recent_reservation_id_;
 
 };
 
@@ -1633,7 +1633,10 @@ public:
 
     /// @brief Statement Tags
     ///
-    /// The contents of the enum are indexes into the list of SQL statements
+    /// The contents of the enum are indexes into the list of SQL statements.
+    /// It is assumed that the order is such that the indicies of statements
+    /// reading the database are less than those of statements modifying the
+    /// database.
     enum StatementIndex {
         GET_HOST_DHCPID,        // Gets hosts by host identifier
         GET_HOST_ADDR,          // Gets hosts by IPv4 address
@@ -1986,33 +1989,18 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters)
     // information from the database, so they can be used even if the
     // database is read only for the current user.
     conn_.prepareStatements(tagged_statements.begin(),
-                            tagged_statements.begin() + WRITE_STMTS_BEGIN,
-                            WRITE_STMTS_BEGIN);
+                            tagged_statements.begin() + WRITE_STMTS_BEGIN);
 
-    std::string readonly_value = "false";
-    try {
-        readonly_value = conn_.getParameter("readonly");
-        boost::algorithm::to_lower(readonly_value);
-    } catch (...) {
-        // Parameter "readonly" hasn't been specified so we simply use
-        // the default value of "false".
-    }
-
-    if (readonly_value == "true") {
-        is_readonly_ = true;
-
-    } else if (readonly_value != "false") {
-        isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value
-                  << "' specified for boolean parameter 'readonly'");
-    }
+    // Check if the backend is explicitly configured to operate with
+    // read only access to the database.
+    is_readonly_ = conn_.configuredReadOnly();
 
     // If we are using read-write mode for the database we also prepare
     // statements for INSERTS etc.
     if (!is_readonly_) {
         // Prepare statements for writing to the database, e.g. INSERT.
         conn_.prepareStatements(tagged_statements.begin() + WRITE_STMTS_BEGIN,
-                                tagged_statements.end(),
-                                tagged_statements.size());
+                                tagged_statements.end());
     } else {
         LOG_INFO(dhcpsrv_logger, DHCPSRV_MYSQL_HOST_DB_READONLY);
     }

diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc
index 7e448428f2bb3794284b7041a63203edd6e8de09..8edf4dfc5cbf29cae63a4d3009a156baf878b7e3 100644 (file)
--- a/src/lib/dhcpsrv/mysql_lease_mgr.cc
+++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc
@@ -1237,8 +1237,7 @@ MySqlLeaseMgr::MySqlLeaseMgr(const MySqlConnection::ParameterMap& parameters)
     }
 
     // Prepare all statements likely to be used.
-    conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end(),
-                            tagged_statements.size());
+    conn_.prepareStatements(tagged_statements.begin(), tagged_statements.end());
 
     // Create the exchange objects for use in exchanging data between the
     // program and the database.

diff --git a/src/lib/dhcpsrv/pgsql_host_data_source.cc b/src/lib/dhcpsrv/pgsql_host_data_source.cc
index d134d3c76623fa19e732ea33ed9d1c126977e878..2ab52809233b1edf2012c62e98a5dbafafea8c12 100644 (file)
--- a/src/lib/dhcpsrv/pgsql_host_data_source.cc
+++ b/src/lib/dhcpsrv/pgsql_host_data_source.cc
@@ -1105,7 +1105,10 @@ public:
 
     /// @brief Statement Tags
     ///
-    /// The contents of the enum are indexes into the list of SQL statements
+    /// The contents of the enum are indexes into the list of SQL statements.
+    /// It is assumed that the order is such that the indicies of statements
+    /// reading the database are less than those of statements modifying the
+    /// database.
     enum StatementIndex {
         GET_HOST_DHCPID,        // Gets hosts by host identifier
         GET_HOST_ADDR,          // Gets hosts by IPv4 address
@@ -1489,22 +1492,9 @@ PgSqlHostDataSourceImpl(const PgSqlConnection::ParameterMap& parameters)
     conn_.prepareStatements(tagged_statements.begin(),
                             tagged_statements.begin() + WRITE_STMTS_BEGIN);
 
-    std::string readonly_value = "false";
-    try {
-        readonly_value = conn_.getParameter("readonly");
-        boost::algorithm::to_lower(readonly_value);
-    } catch (...) {
-        // Parameter "readonly" hasn't been specified so we simply use
-        // the default value of "false".
-    }
-
-    if (readonly_value == "true") {
-        is_readonly_ = true;
-
-    } else if (readonly_value != "false") {
-        isc_throw(DbInvalidReadOnly, "invalid value '" << readonly_value
-                  << "' specified for boolean parameter 'readonly'");
-    }
+    // Check if the backend is explicitly configured to operate with
+    // read only access to the database.
+    is_readonly_ = conn_.configuredReadOnly();
 
     // If we are using read-write mode for the database we also prepare
     // statements for INSERTS etc.

diff --git a/src/share/database/scripts/mysql/dhcpdb_create.mysql b/src/share/database/scripts/mysql/dhcpdb_create.mysql
index 5d6a321200dcef32fa9be9b61c97c5c922683468..80aaf082c2a6b976cbd5b148e0626e661650f8a8 100644 (file)
--- a/src/share/database/scripts/mysql/dhcpdb_create.mysql
+++ b/src/share/database/scripts/mysql/dhcpdb_create.mysql
@@ -465,6 +465,11 @@ ALTER TABLE dhcp6_options
     ADD CONSTRAINT fk_dhcp6_option_scope FOREIGN KEY (scope_id)
     REFERENCES dhcp_option_scope (scope_id);
 
+# Add UNSIGNED to reservation_id
+ALTER TABLE ipv6_reservations
+    MODIFY reservation_id INT UNSIGNED NOT NULL AUTO_INCREMENT;
+
+
 # Update the schema version number
 UPDATE schema_version
 SET version = '4', minor = '2';