# HG changeset patch
# User Sebastien Jodogne <s.jodogne@gmail.com>
# Date 1625551946 -7200
# Node ID 979ae3ea3381289cfb7c2c1af2575a16037a2d82
# Parent  bf852fd773b72d881a61a82291e5392cddf6da47
DANGEROUS commit: Anonymization is now also applied to nested sequences

diff -r bf852fd773b7 -r 979ae3ea3381 NEWS
--- a/NEWS	Mon Jul 05 17:50:58 2021 +0200
+++ b/NEWS	Tue Jul 06 08:12:26 2021 +0200
@@ -1,6 +1,11 @@
 Pending changes in the mainline
 ===============================
 
+General
+-------
+
+* Anonymization is now also applied to nested sequences
+
 REST API
 --------
 
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/DicomModification.cpp
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomModification.cpp	Tue Jul 06 08:12:26 2021 +0200
@@ -44,6 +44,16 @@
 
 namespace Orthanc
 {
+  namespace
+  {
+    enum TagOperation
+    {
+      TagOperation_Keep,
+      TagOperation_Remove
+    };
+  }
+
+  
   DicomModification::DicomTagRange::DicomTagRange(uint16_t groupFrom,
                                                   uint16_t groupTo,
                                                   uint16_t elementFrom,
@@ -76,7 +86,57 @@
       return (that_.IsCleared(tag) ||
               that_.IsRemoved(tag) ||
               that_.IsReplaced(tag));
-    }                         
+    }
+
+    bool IsKeptSequence(const std::vector<DicomTag>& parentTags,
+                        const std::vector<size_t>& parentIndexes,
+                        const DicomTag& tag)
+    {
+      for (DicomModification::ListOfPaths::const_iterator
+             it = that_.keepSequences_.begin(); it != that_.keepSequences_.end(); ++it)
+      {
+        if (DicomPath::IsMatch(*it, parentTags, parentIndexes, tag))
+        {
+          return true;
+        }
+      }
+
+      return false;
+    }
+
+    Action GetDefaultAction(const std::vector<DicomTag>& parentTags,
+                            const std::vector<size_t>& parentIndexes,
+                            const DicomTag& tag)
+    {
+      if (parentTags.empty() ||
+          !that_.isAnonymization_)
+      {
+        // Don't interfere with first-level tags or with modification
+        return Action_None;
+      }
+      else if (IsKeptSequence(parentTags, parentIndexes, tag))
+      {
+        return Action_None;
+      }
+      else if (that_.ArePrivateTagsRemoved() &&
+               tag.IsPrivate())
+      {
+        // New in Orthanc 1.9.5
+        // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ
+        return Action_Remove;
+      }
+      else if (that_.IsCleared(tag) ||
+               that_.IsRemoved(tag))
+      {
+        // New in Orthanc 1.9.5
+        // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ
+        return Action_Remove;
+      }
+      else
+      {
+        return Action_None;
+      }
+    }
 
   public:
     explicit RelationshipsVisitor(DicomModification& that) :
@@ -89,14 +149,15 @@
                                      const DicomTag& tag,
                                      ValueRepresentation vr) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
-    virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags,
-                                      const std::vector<size_t>& parentIndexes,
-                                      const DicomTag& tag) ORTHANC_OVERRIDE
+    virtual Action VisitSequence(const std::vector<DicomTag>& parentTags,
+                                 const std::vector<size_t>& parentIndexes,
+                                 const DicomTag& tag,
+                                 size_t countItems) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
     virtual Action VisitBinary(const std::vector<DicomTag>& parentTags,
@@ -106,7 +167,7 @@
                                const void* data,
                                size_t size) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
     virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags,
