Changes to setName method to support ADS consistency. Re #6199

mantidproject · May 21, 2013 · 77036d2 · 77036d2
1 parent 10ebba9
commit 77036d2
Show file tree

Hide file tree

Showing 13 changed files with 162 additions and 82 deletions.
diff --git a/Code/Mantid/Framework/API/inc/MantidAPI/AnalysisDataService.h b/Code/Mantid/Framework/API/inc/MantidAPI/AnalysisDataService.h
@@ -129,7 +129,7 @@ class DLLExport AnalysisDataServiceImpl : public Kernel::DataService<API::Worksp
    boost::shared_ptr<WSTYPE> retrieveWS(const std::string& name) const
    {
      // Get as a bare workspace
-     boost::shared_ptr<Mantid::API::Workspace> workspace = Kernel::DataService<API::Workspace>::retrieve(name);
+     boost::shared_ptr<Mantid::API::Workspace> workspace = this->retrieve(name);
      // Cast to the desired type and return that.
      return boost::dynamic_pointer_cast<WSTYPE>(workspace);
    }
@@ -140,6 +140,8 @@ class DLLExport AnalysisDataServiceImpl : public Kernel::DataService<API::Worksp
    size_t count(Workspace_const_sptr workspace) const;
    /// Find a stored workspace
    boost::shared_ptr<API::Workspace> find( const std::string& name) const;
+   /// Print contents of the ADS
+   void print() const;
 
 private:
    /// Checks the name is valid, throwing if not

diff --git a/Code/Mantid/Framework/API/inc/MantidAPI/Workspace.h b/Code/Mantid/Framework/API/inc/MantidAPI/Workspace.h
@@ -67,7 +67,7 @@ class MANTID_API_DLL Workspace : public Kernel::DataItem
 
     void virtual setTitle(const std::string&);
     void setComment(const std::string&);
-    virtual void setName(const std::string&);
+    virtual void setName(const std::string& name,bool force = false);
     //virtual const std::string& getTitle() const;
     virtual const std::string getTitle() const;
     const std::string& getComment() const;

diff --git a/Code/Mantid/Framework/API/inc/MantidAPI/WorkspaceGroup.h b/Code/Mantid/Framework/API/inc/MantidAPI/WorkspaceGroup.h
@@ -62,7 +62,7 @@ class MANTID_API_DLL WorkspaceGroup : public Workspace
   /// Return a string ID of the class
   virtual const std::string id() const { return "WorkspaceGroup"; }
   /// Set the name
-  virtual void setName(const std::string &name);
+  virtual void setName(const std::string &name,bool force = false);
   /// The size of the group.
   virtual size_t getMemorySize() const;
   /// Adds a workspace to the group.
@@ -117,10 +117,10 @@ class MANTID_API_DLL WorkspaceGroup : public Workspace
 //  void workspaceDeleteHandle(Mantid::API::WorkspacePostDeleteNotification_ptr notice);
 //  /// Observer for workspace delete notfications
 //  Poco::NObserver<WorkspaceGroup, Mantid::API::WorkspacePostDeleteNotification> m_deleteObserver;
-  /// Callback when a before-replace notification is received
-  void workspaceReplaceHandle(Mantid::API::WorkspaceBeforeReplaceNotification_ptr notice);
-  /// Observer for workspace before-replace notfications
-  Poco::NObserver<WorkspaceGroup, Mantid::API::WorkspaceBeforeReplaceNotification> m_replaceObserver;
+//  /// Callback when a before-replace notification is received
+//  void workspaceReplaceHandle(Mantid::API::WorkspaceBeforeReplaceNotification_ptr notice);
+//  /// Observer for workspace before-replace notfications
+//  Poco::NObserver<WorkspaceGroup, Mantid::API::WorkspaceBeforeReplaceNotification> m_replaceObserver;
 
   /// The list of workspace pointers in the group
   std::vector<Workspace_sptr> m_workspaces;

diff --git a/Code/Mantid/Framework/API/src/Algorithm.cpp b/Code/Mantid/Framework/API/src/Algorithm.cpp
@@ -1222,8 +1222,6 @@ namespace Mantid
       // Finish up
       for (size_t i=0; i<outGroups.size(); i++)
       {
-        // Go back to observing ADS in each group.
-        outGroups[i]->observeADSNotifications(true);
         outGroups[i]->updated();
       }
 

