# HG changeset patch
# User Alain Mazy <am@osimis.io>
# Date 1700755201 -3600
# Node ID 8536b98eff2bbdc5fd3b12dcfd52701320d36820
# Parent  40112827c56f4401af0534b503aec1f1b90e466b# Parent  ac68a4383e51047f5406eb94a36ae93714efe424
merge 'debug-telemis'

diff -r 40112827c56f -r 8536b98eff2b NEWS
--- a/NEWS	Thu Nov 23 10:15:19 2023 +0100
+++ b/NEWS	Thu Nov 23 17:00:01 2023 +0100
@@ -63,6 +63,11 @@
   https://discourse.orthanc-server.org/t/c-find-fails-on-unknown-specific-character-set-iso-2022-ir-6-iso-2022-ir-100/3947
 * When exporting a study archive, make sure to use the PatientName from the study and not from the patient
   in case of PatientID collision.
+* DICOM C-Store:
+  - Avoid some unneccessary renegotiation of DICOM association.
+  - Force renegotiation in case no presentation context were accepted in previous association (we have
+    observed PACS that were not consistent in the accepted presentation contexts)
+  - Improved logging
 * Upgraded dependencies for static builds:
   - boost 1.83.0
 * Upgraded minizip library to stay away from CVE-2023-45853 although Orthanc is likely not affected since zip
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp
--- a/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Thu Nov 23 10:15:19 2023 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomAssociation.cpp	Thu Nov 23 17:00:01 2023 +0100
@@ -108,6 +108,8 @@
     
   void DicomAssociation::CloseInternal()
   {
+    CLOG(INFO, DICOM) << "Closing DICOM association";
+
 #if ORTHANC_ENABLE_SSL == 1
     tls_.reset(NULL);  // Transport layer must be destroyed before the association itself
 #endif
@@ -393,8 +395,10 @@
       LST_Position(l, (LST_NODE*)pc);
       while (pc)
       {
-        if (pc->result == ASC_P_ACCEPTANCE)
+        if (pc->result == ASC_P_ACCEPTANCE && strlen(pc->abstractSyntax) > 0)
         {
+          CLOG(TRACE, DICOM) << "DicomAssociation::Open, adding SOPClassUID " << pc->abstractSyntax << " - TS " << pc->acceptedTransferSyntax << " - PC ID " << boost::lexical_cast<std::string>(static_cast<int>(pc->presentationContextID));
+
           DicomTransferSyntax transferSyntax;
           if (LookupTransferSyntax(transferSyntax, pc->acceptedTransferSyntax))
           {
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp
--- a/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp	Thu Nov 23 10:15:19 2023 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.cpp	Thu Nov 23 17:00:01 2023 +0100
@@ -262,26 +262,33 @@
 
     if (LookupPresentationContext(presentationContextId, sopClassUid, transferSyntax))
     {
+      CLOG(INFO, DICOM) << "Found an accepted presentation context for SOPClassUID " << sopClassUid << " and transfer syntax " << GetTransferSyntaxUid(transferSyntax);
       return true;
     }
 
     // The association must be re-negotiated
     if (association_->IsOpen())
     {
-      CLOG(INFO, DICOM) << "Re-negotiating DICOM association with "
-                        << parameters_.GetRemoteModality().GetApplicationEntityTitle();
+      CLOG(INFO, DICOM) << "No accepted presentation context found, re-negotiating DICOM association with "
+                        << parameters_.GetRemoteModality().GetApplicationEntityTitle()
+                        << " for SOPClassUID " << sopClassUid << " TransferSyntax =" << GetTransferSyntaxUid(transferSyntax);
 
-      // Don't renegociate if we know that the remote modality was
+      // Check if we know that the remote modality was
       // already proposed this individual transfer syntax (**)
-      if (proposedOriginalClasses_.find(std::make_pair(sopClassUid, transferSyntax)) !=
-          proposedOriginalClasses_.end())
+      if (proposedOriginalClasses_.find(std::make_pair(sopClassUid, transferSyntax)) != proposedOriginalClasses_.end())
       {
         CLOG(INFO, DICOM) << "The remote modality has already rejected SOP class UID \""
                           << sopClassUid << "\" with transfer syntax \""
-                          << GetTransferSyntaxUid(transferSyntax) << "\", don't renegotiate";
-        return false;
+                          << GetTransferSyntaxUid(transferSyntax) << "\", but we will renegotiate anyway";
+        // always renegotiating since 1.12.2 // return false;
       }
     }
+    else
+    {
+      CLOG(INFO, DICOM) << "Negotiating DICOM association with "
+                        << parameters_.GetRemoteModality().GetApplicationEntityTitle()
+                        << " for SOPClassUID " << sopClassUid << " TransferSyntax =" << GetTransferSyntaxUid(transferSyntax);
+    }
 
     association_->ClearPresentationContexts();
     proposedOriginalClasses_.clear();
@@ -374,6 +381,8 @@
     DicomTransferSyntax transferSyntax;
     LookupParameters(sopClassUid, sopInstanceUid, transferSyntax, dicom);
 
+    LOG(INFO) << "Performing C-Store on instance of SOPClassUID '" << sopClassUid << "'";
+
     uint8_t presID;
     if (!NegotiatePresentationContext(presID, sopClassUid, transferSyntax, proposeUncompressedSyntaxes_,
                                       DicomTransferSyntax_LittleEndianExplicit))
@@ -480,14 +489,17 @@
                                                    DicomTransferSyntax preferred)
   {
     acceptedSyntaxes.clear();
+    std::map<DicomTransferSyntax, uint8_t> contexts;
 
     // Make sure a negotiation has already occurred for this transfer
-    // syntax. We don't use the return code: Transcoding is possible
-    // even if the "sourceSyntax" is not supported.
-    uint8_t presID;
-    NegotiatePresentationContext(presID, sopClassUid, sourceSyntax, hasPreferred, preferred);
+    // syntax if we have not negotiated yet. 
+    // We don't use the return code: Transcoding is possible even if the "sourceSyntax" is not supported.
+    if (!association_->IsOpen() || !association_->LookupAcceptedPresentationContext(contexts, sopClassUid))
+    {
+      uint8_t presID;
+      NegotiatePresentationContext(presID, sopClassUid, sourceSyntax, hasPreferred, preferred);
+    }
 
-    std::map<DicomTransferSyntax, uint8_t> contexts;
     if (association_->LookupAcceptedPresentationContext(contexts, sopClassUid))
     {
       for (std::map<DicomTransferSyntax, uint8_t>::const_iterator
@@ -524,6 +536,12 @@
     std::set<DicomTransferSyntax> accepted;
     LookupTranscoding(accepted, sopClassUid, sourceSyntax, true, preferredTransferSyntax);
 
+    if (accepted.size() == 0)
+    {
+      throw OrthancException(ErrorCode_NoPresentationContext, "Cannot C-Store an instance of SOPClassUID " + 
+                             sopClassUid + ", the destination has not accepted any TransferSyntax for this SOPClassUID.");
+    }
+
     if (accepted.find(sourceSyntax) != accepted.end())
     {
       // No need for transcoding
@@ -544,6 +562,8 @@
       bool isDestructiveCompressionAllowed = false;
       std::set<DicomTransferSyntax> attemptedSyntaxes;
 
+      LOG(INFO) << "Transcoding is required to C-Store an instance of SOPClassUID '" << sopClassUid << "', preferredTransferSyntax is " << GetTransferSyntaxUid(preferredTransferSyntax);
+
       if (accepted.find(preferredTransferSyntax) != accepted.end())
       {
         // New in Orthanc 1.9.0: The preferred transfer syntax is
@@ -629,7 +649,8 @@
           s += " " + std::string(GetTransferSyntaxUid(*it));
         }
         
-        throw OrthancException(ErrorCode_NotImplemented, "Cannot transcode from " +
+        throw OrthancException(ErrorCode_NotImplemented, "Cannot transcode instance of SOPClassUID " + 
+                               sopClassUid + " from " +
                                std::string(GetTransferSyntaxUid(sourceSyntax)) +
                                " to one of [" + s + " ]");
       }
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/DicomNetworking/DicomStoreUserConnection.h
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp
--- a/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp	Thu Nov 23 10:15:19 2023 +0100
+++ b/OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.cpp	Thu Nov 23 17:00:01 2023 +0100
@@ -186,7 +186,6 @@
     manufacturer_ = StringToModalityManufacturer(manufacturer);
   }
 
-
   void RemoteModalityParameters::UnserializeArray(const Json::Value& serialized)
   {
     assert(serialized.type() == Json::arrayValue);
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/DicomNetworking/RemoteModalityParameters.h
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/Enumerations.cpp
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/Sources/Enumerations.h
diff -r 40112827c56f -r 8536b98eff2b OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp
diff -r 40112827c56f -r 8536b98eff2b OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp	Thu Nov 23 10:15:19 2023 +0100
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestModalities.cpp	Thu Nov 23 17:00:01 2023 +0100
@@ -1520,7 +1520,9 @@
 
     std::string sopClassUid, sopInstanceUid;
     connection.Store(sopClassUid, sopInstanceUid, call.GetBodyData(),
-                     call.GetBodySize(), false /* Not a C-MOVE */, "", 0);
+                     call.GetBodySize(), 
+                     false /* Not a C-MOVE */, 
+                     "", 0);
 
     Json::Value answer = Json::objectValue;
     answer[SOP_CLASS_UID] = sopClassUid;
diff -r 40112827c56f -r 8536b98eff2b OrthancServer/Sources/ServerContext.cpp
--- a/OrthancServer/Sources/ServerContext.cpp	Thu Nov 23 10:15:19 2023 +0100
+++ b/OrthancServer/Sources/ServerContext.cpp	Thu Nov 23 17:00:01 2023 +0100
@@ -1952,9 +1952,10 @@
                                            uint16_t moveOriginatorId)
   {
     const void* data = dicom.empty() ? NULL : dicom.c_str();
-    
+    const RemoteModalityParameters& modality = connection.GetParameters().GetRemoteModality();
+
     if (!transcodeDicomProtocol_ ||
-        !connection.GetParameters().GetRemoteModality().IsTranscodingAllowed())
+        !modality.IsTranscodingAllowed())
     {
       connection.Store(sopClassUid, sopInstanceUid, data, dicom.size(),
                        hasMoveOriginator, moveOriginatorAet, moveOriginatorId);