Description Pål Stenslet 2008-02-08 02:52:16 EST

Build ID: M20071023-1652

Steps To Reproduce:
This has been reproduced using the org.eclipse.emf.transaction.exapmle plugin (the emf-transaction-examples-1.1.1.zip download).

The example was modified as follows (code details are given below):
(1) Use a RecordingCommand when changing the last name of a writer
(2) Add a ResourceSetListener that triggers a RecordingCommand to modify the title of the writer's first book, whenever the last name of the writer has changed

When editing the writers last name in the properties view, the title of the first book is updated accordingly.

However, when undoing the operation, the RecordingCommand that updates the book's title receives the undo() message twice. The second execution of undo effectively redoes the command. Hence the book's title does not appear to be affected by the undo.

---Code details---

(1) The ResourceSetListener:

public class MyResourceSetListener extends ResourceSetListenerImpl {
   public Command transactionAboutToCommit(ResourceSetChangeEvent event)
      throws RollbackException {

      List notifications = event.getNotifications();
      CompoundCommand cc = new CompoundCommand();
      Set handledWriters = new HashSet();

      for (Iterator i = notifications.iterator(); i.hasNext();) {
	 Notification n = (Notification)i.next();

	 if (n.getNotifier() instanceof EObject && n.getFeature() == EXTLibraryPackage.eINSTANCE.getPerson_LastName() && !handledWriters.contains(n)) {
	    EObject notifier = (EObject)n.getNotifier();
	    if (notifier.eClass() == EXTLibraryPackage.eINSTANCE.getWriter()) {
	       final Writer w = (Writer)notifier;

	       if (!w.getBooks().isEmpty()) {
		   final Book firstBook = (Book)w.getBooks().get(0);
		   if (!firstBook.getTitle().startsWith(w.getLastName())) {
		   // We can use any EMF command here
			  cc.append(new RecordingCommand(event.getEditingDomain()) {
				  public void doExecute() {
					  firstBook.setTitle(w.getLastName() + ":" + firstBook.getTitle());
				  }
			  }
			  );
		   }
	       }

	       handledWriters.add(w);
	    }
	 }
      }

      // It is important to return null if we have nothing to
      // contribute to this transaction.
      return cc.isEmpty() ? null : cc;
   }
}


(2) Adding the ResourceSetListener in EXTLibraryEditingDomainFactory.createEditingDomain():

public TransactionalEditingDomain createEditingDomain() {
	// create an editing domain with a default resource set implementation
	//    and delegating command execution to the default (workbench)
	//    operation history
	TransactionalEditingDomain result = WorkspaceEditingDomainFactory.INSTANCE.createEditingDomain();
	//TransactionalEditingDomain result = TransactionalEditingDomain.Factory.INSTANCE.createEditingDomain();

	// add an exception handler to the editing domain's command stack
	((TransactionalCommandStack) result.getCommandStack()).setExceptionHandler(
			new CommandStackExceptionHandler());

	result.addResourceSetListener(new MyResourceSetListener());

	return result;
}


(3) To force the use of a RecordingCommand to set Writer lastName, a private ItemPropertyDescriptor subclass is added in PersonItemProvider:

private class LastNamePropertyDescriptor extends ItemPropertyDescriptor 
  {

      public LastNamePropertyDescriptor
              (AdapterFactory adapterFactory,
               ResourceLocator resourceLocator,
               String displayName,
               String description,
               EStructuralFeature feature,
               boolean isSettable,
               boolean multiLine,
               boolean sortChoices,
               Object staticImage,
               String category,
               String[] filterFlags) {
          super(adapterFactory,
                resourceLocator,
                displayName,
                description,
                feature,
                isSettable,
                multiLine,
                sortChoices,
                staticImage,
                category,
                filterFlags);
      }

      public Object getPropertyValue(Object object) {
          return ((Person)object).getLastName();
      }

      public void setPropertyValue(Object object, Object value) {
    	  final Person rc = (Person) object;
          final TransactionalEditingDomain editingDomain = (TransactionalEditingDomain) getEditingDomain(object);
          final String newName = (String)value;
          List<Command> cmdList = new ArrayList<Command>();
          cmdList.add(new RecordingCommand(editingDomain) {
        	  public void doExecute() {
        		  rc.setLastName(newName);
        	  }
          });
          editingDomain.getCommandStack().execute(new CompoundCommand(cmdList));
      }

      public void resetPropertyValue(Object object) {
          super.resetPropertyValue(object);
      }
  }
  
  
(4) To use LastNamePropertyDescriptor, the PersonItemProvider.addLastNamePropertyDescriptor() is modified as follows:

  /**
   * This adds a property descriptor for the Last Name feature.
   * <!-- begin-user-doc -->
   * <!-- end-user-doc -->
   * @generated NOT
   */
  protected void addLastNamePropertyDescriptor(Object object)
  {
    itemPropertyDescriptors.add
      //(createItemPropertyDescriptor
      (new LastNamePropertyDescriptor
        (((ComposeableAdapterFactory)adapterFactory).getRootAdapterFactory(),
         getResourceLocator(),
         getString("_UI_Person_lastName_feature"), //$NON-NLS-1$
         getString("_UI_PropertyDescriptor_description", "_UI_Person_lastName_feature", "_UI_Person_type"), //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
         EXTLibraryPackage.Literals.PERSON__LAST_NAME,
         true,
         false,
         false,
         ItemPropertyDescriptor.GENERIC_VALUE_IMAGE,
         null,
         null));
  }

More information:
The first run of RecordingCommand.undo() is done as a consequence of an explicit attempt to undo the triggerCommand in EMFCommandOperation.doUndo().

The second RecordingCommand.undo() is done in accordance with a CommandChangeDescription that is recorded in the transaction by the TransactionImpl.addTriggers() method.

If I replace the RecordingCommand with a SetCommand in the ResourceSetListener, I have verified that undo is still run twice on the SetCommand. However, since the SetCommand keeps track of its own oldValue, the second run of undo does not change anything.

One simple fix could be to make RecordingCommand more robust to inconsistent use of undo/redo: Instead of merely running applyAndReverse() in respose to both undo/redo, it should check to see what the most recent action was. If an attempt is made to repeat the most recent action, the RecordingCommand should do nothing.