Do not allow duplicate observers on an algorithm

Add unit test for this

Refs #10377

Code/Mantid/Framework/API/inc/MantidAPI/Algorithm.h

@@ -219,9 +219,10 @@ class MANTID_API_DLL Algorithm : public IAlgorithm, public Kernel::PropertyManag
 
   /// Add an observer for a notification
   void addObserver(const Poco::AbstractObserver& observer) const;
-
   /// Remove an observer
   void removeObserver(const Poco::AbstractObserver& observer) const;
+  /// Gets the number of added observers
+  size_t numberOfObservers() const;
 
   /// Raises the cancel flag.
   virtual void cancel();
@@ -403,6 +404,9 @@ class MANTID_API_DLL Algorithm : public IAlgorithm, public Kernel::PropertyManag
   bool m_groupsHaveSimilarNames;
   /// A non-recursive mutex for thread-safety
   mutable Kernel::Mutex m_mutex;
+
+  /// Vector storing added observers
+  mutable std::vector<const Poco::AbstractObserver*> m_externalObservers;
 };
 
 ///Typedef for a shared pointer to an Algorithm

Code/Mantid/Framework/API/src/Algorithm.cpp

@@ -195,15 +195,48 @@ namespace Mantid
     */
     void Algorithm::addObserver(const Poco::AbstractObserver& observer)const
     {
+      const Poco::AbstractObserver* o = &observer;
+
+      for(auto it = m_externalObservers.begin(); it != m_externalObservers.end(); ++it)
+      {
+        if(*it == o)
+          return;
+      }
+
       notificationCenter().addObserver(observer);
+      m_externalObservers.push_back(o);
     }
 
     /**  Remove an observer
     @param observer :: Reference to the observer to remove
     */
     void Algorithm::removeObserver(const Poco::AbstractObserver& observer)const
     {
-      notificationCenter().removeObserver(observer);
+      bool found = false;
+
+      auto it = m_externalObservers.begin();
+      for(; it != m_externalObservers.end(); ++it)
+      {
+        if(*it == &observer)
+        {
+          found = true;
+          break;
+        }
+      }
+
+      if(found)
+      {
+        notificationCenter().removeObserver(observer);
+        m_externalObservers.erase(it);
+      }
+    }
+
+    /** Gets the number of added observers.
+     * @returns Observer count
+     */
+    size_t Algorithm::numberOfObservers() const
+    {
+      return m_externalObservers.size();
     }
 
     //---------------------------------------------------------------------------------------------

Code/Mantid/Framework/API/test/AlgorithmTest.h

@@ -15,8 +15,9 @@
 #include "MantidKernel/RebinParamsValidator.h"
 #include "FakeAlgorithms.h"
 #include <map>
+#include <Poco/NObserver.h>
 
-using namespace Mantid::Kernel; 
+using namespace Mantid::Kernel;
 using namespace Mantid::API;
 
 class StubbedWorkspaceAlgorithm : public Algorithm
@@ -92,7 +93,7 @@ class AlgorithmWithValidateInputs : public Algorithm
   const std::string workspaceMethodName() const { return "methodname"; }
   const std::string workspaceMethodOnTypes() const { return "MatrixWorkspace;ITableWorkspace"; }
   const std::string workspaceMethodInputProperty() const { return "InputWorkspace"; }
-  
+
   void init()
   {
     declareProperty("PropertyA", 12);
@@ -169,7 +170,7 @@ class AlgorithmTest : public CxxTest::TestSuite
     Mantid::API::AlgorithmFactory::Instance().unsubscribe("ToyAlgorithm",1);
     Mantid::API::AlgorithmFactory::Instance().unsubscribe("ToyAlgorithmTwo", 1);
   }
-  
+
   void testAlgorithm()
   {
     std::string theName = alg.name();
@@ -239,6 +240,31 @@ class AlgorithmTest : public CxxTest::TestSuite
     TS_ASSERT( myAlg.isExecuted() );
   }
 
