--- a/NEWS	Tue Oct 20 09:52:42 2020 +0200
+++ b/NEWS	Mon Oct 26 12:23:36 2020 +0100
@@ -1,7 +1,7 @@
 Pending changes in the mainline
 ===============================
 
-* Fix reporting of client-side store warnings/errors in C-GET SCP
+* C-GET SCP: Fix responses and handling of cancel
 
 
 Version 1.8.0 (2020-10-16)

--- a/OrthancFramework/Sources/DicomNetworking/IGetRequestHandler.h	Tue Oct 20 09:52:42 2020 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/IGetRequestHandler.h	Mon Oct 26 12:23:36 2020 +0100
@@ -34,13 +34,6 @@
   class IGetRequestHandler : boost::noncopyable
   {
   public:
-    enum Status
-    {
-      Status_Success,
-      Status_Failure,
-      Status_Warning
-    };
-    
     virtual ~IGetRequestHandler()
     {
     }
@@ -52,10 +45,9 @@
                         uint32_t timeout) = 0;
     
     virtual unsigned int GetSubOperationCount() const = 0;
-    
-    virtual Status DoNext(T_ASC_Association *) = 0;
-    
-    virtual unsigned int GetRemainingCount() const = 0;
+
+    // Must return "false" iff. a "Cancel" was returned by the remote SCU
+    virtual bool DoNext(T_ASC_Association *) = 0;
     
     virtual unsigned int GetCompletedCount() const = 0;
     

--- a/OrthancFramework/Sources/DicomNetworking/Internals/GetScp.cpp	Tue Oct 20 09:52:42 2020 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/Internals/GetScp.cpp	Mon Oct 26 12:23:36 2020 +0100
@@ -99,12 +99,14 @@
       std::string remoteAet_;
       std::string calledAet_;
       int timeout_;
+      bool canceled_;
 
       GetScpData() :
         handler_(NULL),
         lastRequest_(NULL),
         assoc_(NULL),
-        timeout_(0)
+        timeout_(0),
+        canceled_(false)
       {
       };
     };
@@ -129,6 +131,56 @@
       }
     }
 
+
+    static void FillResponse(T_DIMSE_C_GetRSP& response,
+                             DcmDataset** failedIdentifiers,
+                             const IGetRequestHandler& handler)
+    {
+      response.DimseStatus = STATUS_Success;
+
+      size_t processedCount = (handler.GetCompletedCount() +
+                               handler.GetFailedCount() +
+                               handler.GetWarningCount());
+
+      if (processedCount > handler.GetSubOperationCount())
+      {
+        throw OrthancException(ErrorCode_InternalError);
+      }
+
+      response.NumberOfRemainingSubOperations = (handler.GetSubOperationCount() - processedCount);
+      response.NumberOfCompletedSubOperations = handler.GetCompletedCount();
+      response.NumberOfFailedSubOperations = handler.GetFailedCount();
+      response.NumberOfWarningSubOperations = handler.GetWarningCount();
+
+      // http://dicom.nema.org/medical/dicom/current/output/chtml/part04/sect_C.4.3.3.html
+      
+      if (handler.GetFailedCount() > 0 ||
+          handler.GetWarningCount() > 0) 
+      {
+        /**
+         * "Warning if one or more sub-operations were successfully
+         * completed and one or more sub-operations were unsuccessful
+         * or had a status of warning. Warning if all sub-operations
+         * had a status of Warning"
+         **/
+        response.DimseStatus = STATUS_GET_Warning_SubOperationsCompleteOneOrMoreFailures;
+      }
+
+      if (handler.GetFailedCount() > 0 &&
+          handler.GetFailedCount() == handler.GetSubOperationCount())
+      {
+        /**
+         * "Failure or Refused if all sub-operations were
+         * unsuccessful." => We choose to generate a "Refused - Out
+         * of Resources - Unable to perform suboperations" status.
+         */
+        response.DimseStatus = STATUS_GET_Refused_OutOfResourcesSubOperations;
+      }
+            
+      *failedIdentifiers = BuildFailedInstanceList(handler.GetFailedUids());
+    }
+    
+
     static void GetScpCallback(
       /* in */ 
       void *callbackData,  
@@ -141,6 +193,9 @@
       DcmDataset **responseIdentifiers,
       DcmDataset **statusDetail)
     {
+      assert(response != NULL);
+      assert(responseIdentifiers != NULL);
+      
       bzero(response, sizeof(T_DIMSE_C_GetRSP));
       *statusDetail = NULL;
       *responseIdentifiers = NULL;   
@@ -180,68 +235,52 @@
         return;
       }
 
-      if (data.handler_->GetRemainingCount() == 0)
+      if (data.canceled_)
       {
-        response->DimseStatus = STATUS_Success;
+        LOG(ERROR) << "IGetRequestHandler Failed: Cannot pursue a request that was canceled by the SCU";
+        response->DimseStatus = STATUS_GET_Failed_UnableToProcess;
+        return;
+      }
+      
+      if (data.handler_->GetSubOperationCount() ==
+          data.handler_->GetCompletedCount() +
+          data.handler_->GetFailedCount() +
+          data.handler_->GetWarningCount())
+      {
+        // We're all done
+        FillResponse(*response, responseIdentifiers, *data.handler_);
       }
       else
       {
-        IGetRequestHandler::Status status;
-
+        bool isContinue;
+        
         try
         {
-          status = data.handler_->DoNext(data.assoc_);
+          isContinue = data.handler_->DoNext(data.assoc_);
         }
         catch (OrthancException& e)
         {
           // Internal error!
           LOG(ERROR) << "IGetRequestHandler Failed: " << e.What();
+          FillResponse(*response, responseIdentifiers, *data.handler_);
+
+          // Fix the status code that is computed by "FillResponse()"
           response->DimseStatus = STATUS_GET_Failed_UnableToProcess;
           return;
         }
-        
-        if (status == STATUS_Success)
+
+        FillResponse(*response, responseIdentifiers, *data.handler_);
+
+        if (isContinue)
         {
-          if (responseCount < static_cast<int>(data.handler_->GetRemainingCount()))
-          {
-            response->DimseStatus = STATUS_Pending;
-          }
-          else
-          {
-            response->DimseStatus = STATUS_Success;
-          }
+          response->DimseStatus = STATUS_Pending;
         }
         else
         {
-          response->DimseStatus = STATUS_GET_Failed_UnableToProcess;
-          
-          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_->GetFailedCount() > 0) &&
-              ((data.handler_->GetCompletedCount() +
-                data.handler_->GetWarningCount()) == 0)) 
-          {
-            response->DimseStatus = STATUS_GET_Refused_OutOfResourcesSubOperations;
-          }
-          
-          *responseIdentifiers = BuildFailedInstanceList(data.handler_->GetFailedUids());
+          response->DimseStatus = STATUS_GET_Cancel_SubOperationsTerminatedDueToCancelIndication;
+          data.canceled_ = true;
         }
       }
-      
-      response->NumberOfRemainingSubOperations = data.handler_->GetRemainingCount();
-      response->NumberOfCompletedSubOperations = data.handler_->GetCompletedCount();
-      response->NumberOfFailedSubOperations = data.handler_->GetFailedCount();
-      response->NumberOfWarningSubOperations = data.handler_->GetWarningCount();
     }
   }
 

--- a/OrthancServer/Sources/OrthancGetRequestHandler.cpp	Tue Oct 20 09:52:42 2020 +0200
+++ b/OrthancServer/Sources/OrthancGetRequestHandler.cpp	Mon Oct 26 12:23:36 2020 +0100
@@ -66,12 +66,12 @@
     }
   }
 
-  OrthancGetRequestHandler::Status
-  OrthancGetRequestHandler::DoNext(T_ASC_Association* assoc)
+
+  bool OrthancGetRequestHandler::DoNext(T_ASC_Association* assoc)
   {
     if (position_ >= instances_.size())
     {
-      return Status_Failure;
+      throw OrthancException(ErrorCode_ParameterOutOfRange);
     }
     
     const std::string& id = instances_[position_++];
@@ -81,7 +81,7 @@
     
     if (dicom.empty())
     {
-      return Status_Failure;
+      throw OrthancException(ErrorCode_BadFileFormat);
     }
 
     std::unique_ptr<DcmFileFormat> parsed(
@@ -107,17 +107,7 @@
     std::string sopClassUid(a.c_str());
     std::string sopInstanceUid(b.c_str());
     
-    Status status = PerformGetSubOp(assoc, sopClassUid, sopInstanceUid, parsed.release());
-    
-    if (getCancelled_)
-    {
-      LOG(INFO) << "C-GET SCP: Received C-Cancel RQ";
-      return Status_Failure;
-    }    
-    else 
-    {
-      return status;
-    }
+    return PerformGetSubOp(assoc, sopClassUid, sopInstanceUid, parsed.release());
   }
 
   
@@ -231,11 +221,10 @@
   }                                                           
 
 
