# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1447858232 -3600
# Node ID 1b1d5470233f698ff823fe0d96ada6ffc20edda0
# Parent  164d789113829b6f794948d6593b04391f0a767f
refactoring of DicomFindAnswers

diff -r 164d78911382 -r 1b1d5470233f CMakeLists.txt
--- a/CMakeLists.txt	Wed Nov 18 12:00:14 2015 +0100
+++ b/CMakeLists.txt	Wed Nov 18 15:50:32 2015 +0100
@@ -155,7 +155,6 @@
   OrthancServer/DicomProtocol/DicomFindAnswers.cpp
   OrthancServer/DicomProtocol/DicomServer.cpp
   OrthancServer/DicomProtocol/DicomUserConnection.cpp
-  OrthancServer/DicomProtocol/DicomWorklistAnswers.cpp
   OrthancServer/DicomProtocol/RemoteModalityParameters.cpp
   OrthancServer/DicomProtocol/ReusableDicomUserConnection.cpp
   OrthancServer/ExportedResource.cpp
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/DicomProtocol/DicomFindAnswers.cpp
--- a/OrthancServer/DicomProtocol/DicomFindAnswers.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/DicomProtocol/DicomFindAnswers.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -34,6 +34,10 @@
 #include "DicomFindAnswers.h"
 
 #include "../FromDcmtkBridge.h"
+#include "../ToDcmtkBridge.h"
+#include "../../Core/OrthancException.h"
+
+#include <memory>
 
 namespace Orthanc
 {
@@ -41,8 +45,11 @@
   {
     for (size_t i = 0; i < items_.size(); i++)
     {
+      assert(items_[i] != NULL);
       delete items_[i];
     }
+
+    items_.clear();
   }
 
   void DicomFindAnswers::Reserve(size_t size)
@@ -53,6 +60,46 @@
     }
   }
 
+
+  void DicomFindAnswers::Add(const DicomMap& map)
+  {
+    items_.push_back(new ParsedDicomFile(map));
+  }
+
+  void DicomFindAnswers::Add(ParsedDicomFile& dicom)
+  {
+    items_.push_back(dicom.Clone());
+  }
+
+  void DicomFindAnswers::Add(const char* dicom,
+                             size_t size)
+  {
+    items_.push_back(new ParsedDicomFile(dicom, size));
+  }
+
+
+  ParsedDicomFile& DicomFindAnswers::GetAnswer(size_t index) const
+  {
+    if (index < items_.size())
+    {
+      return *items_.at(index);
+    }
+    else
+    {
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
+    }
+  }
+
+
+  void DicomFindAnswers::ToJson(Json::Value& target,
+                                size_t index,
+                                bool simplify) const
+  {
+    DicomToJsonFormat format = (simplify ? DicomToJsonFormat_Simple : DicomToJsonFormat_Full);
+    GetAnswer(index).ToJson(target, format, DicomToJsonFlags_None, 0);
+  }
+
+
   void DicomFindAnswers::ToJson(Json::Value& target,
                                 bool simplify) const
   {
@@ -60,8 +107,8 @@
 
     for (size_t i = 0; i < GetSize(); i++)
     {
-      Json::Value answer(Json::objectValue);
-      FromDcmtkBridge::ToJson(answer, GetAnswer(i), simplify);
+      Json::Value answer;
+      ToJson(answer, i, simplify);
       target.append(answer);
     }
   }
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/DicomProtocol/DicomFindAnswers.h
--- a/OrthancServer/DicomProtocol/DicomFindAnswers.h	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/DicomProtocol/DicomFindAnswers.h	Wed Nov 18 15:50:32 2015 +0100
@@ -32,17 +32,14 @@
 
 #pragma once
 
