From 0664f64f8df9650baaef6bd858a7b7b5fe474019 Mon Sep 17 00:00:00 2001
From: Eric Cano <Eric.Cano@cern.ch>
Date: Mon, 8 Oct 2018 17:47:05 +0200
Subject: [PATCH] Created partial unit test for repack and fixed detected
 issues.

---
 objectstore/RootEntry.cpp                 |  2 +-
 objectstore/cta.proto                     |  2 --
 scheduler/OStoreDB/OStoreDB.cpp           | 10 +++++++---
 scheduler/OStoreDB/OStoreDB.hpp           |  2 ++
 scheduler/OStoreDB/OStoreDBFactory.hpp    |  5 +++++
 scheduler/Scheduler.cpp                   |  6 +++---
 scheduler/Scheduler.hpp                   |  6 ++----
 scheduler/SchedulerDatabase.hpp           |  2 ++
 scheduler/SchedulerTest.cpp               | 16 ++++++++++++++++
 xroot_plugins/XrdSsiCtaRequestMessage.cpp |  6 +++---
 10 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/objectstore/RootEntry.cpp b/objectstore/RootEntry.cpp
index f90efdb3bb..3c94293151 100644
--- a/objectstore/RootEntry.cpp
+++ b/objectstore/RootEntry.cpp
@@ -878,7 +878,7 @@ std::string RootEntry::addOrGetRepackQueueAndCommit(AgentReference& agentRef, Re
   // Check the repack queue does not already exist
   try {
     return getRepackQueueAddress(queueType);
-  } catch (NoSuchRetrieveQueue &) {}
+  } catch (NoSuchRepackQueue &) {}
   // The queue is not there yet. Create it.
   // Insert the archive queue pointer in the root entry, then the queue.
   std::string repackQueueNameHeader = "RepackQueue";
