Renaming a workspace in a group can cause a name clash

[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.

[Samuel Jackson]

Fixed bug with data service creating multiple workspace references

Refs #7807

Changeset: 4c63d9a6344c565b56ded1612a587de53fc44463

[Samuel Jackson]

Corrected minor spelling mistake.

Refs #7807

Changeset: 1a1a52d4a42bd11b426a33c25b6017c2c6506ead

[Samuel Jackson]

Added silent removal to workspace group replace handler.

Refs #7807

Changeset: e72a9c311e485df4b92b6fe4baaca55f0f8eb913

[Samuel Jackson]

Fixed issue with not removing duplicate workspace outside group.

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

Refs #7807

Changeset: 4f4abbc4262ccda516951a864f41ca642b77b520

[Samuel Jackson]

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.

[Peter Parker]

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]

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]

Only post replace notifications when a replace is happening.

Refs #7807

Changeset: 59c0aeeec624603074e25cedbc808c4d5ac1448e

[Samuel Jackson]

Updating WorkspaceGroup unit tests.

Also updated RenameWorkspace documentation with know issue.

Refs #7807

Changeset: e376da7973f81d67b7a29d458c31c854a7e3dbf7

[Peter Parker]

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]

Refs #7807 Fix for failing unit test

Changeset: 7c01f0c927bae4db4a8845481cd4903622a40c5f

[Karl Palmen]

KernelTest DataServiceTest crashes when run on a debug build.

[Martyn Gigg]

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]

As I'm leaving, I'm transferring ownership to Roman on Martyn's advice.

[Stuart Campbell]

This ticket has been transferred to github issue ​8652