-#include "../../Core/DicomFormat/DicomMap.h"
-
-#include <vector>
-#include <json/json.h>
+#include "../ParsedDicomFile.h"
 
 namespace Orthanc
 {
   class DicomFindAnswers : public boost::noncopyable
   {
   private:
-    std::vector<DicomMap*> items_;
+    std::vector<ParsedDicomFile*> items_;
 
   public:
     ~DicomFindAnswers()
@@ -54,22 +51,25 @@
 
     void Reserve(size_t index);
 
-    void Add(const DicomMap& map)
-    {
-      items_.push_back(map.Clone());
-    }
+    void Add(const DicomMap& map);
+
+    void Add(ParsedDicomFile& dicom);
+
+    void Add(const char* dicom,
+             size_t size);
 
     size_t GetSize() const
     {
       return items_.size();
     }
 
-    const DicomMap& GetAnswer(size_t index) const
-    {
-      return *items_.at(index);
-    }
+    ParsedDicomFile& GetAnswer(size_t index) const;
 
     void ToJson(Json::Value& target,
                 bool simplify) const;
+
+    void ToJson(Json::Value& target,
+                size_t index,
+                bool simplify) const;
   };
 }
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/DicomProtocol/DicomWorklistAnswers.cpp
--- a/OrthancServer/DicomProtocol/DicomWorklistAnswers.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,71 +0,0 @@
-/**
- * Orthanc - A Lightweight, RESTful DICOM Store
- * Copyright (C) 2012-2015 Sebastien Jodogne, Medical Physics
- * Department, University Hospital of Liege, 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 "../PrecompiledHeadersServer.h"
-#include "DicomWorklistAnswers.h"
-
-#include "../../Core/OrthancException.h"
-
-namespace Orthanc
-{
-  void DicomWorklistAnswers::Clear()
-  {
-    for (size_t i = 0; i < items_.size(); i++)
-    {
-      delete items_[i];
-    }
-
-    items_.clear();
-  }
-
-  void DicomWorklistAnswers::Add(ParsedDicomFile& dicom)
-  {
-    items_.push_back(dicom.Clone());
-  }
-
-  void DicomWorklistAnswers::Add(const std::string& dicom)
-  {
-    items_.push_back(new ParsedDicomFile(dicom));
-  }
-
-  const ParsedDicomFile& DicomWorklistAnswers::GetAnswer(size_t index) const
-  {
-    if (index < items_.size())
-    {
-      return *items_[index];
-    }
-    else
-    {
-      throw OrthancException(ErrorCode_ParameterOutOfRange);
-    }
-  }
-}
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/DicomProtocol/DicomWorklistAnswers.h
--- a/OrthancServer/DicomProtocol/DicomWorklistAnswers.h	Wed Nov 18 12:00:14 2015 +0100
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,63 +0,0 @@
-/**
- * Orthanc - A Lightweight, RESTful DICOM Store
- * Copyright (C) 2012-2015 Sebastien Jodogne, Medical Physics
- * Department, University Hospital of Liege, 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 "../ParsedDicomFile.h"
-
-namespace Orthanc
-{
-  class DicomWorklistAnswers : public boost::noncopyable
-  {
-  private:
-    std::vector<ParsedDicomFile*> items_;
-
-  public:
-    ~DicomWorklistAnswers()
-    {
-      Clear();
-    }
-
-    void Clear();
-
-    void Add(ParsedDicomFile& dicom);
-
-    void Add(const std::string& dicom);
-
-    size_t GetSize() const
-    {
-      return items_.size();
-    }
-
-    const ParsedDicomFile& GetAnswer(size_t index) const;
-  };
-}
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/DicomProtocol/IWorklistRequestHandler.h
--- a/OrthancServer/DicomProtocol/IWorklistRequestHandler.h	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/DicomProtocol/IWorklistRequestHandler.h	Wed Nov 18 15:50:32 2015 +0100
@@ -32,7 +32,7 @@
 
 #pragma once
 
-#include "DicomWorklistAnswers.h"
+#include "DicomFindAnswers.h"
 
 namespace Orthanc
 {
@@ -49,7 +49,7 @@
      * Cancel request" DIMSE code would be returned.
      * https://www.dabsoft.ch/dicom/4/V.4.1/
      **/
