From 8176bc2c2af45191c16d23b0b243814e1234217c Mon Sep 17 00:00:00 2001
From: Steven Murray <Steven.Murray@cern.ch>
Date: Mon, 29 Jul 2019 14:10:04 +0200
Subject: [PATCH] Fixed cta/CTA#581 ORA-02292 when deleting a non-empty logical
 library

---
 catalogue/CMakeLists.txt                      |  2 +
 catalogue/CatalogueTest.cpp                   | 65 ++++++++++++++++++-
 catalogue/RdbmsCatalogue.cpp                  | 22 ++++++-
 .../UserSpecifiedANonEmptyLogicalLibrary.cpp  | 33 ++++++++++
 .../UserSpecifiedANonEmptyLogicalLibrary.hpp  | 46 +++++++++++++
 ...serSpecifiedANonExistentLogicalLibrary.cpp | 33 ++++++++++
 ...serSpecifiedANonExistentLogicalLibrary.hpp | 46 +++++++++++++
 7 files changed, 243 insertions(+), 4 deletions(-)
 create mode 100644 catalogue/UserSpecifiedANonEmptyLogicalLibrary.cpp
 create mode 100644 catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp
 create mode 100644 catalogue/UserSpecifiedANonExistentLogicalLibrary.cpp
 create mode 100644 catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp

