changeset 2598:34dc57f4a7d2 jobs

simplification of JobStepResult
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 16 May 2018 16:23:20 +0200
parents ef7ba1b21d58
children 593d6b0f4cba
files Core/JobsEngine/IJob.h Core/JobsEngine/JobStepResult.cpp Core/JobsEngine/JobStepResult.h Core/JobsEngine/JobStepRetry.h Core/JobsEngine/JobsEngine.cpp Core/JobsEngine/SetOfInstancesJob.cpp Core/JobsEngine/SetOfInstancesJob.h Resources/CMake/OrthancFrameworkConfiguration.cmake UnitTestsSources/MultiThreadingTests.cpp
diffstat 9 files changed, 155 insertions(+), 115 deletions(-) [+]
line wrap: on
line diff
--- a/Core/JobsEngine/IJob.h	Wed May 16 14:40:05 2018 +0200
+++ b/Core/JobsEngine/IJob.h	Wed May 16 16:23:20 2018 +0200
@@ -50,7 +50,7 @@
     // Method called once the job enters the jobs engine
     virtual void Start() = 0;
     
-    virtual JobStepResult* ExecuteStep() = 0;
+    virtual JobStepResult ExecuteStep() = 0;
 
     // Method called once the job is resubmitted after a failure
     virtual void SignalResubmit() = 0;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/Core/JobsEngine/JobStepResult.cpp	Wed May 16 16:23:20 2018 +0200