-    virtual bool Handle(DicomWorklistAnswers& answers,
+    virtual bool Handle(DicomFindAnswers& answers,
                         const ParsedDicomFile& query,
                         const std::string& remoteIp,
                         const std::string& remoteAet) = 0;
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/Internals/FindScp.cpp
--- a/OrthancServer/Internals/FindScp.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/Internals/FindScp.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -87,7 +87,7 @@
 #include "../../Core/Logging.h"
 #include "../../Core/OrthancException.h"
 
-
+#include <dcmtk/dcmdata/dcfilefo.h>
 
 namespace Orthanc
 {
@@ -134,9 +134,9 @@
             if (data.worklistHandler_ != NULL)
             {
               // TODO
-              std::auto_ptr<ParsedDicomFile> query(ParsedDicomFile::CreateFromDcmtkDataset(requestIdentifiers));
+              /*std::auto_ptr<ParsedDicomFile> query(ParsedDicomFile::CreateFromDcmtkDataset(requestIdentifiers));
               DicomWorklistAnswers a;
-              data.worklistHandler_->Handle(a, *query, *data.remoteIp_, *data.remoteAet_);
+              data.worklistHandler_->Handle(a, *query, *data.remoteIp_, *data.remoteAet_);*/
               ok = true;
             }
             else
@@ -187,7 +187,12 @@
       {
         // There are pending results that are still to be sent
         response->DimseStatus = STATUS_Pending;
-        *responseIdentifiers = ToDcmtkBridge::Convert(data.answers_.GetAnswer(responseCount - 1));
+
+        void* obj = data.answers_.GetAnswer(responseCount - 1).GetDcmtkObject();
+        DcmFileFormat* fileFormat = static_cast<DcmFileFormat*>(obj);
+        assert(fileFormat != NULL);
+
+        *responseIdentifiers = new DcmDataset(*fileFormat->getDataset());
       }
       else if (data.noCroppingOfResults_)
       {
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/OrthancRestApi/OrthancRestModalities.cpp
--- a/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/OrthancRestApi/OrthancRestModalities.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -280,6 +280,18 @@
   }
 
 
+  static void CopyTagIfExists(DicomMap& target,
+                              ParsedDicomFile& source,
+                              const DicomTag& tag)
+  {
+    std::string tmp;
+    if (source.GetTagValue(tmp, tag))
+    {
+      target.SetValue(tag, tmp);
+    }
+  }
+
+
   static void DicomFind(RestApiPostCall& call)
   {
     LOG(WARNING) << "This URI is deprecated: " << call.FlattenUri();
@@ -303,15 +315,16 @@
     Json::Value result = Json::arrayValue;
     for (size_t i = 0; i < patients.GetSize(); i++)
     {
-      Json::Value patient(Json::objectValue);
-      FromDcmtkBridge::ToJson(patient, patients.GetAnswer(i), true);
+      Json::Value patient;
+      patients.ToJson(patient, i, true);
 
       DicomMap::SetupFindStudyTemplate(m);
       if (!MergeQueryAndTemplate(m, call.GetBodyData(), call.GetBodySize()))
       {
         return;
       }
-      m.CopyTagIfExists(patients.GetAnswer(i), DICOM_TAG_PATIENT_ID);
+
+      CopyTagIfExists(m, patients.GetAnswer(i), DICOM_TAG_PATIENT_ID);
 
       DicomFindAnswers studies;
       FindStudy(studies, locker.GetConnection(), m);
@@ -321,16 +334,17 @@
       // Loop over the found studies
       for (size_t j = 0; j < studies.GetSize(); j++)
       {
-        Json::Value study(Json::objectValue);
-        FromDcmtkBridge::ToJson(study, studies.GetAnswer(j), true);
+        Json::Value study;
+        studies.ToJson(study, j, true);
 
         DicomMap::SetupFindSeriesTemplate(m);
         if (!MergeQueryAndTemplate(m, call.GetBodyData(), call.GetBodySize()))
         {
           return;
         }
-        m.CopyTagIfExists(studies.GetAnswer(j), DICOM_TAG_PATIENT_ID);
-        m.CopyTagIfExists(studies.GetAnswer(j), DICOM_TAG_STUDY_INSTANCE_UID);
+
+        CopyTagIfExists(m, studies.GetAnswer(j), DICOM_TAG_PATIENT_ID);
+        CopyTagIfExists(m, studies.GetAnswer(j), DICOM_TAG_STUDY_INSTANCE_UID);
 
         DicomFindAnswers series;
         FindSeries(series, locker.GetConnection(), m);
@@ -339,8 +353,8 @@
         study["Series"] = Json::arrayValue;
         for (size_t k = 0; k < series.GetSize(); k++)
         {
-          Json::Value series2(Json::objectValue);
-          FromDcmtkBridge::ToJson(series2, series.GetAnswer(k), true);
+          Json::Value series2;
+          series.ToJson(series2, k, true);
           study["Series"].append(series2);
         }
 
@@ -465,8 +479,13 @@
   static void GetQueryOneAnswer(RestApiGetCall& call)
   {
     size_t index = boost::lexical_cast<size_t>(call.GetUriComponent("index", ""));
+
     QueryAccessor query(call);
-    AnswerDicomMap(call, query->GetAnswer(index), call.HasArgument("simplify"));
+
+    DicomMap map;
+    query->GetAnswer(map, index);
+
+    AnswerDicomMap(call, map, call.HasArgument("simplify"));
   }
 
 
@@ -547,7 +566,9 @@
 
     // Ensure that the answer of interest does exist
     size_t index = boost::lexical_cast<size_t>(call.GetUriComponent("index", ""));
-    query->GetAnswer(index);
+
+    DicomMap map;
+    query->GetAnswer(map, index);
 
     RestApi::AutoListChildren(call);
   }
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/ParsedDicomFile.cpp
--- a/OrthancServer/ParsedDicomFile.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/ParsedDicomFile.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -823,9 +823,15 @@
   }
 
 
-  ParsedDicomFile::ParsedDicomFile(void* fileFormat) : pimpl_(new PImpl)
+  ParsedDicomFile::ParsedDicomFile(const DicomMap& map) : pimpl_(new PImpl)
   {
-    pimpl_->file_.reset(static_cast<DcmFileFormat*>(fileFormat));
+    std::auto_ptr<DcmDataset> dataset(ToDcmtkBridge::Convert(map));
+
+    // NOTE: This implies an unnecessary memory copy of the dataset, but no way to get around
+    // http://support.dcmtk.org/redmine/issues/544
+    std::auto_ptr<DcmFileFormat> fileFormat(new DcmFileFormat(dataset.get()));
+
+    pimpl_->file_.reset(fileFormat.release());
   }
 
 
@@ -1228,15 +1234,4 @@
   {
     FromDcmtkBridge::Convert(tags, *pimpl_->file_->getDataset());
   }
-
-
-  ParsedDicomFile* ParsedDicomFile::CreateFromDcmtkDataset(void* dataset)
-  {
-    assert(dataset != NULL);
-
-    DcmDataset *d = static_cast<DcmDataset*>(dataset);
-    std::auto_ptr<DcmFileFormat> fileFormat(new DcmFileFormat(d));
-    
-    return new ParsedDicomFile(fileFormat.release());
-  }
 }
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/ParsedDicomFile.h
--- a/OrthancServer/ParsedDicomFile.h	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/ParsedDicomFile.h	Wed Nov 18 15:50:32 2015 +0100
@@ -58,11 +58,11 @@
                           const std::string& value,
                           bool decodeBinaryTags);
 
-    ParsedDicomFile(void* fileFormat);   // Create by embedding a DcmFileFormat (takes ownership)
-
   public:
     ParsedDicomFile();  // Create a minimal DICOM instance
 
+    ParsedDicomFile(const DicomMap& map);
+
     ParsedDicomFile(const char* content,
                     size_t size);
 
@@ -152,8 +152,6 @@
     bool ExtractPdf(std::string& pdf);
 
     void Convert(DicomMap& tags);
-
-    static ParsedDicomFile* CreateFromDcmtkDataset(void* dataset);
   };
 
 }
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/QueryRetrieveHandler.cpp
--- a/OrthancServer/QueryRetrieveHandler.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/QueryRetrieveHandler.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -95,31 +95,24 @@
   }
 
 
