Fixed cta/CTA#927 Bad message for archive route pointing non-existing pool

catalogue/Catalogue.hpp

@@ -89,6 +89,7 @@ CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonExistentDiskSystem);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonEmptyDiskSystemAfterDelete);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonEmptyLogicalLibrary);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonEmptyTape);
+CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonExistentArchiveRoute);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonExistentLogicalLibrary);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonExistentTape);
 CTA_GENERATE_USER_EXCEPTION_CLASS(UserSpecifiedANonExistentTapePool);
@@ -545,7 +546,18 @@ public:
     const std::string &storageClassName,
     const std::string &tapePoolName) const = 0;
 
+  /**
+   * Modifies the tape pool of the specified archive route.
+   *
+   * @param admin The administrator.
+   * @param storageClassName The name of the storage class.
+   * @param copyNb The copy number.
+   * @param tapePoolName The name of the tape pool.
+   * @throw UserSpecifiedANonExistentTapePool if the user specified a
+   * non-existent tape pool.
+   */
   virtual void modifyArchiveRouteTapePoolName(const common::dataStructures::SecurityIdentity &admin, const std::string &storageClassName, const uint32_t copyNb, const std::string &tapePoolName) = 0;
+
   virtual void modifyArchiveRouteComment(const common::dataStructures::SecurityIdentity &admin, const std::string &storageClassName, const uint32_t copyNb, const std::string &comment) = 0;
 
   virtual void createLogicalLibrary(const common::dataStructures::SecurityIdentity &admin, const std::string &name, const bool isDisabled, const std::string &comment) = 0;

catalogue/CatalogueTest.cpp

@@ -3279,6 +3279,46 @@ TEST_P(cta_catalogue_CatalogueTest, modifyArchiveRouteTapePoolName) {
   }
 }
+TEST_P(cta_catalogue_CatalogueTest, modifyArchiveRouteTapePoolName_nonExistentTapePool) {
+  using namespace cta;
+      
+  m_catalogue->createVirtualOrganization(m_admin, m_vo);
+  m_catalogue->createStorageClass(m_admin, m_storageClassSingleCopy);
+
+  const uint64_t nbPartialTapes = 2;
+  const bool isEncrypted = true;
+  const cta::optional<std::string> supply("value for the supply pool mechanism");
+  m_catalogue->createTapePool(m_admin, m_tape1.tapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create tape pool");
+
+  const std::string anotherTapePoolName = "another_tape_pool";
+  m_catalogue->createTapePool(m_admin, anotherTapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create another tape pool");
+
+  const uint32_t copyNb = 1;
+  const std::string comment = "Create archive route";
+  m_catalogue->createArchiveRoute(m_admin, m_storageClassSingleCopy.name, copyNb, m_tape1.tapePoolName, comment);
+
+  {
+    const std::list<common::dataStructures::ArchiveRoute> routes = m_catalogue->getArchiveRoutes();
+      
+    ASSERT_EQ(1, routes.size());
+      
+    const common::dataStructures::ArchiveRoute route = routes.front();
+    ASSERT_EQ(m_storageClassSingleCopy.name, route.storageClassName);
+    ASSERT_EQ(copyNb, route.copyNb);
+    ASSERT_EQ(m_tape1.tapePoolName, route.tapePoolName);
+    ASSERT_EQ(comment, route.comment);
+
+    const common::dataStructures::EntryLog creationLog = route.creationLog;
+    ASSERT_EQ(m_admin.username, creationLog.username);
+    ASSERT_EQ(m_admin.host, creationLog.host);
+  
+    const common::dataStructures::EntryLog lastModificationLog = route.lastModificationLog;
+    ASSERT_EQ(creationLog, lastModificationLog);
+  }
+
+  ASSERT_THROW(m_catalogue->modifyArchiveRouteTapePoolName(m_admin, m_storageClassSingleCopy.name, copyNb, "non_existent_tape_pool"), catalogue::UserSpecifiedANonExistentTapePool);
+}
+
 TEST_P(cta_catalogue_CatalogueTest, modifyArchiveRouteTapePoolName_nonExistentArchiveRoute) {
   using namespace cta;
       
@@ -3291,7 +3331,7 @@ TEST_P(cta_catalogue_CatalogueTest, modifyArchiveRouteTapePoolName_nonExistentAr
   m_catalogue->createTapePool(m_admin, m_tape1.tapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create tape pool");
   const uint32_t copyNb = 1;
-  ASSERT_THROW(m_catalogue->modifyArchiveRouteTapePoolName(m_admin, m_storageClassSingleCopy.name, copyNb, m_tape1.tapePoolName), exception::UserError);
+  ASSERT_THROW(m_catalogue->modifyArchiveRouteTapePoolName(m_admin, m_storageClassSingleCopy.name, copyNb, m_tape1.tapePoolName), catalogue::UserSpecifiedANonExistentArchiveRoute);
 }
 TEST_P(cta_catalogue_CatalogueTest, modifyArchiveRouteComment) {

catalogue/RdbmsCatalogue.cpp

@@ -2968,6 +2968,15 @@ void RdbmsCatalogue::modifyArchiveRouteTapePoolName(const common::dataStructures
             "STORAGE_CLASS_NAME = :STORAGE_CLASS_NAME) AND "
         "COPY_NB = :COPY_NB";
     auto conn = m_connPool.getConn();
+
+    if(!archiveRouteExists(conn, storageClassName, copyNb)) {
+      throw UserSpecifiedANonExistentArchiveRoute("Archive route does not exist");
+    }
+
+    if(!tapePoolExists(conn, tapePoolName)) {
+      throw UserSpecifiedANonExistentTapePool("Tape pool does not exist");
+    }
+
     auto stmt = conn.createStmt(sql);
     stmt.bindString(":TAPE_POOL_NAME", tapePoolName);
     stmt.bindString(":LAST_UPDATE_USER_NAME", admin.username);
@@ -2979,11 +2988,14 @@ void RdbmsCatalogue::modifyArchiveRouteTapePoolName(const common::dataStructures
 
     if(0 == stmt.getNbAffectedRows()) {
       exception::UserError ue;
-      ue.getMessage() << "Cannot modify archive route for storage-class " << ":" + storageClassName +
-        " and copy number " << copyNb << " because either it or tape pool " + tapePoolName + " does not exist";
+      ue.getMessage() << "Either the archive route or the tape pool does not exist";
       throw ue;
     }
-  } catch(exception::UserError &) {
+  } catch(exception::UserError &ue) {
+    std::ostringstream msg;
+    msg << "Cannot modify tape pool of archive route: storageClassName=" << storageClassName << " copyNb=" << copyNb <<
+      " tapePoolName=" << tapePoolName << ": " << ue.getMessage().str();
+    ue.getMessage().str(msg.str());
     throw;
   } catch(exception::Exception &ex) {
     ex.getMessage().str(std::string(__FUNCTION__) + ": " + ex.getMessage().str());