-  OrthancGetRequestHandler::Status
-  OrthancGetRequestHandler::PerformGetSubOp(T_ASC_Association* assoc,
-                                            const std::string& sopClassUid,
-                                            const std::string& sopInstanceUid,
-                                            DcmFileFormat* dicomRaw)
+  bool OrthancGetRequestHandler::PerformGetSubOp(T_ASC_Association* assoc,
+                                                 const std::string& sopClassUid,
+                                                 const std::string& sopInstanceUid,
+                                                 DcmFileFormat* dicomRaw)
   {
     assert(dicomRaw != NULL);
     std::unique_ptr<DcmFileFormat> dicom(dicomRaw);
@@ -243,11 +232,12 @@
     DicomTransferSyntax sourceSyntax;
     if (!FromDcmtkBridge::LookupOrthancTransferSyntax(sourceSyntax, *dicom))
     {
-      nFailed_++;
+      failedCount_++;
       AddFailedUIDInstance(sopInstanceUid);
-      LOG(ERROR) << "C-GET SCP: Unknown transfer syntax: ("
-                 << dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT") << ") " << sopClassUid;
-      return Status_Failure;
+      throw OrthancException(ErrorCode_NetworkProtocol, 
+                             "C-GET SCP: Unknown transfer syntax: (" +
+                             std::string(dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT")) +
+                             ") " + sopClassUid);
     }
 
     bool allowTranscoding = (context_.IsTranscodeDicomProtocol() &&
@@ -259,11 +249,12 @@
                                    sourceSyntax, allowTranscoding) ||
         presId == 0)
     {
-      nFailed_++;
+      failedCount_++;
       AddFailedUIDInstance(sopInstanceUid);
-      LOG(ERROR) << "C-GET SCP: storeSCU: No presentation context for: ("
-                 << dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT") << ") " << sopClassUid;
-      return Status_Failure;
+      throw OrthancException(ErrorCode_NetworkProtocol,
+                             "C-GET SCP: storeSCU: No presentation context for: (" +
+                             std::string(dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT")) +
+                             ") " + sopClassUid);
     }
     else
     {
@@ -281,11 +272,12 @@
           pc.acceptedRole != ASC_SC_ROLE_SCUSCP)
       {
         // the role is not appropriate
-        nFailed_++;
+        failedCount_++;
         AddFailedUIDInstance(sopInstanceUid);
-        LOG(ERROR) << "C-GET SCP: storeSCU: [No presentation context with requestor SCP role for: ("
-                   << dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT") << ") " << sopClassUid;
-        return Status_Failure;
+        throw OrthancException(ErrorCode_NetworkProtocol,
+                               "C-GET SCP: storeSCU: [No presentation context with requestor SCP role for: (" +
+                               std::string(dcmSOPClassUIDToModality(sopClassUid.c_str(), "OT")) +
+                               ") " + sopClassUid);
       }
     }
 
@@ -348,37 +340,29 @@
       else
       {
         // Cannot transcode
-        nFailed_++;
+        failedCount_++;
         AddFailedUIDInstance(sopInstanceUid);
-        LOG(ERROR) << "C-GET SCP: Cannot transcode " << sopClassUid
-                   << " from transfer syntax " << GetTransferSyntaxUid(sourceSyntax)
-                   << " to " << GetTransferSyntaxUid(selectedSyntax);
-        return Status_Failure;
+        throw OrthancException(ErrorCode_NotImplemented,
+                               "C-GET SCP: Cannot transcode " + sopClassUid +
+                               " from transfer syntax " + GetTransferSyntaxUid(sourceSyntax) +
+                               " to " + GetTransferSyntaxUid(selectedSyntax));
       }      
     }
 
-    Status status;
+    bool isContinue;
+    
     if (cond.good())
     {
       if (cancelParameters.cancelEncountered)
       {
-        if (origPresId_ == cancelParameters.presId &&
-            origMsgId_ == cancelParameters.req.MessageIDBeingRespondedTo)
-        {
-          getCancelled_ = true;
-        }
-        else
-        {
-          LOG(ERROR) << "C-GET SCP: Unexpected C-Cancel-RQ encountered: pid=" << (int)cancelParameters.presId
-                     << ", mid=" << (int)cancelParameters.req.MessageIDBeingRespondedTo;
-        }
+        LOG(INFO) << "C-GET SCP: Received C-Cancel RQ";
+        isContinue = false;
       }
-      
-      if (rsp.DimseStatus == STATUS_Success)
+      else if (rsp.DimseStatus == STATUS_Success)
       {
         // everything ok
-        nCompleted_++;
-        status = Status_Success;
+        completedCount_++;
+        isContinue = true;
       }
       else if ((rsp.DimseStatus & 0xf000) == 0xb000)
       {
@@ -386,25 +370,25 @@
         warningCount_++;
         LOG(ERROR) << "C-GET SCP: Store Warning: Response Status: "
                    << DU_cstoreStatusString(rsp.DimseStatus);
-        status = Status_Warning;
+        isContinue = true;
       }
       else
       {
-        nFailed_++;
+        failedCount_++;
         AddFailedUIDInstance(sopInstanceUid);
         // print a status message
         LOG(ERROR) << "C-GET SCP: Store Failed: Response Status: "
                    << DU_cstoreStatusString(rsp.DimseStatus);
-        status = Status_Failure;
+        isContinue = true;
       }
     }
     else
     {
-      nFailed_++;
+      failedCount_++;
       AddFailedUIDInstance(sopInstanceUid);
       OFString temp_str;
       LOG(ERROR) << "C-GET SCP: storeSCU: Store Request Failed: " << DimseCondition::dump(temp_str, cond);
-      status = Status_Failure;
+      isContinue = true;
     }
     
     if (stDetail.get() != NULL)