-  const DicomMap& QueryRetrieveHandler::GetAnswer(size_t i)
+  void QueryRetrieveHandler::GetAnswer(DicomMap& target,
+                                       size_t i)
   {
     Run();
-
-    if (i >= answers_.GetSize())
-    {
-      throw OrthancException(ErrorCode_ParameterOutOfRange);
-    }
-
-    return answers_.GetAnswer(i);
+    answers_.GetAnswer(i).Convert(target);
   }
 
 
   void QueryRetrieveHandler::Retrieve(const std::string& target,
                                       size_t i)
   {
-    Run();
+    DicomMap map;
+    GetAnswer(map, i);
 
-    if (i >= answers_.GetSize())
     {
-      throw OrthancException(ErrorCode_ParameterOutOfRange);
+      ReusableDicomUserConnection::Locker locker(context_.GetReusableDicomUserConnection(), localAet_, modality_);
+      locker.GetConnection().Move(target, map);
     }
-
-    ReusableDicomUserConnection::Locker locker(context_.GetReusableDicomUserConnection(), localAet_, modality_);
-    locker.GetConnection().Move(target, answers_.GetAnswer(i));
   }
 
 
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/QueryRetrieveHandler.h
--- a/OrthancServer/QueryRetrieveHandler.h	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/QueryRetrieveHandler.h	Wed Nov 18 15:50:32 2015 +0100
@@ -85,7 +85,8 @@
 
     size_t GetAnswerCount();
 
