RecallReportPackerBadBadEnd now compiles, runs, but does not pass

scheduler/ArchiveMount.cpp

@@ -73,23 +73,18 @@ std::string cta::ArchiveMount::getMountTransactionId() const throw(){
   return "UNKNOWN_MOUNTTRANSACTIONID_FOR_ARCHIVE_MOUNT";
 }
 
-std::unique_ptr<cta::ArchiveJob> cta::ArchiveMount::getNextJob(){
-  cta::exception::Exception ex("Not implemented");
-  throw ex;
+//------------------------------------------------------------------------------
+// getNextJob
+//------------------------------------------------------------------------------
+std::unique_ptr<cta::ArchiveJob> cta::ArchiveMount::getNextJob() {
+  throw NotImplemented(std::string(__FUNCTION__) + ": Not implemented");
 }
     
 //------------------------------------------------------------------------------
 // complete
 //------------------------------------------------------------------------------
 void cta::ArchiveMount::complete() {
-  throw NotImplemented ("");
-}
-
-//------------------------------------------------------------------------------
-// failed
-//------------------------------------------------------------------------------
-void cta::ArchiveMount::failed(const std::exception &ex) {
-  throw NotImplemented ("");
+  throw NotImplemented(std::string(__FUNCTION__) + ": Not implemented");
 }
 
 //------------------------------------------------------------------------------

scheduler/ArchiveMount.hpp

@@ -78,7 +78,7 @@ namespace cta {
     virtual std::string getMountTransactionId() const throw();
 
     /**
-     * Indicates that the mount was successful.
+     * Indicates that the mount was completed.
      *
      * @param checksumOfTransfer The adler-32 checksum of the file as calculated
      * during the execution of the job.
@@ -87,13 +87,6 @@ namespace cta {
      */
     virtual void complete();
 
-    /**
-     * Indicates that the mount failed.
-     *
-     * @param ex The reason for the failure.
-     */
-    virtual void failed(const std::exception &ex);
-
     /**
      * Job factory
      *

scheduler/RetrieveMount.cpp

@@ -60,26 +60,20 @@ std::string cta::RetrieveMount::getMountTransactionId() const throw(){
   return "UNKNOWN_MOUNTTRANSACTIONID_FOR_RETRIEVE_MOUNT";
 }
 
+//------------------------------------------------------------------------------
+// getNextJob
+//------------------------------------------------------------------------------
 std::unique_ptr<cta::RetrieveJob> cta::RetrieveMount::getNextJob() {
-  cta::exception::Exception ex("Not implemented");
-  throw ex;
+  throw NotImplemented(std::string(__FUNCTION__) + ": Not implemented");
 }
 
-
 //------------------------------------------------------------------------------
 // complete
 //------------------------------------------------------------------------------
 void cta::RetrieveMount::complete() {
-  throw NotImplemented ("");
+  throw NotImplemented(std::string(__FUNCTION__) + ": Not implemented");
 }
 
-//------------------------------------------------------------------------------
-// failed
-//------------------------------------------------------------------------------
-void cta::RetrieveMount::failed(const std::exception &ex) {
-  throw NotImplemented ("");
-} 
-
 //------------------------------------------------------------------------------
 // destructor
 //------------------------------------------------------------------------------

scheduler/RetrieveMount.hpp

@@ -83,7 +83,7 @@ namespace cta {
     virtual std::string getMountTransactionId() const throw();
 
     /**
-     * Indicates that the mount was successful.
+     * Indicates that the mount was completed.
      *
      * @param checksumOfTransfer The adler-32 checksum of the file as calculated
      * during the execution of the job.
@@ -92,13 +92,6 @@ namespace cta {
      */
     virtual void complete();
 
-    /**
-     * Indicates that the mount failed.
-     *
-     * @param ex The reason for the failure.
-     */
-    virtual void failed(const std::exception &ex);
-
     /**
      * Job factory
      *

scheduler/TapeMount.hpp

@@ -59,7 +59,7 @@ namespace cta {
     virtual std::string getMountTransactionId() const throw() = 0;
 
     /**
-     * Indicates that the mount was successful.
+     * Indicates that the mount was completed.
      *
      * @param checksumOfTransfer The adler-32 checksum of the file as calculated
      * during the execution of the job.
@@ -68,13 +68,6 @@ namespace cta {
      */
     virtual void complete() = 0;
 
-    /**
-     * Indicates that the mount failed.
-     *
-     * @param ex The reason for the failure.
-     */
-    virtual void failed(const std::exception &ex) = 0;
-    
     /**
      * Destructor.
      */

tapeserver/castor/tape/tapeserver/daemon/DataTransferSession.cpp

@@ -191,7 +191,7 @@ castor::tape::tapeserver::daemon::Session::EndOfSessionAction
       log::LogContext::ScopedParam sp1(lc, log::Param("errorMessage", "Aborted: empty recall mount"));
       log::LogContext::ScopedParam sp2(lc, log::Param("errorCode", SEINTERNAL));
       try {
-        retrieveMount->failed(cta::exception::Exception("Aborted: empty recall mount"));
+        retrieveMount->complete();
         log::LogContext::ScopedParam sp08(lc, log::Param("MountTransactionId", retrieveMount->getMountTransactionId()));
         log::LogContext::ScopedParam sp11(lc, log::Param("errorMessage", "Aborted: empty recall mount"));
         log::LogContext::ScopedParam sp12(lc, log::Param("errorCode", SEINTERNAL));
@@ -316,7 +316,7 @@ castor::tape::tapeserver::daemon::Session::EndOfSessionAction
       log::LogContext::ScopedParam sp1(lc, log::Param("errorMessage", "Aborted: empty migration mount"));
       log::LogContext::ScopedParam sp2(lc, log::Param("errorCode", SEINTERNAL));
       try {
-        archiveMount->failed(cta::exception::Exception("Aborted: empty migration mount"));
+        archiveMount->complete();
         log::LogContext::ScopedParam sp1(lc, log::Param("MountTransactionId", archiveMount->getMountTransactionId()));
         lc.log(LOG_ERR, "Notified client of end session with error");
       } catch(castor::exception::Exception & ex) {
@@ -361,7 +361,7 @@ castor::tape::tapeserver::daemon::DataTransferSession::findDrive(const DriveConf
     
     std::stringstream errMsg;
     errMsg << "Drive not found on this path" << lc;
-    mount->failed(cta::exception::Exception("Drive unit not found"));
+    mount->complete();
     log::LogContext::ScopedParam sp10(lc, log::Param("tapebridgeTransId", mount->getMountTransactionId()));
     log::LogContext::ScopedParam sp13(lc, log::Param("errorMessage", errMsg.str()));
     log::LogContext::ScopedParam sp14(lc, log::Param("errorCode", SEINTERNAL));
@@ -376,7 +376,7 @@ castor::tape::tapeserver::daemon::DataTransferSession::findDrive(const DriveConf
     
     std::stringstream errMsg;
     errMsg << "Error looking to path to tape drive: " << lc;
-    mount->failed(cta::exception::Exception("Drive unit not found"));
+    mount->complete();
     log::LogContext::ScopedParam sp11(lc, log::Param("tapebridgeTransId", mount->getMountTransactionId()));
     log::LogContext::ScopedParam sp14(lc, log::Param("errorMessage", errMsg.str()));
     log::LogContext::ScopedParam sp15(lc, log::Param("errorCode", SEINTERNAL));
@@ -390,7 +390,7 @@ castor::tape::tapeserver::daemon::DataTransferSession::findDrive(const DriveConf
     
     std::stringstream errMsg;
     errMsg << "Unexpected exception while looking for drive" << lc;
-    mount->failed(cta::exception::Exception("Drive unit not found"));
+    mount->complete();
     log::LogContext::ScopedParam sp10(lc, log::Param("tapebridgeTransId", mount->getMountTransactionId()));
     log::LogContext::ScopedParam sp13(lc, log::Param("errorMessage", errMsg.str()));
     log::LogContext::ScopedParam sp14(lc, log::Param("errorCode", SEINTERNAL));
@@ -411,7 +411,7 @@ castor::tape::tapeserver::daemon::DataTransferSession::findDrive(const DriveConf
     
     std::stringstream errMsg;
     errMsg << "Error opening tape drive" << lc;
-    mount->failed(cta::exception::Exception("Drive unit not found"));
+    mount->complete();
     log::LogContext::ScopedParam sp11(lc, log::Param("tapebridgeTransId", mount->getMountTransactionId()));
     log::LogContext::ScopedParam sp14(lc, log::Param("errorMessage", errMsg.str()));
     log::LogContext::ScopedParam sp15(lc, log::Param("errorCode", SEINTERNAL));
@@ -425,7 +425,7 @@ castor::tape::tapeserver::daemon::DataTransferSession::findDrive(const DriveConf
     
     std::stringstream errMsg;
     errMsg << "Unexpected exception while opening drive" << lc;
-    mount->failed(cta::exception::Exception("Drive unit not found"));
+    mount->complete();
     log::LogContext::ScopedParam sp10(lc, log::Param("tapebridgeTransId", mount->getMountTransactionId()));
     log::LogContext::ScopedParam sp13(lc, log::Param("errorMessage", errMsg.str()));
     log::LogContext::ScopedParam sp14(lc, log::Param("errorCode", SEINTERNAL));

tapeserver/castor/tape/tapeserver/daemon/DiskWriteTask.cpp

@@ -152,7 +152,7 @@ bool DiskWriteTask::execute(RecallReportPacker& reporter,log::LogContext& lc,
           .add("errorCode", e.code());
     logWithStat(LOG_ERR, "File writing to disk failed.", lc);
     lc.logBacktrace(LOG_ERR, e.backtrace());
-    reporter.reportFailedJob(std::move(m_retrieveJob),e.getMessageValue(),e.code());
+    reporter.reportFailedJob(std::move(m_retrieveJob),e);
 
     
     //got an exception, return false

tapeserver/castor/tape/tapeserver/daemon/MigrationReportPacker.cpp

@@ -201,8 +201,8 @@ void MigrationReportPacker::ReportEndofSession::execute(MigrationReportPacker& r
     }
   }
   else {
-    // We have some errors: report end of session as such to the client
-    reportPacker.m_archiveMount->failed(cta::exception::Exception("Previous file errors"));
+    // We have some errors
+    reportPacker.m_archiveMount->complete();
     log::ScopedParamContainer sp(reportPacker.m_lc);
     sp.add("errorMessage", "Previous file errors")
       .add("errorCode", SEINTERNAL);
@@ -217,13 +217,14 @@ void MigrationReportPacker::ReportEndofSession::execute(MigrationReportPacker& r
   }
   reportPacker.m_continue=false;
 }
+
 //------------------------------------------------------------------------------
 //ReportEndofSessionWithErrors::execute
 //------------------------------------------------------------------------------
 void MigrationReportPacker::ReportEndofSessionWithErrors::execute(MigrationReportPacker& reportPacker){
   
   if(reportPacker.m_errorHappened) {
-    reportPacker.m_archiveMount->failed(cta::exception::Exception(m_message));
+    reportPacker.m_archiveMount->complete();
     log::ScopedParamContainer sp(reportPacker.m_lc);
     sp.add("errorMessage", m_message)
       .add("errorCode", m_errorCode);
@@ -235,7 +236,7 @@ void MigrationReportPacker::ReportEndofSessionWithErrors::execute(MigrationRepor
     if (ENOSPC != m_errorCode) {
       m_errorCode = SEINTERNAL;
     }
-    reportPacker.m_archiveMount->failed(cta::exception::Exception(msg)); 
+    reportPacker.m_archiveMount->complete(); 
     reportPacker.m_lc.log(LOG_INFO,msg);
   }
   if(reportPacker.m_watchdog) {
@@ -276,7 +277,7 @@ void MigrationReportPacker::WorkerThread::run(){
       catch(const failedMigrationRecallResult& e){
         //here we catch a failed report MigrationResult. We try to close and it that fails too
         //we end up in the catch below
-        m_parent.m_archiveMount->failed(e);
+        m_parent.m_archiveMount->complete();
         m_parent.m_lc.log(LOG_INFO,"Successfully closed client's session after the failed report MigrationResult");
         if (m_parent.m_watchdog) {
           m_parent.m_watchdog->addToErrorCount("Error_clientCommunication");

tapeserver/castor/tape/tapeserver/daemon/RecallReportPacker.cpp

@@ -69,8 +69,8 @@ void RecallReportPacker::reportCompletedJob(std::unique_ptr<cta::RetrieveJob> su
 //reportFailedJob
 //------------------------------------------------------------------------------  
 void RecallReportPacker::reportFailedJob(std::unique_ptr<cta::RetrieveJob> failedRetrieveJob
-,const std::string& msg,int error_code){
-  std::unique_ptr<Report> rep(new ReportError(std::move(failedRetrieveJob),msg,error_code));
+, const std::exception &ex){
+  std::unique_ptr<Report> rep(new ReportError(std::move(failedRetrieveJob),ex));
   castor::server::MutexLocker ml(&m_producterProtection);
   m_fifo.push(rep.release());
 }
@@ -115,7 +115,7 @@ void RecallReportPacker::ReportEndofSession::execute(RecallReportPacker& parent)
     else {
       const std::string& msg ="RecallReportPacker::EndofSession has been reported  but an error happened somewhere in the process";
       parent.m_lc.log(LOG_ERR,msg);
-      parent.m_retrieveMount->failed(cta::exception::Exception(msg));
+      parent.m_retrieveMount->complete();
       if (parent.m_watchdog) {
         parent.m_watchdog->addParameter(log::Param("status","failure"));
         // We have a race condition here between the processing of this message by
@@ -130,14 +130,14 @@ void RecallReportPacker::ReportEndofSession::execute(RecallReportPacker& parent)
 //------------------------------------------------------------------------------
 void RecallReportPacker::ReportEndofSessionWithErrors::execute(RecallReportPacker& parent){
   if(parent.m_errorHappened) {
-    parent.m_retrieveMount->failed(cta::exception::Exception(m_message));    
+    parent.m_retrieveMount->complete();
     LogContext::ScopedParam(parent.m_lc,Param("errorCode",m_error_code));
     parent.m_lc.log(LOG_ERR,m_message);
   }
   else{
    const std::string& msg ="RecallReportPacker::EndofSessionWithErrors has been reported  but NO error was detected during the process";
    parent.m_lc.log(LOG_ERR,msg);  
-   parent.m_retrieveMount->failed(cta::exception::Exception(msg));    
+   parent.m_retrieveMount->complete();
   }
   if (parent.m_watchdog) {
     parent.m_watchdog->addParameter(log::Param("status","failure"));
@@ -152,7 +152,7 @@ void RecallReportPacker::ReportEndofSessionWithErrors::execute(RecallReportPacke
 //------------------------------------------------------------------------------
 void RecallReportPacker::ReportError::execute(RecallReportPacker& parent){
   parent.m_errorHappened=true;
-  m_failedRetrieveJob->failed(cta::exception::Exception(m_error_msg));
+  m_failedRetrieveJob->failed(m_ex);
 }
 //------------------------------------------------------------------------------
 //WorkerThread::WorkerThread

tapeserver/castor/tape/tapeserver/daemon/RecallReportPacker.hpp

@@ -60,10 +60,9 @@ public:
    * Create into the MigrationReportPacker a report for the failed migration
    * of migratedFile
    * @param migratedFile the file which failed 
-   * @param msg the message error with the failure 
-   * @param error_code the error code with the failure 
+   * @param ex the reason for the failure
    */
-  virtual void reportFailedJob(std::unique_ptr<cta::RetrieveJob> failedRetrieveJob, const std::string& msg,int error_code);
+  virtual void reportFailedJob(std::unique_ptr<cta::RetrieveJob> failedRetrieveJob, const std::exception& ex);
        
   /**
    * Create into the MigrationReportPacker a report for the nominal end of session
@@ -117,16 +116,18 @@ private:
     virtual void execute(RecallReportPacker& reportPacker);
   };
   class ReportError : public Report {
-    const std::string m_error_msg;
-    const int m_error_code;
+    const std::exception m_ex;
     
     /**
      * The failed retrieve job to be reported immediately
      */
     std::unique_ptr<cta::RetrieveJob> m_failedRetrieveJob;
   public:
-    ReportError(std::unique_ptr<cta::RetrieveJob> failedRetrieveJob,std::string msg,int error_code):
-    Report(false),m_error_msg(msg),m_error_code(error_code), m_failedRetrieveJob(std::move(failedRetrieveJob)) {}
+    ReportError(std::unique_ptr<cta::RetrieveJob> failedRetrieveJob, const std::exception &ex):
+    Report(false),
+    m_ex(ex),
+    m_failedRetrieveJob(std::move(failedRetrieveJob)) {
+    }
 
     virtual void execute(RecallReportPacker& reportPacker);
   };

tapeserver/castor/tape/tapeserver/daemon/RecallReportPackerTest.cpp

@@ -23,6 +23,7 @@
 
 #include "castor/tape/tapeserver/daemon/RecallReportPacker.hpp"
 #include "castor/log/StringLogger.hpp"
+#include "common/exception/Exception.hpp"
 #include "serrno.h"
 #include "scheduler/mockDB/MockSchedulerDatabase.hpp"
 
@@ -52,6 +53,7 @@ protected:
     
 
     MOCK_METHOD2(complete, void(const uint32_t checksumOfTransfer, const uint64_t fileSizeOfTransfer));
+    MOCK_METHOD1(fail, void(const std::exception &ex));
   }; // class MockRetrieveJob
 
   class MockRetrieveMount: public cta::RetrieveMount {
@@ -112,7 +114,8 @@ TEST_F(castor_tape_tapeserver_daemonTest, RecallReportPackerNominal) {
 
   castor::log::StringLogger log("castor_tape_tapeserver_RecallReportPackerNominal");
   castor::log::LogContext lc(log);
-  castor::tape::tapeserver::daemon::RecallReportPacker rrp(dynamic_cast<cta::RetrieveMount *>(&retrieveMount),lc);
+  castor::tape::tapeserver::daemon::RecallReportPacker rrp(
+    dynamic_cast<cta::RetrieveMount *>(&retrieveMount),lc);
   rrp.startThreads();
 
   rrp.reportCompletedJob(std::move(job1),0,0);
@@ -125,40 +128,51 @@ TEST_F(castor_tape_tapeserver_daemonTest, RecallReportPackerNominal) {
   ASSERT_NE(std::string::npos, temp.find("Nominal RecallReportPacker::EndofSession has been reported"));
 }
 
-/*
 TEST_F(castor_tape_tapeserver_daemonTest, RecallReportPackerBadBadEnd) {
   MockRetrieveMount retrieveMount;
 
   ::testing::InSequence dummy;
-  EXPECT_CALL(retrieveMount, internalGetNextJob()).Times(4);
+  std::unique_ptr<cta::RetrieveJob> job1;
+  {
+    std::unique_ptr<MockRetrieveJob> mockJob(new MockRetrieveJob());
+    EXPECT_CALL(*mockJob, complete(_,_)).Times(1);
+    job1.reset(mockJob.release());
+  }
+  std::unique_ptr<cta::RetrieveJob> job2;
+  {
+    std::unique_ptr<MockRetrieveJob> mockJob(new MockRetrieveJob());
+    EXPECT_CALL(*mockJob, complete(_,_)).Times(1);
+    job2.reset(mockJob.release());
+  }
+  std::unique_ptr<cta::RetrieveJob> job3;
+  {
+    std::unique_ptr<MockRetrieveJob> mockJob(new MockRetrieveJob());
+    EXPECT_CALL(*mockJob, fail(_)).Times(1);
+    job3.reset(mockJob.release());
+  }
   EXPECT_CALL(retrieveMount, complete()).Times(1);
 
-  std::string error_msg="ERROR_TEST_MSG";
-  int error_code=std::numeric_limits<int>::max();
-
-  ::testing::InSequence dummy;
-  EXPECT_CALL(client, reportRecallResults(_,_)).Times(2);
-  EXPECT_CALL(client, reportEndOfSessionWithError(error_msg,error_code,_)).Times(1);
-  
   castor::log::StringLogger log("castor_tape_tapeserver_RecallReportPackerBadBadEnd");
   castor::log::LogContext lc(log);
 
   std::unique_ptr<cta::MockSchedulerDatabase> mdb(new cta::MockSchedulerDatabase);
-  tapeserver::daemon::RecallReportPacker rrp(dynamic_cast<cta::RetrieveMount *>((mdb->getNextMount("ll","drive")).get()),lc);
+  castor::tape::tapeserver::daemon::RecallReportPacker rrp(
+    dynamic_cast<cta::RetrieveMount *>(&retrieveMount),lc);
   rrp.startThreads();
 
-  tapegateway::FileToRecallStruct recalledFiled;
-  rrp.reportCompletedJob(recalledFiled,0,0);
-  rrp.reportCompletedJob(recalledFiled,0,0);
-  rrp.reportCompletedJob(recalledFiled,0,0);
-  rrp.reportFailedJob(recalledFiled,error_msg,error_code);
-  rrp.reportEndOfSessionWithErrors(error_msg,error_code);
+  rrp.reportCompletedJob(std::move(job1),0,0);
+  rrp.reportCompletedJob(std::move(job2),0,0);
+
+  const std::string error_msg = "ERROR_TEST_MSG";
+  const cta::exception::Exception ex(error_msg);
+  rrp.reportFailedJob(std::move(job3),ex);
+
+  rrp.reportEndOfSession();
   rrp.waitThread();
 
-  std::string temp = log.getLog();
+  const std::string temp = log.getLog();
   ASSERT_NE(std::string::npos, temp.find(error_msg));
 }
-*/
 
 /*
 TEST_F(castor_tape_tapeserver_daemonTest, RecallReportPackerBadGoodEnd) {