From: Marcin Siodelski Date: Tue, 17 May 2016 13:23:27 +0000 (+0200) Subject: [4281] Updated recently modified files with proper commentary. X-Git-Tag: trac4106_update_base~7^2~3^2~8 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a2847d55e1513925303aa95d400284bcde8a553;p=thirdparty%2Fkea.git [4281] Updated recently modified files with proper commentary. Also, fixed a couple of minor issues. --- diff --git a/src/lib/dhcp/libdhcp++.cc b/src/lib/dhcp/libdhcp++.cc index fdbc1437c1..3b746ef9b8 100644 --- a/src/lib/dhcp/libdhcp++.cc +++ b/src/lib/dhcp/libdhcp++.cc @@ -863,21 +863,12 @@ LibDHCP::optionSpaceToVendorId(const std::string& option_space) { try { check = boost::lexical_cast(x); } catch (const boost::bad_lexical_cast &) { - /// @todo: Should we throw here? - // isc_throw(BadValue, "Failed to parse vendor-X value (" << x - // << ") as unsigned 32-bit integer."); return (0); } if (check > std::numeric_limits::max()) { - /// @todo: Should we throw here? - //isc_throw(BadValue, "Value " << x << "is too large" - // << " for unsigned 32-bit integer."); return (0); } if (check < 0) { - /// @todo: Should we throw here? - // isc_throw(BadValue, "Value " << x << "is negative." - // << " Only 0 or larger are allowed for unsigned 32-bit integer."); return (0); } diff --git a/src/lib/dhcpsrv/cfg_option.cc b/src/lib/dhcpsrv/cfg_option.cc index 4532e5a217..b901d6030d 100644 --- a/src/lib/dhcpsrv/cfg_option.cc +++ b/src/lib/dhcpsrv/cfg_option.cc @@ -65,6 +65,8 @@ CfgOption::getVendorIdsSpaceNames() const { for (std::list::const_iterator id = ids.begin(); id != ids.end(); ++id) { std::ostringstream s; + // Vendor space name is constructed as "vendor-XYZ" where XYZ is an + // uint32_t value, without leading zeros. s << "vendor-" << *id; names.push_back(s.str()); } diff --git a/src/lib/dhcpsrv/cfg_option.h b/src/lib/dhcpsrv/cfg_option.h index 52f85f96bd..065316fa6a 100644 --- a/src/lib/dhcpsrv/cfg_option.h +++ b/src/lib/dhcpsrv/cfg_option.h @@ -18,7 +18,6 @@ #include #include #include -#include #include namespace isc { @@ -34,10 +33,10 @@ struct OptionDescriptor { /// @brief Option instance. OptionPtr option_; - /// @briefPersistence flag. + /// @brief Persistence flag. /// - /// If true option is always sent to the client, if false option is - /// sent to the client on request. + /// If true, option is always sent to the client. If false, option is + /// sent to the client when requested using ORO or PRL option. bool persistent_; /// @brief Option value in textual (CSV) format. @@ -48,9 +47,10 @@ struct OptionDescriptor { /// database instead of the binary format. /// /// Note that this value is carried in the option descriptor, rather than - /// @c Option instance because it is a server specific value. + /// @c Option instance because it is a server specific value (same as + /// persistence flag). /// - /// An example of the formatted value is: 2001:db8:1::1, 23, some text + /// An example of the formatted value is: "2001:db8:1::1, 23, some text" /// for the option which carries IPv6 address, a number and a text. std::string formatted_value_; @@ -276,14 +276,11 @@ public: void add(const OptionPtr& option, const bool persistent, const std::string& option_space); - /// @brief A variant of the method which takes option descriptor as an - /// argument. - /// - /// This method works exactly the same as the other variant, but it takes - /// option descriptor as an argument. + /// @brief A variant of the @ref CfgOption::add method which takes option + /// descriptor as an argument. /// /// @param desc Option descriptor holding option instance and other - /// parameters pertaining to this option. + /// parameters pertaining to the option. /// @param option_space Option space name. /// /// @throw isc::BadValue if the option space is invalid. @@ -372,7 +369,13 @@ public: return (*od_itr); } - /// @brief Returns a list of all configured option space names. + /// @brief Returns a list of configured option space names. + /// + /// The returned option space names exclude vendor option spaces, + /// such as "vendor-1234". These are returned by the + /// @ref getVendorIdsSpaceNames. + /// + /// @return List comprising option space names. std::list getOptionSpaceNames() const { return (options_.getOptionSpaceNames()); } @@ -385,7 +388,10 @@ public: /// @brief Returns a list of option space names for configured vendor ids. /// /// For each vendor-id the option space name returned is constructed - /// as "vendor-". + /// as "vendor-XYZ" where XYZ is a @c uint32_t value without leading + /// zeros. + /// + /// @return List comprising option space names for vendor options. std::list getVendorIdsSpaceNames() const; private: diff --git a/src/lib/dhcpsrv/mysql_connection.cc b/src/lib/dhcpsrv/mysql_connection.cc index 8096cfd7aa..6da2dae458 100755 --- a/src/lib/dhcpsrv/mysql_connection.cc +++ b/src/lib/dhcpsrv/mysql_connection.cc @@ -30,6 +30,8 @@ const int MLM_MYSQL_FETCH_FAILURE = 1; MySqlTransaction::MySqlTransaction(MySqlConnection& conn) : conn_(conn), committed_(false) { + // We create prepared statements for all other queries, but MySQL + // don't support prepared statements for START TRANSACTION. int status = mysql_query(conn_.mysql_, "START TRANSACTION"); if (status != 0) { isc_throw(DbOperationError, "unable to start transaction, " @@ -38,6 +40,8 @@ MySqlTransaction::MySqlTransaction(MySqlConnection& conn) } MySqlTransaction::~MySqlTransaction() { + // Rollback if the MySqlTransaction::commit wasn't explicitly + // called. if (!committed_) { conn_.rollback(); } diff --git a/src/lib/dhcpsrv/mysql_connection.h b/src/lib/dhcpsrv/mysql_connection.h index 866bf6472a..09dfe72303 100755 --- a/src/lib/dhcpsrv/mysql_connection.h +++ b/src/lib/dhcpsrv/mysql_connection.h @@ -140,23 +140,60 @@ private: MYSQL* mysql_; ///< Initialization context }; +/// @brief Forward declaration to @ref MySqlConnection. class MySqlConnection; +/// @brief RAII object representing MySQL transaction. +/// +/// An instance of this class should be created in a scope where multiple +/// INSERT statements should be executed within a single transaction. The +/// transaction is started when the constructor of this class is invoked. +/// The transaction is ended when the @ref MySqlTransaction::commit is +/// explicitly called or when the instance of this class is destroyed. +/// The @ref MySqlTransaction::commit commits changes to the database +/// and the changes remain in the database when the instance of the +/// class is destroyed. If the class instance is destroyed before the +/// @ref MySqlTransaction::commit is called, the transaction is rolled +/// back. The rollback on destruction guarantees that partial data is +/// not stored in the database when there is an error during any +/// of the operations belonging to a transaction. +/// +/// The default MySQL backend configuration enables 'autocommit'. +/// Starting a transaction overrides 'autocommit' setting for this +/// particular transaction only. It does not affect the global 'autocommit' +/// setting for the database connection, i.e. all modifications to the +/// database which don't use transactions will still be auto committed. class MySqlTransaction : public boost::noncopyable { public: + /// @brief Constructor. + /// + /// Starts transaction by making a "START TRANSACTION" query. + /// + /// @param conn MySQL connection to use for the transaction. This + /// connection will be later used to commit or rollback changes. + /// + /// @throw DbOperationError if "START TRANSACTION" query fails. MySqlTransaction(MySqlConnection& conn); + /// @brief Destructor. + /// + /// Rolls back the transaction if changes haven't been committed. ~MySqlTransaction(); + /// @brief Commits transaction. void commit(); private: + /// @brief Holds reference to the MySQL database connection. MySqlConnection& conn_; + /// @brief Boolean flag indicating if the transaction has been committed. + /// + /// This flag is used in the class destructor to assess if the + /// transaction should be rolled back. bool committed_; - }; diff --git a/src/lib/dhcpsrv/mysql_host_data_source.cc b/src/lib/dhcpsrv/mysql_host_data_source.cc index 3bd484dd45..4539c617cd 100644 --- a/src/lib/dhcpsrv/mysql_host_data_source.cc +++ b/src/lib/dhcpsrv/mysql_host_data_source.cc @@ -95,11 +95,10 @@ TaggedStatement tagged_statements[] = { "persistent, dhcp_client_class, dhcp6_subnet_id, host_id) " " VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)"}, - // Retrieves host information along with IPv6 reservations associated - // with this host. If the host exists in multiple subnets, all hosts - // having a specified identifier will be returned from those subnets. - // Because LEFT JOIN clause is used, the number of rows returned for - // a single host depends on the number of reservations. + // Retrieves host information, IPv6 reservations and both DHCPv4 and + // DHCPv6 options associated with the host. The LEFT JOIN clause is used + // to retrieve information from 4 different tables using a single query. + // Hence, this query returns multiple rows for a single host. {MySqlHostDataSource::GET_HOST_DHCPID, "SELECT DISTINCT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, " @@ -120,9 +119,9 @@ TaggedStatement tagged_statements[] = { "WHERE dhcp_identifier = ? AND dhcp_identifier_type = ? " "ORDER BY h.host_id, o4.option_id, o6.option_id, r.reservation_id"}, - // Retrieves host information by IPv4 address. This should typically - // return a single host, but if we ever allow for defining subnets - // with overlapping address pools, multiple hosts may be returned. + // Retrieves host information along with the DHCPv4 options associated with + // it. Left joining the dhcp4_options table results in multiple rows being + // returned for the same host. The host is retrieved by IPv4 address. {MySqlHostDataSource::GET_HOST_ADDR, "SELECT DISTINCT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " @@ -135,8 +134,9 @@ TaggedStatement tagged_statements[] = { "WHERE ipv4_address = ? " "ORDER BY h.host_id, o.option_id"}, - // Retrieves host information by subnet identifier and unique - // identifier of a client. This is expected to return a single host. + // Retrieves host information and DHCPv4 options using subnet identifier + // and client's identifier. Left joining the dhcp4_options table results in + // multiple rows being returned for the same host. {MySqlHostDataSource::GET_HOST_SUBID4_DHCPID, "SELECT DISTINCT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " @@ -150,12 +150,9 @@ TaggedStatement tagged_statements[] = { " AND h.dhcp_identifier = ? " "ORDER BY h.host_id, o.option_id"}, - // Retrieves host information by subnet identifier and unique - // identifier of a client. This query should return information - // for a single host but multiple rows are returned due to - // use of LEFT JOIN clause. The number of rows returned for a single - // host dpeneds on the number of IPv6 reservations existing for - // this client. + // Retrieves host information, IPv6 reservations and DHCPv6 options + // associated with a host. The number of rows returned is a multiplication + // of number of IPv6 reservations and DHCPv6 options. {MySqlHostDataSource::GET_HOST_SUBID6_DHCPID, "SELECT DISTINCT h.host_id, h.dhcp_identifier, " "h.dhcp_identifier_type, h.dhcp4_subnet_id, " @@ -174,9 +171,10 @@ TaggedStatement tagged_statements[] = { "AND h.dhcp_identifier = ? " "ORDER BY h.host_id, o.option_id, r.reservation_id"}, - // Retrieves host information using subnet identifier and the - // IPv4 address reservation. This should return inforamation for - // a single host. + // Retrieves host information and DHCPv4 options for the host using subnet + // identifier and IPv4 reservation. Left joining the dhcp4_options table + // results in multiple rows being returned for the host. The number of + // rows depends on the number of options defined for the host. {MySqlHostDataSource::GET_HOST_SUBID_ADDR, "SELECT DISTINCT h.host_id, h.dhcp_identifier, h.dhcp_identifier_type, " "h.dhcp4_subnet_id, h.dhcp6_subnet_id, h.ipv4_address, h.hostname, " @@ -189,12 +187,12 @@ TaggedStatement tagged_statements[] = { "WHERE h.dhcp4_subnet_id = ? AND h.ipv4_address = ? " "ORDER BY h.host_id, o.option_id"}, - // Retrieves host information using IPv6 prefix and prefix length - // or IPv6 address. This query returns host information for a - // single host. However, multiple rows are returned by this - // query due to use of LEFT JOIN clause with 'ipv6_reservations' - // table. The number of rows returned depends on the number of - // reservations for a particular host. + // Retrieves host information, IPv6 reservations and DHCPv6 options + // associated with a host using prefix and prefix length. This query + // returns host information for a single host. However, multiple rows + // are returned due to left joining IPv6 reservations and DHCPv6 options. + // The number of rows returned is multiplication of number of existing + // IPv6 reservations and DHCPv6 options. {MySqlHostDataSource::GET_HOST_PREFIX, "SELECT DISTINCT h.host_id, h.dhcp_identifier, " "h.dhcp_identifier_type, h.dhcp4_subnet_id, " @@ -225,29 +223,25 @@ TaggedStatement tagged_statements[] = { /// @brief This class provides mechanisms for sending and retrieving /// information from the 'hosts' table. /// -/// This class should be used to create new entries in the 'hosts' -/// table and to retrieve DHCPv4 reservations from this table. The -/// queries used with this class do not retrieve IPv6 reservations for -/// the hosts to minimize negative impact on performance. -/// -/// The derived class MySqlHostIPv6Exchange extends this class to facilitate -/// retrieving IPv6 reservations along with the host information. +/// This class is used to insert and retrieve entries from the 'hosts' table. +/// The queries used with this class do not retrieve IPv6 reservations or +/// options associated with a host to minimize impact on performance. Other +/// classes derived from @ref MySqlHostExchange should be used to retrieve +/// information about IPv6 reservations and options. class MySqlHostExchange { private: - /// @brief Number of columns returned for queries used with this class. + /// @brief Number of columns returned for SELECT queries send by this class. static const size_t HOST_COLUMNS = 9; public: /// @brief Constructor /// - /// The initialization of the variables here is only to satisfy cppcheck - - /// all variables are initialized/set in the methods before they are used. - /// - /// @param additional_columns_num Additional number of columns to standard - /// set of columns used by this class, for which resources should be - /// allocated. + /// @param additional_columns_num This value is set by the derived classes + /// to indicate how many additional columns will be returned by SELECT + /// queries performed by the derived class. This constructor will allocate + /// resources for these columns, e.g. binding table, error indicators. MySqlHostExchange(const size_t additional_columns_num = 0) : columns_num_(HOST_COLUMNS + additional_columns_num), bind_(columns_num_), columns_(columns_num_), @@ -268,7 +262,9 @@ public: memset(dhcp4_client_classes_, 0, sizeof(dhcp4_client_classes_)); memset(dhcp6_client_classes_, 0, sizeof(dhcp6_client_classes_)); - // Set the column names (for error messages) + // Set the column names for use by this class. This only comprises + // names used by the MySqlHostExchange class. Derived classes will + // need to set names for the columns they use. columns_[0] = "host_id"; columns_[1] = "dhcp_identifier"; columns_[2] = "dhcp_identifier_type"; @@ -286,6 +282,20 @@ public: virtual ~MySqlHostExchange() { } + /// @brief Returns index of the first uninitialized column name. + /// + /// This method is called by the derived classes to determine which + /// column indexes are available for the derived classes within a + /// binding array, error array and column names. This method + /// determines the first available index by searching the first + /// empty value within the columns_ vector. Previously we relied on + /// the fixed values set for each class, but this was hard to maintain + /// when new columns were added to the SELECT queries. It required + /// modifying indexes in all derived classes. + /// + /// Derived classes must call this method in their constructors and + /// use returned value as an index for the first column used by the + /// derived class and increment this value for each subsequent column. size_t findAvailColumn() const { std::vector::const_iterator empty_column = std::find(columns_.begin(), columns_.end(), std::string()); @@ -627,8 +637,8 @@ public: /// adding duplicated hosts to the collection, assuming that processed /// rows are primarily ordered by host id column. /// - /// @todo This method will need to be extended to process options - /// associated with hosts. + /// This method must be overriden in the derived classes to also + /// retrieve IPv6 reservations and DHCP options associated with a host. /// /// @param [out] hosts Collection of hosts to which a new host created /// from the processed data should be inserted. @@ -749,14 +759,44 @@ private: }; +/// @brief Extends base exchange class with ability to retrieve DHCP options +/// from the 'dhcp4_options' and 'dhcp6_options' tables. +/// +/// This class provides means to retrieve both DHCPv4 and DHCPv6 options +/// along with the host information. It is not used to retrieve IPv6 +/// reservations. The following types of queries are supported: +/// - SELECT ? FROM hosts LEFT JOIN dhcp4_options LEFT JOIN dhcp6_options ... +/// - SELECT ? FROM hosts LEFT JOIN dhcp4_options ... +/// - SELECT ? FROM hosts LEFT JOIN dhcp6_options ... class MySqlHostExchangeOpts : public MySqlHostExchange { private: - /// @brief Number of columns holding option information. + /// @brief Number of columns holding DHCPv4 or DHCPv6 option information. static const size_t OPTION_COLUMNS = 6; + /// @brief Receives DHCPv4 or DHCPv6 options information from the + /// dhcp4_options or dhcp6_options tables respectively. + /// + /// The MySqlHostExchangeOpts class holds two respective instances of this + /// class, one for receiving DHCPv4 options, one for receiving DHCPv6 + /// options. + /// + /// The following are the basic functions of this class: + /// - bind class members to specific columns in MySQL binding tables, + /// - set DHCP options specific column names, + /// - create instances of options retrieved from the database. + /// + /// The reason for isolating those functions in a separate C++ class is + /// to prevent code duplication for handling DHCPv4 and DHCPv6 options. class OptionProcessor { public: + + /// @brief Constructor. + /// + /// @param universe V4 or V6. The type of the options' instances + /// created by this class depends on this parameter. + /// @param start_column Index of the first column to be used by this + /// class. OptionProcessor(const Option::Universe& universe, const size_t start_column) : universe_(universe), start_column_(start_column), option_id_(0), @@ -776,6 +816,7 @@ private: memset(space_, 0, sizeof(space_)); } + /// @brief Returns identifier of the currently processed option. uint64_t getOptionId() const { if (option_id_null_ == MLM_FALSE) { return (option_id_); @@ -783,67 +824,112 @@ private: return (0); } + /// @brief Creates instance of the currently processed option. + /// + /// This method detects if the currently processed option is a new + /// instance. It makes it determination by comparing the identifier + /// of the currently processed option, with the most recently processed + /// option. If the current value is greater than the id of the recently + /// processed option it is assumed that the processed row holds new + /// option information. In such case the option instance is created and + /// inserted into the configuration passed as argument. + /// + /// @param cfg Pointer to the configuration object into which new + /// option instances should be inserted. void retrieveOption(const CfgOptionPtr& cfg) { - if (option_id_ == 0) { + // option_id may be NULL if dhcp4_options or dhcp6_options table + // doesn't contain any options for the particular host. Also, the + // current option id must be greater than id if the most recent + // option because options are ordered by option id. Otherwise + // we assume that this is already processed option. + if ((option_id_null_ == MLM_TRUE) || + (most_recent_option_id_ >= option_id_)) { return; } - if (most_recent_option_id_ < option_id_) { - most_recent_option_id_ = option_id_; - - std::string space; - if (space_null_ == MLM_FALSE) { - space_[space_length_] = '\0'; - space.assign(space_); - } - - std::string formatted_value; - if (formatted_value_null_ == MLM_FALSE) { - formatted_value_[formatted_value_length_] = '\0'; - formatted_value.assign(formatted_value_); - } + // Remember current option id as the most recent processed one. We + // will be comparing it with option ids in subsequent rows. + most_recent_option_id_ = option_id_; + + // Convert it to string object for easier comparison. + std::string space; + if (space_null_ == MLM_FALSE) { + // Typically, the string values returned by the database are not + // NULL terminated. + space_[space_length_] = '\0'; + space.assign(space_); + } - OptionDefinitionPtr def; - if ((space == DHCP4_OPTION_SPACE) || (space == DHCP6_OPTION_SPACE)) { - def = LibDHCP::getOptionDef(universe_, code_); - } + // Convert formatted_value to string as well. + std::string formatted_value; + if (formatted_value_null_ == MLM_FALSE) { + formatted_value_[formatted_value_length_] = '\0'; + formatted_value.assign(formatted_value_); + } - if (!def && (space != DHCP4_OPTION_SPACE) && - (space != DHCP6_OPTION_SPACE)) { - uint32_t vendor_id = LibDHCP::optionSpaceToVendorId(space); - if (vendor_id > 0) { - def = LibDHCP::getVendorOptionDef(universe_, vendor_id, code_); - } - } + // Options are held in a binary or textual format in the database. + // This is similar to having an option specified in a server + // configuration file. Such option is converted to appropriate C++ + // class, using option definition. Thus, we need to find the + // option definition for this option code and option space. + + // If the option space is a standard DHCPv4 or DHCPv6 option space, + // this is most likely a standard option, for which we have a + // definition created within libdhcp++. + OptionDefinitionPtr def; + if ((space == DHCP4_OPTION_SPACE) || (space == DHCP6_OPTION_SPACE)) { + def = LibDHCP::getOptionDef(universe_, code_); + } - if (!def) { - def = LibDHCP::getRuntimeOptionDef(space, code_); + // Otherwise, we may check if this an option encapsulated within the + // vendor space. + if (!def && (space != DHCP4_OPTION_SPACE) && + (space != DHCP6_OPTION_SPACE)) { + uint32_t vendor_id = LibDHCP::optionSpaceToVendorId(space); + if (vendor_id > 0) { + def = LibDHCP::getVendorOptionDef(universe_, vendor_id, code_); } + } + // In all other cases, we use runtime option definitions, which + // should be also registered within the libdhcp++. + if (!def) { + def = LibDHCP::getRuntimeOptionDef(space, code_); + } - OptionPtr option; + OptionPtr option; - if (!def) { + if (!def) { + // If no definition found, we use generic option type. + OptionBuffer buf(value_, value_ + value_length_); + option.reset(new Option(universe_, code_, buf.begin(), + buf.end())); + } else { + // The option value may be specified in textual or binary format + // in the database. If formatted_value is empty, the binary + // format is used. Depending on the format we use a different + // variant of the optionFactory function. + if (formatted_value.empty()) { OptionBuffer buf(value_, value_ + value_length_); - option.reset(new Option(universe_, code_, buf.begin(), - buf.end())); + option = def->optionFactory(universe_, code_, buf.begin(), + buf.end()); } else { - if (formatted_value.empty()) { - OptionBuffer buf(value_, value_ + value_length_); - option = def->optionFactory(universe_, code_, buf.begin(), - buf.end()); - } else { - std::vector split_vec; - boost::split(split_vec, formatted_value, boost::is_any_of(",")); - option = def->optionFactory(universe_, code_, split_vec); - } + // Spit the value specified in comma separated values + // format. + std::vector split_vec; + boost::split(split_vec, formatted_value, boost::is_any_of(",")); + option = def->optionFactory(universe_, code_, split_vec); } - - OptionDescriptor desc(option, persistent_, formatted_value); - cfg->add(desc, space); } + + OptionDescriptor desc(option, persistent_, formatted_value); + cfg->add(desc, space); } + /// @brief Specify column names. + /// + /// @param [out] columns Reference to a vector holding names of option + /// specific columns. void setColumnNames(std::vector& columns) { columns[option_id_index_] = "option_id"; columns[code_index_] = "code"; @@ -853,7 +939,15 @@ private: columns[persistent_index_] = "persistent"; } + /// @brief Initialize binding table fields for options. + /// + /// Resets most_recent_option_id_ value to 0. + /// + /// @param [out] bind Binding table. void setBindFields(std::vector& bind) { + // This method is called just before making a new query, so we + // reset the most_recent_option_id_ to start over with options + // processing. most_recent_option_id_ = 0; // option_id : INT UNSIGNED NOT NULL AUTO_INCREMENT, @@ -899,76 +993,79 @@ private: private: + /// @brief Universe: V4 or V6. Option::Universe universe_; + /// @brief Index of first column used by this class. size_t start_column_; - /// Option id. + /// @brief Option id. uint64_t option_id_; - /// Option code. + /// @brief Option code. uint16_t code_; - /// Buffer holding binary value of an option. + /// @brief Buffer holding binary value of an option. uint8_t value_[OPTION_VALUE_MAX_LEN]; - /// Option value length. + /// @brief Option value length. unsigned long value_length_; - /// Buffer holding textual value of an option. + /// @brief Buffer holding textual value of an option. char formatted_value_[OPTION_FORMATTED_VALUE_MAX_LEN]; - /// Formatted option value length. + /// @brief Formatted option value length. unsigned long formatted_value_length_; - /// Buffer holding option space name. + /// @brief Buffer holding option space name. char space_[OPTION_SPACE_MAX_LEN]; - /// Option space length. + /// @brief Option space length. unsigned long space_length_; - /// Flag indicating if option is always sent or only if requested. + /// @brief Flag indicating if option is always sent or only if + /// requested. bool persistent_; /// @name Boolean values indicating if values of specific columns in /// the database are NULL. //@{ - /// Boolean flag indicating if the DHCPv4 option id is NULL. + /// @brief Boolean flag indicating if the DHCPv4 option id is NULL. my_bool option_id_null_; - /// Boolean flag indicating if the DHCPv4 option code is NULL. + /// @brief Boolean flag indicating if the DHCPv4 option code is NULL. my_bool code_null_; - /// Boolean flag indicating if the DHCPv4 option value is NULL. + /// @brief Boolean flag indicating if the DHCPv4 option value is NULL. my_bool value_null_; - /// Boolean flag indicating if the DHCPv4 formatted option value + /// @brief Boolean flag indicating if the DHCPv4 formatted option value /// is NULL. my_bool formatted_value_null_; - /// Boolean flag indicating if the DHCPv4 option space is NULL. + /// @brief Boolean flag indicating if the DHCPv4 option space is NULL. my_bool space_null_; //@} /// @name Indexes of the specific columns //@{ - /// Option id + /// @brief Option id size_t option_id_index_; - /// Code + /// @brief Code size_t code_index_; - /// Value + /// @brief Value size_t value_index_; - /// Formatted value + /// @brief Formatted value size_t formatted_value_index_; - /// Space + /// @brief Space size_t space_index_; - /// Persistent + /// @brief Persistent size_t persistent_index_; //@} @@ -976,22 +1073,38 @@ private: uint64_t most_recent_option_id_; }; + /// @brief Pointer to the @ref OptionProcessor class. typedef boost::shared_ptr OptionProcessorPtr; public: + /// @brief DHCP option types to be fetched from the database. + /// + /// Supported types are: + /// - Only DHCPv4 options, + /// - Only DHCPv6 options, + /// - Both DHCPv4 and DHCPv6 options. enum FetchedOptions { DHCP4_ONLY, DHCP6_ONLY, DHCP4_AND_DHCP6 }; + /// @brief Constructor. + /// + /// @param fetched_options Specifies if DHCPv4, DHCPv6 or both should + /// be fetched from the database for a host. + /// @param additional_columns_num Number of additional columns for which + /// resources should be allocated, e.g. binding table, column names etc. + /// This parameter should be set to a non zero value by derived classes to + /// allocate resources for the columns supported by derived classes. MySqlHostExchangeOpts(const FetchedOptions& fetched_options, const size_t additional_columns_num = 0) : MySqlHostExchange(getRequiredColumnsNum(fetched_options) + additional_columns_num), opt_proc4_(), opt_proc6_() { + // Create option processor for DHCPv4 options, if required. if ((fetched_options == DHCP4_ONLY) || (fetched_options == DHCP4_AND_DHCP6)) { opt_proc4_.reset(new OptionProcessor(Option::V4, @@ -999,6 +1112,7 @@ public: opt_proc4_->setColumnNames(columns_); } + // Create option processor for DHCPv6 options, if required. if ((fetched_options == DHCP6_ONLY) || (fetched_options == DHCP4_AND_DHCP6)) { opt_proc6_.reset(new OptionProcessor(Option::V6, @@ -1007,10 +1121,17 @@ public: } } + /// @brief Processes the current row. + /// + /// The processed row includes both host information and DHCP option + /// information. Because used SELECT query use LEFT JOIN clause, the + /// some rows contain duplicated host or options entries. This method + /// detects duplicated information and discards such entries. + /// + /// @param [out] hosts Container holding parsed hosts and options. virtual void processFetchedData(ConstHostCollection& hosts) { - HostPtr host; + // Holds pointer to the previously parsed host. HostPtr most_recent_host; - if (!hosts.empty()) { // Const cast is not very elegant way to deal with it, but // there is a good reason to use it here. This method is called @@ -1021,34 +1142,43 @@ public: // most recently added host in a class member but this would // make the code less readable. most_recent_host = boost::const_pointer_cast(hosts.back()); - } - if (!most_recent_host || (most_recent_host->getHostId() != getHostId())) { - host = retrieveHost(); + // If no host has been parsed yet or we're at the row holding next + // host, we create a new host object and put it at the end of the + // list. + if (!most_recent_host || (most_recent_host->getHostId() < getHostId())) { + HostPtr host = retrieveHost(); hosts.push_back(host); most_recent_host = host; } + // Parse DHCPv4 options if required to do so. if (opt_proc4_) { CfgOptionPtr cfg = most_recent_host->getCfgOption4(); opt_proc4_->retrieveOption(cfg); } + // Parse DHCPv6 options if required to do so. if (opt_proc6_) { CfgOptionPtr cfg = most_recent_host->getCfgOption6(); opt_proc6_->retrieveOption(cfg); } } + /// @brief Bind variables for receiving option data. + /// + /// @return Vector of MYSQL_BIND object representing data to be retrieved. virtual std::vector createBindForReceive() { // The following call sets bind_ values between 0 and 8. static_cast(MySqlHostExchange::createBindForReceive()); + // Bind variables for DHCPv4 options. if (opt_proc4_) { opt_proc4_->setBindFields(bind_); } + // Bind variables for DHCPv6 options. if (opt_proc6_) { opt_proc6_->setBindFields(bind_); } @@ -1056,13 +1186,22 @@ public: // Add the error flags setErrorIndicators(bind_, error_); - // Add the data to the vector. Note the end element is one after the - // end of the array. return (bind_); }; private: + /// @brief Returns a number of columns required to retrieve option data. + /// + /// Depending if we need DHCPv4/DHCPv6 options only, or both DHCPv4 and + /// DHCPv6 a different number of columns is required in the binding array. + /// This method returns the number of required columns, according to the + /// value of @c fetched_columns passed in the constructor. + /// + /// @param fetched_columns A value which specifies whether DHCPv4, DHCPv6 or + /// both types of options should be retrieved. + /// + /// @return Number of required columns. static size_t getRequiredColumnsNum(const FetchedOptions& fetched_options) { return (fetched_options == DHCP4_AND_DHCP6 ? 2 * OPTION_COLUMNS : OPTION_COLUMNS); @@ -1079,18 +1218,18 @@ private: OptionProcessorPtr opt_proc6_; }; -/// @Brief This class provides mechanisms for sending and retrieving -/// host information and associated IPv6 reservations. +/// @brief This class provides mechanisms for sending and retrieving +/// host information, DHCPv4 options, DHCPv6 options and IPv6 reservations. /// -/// This class extends the @ref MySqlHostExchange class with the -/// mechanisms to retrieve IPv6 reservations along with host -/// information. It is assumed that both host data and IPv6 -/// reservations are retrieved with a single query (using LEFT JOIN -/// MySQL clause). Because the host to IPv6 reservation is a 1-to-many -/// relation, the same row from the Host table is returned many times -/// (for each IPv6 reservation). This class is responsible for -/// converting those multiple host instances into a single Host -/// object with multiple IPv6 reservations. +/// This class extends the @ref MySqlHostExchangeOpts class with the +/// mechanisms to retrieve IPv6 reservations. This class is used in sitations +/// when it is desired to retrieve DHCPv6 specific information about the host +/// (DHCPv6 options and reservations), or entire information about the host +/// (DHCPv4 options, DHCPv6 options and reservations). The following are the +/// queries used with this class: +/// - SELECT ? FROM hosts LEFT JOIN dhcp4_options LEFT JOIN dhcp6_options +/// LEFT JOIN ipv6_reservations ... +/// - SELECT ? FROM hosts LEFT JOIN dhcp6_options LEFT JOIN ipv6_reservations .. class MySqlHostIPv6Exchange : public MySqlHostExchangeOpts { private: @@ -1117,7 +1256,7 @@ public: memset(ipv6_address_buffer_, 0, sizeof(ipv6_address_buffer_)); - // Append additional columns returned by the queries. + // Provide names of additional columns returned by the queries. columns_[reservation_id_index_] = "reservation_id"; columns_[address_index_] = "address"; columns_[prefix_len_index_] = "prefix_len"; @@ -1135,7 +1274,7 @@ public: return (0); }; - /// @brief Create IPv6 reservation from the data contained in the + /// @brief Creates IPv6 reservation from the data contained in the /// currently processed row. /// /// Called after the MYSQL_BIND array created by createBindForReceive(). @@ -1175,12 +1314,14 @@ public: /// adding duplicated hosts to the collection, assuming that processed /// rows are primarily ordered by host id column. /// - /// For any returned row which contains IPv6 reservation information it - /// creates a @ref IPv6Resrv and appends it to the collection of the - /// IPv6 reservations in a Host object. + /// Depending on the value of the @c fetched_options specified in the + /// constructor, this method also parses options returned as a result + /// of SELECT queries. /// - /// @todo This method will need to be extended to process DHCPv6 options - /// associated with hosts. + /// For any returned row which contains IPv6 reservation information it + /// checks if the reservation is not a duplicate of previously parsed + /// reservation and appends the IPv6Resrv object into the host object + /// if the parsed row contains new reservation information. /// /// @param [out] hosts Collection of hosts to which a new host created /// from the processed data should be inserted. @@ -1223,7 +1364,7 @@ public: // a new SELECT query. most_recent_reservation_id_ = 0; - // The following call sets bind_ values between 0 and 8. + // Bind values supported by parent classes. static_cast(MySqlHostExchangeOpts::createBindForReceive()); // reservation_id : INT UNSIGNED NOT NULL AUTO_INCREMENT @@ -1258,8 +1399,6 @@ public: // Add the error flags setErrorIndicators(bind_, error_); - // Add the data to the vector. Note the end element is one after the - // end of the array. return (bind_); }; @@ -1459,13 +1598,18 @@ private: my_bool error_[RESRV_COLUMNS]; }; +/// @brief This class is used for inserting options into a database. +/// +/// This class supports inserting both DHCPv4 and DHCPv6 options. class MySqlOptionExchange { private: + /// @brief Number of columns in the tables holding options. static const size_t OPTION_COLUMNS = 9; public: + /// @brief Constructor. MySqlOptionExchange() : type_(0), value_len_(0), formatted_value_len_(0), space_(), space_len_(0), persistent_(false), client_class_(), client_class_len_(0), @@ -1474,7 +1618,9 @@ public: BOOST_STATIC_ASSERT(8 < OPTION_COLUMNS); } - + /// @brief Creates binding array to insert option data into database. + /// + /// @return Vector of MYSQL_BIND object representing an option. std::vector createBindForSend(const OptionDescriptor& opt_desc, const std::string& opt_space, @@ -1482,7 +1628,7 @@ public: const HostID& host_id) { // Hold pointer to the option to make sure it remains valid until - // we make a query. + // we complete a query. option_ = opt_desc.option_; memset(bind_, 0, sizeof(bind_)); @@ -1494,16 +1640,19 @@ public: bind_[0].buffer_type = MYSQL_TYPE_NULL; // code: SMALLINT UNSIGNED NOT NULL - type_ = option_->getType(); + type_ = option_->getType(); bind_[1].buffer_type = MYSQL_TYPE_SHORT; bind_[1].buffer = reinterpret_cast(&type_); bind_[1].is_unsigned = MLM_TRUE; // value: BLOB NULL - OutputBuffer buf(opt_desc.option_->len()); - opt_desc.option_->pack(buf); - if ((buf.getLength() > opt_desc.option_->getHeaderLen()) && - opt_desc.formatted_value_.empty()) { + if (opt_desc.formatted_value_.empty() && + (opt_desc.option_->len() > opt_desc.option_->getHeaderLen())) { + // The formatted_value is empty and the option value is + // non-empty so we need to prepare on-wire format for the + // option and store it in the database as a blob. + OutputBuffer buf(opt_desc.option_->len()); + opt_desc.option_->pack(buf); const char* buf_ptr = static_cast(buf.getData()); value_.assign(buf_ptr + opt_desc.option_->getHeaderLen(), buf_ptr + buf.getLength()); @@ -1514,6 +1663,8 @@ public: bind_[2].length = &value_len_; } else { + // No value or formatted_value specified. In this case, the + // value blob is NULL. value_.clear(); bind_[2].buffer_type = MYSQL_TYPE_NULL; } @@ -1581,33 +1732,45 @@ public: private: + /// @brief Option type. uint16_t type_; + /// @brief Option value as binary. std::vector value_; + /// @brief Option value length. size_t value_len_; - /// Formatted option value length. + /// @brief Formatted option value length. unsigned long formatted_value_len_; + /// @brief Option space name. std::string space_; + /// @brief Option space name length. size_t space_len_; + /// @brief Boolean flag indicating if the option is always returned to + /// a client or only when requested. bool persistent_; + /// @brief Client classes for the option. std::string client_class_; + /// @brief Length of the string holding client classes for the option. size_t client_class_len_; + /// @brief Subnet identifier. uint32_t subnet_id_; + /// @brief Host identifier. uint32_t host_id_; + /// @brief Pointer to currently parsed option. OptionPtr option_; + /// @brief Array of MYSQL_BIND elements representing inserted data. MYSQL_BIND bind_[OPTION_COLUMNS]; - }; } // end of anonymous namespace @@ -1644,12 +1807,29 @@ public: /// @param id ID of a host owning this reservation void addResv(const IPv6Resrv& resv, const HostID& id); + /// @brief Inserts a single DHCP option into the database. + /// + /// @param stindex Index of a statement being executed. + /// @param opt_desc Option descriptor holding information about an option + /// to be inserted into the database. + /// @param opt_space Option space name. + /// @param subnet_id Subnet identifier. + /// @param host_id Host identifier. void addOption(const MySqlHostDataSource::StatementIndex& stindex, const OptionDescriptor& opt_desc, const std::string& opt_space, const OptionalValue& subnet_id, - const HostID& id); + const HostID& host_id); + /// @brief Inserts multiple options into the database. + /// + /// @param stindex Index of a statement being executed. + /// @param options_cfg An object holding a collection of options to be + /// inserted into the database. + /// @param host_id Host identifier reference to a value to which + /// host identifier will be assigned if the current host_id value is 0. + /// The host identifier is retrieve from the database using the + /// mysql_insert_id function. void addOptions(const MySqlHostDataSource::StatementIndex& stindex, const ConstCfgOptionPtr& options_cfg, uint64_t& host_id); @@ -1671,13 +1851,14 @@ public: const char* what) const; /// @brief Creates collection of @ref Host objects with associated - /// information such as IPv6 reservations. + /// information such as IPv6 reservations and/or DHCP options.. /// /// This method performs a query which returns host information from /// the 'hosts' table. The query may also use LEFT JOIN clause to - /// retrieve information from other tables, e.g. ipv6_reservations. - /// Whether IPv6 reservations are assigned to the @ref Host objects - /// depends on the type of the exchange object. + /// retrieve information from other tables, e.g. ipv6_reservations, + /// dhcp4_options and dhcp6_options. + /// Whether IPv6 reservations and/or options are assigned to the + /// @ref Host objects depends on the type of the exchange object. /// /// @param stindex Statement index. /// @param bind Pointer to an array of MySQL bindings. @@ -1715,16 +1896,16 @@ public: boost::shared_ptr exchange) const; /// @brief Pointer to the object representing an exchange which - /// can be used to retrieve DHCPv4 reservation. + /// can be used to retrieve hosts and DHCPv4 options. boost::shared_ptr host_exchange_; /// @brief Pointer to an object representing an exchange which can - /// be used to retrieve DHCPv6 reservations. + /// be used to retrieve hosts, DHCPv6 options and IPv6 reservations. boost::shared_ptr host_ipv6_exchange_; /// @brief Pointer to an object representing an exchange which can - /// be used to retrieve DHCPv4 and DHCPv6 reservations using a - /// single query. + /// be used to retrieve hosts, DHCPv4 and DHCPv6 options, and + /// IPv6 reservations using a single query. boost::shared_ptr host_ipv46_exchange_; /// @brief Pointer to an object representing an exchange which can @@ -1732,8 +1913,9 @@ public: boost::shared_ptr host_ipv6_reservation_exchange_; /// @brief Pointer to an object representing an exchange which can - /// be used to insert DHCPv4 option into 'dhcp4_options' table. - boost::shared_ptr host_ipv4_option_exchange_; + /// be used to insert DHCPv4 or DHCPv6 option into dhcp4_options + /// or dhcp6_options table. + boost::shared_ptr host_option_exchange_; /// @brief MySQL connection MySqlConnection conn_; @@ -1747,7 +1929,7 @@ MySqlHostDataSourceImpl(const MySqlConnection::ParameterMap& parameters) host_ipv46_exchange_(new MySqlHostIPv6Exchange(MySqlHostExchangeOpts:: DHCP4_AND_DHCP6)), host_ipv6_reservation_exchange_(new MySqlIPv6ReservationExchange()), - host_ipv4_option_exchange_(new MySqlOptionExchange()), + host_option_exchange_(new MySqlOptionExchange()), conn_(parameters) { // Open the database. @@ -1819,8 +2001,8 @@ MySqlHostDataSourceImpl::addOption(const MySqlHostDataSource::StatementIndex& st const OptionalValue& subnet_id, const HostID& id) { std::vector bind = - host_ipv4_option_exchange_->createBindForSend(opt_desc, opt_space, - subnet_id, id); + host_option_exchange_->createBindForSend(opt_desc, opt_space, + subnet_id, id); addQuery(stindex, bind); } @@ -1829,6 +2011,8 @@ void MySqlHostDataSourceImpl::addOptions(const MySqlHostDataSource::StatementIndex& stindex, const ConstCfgOptionPtr& options_cfg, uint64_t& host_id) { + // Get option space names and vendor space names and combine them within a + // single list. std::list option_spaces = options_cfg->getOptionSpaceNames(); std::list vendor_spaces = options_cfg->getVendorIdsSpaceNames(); option_spaces.insert(option_spaces.end(), vendor_spaces.begin(), @@ -1839,6 +2023,9 @@ MySqlHostDataSourceImpl::addOptions(const MySqlHostDataSource::StatementIndex& s if ((host_id == 0) && !option_spaces.empty()) { host_id = mysql_insert_id(conn_.mysql_); } + + // For each option space retrieve all options and insert them into the + // database. for (std::list::const_iterator space = option_spaces.begin(); space != option_spaces.end(); ++space) { OptionContainerPtr options = options_cfg->getAll(*space); @@ -1982,6 +2169,10 @@ MySqlHostDataSource::~MySqlHostDataSource() { void MySqlHostDataSource::add(const HostPtr& host) { + // Initiate MySQL transaction as we will have to make multiple queries + // to insert host information into multiple tables. If that fails on + // any stage, the transaction will be rolled back by the destructor of + // the MySqlTransaction class. MySqlTransaction transaction(impl_->conn_); // Create the MYSQL_BIND array for the host @@ -1993,6 +2184,7 @@ MySqlHostDataSource::add(const HostPtr& host) { // Gets the last inserted hosts id uint64_t host_id = 0; + // Insert IPv6 reservations. IPv6ResrvRange v6resv = host->getIPv6Reservations(); if (std::distance(v6resv.first, v6resv.second) > 0) { host_id = mysql_insert_id(impl_->conn_.mysql_); @@ -2002,16 +2194,19 @@ MySqlHostDataSource::add(const HostPtr& host) { } } + // Insert DHCPv4 options. ConstCfgOptionPtr cfg_option4 = host->getCfgOption4(); if (cfg_option4) { impl_->addOptions(INSERT_V4_OPTION, cfg_option4, host_id); } + // Insert DHCPv6 options. ConstCfgOptionPtr cfg_option6 = host->getCfgOption6(); if (cfg_option6) { impl_->addOptions(INSERT_V6_OPTION, cfg_option6, host_id); } + // Everything went fine, so explicitly commit the transaction. transaction.commit(); } diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc index 7c0d8c363e..415cb7a4a2 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.cc @@ -253,6 +253,7 @@ void GenericHostDataSourceTest::compareHosts(const ConstHostPtr& host1, compareClientClasses(host1->getClientClasses6(), host2->getClientClasses6()); + // Compare DHCPv4 and DHCPv6 options. compareOptions(host1->getCfgOption4(), host2->getCfgOption4()); compareOptions(host1->getCfgOption6(), host2->getCfgOption6()); } @@ -339,27 +340,41 @@ GenericHostDataSourceTest::compareOptions(const ConstCfgOptionPtr& cfg1, ASSERT_TRUE(cfg1); ASSERT_TRUE(cfg2); + // Combine option space names with vendor space names in a single list. std::list option_spaces = cfg2->getOptionSpaceNames(); std::list vendor_spaces = cfg2->getVendorIdsSpaceNames(); option_spaces.insert(option_spaces.end(), vendor_spaces.begin(), vendor_spaces.end()); + // Iterate over all option spaces existing in cfg2. BOOST_FOREACH(std::string space, option_spaces) { + // Retrieve options belonging to the current option space. OptionContainerPtr options1 = cfg1->getAll(space); OptionContainerPtr options2 = cfg2->getAll(space); - ASSERT_TRUE(options1); - ASSERT_TRUE(options2); + ASSERT_TRUE(options1) << "failed for option space " << space; + ASSERT_TRUE(options2) << "failed for option space " << space; - ASSERT_EQ(options1->size(), options2->size()); + // If number of options doesn't match, the test fails. + ASSERT_EQ(options1->size(), options2->size()) + << "failed for option space " << space; + // Iterate over all options within this option space. BOOST_FOREACH(OptionDescriptor desc1, *options1) { OptionDescriptor desc2 = cfg2->get(space, desc1.option_->getType()); - ASSERT_EQ(desc1.persistent_, desc2.persistent_); - ASSERT_EQ(desc1.formatted_value_, desc2.formatted_value_); + // Compare persistent flag. + EXPECT_EQ(desc1.persistent_, desc2.persistent_) + << "failed for option " << space << "." << desc1.option_->getType(); + // Compare formatted value. + EXPECT_EQ(desc1.formatted_value_, desc2.formatted_value_) + << "failed for option " << space << "." << desc1.option_->getType(); + + // Retrieve options. Option* option1 = desc1.option_.get(); Option* option2 = desc2.option_.get(); - ASSERT_TRUE(typeid(*option1) == typeid(*option2)) + // Options must be represented by the same C++ class derived from + // the Option class. + EXPECT_TRUE(typeid(*option1) == typeid(*option2)) << "Comapared DHCP options, having option code " << desc1.option_->getType() << " and belonging to the " << space << " option space, are represented " @@ -367,13 +382,18 @@ GenericHostDataSourceTest::compareOptions(const ConstCfgOptionPtr& cfg1, << typeid(*option1).name() << " vs " << typeid(*option2).name(); + // Because we use different C++ classes to represent different + // options, the simplest way to make sure that the options are + // equal is to simply compare them in wire format. OutputBuffer buf1(option1->len()); ASSERT_NO_THROW(option1->pack(buf1)); OutputBuffer buf2(option2->len()); ASSERT_NO_THROW(option2->pack(buf2)); - ASSERT_EQ(buf1.getLength(), buf2.getLength()); - ASSERT_EQ(0, memcmp(buf1.getData(), buf2.getData(), buf1.getLength())); + ASSERT_EQ(buf1.getLength(), buf2.getLength()) + << "failed for option " << space << "." << desc1.option_->getType(); + EXPECT_EQ(0, memcmp(buf1.getData(), buf2.getData(), buf1.getLength())) + << "failed for option " << space << "." << desc1.option_->getType(); } } } @@ -397,6 +417,8 @@ GenericHostDataSourceTest::createVendorOption(const Option::Universe& universe, std::ostringstream s; if (formatted) { + // Vendor id comprises vendor-id field, for which we need to + // assign a value in the textual (formatted) format. s << vendor_id; } @@ -467,6 +489,9 @@ GenericHostDataSourceTest::addTestOptions(const HostPtr& host, "ipv6-address", true)), "isc2"); + // Register created "runtime" option definitions. They will be used by a + // host data source to convert option data into the appropriate option + // classes when the options are retrieved. LibDHCP::setRuntimeOptionDefs(defs); } @@ -968,37 +993,6 @@ void GenericHostDataSourceTest::testAddDuplicate4() { EXPECT_NO_THROW(hdsptr_->add(host)); } -void GenericHostDataSourceTest::testAddRollback() { - // Make sure we have the pointer to the host data source. - ASSERT_TRUE(hdsptr_); - - MySqlConnection::ParameterMap params; - params["name"] = "keatest"; - params["user"] = "keatest"; - params["password"] = "keatest"; - MySqlConnection conn(params); - ASSERT_NO_THROW(conn.openDatabase()); - int status = mysql_query(conn.mysql_, "DROP TABLE IF EXISTS ipv6_reservations"); - ASSERT_EQ(0, status) << mysql_error(conn.mysql_); - - // Create a host reservations. - HostPtr host = initializeHost6("2001:db8:1::1", Host::IDENT_HWADDR, false); - - host->setIPv4SubnetID(SubnetID(4)); - - // Create IPv6 reservation (for an address) and add it to the host - IPv6Resrv resv(IPv6Resrv::TYPE_NA, IOAddress("2001:db8::2"), 128); - host->addReservation(resv); - - ASSERT_THROW(hdsptr_->add(host), DbOperationError); - - ConstHostPtr from_hds = hdsptr_->get4(host->getIPv4SubnetID(), - host->getIdentifierType(), - &host->getIdentifier()[0], - host->getIdentifier().size()); - ASSERT_FALSE(from_hds); -} - void GenericHostDataSourceTest::testAddr6AndPrefix(){ // Make sure we have the pointer to the host data source. ASSERT_TRUE(hdsptr_); diff --git a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h index 14a68e6071..1e8bb75845 100644 --- a/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h +++ b/src/lib/dhcpsrv/tests/generic_host_data_source_unittest.h @@ -131,24 +131,44 @@ public: /// @brief Compares options within two configurations. /// + /// This method uses gtest macros to signal errors. + /// /// @param cfg1 First configuration. /// @param cfg2 Second configuration. void compareOptions(const ConstCfgOptionPtr& cfg1, const ConstCfgOptionPtr& cfg2) const; + /// @brief Creates an opton descriptor holding an empty option. + /// + /// @param universe V4 or V6. + /// @param option_type Option type. + /// @param persist A boolean flag indicating if the option is always + /// returned to the client or only when requested. + /// + /// @return Descriptor holding an empty option. OptionDescriptor createEmptyOption(const Option::Universe& universe, const uint16_t option_type, const bool persist) const; /// @brief Creates an instance of the option for which it is possible to - /// specify universe, option type and value in the constructor. + /// specify universe, option type, persistence flag and value in + /// the constructor. /// /// Examples of options that can be created using this function are: /// - @ref OptionString /// - different variants of @ref OptionInt. /// - /// @param encapsulated_space - /// @return Instance of the option created. + /// @param universe V4 or V6. + /// @param option_type Option type. + /// @param persist A boolean flag indicating if the option is always + /// returned to the client or only when requested. + /// @param formatted A boolean value selecting if the formatted option + /// value should be used (if true), or binary value (if false). + /// @param value Option value to be assigned to the option. + /// @tparam OptionType Class encapsulating the option. + /// @tparam DataType Option value data type. + /// + /// @return Descriptor holding an instance of the option created. template OptionDescriptor createOption(const Option::Universe& universe, const uint16_t option_type, @@ -159,12 +179,31 @@ public: value)); std::ostringstream s; if (formatted) { + // Using formatted option value. Convert option value to a + // textual format. s << value; } OptionDescriptor desc(option, persist, s.str()); return (desc); } + /// @brief Creates an instance of the option for which it is possible to + /// specify option type, persistence flag and value in the constructor. + /// + /// Examples of options that can be created using this function are: + /// - @ref Option4AddrLst + /// - @ref Option6AddrLst + /// + /// @param option_type Option type. + /// @param persist A boolean flag indicating if the option is always + /// returned to the client or only when requested. + /// @param formatted A boolean value selecting if the formatted option + /// value should be used (if true), or binary value (if false). + /// @param value Option value to be assigned to the option. + /// @tparam OptionType Class encapsulating the option. + /// @tparam DataType Option value data type. + /// + /// @return Descriptor holding an instance of the option created. template OptionDescriptor createOption(const uint16_t option_type, const bool persist, @@ -174,6 +213,8 @@ public: std::ostringstream s; if (formatted) { + // Using formatted option value. Convert option value to a + // textual format. s << value; } @@ -181,6 +222,22 @@ public: return (desc); } + /// @brief Creates an instance of the option holding list of IP addresses. + /// + /// @param option_type Option type. + /// @param persist A boolean flag indicating if the option is always + /// returned to the client or only when requested. + /// @param formatted A boolean value selecting if the formatted option + /// value should be used (if true), or binary value (if false). + /// @param address1 First address to be included. If address is empty, it is + /// not included. + /// @param address2 Second address to be included. If address is empty, it + /// is not included. + /// @param address3 Third address to be included. If address is empty, it + /// is not included. + /// @tparam OptionType Class encapsulating the option. + /// + /// @return Descriptor holding an instance of the option created. template OptionDescriptor createAddressOption(const uint16_t option_type, @@ -190,6 +247,7 @@ public: const std::string& address2 = "", const std::string& address3 = "") const { std::ostringstream s; + // First address. typename OptionType::AddressContainer addresses; if (!address1.empty()) { addresses.push_back(asiolink::IOAddress(address1)); @@ -197,6 +255,7 @@ public: s << address1; } } + // Second address. if (!address2.empty()) { addresses.push_back(asiolink::IOAddress(address2)); if (formatted) { @@ -206,6 +265,7 @@ public: s << address2; } } + // Third address. if (!address3.empty()) { addresses.push_back(asiolink::IOAddress(address3)); if (formatted) { @@ -216,16 +276,49 @@ public: } } - boost::shared_ptr option(new OptionType(option_type, addresses)); + boost::shared_ptr option(new OptionType(option_type, + addresses)); OptionDescriptor desc(option, persist, s.str()); return (desc); } + /// @brief Creates an instance of the vendor option. + /// + /// @param universe V4 or V6. + /// @param persist A boolean flag indicating if the option is always + /// returned to the client or only when requested. + /// @param formatted A boolean value selecting if the formatted option + /// value should be used (if true), or binary value (if false). + /// @param vendor_id Vendor identifier. + /// + /// @return Descriptor holding an instance of the option created. OptionDescriptor createVendorOption(const Option::Universe& universe, const bool persist, const bool formatted, const uint32_t vendor_id) const; + /// @brief Adds multiple options into the host. + /// + /// This method creates the following options into the host object: + /// - DHCPv4 boot file name option, + /// - DHCPv4 default ip ttl option, + /// - DHCPv4 option 1 within vendor-encapsulated-options space, + /// - DHCPv4 option 254 with a single IPv4 address, + /// - DHCPv4 option 1 within isc option space, + /// - DHCPv6 boot file url option, + /// - DHCPv6 information refresh time option, + /// - DHCPv6 vendor option with vendor id 2495, + /// - DHCPv6 option 1024, with a sigle IPv6 address, + /// - DHCPv6 empty option 1, within isc2 option space, + /// - DHCPv6 option 2, within isc2 option space with 3 IPv6 addresses, + /// + /// This method also creates option definitions for the non-standard + /// options and registers them in the LibDHCP as runtime option + /// definitions. + /// + /// @param host Host object into which options should be added. + /// @param formatted A boolean value selecting if the formatted option + /// value should be used (if true), or binary value (if false). void addTestOptions(const HostPtr& host, const bool formatted) const; /// @brief Pointer to the host data source @@ -342,8 +435,6 @@ public: /// Uses gtest macros to report failures. void testAddDuplicate4(); - void testAddRollback(); - /// @brief Test that DHCPv4 options can be inserted and retrieved from /// the database. /// diff --git a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc index 64801a6c6d..7f93e3149f 100644 --- a/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc +++ b/src/lib/dhcpsrv/tests/mysql_host_data_source_unittest.cc @@ -437,13 +437,56 @@ TEST_F(MySqlHostDataSourceTest, formattedOptionsReservations6) { } // This test verifies that DHCPv4 and DHCPv6 options can be inserted in a -/// textual format and retrieved with a single query to the database. +// textual format and retrieved with a single query to the database. TEST_F(MySqlHostDataSourceTest, formattedOptionsReservations46) { testOptionsReservations46(true); } +// This test checks transactional insertion of the host information +// into the database. The failure to insert host information at +// any stage should cause the whole transaction to be rolled back. TEST_F(MySqlHostDataSourceTest, testAddRollback) { - testAddRollback(); + // Make sure we have the pointer to the host data source. + ASSERT_TRUE(hdsptr_); + + // To test the transaction rollback mechanism we need to cause the + // insertion of host information to fail at some stage. The 'hosts' + // table should be updated correctly but the failure should occur + // when inserting reservations or options. The simplest way to + // achieve that is to simply drop one of the tables. To do so, we + // connect to the database and issue a DROP query. + MySqlConnection::ParameterMap params; + params["name"] = "keatest"; + params["user"] = "keatest"; + params["password"] = "keatest"; + MySqlConnection conn(params); + ASSERT_NO_THROW(conn.openDatabase()); + int status = mysql_query(conn.mysql_, + "DROP TABLE IF EXISTS ipv6_reservations"); + ASSERT_EQ(0, status) << mysql_error(conn.mysql_); + + // Create a host with a reservation. + HostPtr host = initializeHost6("2001:db8:1::1", Host::IDENT_HWADDR, false); + // Let's assign some DHCPv4 subnet to the host, because we will use the + // DHCPv4 subnet to try to retrieve the host after failed insertion. + host->setIPv4SubnetID(SubnetID(4)); + + // There is no ipv6_reservations table, so the insertion should fail. + ASSERT_THROW(hdsptr_->add(host), DbOperationError); + + // Even though we have created a DHCPv6 host, we can't use get6() + // method to retrieve the host from the database, because the + // query would expect that the ipv6_reservations table is present. + // Therefore, the query would fail. Instead, we use the get4 method + // which uses the same client identifier, but doesn't attempt to + // retrieve the data from ipv6_reservations table. The query should + // pass but return no host because the (insert) transaction is expected + // to be rolled back. + ConstHostPtr from_hds = hdsptr_->get4(host->getIPv4SubnetID(), + host->getIdentifierType(), + &host->getIdentifier()[0], + host->getIdentifier().size()); + EXPECT_FALSE(from_hds); } }; // Of anonymous namespace