Mercurial > hg > orthanc

changeset 2842:ff0ed5ea9e4e

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
trailing step in SetOfInstancesJob
author Sebastien Jodogne <s.jodogne@gmail.com>
date Wed, 26 Sep 2018 15:55:42 +0200
parents bf019ee38498
children 4ee3a759afea
files Core/JobsEngine/SetOfInstancesJob.cpp Core/JobsEngine/SetOfInstancesJob.h OrthancServer/ServerJobs/DicomModalityStoreJob.cpp OrthancServer/ServerJobs/DicomModalityStoreJob.h OrthancServer/ServerJobs/OrthancPeerStoreJob.cpp OrthancServer/ServerJobs/OrthancPeerStoreJob.h OrthancServer/ServerJobs/ResourceModificationJob.cpp OrthancServer/ServerJobs/ResourceModificationJob.h UnitTestsSources/MultiThreadingTests.cpp
diffstat 9 files changed, 335 insertions(+), 38 deletions(-) [+]
line wrap: on
line diff
--- a/Core/JobsEngine/SetOfInstancesJob.cpp	Wed Sep 26 09:50:30 2018 +0200
+++ b/Core/JobsEngine/SetOfInstancesJob.cpp	Wed Sep 26 15:55:42 2018 +0200
@@ -39,7 +39,8 @@
 
 namespace Orthanc
 {
-  SetOfInstancesJob::SetOfInstancesJob() :
+  SetOfInstancesJob::SetOfInstancesJob(bool hasTrailingStep) :
+    hasTrailingStep_(hasTrailingStep),
     started_(false),
     permissive_(false),
     position_(0)
@@ -60,6 +61,19 @@
   }
 
     
