# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1589961140 -7200
# Node ID 67b4572834998bc6ae22f883b39f5e8a70a7abfc
# Parent  620e87e9e8162717cddbc1ff084a413feaae4dcf
coding style

diff -r 620e87e9e816 -r 67b457283499 Core/DicomNetworking/IGetRequestHandler.h
--- a/Core/DicomNetworking/IGetRequestHandler.h	Wed May 20 09:19:35 2020 +0200
+++ b/Core/DicomNetworking/IGetRequestHandler.h	Wed May 20 09:52:20 2020 +0200
@@ -43,7 +43,7 @@
 
 namespace Orthanc
 {
-  class IGetRequestHandler
+  class IGetRequestHandler : boost::noncopyable
   {
   public:
     enum Status
@@ -66,15 +66,14 @@
     
     virtual Status DoNext(T_ASC_Association *) = 0;
     
-    virtual unsigned int nRemaining() const = 0;
+    virtual unsigned int GetRemainingCount() const = 0;
     
-    virtual unsigned int nCompleted() const = 0;
+    virtual unsigned int GetCompletedCount() const = 0;
     
-    virtual unsigned int warningCount() const = 0;
+    virtual unsigned int GetWarningCount() const = 0;
     
-    virtual const std::string& failedUids() const = 0;
+    virtual unsigned int GetFailedCount() const = 0;
     
-    virtual unsigned int nFailed() const = 0;
+    virtual const std::string& GetFailedUids() const = 0;
   };
-
 }
diff -r 620e87e9e816 -r 67b457283499 Core/DicomNetworking/Internals/GetScp.cpp
--- a/Core/DicomNetworking/Internals/GetScp.cpp	Wed May 20 09:19:35 2020 +0200
+++ b/Core/DicomNetworking/Internals/GetScp.cpp	Wed May 20 09:52:20 2020 +0200
@@ -94,11 +94,6 @@
 
 #include <boost/lexical_cast.hpp>
 
-/**
- * Macro specifying whether to apply the patch suggested in issue 66:
- * "Orthanc responses C-GET with zero Get Originator Message ID"
- * https://bitbucket.org/sjodogne/orthanc/issues/66/
- **/
 
 namespace Orthanc
 {
@@ -192,7 +187,7 @@
         return;
       }
 
-      if (data.handler_->nRemaining() == 0)
+      if (data.handler_->GetRemainingCount() == 0)
       {
         response->DimseStatus = STATUS_Success;
       }
@@ -214,7 +209,7 @@
         
         if (status == STATUS_Success)
         {
-          if (responseCount < static_cast<int>(data.handler_->nRemaining()))
+          if (responseCount < static_cast<int>(data.handler_->GetRemainingCount()))
           {
             response->DimseStatus = STATUS_Pending;
           }
@@ -227,31 +222,33 @@
         {
           response->DimseStatus = STATUS_GET_Failed_UnableToProcess;
           
-          if (data.handler_->nFailed() > 0 ||
-              data.handler_->warningCount() > 0) 
+          if (data.handler_->GetFailedCount() > 0 ||
+              data.handler_->GetWarningCount() > 0) 
           {
             response->DimseStatus = STATUS_GET_Warning_SubOperationsCompleteOneOrMoreFailures;
           }
+          
           /*
            * if all the sub-operations failed then we need to generate
            * a failed or refused status.  cf. DICOM part 4, C.4.3.3.1
            * we choose to generate a "Refused - Out of Resources -
            * Unable to perform suboperations" status.
            */
-          if ((data.handler_->nFailed() > 0) &&
-              ((data.handler_->nCompleted() + data.handler_->warningCount()) == 0)) 
+          if ((data.handler_->GetFailedCount() > 0) &&
+              ((data.handler_->GetCompletedCount() +
+                data.handler_->GetWarningCount()) == 0)) 
           {
             response->DimseStatus = STATUS_GET_Refused_OutOfResourcesSubOperations;
           }
           
-          *responseIdentifiers = BuildFailedInstanceList(data.handler_->failedUids());
+          *responseIdentifiers = BuildFailedInstanceList(data.handler_->GetFailedUids());
         }
       }
       
-      response->NumberOfRemainingSubOperations = data.handler_->nRemaining();
-      response->NumberOfCompletedSubOperations = data.handler_->nCompleted();
-      response->NumberOfFailedSubOperations = data.handler_->nFailed();
-      response->NumberOfWarningSubOperations = data.handler_->warningCount();
+      response->NumberOfRemainingSubOperations = data.handler_->GetRemainingCount();
+      response->NumberOfCompletedSubOperations = data.handler_->GetCompletedCount();
+      response->NumberOfFailedSubOperations = data.handler_->GetFailedCount();
+      response->NumberOfWarningSubOperations = data.handler_->GetWarningCount();
     }
   }
 
diff -r 620e87e9e816 -r 67b457283499 OrthancServer/OrthancGetRequestHandler.cpp
--- a/OrthancServer/OrthancGetRequestHandler.cpp	Wed May 20 09:19:35 2020 +0200
+++ b/OrthancServer/OrthancGetRequestHandler.cpp	Wed May 20 09:52:20 2020 +0200
@@ -86,21 +86,28 @@
     // Determine the storage SOP class UID for this instance
     DIC_UI sopClass;
     DIC_UI sopInstance;
-    
+
+    {
+      bool ok;
+      
 #if DCMTK_VERSION_NUMBER >= 364
-    if (!DU_findSOPClassAndInstanceInDataSet(static_cast<DcmItem *> (parsed->getDataset()),
-                                             sopClass, sizeof(sopClass),
-                                             sopInstance, sizeof(sopInstance)))
+      ok = DU_findSOPClassAndInstanceInDataSet(static_cast<DcmItem *> (parsed->getDataset()),
+                                               sopClass, sizeof(sopClass),
+                                               sopInstance, sizeof(sopInstance));
 #else
-      if (!DU_findSOPClassAndInstanceInDataSet(parsed->getDataset(), sopClass, sopInstance))
+      ok = DU_findSOPClassAndInstanceInDataSet(parsed->getDataset(), sopClass, sopInstance);
 #endif
+
+      if (!ok)
       {
         throw OrthancException(ErrorCode_NoSopClassOrInstance,
                                "Unable to determine the SOP class/instance for C-STORE with AET " +
                                originatorAet_);
       }
+    }
     
-    OFCondition cond = performGetSubOp(assoc, sopClass, sopInstance, parsed->getDataset());
+    OFCondition cond = PerformGetSubOp(assoc, sopClass, sopInstance, parsed->getDataset());
+    
     if (getCancelled_)
     {
       LOG(INFO) << "Get SCP: Received C-Cancel RQ";
@@ -115,7 +122,7 @@
   }
 
   
-  void OrthancGetRequestHandler::addFailedUIDInstance(const char *sopInstance)
+  void OrthancGetRequestHandler::AddFailedUIDInstance(const char *sopInstance)
   {
     if (failedUIDs_.empty())
     {
@@ -128,7 +135,7 @@
   }
 
 
-  OFCondition OrthancGetRequestHandler::performGetSubOp(T_ASC_Association* assoc, 
+  OFCondition OrthancGetRequestHandler::PerformGetSubOp(T_ASC_Association* assoc, 
                                                         DIC_UI sopClass, 
                                                         DIC_UI sopInstance, 
                                                         DcmDataset *dataset)
@@ -148,7 +155,7 @@
     if (presId == 0)
     {
       nFailed_++;
-      addFailedUIDInstance(sopInstance);
+      AddFailedUIDInstance(sopInstance);
       LOG(ERROR) << "Get SCP: storeSCU: No presentation context for: ("
                  << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass;
       return DIMSE_NOVALIDPRESENTATIONCONTEXTID;
@@ -159,11 +166,12 @@
       T_ASC_PresentationContext pc;
       ASC_findAcceptedPresentationContext(assoc->params, presId, &pc);
       // the acceptedRole is the association requestor role
-      if ((pc.acceptedRole != ASC_SC_ROLE_SCP) && (pc.acceptedRole != ASC_SC_ROLE_SCUSCP))
+      if ((pc.acceptedRole != ASC_SC_ROLE_SCP) &&
+          (pc.acceptedRole != ASC_SC_ROLE_SCUSCP))
       {
         // the role is not appropriate
         nFailed_++;
-        addFailedUIDInstance(sopInstance);
+        AddFailedUIDInstance(sopInstance);
         LOG(ERROR) <<"Get SCP: storeSCU: [No presentation context with requestor SCP role for: ("
                    << dcmSOPClassUIDToModality(sopClass, "OT") << ") " << sopClass;
         return DIMSE_NOVALIDPRESENTATIONCONTEXTID;
@@ -190,8 +198,8 @@
     {
       if (cancelParameters.cancelEncountered)
       {
-        if (origPresId == cancelParameters.presId &&
-            origMsgId == cancelParameters.req.MessageIDBeingRespondedTo)
+        if (origPresId_ == cancelParameters.presId &&
+            origMsgId_ == cancelParameters.req.MessageIDBeingRespondedTo)
         {
           getCancelled_ = OFTrue;
         }
@@ -217,7 +225,7 @@
       else
       {
         nFailed_++;
-        addFailedUIDInstance(sopInstance);
+        AddFailedUIDInstance(sopInstance);
         // print a status message
         LOG(ERROR) << "Get SCP: Store Failed: Response Status: "
                    << DU_cstoreStatusString(rsp.DimseStatus);
@@ -226,7 +234,7 @@
     else
     {
       nFailed_++;
-      addFailedUIDInstance(sopInstance);
+      AddFailedUIDInstance(sopInstance);
       OFString temp_str;
       LOG(ERROR) << "Get SCP: storeSCU: Store Request Failed: " << DimseCondition::dump(temp_str, cond);
     }
@@ -338,7 +346,12 @@
 
     std::vector<std::string> publicIds;
 
-    bool retVal = LookupIdentifiers(publicIds, level, input);
+    if (!LookupIdentifiers(publicIds, level, input))
+    {
+      LOG(ERROR) << "Cannot determine what resources are requested by C-GET";
+      return false; 
+    }
+    
     localAet_ = context_.GetDefaultLocalApplicationEntityTitle();
     position_ = 0;
     originatorAet_ = originatorAet;
@@ -350,8 +363,8 @@
     
     for (size_t i = 0; i < publicIds.size(); i++)
     {
-      LOG(INFO) << "Sending resource " << publicIds[i] << " to modality \""
-                << originatorAet << "\" in synchronous mode";
+      LOG(INFO) << "C-GET: Sending resource " << publicIds[i]
+                << " to modality \"" << originatorAet << "\"";
       
       std::list<std::string> tmp;
       context_.GetIndex().GetChildInstances(tmp, publicIds[i]);
@@ -371,6 +384,6 @@
     nFailed_ = 0;
     warningCount_ = 0;
 
-    return retVal;    
+    return true;
   }
 };
diff -r 620e87e9e816 -r 67b457283499 OrthancServer/OrthancGetRequestHandler.h
--- a/OrthancServer/OrthancGetRequestHandler.h	Wed May 20 09:19:35 2020 +0200
+++ b/OrthancServer/OrthancGetRequestHandler.h	Wed May 20 09:52:20 2020 +0200
@@ -58,8 +58,8 @@
     std::string failedUIDs_;
     
     T_DIMSE_Priority priority_;
-    DIC_US origMsgId;
-    T_ASC_PresentationContextID origPresId;
+    DIC_US origMsgId_;
+    T_ASC_PresentationContextID origPresId_;
     
     bool getCancelled_;
 
@@ -67,12 +67,12 @@
                            ResourceType level,
                            const DicomMap& input);
     
-    OFCondition performGetSubOp(T_ASC_Association *assoc,
+    OFCondition PerformGetSubOp(T_ASC_Association *assoc,
                                 DIC_UI sopClass,
                                 DIC_UI sopInstance,
                                 DcmDataset *dataset);
     
-    void addFailedUIDInstance(const char *sopInstance);
+    void AddFailedUIDInstance(const char *sopInstance);
 
   public:
     OrthancGetRequestHandler(ServerContext& context);
@@ -82,34 +82,34 @@
                 const std::string& originatorAet,
                 const std::string& calledAet);
     
-    virtual Status DoNext(T_ASC_Association *) ORTHANC_OVERRIDE;
+    virtual Status DoNext(T_ASC_Association *assoc) ORTHANC_OVERRIDE;
     
     virtual unsigned int GetSubOperationCount() const ORTHANC_OVERRIDE
     {
-      return (unsigned int) instances_.size();
+      return static_cast<unsigned int>(instances_.size());
     }
     
-    virtual unsigned int nRemaining() const ORTHANC_OVERRIDE
+    virtual unsigned int GetRemainingCount() const ORTHANC_OVERRIDE
     {
       return nRemaining_;
     }
     
-    virtual unsigned int nCompleted() const ORTHANC_OVERRIDE
+    virtual unsigned int GetCompletedCount() const ORTHANC_OVERRIDE
     {
       return nCompleted_;
     }
     
-    virtual unsigned int warningCount() const ORTHANC_OVERRIDE
+    virtual unsigned int GetWarningCount() const ORTHANC_OVERRIDE
     {
       return warningCount_;
     }
     
-    virtual unsigned int nFailed() const ORTHANC_OVERRIDE
+    virtual unsigned int GetFailedCount() const ORTHANC_OVERRIDE
     {
       return nFailed_;
     }
     
-    virtual const std::string& failedUids() const ORTHANC_OVERRIDE
+    virtual const std::string& GetFailedUids() const ORTHANC_OVERRIDE
     {
       return failedUIDs_;
     }