diff --git a/Code/Mantid/Framework/API/src/AnalysisDataService.cpp b/Code/Mantid/Framework/API/src/AnalysisDataService.cpp
@@ -64,6 +64,11 @@ namespace Mantid
     void AnalysisDataServiceImpl::add( const std::string& name, const boost::shared_ptr<API::Workspace>& workspace)
     {
       verifyName(name);
+      if ( doesExist(name) )
+      {
+          std::string error="ADS : Unable to add workspace : '"+name+"'";
+          throw std::runtime_error(error);
+      }
       //Attach the name to the workspace
       if( workspace ) workspace->setName(name);
       Kernel::DataService<API::Workspace>::add(name, workspace);
@@ -79,8 +84,8 @@ namespace Mantid
     {
       verifyName(name);
 
-      //Attach the name to the workspace
-      if( workspace ) workspace->setName(name);
+      //Attach the new name to the workspace
+      if( workspace ) workspace->setName(name,true);
       Kernel::DataService<API::Workspace>::addOrReplace(name, workspace);
     }
 
@@ -91,10 +96,10 @@ namespace Mantid
      */
     void AnalysisDataServiceImpl::rename( const std::string& oldName, const std::string& newName)
     {
+      auto ws = retrieve( oldName );
       Kernel::DataService<API::Workspace>::rename( oldName, newName );
       //Attach the new name to the workspace
-      auto ws = retrieve( newName );
-      ws->setName( newName );
+      ws->setName( newName, true );
     }
 
     /**
@@ -153,6 +158,7 @@ namespace Mantid
         {
             if ( (**it).getUpperCaseName() == foundName )
             {
+                (**it).setName("",true); // this call goes before remove(name) to work correctly with workspace groups
                 Kernel::DataService<Workspace>::remove( name );
                 return true;
             }
@@ -207,6 +213,26 @@ namespace Mantid
         return boost::shared_ptr<Workspace>();
     }
 
+    /**
+     * Print the names of all the workspaces in the ADS to the logger (at debug level)
+     *
+     */
+    void AnalysisDataServiceImpl::print() const
+    {
+        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();
+          std::cerr << (**it).name() << std::endl;
+          WorkspaceGroup* wsg = dynamic_cast<WorkspaceGroup*>(ws);
+          if ( wsg )
+          {
+              wsg->print("  ");
+          }
+        }
+    }
+
     //-------------------------------------------------------------------------
     // Private methods
     //-------------------------------------------------------------------------

diff --git a/Code/Mantid/Framework/API/src/Workspace.cpp b/Code/Mantid/Framework/API/src/Workspace.cpp
@@ -51,9 +51,15 @@ void Workspace::setComment(const std::string& c)
 /** Set the name field of the workspace
  *
  *  @param name :: The name
+ *  @param force :: If true the name must be set regardless.
  */
