Commit 72bc8cc9 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[#642,!373] Addressed review comments.

- Don't allow for deleting logical server 'all'.
- Additional tests to make sure that other servers aren't affected by deletion.
- Added note that getAll() doesn't return logical server all.
parent 9346fb88
......@@ -2846,7 +2846,7 @@ MySqlConfigBackendDHCPv4::deleteServer4(const ServerTag& server_tag) {
.arg(server_tag.get());
uint64_t result = impl_->deleteServer(MySqlConfigBackendDHCPv4Impl::CREATE_AUDIT_REVISION,
MySqlConfigBackendDHCPv4Impl::DELETE_SERVER4,
server_tag.get());
server_tag);
LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER4_RESULT)
.arg(result);
return (result);
......
......@@ -227,6 +227,9 @@ public:
/// @brief Retrieves all servers.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @return Collection of servers from the backend.
virtual db::ServerCollection
getAllServers4() const;
......@@ -480,6 +483,8 @@ public:
///
/// @param server_tag Tag of the server to be deleted.
/// @return Number of deleted servers.
/// @throw isc::InvalidOperation when trying to delete the logical
/// server 'all'.
virtual uint64_t
deleteServer4(const data::ServerTag& server_tag);
......
......@@ -3233,7 +3233,7 @@ MySqlConfigBackendDHCPv6::deleteServer6(const ServerTag& server_tag) {
.arg(server_tag.get());
uint64_t result = impl_->deleteServer(MySqlConfigBackendDHCPv6Impl::CREATE_AUDIT_REVISION,
MySqlConfigBackendDHCPv6Impl::DELETE_SERVER6,
server_tag.get());
server_tag);
LOG_DEBUG(mysql_cb_logger, DBGLVL_TRACE_BASIC, MYSQL_CB_DELETE_SERVER6_RESULT)
.arg(result);
return (result);
......
......@@ -227,6 +227,9 @@ public:
/// @brief Retrieves all servers.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @return Collection of servers from the backend.
virtual db::ServerCollection
getAllServers6() const;
......@@ -513,6 +516,8 @@ public:
///
/// @param server_tag Tag of the server to be deleted.
/// @return Number of deleted servers.
/// @throw isc::InvalidOperation when trying to delete the logical
/// server 'all'.
virtual uint64_t
deleteServer6(const data::ServerTag& server_tag);
......
......@@ -964,7 +964,14 @@ MySqlConfigBackendImpl::createUpdateServer(const int& create_audit_revision,
uint64_t
MySqlConfigBackendImpl::deleteServer(const int& create_audit_revision,
const int& delete_index,
const std::string& server_tag) {
const ServerTag& server_tag) {
// It is not allowed to delete 'all' logical server.
if (server_tag.amAll()) {
isc_throw(InvalidOperation, "'all' is a name reserved for the server tag which"
" associates the configuration elements with all servers connecting"
" to the database and can't be deleted");
}
MySqlTransaction transaction(conn_);
......@@ -976,7 +983,7 @@ MySqlConfigBackendImpl::deleteServer(const int& create_audit_revision,
// Specify which server should be deleted.
MySqlBindingCollection in_bindings = {
MySqlBinding::createString(server_tag)
MySqlBinding::createString(server_tag.get())
};
// Attempt to delete the server.
......
......@@ -655,8 +655,10 @@ public:
/// @param create_index Index of the DELETE query to be executed.
/// @param server_tag Tag of the server to be deleted.
/// @return Number of deleted servers.
/// @throw isc::InvalidOperation when trying to delete the logical
/// server 'all'.
uint64_t deleteServer(const int& create_audit_revision, const int& index,
const std::string& server_tag);
const data::ServerTag& server_tag);
/// @brief Attempts to delete all servers.
///
......
......@@ -506,6 +506,12 @@ TEST_F(MySqlConfigBackendDHCPv4Test, createUpdateDeleteServer) {
EXPECT_NO_THROW(servers_deleted = cbptr_->deleteServer4(ServerTag("server2")));
EXPECT_EQ(0, servers_deleted);
// Make sure that the server1 wasn't deleted.
EXPECT_NO_THROW(returned_server = cbptr_->getServer4(ServerTag("server1")));
EXPECT_TRUE(returned_server);
// Deleting logical server 'all' is not allowed.
EXPECT_THROW(cbptr_->deleteServer4(ServerTag()), isc::InvalidOperation);
// Delete the existing server.
EXPECT_NO_THROW(servers_deleted = cbptr_->deleteServer4(ServerTag("server1")));
......
......@@ -547,6 +547,12 @@ TEST_F(MySqlConfigBackendDHCPv6Test, createUpdateDeleteServer) {
EXPECT_NO_THROW(servers_deleted = cbptr_->deleteServer6(ServerTag("server2")));
EXPECT_EQ(0, servers_deleted);
// Make sure that the server1 wasn't deleted.
EXPECT_NO_THROW(returned_server = cbptr_->getServer6(ServerTag("server1")));
EXPECT_TRUE(returned_server);
// Deleting logical server 'all' is not allowed.
EXPECT_THROW(cbptr_->deleteServer6(ServerTag()), isc::InvalidOperation);
// Delete the existing server.
EXPECT_NO_THROW(servers_deleted = cbptr_->deleteServer6(ServerTag("server1")));
......
......@@ -231,6 +231,9 @@ public:
/// @brief Retrieves all servers.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @return Collection of servers from the backend.
virtual db::ServerCollection
getAllServers4() const = 0;
......
......@@ -232,6 +232,9 @@ public:
/// @brief Retrieves all servers.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @return Collection of servers from the backend.
virtual db::ServerCollection
getAllServers6() const = 0;
......
......@@ -235,6 +235,9 @@ public:
/// @brief Retrieves all servers from the particular backend.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @param backend_selector Backend selector.
/// @return Collection of servers from the backend.
virtual db::ServerCollection
......
......@@ -234,6 +234,9 @@ public:
/// @brief Retrieves all servers from the particular backend.
///
/// This method returns the list of servers excluding the logical server
/// 'all'.
///
/// @param backend_selector Backend selector.
/// @return Collection of servers from the backend.
virtual db::ServerCollection
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment