Mercurial > hg > orthanc

changeset 6317:94b94ad90101 default tip

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
fix dicom+json when the instance contains an empty element in a sequence
author Alain Mazy <am@orthanc.team>
date Fri, 19 Sep 2025 15:09:10 +0200
parents 72ae47e6bc4e
children
files NEWS OrthancFramework/Sources/DicomParsing/DicomModification.cpp OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp OrthancFramework/Sources/DicomParsing/ITagVisitor.h OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp
diffstat 7 files changed, 115 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Thu Sep 18 15:35:28 2025 +0200
+++ b/NEWS	Fri Sep 19 15:09:10 2025 +0200
@@ -35,6 +35,9 @@
 * Fix: DicomGetScu jobs are now saved in DB.
 * Fix: When the configuration option "MaximumStorageCacheSize" was set to 0, the default value (128)
        was actually applied.  From now on, a value of 0 really means that the storage cache is disabled.
+* Fix: Orthanc was unable to convert the tags into dicom+json format if the instance contained an 
+       empty element in a sequence.  This was preventing access to /dicom-web/../metadata routes and prevented
+       visualization in e.g. the Stone Web viewer and OHIF.
 
 
 Version 1.12.9 (2025-08-11)
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Fri Sep 19 15:09:10 2025 +0200
@@ -178,6 +178,12 @@
       return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
+    virtual Action VisitEmptyElement(const std::vector<DicomTag>& parentTags,
+                                     const std::vector<size_t>& parentIndexes) ORTHANC_OVERRIDE
+    {
+      return Action_None;
+    }
+
     virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags,
                                  const std::vector<size_t>& parentIndexes,
                                  const DicomTag& tag,
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Fri Sep 19 15:09:10 2025 +0200
@@ -241,6 +241,25 @@
                                                const std::vector<size_t>& parentIndexes,
                                                const DicomTag& tag)
   {
+    Json::Value& node = CreateEmptyNode(parentTags, parentIndexes);
+    assert(node.type() == Json::objectValue);
+
+    std::string t = FormatTag(tag);
+    if (node.isMember(t))
+    {
+      throw OrthancException(ErrorCode_InternalError);
+    }
+    else
+    {
+      node[t] = Json::objectValue;
+      return node[t];
+    }
+  }
+
+
+  Json::Value& DicomWebJsonVisitor::CreateEmptyNode(const std::vector<DicomTag>& parentTags,
+                                                    const std::vector<size_t>& parentIndexes)
+  {
     assert(parentTags.size() == parentIndexes.size());      
 
     Json::Value* node = &result_;
@@ -291,16 +310,7 @@
 
     assert(node->type() == Json::objectValue);
 
-    std::string t = FormatTag(tag);
-    if (node->isMember(t))
-    {
-      throw OrthancException(ErrorCode_InternalError);
-    }
-    else
-    {
-      (*node) [t] = Json::objectValue;
-      return (*node) [t];
-    }
+    return *node;
   }
 
     
@@ -425,6 +435,14 @@
     return Action_None;
   }
   
+  ITagVisitor::Action
+  DicomWebJsonVisitor::VisitEmptyElement(const std::vector<DicomTag>& parentTags,
+                                         const std::vector<size_t>& parentIndexes)
+  {
+    CreateEmptyNode(parentTags, parentIndexes);
+    return Action_None;
+  }
+
 
   ITagVisitor::Action
   DicomWebJsonVisitor::VisitBinary(const std::vector<DicomTag>& parentTags,
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Fri Sep 19 15:09:10 2025 +0200
@@ -70,6 +70,9 @@
                             const std::vector<size_t>& parentIndexes,
                             const DicomTag& tag);
 
+    Json::Value& CreateEmptyNode(const std::vector<DicomTag>& parentTags,
+                                 const std::vector<size_t>& parentIndexes);
+
     static Json::Value FormatInteger(int64_t value);
 
     static Json::Value FormatDouble(double value);
@@ -109,6 +112,10 @@
                                size_t size)
       ORTHANC_OVERRIDE;
 
+    virtual Action VisitEmptyElement(const std::vector<DicomTag>& parentTags,
+                                     const std::vector<size_t>& parentIndexes)
+      ORTHANC_OVERRIDE;
+
     virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags,
                                  const std::vector<size_t>& parentIndexes,
                                  const DicomTag& tag,
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Fri Sep 19 15:09:10 2025 +0200
@@ -2689,22 +2689,28 @@
 
     std::set<DcmTagKey> toRemove;
     