@@ -115,7 +176,7 @@
                                  ValueRepresentation vr,
                                  const std::vector<int64_t>& values) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
     virtual Action VisitDoubles(const std::vector<DicomTag>& parentTags,
@@ -124,7 +185,7 @@
                                 ValueRepresentation vr,
                                 const std::vector<double>& value) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
     virtual Action VisitAttributes(const std::vector<DicomTag>& parentTags,
@@ -132,7 +193,7 @@
                                    const DicomTag& tag,
                                    const std::vector<DicomTag>& value) ORTHANC_OVERRIDE
     {
-      return Action_None;
+      return GetDefaultAction(parentTags, parentIndexes, tag);
     }
 
     virtual Action VisitString(std::string& newValue,
@@ -187,18 +248,30 @@
       {
         // We are within a sequence
 
-        if (!that_.keepSequences_.empty())
+        if (IsKeptSequence(parentTags, parentIndexes, tag))
         {
           // New in Orthanc 1.9.4 - Solves issue LSD-629
-          DicomPath path(parentTags, parentIndexes, tag);
-          
-          for (ListOfPaths::const_iterator it = that_.keepSequences_.begin();
-               it != that_.keepSequences_.end(); ++it)
+          return Action_None;
+        }
+
+        if (that_.isAnonymization_)
+        {
+          // New in Orthanc 1.9.5, similar to "GetDefaultAction()"
+          // https://groups.google.com/g/orthanc-users/c/l1mcYCC2u-k/m/jOdGYuagAgAJ
+          if (that_.ArePrivateTagsRemoved() &&
+              tag.IsPrivate())
           {
-            if (DicomPath::IsMatch(*it, path))
-            {
-              return Action_None;
-            }
+            return Action_Remove;
+          }
+          else if (that_.IsRemoved(tag))
+          {
+            return Action_Remove;
+          }
+          else if (that_.IsCleared(tag))
+          {
+            // This is different from "GetDefaultAction()", because we know how to clear string tags
+            newValue.clear();
+            return Action_Replace;
           }
         }
 
@@ -1106,7 +1179,7 @@
 
   static void ParseListOfTags(DicomModification& target,
                               const Json::Value& query,
-                              DicomModification::TagOperation operation,
+                              TagOperation operation,
                               bool force)
   {
     if (!query.isArray())
@@ -1131,18 +1204,18 @@
       {
         throw OrthancException(ErrorCode_BadRequest,
                                "Marking tag \"" + name + "\" as to be " +
-                               (operation == DicomModification::TagOperation_Keep ? "kept" : "removed") +
+                               (operation == TagOperation_Keep ? "kept" : "removed") +
                                " requires the \"Force\" option to be set to true");
       }
 
       switch (operation)
       {
-        case DicomModification::TagOperation_Keep:
+        case TagOperation_Keep:
           target.Keep(path);
           LOG(TRACE) << "Keep: " << name << " = " << path.Format();
           break;
 
-        case DicomModification::TagOperation_Remove:
+        case TagOperation_Remove:
           target.Remove(path);
           LOG(TRACE) << "Remove: " << name << " = " << path.Format();
           break;
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/DicomModification.h
--- a/OrthancFramework/Sources/DicomParsing/DicomModification.h	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomModification.h	Tue Jul 06 08:12:26 2021 +0200
@@ -39,12 +39,6 @@
      **/
 
   public:
-    enum TagOperation
-    {
-      TagOperation_Keep,
-      TagOperation_Remove
-    };
-
     class IDicomIdentifierGenerator : public boost::noncopyable
     {
     public:
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.cpp	Tue Jul 06 08:12:26 2021 +0200
@@ -382,11 +382,13 @@
 
 
   ITagVisitor::Action
-  DicomWebJsonVisitor::VisitEmptySequence(const std::vector<DicomTag>& parentTags,
-                                          const std::vector<size_t>& parentIndexes,
-                                          const DicomTag& tag)
+  DicomWebJsonVisitor::VisitSequence(const std::vector<DicomTag>& parentTags,
+                                     const std::vector<size_t>& parentIndexes,
+                                     const DicomTag& tag,
+                                     size_t countItems)
   {
-    if (tag.GetElement() != 0x0000)
+    if (countItems == 0 &&
+        tag.GetElement() != 0x0000)
     {
       Json::Value& node = CreateNode(parentTags, parentIndexes, tag);
       node[KEY_VR] = EnumerationToString(ValueRepresentation_Sequence);
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h
--- a/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/DicomWebJsonVisitor.h	Tue Jul 06 08:12:26 2021 +0200
@@ -91,9 +91,10 @@
                                      ValueRepresentation vr)
       ORTHANC_OVERRIDE;
 
-    virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags,
-                                      const std::vector<size_t>& parentIndexes,
-                                      const DicomTag& tag)
+    virtual Action VisitSequence(const std::vector<DicomTag>& parentTags,
+                                 const std::vector<size_t>& parentIndexes,
+                                 const DicomTag& tag,
+                                 size_t countItems)
       ORTHANC_OVERRIDE;
 
     virtual Action VisitBinary(const std::vector<DicomTag>& parentTags,
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp
--- a/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/FromDcmtkBridge.cpp	Tue Jul 06 08:12:26 2021 +0200
@@ -2415,6 +2415,20 @@
       evr = EVR_UN;
     }
 
+    if (evr == EVR_UN)
+    {
+      // New in Orthanc 1.9.5
+      DictionaryLocker locker;
+      
+      const DcmDictEntry* entry = locker->findEntry(element.getTag().getXTag(),
+                                                    element.getTag().getPrivateCreator());
+
+      if (entry != NULL)
+      {
+        evr = entry->getEVR();
+      }
+    }
+
     const ValueRepresentation vr = FromDcmtkBridge::Convert(evr);
 
     
@@ -2846,41 +2860,38 @@
       // etc. are not." The following dynamic_cast is thus OK.
       DcmSequenceOfItems& sequence = dynamic_cast<DcmSequenceOfItems&>(element);
 
-      if (sequence.card() == 0)
+      ITagVisitor::Action action = visitor.VisitSequence(parentTags, parentIndexes, tag, sequence.card());
+
+      switch (action)
       {
-        ITagVisitor::Action action = visitor.VisitEmptySequence(parentTags, parentIndexes, tag);
-
-        switch (action)
-        {
-          case ITagVisitor::Action_None:
-            return true;
-
-          case ITagVisitor::Action_Remove:
-            return false;
-
-          case ITagVisitor::Action_Replace:
-            throw OrthancException(ErrorCode_NotImplemented, "Iterator cannot replace sequences");
-
-          default:
-            throw OrthancException(ErrorCode_ParameterOutOfRange);
-        }
+        case ITagVisitor::Action_None:
+          if (sequence.card() != 0)  // Minor optimization to avoid creating "tags" and "indexes" if not needed
+          {
+            std::vector<DicomTag> tags = parentTags;
+            std::vector<size_t> indexes = parentIndexes;
+            tags.push_back(tag);
+            indexes.push_back(0);
+
+            for (unsigned long i = 0; i < sequence.card(); i++)
+            {
+              indexes.back() = static_cast<size_t>(i);
+              DcmItem* child = sequence.getItem(i);
+              ApplyVisitorToDataset(*child, visitor, tags, indexes, encoding, hasCodeExtensions);
+            }
+          }
+
+          return true;  // Keep
+
+        case ITagVisitor::Action_Remove:
+          return false;
+
+        case ITagVisitor::Action_Replace:
+          throw OrthancException(ErrorCode_NotImplemented, "Iterator cannot replace sequences");
+
+        default:
+          throw OrthancException(ErrorCode_ParameterOutOfRange);
       }
-      else
-      {
-        std::vector<DicomTag> tags = parentTags;
-        std::vector<size_t> indexes = parentIndexes;
-        tags.push_back(tag);
-        indexes.push_back(0);
-
-        for (unsigned long i = 0; i < sequence.card(); i++)
-        {
-          indexes.back() = static_cast<size_t>(i);
-          DcmItem* child = sequence.getItem(i);
-          ApplyVisitorToDataset(*child, visitor, tags, indexes, encoding, hasCodeExtensions);
-        }
-
-        return true;  // Keep
-      }
+
     }
   }
 
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/Sources/DicomParsing/ITagVisitor.h
--- a/OrthancFramework/Sources/DicomParsing/ITagVisitor.h	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/Sources/DicomParsing/ITagVisitor.h	Tue Jul 06 08:12:26 2021 +0200
@@ -51,9 +51,10 @@
                                      ValueRepresentation vr) = 0;
 
     // SQ - can return "Remove" or "None"
-    virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags,
-                                      const std::vector<size_t>& parentIndexes,
-                                      const DicomTag& tag) = 0;
+    virtual Action VisitSequence(const std::vector<DicomTag>& parentTags,
+                                 const std::vector<size_t>& parentIndexes,
+                                 const DicomTag& tag,
+                                 size_t countItems) = 0;
 
     // SL, SS, UL, US - can return "Remove" or "None"
     virtual Action VisitIntegers(const std::vector<DicomTag>& parentTags,
diff -r bf852fd773b7 -r 979ae3ea3381 OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp
--- a/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Mon Jul 05 17:50:58 2021 +0200
+++ b/OrthancFramework/UnitTestsSources/FromDcmtkTests.cpp	Tue Jul 06 08:12:26 2021 +0200
@@ -2714,7 +2714,10 @@
     ASSERT_NE("1.2.840.113619.2.176.2025.1499492.7040.1171286241.719", vv1[REF_IM_SEQ][0][REF_SOP_INSTANCE].asString());
     ASSERT_NE("1.2.840.113619.2.176.2025.1499492.7040.1171286241.726", vv1[REF_IM_SEQ][1][REF_SOP_INSTANCE].asString());
     ASSERT_NE("1.2.840.113704.1.111.7016.1342451220.40", vv1[REL_SERIES_SEQ][0][STUDY_INSTANCE_UID].asString());
-    ASSERT_EQ("WORLD", vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0][SERIES_DESCRIPTION].asString());
+
+    // Contrarily to Orthanc 1.9.4, the "SERIES_DESCRIPTION" is also removed from nested sequences
+    ASSERT_EQ(1u, vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0].size());
+    ASSERT_EQ("122403", vv1[REL_SERIES_SEQ][0][PURPOSE_CODE_SEQ][0]["0008,0100"].asString());
   }
 
   {
@@ -2774,17 +2777,35 @@
       }        
     }
 