-void Workspace::setName(const std::string& name)
+void Workspace::setName(const std::string& name, bool force)
 {
+  if ( ! m_name.empty() && ! force && name != m_name )
+  {
+      // cannot use setName to rename a workspace in ADS
+      throw std::runtime_error("Cannot change name of workspace " + m_name);
+  }
   m_name = name;
   m_upperCaseName = name;
   std::transform(m_upperCaseName.begin(), m_upperCaseName.end(), m_upperCaseName.begin(),toupper);

diff --git a/Code/Mantid/Framework/API/src/WorkspaceGroup.cpp b/Code/Mantid/Framework/API/src/WorkspaceGroup.cpp
@@ -17,11 +17,11 @@ namespace API
 
 Kernel::Logger& WorkspaceGroup::g_log = Kernel::Logger::get("WorkspaceGroup");
 
-size_t WorkspaceGroup::g_maxNestingLevel = 100;
+size_t WorkspaceGroup::g_maxNestingLevel = 5;
 
 WorkspaceGroup::WorkspaceGroup(const bool observeADS) :
   Workspace(), 
-  m_replaceObserver(*this, &WorkspaceGroup::workspaceReplaceHandle),
+  //m_replaceObserver(*this, &WorkspaceGroup::workspaceReplaceHandle),
   m_workspaces(), m_observingADS(false), m_nameCounter(0)
 {
   observeADSNotifications(observeADS);
@@ -34,19 +34,51 @@ WorkspaceGroup::~WorkspaceGroup()
 
 /**
  * Override the base method to set names of member workspaces if they are empty.
+ * Ensures that member names are unique in the ADS.
+ *
  * @param name :: The new name.
+ * @param force :: If true the name must be set (to realise workspace replacement in the ADS)
  */
-void WorkspaceGroup::setName(const std::string &name)
+void WorkspaceGroup::setName(const std::string &name, bool force)
 {
-    Workspace::setName(name);
-    for (auto it = m_workspaces.begin(); it != m_workspaces.end(); ++it)
+    Workspace::setName(name, force);
+    // record workspaces received new names to roll back in case of exception
+    std::vector<bool> newNames(size(),false);
+    size_t i = 0;
+    for (auto it = m_workspaces.begin(); it != m_workspaces.end(); ++it,++i)
     {
         std::string wsName = (**it).name();
-        if ( wsName.empty() )
+        if ( wsName.empty() && ! name.empty() )
         {
             ++m_nameCounter;
             wsName = name + "_" + boost::lexical_cast<std::string>(m_nameCounter);
+            // check that this name is unique in the ADS
+            if ( AnalysisDataService::Instance().doesExist(wsName) )
+            {
+                auto ws = AnalysisDataService::Instance().retrieve( wsName );
+                if ( ! force && ws != *it )
+                {
+                    // name is not unique: unset new names and throw
+                    size_t n = static_cast<size_t>(it - m_workspaces.begin());
+                    for(size_t j = 0; j < n; ++j)
+                    {
+                        if ( newNames[j] ) m_workspaces[j]->setName("",true);
+                    }
+                    throw std::runtime_error( "Cannot set name of workspace group member: name " + wsName + " already exists." );
+                }
+            }
             (**it).setName(wsName);
+            newNames[i] = true;
+        }
+        else if ( name.empty() )
+        {
+            // setting empty name means workspace is being removed from the ADS.
+            // this relies on ADS calling this method before removing group's pointer from the storage
+            if ( AnalysisDataService::Instance().count(*it) <= 1 )
+            {
+                // set empty name only if this member is unique in the ADS
+                (**it).setName("",true);
+            }
         }
     }
 }
@@ -75,15 +107,15 @@ void WorkspaceGroup::observeADSNotifications(const bool observeADS)
   {
     if(!m_observingADS)
     {
-      AnalysisDataService::Instance().notificationCenter.addObserver(m_replaceObserver);
+      //AnalysisDataService::Instance().notificationCenter.addObserver(m_replaceObserver);
       m_observingADS = true;
     }
   }
   else
   {
     if(m_observingADS)
     {
-      AnalysisDataService::Instance().notificationCenter.removeObserver(m_replaceObserver);
+      //AnalysisDataService::Instance().notificationCenter.removeObserver(m_replaceObserver);
       m_observingADS = false;
     }
   }
@@ -96,6 +128,7 @@ 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;
 }
@@ -114,7 +147,7 @@ void WorkspaceGroup::addWorkspace(Workspace_sptr workspace)
     m_workspaces.push_back( workspace );
     if ( ! name().empty() )
     {
-        // checking for non-empty name - a faster way of chacking for being in the ADS
+        // checking for non-empty name - a faster way of checking for being in the ADS
         if ( workspace->name().empty() )
         {
             ++m_nameCounter;
@@ -123,9 +156,11 @@ void WorkspaceGroup::addWorkspace(Workspace_sptr workspace)
         }
         else
         {
+            std::string wsName = workspace->name();
             bool observing = m_observingADS;
             observeADSNotifications( false );
             AnalysisDataService::Instance().removeFromTopLevel( workspace->name() );
+            workspace->setName(wsName);
             observeADSNotifications( observing );
         }
     }
@@ -294,7 +329,9 @@ void WorkspaceGroup::removeAll()
         if ( ! ws->name().empty() )
         {
             // leave removed workspace in the ADS
-            AnalysisDataService::Instance().add( ws->name(), ws );
+            std::string wsName = ws->name();
+            ws->setName("",true);
+            AnalysisDataService::Instance().add( wsName, ws );
         }
     }
     m_workspaces.clear();
@@ -313,8 +350,8 @@ void WorkspaceGroup::remove(const std::string& wsName)
     {
       // leave removed workspace in the ADS
       auto ws = *it;
-      AnalysisDataService::Instance().add( ws->name(), ws );
       m_workspaces.erase(it);
+      AnalysisDataService::Instance().add( ws->name(), ws );
       break;
     }
   }
@@ -348,6 +385,7 @@ void WorkspaceGroup::deepRemove(const std::string &name, bool convertToUpperCase
     {
       if ( (**it).getUpperCaseName() == upperName )
       {
+        (**it).setName("",true);
         it = m_workspaces.erase(it);
       }
       else
@@ -392,6 +430,7 @@ void WorkspaceGroup::print(const std::string &padding) const
   for (auto itr = m_workspaces.begin(); itr != m_workspaces.end(); ++itr)
   {
     g_log.debug() << padding << (**itr).name() << std::endl;
+    std::cerr << padding << (**itr).name() << std::endl;
     const WorkspaceGroup *grp = dynamic_cast<const WorkspaceGroup*>( itr->get() );
     if ( grp )
     {
@@ -405,24 +444,24 @@ void WorkspaceGroup::print(const std::string &padding) const
  * 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 );
-}
+//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
@@ -431,7 +470,7 @@ void WorkspaceGroup::workspaceReplaceHandle(Mantid::API::WorkspaceBeforeReplaceN
 bool WorkspaceGroup::isEmpty() const
 {
   Poco::Mutex::ScopedLock _lock(m_mutex);
-	return m_workspaces.empty();
+    return m_workspaces.empty();
 }
 
 //------------------------------------------------------------------------------