@@ -0,0 +1,81 @@
+/**
+ * Orthanc - A Lightweight, RESTful DICOM Store
+ * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
+ * Department, University Hospital of Liege, Belgium
+ * Copyright (C) 2017-2018 Osimis S.A., Belgium
+ *
+ * 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.
+ *
+ * In addition, as a special exception, the copyright holders of this
+ * program give permission to link the code of its release with the
+ * OpenSSL project's "OpenSSL" library (or with modified versions of it
+ * that use the same license as the "OpenSSL" library), and distribute
+ * the linked executables. You must obey the GNU General Public License
+ * in all respects for all of the code used other than "OpenSSL". If you
+ * modify file(s) with this exception, you may extend this exception to
+ * your version of the file(s), but you are not obligated to do so. If
+ * you do not wish to do so, delete this exception statement from your
+ * version. If you delete this exception statement from all source files
+ * in the program, then also delete it here.
+ * 
+ * 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 "../PrecompiledHeaders.h"
+#include "JobStepResult.h"
+
+#include "../OrthancException.h"
+
+namespace Orthanc
+{
+  JobStepResult JobStepResult::Retry(unsigned int timeout)
+  {
+    JobStepResult result(JobStepCode_Retry);
+    result.timeout_ = timeout;
+    return result;
+  }
+
+
+  JobStepResult JobStepResult::Failure(const ErrorCode& error)
+  {
+    JobStepResult result(JobStepCode_Failure);
+    result.error_ = error;
+    return result;
+  }
+
+
+  unsigned int JobStepResult::GetRetryTimeout() const
+  {
+    if (code_ == JobStepCode_Retry)
+    {
+      return timeout_;
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+  }
+
+
+  ErrorCode JobStepResult::GetFailureCode() const
+  {
+    if (code_ == JobStepCode_Failure)
+    {
+      return error_;
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
+    }
+  }
+}
--- a/Core/JobsEngine/JobStepResult.h	Wed May 16 14:40:05 2018 +0200
+++ b/Core/JobsEngine/JobStepResult.h	Wed May 16 16:23:20 2018 +0200
@@ -40,21 +40,46 @@
   class JobStepResult
   {
   private:
-    JobStepCode code_;
+    JobStepCode   code_;
+    unsigned int  timeout_;
+    ErrorCode     error_;
     
-  public:
     explicit JobStepResult(JobStepCode code) :
-      code_(code)
+      code_(code),
+      timeout_(0),
+      error_(ErrorCode_Success)
     {
     }
 
-    virtual ~JobStepResult()
+  public:
+    explicit JobStepResult() :
+      code_(JobStepCode_Failure),
+      timeout_(0),
+      error_(ErrorCode_InternalError)
     {
     }
 
+    static JobStepResult Success()
+    {
+      return JobStepResult(JobStepCode_Success);
+    }
+
+    static JobStepResult Continue()
+    {
+      return JobStepResult(JobStepCode_Continue);
+    }
+
+    static JobStepResult Retry(unsigned int timeout);
+
+    static JobStepResult Failure(const ErrorCode& error);
+
     JobStepCode GetCode() const
     {
       return code_;
     }
+
+    unsigned int GetRetryTimeout() const;
+
+    ErrorCode GetFailureCode() const;
   };
 }
--- a/Core/JobsEngine/JobStepRetry.h	Wed May 16 14:40:05 2018 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,57 +0,0 @@
-/**
- * Orthanc - A Lightweight, RESTful DICOM Store
- * Copyright (C) 2012-2016 Sebastien Jodogne, Medical Physics
- * Department, University Hospital of Liege, Belgium
- * Copyright (C) 2017-2018 Osimis S.A., Belgium
- *
- * 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.
- *
- * In addition, as a special exception, the copyright holders of this
- * program give permission to link the code of its release with the
- * OpenSSL project's "OpenSSL" library (or with modified versions of it
- * that use the same license as the "OpenSSL" library), and distribute
- * the linked executables. You must obey the GNU General Public License
- * in all respects for all of the code used other than "OpenSSL". If you
- * modify file(s) with this exception, you may extend this exception to
- * your version of the file(s), but you are not obligated to do so. If
- * you do not wish to do so, delete this exception statement from your
- * version. If you delete this exception statement from all source files
- * in the program, then also delete it here.
- * 
- * 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 "JobStepResult.h"
-
-namespace Orthanc
-{
-  class JobStepRetry : public JobStepResult
-  {
-  private:
-    unsigned int  timeout_;   // Retry after "timeout_" milliseconds
-
-  public:
-    JobStepRetry(unsigned int timeout) :
-      JobStepResult(JobStepCode_Retry),
-      timeout_(timeout)
-    {
-    }
-
-    unsigned int  GetTimeout() const
-    {
-      return timeout_;
-    }
-  };
-}
--- a/Core/JobsEngine/JobsEngine.cpp	Wed May 16 14:40:05 2018 +0200
+++ b/Core/JobsEngine/JobsEngine.cpp	Wed May 16 16:23:20 2018 +0200
@@ -34,8 +34,6 @@
 #include "../PrecompiledHeaders.h"
 #include "JobsEngine.h"
 
-#include "JobStepRetry.h"
-
 #include "../Logging.h"
 #include "../OrthancException.h"
 
@@ -67,59 +65,47 @@
       return false;
     }
 
-    std::auto_ptr<JobStepResult> result;
-
-    {
-      try
-      {
-        result.reset(running.GetJob().ExecuteStep());
+    JobStepResult result;
 
-        if (result->GetCode() == JobStepCode_Failure)
-        {
-          running.UpdateStatus(ErrorCode_InternalError);
-        }
-        else
-        {
-          running.UpdateStatus(ErrorCode_Success);
-        }
-      }
-      catch (OrthancException& e)
-      {
-        running.UpdateStatus(e.GetErrorCode());
-      }
-      catch (boost::bad_lexical_cast&)
-      {
-        running.UpdateStatus(ErrorCode_BadFileFormat);
-      }
-      catch (...)
-      {
-        running.UpdateStatus(ErrorCode_InternalError);
-      }
-
-      if (result.get() == NULL)
-      {
-        result.reset(new JobStepResult(JobStepCode_Failure));
-      }
+    try
+    {
+      result = running.GetJob().ExecuteStep();
+    }
+    catch (OrthancException& e)
+    {
+      result = JobStepResult::Failure(e.GetErrorCode());
+    }
+    catch (boost::bad_lexical_cast&)
+    {
+      result = JobStepResult::Failure(ErrorCode_BadFileFormat);
+    }
+    catch (...)
+    {
+      result = JobStepResult::Failure(ErrorCode_InternalError);
     }
 
-    switch (result->GetCode())
+    switch (result.GetCode())
     {
       case JobStepCode_Success:
+        running.UpdateStatus(ErrorCode_Success);
         running.GetJob().ReleaseResources();
         running.MarkSuccess();
         return false;
 
       case JobStepCode_Failure:
         running.GetJob().ReleaseResources();
+        running.UpdateStatus(result.GetFailureCode());
         running.MarkFailure();
         return false;
 
       case JobStepCode_Retry:
         running.GetJob().ReleaseResources();
-        running.MarkRetry(dynamic_cast<JobStepRetry&>(*result).GetTimeout());
+        running.UpdateStatus(ErrorCode_Success);
+        running.MarkRetry(result.GetRetryTimeout());
         return false;
 
       case JobStepCode_Continue:
+        running.UpdateStatus(ErrorCode_Success);
         return true;
             
       default:
--- a/Core/JobsEngine/SetOfInstancesJob.cpp	Wed May 16 14:40:05 2018 +0200
+++ b/Core/JobsEngine/SetOfInstancesJob.cpp	Wed May 16 16:23:20 2018 +0200
@@ -113,7 +113,7 @@
   }
 
 
-  JobStepResult* SetOfInstancesJob::ExecuteStep()
+  JobStepResult SetOfInstancesJob::ExecuteStep()
   {
     if (!started_)
     {
@@ -125,13 +125,13 @@
     {
       // No instance to handle, we're done
       position_ = 1;
-      return new JobStepResult(JobStepCode_Success);
+      return JobStepResult::Success();
     }
 
     if (position_ >= instances_.size())
     {
       // Already done
-      return new JobStepResult(JobStepCode_Failure);
+      throw OrthancException(ErrorCode_BadSequenceOfCalls);
     }
 
     const std::string currentInstance = instances_[position_];
@@ -144,7 +144,7 @@
 
       if (!ok && !permissive_)
       {
-        throw OrthancException(ErrorCode_InternalError);
+        return JobStepResult::Failure(ErrorCode_InternalError);
       }
     }
     catch (OrthancException& e)
@@ -169,11 +169,11 @@
     if (position_ == instances_.size())
     {
       // We're done
-      return new JobStepResult(JobStepCode_Success);
+      return JobStepResult::Success();
     }
     else
     {
-      return new JobStepResult(JobStepCode_Continue);
+      return JobStepResult::Continue();
     }
   }
 
--- a/Core/JobsEngine/SetOfInstancesJob.h	Wed May 16 14:40:05 2018 +0200
+++ b/Core/JobsEngine/SetOfInstancesJob.h	Wed May 16 16:23:20 2018 +0200
@@ -94,7 +94,7 @@
       return failedInstances_;
     }
   
-    virtual JobStepResult* ExecuteStep();
+    virtual JobStepResult ExecuteStep();
     
     virtual void GetInternalContent(Json::Value& value);
   };
--- a/Resources/CMake/OrthancFrameworkConfiguration.cmake	Wed May 16 14:40:05 2018 +0200
+++ b/Resources/CMake/OrthancFrameworkConfiguration.cmake	Wed May 16 16:23:20 2018 +0200
@@ -500,6 +500,7 @@
     ${ORTHANC_ROOT}/Core/FileStorage/FilesystemStorage.cpp
     ${ORTHANC_ROOT}/Core/JobsEngine/JobInfo.cpp
     ${ORTHANC_ROOT}/Core/JobsEngine/JobStatus.cpp
+    ${ORTHANC_ROOT}/Core/JobsEngine/JobStepResult.cpp
     ${ORTHANC_ROOT}/Core/JobsEngine/JobsEngine.cpp
     ${ORTHANC_ROOT}/Core/JobsEngine/JobsRegistry.cpp
     ${ORTHANC_ROOT}/Core/JobsEngine/SetOfInstancesJob.cpp
--- a/UnitTestsSources/MultiThreadingTests.cpp	Wed May 16 14:40:05 2018 +0200
+++ b/UnitTestsSources/MultiThreadingTests.cpp	Wed May 16 16:23:20 2018 +0200
@@ -34,7 +34,6 @@
 #include "PrecompiledHeadersUnitTests.h"
 #include "gtest/gtest.h"
 
-#include "../Core/JobsEngine/JobStepRetry.h"
 #include "../Core/JobsEngine/JobsEngine.h"
 #include "../Core/MultiThreading/Locker.h"
 #include "../Core/OrthancException.h"
@@ -244,20 +243,20 @@
 class DummyJob : public Orthanc::IJob
 {
 private:
-  JobStepResult  result_;
+  bool         fails_;
   unsigned int count_;
   unsigned int steps_;
 
 public:
   DummyJob() :
-    result_(Orthanc::JobStepCode_Success),
+    fails_(false),
     count_(0),
     steps_(4)
   {
   }
 
-  explicit DummyJob(JobStepResult result) :
-    result_(result),
+  explicit DummyJob(bool fails) :
+  fails_(fails),
     count_(0),
     steps_(4)
   {
@@ -271,18 +270,22 @@
   {
   }
     
-  virtual JobStepResult* ExecuteStep()
+  virtual JobStepResult ExecuteStep()
   {
     boost::this_thread::sleep(boost::posix_time::milliseconds(10));
 
-    if (count_ == steps_ - 1)
+    if (fails_)
     {
-      return new JobStepResult(result_);
+      return JobStepResult::Failure(ErrorCode_ParameterOutOfRange);
+    }
+    else if (count_ == steps_ - 1)
+    {
+      return JobStepResult::Success();
     }
     else
     {
       count_++;
-      return new JobStepResult(JobStepCode_Continue);
+      return JobStepResult::Continue();
     }
   }
 
@@ -767,7 +770,8 @@
 
   if (1)
   {
-    printf(">> %d\n", engine.GetRegistry().SubmitAndWait(new DummyJob(JobStepResult(Orthanc::JobStepCode_Failure)), rand() % 10));
+    ASSERT_TRUE(engine.GetRegistry().SubmitAndWait(new DummyJob(), rand() % 10));
+    ASSERT_FALSE(engine.GetRegistry().SubmitAndWait(new DummyJob(true), rand() % 10));
   }
 
   boost::this_thread::sleep(boost::posix_time::milliseconds(100));