diff --git a/objectstore/cta.proto b/objectstore/cta.proto
index 9cde1ee0cb..e26b34b352 100644
--- a/objectstore/cta.proto
+++ b/objectstore/cta.proto
@@ -84,12 +84,10 @@ message AgentRegisterPointer {
 
 message RepackIndexPointer {
   required string address = 105;
-  required EntryLog log = 106;
 }
 
 message RepackQueuePointer {
   required string address = 107;
-  required EntryLog log = 108;
 }
 
 
diff --git a/scheduler/OStoreDB/OStoreDB.cpp b/scheduler/OStoreDB/OStoreDB.cpp
index 77f25e73d2..6d6157f06a 100644
--- a/scheduler/OStoreDB/OStoreDB.cpp
+++ b/scheduler/OStoreDB/OStoreDB.cpp
@@ -1041,9 +1041,6 @@ void OStoreDB::queueRepack(const std::string& vid, const std::string& bufferURL,
   rr->setVid(vid);
   rr->setRepackType(repackType);
   // Try to reference the object in the index (will fail if there is already a request with this VID.
-  RootEntry re(m_objectStore);
-  re.fetchNoLock();
-  RepackIndex ri(re.addOrGetRepackIndexAndCommit(*m_agentReference, lc), m_objectStore);
   try {
     Helpers::registerRepackRequestToIndex(vid, rr->getAddressIfSet(), *m_agentReference, m_objectStore, lc);
   } catch (objectstore::RepackIndex::VidAlreadyRegistered &) {
@@ -1064,6 +1061,13 @@ void OStoreDB::queueRepack(const std::string& vid, const std::string& bufferURL,
   }
 }
 
+//------------------------------------------------------------------------------
+// OStoreDB::queueRepack()
+//------------------------------------------------------------------------------
+std::list<common::dataStructures::RepackInfo> OStoreDB::getRepackInfo() {
+  // TODO
+  throw exception::Exception("No implemented");
+}
 
 //------------------------------------------------------------------------------
 // OStoreDB::getDriveStates()
diff --git a/scheduler/OStoreDB/OStoreDB.hpp b/scheduler/OStoreDB/OStoreDB.hpp
index c0de2094d6..f6f2018235 100644
--- a/scheduler/OStoreDB/OStoreDB.hpp
+++ b/scheduler/OStoreDB/OStoreDB.hpp
@@ -278,6 +278,8 @@ public:
   void queueRepack(const std::string& vid, const std::string& bufferURL, common::dataStructures::RepackType repackType, 
       log::LogContext &logContext) override;
   
+  std::list<common::dataStructures::RepackInfo> getRepackInfo() override;
+  
   /* === Drive state handling  ============================================== */
   /**
    * Get states of all drives.
diff --git a/scheduler/OStoreDB/OStoreDBFactory.hpp b/scheduler/OStoreDB/OStoreDBFactory.hpp
index 16d1e53dea..111a3f9a8e 100644
--- a/scheduler/OStoreDB/OStoreDBFactory.hpp
+++ b/scheduler/OStoreDB/OStoreDBFactory.hpp
@@ -156,6 +156,11 @@ public:
     m_OStoreDB.queueRepack(vid, bufferURL, repackType, lc);
   }
   
+  std::list<common::dataStructures::RepackInfo> getRepackInfo() override {
+    return m_OStoreDB.getRepackInfo();
+  }
+
+  
   std::list<cta::common::dataStructures::DriveState> getDriveStates(log::LogContext & lc) const override {
     return m_OStoreDB.getDriveStates(lc);
   }
diff --git a/scheduler/Scheduler.cpp b/scheduler/Scheduler.cpp
index d3daeba4e3..83d82fe345 100644
--- a/scheduler/Scheduler.cpp
+++ b/scheduler/Scheduler.cpp
@@ -319,14 +319,14 @@ void Scheduler::cancelRepack(const common::dataStructures::SecurityIdentity &cli
 //------------------------------------------------------------------------------
 // getRepacks
 //------------------------------------------------------------------------------
-std::list<common::dataStructures::RepackInfo> Scheduler::getRepacks(const common::dataStructures::SecurityIdentity &cliIdentity) {
-  throw exception::Exception(std::string("Not implemented: ") + __PRETTY_FUNCTION__);
+std::list<common::dataStructures::RepackInfo> Scheduler::getRepacks() {
+  return m_db.getRepackInfo();
 }
 
 //------------------------------------------------------------------------------
 // getRepack
 //------------------------------------------------------------------------------
-common::dataStructures::RepackInfo Scheduler::getRepack(const common::dataStructures::SecurityIdentity &cliIdentity, const std::string &vid) {
+common::dataStructures::RepackInfo Scheduler::getRepack(const std::string &vid) {
   throw exception::Exception(std::string("Not implemented: ") + __PRETTY_FUNCTION__); 
 }
 
diff --git a/scheduler/Scheduler.hpp b/scheduler/Scheduler.hpp
index 4b18c7a65c..ec730c41eb 100644
--- a/scheduler/Scheduler.hpp
+++ b/scheduler/Scheduler.hpp
@@ -193,10 +193,8 @@ public:
   void queueRepack(const common::dataStructures::SecurityIdentity &cliIdentity, const std::string &vid, 
     const std::string & bufferURL, const common::dataStructures::RepackType repackType, log::LogContext & lc);
   void cancelRepack(const cta::common::dataStructures::SecurityIdentity &cliIdentity, const std::string &vid);
-  std::list<cta::common::dataStructures::RepackInfo> getRepacks(
-    const cta::common::dataStructures::SecurityIdentity &cliIdentity);
-  cta::common::dataStructures::RepackInfo getRepack(
-    const cta::common::dataStructures::SecurityIdentity &cliIdentity, const std::string &vid);
+  std::list<cta::common::dataStructures::RepackInfo> getRepacks();
+  cta::common::dataStructures::RepackInfo getRepack(const std::string &vid);
 
   void shrink(const cta::common::dataStructures::SecurityIdentity &cliIdentity, const std::string &tapepool); 
     // removes extra tape copies from a specific pool(usually an "_2" pool)
diff --git a/scheduler/SchedulerDatabase.hpp b/scheduler/SchedulerDatabase.hpp
index a1145991df..fe32434219 100644
--- a/scheduler/SchedulerDatabase.hpp
+++ b/scheduler/SchedulerDatabase.hpp
@@ -31,6 +31,7 @@
 #include "common/dataStructures/RetrieveJob.hpp"
 #include "common/dataStructures/RetrieveRequest.hpp"
 #include "common/dataStructures/RepackType.hpp"
+#include "common/dataStructures/RepackInfo.hpp"
 #include "common/dataStructures/SecurityIdentity.hpp"
 #include "common/remoteFS/RemotePathAndStatus.hpp"
 #include "common/log/LogContext.hpp"
@@ -355,6 +356,7 @@ public:
   /*============ Repack management: user side ================================*/
   virtual void queueRepack(const std::string & vid, const std::string & bufferURL,
       common::dataStructures::RepackType repackType, log::LogContext & lc) = 0;
+  virtual std::list<common::dataStructures::RepackInfo> getRepackInfo() = 0;
   
   /*============ Repack management: tape server side =========================*/
   
diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp
index d40421a22e..22257bb93e 100644
--- a/scheduler/SchedulerTest.cpp
+++ b/scheduler/SchedulerTest.cpp
@@ -725,6 +725,22 @@ TEST_P(SchedulerTest, showqueues) {
   ASSERT_EQ(1, queuesSummary.size());
 }
 
+TEST_P(SchedulerTest, repack) {
+  using namespace cta;
+  
+  setupDefaultCatalogue();
+  
+  Scheduler &scheduler = getScheduler();
+  
+  log::DummyLogger dl("", "");
+  log::LogContext lc(dl);
+  
+  common::dataStructures::SecurityIdentity cliId;
+  scheduler.queueRepack(cliId, "Tape", "root://server/repackDir", common::dataStructures::RepackType::justrepack, lc);
+  // TODO
+  // auto repacks = scheduler.getRepacks();
+}
+
 #undef TEST_MOCK_DB
 #ifdef TEST_MOCK_DB
 static cta::MockSchedulerDatabaseFactory mockDbFactory;
diff --git a/xroot_plugins/XrdSsiCtaRequestMessage.cpp b/xroot_plugins/XrdSsiCtaRequestMessage.cpp
index 353c7e66a2..d32bab5fd2 100644
--- a/xroot_plugins/XrdSsiCtaRequestMessage.cpp
+++ b/xroot_plugins/XrdSsiCtaRequestMessage.cpp
@@ -1407,9 +1407,9 @@ void RequestMessage::processRepack_Ls(const cta::admin::AdminCmd &admincmd, cta:
    std::list<cta::common::dataStructures::RepackInfo> list;
 
    if(!vid) {      
-      list = m_scheduler.getRepacks(m_cliIdentity);
+      list = m_scheduler.getRepacks();
    } else {
-      list.push_back(m_scheduler.getRepack(m_cliIdentity, vid.value()));
+      list.push_back(m_scheduler.getRepack(vid.value()));
    }
 
    if(!list.empty())
@@ -1459,7 +1459,7 @@ void RequestMessage::processRepack_Err(const cta::admin::AdminCmd &admincmd, cta
 
    auto &vid = getRequired(OptionString::VID);
 
-   cta::common::dataStructures::RepackInfo info = m_scheduler.getRepack(m_cliIdentity, vid);
+   cta::common::dataStructures::RepackInfo info = m_scheduler.getRepack(vid);
 
    if(!info.errors.empty())
    {
-- 
GitLab