Added methods to the ADS for working with groups. Re #6199

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

@@ -142,6 +142,10 @@ class DLLExport AnalysisDataServiceImpl : public Kernel::DataService<API::Worksp
    boost::shared_ptr<API::Workspace> find( const std::string& name) const;
    /// Print contents of the ADS
    void print() const;
+   /// Add a workspace to a group
+   void addToGroup(const std::string& grpName, const std::string& wsName);
+   /// Remove a workspace from a group.
+   void removeFromGroup(const std::string& grpName, const std::string& wsName);
 
 private:
    /// Checks the name is valid, throwing if not

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

@@ -80,7 +80,8 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem
     bool isDirty(const int n=1) const;
     /// Get the footprint in memory in bytes.
     virtual size_t getMemorySize() const = 0;
-
+    /// Check if the workspace is in the ADS.
+    bool inADS() const { return ! m_name.empty(); }
 
     /// Returns a reference to the WorkspaceHistory
     WorkspaceHistory& history() { return m_history; }

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

@@ -230,19 +230,56 @@ namespace Mantid
     void AnalysisDataServiceImpl::print() const
     {
         g_log.debug() << "Workspaces in ADS:" << std::endl;
+        std::cerr << "Workspaces in ADS:" << std::endl;
         std::vector<Workspace_sptr> workspaces = getObjects();
         for(auto it = workspaces.begin(); it != workspaces.end(); ++it)
         {
           Workspace *ws = it->get();
-          g_log.debug() << (**it).name() << std::endl;
+          g_log.debug() << " - " << (**it).name() << std::endl;
+          std::cerr << " - " << (**it).name() << std::endl;
           WorkspaceGroup* wsg = dynamic_cast<WorkspaceGroup*>(ws);
           if ( wsg )
           {
-              wsg->print("  ");
+              wsg->print("     ");
           }
         }
     }
 
+    /**
+     * Add a workspace to a workspace group. Both workspaces must be in the ADS.
+     * @param grpName :: A name of a group to add to.
+     * @param wsName :: A name of a workspace to add to a group.
+     */
+    void AnalysisDataServiceImpl::addToGroup(const std::string &grpName, const std::string &wsName)
+    {
+        Workspace_sptr ws = retrieve( wsName );
+        // if the workspace is on top level - remove it
+        removeObject( wsName, false );
+        WorkspaceGroup_sptr group = retrieveWS<WorkspaceGroup>( grpName );
+        group->addWorkspace( ws );
+        // notify that the group changed
+        notificationCenter.postNotification(new GroupUpdatedNotification( grpName ));
+    }
+
+    /**
+     * Remove a workspace from a workspace group. Both workspaces must be in the ADS.
+     * @param grpName :: A name of a group to remove from.
+     * @param wsName :: A name of a workspace to remove from a group.
+     */
+    void AnalysisDataServiceImpl::removeFromGroup(const std::string &grpName, const std::string &wsName)
+    {
+        Workspace_sptr ws = retrieve( wsName );
+        WorkspaceGroup_sptr group = retrieveWS<WorkspaceGroup>( grpName );
+        group->removeWorkspace( ws );
+        if ( ! doesExist(wsName) )
+        {
+            // add removed worksapce back if none of other groups contains it
+            addObject( wsName, ws, false );
+        }
+        // notify that the group changed
+        notificationCenter.postNotification(new GroupUpdatedNotification( grpName ));
+    }
+
     //-------------------------------------------------------------------------
     // Private methods
     //-------------------------------------------------------------------------

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

