From 53a1008c464994b3911efb716b1e6799772eeee9 Mon Sep 17 00:00:00 2001
From: Benjamin Michael Wynne <b.m.wynne@ed.ac.uk>
Date: Thu, 20 Dec 2018 09:56:37 +0000
Subject: [PATCH] Uniqueness checking fixes for typeless TC adds

---
 .../xAODTrigger/Root/TrigComposite_v1.cxx     | 54 ++++++++++++++++---
 .../ut_xaodtrigger_trigcomposite_test.cxx     | 29 ++++++++++
 2 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/Event/xAOD/xAODTrigger/Root/TrigComposite_v1.cxx b/Event/xAOD/xAODTrigger/Root/TrigComposite_v1.cxx
index aea7b853e1e4..77f4b31c3d9a 100644
--- a/Event/xAOD/xAODTrigger/Root/TrigComposite_v1.cxx
+++ b/Event/xAOD/xAODTrigger/Root/TrigComposite_v1.cxx
@@ -477,17 +477,40 @@ namespace xAOD {
      // Loop over collections
      if ( endIndex - beginIndex > 1 ) {
 
+       // Check uniqueness
        const std::string mangledName = name + s_collectionSuffix;
-       for ( unsigned int index = beginIndex; index < endIndex; ++index ) {
+       const std::vector< std::string >& names = linkColNames();
+       int oldStart = -1;
+       int oldEnd = -1;
+       for( size_t nameIndex = 0; nameIndex < names.size(); ++nameIndex ) {
+
+         // Look for an existing collection with the same name
+         if( names[ nameIndex ] == mangledName ) {
+           oldEnd = nameIndex + 1;
+           if ( oldStart == -1 ) oldStart = nameIndex;
+         }
+         else if ( oldStart != -1 ) {
+           // If the start has been found, we must now be past the ned
+           break;
+         }
+       }
 
-         // Check uniqueness
-         if ( std::find( linkColNamesNC().begin(), linkColNamesNC().end(), mangledName ) == linkColNamesNC().end() ) {
+       // Erase the old collection, if there was one
+       if ( oldStart != -1 ) {
 
-           this->linkColNamesNC().push_back( mangledName );
-           this->linkColKeysNC().push_back( key );
-           this->linkColIndicesNC().push_back( index );
-           this->linkColClidsNC().push_back( clid );
-         }
+         this->linkColNamesNC().erase( this->linkColNamesNC().begin() + oldStart, this->linkColNamesNC().begin() + oldEnd );
+         this->linkColKeysNC().erase( this->linkColKeysNC().begin() + oldStart, this->linkColKeysNC().begin() + oldEnd );
+         this->linkColIndicesNC().erase( this->linkColIndicesNC().begin() + oldStart, this->linkColIndicesNC().begin() + oldEnd );
+         this->linkColClidsNC().erase( this->linkColClidsNC().begin() + oldStart, this->linkColClidsNC().begin() + oldEnd );
+       }
+
+       // Append the new collection
+       for ( unsigned int index = beginIndex; index < endIndex; ++index ) {
+
+         this->linkColNamesNC().push_back( mangledName );
+         this->linkColKeysNC().push_back( key );
+         this->linkColIndicesNC().push_back( index );
+         this->linkColClidsNC().push_back( clid );
        }
      }
      else {
@@ -500,6 +523,21 @@ namespace xAOD {
          this->linkColIndicesNC().push_back( beginIndex );
          this->linkColClidsNC().push_back( clid );
        }
+       else {
+
+         // Over-write an existing object
+         const std::vector< std::string >& names = linkColNames();
+         for( size_t nameIndex = 0; nameIndex < names.size(); ++nameIndex ) {
+
+           if( names[ nameIndex ] == name ) {
+
+             this->linkColKeysNC()[ nameIndex ] = key;
+             this->linkColIndicesNC()[ nameIndex ] = beginIndex;
+             this->linkColClidsNC()[ nameIndex ] = clid;
+             break; // Names are unique, so stop once found
+           }
+         }
+       }
      }
    }
 
diff --git a/Event/xAOD/xAODTrigger/test/ut_xaodtrigger_trigcomposite_test.cxx b/Event/xAOD/xAODTrigger/test/ut_xaodtrigger_trigcomposite_test.cxx
index 85dbf283da0e..3f02ee018d77 100644
--- a/Event/xAOD/xAODTrigger/test/ut_xaodtrigger_trigcomposite_test.cxx
+++ b/Event/xAOD/xAODTrigger/test/ut_xaodtrigger_trigcomposite_test.cxx
@@ -314,6 +314,35 @@ int main() {
 
    std::cout << "Copy link-by-link OK" << std::endl;
 
+   // Test single typeless add
+   obj->typelessSetObjectLink( "typeless1", 123, ClassID_traits< xAOD::MuonRoIContainer >::ID(), 11, 12 );
+   ElementLink< xAOD::MuonRoIContainer > getMuonRoILink = obj->objectLink<xAOD::MuonRoIContainer>("typeless1");
+   SIMPLE_ASSERT(getMuonRoILink == ElementLink<xAOD::MuonRoIContainer>( 123, 11 ));
+
+   // Test single typeless overwrite
+   obj->typelessSetObjectLink( "typeless1", 123, ClassID_traits< xAOD::MuonRoIContainer >::ID(), 12, 13 );
+   getMuonRoILink = obj->objectLink<xAOD::MuonRoIContainer>("typeless1");
+   SIMPLE_ASSERT(getMuonRoILink != ElementLink<xAOD::MuonRoIContainer>( 123, 11 ));
+   SIMPLE_ASSERT(getMuonRoILink == ElementLink<xAOD::MuonRoIContainer>( 123, 12 ));
+
+   // Test typeless collection add
+   obj->typelessSetObjectLink( "typeless2", 123, ClassID_traits< xAOD::MuonRoIContainer >::ID(), 12, 15 );
+   ElementLinkVector<xAOD::MuonRoIContainer> getMuonRoILinks = obj->objectCollectionLinks<xAOD::MuonRoIContainer>("typeless2");
+   ElementLinkVector<xAOD::MuonRoIContainer> elementLinks;
+   elementLinks.push_back( ElementLink<xAOD::MuonRoIContainer>( 123, 12 ) );
+   elementLinks.push_back( ElementLink<xAOD::MuonRoIContainer>( 123, 13 ) );
+   elementLinks.push_back( ElementLink<xAOD::MuonRoIContainer>( 123, 14 ) );
+   SIMPLE_ASSERT(getMuonRoILinks == elementLinks);
+
+   // Test typeless collection overwrite
+   obj->typelessSetObjectLink( "typeless2", 123, ClassID_traits< xAOD::MuonRoIContainer >::ID(), 5, 7 );
+   getMuonRoILinks = obj->objectCollectionLinks<xAOD::MuonRoIContainer>("typeless2");
+   SIMPLE_ASSERT(getMuonRoILinks != elementLinks);
+   elementLinks.clear();
+   elementLinks.push_back( ElementLink<xAOD::MuonRoIContainer>( 123, 5 ) );
+   elementLinks.push_back( ElementLink<xAOD::MuonRoIContainer>( 123, 6 ) );
+   SIMPLE_ASSERT(getMuonRoILinks == elementLinks);
+
    // Apparently everything went well:
    std::cout << "All tests successful." << std::endl;
 
-- 
GitLab