-    const DicomMap& GetAnswer(size_t i);
+    void GetAnswer(DicomMap& target,
+                   size_t i);
 
     void Retrieve(const std::string& target,
                   size_t i);
diff -r 164d78911382 -r 1b1d5470233f OrthancServer/main.cpp
--- a/OrthancServer/main.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/OrthancServer/main.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -100,7 +100,7 @@
   {
   }
 
-  virtual bool Handle(DicomWorklistAnswers& answers,
+  virtual bool Handle(DicomFindAnswers& answers,
                       const ParsedDicomFile& query,
                       const std::string& remoteIp,
                       const std::string& remoteAet)
diff -r 164d78911382 -r 1b1d5470233f UnitTestsSources/FromDcmtkTests.cpp
--- a/UnitTestsSources/FromDcmtkTests.cpp	Wed Nov 18 12:00:14 2015 +0100
+++ b/UnitTestsSources/FromDcmtkTests.cpp	Wed Nov 18 15:50:32 2015 +0100
@@ -43,6 +43,7 @@
 #include "../Core/Images/PngWriter.h"
 #include "../Core/Uuid.h"
 #include "../Resources/EncodingTests.h"
+#include "../OrthancServer/DicomProtocol/DicomFindAnswers.h"
 
 #include <dcmtk/dcmdata/dcelem.h>
 
@@ -606,3 +607,25 @@
   ASSERT_EQ("application/octet-stream", mime);
   ASSERT_EQ("Pixels", content);
 }
+
+
+TEST(DicomFindAnswers, Basic)
+{
+  DicomFindAnswers a;
+
+  {
+    DicomMap m;
+    m.SetValue(DICOM_TAG_PATIENT_ID, "hello");
+    a.Add(m);
+  }
+
+  {
+    DicomMap m;
+    m.SetValue(DICOM_TAG_PATIENT_ID, "world");
+    a.Add(m);
+  }
+
+  Json::Value j;
+  a.ToJson(j, true);
+  ASSERT_EQ(2u, j.size());
+}