Eclipse Community Forums: EMF » Transactional command stack non-redoable with command or wrapped operation trigger

Help

Home

Home » Modeling » EMF » Transactional command stack non-redoable with command or wrapped operation trigger

Show: Today's Messages :: Show Polls :: Message Navigator

Transactional command stack non-redoable with command or wrapped operation trigger [message #857458]

Thu, 26 April 2012 14:55

Adrian Price

Messages: 61
Registered: July 2009

Member

Good day eclipse.tools.emf,

There seems to be a problem whereby a transactional editing domain with
a resource set listener can be non-redoable via the command stack.

In my use case the listeners return EMFOperationCommands to perform what
are effectively non-EMF changes. (Actually they apply EMF model element
updates via commands executed on other, non-transactional editing
domains, but that has no bearing on the problem.)

However, execute - undo - redo fails at the redo step because:

- The transactional editing domain's command stack wraps the
trigger-provided Command in an EMFCommandOperation (which extends
AbstractEMFOperation) prior to executing it on the operation history.

- The EMFCommandOperation(AbstractEMFOperation).execute() calls
EMFCommandOperation.doExecute() which executes the wrapped trigger
Command; AbstractEMFOperation.execute() then obtains the
TransactionChangeDescription (a CommandChangeDescription) which it
stores in its private 'change' field.

- EMFCommandOperation.canUndo() calls super
AbstractEMFOperation.canUndo() which calls change.canApply(). Similarly,
EMFCommandOperation.canRedo() calls super AbstractEMFOperation.canRedo()
which calls change.canApply().

- AbstractEMFOperation.doUndo() and doRedo() are both implemented by
calling change.applyAndReverse(), thus toggling the change description
between undo and redo modes.

- HOWEVER, EMFCommandOperation.doUndo() overrides but *does not* call
super AbstractEMFOperation.doUndo(). Similarly,
EMFCommandOperation.doRedo() overrides but *does not* call
AbstractEMFOperation.doRedo(). Thus following command execution the
transaction change description is always left in its initial state that
does not accurately reflect the last undo or redo performed on the
owning operation. The CommandChangeDescription.isRedone field remains
set to true, so canApply() always calls command.canUndo().

- EMFCommandOperation.canRedo() calls super
AbstractEMFOperation.canRedo() which calls change.canApply() which calls
the wrapped command's canUndo() which naturally returns false,
preventing the redo from being performed.

I have attached a modified AbstractEMFOperationTest which illustrates
the problem. The
test_triggerCommands_CommandStack_nonEMF_OperationCommand test fails
against both org.eclipse.emf.workspace_1.2.0 and
org.eclipse.emf.workspace_1.3.0.

If someone could confirm that this is a genuine bug rather than user
error on my part, I would be most grateful indeed.

Thanks,

Adrian M. Price
Senior Architect
TIBCO Software Inc.

package org.eclipse.emf.workspace.tests;

import junit.framework.Test;
import junit.framework.TestSuite;

import org.eclipse.core.commands.ExecutionException;
import org.eclipse.core.commands.operations.AbstractOperation;
import org.eclipse.core.runtime.IAdaptable;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.IStatus;
import org.eclipse.core.runtime.Status;
import org.eclipse.emf.common.command.Command;
import org.eclipse.emf.common.command.CommandStack;
import org.eclipse.emf.common.notify.Notification;
import org.eclipse.emf.edit.command.AddCommand;
import org.eclipse.emf.edit.command.SetCommand;
import org.eclipse.emf.examples.extlibrary.EXTLibraryFactory;
import org.eclipse.emf.examples.extlibrary.EXTLibraryPackage;
import org.eclipse.emf.examples.extlibrary.Library;
import org.eclipse.emf.transaction.TransactionalEditingDomain;
import org.eclipse.emf.transaction.TriggerListener;
import org.eclipse.emf.workspace.AbstractEMFOperation;
import org.eclipse.emf.workspace.EMFOperationCommand;
import org.eclipse.emf.workspace.tests.fixtures.ExternalDataCommand;

/**
* Tests the {@link AbstractEMFOperation} framework. Extended to show operation trigger redo failure when executed through command
* stack.
* @author Christian W. Damus (cdamus)
* @author Adrian M. Price (TIBCO Software, Inc.)
*/
public class AbstractEMFOperationTestEx extends AbstractTest {
/**
* A test operation that performs changes on external data in the form of a string.
* @author Adrian Price (aprice)
*/
public class ExternalDataOperation extends AbstractOperation {
private boolean canExecute = true;
private String[] externalData;
private String oldValue;
private String newValue;

public ExternalDataOperation(String[] externalData, String newValue) {
super("Change External Data"); //$NON-NLS-1$

this.externalData = externalData;
this.newValue = newValue;
}

@Override
public boolean canExecute() {
return canExecute;
}

@Override
public boolean canRedo() {
return canExecute;
}

@Override
public boolean canUndo() {
return !canExecute;
}

@Override
public IStatus execute(IProgressMonitor monitor, IAdaptable info) throws ExecutionException {
// change the external (non-EMF) data
oldValue = externalData[0];
externalData[0] = newValue;
canExecute = false;
return Status.OK_STATUS;
}

@Override
public IStatus redo(IProgressMonitor monitor, IAdaptable info) throws ExecutionException {
externalData[0] = newValue;
canExecute = false;
return Status.OK_STATUS;
}

@Override
public IStatus undo(IProgressMonitor monitor, IAdaptable info) throws ExecutionException {
externalData[0] = oldValue;
canExecute = true;
return Status.OK_STATUS;
}
}

public static Test suite() {
return new TestSuite(AbstractEMFOperationTestEx.class, "EMF Operation Tests"); //$NON-NLS-1$
}

public AbstractEMFOperationTestEx(String name) {
super(name);
}

/**
* Tests that trigger commands are executed correctly when executing commands via the command stack, including undo and redo,
* where those triggers do non-EMF work.
*/
public void test_triggerCommands_CommandStack_nonEMF() {
final String[] externalData = new String[] { "..." }; //$NON-NLS-1$

// one trigger sets the external data
domain.addResourceSetListener(new TriggerListener() {
@Override
protected Command trigger(TransactionalEditingDomain domain, Notification notification) {
if (notification.getFeature() == EXTLibraryPackage.Literals.LIBRARY__NAME) {
return new ExternalDataCommand(externalData, notification.getNewStringValue());
}
return null;
}
});

final Library[] newLibrary = new Library[1];

newLibrary[0] = EXTLibraryFactory.eINSTANCE.createLibrary();
assertNull(newLibrary[0].getName());
assertTrue(newLibrary[0].getBranches().isEmpty());
Command cmd =
new AddCommand(domain, root.getBranches(), newLibrary[0]).chain(new SetCommand(domain, newLibrary[0],
EXTLibraryPackage.Literals.LIBRARY__NAME, "New Library")); //$NON-NLS-1$
CommandStack cmdStk = domain.getCommandStack();
cmdStk.execute(cmd);

startReading();

assertEquals("New Library", newLibrary[0].getName()); //$NON-NLS-1$
assertEquals("New Library", externalData[0]); //$NON-NLS-1$

commit();

assertTrue("Cannot undo", cmdStk.canUndo()); //$NON-NLS-1$
cmdStk.undo();

startReading();

// verify that the changes were undone
assertFalse(root.getBranches().contains(newLibrary[0]));
assertEquals("...", externalData[0]); //$NON-NLS-1$

commit();

assertTrue("Cannot redo", cmdStk.canRedo()); //$NON-NLS-1$
cmdStk.redo();

startReading();

// verify that the changes were redone
assertTrue(root.getBranches().contains(newLibrary[0]));
assertEquals("New Library", newLibrary[0].getName()); //$NON-NLS-1$
assertEquals("New Library", externalData[0]); //$NON-NLS-1$

commit();
}

/**
* Tests that trigger commands are executed correctly when executing commands via the command stack, including undo and redo,
* where those triggers do non-EMF work.
*/
public void test_triggerCommands_CommandStack_nonEMF_OperationCommand() {
final String[] externalData = new String[] { "..." }; //$NON-NLS-1$

// one trigger sets the external data
domain.addResourceSetListener(new TriggerListener() {
@Override
public boolean isAggregatePrecommitListener() {
return true;
}

@Override
protected Command trigger(TransactionalEditingDomain domain, Notification notification) {
if (notification.getFeature() == EXTLibraryPackage.Literals.LIBRARY__NAME) {
return new EMFOperationCommand(domain,
new ExternalDataOperation(externalData, notification.getNewStringValue()));
}

return null;
}
});

final Library[] newLibrary = new Library[1];

newLibrary[0] = EXTLibraryFactory.eINSTANCE.createLibrary();
assertNull(newLibrary[0].getName());
assertTrue(newLibrary[0].getBranches().isEmpty());
Command cmd =
new AddCommand(domain, root.getBranches(), newLibrary[0]).chain(new SetCommand(domain, newLibrary[0],
EXTLibraryPackage.Literals.LIBRARY__NAME, "New Library")); //$NON-NLS-1$
CommandStack cmdStk = domain.getCommandStack();
cmdStk.execute(cmd);

startReading();

assertEquals("New Library", newLibrary[0].getName()); //$NON-NLS-1$
assertEquals("New Library", externalData[0]); //$NON-NLS-1$

commit();

assertTrue("Cannot undo", cmdStk.canUndo()); //$NON-NLS-1$
cmdStk.undo();

startReading();

// verify that the changes were undone
assertFalse(root.getBranches().contains(newLibrary[0]));
assertEquals("...", externalData[0]); //$NON-NLS-1$

commit();

assertTrue("Cannot redo", cmdStk.canRedo()); //$NON-NLS-1$
cmdStk.redo();

startReading();

// verify that the changes were redone
assertTrue(root.getBranches().contains(newLibrary[0]));
assertEquals("New Library", newLibrary[0].getName()); //$NON-NLS-1$
assertEquals("New Library", externalData[0]); //$NON-NLS-1$

commit();
}
}

Attachment: AbstractEMFOperationTestEx.java
(Size: 8.08KB, Downloaded 129 times)

Report message to a moderator

Re: Transactional command stack non-redoable with command or wrapped operation trigger [message #857505 is a reply to message #857458]

Thu, 26 April 2012 15:44

Ed Merks

Messages: 33216
Registered: July 2009

Senior Member

Adrian,

I'd suggest opening a bugzilla. I don't personally have any significant
experience with the transaction editing domain framework, so hopefully
someone who does will have a look.

On 26/04/2012 4:55 PM, Adrian Price wrote:
> Good day eclipse.tools.emf,
>
> There seems to be a problem whereby a transactional editing domain
> with a resource set listener can be non-redoable via the command stack.
>
> In my use case the listeners return EMFOperationCommands to perform
> what are effectively non-EMF changes. (Actually they apply EMF model
> element updates via commands executed on other, non-transactional
> editing domains, but that has no bearing on the problem.)
>
> However, execute - undo - redo fails at the redo step because:
>
> - The transactional editing domain's command stack wraps the
> trigger-provided Command in an EMFCommandOperation (which extends
> AbstractEMFOperation) prior to executing it on the operation history.
>
> - The EMFCommandOperation(AbstractEMFOperation).execute() calls
> EMFCommandOperation.doExecute() which executes the wrapped trigger
> Command; AbstractEMFOperation.execute() then obtains the
> TransactionChangeDescription (a CommandChangeDescription) which it
> stores in its private 'change' field.
>
> - EMFCommandOperation.canUndo() calls super
> AbstractEMFOperation.canUndo() which calls change.canApply().
> Similarly, EMFCommandOperation.canRedo() calls super
> AbstractEMFOperation.canRedo() which calls change.canApply().
>
> - AbstractEMFOperation.doUndo() and doRedo() are both implemented by
> calling change.applyAndReverse(), thus toggling the change description
> between undo and redo modes.
>
> - HOWEVER, EMFCommandOperation.doUndo() overrides but *does not* call
> super AbstractEMFOperation.doUndo(). Similarly,
> EMFCommandOperation.doRedo() overrides but *does not* call
> AbstractEMFOperation.doRedo(). Thus following command execution the
> transaction change description is always left in its initial state
> that does not accurately reflect the last undo or redo performed on
> the owning operation. The CommandChangeDescription.isRedone field
> remains set to true, so canApply() always calls command.canUndo().
>
> - EMFCommandOperation.canRedo() calls super
> AbstractEMFOperation.canRedo() which calls change.canApply() which
> calls the wrapped command's canUndo() which naturally returns false,
> preventing the redo from being performed.
>
> I have attached a modified AbstractEMFOperationTest which illustrates
> the problem. The
> test_triggerCommands_CommandStack_nonEMF_OperationCommand test fails
> against both org.eclipse.emf.workspace_1.2.0 and
> org.eclipse.emf.workspace_1.3.0.
>
> If someone could confirm that this is a genuine bug rather than user
> error on my part, I would be most grateful indeed.
>
> Thanks,
>
> Adrian M. Price
> Senior Architect
> TIBCO Software Inc.

Ed Merks
Professional Support: https://www.macromodeling.com/

Report message to a moderator