[catalogue] Added the tape states to the catalogue::getTapes() methods +...

[catalogue] Added the tape states to the catalogue::getTapes() methods + TapeSearchCriteria + unit tests + refactoring

catalogue/CatalogueTest.cpp

@@ -4242,34 +4242,6 @@ TEST_P(cta_catalogue_CatalogueTest, createTape_same_twice) {
   }
 }
-TEST_P(cta_catalogue_CatalogueTest, createTape_NoStateProvided) {
-  using namespace cta;
-
-  const bool logicalLibraryIsDisabled= false;
-  const uint64_t nbPartialTapes = 2;
-  const bool isEncrypted = true;
-  const cta::optional<std::string> supply("value for the supply pool mechanism");
-
-  m_catalogue->createMediaType(m_admin, m_mediaType);
-  m_catalogue->createLogicalLibrary(m_admin, m_tape1.logicalLibraryName, logicalLibraryIsDisabled, "Create logical library");
-
-  m_catalogue->createVirtualOrganization(m_admin, m_vo);
-  m_catalogue->createTapePool(m_admin, m_tape1.tapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create tape pool");
-
-  catalogue::CreateTapeAttributes tape;
-  tape.vid = "VIDONE";
-  tape.mediaType = getMediaType().name;
-  tape.vendor = "vendor";
-  tape.logicalLibraryName = "logical_library";
-  tape.tapePoolName = "tape_pool";
-  tape.full = false;
-  tape.disabled = false;
-  tape.readOnly = false;
-  tape.comment = "Creation of tape one";
-  
-  ASSERT_THROW(m_catalogue->createTape(m_admin, tape),cta::catalogue::UserSpecifiedANonExistentTapeState);  
-}
-
 TEST_P(cta_catalogue_CatalogueTest, createTape_StateDoesNotExist) {
   using namespace cta;
@@ -5489,7 +5461,7 @@ TEST_P(cta_catalogue_CatalogueTest, modifyTapeState_nonExistentTape) {
   ASSERT_THROW(m_catalogue->modifyTapeState("DOES_NOT_EXIST",state,cta::nullopt,m_admin.username),cta::catalogue::UserSpecifiedANonExistentTape);
 }
-TEST_P(cta_catalogue_CatalogueTest, modifyTapeState_nonInitializedState) {
+TEST_P(cta_catalogue_CatalogueTest, modifyTapeState_nonExistentState) {
   using namespace cta;
   const bool logicalLibraryIsDisabled= false;
@@ -5505,7 +5477,7 @@ TEST_P(cta_catalogue_CatalogueTest, modifyTapeState_nonInitializedState) {
   m_catalogue->createTape(m_admin, m_tape1);
-  common::dataStructures::Tape::State state;
+  common::dataStructures::Tape::State state = (common::dataStructures::Tape::State)42;
   ASSERT_THROW(m_catalogue->modifyTapeState(m_tape1.vid,state,cta::nullopt,m_admin.username),cta::catalogue::UserSpecifiedANonExistentTapeState);
 }
@@ -5548,9 +5520,43 @@ TEST_P(cta_catalogue_CatalogueTest, modifyTapeState) {
   m_catalogue->createTape(m_admin, m_tape1);
   std::string reason = "tape broken";
-  ASSERT_NO_THROW(m_catalogue->modifyTapeState(m_tape1.vid,common::dataStructures::Tape::State::BROKEN,reason,m_admin.username));
   
-  //TODO: Get the tape back to see if the state has been changed with the reason
+  std::string vid = m_tape1.vid;
+  ASSERT_NO_THROW(m_catalogue->modifyTapeState(vid,common::dataStructures::Tape::State::BROKEN,reason,m_admin.username));
+  
+  {
+    //catalogue getTapesByVid test
+    auto vidToTapeMap = m_catalogue->getTapesByVid({vid});
+    auto tape = vidToTapeMap.at(vid);
+    ASSERT_EQ(vid,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::BROKEN,tape.state);
+    ASSERT_EQ(reason,tape.stateReason);
+    ASSERT_EQ(m_admin.username,tape.stateModifiedBy);
+    ASSERT_NE(0,tape.stateUpdateTime);
+  }
+  
+  {
+    //Get tape by search criteria test
+    cta::catalogue::TapeSearchCriteria criteria;
+    criteria.vid = vid;
+    auto tapes = m_catalogue->getTapes(criteria);
+    auto tape = tapes.front();
+    ASSERT_EQ(vid,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::BROKEN,tape.state);
+    ASSERT_EQ(reason,tape.stateReason);
+    ASSERT_EQ(m_admin.username,tape.stateModifiedBy);
+    ASSERT_NE(0,tape.stateUpdateTime);
+  }
+  
+  {
+    auto tapes = m_catalogue->getAllTapes();
+    auto tape = tapes.at(vid);
+    ASSERT_EQ(vid,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::BROKEN,tape.state);
+    ASSERT_EQ(reason,tape.stateReason);
+    ASSERT_EQ(m_admin.username,tape.stateModifiedBy);
+    ASSERT_NE(0,tape.stateUpdateTime);
+  }
 }
 TEST_P(cta_catalogue_CatalogueTest, modifyTapeStateResetReasonWhenBackToActiveState) {
@@ -5568,13 +5574,80 @@ TEST_P(cta_catalogue_CatalogueTest, modifyTapeStateResetReasonWhenBackToActiveSt
   m_catalogue->createTapePool(m_admin, m_tape1.tapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create tape pool");
   m_catalogue->createTape(m_admin, m_tape1);
+  
+  std::string vid = m_tape1.vid;
   std::string reason = "Broken tape";
-  ASSERT_NO_THROW(m_catalogue->modifyTapeState(m_tape1.vid,common::dataStructures::Tape::State::BROKEN,reason,m_admin.username));
+  ASSERT_NO_THROW(m_catalogue->modifyTapeState(vid,common::dataStructures::Tape::State::BROKEN,reason,m_admin.username));
   
-  ASSERT_NO_THROW(m_catalogue->modifyTapeState(m_tape1.vid,common::dataStructures::Tape::State::ACTIVE,cta::nullopt,m_admin.username));
+  ASSERT_NO_THROW(m_catalogue->modifyTapeState(vid,common::dataStructures::Tape::State::ACTIVE,cta::nullopt,m_admin.username));
   
-  //TODO: Get the tape back to see if the state has been changed with the reason reset to null
+  {
+    auto vidToTapeMap = m_catalogue->getTapesByVid({vid});
+    auto tape = vidToTapeMap.at(vid);
+    ASSERT_EQ(vid,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::ACTIVE,tape.state);
+    ASSERT_FALSE(tape.stateReason);
+    ASSERT_EQ(m_admin.username,tape.stateModifiedBy);
+    ASSERT_NE(0,tape.stateUpdateTime);
+  }
+}
+
+TEST_P(cta_catalogue_CatalogueTest, getTapesSearchCriteriaByState) {
+  using namespace cta;
+
+  const bool logicalLibraryIsDisabled= false;
+  const uint64_t nbPartialTapes = 2;
+  const bool isEncrypted = true;
+  const cta::optional<std::string> supply("value for the supply pool mechanism");
+
+  m_catalogue->createMediaType(m_admin, m_mediaType);
+  m_catalogue->createLogicalLibrary(m_admin, m_tape1.logicalLibraryName, logicalLibraryIsDisabled, "Create logical library");
+
+  m_catalogue->createVirtualOrganization(m_admin, m_vo);
+  m_catalogue->createTapePool(m_admin, m_tape1.tapePoolName, m_vo.name, nbPartialTapes, isEncrypted, supply, "Create tape pool");
+
+  m_catalogue->createTape(m_admin, m_tape1);
+  m_catalogue->createTape(m_admin, m_tape2);
+  
+  std::string vidTape1 = m_tape1.vid;
+  std::string vidTape2 = m_tape2.vid;
+  
+  {
+    cta::catalogue::TapeSearchCriteria criteria;
+    criteria.state = common::dataStructures::Tape::ACTIVE;
+    auto tapes = m_catalogue->getTapes(criteria);
+    ASSERT_EQ(2,tapes.size());
+    auto tape = tapes.front();
+    ASSERT_EQ(vidTape1,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::ACTIVE,tape.state);
+    ASSERT_FALSE(tape.stateReason);
+    ASSERT_EQ(m_admin.username + "@" + m_admin.host,tape.stateModifiedBy);
+    ASSERT_NE(0,tape.stateUpdateTime);
+  }
+  
+  std::string reason = "Broken tape";
+  ASSERT_NO_THROW(m_catalogue->modifyTapeState(vidTape1,common::dataStructures::Tape::State::BROKEN,reason,m_admin.username));
+  
+  {
+    cta::catalogue::TapeSearchCriteria criteria;
+    criteria.state = common::dataStructures::Tape::ACTIVE;
+    auto tapes = m_catalogue->getTapes(criteria);
+    ASSERT_EQ(1,tapes.size());
+    auto tape = tapes.front();
+    //The tape 2 is ACTIVE so this is the one we expect
+    ASSERT_EQ(vidTape2,tape.vid);
+  }
+  {
+    cta::catalogue::TapeSearchCriteria criteria;
+    criteria.state = common::dataStructures::Tape::BROKEN;
+    auto tapes = m_catalogue->getTapes(criteria);
+    ASSERT_EQ(1,tapes.size());
+    auto tape = tapes.front();
+    //The tape 2 is ACTIVE so this is the one we expect
+    ASSERT_EQ(vidTape1,tape.vid);
+    ASSERT_EQ(common::dataStructures::Tape::BROKEN,tape.state);
+  }
 }
 TEST_P(cta_catalogue_CatalogueTest, tapeLabelled) {

catalogue/RdbmsCatalogue.cpp

@@ -3166,6 +3166,7 @@ void RdbmsCatalogue::createTape(
     bool readOnly = tape.readOnly;
     // Translate an empty comment string to a NULL database value
     const optional<std::string> tapeComment = tape.comment && tape.comment->empty() ? nullopt : tape.comment;
+    const optional<std::string> stateReason = tape.stateReason && tape.stateReason->empty() ? nullopt : tape.stateReason;
     
     if(vid.empty()) {
       throw UserSpecifiedAnEmptyStringVid("Cannot create tape because the VID is an empty string");
@@ -3190,18 +3191,15 @@ void RdbmsCatalogue::createTape(
     
     std::string tapeState;
     try {
-      tapeState = common::dataStructures::Tape::STATE_TO_STRING_MAP.at(tape.state);
-    } catch(std::out_of_range &) {
-      std::string errorMsg = "Cannot create tape because the state specified does not exist. Possible values for state are:";
-      for(const auto &kv: common::dataStructures::Tape::STRING_TO_STATE_MAP){
-        errorMsg += " " + kv.first;
-      }
-      throw UserSpecifiedANonExistentTapeState(std::string(errorMsg));
+      tapeState = common::dataStructures::Tape::stateToString(tape.state);
+    } catch(cta::exception::Exception &ex) {
+      std::string errorMsg = "Cannot create tape because the state specified does not exist. Possible values for state are: " + common::dataStructures::Tape::getAllPossibleStates();
+      throw UserSpecifiedANonExistentTapeState(errorMsg);
     }
     
     if(tape.state != common::dataStructures::Tape::ACTIVE){
-      if(!tape.stateReason){
-        throw UserSpecifiedAnEmptyStringReasonWhenTapeStateNotActive("Cannot create tape because no reason has been provided for the state " + tape.state);
+      if(!stateReason){
+        throw UserSpecifiedAnEmptyStringReasonWhenTapeStateNotActive("Cannot create tape because no reason has been provided for the state " + tapeState);
       }
     }
 
@@ -3300,7 +3298,7 @@ void RdbmsCatalogue::createTape(
     stmt.bindString(":USER_COMMENT", tapeComment);
     
     std::string stateModifiedBy = admin.username + "@" + admin.host;
-    stmt.bindString(":TAPE_STATE",cta::common::dataStructures::Tape::STATE_TO_STRING_MAP.at(tape.state));
+    stmt.bindString(":TAPE_STATE",cta::common::dataStructures::Tape::stateToString(tape.state));
     stmt.bindString(":STATE_REASON",tape.stateReason);
     stmt.bindUint64(":STATE_UPDATE_TIME",now);
     stmt.bindString(":STATE_MODIFIED_BY", stateModifiedBy);
@@ -3327,7 +3325,7 @@ void RdbmsCatalogue::createTape(
        .add("isReadOnly", readOnly ? 1 : 0)
        .add("isFromCastor", isFromCastor ? 1 : 0)
        .add("userComment", tape.comment ? tape.comment.value() : "")
-       .add("tapeState",cta::common::dataStructures::Tape::STATE_TO_STRING_MAP.at(tape.state))
+       .add("tapeState",cta::common::dataStructures::Tape::stateToString(tape.state))
        .add("stateReason",tape.stateReason ? tape.stateReason.value() : "")
        .add("stateUpdateTime",now)
        .add("stateModifiedBy",stateModifiedBy)
@@ -3522,7 +3520,7 @@ std::list<common::dataStructures::Tape> RdbmsCatalogue::getTapes(rdbms::Conn &co
         "TAPE.USER_COMMENT AS USER_COMMENT,"
             
         "TAPE.TAPE_STATE AS TAPE_STATE,"
-        "TAPE.STATE_REASON AS TAPE_REASON,"
+        "TAPE.STATE_REASON AS STATE_REASON,"
         "TAPE.STATE_UPDATE_TIME AS STATE_UPDATE_TIME,"
         "TAPE.STATE_MODIFIED_BY AS STATE_MODIFIED_BY,"
 
@@ -3554,7 +3552,8 @@ std::list<common::dataStructures::Tape> RdbmsCatalogue::getTapes(rdbms::Conn &co
        searchCriteria.disabled ||
        searchCriteria.full ||
        searchCriteria.readOnly ||
-       searchCriteria.diskFileIds) {
+       searchCriteria.diskFileIds ||
+       searchCriteria.state) {
       sql += " WHERE";
     }
 
@@ -3623,6 +3622,12 @@ std::list<common::dataStructures::Tape> RdbmsCatalogue::getTapes(rdbms::Conn &co
       addedAWhereConstraint = true;
     }
 
+    if(searchCriteria.state) {
+      if(addedAWhereConstraint) sql += " AND ";
+      sql += " TAPE.TAPE_STATE = :TAPE_STATE";
+      addedAWhereConstraint = true;
+    }
+    
     sql += " ORDER BY TAPE.VID";
 
     auto stmt = conn.createStmt(sql);
@@ -3637,6 +3642,11 @@ std::list<common::dataStructures::Tape> RdbmsCatalogue::getTapes(rdbms::Conn &co
     if(searchCriteria.disabled) stmt.bindBool(":IS_DISABLED", searchCriteria.disabled.value());
     if(searchCriteria.full) stmt.bindBool(":IS_FULL", searchCriteria.full.value());
     if(searchCriteria.readOnly) stmt.bindBool(":IS_READ_ONLY", searchCriteria.readOnly.value());
+    try{
+      if(searchCriteria.state) stmt.bindString(":TAPE_STATE",cta::common::dataStructures::Tape::stateToString(searchCriteria.state.value()));
+    } catch(cta::exception::Exception &ex){
+      throw cta::exception::UserError(std::string("The state provided does not exist. Possible values are: ") + cta::common::dataStructures::Tape::getAllPossibleStates());
+    }
 
     // Disk file ID lookup requires multiple queries
     std::vector<std::string>::const_iterator diskFileId_it;
@@ -3683,6 +3693,12 @@ std::list<common::dataStructures::Tape> RdbmsCatalogue::getTapes(rdbms::Conn &co
 
         auto optionalComment = rset.columnOptionalString("USER_COMMENT");
         tape.comment = optionalComment ? optionalComment.value() : "";
+        
+        tape.setState(rset.columnString("TAPE_STATE"));
+        tape.stateReason = rset.columnOptionalString("STATE_REASON");
+        tape.stateUpdateTime = rset.columnUint64("STATE_UPDATE_TIME");
+        tape.stateModifiedBy = rset.columnString("STATE_MODIFIED_BY");
+        
         tape.creationLog.username = rset.columnString("CREATION_LOG_USER_NAME");
         tape.creationLog.host = rset.columnString("CREATION_LOG_HOST_NAME");
         tape.creationLog.time = rset.columnUint64("CREATION_LOG_TIME");
@@ -3810,6 +3826,11 @@ std::string RdbmsCatalogue::getSelectTapesBy100VidsSql() const {
       "TAPE.WRITE_MOUNT_COUNT AS WRITE_MOUNT_COUNT,"
 
       "TAPE.USER_COMMENT AS USER_COMMENT,"
+          
+      "TAPE.TAPE_STATE AS TAPE_STATE,"
+      "TAPE.STATE_REASON AS STATE_REASON,"
+      "TAPE.STATE_UPDATE_TIME AS STATE_UPDATE_TIME,"
+      "TAPE.STATE_MODIFIED_BY AS STATE_MODIFIED_BY,"
 
       "TAPE.CREATION_LOG_USER_NAME AS CREATION_LOG_USER_NAME,"
       "TAPE.CREATION_LOG_HOST_NAME AS CREATION_LOG_HOST_NAME,"
@@ -3870,6 +3891,12 @@ void RdbmsCatalogue::executeGetTapesBy100VidsStmtAndCollectResults(rdbms::Stmt &
     tape.writeMountCount = rset.columnUint64("WRITE_MOUNT_COUNT");
     auto optionalComment = rset.columnOptionalString("USER_COMMENT");
     tape.comment = optionalComment ? optionalComment.value() : "";
+    
+    tape.setState(rset.columnString("TAPE_STATE"));
+    tape.stateReason = rset.columnOptionalString("STATE_REASON");
+    tape.stateUpdateTime = rset.columnUint64("STATE_UPDATE_TIME");
+    tape.stateModifiedBy = rset.columnString("STATE_MODIFIED_BY");
+    
     tape.creationLog.username = rset.columnString("CREATION_LOG_USER_NAME");
     tape.creationLog.host = rset.columnString("CREATION_LOG_HOST_NAME");
     tape.creationLog.time = rset.columnUint64("CREATION_LOG_TIME");
@@ -4470,26 +4497,20 @@ void RdbmsCatalogue::modifyTapeState(const std::string &vid, const common::dataS
     using namespace common::dataStructures;
     const time_t now = time(nullptr);
     
-    cta::optional<std::string> stateReasonCopy = stateReason;
+    const optional<std::string> stateReasonCopy = stateReason && stateReason->empty() ? nullopt : stateReason;
     
     std::string stateStr;
     try {
-      stateStr = Tape::STATE_TO_STRING_MAP.at(state);
-    } catch(std::out_of_range & ex){
-      std::string errorMsg = "The state provided in parameter (" + std::to_string(state) + ") is not known or has not been initialized existing states are:";
-      for(const auto & kv: Tape::STRING_TO_STATE_MAP){
-        errorMsg += " " + kv.first;
-      }
+      stateStr = cta::common::dataStructures::Tape::stateToString(state);
+    } catch(cta::exception::Exception & ex){
+      std::string errorMsg = "The state provided in parameter (" + std::to_string(state) + ") is not known or has not been initialized existing states are:" + common::dataStructures::Tape::getAllPossibleStates();
       throw UserSpecifiedANonExistentTapeState(errorMsg);
     }
     
-    //Check the reason is set for all the status except the ACTIVE one, this is the only status that allows the reason to be set to null.
-    if(!stateReason || (stateReason && cta::utils::trimString(stateReason.value()).empty())){
-      if(state != Tape::State::ACTIVE){
+    //Check the reason is set for all the status except the ACTIVE one, this is the only state that allows the reason to be set to null.
+    if(state != Tape::State::ACTIVE){
+      if(!stateReasonCopy){
         throw UserSpecifiedAnEmptyStringReasonWhenTapeStateNotActive(std::string("Cannot modify the state of the tape ") + vid + " to " + stateStr + " because the reason has not been provided."); 
-      } else {
-        //State is active, but no reason provided: we will reset the field so we assign nullopt to the state reason
-        stateReasonCopy = cta::nullopt;
       }
     }
     

catalogue/TapeSearchCriteria.hpp

@@ -91,6 +91,12 @@ struct TapeSearchCriteria {
    * List of disk file IDs to search for.
    */
   optional<std::vector<std::string>> diskFileIds;
+  
+  /**
+   * The state of the tapes to look for
+   */
+  optional<common::dataStructures::Tape::State> state;
+  
 }; // struct TapeSearchCriteria
 
 } // namespace catalogue

common/dataStructures/Tape.cpp

@@ -19,6 +19,7 @@
 #include "common/dataStructures/Tape.hpp"
 #include "common/dataStructures/utils.hpp"
 #include "common/exception/Exception.hpp"
+#include "common/utils/utils.hpp"
 
 namespace cta {
 namespace common {
@@ -50,6 +51,16 @@ const std::map<std::string,Tape::State> Tape::STRING_TO_STATE_MAP = {
   {"DISABLED",Tape::State::DISABLED}
 };
 
+std::string Tape::getAllPossibleStates(){
+  std::string ret;
+  for(auto &kv: STRING_TO_STATE_MAP){
+    ret += kv.first + " ";
+  }
+  if(ret.size())
+    ret = ret.substr(0,ret.size() - 1);
+  return ret;
+}
+
 //------------------------------------------------------------------------------
 // operator==
 //------------------------------------------------------------------------------
@@ -79,6 +90,33 @@ bool Tape::operator!=(const Tape &rhs) const {
   return !operator==(rhs);
 }
 
+void Tape::setState(const std::string& state) {
+  this->state = Tape::stringToState(state);
+}
+
+std::string Tape::getStateStr() const {
+  return Tape::stateToString(state);
+}
+
+std::string Tape::stateToString(const Tape::State & state) {
+  try {
+    return Tape::STATE_TO_STRING_MAP.at(state);
+  } catch (std::out_of_range &ex){
+    throw cta::exception::Exception(std::string("In Tape::stateToString(): The state given (") + std::to_string(state) + ") does not exist.");
+  }
+}
+
+Tape::State Tape::stringToState(const std::string& state) {
+  try {
+    std::string stateCopy = state;
+    cta::utils::toUpper(stateCopy);
+    return Tape::STRING_TO_STATE_MAP.at(state);
+  } catch(std::out_of_range &ex){
+    throw cta::exception::Exception(std::string("In Tape::stringToState(): The state given (") + state + ") does not exist. Possible values are " + Tape::getAllPossibleStates());
+  }
+}
+
+
 //------------------------------------------------------------------------------
 // operator<<
 //------------------------------------------------------------------------------

common/dataStructures/Tape.hpp

@@ -46,11 +46,33 @@ struct Tape {
   static const std::map<State,std::string> STATE_TO_STRING_MAP;
   static const std::map<std::string,State> STRING_TO_STATE_MAP;
   
+  static std::string getAllPossibleStates();
+  
   Tape();
 
   bool operator==(const Tape &rhs) const;
 
   bool operator!=(const Tape &rhs) const;
+  
+  void setState(const std::string & state);
+  
+  std::string getStateStr() const;
+  
+  /**
+   * Return the string representation of the tape State passed in parameter
+   * @param state the state to return the string representation
+   * @return the string representation of the state passed in parameter
+   * @throws cta::exception::Exception if the state passed in parameter does not exist
+   */
+  static std::string stateToString(const State &state);
+  
+  /**
+   * Return the state value according to the state passed in parameter (not case sensitive)
+   * @param state the string that identifies the state enum value to return
+   * @return the state corresponding to the State enum value
+   * @throws cta::exception::Exception if the state passed in parameter does not match any existing State enum value
+   */
+  static State stringToState(const std::string & state);
 
   std::string vid;
   std::string mediaType;
@@ -89,7 +111,7 @@ struct Tape {
   State state;
   optional<std::string> stateReason;
   std::string stateModifiedBy;
-  time_t stateModifiedTime;
+  time_t stateUpdateTime;
   
 }; // struct Tape