ENH: Improve SubjectHierarchy API for UIDs

Basic methods for getting and setting UIDs in subject hierarchy items were missing.
Fixed by adding RemoveItemUID, GetItemUIDNames, HasItemUID methods.

Libs/MRML/Core/Testing/CMakeLists.txt

@@ -81,6 +81,7 @@ create_test_sourcelist(Tests ${KIT}CxxTests.cxx
   vtkMRMLStorableNodeTest1.cxx
   vtkMRMLStorageNodeTest1.cxx
   vtkMRMLStreamingVolumeNodeTest1.cxx
+  vtkMRMLSubjectHierarchyNodeTest1.cxx
   vtkMRMLTableNodeTest1.cxx
   vtkMRMLTableStorageNodeTest1.cxx
   vtkMRMLTableSQLiteStorageNodeTest.cxx

Libs/MRML/Core/Testing/vtkMRMLSubjectHierarchyNodeTest1.cxx

@@ -0,0 +1,119 @@
+/*==============================================================================
+
+  Copyright (c) Laboratory for Percutaneous Surgery (PerkLab)
+  Queen's University, Kingston, ON, Canada. All Rights Reserved.
+
+  See COPYRIGHT.txt
+  or http://www.slicer.org/copyright/copyright.txt for details.
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+==============================================================================*/
+
+#include "vtkMRMLCoreTestingMacros.h"
+#include "vtkMRMLModelNode.h"
+#include "vtkMRMLScene.h"
+#include "vtkMRMLSubjectHierarchyNode.h"
+
+int vtkMRMLSubjectHierarchyNodeTest1(int , char * [])
+{
+  // Add a scene with 3 text nodes
+  vtkNew<vtkMRMLScene> scene;
+
+  // Check if subject hierarchy node is provided
+  vtkMRMLSubjectHierarchyNode* shNode = scene->GetSubjectHierarchyNode();
+  CHECK_NOT_NULL(shNode);
+
+  vtkNew<vtkMRMLModelNode> dataNode1;
+  scene->AddNode(dataNode1);
+
+  // Create subject hierarchy item
+  vtkIdType itemId1 = shNode->CreateItem(shNode->GetSceneItemID(), dataNode1, "test");
+  CHECK_BOOL(itemId1 == vtkMRMLSubjectHierarchyNode::GetInvalidItemID(), false);
+
+  // Test UIDs
+  /////////////////////////
+
+  std::string uidName = "TESTID";
+  std::string uidValue = "123";
+
+  // Test API with missing UID
+  CHECK_BOOL(shNode->HasItemUID(itemId1, uidName), false);
+  CHECK_STD_STRING(shNode->GetItemUID(itemId1, uidName), "");
+  CHECK_INT(shNode->GetItemUIDNames(itemId1).size(), 0);
+
+  // Test after UID is set
+
+  shNode->SetItemUID(itemId1, uidName, uidValue);
+
+  CHECK_BOOL(shNode->HasItemUID(itemId1, uidName), true);
+  CHECK_STD_STRING(shNode->GetItemUID(itemId1, uidName), uidValue);
+  CHECK_INT(shNode->GetItemUIDNames(itemId1).size(), 1);
+
+  // Test after UID is removed
+
+  CHECK_BOOL(shNode->RemoveItemUID(itemId1, uidName), true);
+
+  CHECK_BOOL(shNode->HasItemUID(itemId1, uidName), false);
+  CHECK_STD_STRING(shNode->GetItemUID(itemId1, uidName), "");
+  CHECK_INT(shNode->GetItemUIDNames(itemId1).size(), 0);
+
+  // Test getting list of all UID names
+
+  shNode->SetItemUID(itemId1, "abc", "3");
+  shNode->SetItemUID(itemId1, "defffff", "5");
+  shNode->SetItemUID(itemId1, "ggggg", "8");
+
+  auto ids = shNode->GetItemUIDNames(itemId1);
+  CHECK_INT(ids.size(), 3);
+  // Check if all 3 strings are found in the returned list
+  CHECK_BOOL(std::find(ids.begin(), ids.end(), "abc") != ids.end(), true);
+  CHECK_BOOL(std::find(ids.begin(), ids.end(), "defffff") != ids.end(), true);
+  CHECK_BOOL(std::find(ids.begin(), ids.end(), "ggggg") != ids.end(), true);
+
+  // Test attributes
+  /////////////////////////
+
+  std::string attributeName = "TESTATT";
+  std::string attributeValue = "987";
+
+  // Test API with missing attributes
+  CHECK_BOOL(shNode->HasItemAttribute(itemId1, attributeName), false);
+  CHECK_STD_STRING(shNode->GetItemAttribute(itemId1, attributeName), "");
+  CHECK_INT(shNode->GetItemAttributeNames(itemId1).size(), 0);
+
+  // Test after attributes is set
+
+  shNode->SetItemAttribute(itemId1, attributeName, attributeValue);
+
+  CHECK_BOOL(shNode->HasItemAttribute(itemId1, attributeName), true);
+  CHECK_STD_STRING(shNode->GetItemAttribute(itemId1, attributeName), attributeValue);
+  CHECK_INT(shNode->GetItemAttributeNames(itemId1).size(), 1);
+
+  // Test after attributes is removed
+
+  CHECK_BOOL(shNode->RemoveItemAttribute(itemId1, attributeName), true);
+
+  CHECK_BOOL(shNode->HasItemAttribute(itemId1, attributeName), false);
+  CHECK_STD_STRING(shNode->GetItemAttribute(itemId1, attributeName), "");
+  CHECK_INT(shNode->GetItemAttributeNames(itemId1).size(), 0);
+
+  // Test getting list of all attributes names
+
+  shNode->SetItemAttribute(itemId1, "asd", "rrr");
+  shNode->SetItemAttribute(itemId1, "zxcv", "fff");
+  shNode->SetItemAttribute(itemId1, "qwer", "vvv");
+
+  auto atts = shNode->GetItemAttributeNames(itemId1);
+  CHECK_INT(atts.size(), 3);
+  // Check if all 3 strings are found in the returned list
+  CHECK_BOOL(std::find(atts.begin(), atts.end(), "asd") != atts.end(), true);
+  CHECK_BOOL(std::find(atts.begin(), atts.end(), "zxcv") != atts.end(), true);
+  CHECK_BOOL(std::find(atts.begin(), atts.end(), "qwer") != atts.end(), true);
+
+  return EXIT_SUCCESS;
+}

Libs/MRML/Core/vtkMRMLSubjectHierarchyNode.cxx

@@ -129,9 +129,19 @@ class vtkSubjectHierarchyItem : public vtkObject
 
   /// Set UID to the item
   void SetUID(std::string uidName, std::string uidValue);
+  /// Remove UID from the item
+  /// \return True if UID was removed, false if not found
+  bool RemoveUID(std::string uidName);
   /// Get a UID with a given name
   /// \return The UID value if exists, empty string if does not
   std::string GetUID(std::string uidName);
+  /// Get UID names
+  /// \return List of UID names
+  std::vector<std::string> GetUIDNames();
+  /// Determine if a given UID is present in an item.
+  /// Especially useful if need to determine whether a UID value is empty string or the UID is missing
+  bool HasUID(std::string uidName);
+
   /// Set attribute to item
   /// \parameter attributeValue Value of attribute. If empty string, then attribute is removed
   void SetAttribute(std::string attributeName, std::string attributeValue);
@@ -1280,33 +1290,70 @@ void vtkSubjectHierarchyItem::RemoveAllChildren()
 void vtkSubjectHierarchyItem::SetUID(std::string uidName, std::string uidValue)
 {
   // Use the find function to prevent adding an empty UID to the map
-  if (this->UIDs.find(uidName) != this->UIDs.end())
+  auto it = this->UIDs.find(uidName);
+  if (it != this->UIDs.end())
   {
     // Log warning if the new UID value is different than the one already set
-    if (this->UIDs[uidName].compare(uidValue))
+    if (it->second == uidValue)
     {
-      vtkWarningMacro( "SetUID: UID with name '" << uidName << "' already exists in subject hierarchy item '" << this->GetName()
-        << "' with value '" << this->UIDs[uidName] << "'. Replacing it with value '" << uidValue << "'" );
+      return; // Do nothing if the UID values match
     }
     else
     {
-      return; // Do nothing if the UID values match
+      vtkWarningMacro( "SetUID: UID with name '" << uidName << "' already exists in subject hierarchy item '" << this->GetName()
+        << "' with value '" << it->second << "'. Replacing it with value '" << uidValue << "'" );
     }
   }
   this->UIDs[uidName] = uidValue;
   this->InvokeEvent(vtkMRMLSubjectHierarchyNode::SubjectHierarchyItemUIDAddedEvent, this);
   this->Modified();
 }
 
+//---------------------------------------------------------------------------
+bool vtkSubjectHierarchyItem::RemoveUID(std::string uidName)
+{
+  auto it = this->UIDs.find(uidName);
+  if (it == this->UIDs.end())
+  {
+    // not found
+    return false;
+  }
+
+  // Use the find function to prevent adding an empty UID to the map
+  this->UIDs.erase(it);
+  this->Modified();
+  return true;
+}
+
 //---------------------------------------------------------------------------
 std::string vtkSubjectHierarchyItem::GetUID(std::string uidName)
 {
   // Use the find function to prevent adding an empty UID to the map
-  if (this->UIDs.find(uidName) != this->UIDs.end())
+  auto it = this->UIDs.find(uidName);
+  if (it == this->UIDs.end())
   {
-    return this->UIDs[uidName];
+    // not found
+    return std::string();
   }
-  return std::string();
+  return it->second;
+}
+
+//---------------------------------------------------------------------------
+std::vector<std::string> vtkSubjectHierarchyItem::GetUIDNames()
+{
+  std::vector<std::string> uidNameList;
+  std::map<std::string, std::string>::const_iterator uidIt;
+  for (uidIt=this->UIDs.cbegin(); uidIt!=this->UIDs.cend(); ++uidIt)
+  {
+    uidNameList.push_back(uidIt->first);
+  }
+  return uidNameList;
+}
+
+//---------------------------------------------------------------------------
+bool vtkSubjectHierarchyItem::HasUID(std::string uidName)
+{
+  return (this->UIDs.find(uidName) != this->UIDs.end());
 }
 
 //---------------------------------------------------------------------------
@@ -1318,10 +1365,11 @@ void vtkSubjectHierarchyItem::SetAttribute(std::string attributeName, std::strin
     return;
   }
   // Use the find function to prevent adding an empty attribute to the map
-  if ( this->Attributes.find(attributeName) != this->Attributes.end()
-    && !attributeValue.compare(this->Attributes[attributeName]) )
+  auto it = this->Attributes.find(attributeName);
+  if (it != this->Attributes.end() && it->second == attributeValue)
   {
-    return; // Attribute to set is same as original value, nothing to do
+    // Attribute to set is same as original value, nothing to do
+    return;
   }
   this->Attributes[attributeName] = attributeValue;
   this->InvokeEvent(vtkMRMLSubjectHierarchyNode::SubjectHierarchyItemOwnerPluginSearchRequested, this);
@@ -1331,40 +1379,39 @@ void vtkSubjectHierarchyItem::SetAttribute(std::string attributeName, std::strin
 //---------------------------------------------------------------------------
 bool vtkSubjectHierarchyItem::RemoveAttribute(std::string attributeName)
 {
-  if (this->Attributes.size() == 0)
+  auto it = this->Attributes.find(attributeName);
+  if (it == this->Attributes.end())
   {
+    // not found, nothing to remove
     return false;
   }
 
   // Use the find function to prevent adding an empty attribute to the map
-  if (this->Attributes.find(attributeName) != this->Attributes.end())
-  {
-    this->Attributes.erase(attributeName);
-    this->InvokeEvent(vtkMRMLSubjectHierarchyNode::SubjectHierarchyItemOwnerPluginSearchRequested, this);
-    this->Modified();
-    return true;
-  }
-  return false;
+  this->Attributes.erase(it);
+  this->InvokeEvent(vtkMRMLSubjectHierarchyNode::SubjectHierarchyItemOwnerPluginSearchRequested, this);
+  this->Modified();
+  return true;
 }
 
 //---------------------------------------------------------------------------
 std::string vtkSubjectHierarchyItem::GetAttribute(std::string attributeName)
 {
   // Use the find function to prevent adding an empty attribute to the map
-  if ( this->Attributes.size() > 0
-    && this->Attributes.find(attributeName) != this->Attributes.end() )
+  auto it = this->Attributes.find(attributeName);
+  if (it == this->Attributes.end())
   {
-    return this->Attributes[attributeName];
+    // not found
+    return std::string();
   }
-  return std::string();
+  return it->second;
 }
 
 //---------------------------------------------------------------------------
 std::vector<std::string> vtkSubjectHierarchyItem::GetAttributeNames()
 {
   std::vector<std::string> attributeNameList;
-  std::map<std::string, std::string>::iterator attributeIt;
-  for (attributeIt=this->Attributes.begin(); attributeIt!=this->Attributes.end(); ++attributeIt)
+  std::map<std::string, std::string>::const_iterator attributeIt;
+  for (attributeIt=this->Attributes.cbegin(); attributeIt!=this->Attributes.cend(); ++attributeIt)
   {
     attributeNameList.push_back(attributeIt->first);
   }
@@ -1374,8 +1421,7 @@ std::vector<std::string> vtkSubjectHierarchyItem::GetAttributeNames()
 //---------------------------------------------------------------------------
 bool vtkSubjectHierarchyItem::HasAttribute(std::string attributeName)
 {
-  return ( this->Attributes.size() > 0
-        && this->Attributes.find(attributeName) != this->Attributes.end() );
+  return (this->Attributes.find(attributeName) != this->Attributes.end());
 }
 
 //---------------------------------------------------------------------------
@@ -2352,6 +2398,25 @@ void vtkMRMLSubjectHierarchyNode::SetItemUID(vtkIdType itemID, std::string uidNa
   item->SetUID(uidName, uidValue); // Events are invoked within this call
 }
 
+//---------------------------------------------------------------------------
+bool vtkMRMLSubjectHierarchyNode::RemoveItemUID(vtkIdType itemID, std::string uidName)
+{
+  if (!itemID)
+  {
+    vtkWarningMacro("RemoveItemUID: Invalid item ID given");
+    return false;
+  }
+  vtkSubjectHierarchyItem* item = this->Internal->FindItemByID(itemID);
+  if (!item)
+  {
+    vtkErrorMacro("RemoveItemUID: Failed to find subject hierarchy item by ID " << itemID);
+    return false;
+  }
+
+  bool result = item->RemoveUID(uidName); // Events are invoked within this call
+  return result;
+}
+
 //----------------------------------------------------------------------------
 std::string vtkMRMLSubjectHierarchyNode::GetItemUID(vtkIdType itemID, std::string uidName)
 {
@@ -2370,6 +2435,41 @@ std::string vtkMRMLSubjectHierarchyNode::GetItemUID(vtkIdType itemID, std::strin
   return item->GetUID(uidName);
 }
 
+//---------------------------------------------------------------------------
+std::vector<std::string> vtkMRMLSubjectHierarchyNode::GetItemUIDNames(vtkIdType itemID)
+{
+  if (!itemID)
+  {
+    vtkWarningMacro("GetItemUIDNames: Invalid item ID given");
+    return std::vector<std::string>();
+  }
+  vtkSubjectHierarchyItem* item = this->Internal->FindItemByID(itemID);
+  if (!item)
+  {
+    vtkErrorMacro("GetItemUIDNames: Failed to find subject hierarchy item by ID " << itemID);
+    return std::vector<std::string>();
+  }
+
+  return item->GetUIDNames();
+}
+
+//---------------------------------------------------------------------------
+bool vtkMRMLSubjectHierarchyNode::HasItemUID(vtkIdType itemID, std::string uidName)
+{
+  if (!itemID)
+  {
+    vtkWarningMacro("HasItemUID: Invalid item ID given");
+    return false;
+  }
+  vtkSubjectHierarchyItem* item = this->Internal->FindItemByID(itemID);
+  if (!item)
+  {
+    vtkErrorMacro("HasItemUID: Failed to find subject hierarchy item by ID " << itemID);
+    return false;
+  }
+
+  return item->HasUID(uidName);
+}
 
 //----------------------------------------------------------------------------
 void vtkMRMLSubjectHierarchyNode::SetItemAttribute(vtkIdType itemID, std::string attributeName, std::string attributeValue)

Libs/MRML/Core/vtkMRMLSubjectHierarchyNode.h

@@ -150,9 +150,19 @@ class VTK_MRML_EXPORT vtkMRMLSubjectHierarchyNode : public vtkMRMLNode
 
   /// Set UID to the subject hierarchy item
   void SetItemUID(vtkIdType itemID, std::string uidName, std::string uidValue);
+  /// Remove UID from subject hierarchy item
+  /// \return True if UID was removed, false if item or UID is not found
+  bool RemoveItemUID(vtkIdType itemID, std::string uidName);
   /// Get a UID with a given name
   /// \return The UID value if exists, empty string if does not
   std::string GetItemUID(vtkIdType itemID, std::string uidName);
+  /// Get UID names for a subject hierarchy item
+  /// \return List of UID names
+  std::vector<std::string> GetItemUIDNames(vtkIdType itemID);
+  /// Determine if a given UID is present in an item.
+  /// Especially useful if need to determine whether a UID value is empty string or the UID is missing
+  /// \return True if UID exists, false if item or UID is not found
+  bool HasItemUID(vtkIdType itemID, std::string uidName);
 
   /// Add attribute to the subject hierarchy item
   void SetItemAttribute(vtkIdType itemID, std::string attributeName, std::string attributeValue);