@@ -418,7 +402,7 @@
       LOG(INFO) << "  Status Detail: " << s.str();
     }
     
-    return status;
+    return isContinue;
   }
 
   bool OrthancGetRequestHandler::LookupIdentifiers(std::list<std::string>& publicIds,
@@ -492,14 +476,12 @@
 
 
     OrthancGetRequestHandler::OrthancGetRequestHandler(ServerContext& context) :
-      context_(context),
-      getCancelled_(false)
+      context_(context)
     {
       position_ = 0;
-      nRemaining_ = 0;
-      nCompleted_  = 0;
+      completedCount_  = 0;
       warningCount_ = 0;
-      nFailed_ = 0;
+      failedCount_ = 0;
       timeout_ = 0;
     }
 
@@ -575,11 +557,9 @@
     }
 
     failedUIDs_.clear();
-    getCancelled_ = false;
 
-    nRemaining_ = GetSubOperationCount();
-    nCompleted_ = 0;
-    nFailed_ = 0;
+    completedCount_ = 0;
+    failedCount_ = 0;
     warningCount_ = 0;
     timeout_ = timeout;
 

--- a/OrthancServer/Sources/OrthancGetRequestHandler.h	Tue Oct 20 09:52:42 2020 +0200
+++ b/OrthancServer/Sources/OrthancGetRequestHandler.h	Mon Oct 26 12:23:36 2020 +0100
@@ -56,26 +56,22 @@
     RemoteModalityParameters remote_;
     std::string originatorAet_;
     
-    unsigned int nRemaining_;
-    unsigned int nCompleted_;
+    unsigned int completedCount_;
     unsigned int warningCount_;
-    unsigned int nFailed_;
+    unsigned int failedCount_;
     std::string failedUIDs_;
     
-    DIC_US origMsgId_;
-    T_ASC_PresentationContextID origPresId_;
-    
-    bool getCancelled_;
     uint32_t timeout_;
 
     bool LookupIdentifiers(std::list<std::string>& publicIds,
                            ResourceType level,
                            const DicomMap& input) const;
     
-    Status PerformGetSubOp(T_ASC_Association *assoc,
-                           const std::string& sopClassUid,
-                           const std::string& sopInstanceUid,
-                           DcmFileFormat* datasetRaw);
+    // Returns "false" iff cancel
+    bool PerformGetSubOp(T_ASC_Association *assoc,
+                         const std::string& sopClassUid,
+                         const std::string& sopInstanceUid,
+                         DcmFileFormat* datasetRaw);
     
     void AddFailedUIDInstance(const std::string& sopInstance);
 
@@ -87,22 +83,17 @@
                         const std::string& originatorAet,
                         const std::string& calledAet,
                         uint32_t timeout) ORTHANC_OVERRIDE;
-    
-    virtual Status DoNext(T_ASC_Association *assoc) ORTHANC_OVERRIDE;
+
+    virtual bool DoNext(T_ASC_Association *assoc) ORTHANC_OVERRIDE;
     
     virtual unsigned int GetSubOperationCount() const ORTHANC_OVERRIDE
     {
       return static_cast<unsigned int>(instances_.size());
     }
     
-    virtual unsigned int GetRemainingCount() const ORTHANC_OVERRIDE
-    {
-      return nRemaining_;
-    }
-    
     virtual unsigned int GetCompletedCount() const ORTHANC_OVERRIDE
     {
-      return nCompleted_;
+      return completedCount_;
     }
     
     virtual unsigned int GetWarningCount() const ORTHANC_OVERRIDE
@@ -112,7 +103,7 @@
     
     virtual unsigned int GetFailedCount() const ORTHANC_OVERRIDE
     {
-      return nFailed_;
+      return failedCount_;
     }
     
     virtual const std::string& GetFailedUids() const ORTHANC_OVERRIDE