-    virtual Action VisitEmptySequence(const std::vector<DicomTag>& parentTags,
-                                      const std::vector<size_t>& parentIndexes,
-                                      const DicomTag& tag) ORTHANC_OVERRIDE
+    virtual Action VisitSequence(const std::vector<DicomTag>& parentTags,
+                                 const std::vector<size_t>& parentIndexes,
+                                 const DicomTag& tag,
+                                 size_t countItems) ORTHANC_OVERRIDE
     {
       seen_ |= (1 << 1);
       
-      if (parentTags.size() == 1u &&
-          parentIndexes.size() == 1u &&
-          parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE &&
-          parentIndexes[0] == 0u &&
-          DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedPatientSequence)
+      if (parentTags.size() == 0u &&
+          parentIndexes.size() == 0u &&
+          tag == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE &&
+          countItems == 1)
+      {
+        return Action_None;
+      }
+      else if (parentTags.size() == 1u &&
+               parentIndexes.size() == 1u &&
+               parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE &&
+               parentIndexes[0] == 0u &&
+               countItems == 0 &&
+               DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedPatientSequence)
+      {
+        return Action_Remove;
+      }
+      else if (parentTags.size() == 1u &&
+               parentIndexes.size() == 1u &&
+               parentTags[0] == DICOM_TAG_REFERENCED_IMAGE_SEQUENCE &&
+               parentIndexes[0] == 0u &&
+               countItems == 1 &&
+               DcmTagKey(tag.GetGroup(), tag.GetElement()) == DCM_ReferencedStudySequence)
       {
         return Action_Remove;
       }
@@ -2914,7 +2935,8 @@
     v["ReferencedSOPClassUID"] = "1.2.840.10008.5.1.4.1.1.4";
     v["ReferencedImageSequence"][0]["ReferencedSOPClassUID"] = "1.2.840.10008.5.1.4.1.1.4";
     v["ReferencedImageSequence"][0]["ReferencedSOPInstanceUID"] = "1.2.840.113619.2.176.2025.1499492.7040.1171286241.719";
-    v["ReferencedImageSequence"][0]["ReferencedPatientSequence"] = Json::arrayValue;  // Empty sequence
+    v["ReferencedImageSequence"][0]["ReferencedPatientSequence"] = Json::arrayValue;  // Empty nested sequence
+    v["ReferencedImageSequence"][0]["ReferencedStudySequence"][0]["PatientID"] = "Hello";  // Non-empty nested sequence
     v["ReferencedImageSequence"][0]["0011,1311"] = "abcd";  // Binary
 
     dicom.reset(ParsedDicomFile::CreateFromJson(v, DicomFromJsonFlags_None, "PrivateCreator"));