From 0f11e728a3d127d18761c3bfa1174bb50e8a4ebf Mon Sep 17 00:00:00 2001
From: Steven Murray <Steven.Murray@cern.ch>
Date: Tue, 7 May 2019 00:14:12 +0200
Subject: [PATCH] cta/CTA#490 Deleting non empty pool gives unfriendly message:
Fixed.
---
catalogue/CatalogueTest.cpp | 85 ++++++++++++++++++++++++++++++++++++
catalogue/RdbmsCatalogue.cpp | 44 ++++++++++++++++---
catalogue/RdbmsCatalogue.hpp | 11 +++++
3 files changed, 134 insertions(+), 6 deletions(-)
diff --git a/catalogue/CatalogueTest.cpp b/catalogue/CatalogueTest.cpp
index bfed77f490..fbe77e6b42 100644
--- a/catalogue/CatalogueTest.cpp
+++ b/catalogue/CatalogueTest.cpp
@@ -879,6 +879,91 @@ TEST_P(cta_catalogue_CatalogueTest, deleteTapePool) {
ASSERT_TRUE(m_catalogue->getTapePools().empty());
}
+TEST_P(cta_catalogue_CatalogueTest, deleteTapePool_notEmpty) {
+ using namespace cta;
+
+ ASSERT_TRUE(m_catalogue->getTapes().empty());
+
+ const std::string vid = "vid";
+
+ ASSERT_FALSE(m_catalogue->tapeExists(vid));
+
+ const std::string mediaType = "media_type";
+ const std::string vendor = "vendor";
+ const std::string logicalLibraryName = "logical_library_name";
+ const std::string tapePoolName = "tape_pool_name";
+ const std::string vo = "vo";
+ const uint64_t capacityInBytes = (uint64_t)10 * 1000 * 1000 * 1000 * 1000;
+ const bool disabledValue = true;
+ const bool fullValue = false;
+ const std::string comment = "Create tape";
+
+ m_catalogue->createLogicalLibrary(m_admin, logicalLibraryName,
+ "Create logical library");
+
+ m_catalogue->createTapePool(m_admin, tapePoolName, vo, 2, true, "Create tape pool");
+ {
+ const auto pools = m_catalogue->getTapePools();
+ ASSERT_EQ(1, pools.size());
+
+ const auto &pool = pools.front();
+ ASSERT_EQ(tapePoolName, pool.name);
+ ASSERT_EQ(vo, pool.vo);
+ ASSERT_EQ(0, pool.nbTapes);
+ ASSERT_EQ(0, pool.capacityBytes);
+ ASSERT_EQ(0, pool.dataBytes);
+ ASSERT_EQ(0, pool.nbPhysicalFiles);
+ }
+
+ m_catalogue->createTape(m_admin, vid, mediaType, vendor, logicalLibraryName, tapePoolName, capacityInBytes,
+ disabledValue, fullValue, comment);
+
+ ASSERT_TRUE(m_catalogue->tapeExists(vid));
+
+ const auto tapes = m_catalogue->getTapes();
+
+ ASSERT_EQ(1, tapes.size());
+
+ {
+ const auto tape = tapes.front();
+ ASSERT_EQ(vid, tape.vid);
+ ASSERT_EQ(mediaType, tape.mediaType);
+ ASSERT_EQ(vendor, tape.vendor);
+ ASSERT_EQ(logicalLibraryName, tape.logicalLibraryName);
+ ASSERT_EQ(tapePoolName, tape.tapePoolName);
+ ASSERT_EQ(vo, tape.vo);
+ ASSERT_EQ(capacityInBytes, tape.capacityInBytes);
+ ASSERT_TRUE(disabledValue == tape.disabled);
+ ASSERT_TRUE(fullValue == tape.full);
+ ASSERT_EQ(comment, tape.comment);
+ ASSERT_FALSE(tape.labelLog);
+ ASSERT_FALSE(tape.lastReadLog);
+ ASSERT_FALSE(tape.lastWriteLog);
+
+ const auto creationLog = tape.creationLog;
+ ASSERT_EQ(m_admin.username, creationLog.username);
+ ASSERT_EQ(m_admin.host, creationLog.host);
+
+ const auto lastModificationLog = tape.lastModificationLog;
+ ASSERT_EQ(creationLog, lastModificationLog);
+ }
+
+ {
+ const auto pools = m_catalogue->getTapePools();
+ ASSERT_EQ(1, pools.size());
+
+ const auto &pool = pools.front();
+ ASSERT_EQ(tapePoolName, pool.name);
+ ASSERT_EQ(vo, pool.vo);
+ ASSERT_EQ(1, pool.nbTapes);
+ ASSERT_EQ(capacityInBytes, pool.capacityBytes);
+ ASSERT_EQ(0, pool.dataBytes);
+ ASSERT_EQ(0, pool.nbPhysicalFiles);
+ }
+
+ ASSERT_THROW(m_catalogue->deleteTapePool(tapePoolName), exception::UserError);
+}
+
TEST_P(cta_catalogue_CatalogueTest, createTapePool_emptyStringTapePoolName) {
using namespace cta;
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index 461cb707c7..fadc2e849c 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -858,14 +858,20 @@ bool RdbmsCatalogue::archiveRouteExists(rdbms::Conn &conn, const std::string &di
//------------------------------------------------------------------------------
void RdbmsCatalogue::deleteTapePool(const std::string &name) {
try {
- const char *const sql = "DELETE FROM TAPE_POOL WHERE TAPE_POOL_NAME = :TAPE_POOL_NAME";
auto conn = m_connPool.getConn();
- auto stmt = conn.createStmt(sql);
- stmt.bindString(":TAPE_POOL_NAME", name);
- stmt.executeNonQuery();
+ const uint64_t nbTapesInPool = getNbTapesInPool(conn, name);
- if(0 == stmt.getNbAffectedRows()) {
- throw exception::UserError(std::string("Cannot delete tape-pool ") + name + " because it does not exist");
+ if(0 == nbTapesInPool) {
+ const char *const sql = "DELETE FROM TAPE_POOL WHERE TAPE_POOL_NAME = :TAPE_POOL_NAME";
+ auto stmt = conn.createStmt(sql);
+ stmt.bindString(":TAPE_POOL_NAME", name);
+ stmt.executeNonQuery();
+
+ if(0 == stmt.getNbAffectedRows()) {
+ throw exception::UserError(std::string("Cannot delete tape-pool ") + name + " because it does not exist");
+ }
+ } else {
+ throw exception::UserError(std::string("Cannot delete tape-pool ") + name + " because it is not empty");
}
} catch(exception::UserError &) {
throw;
@@ -5338,6 +5344,32 @@ void RdbmsCatalogue::checkTapeItemWrittenFieldsAreSet(const std::string& calling
}
}
+//------------------------------------------------------------------------------
+// getNbTapesInPool
+//------------------------------------------------------------------------------
+uint64_t RdbmsCatalogue::getNbTapesInPool(rdbms::Conn &conn, const std::string &name) const {
+ try {
+ const char *const sql =
+ "SELECT "
+ "COUNT(*) AS NB_TAPES "
+ "FROM "
+ "TAPE "
+ "WHERE "
+ "TAPE.TAPE_POOL_NAME = :TAPE_POOL_NAME";
+ auto stmt = conn.createStmt(sql);
+ stmt.bindString(":TAPE_POOL_NAME", name);
+ auto rset = stmt.executeQuery();
+ if(!rset.next()) {
+ throw exception::Exception("Result set of SELECT COUNT(*) is empty");
+ }
+ return rset.columnUint64("NB_TAPES");
+ } catch(exception::UserError &) {
+ throw;
+ } catch(exception::Exception &ex) {
+ ex.getMessage().str(std::string(__FUNCTION__) + ": " + ex.getMessage().str());
+ throw;
+ }
+}
} // namespace catalogue
} // namespace cta
diff --git a/catalogue/RdbmsCatalogue.hpp b/catalogue/RdbmsCatalogue.hpp
index b08176540d..902fd78689 100644
--- a/catalogue/RdbmsCatalogue.hpp
+++ b/catalogue/RdbmsCatalogue.hpp
@@ -1209,6 +1209,17 @@ protected:
*/
bool isNonCachedAdmin(const common::dataStructures::SecurityIdentity &admin) const;
+ /**
+ * Returns the number of tapes in the specified tape pool.
+ *
+ * If the tape pool does not exist then this method returns 0.
+ *
+ * @param conn The database connection.
+ * @param name The name of the tape pool.
+ * @return The number of tapes in the specified tape pool.
+ */
+ uint64_t getNbTapesInPool(rdbms::Conn &conn, const std::string &name) const;
+
/**
* Cached versions of tape copy to tape tape pool mappings for specific
* storage classes.
--
GitLab