Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » EMF » [CDO][EMF Command] problems with SetCommand.redo from empty List(Problems with redo of SetCommand on CDO model when current value is empty List)
[CDO][EMF Command] problems with SetCommand.redo from empty List [message #1001717] Thu, 17 January 2013 09:16 Go to next message
Mads Sørhaug is currently offline Mads Sørhaug
Messages: 1
Registered: July 2012
Junior Member
I'm using EMF Commands to enable undo/redo of modifications on a CDO model, and am now experiencing an issue with redoing SetCommand when it has set an empty List.

My test case:
@Test
public void setCommand_emptyListInHistory_isNotRedoable(){
  assertEquals(ImmutableList.of(createIntervall(10, 20)), selectionLine.getEmployees());
  SetCommand setCommand = new SetCommand(editDomain, selectionLine, SELECTION_EMPLOYEES, new BasicEList<Intervall>());
  commandStack.execute(setCommand);
  assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
  SetCommand setCommand2 = new SetCommand(editDomain, selectionLine, SELECTION_EMPLOYEES, ImmutableList.of(createIntervall(20, 30)));
  commandStack.execute(setCommand2);
  assertEquals(ImmutableList.of(createIntervall(20, 30)),selectionLine.getEmployees());
  commandStack.undo();
  assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
  commandStack.undo();
  assertEquals(ImmutableList.of(createIntervall(10, 20)), selectionLine.getEmployees());
  commandStack.redo();
  assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());

  commandStack.redo();

  //This is what happens
  assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());

  //This is what I expected
  //assertEquals(ImmutableList.of(createIntervall(20, 30)),selectionLine.getEmployees());
}

Basically the first redo after an empty List has been set, it is not able to execute correctly.

What seems to be happening is that SetCommand.prepare does
if(owner.eIsSet(feature)) oldValue = UNSET_VALUE
, since eIsSet is false for emptyList, then doUndo() does
if(oldValue == UNSET_VALUE) owner.eUnset(feature);
however doRedo goes into else in
if (value == UNSET_VALUE){owner.eUnset(feature);} else {owner.eSet(feature, value);}
as I originally expected, but doesn't set anything when redoing to the second (non empty) value.

Does anyone have any experience with this issue, or is there something obvious I'm doing wrong?

I can add that if I add the following into doUndo (before else if (oldValue == UNSET_VALUE)) of an overridden SetCommand it works:
else if (oldValue == UNSET_VALUE && feature.isMany())
{
  //Hack to get setting of list with empty List working with redo
  oldValue = new BasicEList<Object>();
  owner.eSet(feature, oldValue);
}
Re: [CDO][EMF Command] problems with SetCommand.redo from empty List [message #1003085 is a reply to message #1001717] Sun, 20 January 2013 07:20 Go to previous message
Ed Merks is currently offline Ed Merks
Messages: 26062
Registered: July 2009
Senior Member
Mads,

Comments below.

On 17/01/2013 2:44 PM, Mads Sørhaug wrote:
> I'm using EMF Commands to enable undo/redo of modifications on a CDO
> model,
Is it a CDO-specific problem?
> and am now experiencing an issue with redoing SetCommand when it has
> set an empty List.
>
> My test case:
>
> @Test
> public void setCommand_emptyListInHistory_isNotRedoable(){
> assertEquals(ImmutableList.of(createIntervall(10, 20)),
> selectionLine.getEmployees());
> SetCommand setCommand = new SetCommand(editDomain, selectionLine,
> SELECTION_EMPLOYEES, new BasicEList<Intervall>());
> commandStack.execute(setCommand);
> assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
> SetCommand setCommand2 = new SetCommand(editDomain, selectionLine,
> SELECTION_EMPLOYEES, ImmutableList.of(createIntervall(20, 30)));
> commandStack.execute(setCommand2);
> assertEquals(ImmutableList.of(createIntervall(20,
> 30)),selectionLine.getEmployees());
> commandStack.undo();
> assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
> commandStack.undo();
> assertEquals(ImmutableList.of(createIntervall(10, 20)),
> selectionLine.getEmployees());
> commandStack.redo();
> assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
>
> commandStack.redo();
>
> //This is what happens
> assertEquals(new BasicEList<Intervall>(),selectionLine.getEmployees());
>
> //This is what I expected
> //assertEquals(ImmutableList.of(createIntervall(20,
> 30)),selectionLine.getEmployees());
> }
>
> Basically the first redo after an empty List has been set, it is not
> able to execute correctly.
>
> What seems to be happening is that SetCommand.prepare does
> if(owner.eIsSet(feature)) oldValue = UNSET_VALUE, since eIsSet is
> false for emptyList, then doUndo() does if(oldValue == UNSET_VALUE)
> owner.eUnset(feature); however doRedo goes into else in if (value ==
> UNSET_VALUE){owner.eUnset(feature);} else {owner.eSet(feature,
> value);} as I originally expected, but doesn't set anything when
> redoing to the second (non empty) value.
So it sounds like the command is doing the right thing... It should
have a copy of the old value it (which you say it does) so eSet should
result in that value ending up being the the contents of the list
owner's list for that feature, but you say with CDO it doesn't...
>
> Does anyone have any experience with this issue, or is there something
> obvious I'm doing wrong?
It looks correct what you're doing.
>
> I can add that if I add the following into doUndo (before else if
> (oldValue == UNSET_VALUE)) of an overridden SetCommand it works:
>
> else if (oldValue == UNSET_VALUE && feature.isMany())
> {
> //Hack to get setting of list with empty List working with redo
> oldValue = new BasicEList<Object>();
> owner.eSet(feature, oldValue);
> }
It certainly sounds like something is wrong. If it's reproducible
without CDO then it's something wrong in SetCommand, otherwise something
wrong with CDO.

Could you create a self contained test case that doesn't depend on a CDO
generated model and open a bugzilla (assuming it's reproducible that
way)? Then I'll have a close look and fix whatever the problem is.

I should note that your workaround doesn't look exactly correct: even
multi-valued features can be unsettable, so it makes a difference to the
final state (for unsettable lists) to call eUnset verses calling eSet to
an empty list.
Previous Topic:[XCore] Altering generated code
Next Topic:[Teneo] One-To-One Mappings Ignoring Column Names
Goto Forum:
  


Current Time: Mon Sep 22 18:32:39 GMT 2014

Powered by FUDForum. Page generated in 0.01592 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software