Renaming a workspace in a group can cause a name clash

[mantid-builder]

This issue was originally TRAC 7807

Original Reporter: Samuel Jackson

If you rename two workspaces inside a workspace group to be the same thing you end up with two references to the same workspace. Also, deleting one does not remove the other reference.

[mantid-builder]

Samuel Jackson (2013-09-04T14:34:58):
Fixed bug with data service creating multiple workspace references

Refs http://trac.mantidproject.org/mantid/ticket/7807

4c63d9a

---

Samuel Jackson (2013-09-04T14:34:58):
Corrected minor spelling mistake.

Refs http://trac.mantidproject.org/mantid/ticket/7807

1a1a52d

---

Samuel Jackson (2013-09-04T14:34:58):
Added silent removal to workspace group replace handler.

Refs http://trac.mantidproject.org/mantid/ticket/7807

e72a9c3

---

Samuel Jackson (2013-09-04T14:34:59):
Fixed issue with not removing duplicate workspace outside group.

Also added rename notifications to data service.
Unit tests for workspace group also updated.

Refs http://trac.mantidproject.org/mantid/ticket/7807

4f4abbc

---

Samuel Jackson (2013-09-05T14:50:05):
'''To Tester'''

I would start by checking you can reproduce the problem. Load a few nexus files and group them. Then rename a child workspace to use the same name as another child workspace and verify that you get two pointers to the same workspace. Then check that this has been fixed in this branch.

I would also suggest checking the behaviour of the following cases:

• Rename a workspace not in a group to the same name as another workspace not in a group.
• Rename a child workspace to the same name as a workspace outside a group
• Rename a workspace not in a group to be the same as a child workspace.
• Rename a child of one group to the same name as a child in a different group.
  • This will cause a known edge case where we have two pointers to the same ws.
• Rename a group to be the same name as another group.
  • Note: This should destroy the original group and make all it's children top level workspaces.

And any other edge cases you may think of.

---

@PeterParker (2013-09-06T15:21:16):
Unfortunately, the changes for this ticket have meant that KernelTest_DataServiceTest fails on Windows (VS 2012) in Debug mode. Release mode is obviously fine. The failed assertion can be found at line 156 of DataServiceTest.h:

TSM_ASSERT_THROWS_NOTHING("Nothing should happen if the names match", svc.rename("One","One") );

The culprit is at line 301 of DataService.h:

notificationCenter.postNotification(new BeforeReplaceNotification(newName, it->second, object));

When it->second is called, "it" is not valid.

---

Also, now that the ticket has been reopened, has the expected functionality of the edge cases listed in comment 5 been decided to the extent where it can be "written in stone"? If so, unit tests to cover those cases would be particularly worthwhile in my opinion (given how much rests on GroupWorkspaces, and how often the surrounding code gets modified).

---

Samuel Jackson (2013-09-09T10:13:37):
The suggested cases were the agreed behaviour for the listed edge cases as discussed with Nick and Martyn based on what the system was currently doing (or expected to do) before this ticket. So I don't think it's unreasonable to get these written up as unit tests. They can always be changed if we decide on different behaviour later.

---

Samuel Jackson (2013-09-10T09:53:08):
Only post replace notifications when a replace is happening.

Refs http://trac.mantidproject.org/mantid/ticket/7807

59c0aee

---

Samuel Jackson (2013-09-10T09:53:08):
Updating WorkspaceGroup unit tests.

Also updated RenameWorkspace documentation with know issue.

Refs http://trac.mantidproject.org/mantid/ticket/7807

e376da7

---

@PeterParker (2013-09-13T15:13:52):
Visual Studio's Debug mode is being picky again and failing a unit test. As we discussed, the offending line is 322 of DataService.h:

if(it != datamap.end())

---

Samuel Jackson (2013-09-17T08:27:40):
Refs http://trac.mantidproject.org/mantid/ticket/7807 Fix for failing unit test

7c01f0c

---

@KarlPalmen (2013-09-20T14:00:02):
KernelTest DataServiceTest crashes when run on a debug build.

---

@martyngigg (2013-09-21T10:48:21):
I know we've discussed this before but I'm having seconds thoughts now about rename being anything more than a simple delete then add. The original use case for it being different is gone now that WorkspaceGroups hold their pointers and the GUI works entirely differently to how it used to.

Can you hold off doing anything to this until we can talk about it more. I want to get Roman's thoughts on this too as he put the rename in in the first place.

---

Samuel Jackson (2014-08-11T07:29:21):
As I'm leaving, I'm transferring ownership to Roman on Martyn's advice.

[NickDraper]

This does still happen in the world of GroupWorkspaces holding pointers.

Perhaps Rename should refuse if a workspace of that name is in the ADS (or in a Group in the ADS).

[NickDraper]

Rewritten into #16263

Therefore this is a duplicate