# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1538152580 -7200
# Node ID d386abc181332f4022d8dd05dc5ab69530e3b859
# Parent  218e2c864d1d50ae8ca12885740fe590c6b0a93e
simplification in SplitStudyJob, fix possible memory leak

diff -r 218e2c864d1d -r d386abc18133 Core/DicomParsing/DicomDirWriter.cpp
--- a/Core/DicomParsing/DicomDirWriter.cpp	Fri Sep 28 17:59:44 2018 +0200
+++ b/Core/DicomParsing/DicomDirWriter.cpp	Fri Sep 28 18:36:20 2018 +0200
@@ -504,14 +504,6 @@
   {
   }
 
-  DicomDirWriter::~DicomDirWriter()
-  {
-    if (pimpl_)
-    {
-      delete pimpl_;
-    }
-  }
-
   void DicomDirWriter::SetUtcUsed(bool utc)
   {
     pimpl_->SetUtcUsed(utc);
diff -r 218e2c864d1d -r d386abc18133 Core/DicomParsing/DicomDirWriter.h
--- a/Core/DicomParsing/DicomDirWriter.h	Fri Sep 28 17:59:44 2018 +0200
+++ b/Core/DicomParsing/DicomDirWriter.h	Fri Sep 28 18:36:20 2018 +0200
@@ -43,13 +43,11 @@
   {
   private:
     class PImpl;
-    PImpl* pimpl_;
+    boost::shared_ptr<PImpl>  pimpl_;
 
   public:
     DicomDirWriter();
 
-    ~DicomDirWriter();
-
     void SetUtcUsed(bool utc);
 
     bool IsUtcUsed() const;
diff -r 218e2c864d1d -r d386abc18133 Core/DicomParsing/ParsedDicomFile.cpp
--- a/Core/DicomParsing/ParsedDicomFile.cpp	Fri Sep 28 17:59:44 2018 +0200
+++ b/Core/DicomParsing/ParsedDicomFile.cpp	Fri Sep 28 18:36:20 2018 +0200
@@ -944,59 +944,48 @@
   void ParsedDicomFile::CreateFromDicomMap(const DicomMap& source,
                                            Encoding defaultEncoding)
   {
-    try
-    {
-      pimpl_->file_.reset(new DcmFileFormat);
+    pimpl_->file_.reset(new DcmFileFormat);
+
+    const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET);
 
-      const DicomValue* tmp = source.TestAndGetValue(DICOM_TAG_SPECIFIC_CHARACTER_SET);
+    if (tmp == NULL)
+    {
+      SetEncoding(defaultEncoding);
+    }
+    else if (tmp->IsBinary())
+    {
+      LOG(ERROR) << "Invalid binary string in the SpecificCharacterSet (0008,0005) tag";
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+    else if (tmp->IsNull() ||
+             tmp->GetContent().empty())
+    {
+      SetEncoding(defaultEncoding);
+    }
+    else
+    {
+      Encoding encoding;
 
-      if (tmp == NULL)
+      if (GetDicomEncoding(encoding, tmp->GetContent().c_str()))
       {
-        SetEncoding(defaultEncoding);
-      }
-      else if (tmp->IsBinary())
-      {
-        LOG(ERROR) << "Invalid binary string in the SpecificCharacterSet (0008,0005) tag";
-        throw OrthancException(ErrorCode_ParameterOutOfRange);
-      }
-      else if (tmp->IsNull() ||
-               tmp->GetContent().empty())
-      {
-        SetEncoding(defaultEncoding);
+        SetEncoding(encoding);
       }
       else
       {
-        Encoding encoding;
-
-        if (GetDicomEncoding(encoding, tmp->GetContent().c_str()))
-        {
-          SetEncoding(encoding);
-        }
-        else
-        {
-          LOG(ERROR) << "Unsupported value for the SpecificCharacterSet (0008,0005) tag: \""
-                     << tmp->GetContent() << "\"";        
-          throw OrthancException(ErrorCode_ParameterOutOfRange);
-        }
-      }
-
-      for (DicomMap::Map::const_iterator 
-             it = source.map_.begin(); it != source.map_.end(); ++it)
-      {
-        if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET &&
-            !it->second->IsNull())
-        {
-          ReplacePlainString(it->first, it->second->GetContent());
-        }
+        LOG(ERROR) << "Unsupported value for the SpecificCharacterSet (0008,0005) tag: \""
+                   << tmp->GetContent() << "\"";        
+        throw OrthancException(ErrorCode_ParameterOutOfRange);
       }
     }
-    catch (OrthancException&)
+
+    for (DicomMap::Map::const_iterator 
+           it = source.map_.begin(); it != source.map_.end(); ++it)
     {
-      // Manually delete the PImpl to avoid a memory leak due to
-      // throwing the exception in the constructor
-      delete pimpl_;
-      pimpl_ = NULL;
-      throw;
+      if (it->first != DICOM_TAG_SPECIFIC_CHARACTER_SET &&
+          !it->second->IsNull())
+      {
+        ReplacePlainString(it->first, it->second->GetContent());
+      }
     }
   }
 
@@ -1061,12 +1050,6 @@
   }
 
 
-  ParsedDicomFile::~ParsedDicomFile()
-  {
-    delete pimpl_;
-  }
-
-
   DcmFileFormat& ParsedDicomFile::GetDcmtkObject() const
   {
     return *pimpl_->file_.get();
diff -r 218e2c864d1d -r d386abc18133 Core/DicomParsing/ParsedDicomFile.h
--- a/Core/DicomParsing/ParsedDicomFile.h	Fri Sep 28 17:59:44 2018 +0200
+++ b/Core/DicomParsing/ParsedDicomFile.h	Fri Sep 28 18:36:20 2018 +0200
@@ -63,6 +63,8 @@
 #  include "../RestApi/RestApiOutput.h"
 #endif
 
+#include <boost/shared_ptr.hpp>
+
 
 class DcmDataset;
 class DcmFileFormat;
@@ -73,7 +75,7 @@
   {
   private:
     struct PImpl;
-    PImpl* pimpl_;
+    boost::shared_ptr<PImpl> pimpl_;
 
     ParsedDicomFile(ParsedDicomFile& other,
                     bool keepSopInstanceUid);
@@ -108,8 +110,6 @@
 
     ParsedDicomFile(DcmFileFormat& dicom);
 
-    ~ParsedDicomFile();
-
     DcmFileFormat& GetDcmtkObject() const;
 
     ParsedDicomFile* Clone(bool keepSopInstanceUid);
diff -r 218e2c864d1d -r d386abc18133 OrthancServer/ServerJobs/SplitStudyJob.cpp
--- a/OrthancServer/ServerJobs/SplitStudyJob.cpp	Fri Sep 28 17:59:44 2018 +0200
+++ b/OrthancServer/ServerJobs/SplitStudyJob.cpp	Fri Sep 28 18:36:20 2018 +0200
@@ -85,25 +85,16 @@
     /**
      * Chose the target UIDs
      **/
-    
-    std::string series;
-    if (!modified->GetTagValue(series, DICOM_TAG_SERIES_INSTANCE_UID))
+
+    assert(modified->GetHasher().HashStudy() == sourceStudy_);
+
+    std::string series = modified->GetHasher().HashSeries();
+
+    SeriesUidMap::const_iterator targetSeriesUid = seriesUidMap_.find(series);
+
+    if (targetSeriesUid == seriesUidMap_.end())
     {
       throw OrthancException(ErrorCode_BadFileFormat);  // Should never happen
-    }    
-
-    std::string targetSeriesUid;
-    SeriesUidMap::const_iterator found = targetSeries_.find(series);
-
-    if (found == targetSeries_.end())
-    {
-      // Choose a random SeriesInstanceUID for this series
-      targetSeriesUid = FromDcmtkBridge::GenerateUniqueIdentifier(ResourceType_Series);
-      targetSeries_[series] = targetSeriesUid;
-    }
-    else
-    {
-      targetSeriesUid = found->second;
     }
 
 
@@ -129,7 +120,7 @@
      **/
     
     modified->ReplacePlainString(DICOM_TAG_STUDY_INSTANCE_UID, targetStudyUid_);
-    modified->ReplacePlainString(DICOM_TAG_SERIES_INSTANCE_UID, targetSeriesUid);
+    modified->ReplacePlainString(DICOM_TAG_SERIES_INSTANCE_UID, targetSeriesUid->second);
 
     if (targetStudy_.empty())
     {
@@ -186,15 +177,6 @@
       LOG(ERROR) << "Cannot split unknown study: " << sourceStudy;
       throw OrthancException(ErrorCode_UnknownResource);
     }
-
-    std::list<std::string> children;
-    context_.GetIndex().GetChildren(children, sourceStudy);
-
-    for (std::list<std::string>::const_iterator
-           it = children.begin(); it != children.end(); ++it)
-    {
-      sourceSeries_.insert(*it);
-    }
   }
   
 
@@ -219,17 +201,23 @@
 
   void SplitStudyJob::AddSourceSeries(const std::string& series)
   {
+    std::string parent;
+
     if (IsStarted())
     {
       throw OrthancException(ErrorCode_BadSequenceOfCalls);
     }
-    else if (sourceSeries_.find(series) == sourceSeries_.end())
+    else if (!context_.GetIndex().LookupParent(parent, series, ResourceType_Study) ||
+             parent != sourceStudy_)
     {
       LOG(ERROR) << "This series does not belong to the study to be split: " << series;
       throw OrthancException(ErrorCode_UnknownResource);
     }
     else
     {
+      // Generate a target SeriesInstanceUID for this series
+      seriesUidMap_[series] = FromDcmtkBridge::GenerateUniqueIdentifier(ResourceType_Series);
+
       // Add all the instances of the series as to be processed
       std::list<std::string> instances;
       context_.GetIndex().GetChildren(instances, series);
@@ -292,12 +280,11 @@
   }
 
 
+  static const char* KEEP_SOURCE = "KeepSource";
   static const char* SOURCE_STUDY = "SourceStudy";
-  static const char* SOURCE_SERIES = "SourceSeries";
-  static const char* KEEP_SOURCE = "KeepSource";
   static const char* TARGET_STUDY = "TargetStudy";
   static const char* TARGET_STUDY_UID = "TargetStudyUID";
-  static const char* TARGET_SERIES = "TargetSeries";
+  static const char* SERIES_UID_MAP = "SeriesUIDMap";
   static const char* ORIGIN = "Origin";
   static const char* REPLACEMENTS = "Replacements";
   static const char* REMOVALS = "Removals";
@@ -318,10 +305,9 @@
 
     keepSource_ = SerializationToolbox::ReadBoolean(serialized, KEEP_SOURCE);
     sourceStudy_ = SerializationToolbox::ReadString(serialized, SOURCE_STUDY);
-    SerializationToolbox::ReadSetOfStrings(sourceSeries_, serialized, SOURCE_SERIES);
     targetStudy_ = SerializationToolbox::ReadString(serialized, TARGET_STUDY);
     targetStudyUid_ = SerializationToolbox::ReadString(serialized, TARGET_STUDY_UID);
-    SerializationToolbox::ReadMapOfStrings(targetSeries_, serialized, TARGET_SERIES);
+    SerializationToolbox::ReadMapOfStrings(seriesUidMap_, serialized, SERIES_UID_MAP);
     origin_ = DicomInstanceOrigin(serialized[ORIGIN]);
     SerializationToolbox::ReadMapOfTags(replacements_, serialized, REPLACEMENTS);
     SerializationToolbox::ReadSetOfTags(removals_, serialized, REMOVALS);
@@ -338,10 +324,9 @@
     {
       target[KEEP_SOURCE] = keepSource_;
       target[SOURCE_STUDY] = sourceStudy_;
-      SerializationToolbox::WriteSetOfStrings(target, sourceSeries_, SOURCE_SERIES);
       target[TARGET_STUDY] = targetStudy_;
       target[TARGET_STUDY_UID] = targetStudyUid_;
-      SerializationToolbox::WriteMapOfStrings(target, targetSeries_, TARGET_SERIES);
+      SerializationToolbox::WriteMapOfStrings(target, seriesUidMap_, SERIES_UID_MAP);
       origin_.Serialize(target[ORIGIN]);
       SerializationToolbox::WriteMapOfTags(target, replacements_, REPLACEMENTS);
       SerializationToolbox::WriteSetOfTags(target, removals_, REMOVALS);
diff -r 218e2c864d1d -r d386abc18133 OrthancServer/ServerJobs/SplitStudyJob.h
--- a/OrthancServer/ServerJobs/SplitStudyJob.h	Fri Sep 28 17:59:44 2018 +0200
+++ b/OrthancServer/ServerJobs/SplitStudyJob.h	Fri Sep 28 18:36:20 2018 +0200
@@ -47,13 +47,12 @@
     
     
     ServerContext&         context_;
+    std::set<DicomTag>     allowedTags_;
     bool                   keepSource_;
     std::string            sourceStudy_;
-    std::set<std::string>  sourceSeries_;
     std::string            targetStudy_;
     std::string            targetStudyUid_;
-    SeriesUidMap           targetSeries_;
-    std::set<DicomTag>     allowedTags_;
+    SeriesUidMap           seriesUidMap_;
     DicomInstanceOrigin    origin_;
     Replacements           replacements_;
     std::set<DicomTag>     removals_;