From: Francis Dupont Date: Fri, 16 Mar 2018 13:42:24 +0000 (+0100) Subject: Checkpoint: addressed final comment but not yet tested X-Git-Tag: trac5458a_base~29^2~3 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=d6a49e1f2fb97552c3481294acbbafb18b4d2f99;p=thirdparty%2Fkea.git Checkpoint: addressed final comment but not yet tested --- diff --git a/src/hooks/dhcp/lease_cmds/lease_cmds.cc b/src/hooks/dhcp/lease_cmds/lease_cmds.cc index c0d593534b..8fc6972af6 100644 --- a/src/hooks/dhcp/lease_cmds/lease_cmds.cc +++ b/src/hooks/dhcp/lease_cmds/lease_cmds.cc @@ -482,66 +482,50 @@ LeaseCmdsImpl::leaseGetAllHandler(CalloutHandle& handle) { // be returned. if (cmd_args_) { ConstElementPtr subnets = cmd_args_->get("subnets"); - if (subnets) { - if (subnets->getType() != Element::list) { - isc_throw(BadValue, "'subnets' parameter must be a list"); + if (!subnets) { + isc_throw(BadValue, "'subnets' parameter not specified"); + } + if (subnets->getType() != Element::list) { + isc_throw(BadValue, "'subnets' parameter must be a list"); + } + + const std::vector& subnet_ids = subnets->listValue(); + for (auto subnet_id = subnet_ids.begin(); + subnet_id != subnet_ids.end(); + ++subnet_id) { + if ((*subnet_id)->getType() != Element::integer) { + isc_throw(BadValue, "listed subnet identifiers must be numbers"); } - const std::vector& subnet_ids = subnets->listValue(); - for (auto subnet_id = subnet_ids.begin(); subnet_id != subnet_ids.end(); - ++subnet_id) { - if ((*subnet_id)->getType() != Element::integer) { - isc_throw(BadValue, "listed subnet identifiers must be numbers"); + if (v4) { + Lease4Collection leases = + LeaseMgrFactory::instance().getLeases4((*subnet_id)->intValue()); + for (auto lease : leases) { + ElementPtr lease_json = lease->toElement(); + leases_json->add(lease_json); } - - if (v4) { - Lease4Collection leases = - LeaseMgrFactory::instance().getLeases4((*subnet_id)->intValue()); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); - leases_json->add(lease_json); - } - } else { - Lease6Collection leases = - LeaseMgrFactory::instance().getLeases6((*subnet_id)->intValue(), - Lease::TYPE_NA); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); - leases_json->add(lease_json); - } - - leases = LeaseMgrFactory::instance().getLeases6((*subnet_id)->intValue(), - Lease::TYPE_TA); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); - leases_json->add(lease_json); - } - - leases = LeaseMgrFactory::instance().getLeases6((*subnet_id)->intValue(), - Lease::TYPE_PD); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); - leases_json->add(lease_json); - } + } else { + Lease6Collection leases = + LeaseMgrFactory::instance().getLeases6((*subnet_id)->intValue()); + for (auto lease : leases) { + ElementPtr lease_json = lease->toElement(); + leases_json->add(lease_json); } } - - } else { - isc_throw(BadValue, "'subnets' parameter not specified"); } } else { // There is no 'subnets' argument so let's return all leases. if (v4) { Lease4Collection leases = LeaseMgrFactory::instance().getLeases4(); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); + for (auto lease : leases) { + ElementPtr lease_json = lease->toElement(); leases_json->add(lease_json); } } else { Lease6Collection leases = LeaseMgrFactory::instance().getLeases6(); - for (auto lease = leases.begin(); lease != leases.end(); ++lease) { - ElementPtr lease_json = (*lease)->toElement(); + for (auto lease : leases) { + ElementPtr lease_json = lease->toElement(); leases_json->add(lease_json); } } diff --git a/src/lib/dhcpsrv/cql_lease_mgr.cc b/src/lib/dhcpsrv/cql_lease_mgr.cc index fb114dfdf5..5dcdfd8f9b 100644 --- a/src/lib/dhcpsrv/cql_lease_mgr.cc +++ b/src/lib/dhcpsrv/cql_lease_mgr.cc @@ -1719,7 +1719,7 @@ CqlLeaseMgr::getLeases6(Lease::Type lease_type, const DUID &duid, uint32_t iaid, } Lease6Collection -CqlLeaseMgr::getLeases6(SubnetID, Lease::Type) const { +CqlLeaseMgr::getLeases6(SubnetID) const { isc_throw(NotImplemented, "getLeases6(subnet_id) is not implemented"); } diff --git a/src/lib/dhcpsrv/cql_lease_mgr.h b/src/lib/dhcpsrv/cql_lease_mgr.h index dde931a103..182ce060c6 100644 --- a/src/lib/dhcpsrv/cql_lease_mgr.h +++ b/src/lib/dhcpsrv/cql_lease_mgr.h @@ -272,17 +272,14 @@ public: uint32_t iaid, SubnetID subnet_id) const override; - /// @brief Returns all IPv6 leases for the particular subnet identifier - /// and lease type. + /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type specifies lease type: (NA, TA or PD) /// /// @return Lease collection (may be empty if no IPv6 lease found). /// @throw NotImplemented because this method is currently not implemented for /// this backend. - virtual Lease6Collection getLeases6(SubnetID subnet_id, - Lease::Type type) const; + virtual Lease6Collection getLeases6(SubnetID subnet_id) const override; /// @brief Returns all IPv6 leases. /// diff --git a/src/lib/dhcpsrv/dhcpsrv_messages.mes b/src/lib/dhcpsrv/dhcpsrv_messages.mes index 2fc38426d5..8e3c7cfebd 100644 --- a/src/lib/dhcpsrv/dhcpsrv_messages.mes +++ b/src/lib/dhcpsrv/dhcpsrv_messages.mes @@ -514,10 +514,9 @@ lease from the memory file database for a client with the specified IAID A debug message issued when the server is attempting to obtain all IPv4 leases for a given subnet identifier from the memory file database. -% DHCPSRV_MEMFILE_GET_SUBID_TYPE6 obtaining IPv6 leases for subnet ID %1 and type %2 +% DHCPSRV_MEMFILE_GET_SUBID6 obtaining IPv6 leases for subnet ID %1 A debug message issued when the server is attempting to obtain all IPv6 -leases for a given subnet identifier and lease type from the memory file -database. +leases for a given subnet identifier from the memory file database. % DHCPSRV_MEMFILE_GET_SUBID_CLIENTID obtaining IPv4 lease for subnet ID %1 and client ID %2 A debug message issued when the server is attempting to obtain an IPv4 @@ -752,9 +751,9 @@ lease from the MySQL database for a client with the specified IAID A debug message issued when the server is attempting to obtain all IPv4 leases for a given subnet identifier from the MySQL database. -% DHCPSRV_MYSQL_GET_SUBID_TYPE6 obtaining IPv6 leases for subnet ID %1 and type %2 +% DHCPSRV_MYSQL_GET_SUBID6 obtaining IPv6 leases for subnet ID %1 A debug message issued when the server is attempting to obtain all IPv6 -leases for a given subnet identifier and lease type from the MySQL database. +leases for a given subnet identifier from the MySQL database. % DHCPSRV_MYSQL_GET_SUBID_CLIENTID obtaining IPv4 lease for subnet ID %1 and client ID %2 A debug message issued when the server is attempting to obtain an IPv4 @@ -922,10 +921,9 @@ lease from the PostgreSQL database for a client with the specified IAID A debug message issued when the server is attempting to obtain all IPv4 leases for a given subnet identifier from the PostgreSQL database. -% DHCPSRV_PGSQL_GET_SUBID_TYPE6 obtaining IPv6 leases for subnet ID %1 and type %2 +% DHCPSRV_PGSQL_GET_SUBID6 obtaining IPv6 leases for subnet ID %1 A debug message issued when the server is attempting to obtain all IPv6 -leases for a given subnet identifier and lease type from the PostgreSQL -database. +leases for a given subnet identifier from the PostgreSQL database. % DHCPSRV_PGSQL_GET_SUBID_CLIENTID obtaining IPv4 lease for subnet ID %1 and client ID %2 A debug message issued when the server is attempting to obtain an IPv4 diff --git a/src/lib/dhcpsrv/lease_mgr.h b/src/lib/dhcpsrv/lease_mgr.h index c4bbf48292..f7775845bd 100644 --- a/src/lib/dhcpsrv/lease_mgr.h +++ b/src/lib/dhcpsrv/lease_mgr.h @@ -341,15 +341,12 @@ public: Lease6Ptr getLease6(Lease::Type type, const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; - /// @brief Returns all IPv6 leases for the particular subnet identifier - /// and lease type. + /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type specifies lease type: (NA, TA or PD) /// /// @return Lease collection (may be empty if no IPv6 lease found). - virtual Lease6Collection getLeases6(SubnetID subnet_id, - Lease::Type type) const = 0; + virtual Lease6Collection getLeases6(SubnetID subnet_id) const = 0; /// @brief Returns all IPv6 leases. /// diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc index 871905b57f..61aaf577b1 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.cc +++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc @@ -832,10 +832,9 @@ Memfile_LeaseMgr::getLeases6(Lease::Type type, } Lease6Collection -Memfile_LeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MEMFILE_GET_SUBID_TYPE6) - .arg(subnet_id) - .arg(Lease::typeToText(type)); +Memfile_LeaseMgr::getLeases6(SubnetID subnet_id) const { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MEMFILE_GET_SUBID6) + .arg(subnet_id); Lease6Collection collection; const Lease6StorageSubnetIdIndex& idx = storage6_.get(); @@ -844,10 +843,7 @@ Memfile_LeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { idx.equal_range(subnet_id); for (auto lease = l.first; lease != l.second; ++lease) { - // Filter out the leases which lease type doesn't match. - if ((*lease)->type_ == type) { - collection.push_back(Lease6Ptr(new Lease6(**lease))); - } + collection.push_back(Lease6Ptr(new Lease6(**lease))); } return (collection); diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.h b/src/lib/dhcpsrv/memfile_lease_mgr.h index c42d799b91..0cb5bc829b 100644 --- a/src/lib/dhcpsrv/memfile_lease_mgr.h +++ b/src/lib/dhcpsrv/memfile_lease_mgr.h @@ -271,15 +271,12 @@ public: uint32_t iaid, SubnetID subnet_id) const; - /// @brief Returns all IPv6 leases for the particular subnet identifier - /// and lease type. + /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type specifies lease type: (NA, TA or PD) /// /// @return Lease collection (may be empty if no IPv6 lease found). - virtual Lease6Collection getLeases6(SubnetID subnet_id, - Lease::Type type) const; + virtual Lease6Collection getLeases6(SubnetID subnet_id) const; /// @brief Returns all IPv6 leases. /// diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.cc b/src/lib/dhcpsrv/mysql_lease_mgr.cc index 19dd8e44be..9f03556488 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.cc +++ b/src/lib/dhcpsrv/mysql_lease_mgr.cc @@ -189,7 +189,7 @@ tagged_statements = { { "FROM lease6 " "WHERE duid = ? AND iaid = ? AND subnet_id = ? " "AND lease_type = ?"}, - {MySqlLeaseMgr::GET_LEASE6_SUBID_TYPE, + {MySqlLeaseMgr::GET_LEASE6_SUBID, "SELECT address, duid, valid_lifetime, " "expire, subnet_id, pref_lifetime, " "lease_type, iaid, prefix_len, " @@ -197,7 +197,7 @@ tagged_statements = { { "hwaddr, hwtype, hwaddr_source, " "state " "FROM lease6 " - "WHERE subnet_id = ? and lease_type = ?"}, + "WHERE subnet_id = ?"}, {MySqlLeaseMgr::GET_LEASE6_EXPIRE, "SELECT address, duid, valid_lifetime, " "expire, subnet_id, pref_lifetime, " @@ -1927,14 +1927,12 @@ MySqlLeaseMgr::getLeases6(Lease::Type lease_type, } Lease6Collection -MySqlLeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, - DHCPSRV_MYSQL_GET_SUBID_TYPE6) - .arg(subnet_id) - .arg(Lease::typeToText(type)); +MySqlLeaseMgr::getLeases6(SubnetID subnet_id) const { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_MYSQL_GET_SUBID6) + .arg(subnet_id); // Set up the WHERE clause value - MYSQL_BIND inbind[2]; + MYSQL_BIND inbind[1]; memset(inbind, 0, sizeof(inbind)); // Subnet ID @@ -1942,14 +1940,9 @@ MySqlLeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { inbind[0].buffer = reinterpret_cast(&subnet_id); inbind[0].is_unsigned = MLM_TRUE; - // LEASE_TYPE - inbind[1].buffer_type = MYSQL_TYPE_TINY; - inbind[1].buffer = reinterpret_cast(&type); - inbind[1].is_unsigned = MLM_TRUE; - // ... and get the data Lease6Collection result; - getLeaseCollection(GET_LEASE6_SUBID_TYPE, inbind, result); + getLeaseCollection(GET_LEASE6_SUBID, inbind, result); return (result); } diff --git a/src/lib/dhcpsrv/mysql_lease_mgr.h b/src/lib/dhcpsrv/mysql_lease_mgr.h index bafafbd8c1..91265cc72b 100644 --- a/src/lib/dhcpsrv/mysql_lease_mgr.h +++ b/src/lib/dhcpsrv/mysql_lease_mgr.h @@ -268,15 +268,12 @@ public: virtual Lease6Collection getLeases6(Lease::Type type, const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; - /// @brief Returns all IPv6 leases for the particular subnet identifier - /// and lease type. + /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type specifies lease type: (NA, TA or PD) /// /// @return Lease collection (may be empty if no IPv6 lease found). - virtual Lease6Collection getLeases6(SubnetID subnet_id, - Lease::Type type) const; + virtual Lease6Collection getLeases6(SubnetID subnet_id) const; /// @brief Returns all IPv6 leases. /// @@ -455,7 +452,7 @@ public: GET_LEASE6_ADDR, // Get lease6 by address GET_LEASE6_DUID_IAID, // Get lease6 by DUID and IAID GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and subnet ID - GET_LEASE6_SUBID_TYPE, // Get IPv6 leases by subnet ID and type + GET_LEASE6_SUBID, // Get IPv6 leases by subnet ID GET_LEASE6_EXPIRE, // Get lease6 by expiration. GET_VERSION, // Obtain version number INSERT_LEASE4, // Add entry to lease4 table diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.cc b/src/lib/dhcpsrv/pgsql_lease_mgr.cc index 156a54290f..c9e68499dd 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.cc +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.cc @@ -168,15 +168,15 @@ PgSqlTaggedStatement tagged_statements[] = { "WHERE lease_type = $1 " "AND duid = $2 AND iaid = $3 AND subnet_id = $4"}, - // GET_LEASE6_SUBID_TYPE - { 2, { OID_INT8, OID_INT2 }, + // GET_LEASE6_SUBID + { 1, { OID_INT8 }, "get_lease6_subid", "SELECT address, duid, valid_lifetime, " "extract(epoch from expire)::bigint, subnet_id, pref_lifetime, " "lease_type, iaid, prefix_len, fqdn_fwd, fqdn_rev, hostname, " "state " "FROM lease6 " - "WHERE subnet_id = $1 AND lease_type = $2"}, + "WHERE subnet_id = $1"}, // GET_LEASE6_EXPIRE { 3, { OID_INT8, OID_TIMESTAMP, OID_INT8 }, @@ -1225,10 +1225,9 @@ PgSqlLeaseMgr::getLeases6(Lease::Type lease_type, const DUID& duid, } Lease6Collection -PgSqlLeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { - LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_GET_SUBID_TYPE6) - .arg(subnet_id) - .arg(Lease::typeToText(type)); +PgSqlLeaseMgr::getLeases6(SubnetID subnet_id) const { + LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL, DHCPSRV_PGSQL_GET_SUBID6) + .arg(subnet_id); // Set up the WHERE clause value PsqlBindArray bind_array; @@ -1237,13 +1236,9 @@ PgSqlLeaseMgr::getLeases6(SubnetID subnet_id, Lease::Type type) const { std::string subnet_id_str = boost::lexical_cast(subnet_id); bind_array.add(subnet_id_str); - // LEASE_TYPE - std::string lease_type_str = boost::lexical_cast(type); - bind_array.add(lease_type_str); - // ... and get the data Lease6Collection result; - getLeaseCollection(GET_LEASE6_SUBID_TYPE, bind_array, result); + getLeaseCollection(GET_LEASE6_SUBID, bind_array, result); return (result); } diff --git a/src/lib/dhcpsrv/pgsql_lease_mgr.h b/src/lib/dhcpsrv/pgsql_lease_mgr.h index 9f41350c9c..54ca6df8ec 100644 --- a/src/lib/dhcpsrv/pgsql_lease_mgr.h +++ b/src/lib/dhcpsrv/pgsql_lease_mgr.h @@ -242,15 +242,12 @@ public: virtual Lease6Collection getLeases6(Lease::Type type, const DUID& duid, uint32_t iaid, SubnetID subnet_id) const; - /// @brief Returns all IPv6 leases for the particular subnet identifier - /// and lease type. + /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type specifies lease type: (NA, TA or PD) /// /// @return Lease collection (may be empty if no IPv6 lease found). - virtual Lease6Collection getLeases6(SubnetID subnet_id, - Lease::Type type) const; + virtual Lease6Collection getLeases6(SubnetID subnet_id) const; /// @brief Returns all IPv6 leases. /// @@ -446,7 +443,7 @@ public: GET_LEASE6_ADDR, // Get lease6 by address GET_LEASE6_DUID_IAID, // Get lease6 by DUID and IAID GET_LEASE6_DUID_IAID_SUBID, // Get lease6 by DUID, IAID and subnet ID - GET_LEASE6_SUBID_TYPE, // Get IPv6 leases by subnet ID and type + GET_LEASE6_SUBID, // Get IPv6 leases by subnet ID GET_LEASE6_EXPIRE, // Get expired lease6 GET_VERSION, // Obtain version number INSERT_LEASE4, // Add entry to lease4 table diff --git a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc index 8ff4c4ecdd..1ec1ed870c 100644 --- a/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/generic_lease_mgr_unittest.cc @@ -1255,14 +1255,9 @@ GenericLeaseMgrTest::testGetLeases6SubnetId() { } // There should be exactly two leases for the subnet id that the second - // lease belongs to, one TA and one PD. - Lease6Collection returned = - lmptr_->getLeases6(leases[1]->subnet_id_, Lease::TYPE_NA); - EXPECT_EQ(0, returned.size()); - returned = lmptr_->getLeases6(leases[1]->subnet_id_,Lease::TYPE_TA); - EXPECT_EQ(1, returned.size()); - returned = lmptr_->getLeases6(leases[1]->subnet_id_,Lease::TYPE_PD); - EXPECT_EQ(1, returned.size()); + // lease belongs to. + Lease6Collection returned = lmptr_->getLeases6(leases[1]->subnet_id_); + EXPECT_EQ(2, returned.size()); } void diff --git a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc index 3a20e7b58e..88ea5a064a 100644 --- a/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc +++ b/src/lib/dhcpsrv/tests/lease_mgr_unittest.cc @@ -181,10 +181,9 @@ public: /// @brief Returns all IPv6 leases for the particular subnet identifier. /// /// @param subnet_id subnet identifier. - /// @param type lease type. /// /// @return Lease collection (may be empty if no IPv6 lease found). - virtual Lease6Collection getLeases6(SubnetID, Lease::Type) const { + virtual Lease6Collection getLeases6(SubnetID) const { return (Lease6Collection()); }