diff --git a/catalogue/CMakeLists.txt b/catalogue/CMakeLists.txt
index 0ad2099073..b576c91ea9 100644
--- a/catalogue/CMakeLists.txt
+++ b/catalogue/CMakeLists.txt
@@ -56,7 +56,9 @@ set (CATALOGUE_LIB_SRC_FILES
   SqliteCatalogue.cpp
   SqliteCatalogueFactory.cpp
   TapeForWriting.cpp
+  UserSpecifiedANonEmptyLogicalLibrary.cpp
   UserSpecifiedANonEmptyTape.cpp
+  UserSpecifiedANonExistentLogicalLibrary.cpp
   UserSpecifiedANonExistentTape.cpp
   UserSpecifiedAnEmptyStringComment.cpp
   UserSpecifiedAnEmptyStringDiskInstanceName.cpp
diff --git a/catalogue/CatalogueTest.cpp b/catalogue/CatalogueTest.cpp
index 9241f81adf..9becc11bc1 100644
--- a/catalogue/CatalogueTest.cpp
+++ b/catalogue/CatalogueTest.cpp
@@ -18,7 +18,9 @@
 
 #include "catalogue/ArchiveFileRow.hpp"
 #include "catalogue/CatalogueTest.hpp"
+#include "catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp"
 #include "catalogue/UserSpecifiedANonEmptyTape.hpp"
+#include "catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp"
 #include "catalogue/UserSpecifiedANonExistentTape.hpp"
 #include "catalogue/UserSpecifiedAnEmptyStringComment.hpp"
 #include "catalogue/UserSpecifiedAnEmptyStringDiskInstanceName.hpp"
@@ -2468,7 +2470,68 @@ TEST_P(cta_catalogue_CatalogueTest, deleteLogicalLibrary_non_existant) {
   using namespace cta;
       
   ASSERT_TRUE(m_catalogue->getLogicalLibraries().empty());
-  ASSERT_THROW(m_catalogue->deleteLogicalLibrary("non_existant_logical_library"), exception::UserError);
+  ASSERT_THROW(m_catalogue->deleteLogicalLibrary("non_existant_logical_library"),
+    catalogue::UserSpecifiedANonExistentLogicalLibrary);
+}
+
+TEST_P(cta_catalogue_CatalogueTest, deleteLogicalLibrary_non_empty) {
+  using namespace cta;
+
+  ASSERT_TRUE(m_catalogue->getTapes().empty());
+
+  const std::string vid = "vid";
+  const std::string mediaType = "media_type";
+  const std::string vendor = "vendor";
+  const std::string logicalLibraryName = "logical_library_name";
+  const bool logicalLibraryIsDisabled= false;
+  const std::string tapePoolName = "tape_pool_name";
+  const std::string vo = "vo";
+  const uint64_t nbPartialTapes = 2;
+  const bool isEncrypted = true;
+  const cta::optional<std::string> supply("value for the supply pool mechanism");
+  const uint64_t capacityInBytes = (uint64_t)10 * 1000 * 1000 * 1000 * 1000;
+  const bool disabledValue = true;
+  const bool fullValue = false;
+  const bool readOnlyValue = true;
+  const std::string comment = "Create tape";
+
+  m_catalogue->createLogicalLibrary(m_admin, logicalLibraryName, logicalLibraryIsDisabled, "Create logical library");
+  m_catalogue->createTapePool(m_admin, tapePoolName, vo, nbPartialTapes, isEncrypted, supply, "Create tape pool");
+  m_catalogue->createTape(m_admin, vid, mediaType, vendor, logicalLibraryName, tapePoolName,
+    capacityInBytes, disabledValue, fullValue, readOnlyValue, 
+    comment);
+
+  const std::list<common::dataStructures::Tape> tapes =
+    m_catalogue->getTapes();
+
+  ASSERT_EQ(1, tapes.size());
+
+  const common::dataStructures::Tape 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_TRUE(readOnlyValue == tape.readOnly);
+  ASSERT_FALSE(tape.isFromCastor);
+  ASSERT_EQ(comment, tape.comment);
+  ASSERT_FALSE(tape.labelLog);
+  ASSERT_FALSE(tape.lastReadLog);
+  ASSERT_FALSE(tape.lastWriteLog);
+
+  const common::dataStructures::EntryLog creationLog = tape.creationLog;
+  ASSERT_EQ(m_admin.username, creationLog.username);
+  ASSERT_EQ(m_admin.host, creationLog.host);
+
+  const common::dataStructures::EntryLog lastModificationLog =
+    tape.lastModificationLog;
+  ASSERT_EQ(creationLog, lastModificationLog);
+
+  ASSERT_THROW(m_catalogue->deleteLogicalLibrary(logicalLibraryName), catalogue::UserSpecifiedANonEmptyLogicalLibrary);
 }
 
 TEST_P(cta_catalogue_CatalogueTest, modifyLogicalLibraryComment) {
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index 3b4a22a4e9..a5c2e975bb 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -22,7 +22,9 @@
 #include "catalogue/RdbmsCatalogueGetArchiveFilesForRepackItor.hpp"
 #include "catalogue/retryOnLostConnection.hpp"
 #include "catalogue/SqliteCatalogueSchema.hpp"
+#include "catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp"
 #include "catalogue/UserSpecifiedANonEmptyTape.hpp"
+#include "catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp"
 #include "catalogue/UserSpecifiedANonExistentTape.hpp"
 #include "catalogue/UserSpecifiedAnEmptyStringComment.hpp"
 #include "catalogue/UserSpecifiedAnEmptyStringDiskInstanceName.hpp"
@@ -1580,14 +1582,28 @@ bool RdbmsCatalogue::logicalLibraryExists(rdbms::Conn &conn, const std::string &
 //------------------------------------------------------------------------------
 void RdbmsCatalogue::deleteLogicalLibrary(const std::string &name) {
   try {
-    const char *const sql = "DELETE FROM LOGICAL_LIBRARY WHERE LOGICAL_LIBRARY_NAME = :LOGICAL_LIBRARY_NAME";
+    const char *const sql =
+      "DELETE "
+      "FROM LOGICAL_LIBRARY "
+      "WHERE "
+        "LOGICAL_LIBRARY_NAME = :LOGICAL_LIBRARY_NAME_1 AND "
+        "NOT EXISTS (SELECT LOGICAL_LIBRARY_NAME FROM TAPE WHERE LOGICAL_LIBRARY_NAME = :LOGICAL_LIBRARY_NAME_2)";
     auto conn = m_connPool.getConn();
     auto stmt = conn.createStmt(sql);
-    stmt.bindString(":LOGICAL_LIBRARY_NAME", name);
+    stmt.bindString(":LOGICAL_LIBRARY_NAME_1", name);
+    stmt.bindString(":LOGICAL_LIBRARY_NAME_2", name);
     stmt.executeNonQuery();
 
+    // The delete statement will effect no rows and will not raise an error if
+    // either the logical library does not exist or if it still contains tapes
     if(0 == stmt.getNbAffectedRows()) {
-      throw exception::UserError(std::string("Cannot delete logical-library ") + name + " because it does not exist");
+      if(logicalLibraryExists(conn, name)) {
+        throw UserSpecifiedANonEmptyLogicalLibrary(std::string("Cannot delete logical library ") + name +
+          " because it contains one or more tapes");
+      } else {
+        throw UserSpecifiedANonExistentLogicalLibrary(std::string("Cannot delete logical library ") + name +
+          " because it does not exist");
+      }
     }
   } catch(exception::UserError &) {
     throw;
diff --git a/catalogue/UserSpecifiedANonEmptyLogicalLibrary.cpp b/catalogue/UserSpecifiedANonEmptyLogicalLibrary.cpp
new file mode 100644
index 0000000000..5564c844b0
--- /dev/null
+++ b/catalogue/UserSpecifiedANonEmptyLogicalLibrary.cpp
@@ -0,0 +1,33 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "UserSpecifiedANonEmptyLogicalLibrary.hpp"
+
+namespace cta {
+namespace catalogue {
+
+//------------------------------------------------------------------------------
+// constructor
+//------------------------------------------------------------------------------
+UserSpecifiedANonEmptyLogicalLibrary::UserSpecifiedANonEmptyLogicalLibrary(const std::string &context,
+  const bool embedBacktrace):
+  UserError(context, embedBacktrace) {
+}
+
+} // namespace catalogue
+} // namespace cta
diff --git a/catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp b/catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp
new file mode 100644
index 0000000000..acaf7dcb69
--- /dev/null
+++ b/catalogue/UserSpecifiedANonEmptyLogicalLibrary.hpp
@@ -0,0 +1,46 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "common/exception/UserError.hpp"
+
+namespace cta {
+namespace catalogue {
+
+/**
+ * User error thrown when a logical library they specified contains one or more
+ * tapes when it should be empty.
+ */
+class UserSpecifiedANonEmptyLogicalLibrary: public exception::UserError {
+public:
+
+  /**
+   * Constructor.
+   *
+   * @param context optional context string added to the message
+   * at initialisation time.
+   * @param embedBacktrace whether to embed a backtrace of where the
+   * exception was throw in the message
+   */
+  UserSpecifiedANonEmptyLogicalLibrary(const std::string &context = "", const bool embedBacktrace = true);
+
+}; // class UserSpecifiedANonEmptyLogicalLibrary
+
+} // namespace catalogue
+} // namespace cta
diff --git a/catalogue/UserSpecifiedANonExistentLogicalLibrary.cpp b/catalogue/UserSpecifiedANonExistentLogicalLibrary.cpp
new file mode 100644
index 0000000000..741216c2f3
--- /dev/null
+++ b/catalogue/UserSpecifiedANonExistentLogicalLibrary.cpp
@@ -0,0 +1,33 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "UserSpecifiedANonExistentLogicalLibrary.hpp"
+
+namespace cta {
+namespace catalogue {
+
+//------------------------------------------------------------------------------
+// constructor
+//------------------------------------------------------------------------------
+UserSpecifiedANonExistentLogicalLibrary::UserSpecifiedANonExistentLogicalLibrary(const std::string &context,
+  const bool embedBacktrace):
+  UserError(context, embedBacktrace) {
+}
+
+} // namespace catalogue
+} // namespace cta
diff --git a/catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp b/catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp
new file mode 100644
index 0000000000..27d1970a98
--- /dev/null
+++ b/catalogue/UserSpecifiedANonExistentLogicalLibrary.hpp
@@ -0,0 +1,46 @@
+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "common/exception/UserError.hpp"
+
+namespace cta {
+namespace catalogue {
+
+/**
+ * User error thrown when a logical library they specified does not exist when
+ * it should.
+ */
+class UserSpecifiedANonExistentLogicalLibrary: public exception::UserError {
+public:
+
+  /**
+   * Constructor.
+   *
+   * @param context optional context string added to the message
+   * at initialisation time.
+   * @param embedBacktrace whether to embed a backtrace of where the
+   * exception was throw in the message
+   */
+  UserSpecifiedANonExistentLogicalLibrary(const std::string &context = "", const bool embedBacktrace = true);
+
+}; // class UserSpecifiedANonExistentLogicalLibrary
+
+} // namespace catalogue
+} // namespace cta
-- 
GitLab