-    for (unsigned long i = 0; i < dataset.card(); i++)
+    if (dataset.card() > 0)
     {
-      DcmElement* element = dataset.getElement(i);
-      if (element == NULL)
+      for (unsigned long i = 0; i < dataset.card(); i++)
       {
-        throw OrthancException(ErrorCode_InternalError);
+        DcmElement* element = dataset.getElement(i);
+        if (element == NULL)
+        {
+          throw OrthancException(ErrorCode_InternalError);
+        }
+        else
+        {
+          if (!ApplyVisitorToElement(*element, visitor, parentTags, parentIndexes, encoding, hasCodeExtensions))
+          {
+            toRemove.insert(element->getTag());
+          }
+        }      
       }
-      else
-      {
-        if (!ApplyVisitorToElement(*element, visitor, parentTags, parentIndexes, encoding, hasCodeExtensions))
-        {
-          toRemove.insert(element->getTag());
-        }
-      }      
     }
-
+    else
+    {
+      visitor.VisitEmptyElement(parentTags, parentIndexes);
+    }
     // Remove all the tags that were planned for removal (cf. ITagVisitor::Action_Remove)
     for (std::set<DcmTagKey>::const_iterator
            it = toRemove.begin(); it != toRemove.end(); ++it)
--- a/OrthancFramework/Sources/DicomParsing/ITagVisitor.h	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/Sources/DicomParsing/ITagVisitor.h	Fri Sep 19 15:09:10 2025 +0200
@@ -93,5 +93,9 @@
                                const DicomTag& tag,
                                ValueRepresentation vr,
                                const std::string& value) = 0;
-  };
+
+    // empty sequence element - can return "Remove" or "None"
+    virtual Action VisitEmptyElement(const std::vector<DicomTag>& parentTags,
+                                     const std::vector<size_t>& parentIndexes) = 0;
+    };
 }
--- a/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Thu Sep 18 15:35:28 2025 +0200
+++ b/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Fri Sep 19 15:09:10 2025 +0200
@@ -2260,6 +2260,47 @@
 }
 
 
+TEST(DicomWebJson, SequenceWithEmptyItem)
+{
+  ParsedDicomFile dicom(false);
+  
+  {
+    std::unique_ptr<DcmSequenceOfItems> sequence(new DcmSequenceOfItems(DCM_OriginalAttributesSequence));
+
+    for (unsigned int i = 0; i < 3; i++)
+    {
+      std::unique_ptr<DcmItem> item(new DcmItem);
+      if (i != 1) // the 2nd element is empty
+      {
+        std::unique_ptr<DcmSequenceOfItems> subSequence(new DcmSequenceOfItems(DCM_ModifiedAttributesSequence));
+
+        std::unique_ptr<DcmItem> subItem(new DcmItem);
+        std::string s = "item" + boost::lexical_cast<std::string>(i);
+        subItem->putAndInsertString(DCM_AccessionNumber, s.c_str(), OFFalse);
+
+        ASSERT_TRUE(subSequence->append(subItem.release()).good());
+        ASSERT_TRUE(item->insert(subSequence.release(), false, false).good());
+      }
+
+      ASSERT_TRUE(sequence->append(item.release()).good());
+    }
+
+    ASSERT_TRUE(dicom.GetDcmtkObject().getDataset()->insert(sequence.release(), false, false).good());
+  }
+
+  DicomWebJsonVisitor visitor;
+  dicom.Apply(visitor);  // this raised an exception in 1.12.9
+
+  const Json::Value& output = visitor.GetResult();
+  // LOG(INFO) << output.toStyledString();
+
+  ASSERT_EQ("SQ", output["04000561"]["vr"].asString());
+  ASSERT_EQ(3u, output["04000561"]["Value"].size());
+
+  ASSERT_EQ("item0", output["04000561"]["Value"][0]["04000550"]["Value"][0]["00080050"]["Value"][0].asString());
+  ASSERT_EQ("item2", output["04000561"]["Value"][2]["04000550"]["Value"][0]["00080050"]["Value"][0].asString());
+}
+
 TEST(ParsedDicomCache, Basic)
 {
   ParsedDicomCache cache(10);
@@ -2987,6 +3028,13 @@
       }
     }
 
+    virtual Action VisitEmptyElement(const std::vector<DicomTag>& parentTags,
+                                     const std::vector<size_t>& parentIndexes) ORTHANC_OVERRIDE
+    {
+      return Action_None;
+    }
+
+
     virtual Action VisitBinary(const std::vector<DicomTag>& parentTags,
                                const std::vector<size_t>& parentIndexes,
                                const DicomTag& tag,