+  void testDuplicateObserverAdd()
+  {
+    ToyAlgorithm myAlg;
+
+    Poco::NObserver<AlgorithmTest, Algorithm::FinishedNotification> finishObserver1(*this, &AlgorithmTest::_finishHandle);
+    Poco::NObserver<AlgorithmTest, Algorithm::FinishedNotification> finishObserver2(*this, &AlgorithmTest::_finishHandle);
+    Poco::NObserver<AlgorithmTest, Algorithm::FinishedNotification> finishObserver3(*this, &AlgorithmTest::_finishHandle);
+
+    myAlg.addObserver(finishObserver1);
+    myAlg.addObserver(finishObserver2);
+    myAlg.addObserver(finishObserver3);
+    myAlg.addObserver(finishObserver1);
+    myAlg.removeObserver(finishObserver3);
+
+    // Duplicate observers should be discarded
+    TS_ASSERT_EQUALS(myAlg.numberOfObservers(), 2);
+  }
+
+  /** A dummy finish handle for use in the abov est case.
+   * @param pNf Poco notification
+   */
+  void _finishHandle(const Poco::AutoPtr<Algorithm::FinishedNotification>& pNf)
+  {
+    UNUSED_ARG(pNf)
+  }
 
   void testSetPropertyValue()
   {
@@ -251,15 +277,15 @@ class AlgorithmTest : public CxxTest::TestSuite
     TS_ASSERT( alg.existsProperty("prop1") )
     TS_ASSERT( ! alg.existsProperty("notThere") )
   }
-  
+
   void testGetPropertyValue()
   {
     std::string value;
     TS_ASSERT_THROWS_NOTHING( value = alg.getPropertyValue("prop2") )
     TS_ASSERT( ! value.compare("1") )
-    TS_ASSERT_THROWS(alg.getPropertyValue("ghjkgh"), Exception::NotFoundError )    
+    TS_ASSERT_THROWS(alg.getPropertyValue("ghjkgh"), Exception::NotFoundError )
   }
-  
+
   void testGetProperties()
   {
     std::vector<Property*> vec = alg.getProperties();
@@ -334,7 +360,7 @@ class AlgorithmTest : public CxxTest::TestSuite
     IAlgorithm_sptr testAlg = runFromString("ToyAlgorithm.1");
     TS_ASSERT_EQUALS(testAlg->name(), "ToyAlgorithm");
     TS_ASSERT_EQUALS(testAlg->version(), 1);
-    
+
     // No brackets
     testAlg = runFromString("ToyAlgorithm.1");
     TS_ASSERT_EQUALS(testAlg->name(), "ToyAlgorithm");
@@ -346,7 +372,7 @@ class AlgorithmTest : public CxxTest::TestSuite
     IAlgorithm_sptr testAlg = runFromString("ToyAlgorithm.1()");
     TS_ASSERT_EQUALS(testAlg->name(), "ToyAlgorithm");
     TS_ASSERT_EQUALS(testAlg->version(), 1);
-    
+
     // No brackets
     testAlg = runFromString("ToyAlgorithm.1");
     TS_ASSERT_EQUALS(testAlg->name(), "ToyAlgorithm");
@@ -359,7 +385,7 @@ class AlgorithmTest : public CxxTest::TestSuite
     IAlgorithm_sptr testAlg = runFromString("ToyAlgorithm.2(prop1=val1,prop2=8,prop3=10.0,Binning=0.2,0.2,1.4)");
     TS_ASSERT_EQUALS(testAlg->name(), "ToyAlgorithm");
     TS_ASSERT_EQUALS(testAlg->version(), 2);
-    
+
     // On gcc we get ambiguous function calls doing
     // std::string s;
     // s = getProperty(...);
@@ -614,7 +640,7 @@ class AlgorithmTest : public CxxTest::TestSuite
     {
       TS_ASSERT( !alg.isExecuted() );
       return WorkspaceGroup_sptr();
-    } 
+    }
     TS_ASSERT( alg.isExecuted() )
     Workspace_sptr out1 = AnalysisDataService::Instance().retrieve("D");
     WorkspaceGroup_sptr group = boost::dynamic_pointer_cast<WorkspaceGroup>(out1);
@@ -780,6 +806,6 @@ class AlgorithmTest : public CxxTest::TestSuite
   MatrixWorkspace_sptr ws3;
 };
 
- 
+
 
 #endif /*ALGORITHMTEST_H_*/