@@ -97,11 +97,18 @@ size_t WorkspaceGroup::getMemorySize() const
  */
 void WorkspaceGroup::add(const std::string& name)
 {
-  Workspace_sptr ws = AnalysisDataService::Instance().retrieve( name );
-  AnalysisDataService::Instance().remove( name );
-  ws->setName( name );
-  addWorkspace( ws );
-  g_log.debug() << "workspacename added to group vector =  " << name <<std::endl;
+    if ( inADS() )
+    {
+        // call the ADS method to handle the notifications properly
+        AnalysisDataService::Instance().addToGroup( this->name(), name );
+    }
+    else
+    {
+        // if group isn't in the ADS just add the pointer
+        Workspace_sptr ws = AnalysisDataService::Instance().retrieve( name );
+        addWorkspace( ws );
+        g_log.debug() << "workspacename added to group vector =  " << name <<std::endl;
+    }
 }
 
 /**
@@ -310,20 +317,25 @@ void WorkspaceGroup::removeAll()
  */
 void WorkspaceGroup::remove(const std::string& wsName)
 {
-  Poco::Mutex::ScopedLock _lock(m_mutex);
-  auto it = m_workspaces.begin();
-  for(; it != m_workspaces.end(); ++it)
-  {
-    if ( (**it).name() == wsName )
+    if ( inADS() )
     {
-      // leave removed workspace in the ADS
-      auto ws = *it;
-      m_workspaces.erase(it);
-      AnalysisDataService::Instance().add( ws->name(), ws );
-      break;
+        AnalysisDataService::Instance().removeFromGroup( this->name(), wsName );
+    }
+    else
+    {
+        Poco::Mutex::ScopedLock _lock(m_mutex);
+        auto it = m_workspaces.begin();
+        for(; it != m_workspaces.end(); ++it)
+        {
+            if ( (**it).name() == wsName )
+            {
+                // leave removed workspace in the ADS
+                auto ws = *it;
+                m_workspaces.erase(it);
+                break;
+            }
+        }
     }
-  }
-  updated();
 }
 
 /**
@@ -403,30 +415,6 @@ void WorkspaceGroup::print(const std::string &padding) const
   }
 }
 
-/**
- * Callback when a after-replace notification is received
- * Replaces a member if it was replaced in the ADS.
- * @param notice :: A pointer to a workspace after-replace notificiation object
- */
-//void WorkspaceGroup::workspaceReplaceHandle(Mantid::API::WorkspaceBeforeReplaceNotification_ptr notice)
-//{
-//  Poco::Mutex::ScopedLock _lock(m_mutex);
-//  bool isObserving = m_observingADS;
-//  if ( isObserving )
-//    observeADSNotifications( false );
-//  const std::string replacedName = notice->object_name();
-//  for(auto citr=m_workspaces.begin(); citr!=m_workspaces.end(); ++citr)
-//  {
-//    if ( (**citr).name() == replacedName )
-//    {
-//      *citr = notice->new_object();
-//      break;
-//    }
-//  }
-//  if ( isObserving )
-//    observeADSNotifications( true );
-//}
-
 /**
  * This method returns true if the workspace group is empty
  * @return true if workspace is empty

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

@@ -17,10 +17,41 @@ namespace
     virtual size_t getMemorySize() const { return 1; }
   };
   typedef boost::shared_ptr<MockWorkspace> MockWorkspace_sptr;
+
+
 }
 
+template<class Notification>
+class TestADSObserver
+{
+  Poco::NObserver<TestADSObserver, Notification> m_observer;
+public:
+  bool received;
+  TestADSObserver():
+    m_observer(*this,&TestADSObserver::handleNotification),
+    received(false)
+  {
+    AnalysisDataService::Instance().notificationCenter.addObserver(m_observer);
+  }
+  ~TestADSObserver()
+  {
+      AnalysisDataService::Instance().notificationCenter.removeObserver(m_observer);
+  }
+
+  //handles notification
+  void handleNotification(const Poco::AutoPtr<Notification>&)
+  {
+    received = true;
+  }
+};
+
 class AnalysisDataServiceTest : public CxxTest::TestSuite
 {
+
+    TestADSObserver<AnalysisDataServiceImpl::GroupUpdatedNotification> m_workspaceGroupObserver;
+    TestADSObserver<AnalysisDataServiceImpl::AddNotification> m_addObserver;
+    TestADSObserver<AnalysisDataServiceImpl::PostDeleteNotification> m_deleteObserver;
+
 public:
 
   void test_IsValid_Returns_An_Empty_String_For_A_Valid_Name_When_All_CharsAre_Allowed()
@@ -316,6 +347,99 @@ class AnalysisDataServiceTest : public CxxTest::TestSuite
     AnalysisDataService::Instance().clear();
   }
 
+  void test_addToGroup()
+  {
+      auto& ADS = AnalysisDataService::Instance();
+      // populate the ADS
+      auto work = addToADS("work");
+      // create a group
+      WorkspaceGroup_sptr group( new WorkspaceGroup );
+      ADS.add("group", group);
+
+      m_workspaceGroupObserver.received = false;
+      m_addObserver.received = false;
+      m_deleteObserver.received = false;
+
+      ADS.addToGroup("group", "work");
+      TS_ASSERT( m_workspaceGroupObserver.received );
+      TS_ASSERT( ! m_addObserver.received );
+      TS_ASSERT( ! m_deleteObserver.received );
+      TS_ASSERT( group->contains("work") );
+      TS_ASSERT( ADS.doesExist("work") );
+      TS_ASSERT_EQUALS(ADS.count(work),1);
+
+      WorkspaceGroup_sptr group1( new WorkspaceGroup );
+      group1->addWorkspace(MockWorkspace_sptr(new MockWorkspace));
+      ADS.add("group1", group1);
+      TS_ASSERT_EQUALS( ADS.size(), 2 );
+
+      m_workspaceGroupObserver.received = false;
+      m_addObserver.received = false;
+      m_deleteObserver.received = false;
+
+      ADS.addToGroup("group", "group1_1");
+      TS_ASSERT( m_workspaceGroupObserver.received );
+      TS_ASSERT( ! m_addObserver.received );
+      TS_ASSERT( ! m_deleteObserver.received );
+      TS_ASSERT( group->contains("group1_1") );
+      TS_ASSERT( group1->contains("group1_1") );
+      TS_ASSERT( ADS.doesExist("group1_1") );
+
+      TS_ASSERT_THROWS_NOTHING(ADS.addToGroup("group","work"));
+      TS_ASSERT_EQUALS(group->size(),2);
+      TS_ASSERT_EQUALS(ADS.count(work),1);
+
+      TS_ASSERT_THROWS(ADS.addToGroup("nogroup","work"),std::runtime_error);// maybe this should create a new group?
+      TS_ASSERT_THROWS(ADS.addToGroup("group","nowork"),std::runtime_error);
+
+      ADS.clear();
+
+  }
+
+  void test_removeFromGroup()
+  {
+      auto& ADS = AnalysisDataService::Instance();
+
+      WorkspaceGroup_sptr group( new WorkspaceGroup );
+      group->addWorkspace(MockWorkspace_sptr(new MockWorkspace));
+      group->addWorkspace(MockWorkspace_sptr(new MockWorkspace));
+      ADS.add("group", group);
+
+      m_workspaceGroupObserver.received = false;
+      m_addObserver.received = false;
+      m_deleteObserver.received = false;
+
+      ADS.removeFromGroup("group", "group_2");
+
+      TS_ASSERT( m_workspaceGroupObserver.received );
+      TS_ASSERT( ! m_addObserver.received );
+      TS_ASSERT( ! m_deleteObserver.received );
+
+      TS_ASSERT( ADS.doesExist("group_2") );
+      TS_ASSERT( ! group->contains("group_2") );
+
+      WorkspaceGroup_sptr group1( new WorkspaceGroup );
+      ADS.add("group1", group1);
+      group1->add("group_1");
+      TS_ASSERT( group->contains("group_1") );
+      TS_ASSERT( group1->contains("group_1") );
+      TS_ASSERT_EQUALS(ADS.count(group->getItem(0)),2);
+
+      m_workspaceGroupObserver.received = false;
+      m_addObserver.received = false;
+      m_deleteObserver.received = false;
+
+      ADS.removeFromGroup("group1", "group_1");
+
+      TS_ASSERT( m_workspaceGroupObserver.received );
+      TS_ASSERT( ! m_addObserver.received );
+      TS_ASSERT( ! m_deleteObserver.received );
+
+      TS_ASSERT( group->contains("group_1") );
+      TS_ASSERT( ! group1->contains("group_1") );
+      TS_ASSERT_EQUALS(ADS.count(group->getItem(0)),1);
+  }
+
 private:
 
   /// If replace=true then usea addOrReplace

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

@@ -517,21 +517,20 @@ class WorkspaceGroupTest : public CxxTest::TestSuite
 
     group->add( "name_0" );
     TS_ASSERT( observer.received );
-    observer.received = false;
 
+    observer.received = false;
     group->remove( "name_0" );
     TS_ASSERT( observer.received );
-    observer.received = false;
 
     AnalysisDataService::Instance().remove("group");
 
+    observer.received = false;
+
     group->add( "name_12" );
     TS_ASSERT( !observer.received );
-    observer.received = false;
 
     group->remove( "name_12" );
     TS_ASSERT( !observer.received );
-    observer.received = false;
 
     AnalysisDataService::Instance().clear();
   }

Code/Mantid/Framework/Algorithms/test/CheckWorkspacesMatchTest.h

@@ -741,6 +741,7 @@ class CheckWorkspacesMatchTest : public CxxTest::TestSuite
 
   void test_Input_With_Two_Groups_When_Single_Item_Checking_Is_Disabled()
   {
+    Mantid::API::AnalysisDataService::Instance().clear();
     // Create a group
     const std::string groupOneName("TestGroupOne");
     WorkspaceGroup_sptr groupOne = WorkspaceCreationHelper::CreateWorkspaceGroup(2, 2, 2, groupOneName);

Code/Mantid/Framework/Algorithms/test/DeleteWorkspaceTest.h

@@ -67,11 +67,12 @@ class DeleteWorkspaceTest : public CxxTest::TestSuite
     dataStore.add(testName2, testWS2);
 
     auto group = WorkspaceGroup_sptr( new WorkspaceGroup );
+    dataStore.add("group", group);
     group->add( testName1 );
     group->add( testName2 );
-    dataStore.add("group", group);
 
     TS_ASSERT_EQUALS(dataStore.size(), 1);
+    dataStore.print();
 
     Mantid::Algorithms::DeleteWorkspace alg;
     alg.initialize();

Code/Mantid/Framework/Algorithms/test/GeneratePeaksTest.h

@@ -56,6 +56,7 @@ class GeneratePeaksTest : public CxxTest::TestSuite
    */
   void test_UserBinningParameters()
   {
+    AnalysisDataService::Instance().clear();
     // 1. Create input
     DataObjects::TableWorkspace_sptr peakparmsws = createTestPeakParameters();