+  size_t SetOfInstancesJob::GetStepsCount() const
+  {
+    if (HasTrailingStep())
+    {
+      return instances_.size() + 1;
+    }
+    else
+    {
+      return instances_.size();
+    }
+  }
+  
+
   void SetOfInstancesJob::AddInstance(const std::string& instance)
   {
     if (started_)
@@ -75,7 +89,7 @@
 
   void SetOfInstancesJob::SetPermissive(bool permissive)
   {
-    if (IsStarted())
+    if (started_)
     {
       throw OrthancException(ErrorCode_BadSequenceOfCalls);
     }
@@ -102,21 +116,23 @@
     
   float SetOfInstancesJob::GetProgress()
   {
-    if (instances_.size() == 0)
+    const size_t steps = GetStepsCount();
+    
+    if (steps == 0)
     {
       return 0;
     }
     else
     {
       return (static_cast<float>(position_) /
-              static_cast<float>(instances_.size()));
+              static_cast<float>(steps));
     }
   }
 
 
   const std::string& SetOfInstancesJob::GetInstance(size_t index) const
   {
-    if (index > instances_.size())
+    if (index >= instances_.size())
     {
       throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
@@ -134,27 +150,38 @@
       throw OrthancException(ErrorCode_InternalError);
     }
     
-    if (instances_.empty() &&
+    const size_t steps = GetStepsCount();
+
+    if (steps == 0 &&
         position_ == 0)
     {
-      // No instance to handle, we're done
+      // Nothing to handle (no instance, nor trailing step): We're done
       position_ = 1;
       return JobStepResult::Success();
     }
-
-    if (position_ >= instances_.size())
+    
+    if (position_ >= steps)
     {
       // Already done
       throw OrthancException(ErrorCode_BadSequenceOfCalls);
     }
 
-    const std::string currentInstance = instances_[position_];
+    bool isTrailingStep = (hasTrailingStep_ &&
+                           position_ + 1 == steps);
     
     bool ok;
       
     try
     {
-      ok = HandleInstance(currentInstance);
+      if (isTrailingStep)
+      {
+        ok = HandleTrailingStep();
+      }
+      else
+      {
+        // Not at the trailing step: Handle the current instance
+        ok = HandleInstance(instances_[position_]);
+      }
 
       if (!ok && !permissive_)
       {
@@ -173,14 +200,15 @@
       }
     }
 
-    if (!ok)
+    if (!ok &&
+        !isTrailingStep)
     {
-      failedInstances_.insert(currentInstance);
+      failedInstances_.insert(instances_[position_]);
     }
 
     position_ += 1;
 
-    if (position_ == instances_.size())
+    if (position_ == steps)
     {
       // We're done
       return JobStepResult::Success();
@@ -191,10 +219,20 @@
     }
   }
 
-    
+
+
+  static const char* KEY_DESCRIPTION = "Description";
+  static const char* KEY_PERMISSIVE = "Permissive";
+  static const char* KEY_POSITION = "Position";
+  static const char* KEY_TYPE = "Type";
+  static const char* KEY_INSTANCES = "Instances";
+  static const char* KEY_FAILED_INSTANCES = "FailedInstances";
+  static const char* KEY_TRAILING_STEP = "TrailingStep";
+
+  
   void SetOfInstancesJob::GetPublicContent(Json::Value& value)
   {
-    value["Description"] = GetDescription();
+    value[KEY_DESCRIPTION] = GetDescription();
     value["InstancesCount"] = static_cast<uint32_t>(instances_.size());
     value["FailedInstancesCount"] = static_cast<uint32_t>(failedInstances_.size());
   }    
@@ -206,14 +244,15 @@
 
     std::string type;
     GetJobType(type);
-    value["Type"] = type;
+    value[KEY_TYPE] = type;
+    
+    value[KEY_PERMISSIVE] = permissive_;
+    value[KEY_POSITION] = static_cast<unsigned int>(position_);
+    value[KEY_DESCRIPTION] = description_;
+    value[KEY_TRAILING_STEP] = hasTrailingStep_;
 
-    value["Permissive"] = permissive_;
-    value["Position"] = static_cast<unsigned int>(position_);
-    value["Description"] = description_;
-
-    SerializationToolbox::WriteArrayOfStrings(value, instances_, "Instances");
-    SerializationToolbox::WriteSetOfStrings(value, failedInstances_, "FailedInstances");
+    SerializationToolbox::WriteArrayOfStrings(value, instances_, KEY_INSTANCES);
+    SerializationToolbox::WriteSetOfStrings(value, failedInstances_, KEY_FAILED_INSTANCES);
 
     return true;
   }
@@ -221,14 +260,24 @@
 
   SetOfInstancesJob::SetOfInstancesJob(const Json::Value& value) :
     started_(false),
-    permissive_(SerializationToolbox::ReadBoolean(value, "Permissive")),
-    position_(SerializationToolbox::ReadUnsignedInteger(value, "Position")),
-    description_(SerializationToolbox::ReadString(value, "Description"))
+    permissive_(SerializationToolbox::ReadBoolean(value, KEY_PERMISSIVE)),
+    position_(SerializationToolbox::ReadUnsignedInteger(value, KEY_POSITION)),
+    description_(SerializationToolbox::ReadString(value, KEY_DESCRIPTION))
   {
-    SerializationToolbox::ReadArrayOfStrings(instances_, value, "Instances");
-    SerializationToolbox::ReadSetOfStrings(failedInstances_, value, "FailedInstances");
+    SerializationToolbox::ReadArrayOfStrings(instances_, value, KEY_INSTANCES);
+    SerializationToolbox::ReadSetOfStrings(failedInstances_, value, KEY_FAILED_INSTANCES);
 
-    if (position_ > instances_.size())
+    if (value.isMember(KEY_TRAILING_STEP))
+    {
+      hasTrailingStep_ = SerializationToolbox::ReadBoolean(value, KEY_TRAILING_STEP);
+    }
+    else
+    {
+      // Backward compatibility with Orthanc <= 1.4.2
+      hasTrailingStep_ = false;
+    }
+
+    if (position_ > GetStepsCount() + 1)
     {
       throw OrthancException(ErrorCode_BadFileFormat);
     }
--- a/Core/JobsEngine/SetOfInstancesJob.h	Wed Sep 26 09:50:30 2018 +0200
+++ b/Core/JobsEngine/SetOfInstancesJob.h	Wed Sep 26 15:55:42 2018 +0200
@@ -42,6 +42,7 @@
   class SetOfInstancesJob : public IJob
   {
   private:
+    bool                      hasTrailingStep_;
     bool                      started_;
     std::vector<std::string>  instances_;
     bool                      permissive_;
@@ -52,11 +53,18 @@
   protected:
     virtual bool HandleInstance(const std::string& instance) = 0;
 
+    virtual bool HandleTrailingStep() = 0;
+
   public:
-    SetOfInstancesJob();
+    SetOfInstancesJob(bool hasTrailingStep);
 
     SetOfInstancesJob(const Json::Value& s);  // Unserialization
 
+    bool HasTrailingStep() const
+    {
+      return hasTrailingStep_;
+    }
+    
     size_t GetPosition() const
     {
       return position_;
@@ -79,6 +87,8 @@
       return instances_.size();
     }
     
+    size_t GetStepsCount() const;
+
     void AddInstance(const std::string& instance);
 
     bool IsPermissive() const
--- a/OrthancServer/ServerJobs/DicomModalityStoreJob.cpp	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/DicomModalityStoreJob.cpp	Wed Sep 26 15:55:42 2018 +0200
@@ -85,7 +85,14 @@
   }
     
 
+  bool DicomModalityStoreJob::HandleTrailingStep()
+  {
+    throw OrthancException(ErrorCode_InternalError);
+  }
+
+
   DicomModalityStoreJob::DicomModalityStoreJob(ServerContext& context) :
+    SetOfInstancesJob(false /* no trailing step */),
     context_(context),
     localAet_("ORTHANC"),
     moveOriginatorId_(0)  // By default, not a C-MOVE
--- a/OrthancServer/ServerJobs/DicomModalityStoreJob.h	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/DicomModalityStoreJob.h	Wed Sep 26 15:55:42 2018 +0200
@@ -55,6 +55,8 @@
   protected:
     virtual bool HandleInstance(const std::string& instance);
     
+    virtual bool HandleTrailingStep();
+
   public:
     DicomModalityStoreJob(ServerContext& context);
 
--- a/OrthancServer/ServerJobs/OrthancPeerStoreJob.cpp	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/OrthancPeerStoreJob.cpp	Wed Sep 26 15:55:42 2018 +0200
@@ -74,6 +74,12 @@
   }
     
 
+  bool OrthancPeerStoreJob::HandleTrailingStep()
+  {
+    throw OrthancException(ErrorCode_InternalError);
+  }
+
+
   void OrthancPeerStoreJob::SetPeer(const WebServiceParameters& peer)
   {
     if (IsStarted())
--- a/OrthancServer/ServerJobs/OrthancPeerStoreJob.h	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/OrthancPeerStoreJob.h	Wed Sep 26 15:55:42 2018 +0200
@@ -51,8 +51,11 @@
   protected:
     virtual bool HandleInstance(const std::string& instance);
     
+    virtual bool HandleTrailingStep();
+
   public:
     OrthancPeerStoreJob(ServerContext& context) :
+      SetOfInstancesJob(false /* no trailing step */),
       context_(context)
     {
     }
--- a/OrthancServer/ServerJobs/ResourceModificationJob.cpp	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/ResourceModificationJob.cpp	Wed Sep 26 15:55:42 2018 +0200
@@ -215,9 +215,14 @@
 
     return true;
   }
-    
 
   
+  bool ResourceModificationJob::HandleTrailingStep()
+  {
+    throw OrthancException(ErrorCode_InternalError);
+  }
+
+
   void ResourceModificationJob::SetModification(DicomModification* modification,
                                                 bool isAnonymization)
   {
--- a/OrthancServer/ServerJobs/ResourceModificationJob.h	Wed Sep 26 09:50:30 2018 +0200
+++ b/OrthancServer/ServerJobs/ResourceModificationJob.h	Wed Sep 26 15:55:42 2018 +0200
@@ -75,8 +75,11 @@
   protected:
     virtual bool HandleInstance(const std::string& instance);
     
+    virtual bool HandleTrailingStep();
+
   public:
     ResourceModificationJob(ServerContext& context) :
+      SetOfInstancesJob(false /* no trailing step */),
       context_(context),
       isAnonymization_(false)
     {
--- a/UnitTestsSources/MultiThreadingTests.cpp	Wed Sep 26 09:50:30 2018 +0200
+++ b/UnitTestsSources/MultiThreadingTests.cpp	Wed Sep 26 15:55:42 2018 +0200
@@ -146,20 +146,58 @@
 
   class DummyInstancesJob : public SetOfInstancesJob
   {
+  private:
+    bool   trailingStepDone_;
+    
   protected:
     virtual bool HandleInstance(const std::string& instance)
     {
       return (instance != "nope");
     }
 
+    virtual bool HandleTrailingStep()
+    {
+      if (HasTrailingStep())
+      {
+        if (trailingStepDone_)
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
+        else
+        {
+          trailingStepDone_ = true;
+          return true;
+        }
+      }
+      else
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
+    }
+
   public:
-    DummyInstancesJob()
+    DummyInstancesJob(bool hasTrailingStep) :
+      SetOfInstancesJob(hasTrailingStep),
+      trailingStepDone_(false)
     {
     }
     
     DummyInstancesJob(const Json::Value& value) :
       SetOfInstancesJob(value)
     {
+      if (HasTrailingStep())
+      {
+        trailingStepDone_ = (GetPosition() == GetStepsCount());
+      }
+      else
+      {
+        trailingStepDone_ = false;
+      }
+    }
+
+    bool IsTrailingStepDone() const
+    {
+      return trailingStepDone_;
     }
     
     virtual void Stop(JobStopReason reason)
@@ -790,7 +828,38 @@
     Json::Value b = 43;
     if (unserialized->Serialize(b))
     {
-      return CheckSameJson(a, b);
+      return (CheckSameJson(a, b));
+    }
+    else
+    {
+      return false;
+    }
+  }
+}
+
+
+static bool CheckIdempotentSetOfInstances(IJobUnserializer& unserializer,
+                                          SetOfInstancesJob& job)
+{
+  Json::Value a = 42;
+  
+  if (!job.Serialize(a))
+  {
+    return false;
+  }
+  else
+  {
+    std::auto_ptr<SetOfInstancesJob> unserialized
+      (dynamic_cast<SetOfInstancesJob*>(unserializer.UnserializeJob(a)));
+  
+    Json::Value b = 43;
+    if (unserialized->Serialize(b))
+    {    
+      return (CheckSameJson(a, b) &&
+              job.HasTrailingStep() == unserialized->HasTrailingStep() &&
+              job.GetPosition() == unserialized->GetPosition() &&
+              job.GetInstancesCount() == unserialized->GetInstancesCount() &&
+              job.GetStepsCount() == unserialized->GetStepsCount());
     }
     else
     {
@@ -956,20 +1025,22 @@
   // This tests SetOfInstancesJob
   
   {
-    DummyInstancesJob job;
+    DummyInstancesJob job(false);
     job.SetDescription("description");
     job.AddInstance("hello");
     job.AddInstance("nope");
     job.AddInstance("world");
     job.SetPermissive(true);
     ASSERT_THROW(job.Step(), OrthancException);  // Not started yet
+    ASSERT_FALSE(job.HasTrailingStep());
+    ASSERT_FALSE(job.IsTrailingStepDone());
     job.Start();
     job.Step();
     job.Step();
 
     {
       DummyUnserializer unserializer;
-      ASSERT_TRUE(CheckIdempotentSerialization(unserializer, job));
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
     }
     
     ASSERT_TRUE(job.Serialize(s));
@@ -1418,7 +1489,7 @@
     job.SetRemoteModality(modality);
     job.SetMoveOriginator("MOVESCU", 42);
 
-    ASSERT_TRUE(CheckIdempotentSerialization(unserializer, job));
+    ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
     ASSERT_TRUE(job.Serialize(s));
   }
 
@@ -1448,7 +1519,7 @@
     OrthancPeerStoreJob job(GetContext());
     job.SetPeer(peer);
     
-    ASSERT_TRUE(CheckIdempotentSerialization(unserializer, job));
+    ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
     ASSERT_TRUE(job.Serialize(s));
   }
 
@@ -1473,7 +1544,7 @@
     job.SetModification(modification.release(), true);
     job.SetOrigin(DicomInstanceOrigin::FromLua());
     
-    ASSERT_TRUE(CheckIdempotentSerialization(unserializer, job));
+    ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
     ASSERT_TRUE(job.Serialize(s));
   }
 
@@ -1510,3 +1581,144 @@
     ASSERT_TRUE(CheckSameJson(s, t));
   }
 }
+
+
+TEST(JobsSerialization, TrailingStep)
+{
+  {
+    Json::Value s;
+    
+    DummyInstancesJob job(false);
+    ASSERT_EQ(0, job.GetStepsCount());
+    ASSERT_EQ(0, job.GetInstancesCount());
+
+    job.Start();
+    ASSERT_EQ(0, job.GetPosition());
+    ASSERT_FALSE(job.HasTrailingStep());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+    
+    ASSERT_EQ(JobStepCode_Success, job.Step().GetCode());
+    ASSERT_EQ(1, job.GetPosition());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_THROW(job.Step(), OrthancException);
+  }
+
+  {
+    Json::Value s;
+    
+    DummyInstancesJob job(false);
+    job.AddInstance("hello");
+    job.AddInstance("world");
+    ASSERT_EQ(2, job.GetStepsCount());
+    ASSERT_EQ(2, job.GetInstancesCount());
+
+    job.Start();
+    ASSERT_EQ(0, job.GetPosition());
+    ASSERT_FALSE(job.HasTrailingStep());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+    
+    ASSERT_EQ(JobStepCode_Continue, job.Step().GetCode());
+    ASSERT_EQ(1, job.GetPosition());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_EQ(JobStepCode_Success, job.Step().GetCode());
+    ASSERT_EQ(2, job.GetPosition());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_THROW(job.Step(), OrthancException);
+  }
+
+  {
+    Json::Value s;
+    
+    DummyInstancesJob job(true);
+    ASSERT_EQ(1, job.GetStepsCount());
+    ASSERT_EQ(0, job.GetInstancesCount());
+
+    job.Start();
+    ASSERT_EQ(0, job.GetPosition());
+    ASSERT_TRUE(job.HasTrailingStep());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+    
+    ASSERT_EQ(JobStepCode_Success, job.Step().GetCode());
+    ASSERT_EQ(1, job.GetPosition());
+    ASSERT_TRUE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_THROW(job.Step(), OrthancException);
+  }
+
+  {
+    Json::Value s;
+    
+    DummyInstancesJob job(true);
+    job.AddInstance("hello");
+    ASSERT_EQ(2, job.GetStepsCount());
+    ASSERT_EQ(1, job.GetInstancesCount());
+
+    job.Start();
+    ASSERT_EQ(0, job.GetPosition());
+    ASSERT_TRUE(job.HasTrailingStep());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+    
+    ASSERT_EQ(JobStepCode_Continue, job.Step().GetCode());
+    ASSERT_EQ(1, job.GetPosition());
+    ASSERT_FALSE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_EQ(JobStepCode_Success, job.Step().GetCode());
+    ASSERT_EQ(2, job.GetPosition());
+    ASSERT_TRUE(job.IsTrailingStepDone());
+    
+    {
+      DummyUnserializer unserializer;
+      ASSERT_TRUE(CheckIdempotentSetOfInstances(unserializer, job));
+    }
+
+    ASSERT_THROW(job.Step(), OrthancException);
+  }
+}