Skip to main content


Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Modeling » EMF » RecordingCommand causes headache
RecordingCommand causes headache [message #423781] Tue, 07 October 2008 09:42 Go to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Hi!

EMF Transaction's RecordingCommand is both a blessing and a curse!
It greatly simplifies model change operations - but mixing
RecordingCommands with "normal" Commands is driving me crazy (especially
concerning undo/redo)!
(See also thread "CompoundCommand containing multiple RecordingCommands
does not undo correctly")

At the moment our rule of thumb is: Don't use RecordingCommands inside
CompoundCommands; only one top-level RecordingCommand is allowed.

This works fine, but now I've implemented a pre-commit trigger that
returns an AddCommand. On undo, the RecordingCommand undos the
AddCommand's changes too; as a result, the undo of the AddCommand fails.

How can we solve this problem? Is there a way to restrict the
RecordingCommand to its own changes only? What about nested
transactions? I thought, trigger-commands are executed in its own
transaction, so why are their changes undone by the "main"
(Recording)command?

Thanks in advance,
Mario
Re: RecordingCommand causes headache [message #423797 is a reply to message #423781] Tue, 07 October 2008 11:03 Go to previous messageGo to next message
Ed Merks is currently offline Ed MerksFriend
Messages: 33142
Registered: July 2009
Senior Member
Mario,

Comments below.

Mario Winterer wrote:
> Hi!
>
> EMF Transaction's RecordingCommand is both a blessing and a curse!
Call an exorcist! :-P
>
> It greatly simplifies model change operations - but mixing
> RecordingCommands with "normal" Commands is driving me crazy
> (especially concerning undo/redo)!
> (See also thread "CompoundCommand containing multiple
> RecordingCommands does not undo correctly")
I'll bet the two get along poorly at best.
>
> At the moment our rule of thumb is: Don't use RecordingCommands inside
> CompoundCommands; only one top-level RecordingCommand is allowed.
I think RecordingCommand and ChangeCommand are a bit different.
ChangeCommand attaches the ChangeRecorder during the doExecute call and
then detaches it. That's a relatively expensive operation, but ensures
that only change made during the doExecute are recorded, and no others...
>
> This works fine, but now I've implemented a pre-commit trigger that
> returns an AddCommand. On undo, the RecordingCommand undos the
> AddCommand's changes too; as a result, the undo of the AddCommand fails.
I wonder if a ChangeCommand would work better for this kind of mixing...
>
> How can we solve this problem? Is there a way to restrict the
> RecordingCommand to its own changes only? What about nested
> transactions? I thought, trigger-commands are executed in its own
> transaction, so why are their changes undone by the "main"
> (Recording)command?
No doubt Christian will have some insights later...
>
> Thanks in advance,
> Mario


Ed Merks
Professional Support: https://www.macromodeling.com/
Re: RecordingCommand causes headache [message #423808 is a reply to message #423797] Tue, 07 October 2008 11:59 Go to previous messageGo to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Ed Merks schrieb:
> Mario,
>
> Comments below.
>
> Mario Winterer wrote:
>> Hi!
>>
>> EMF Transaction's RecordingCommand is both a blessing and a curse!
> Call an exorcist! :-P

Well - I've already got the EMF bible...
What's your and Christian's phone numbers... ;-)

>>
>> It greatly simplifies model change operations - but mixing
>> RecordingCommands with "normal" Commands is driving me crazy
>> (especially concerning undo/redo)!
>> (See also thread "CompoundCommand containing multiple
>> RecordingCommands does not undo correctly")
> I'll bet the two get along poorly at best.
>>
>> At the moment our rule of thumb is: Don't use RecordingCommands inside
>> CompoundCommands; only one top-level RecordingCommand is allowed.
> I think RecordingCommand and ChangeCommand are a bit different.
> ChangeCommand attaches the ChangeRecorder during the doExecute call and
> then detaches it. That's a relatively expensive operation, but ensures
> that only change made during the doExecute are recorded, and no others...

Yes, I've already though of ChangeCommand - we'll do some tests
concerning performance and memory overhead.

>>
>> This works fine, but now I've implemented a pre-commit trigger that
>> returns an AddCommand. On undo, the RecordingCommand undos the
>> AddCommand's changes too; as a result, the undo of the AddCommand fails.
> I wonder if a ChangeCommand would work better for this kind of mixing...

You might be right, but using a ChangeCommand instead of the (much
cheaper) RecordingCommand just for the case there might be a pre-commit
trigger...

>>
>> How can we solve this problem? Is there a way to restrict the
>> RecordingCommand to its own changes only? What about nested
>> transactions? I thought, trigger-commands are executed in its own
>> transaction, so why are their changes undone by the "main"
>> (Recording)command?
> No doubt Christian will have some insights later...
>>
>> Thanks in advance,
>> Mario
Re: RecordingCommand causes headache [message #423810 is a reply to message #423808] Tue, 07 October 2008 12:06 Go to previous messageGo to next message
Ed Merks is currently offline Ed MerksFriend
Messages: 33142
Registered: July 2009
Senior Member
Mario,

Comments below.


Mario Winterer wrote:
> Ed Merks schrieb:
>> Mario,
>>
>> Comments below.
>>
>> Mario Winterer wrote:
>>> Hi!
>>>
>>> EMF Transaction's RecordingCommand is both a blessing and a curse!
>> Call an exorcist! :-P
>
> Well - I've already got the EMF bible...
> What's your and Christian's phone numbers... ;-)
First you have to write a large check... :-P I do monitor IRC's
#eclipse-modeling...
>
>>>
>>> It greatly simplifies model change operations - but mixing
>>> RecordingCommands with "normal" Commands is driving me crazy
>>> (especially concerning undo/redo)!
>>> (See also thread "CompoundCommand containing multiple
>>> RecordingCommands does not undo correctly")
>> I'll bet the two get along poorly at best.
>>>
>>> At the moment our rule of thumb is: Don't use RecordingCommands
>>> inside CompoundCommands; only one top-level RecordingCommand is
>>> allowed.
>> I think RecordingCommand and ChangeCommand are a bit different.
>> ChangeCommand attaches the ChangeRecorder during the doExecute call
>> and then detaches it. That's a relatively expensive operation, but
>> ensures that only change made during the doExecute are recorded, and
>> no others...
>
> Yes, I've already though of ChangeCommand - we'll do some tests
> concerning performance and memory overhead.
Note that it attaches itself only to the notifiers, so that can be used
to limit the scope...
>
>>>
>>> This works fine, but now I've implemented a pre-commit trigger that
>>> returns an AddCommand. On undo, the RecordingCommand undos the
>>> AddCommand's changes too; as a result, the undo of the AddCommand
>>> fails.
>> I wonder if a ChangeCommand would work better for this kind of mixing...
>
> You might be right, but using a ChangeCommand instead of the (much
> cheaper) RecordingCommand just for the case there might be a
> pre-commit trigger...
Measuring is good...
>
>>>
>>> How can we solve this problem? Is there a way to restrict the
>>> RecordingCommand to its own changes only? What about nested
>>> transactions? I thought, trigger-commands are executed in its own
>>> transaction, so why are their changes undone by the "main"
>>> (Recording)command?
>> No doubt Christian will have some insights later...
>>>
>>> Thanks in advance,
>>> Mario


Ed Merks
Professional Support: https://www.macromodeling.com/
Re: RecordingCommand causes headache [message #423851 is a reply to message #423781] Wed, 08 October 2008 00:32 Go to previous messageGo to next message
Eclipse UserFriend
Originally posted by: cdamus.zeligsoft.com

Hi, Mario,

You should be able to use a RecordingCommand as a Trigger on a regular
Command, and a regular Command as a Trigger on a RecordingCommand.
After all, how can the trigger know what kind of command was executing?

The TransactionalCommandStack is supposed to ensure that the nested
transaction in which triggers are run isolates all of the changes
appropriately.

If you can isolate your problem to some kind of repeatable test case
that you can share with us, a bugzilla report would definitely be in order.

Cheers,

Christian


Mario Winterer wrote:
> Hi!
>
> EMF Transaction's RecordingCommand is both a blessing and a curse!
> It greatly simplifies model change operations - but mixing
> RecordingCommands with "normal" Commands is driving me crazy (especially
> concerning undo/redo)!
> (See also thread "CompoundCommand containing multiple RecordingCommands
> does not undo correctly")
>
> At the moment our rule of thumb is: Don't use RecordingCommands inside
> CompoundCommands; only one top-level RecordingCommand is allowed.
>
> This works fine, but now I've implemented a pre-commit trigger that
> returns an AddCommand. On undo, the RecordingCommand undos the
> AddCommand's changes too; as a result, the undo of the AddCommand fails.
>
> How can we solve this problem? Is there a way to restrict the
> RecordingCommand to its own changes only? What about nested
> transactions? I thought, trigger-commands are executed in its own
> transaction, so why are their changes undone by the "main"
> (Recording)command?
>
> Thanks in advance,
> Mario
Re: RecordingCommand causes headache [message #423855 is a reply to message #423851] Wed, 08 October 2008 07:46 Go to previous messageGo to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Hi Christian!

Thanks for your reply.
After looking into the code again, I've found out that I was wrong. Our
"top command" is no RecordingCommand but a DragAndDropCommand that
internally holds a RecordingCommand as drop command.

Now on undo the RecordingCommand is undone, and because it holds a
reference to the main transaction, the entire transaction including the
changes caused by the triggers are undone. So the triggers are undone
twice and an exception is thrown (AddCommands cannot be undone twice...).

After wrapping the DragAndDropCommand in a RecordingCommand, everything
works fine (I think because of the special handling of RecordingCommands
in the wrapping EMFCommandOperation's didCommit() method).

Nevertheless I'm wondering if the RecordingCommand should behave that
way, because that behaviour prevents RecordingCommands to be chained or
used in a CompoundCommand.
In addition I think it is a bug to use the RecordingCommand's chain
method - maybe this method should not exist or at least throw an
UnsupportedOperationException.

Thanks,
Mario

Christian W. Damus schrieb:
> Hi, Mario,
>
> You should be able to use a RecordingCommand as a Trigger on a regular
> Command, and a regular Command as a Trigger on a RecordingCommand. After
> all, how can the trigger know what kind of command was executing?
>
> The TransactionalCommandStack is supposed to ensure that the nested
> transaction in which triggers are run isolates all of the changes
> appropriately.
>
> If you can isolate your problem to some kind of repeatable test case
> that you can share with us, a bugzilla report would definitely be in order.
>
> Cheers,
>
> Christian
>
>
> Mario Winterer wrote:
>> Hi!
>>
>> EMF Transaction's RecordingCommand is both a blessing and a curse!
>> It greatly simplifies model change operations - but mixing
>> RecordingCommands with "normal" Commands is driving me crazy
>> (especially concerning undo/redo)!
>> (See also thread "CompoundCommand containing multiple
>> RecordingCommands does not undo correctly")
>>
>> At the moment our rule of thumb is: Don't use RecordingCommands inside
>> CompoundCommands; only one top-level RecordingCommand is allowed.
>>
>> This works fine, but now I've implemented a pre-commit trigger that
>> returns an AddCommand. On undo, the RecordingCommand undos the
>> AddCommand's changes too; as a result, the undo of the AddCommand fails.
>>
>> How can we solve this problem? Is there a way to restrict the
>> RecordingCommand to its own changes only? What about nested
>> transactions? I thought, trigger-commands are executed in its own
>> transaction, so why are their changes undone by the "main"
>> (Recording)command?
>>
>> Thanks in advance,
>> Mario
Re: RecordingCommand causes headache [message #423858 is a reply to message #423855] Wed, 08 October 2008 09:37 Go to previous messageGo to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
I've written a unit test to show the danger of RecordingCommand.chain().
The test is based on the extended library model example and tries to
undo a chained RecordingCommand in combination with a trigger. (The test
adds a book to the library; the trigger sets the author of the book)

public void testUndoAddBookWithTriggerChainedRecordingCommand() {
editingDomain.addResourceSetListener(new
ResourceSetListenerImpl() {
public Command transactionAboutToCommit(
ResourceSetChangeEvent event
) {
// set the author
return SetCommand.create(editingDomain, book,
EXTLibraryPackage.Literals.BOOK__AUTHOR,
author
);
}
});

Command command = new RecordingCommand(editingDomain) {
protected void doExecute() {
library.getBooks().add(book);
}
};

// SELECT ONE OF THE FOLLOWING TWO VERSIONS
// VERSION 1: THIS CAUSES THE LAST ASSERTION TO FAIL
editingDomain.getCommandStack().execute(
command.chain(IdentityCommand.INSTANCE));

// VERSION 2: USE THIS LINE INSTEAD OF THE ONE ABOVE
// TO MAKE THINKS WORK!
// editingDomain.getCommandStack().execute(command);
// END VERSIONS

assertTrue(library.getBooks().contains(book));
assertEquals(author, book.getAuthor());

editingDomain.getCommandStack().undo();

assertFalse(library.getBooks().contains(book));
assertEquals(null, book.getAuthor());
}

When the RecordingCommand is chained with any other command, the
resulting command is no RecordingCommand any more. As a result, in the
test above, first the author is unset (due to undoing the trigger
command) but then re-set again (due to applyAndReverse of the
RecordingCommand).

(Please tell me if you want to have the entire code of the unit test).

Mario

Mario Winterer schrieb:

> Hi Christian!
>
> Thanks for your reply.
> After looking into the code again, I've found out that I was wrong. Our
> "top command" is no RecordingCommand but a DragAndDropCommand that
> internally holds a RecordingCommand as drop command.
>
> Now on undo the RecordingCommand is undone, and because it holds a
> reference to the main transaction, the entire transaction including the
> changes caused by the triggers are undone. So the triggers are undone
> twice and an exception is thrown (AddCommands cannot be undone twice...).
>
> After wrapping the DragAndDropCommand in a RecordingCommand, everything
> works fine (I think because of the special handling of RecordingCommands
> in the wrapping EMFCommandOperation's didCommit() method).
>
> Nevertheless I'm wondering if the RecordingCommand should behave that
> way, because that behaviour prevents RecordingCommands to be chained or
> used in a CompoundCommand.
> In addition I think it is a bug to use the RecordingCommand's chain
> method - maybe this method should not exist or at least throw an
> UnsupportedOperationException.
>
> Thanks,
> Mario
>
> Christian W. Damus schrieb:
>> Hi, Mario,
>>
>> You should be able to use a RecordingCommand as a Trigger on a regular
>> Command, and a regular Command as a Trigger on a RecordingCommand.
>> After all, how can the trigger know what kind of command was executing?
>>
>> The TransactionalCommandStack is supposed to ensure that the nested
>> transaction in which triggers are run isolates all of the changes
>> appropriately.
>>
>> If you can isolate your problem to some kind of repeatable test case
>> that you can share with us, a bugzilla report would definitely be in
>> order.
>>
>> Cheers,
>>
>> Christian
>>
>>
>> Mario Winterer wrote:
>>> Hi!
>>>
>>> EMF Transaction's RecordingCommand is both a blessing and a curse!
>>> It greatly simplifies model change operations - but mixing
>>> RecordingCommands with "normal" Commands is driving me crazy
>>> (especially concerning undo/redo)!
>>> (See also thread "CompoundCommand containing multiple
>>> RecordingCommands does not undo correctly")
>>>
>>> At the moment our rule of thumb is: Don't use RecordingCommands
>>> inside CompoundCommands; only one top-level RecordingCommand is allowed.
>>>
>>> This works fine, but now I've implemented a pre-commit trigger that
>>> returns an AddCommand. On undo, the RecordingCommand undos the
>>> AddCommand's changes too; as a result, the undo of the AddCommand fails.
>>>
>>> How can we solve this problem? Is there a way to restrict the
>>> RecordingCommand to its own changes only? What about nested
>>> transactions? I thought, trigger-commands are executed in its own
>>> transaction, so why are their changes undone by the "main"
>>> (Recording)command?
>>>
>>> Thanks in advance,
>>> Mario
Re: RecordingCommand causes headache [message #423873 is a reply to message #423858] Wed, 08 October 2008 13:11 Go to previous messageGo to next message
Eclipse UserFriend
Originally posted by: cdamus.zeligsoft.com

I, Mario,

Yes, this JUnit test looks like a good Bugzilla contribution. :-)

I have already fixed one bug in the undo/redo of RecordingCommands that
are used as triggers, but I think more work is needed. Probably, the
RecordingCommand will need to be able to create its own nested
transaction, as the AbstractEMFOperation can do, in the appropriate
circumstances.

Cheers,

Christian

Mario Winterer wrote:
> I've written a unit test to show the danger of RecordingCommand.chain().
> The test is based on the extended library model example and tries to
> undo a chained RecordingCommand in combination with a trigger. (The test
> adds a book to the library; the trigger sets the author of the book)
>
> public void testUndoAddBookWithTriggerChainedRecordingCommand() {
> editingDomain.addResourceSetListener(new
> ResourceSetListenerImpl() {
> public Command transactionAboutToCommit(
> ResourceSetChangeEvent event
> ) {
> // set the author
> return SetCommand.create(editingDomain, book,
> EXTLibraryPackage.Literals.BOOK__AUTHOR,
> author
> );
> }
> });
>
> Command command = new RecordingCommand(editingDomain) {
> protected void doExecute() {
> library.getBooks().add(book);
> }
> };
>
> // SELECT ONE OF THE FOLLOWING TWO VERSIONS
> // VERSION 1: THIS CAUSES THE LAST ASSERTION TO FAIL
> editingDomain.getCommandStack().execute(
> command.chain(IdentityCommand.INSTANCE));
>
> // VERSION 2: USE THIS LINE INSTEAD OF THE ONE ABOVE
> // TO MAKE THINKS WORK!
> // editingDomain.getCommandStack().execute(command);
> // END VERSIONS
>
> assertTrue(library.getBooks().contains(book));
> assertEquals(author, book.getAuthor());
>
> editingDomain.getCommandStack().undo();
>
> assertFalse(library.getBooks().contains(book));
> assertEquals(null, book.getAuthor());
> }
>
> When the RecordingCommand is chained with any other command, the
> resulting command is no RecordingCommand any more. As a result, in the
> test above, first the author is unset (due to undoing the trigger
> command) but then re-set again (due to applyAndReverse of the
> RecordingCommand).
>
> (Please tell me if you want to have the entire code of the unit test).
>
> Mario
>
> Mario Winterer schrieb:
>
>> Hi Christian!
>>
>> Thanks for your reply.
>> After looking into the code again, I've found out that I was wrong.
>> Our "top command" is no RecordingCommand but a DragAndDropCommand that
>> internally holds a RecordingCommand as drop command.
>>
>> Now on undo the RecordingCommand is undone, and because it holds a
>> reference to the main transaction, the entire transaction including
>> the changes caused by the triggers are undone. So the triggers are
>> undone twice and an exception is thrown (AddCommands cannot be undone
>> twice...).
>>
>> After wrapping the DragAndDropCommand in a RecordingCommand,
>> everything works fine (I think because of the special handling of
>> RecordingCommands in the wrapping EMFCommandOperation's didCommit()
>> method).
>>
>> Nevertheless I'm wondering if the RecordingCommand should behave that
>> way, because that behaviour prevents RecordingCommands to be chained
>> or used in a CompoundCommand.
>> In addition I think it is a bug to use the RecordingCommand's chain
>> method - maybe this method should not exist or at least throw an
>> UnsupportedOperationException.
>>
>> Thanks,
>> Mario
>>
>> Christian W. Damus schrieb:
>>> Hi, Mario,
>>>
>>> You should be able to use a RecordingCommand as a Trigger on a
>>> regular Command, and a regular Command as a Trigger on a
>>> RecordingCommand. After all, how can the trigger know what kind of
>>> command was executing?
>>>
>>> The TransactionalCommandStack is supposed to ensure that the nested
>>> transaction in which triggers are run isolates all of the changes
>>> appropriately.
>>>
>>> If you can isolate your problem to some kind of repeatable test case
>>> that you can share with us, a bugzilla report would definitely be in
>>> order.
>>>
>>> Cheers,
>>>
>>> Christian
>>>
>>>
>>> Mario Winterer wrote:
>>>> Hi!
>>>>
>>>> EMF Transaction's RecordingCommand is both a blessing and a curse!
>>>> It greatly simplifies model change operations - but mixing
>>>> RecordingCommands with "normal" Commands is driving me crazy
>>>> (especially concerning undo/redo)!
>>>> (See also thread "CompoundCommand containing multiple
>>>> RecordingCommands does not undo correctly")
>>>>
>>>> At the moment our rule of thumb is: Don't use RecordingCommands
>>>> inside CompoundCommands; only one top-level RecordingCommand is
>>>> allowed.
>>>>
>>>> This works fine, but now I've implemented a pre-commit trigger that
>>>> returns an AddCommand. On undo, the RecordingCommand undos the
>>>> AddCommand's changes too; as a result, the undo of the AddCommand
>>>> fails.
>>>>
>>>> How can we solve this problem? Is there a way to restrict the
>>>> RecordingCommand to its own changes only? What about nested
>>>> transactions? I thought, trigger-commands are executed in its own
>>>> transaction, so why are their changes undone by the "main"
>>>> (Recording)command?
>>>>
>>>> Thanks in advance,
>>>> Mario
Re: RecordingCommand causes headache [message #423903 is a reply to message #423873] Thu, 09 October 2008 11:36 Go to previous messageGo to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Christian W. Damus schrieb:
> I, Mario,
>
> Yes, this JUnit test looks like a good Bugzilla contribution. :-)

Should I file a bug for this purpose?

> I have already fixed one bug in the undo/redo of RecordingCommands that
> are used as triggers, but I think more work is needed. Probably, the
> RecordingCommand will need to be able to create its own nested
> transaction, as the AbstractEMFOperation can do, in the appropriate
> circumstances.
>
> Cheers,
>
> Christian
>
> Mario Winterer wrote:
>> I've written a unit test to show the danger of
>> RecordingCommand.chain(). The test is based on the extended library
>> model example and tries to undo a chained RecordingCommand in
>> combination with a trigger. (The test adds a book to the library; the
>> trigger sets the author of the book)
>>
>> public void testUndoAddBookWithTriggerChainedRecordingCommand() {
>> editingDomain.addResourceSetListener(new
>> ResourceSetListenerImpl() {
>> public Command transactionAboutToCommit(
>> ResourceSetChangeEvent event
>> ) {
>> // set the author
>> return SetCommand.create(editingDomain, book,
>> EXTLibraryPackage.Literals.BOOK__AUTHOR,
>> author
>> );
>> }
>> });
>>
>> Command command = new RecordingCommand(editingDomain) {
>> protected void doExecute() {
>> library.getBooks().add(book);
>> }
>> };
>>
>> // SELECT ONE OF THE FOLLOWING TWO VERSIONS
>> // VERSION 1: THIS CAUSES THE LAST ASSERTION TO FAIL
>> editingDomain.getCommandStack().execute(
>> command.chain(IdentityCommand.INSTANCE));
>>
>> // VERSION 2: USE THIS LINE INSTEAD OF THE ONE ABOVE
>> // TO MAKE THINKS WORK!
>> // editingDomain.getCommandStack().execute(command);
>> // END VERSIONS
>>
>> assertTrue(library.getBooks().contains(book));
>> assertEquals(author, book.getAuthor());
>>
>> editingDomain.getCommandStack().undo();
>>
>> assertFalse(library.getBooks().contains(book));
>> assertEquals(null, book.getAuthor());
>> }
>>
>> When the RecordingCommand is chained with any other command, the
>> resulting command is no RecordingCommand any more. As a result, in the
>> test above, first the author is unset (due to undoing the trigger
>> command) but then re-set again (due to applyAndReverse of the
>> RecordingCommand).
>>
>> (Please tell me if you want to have the entire code of the unit test).
>>
>> Mario
>>
>> Mario Winterer schrieb:
>>
>>> Hi Christian!
>>>
>>> Thanks for your reply.
>>> After looking into the code again, I've found out that I was wrong.
>>> Our "top command" is no RecordingCommand but a DragAndDropCommand
>>> that internally holds a RecordingCommand as drop command.
>>>
>>> Now on undo the RecordingCommand is undone, and because it holds a
>>> reference to the main transaction, the entire transaction including
>>> the changes caused by the triggers are undone. So the triggers are
>>> undone twice and an exception is thrown (AddCommands cannot be undone
>>> twice...).
>>>
>>> After wrapping the DragAndDropCommand in a RecordingCommand,
>>> everything works fine (I think because of the special handling of
>>> RecordingCommands in the wrapping EMFCommandOperation's didCommit()
>>> method).
>>>
>>> Nevertheless I'm wondering if the RecordingCommand should behave that
>>> way, because that behaviour prevents RecordingCommands to be chained
>>> or used in a CompoundCommand.
>>> In addition I think it is a bug to use the RecordingCommand's chain
>>> method - maybe this method should not exist or at least throw an
>>> UnsupportedOperationException.
>>>
>>> Thanks,
>>> Mario
>>>
>>> Christian W. Damus schrieb:
>>>> Hi, Mario,
>>>>
>>>> You should be able to use a RecordingCommand as a Trigger on a
>>>> regular Command, and a regular Command as a Trigger on a
>>>> RecordingCommand. After all, how can the trigger know what kind of
>>>> command was executing?
>>>>
>>>> The TransactionalCommandStack is supposed to ensure that the nested
>>>> transaction in which triggers are run isolates all of the changes
>>>> appropriately.
>>>>
>>>> If you can isolate your problem to some kind of repeatable test case
>>>> that you can share with us, a bugzilla report would definitely be in
>>>> order.
>>>>
>>>> Cheers,
>>>>
>>>> Christian
>>>>
>>>>
>>>> Mario Winterer wrote:
>>>>> Hi!
>>>>>
>>>>> EMF Transaction's RecordingCommand is both a blessing and a curse!
>>>>> It greatly simplifies model change operations - but mixing
>>>>> RecordingCommands with "normal" Commands is driving me crazy
>>>>> (especially concerning undo/redo)!
>>>>> (See also thread "CompoundCommand containing multiple
>>>>> RecordingCommands does not undo correctly")
>>>>>
>>>>> At the moment our rule of thumb is: Don't use RecordingCommands
>>>>> inside CompoundCommands; only one top-level RecordingCommand is
>>>>> allowed.
>>>>>
>>>>> This works fine, but now I've implemented a pre-commit trigger that
>>>>> returns an AddCommand. On undo, the RecordingCommand undos the
>>>>> AddCommand's changes too; as a result, the undo of the AddCommand
>>>>> fails.
>>>>>
>>>>> How can we solve this problem? Is there a way to restrict the
>>>>> RecordingCommand to its own changes only? What about nested
>>>>> transactions? I thought, trigger-commands are executed in its own
>>>>> transaction, so why are their changes undone by the "main"
>>>>> (Recording)command?
>>>>>
>>>>> Thanks in advance,
>>>>> Mario
Re: RecordingCommand causes headache [message #423910 is a reply to message #423903] Thu, 09 October 2008 14:59 Go to previous messageGo to next message
Eclipse UserFriend
Originally posted by: cdamus.zeligsoft.com

Hi, Mario,

Yes, that is what I meant. Sorry to be so opaque. :-)

cW

Mario Winterer wrote:
> Christian W. Damus schrieb:
>> I, Mario,
>>
>> Yes, this JUnit test looks like a good Bugzilla contribution. :-)
>
> Should I file a bug for this purpose?
>
>> I have already fixed one bug in the undo/redo of RecordingCommands
>> that are used as triggers, but I think more work is needed. Probably,
>> the RecordingCommand will need to be able to create its own nested
>> transaction, as the AbstractEMFOperation can do, in the appropriate
>> circumstances.
>>
>> Cheers,
>>
>> Christian

-----8<-----
Re: RecordingCommand causes headache [message #423952 is a reply to message #423910] Fri, 10 October 2008 13:53 Go to previous messageGo to next message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Oh no!
My unit test case works with EMF Transaction Core 1.2.1 (Eclipse 3.4.1).
Yesterday, the same test failed with EMF Transaction Core 1.2.0 (Eclipse
3.4.0). Your bugfix seems to work...

Christian W. Damus schrieb:
> Hi, Mario,
>
> Yes, that is what I meant. Sorry to be so opaque. :-)
>
> cW
>
> Mario Winterer wrote:
>> Christian W. Damus schrieb:
>>> I, Mario,
>>>
>>> Yes, this JUnit test looks like a good Bugzilla contribution. :-)
>>
>> Should I file a bug for this purpose?
>>
>>> I have already fixed one bug in the undo/redo of RecordingCommands
>>> that are used as triggers, but I think more work is needed.
>>> Probably, the RecordingCommand will need to be able to create its own
>>> nested transaction, as the AbstractEMFOperation can do, in the
>>> appropriate circumstances.
>>>
>>> Cheers,
>>>
>>> Christian
>
> -----8<-----
Re: RecordingCommand causes headache [message #423953 is a reply to message #423952] Fri, 10 October 2008 14:09 Go to previous messageGo to next message
Eclipse UserFriend
Originally posted by: cdamus.zeligsoft.com

Hi, Mario,

This is good news! Why do you say "Oh, no?" Is there something else?

:-)

Mario Winterer wrote:
> Oh no!
> My unit test case works with EMF Transaction Core 1.2.1 (Eclipse 3.4.1).
> Yesterday, the same test failed with EMF Transaction Core 1.2.0 (Eclipse
> 3.4.0). Your bugfix seems to work...
>
> Christian W. Damus schrieb:
>> Hi, Mario,
>>
>> Yes, that is what I meant. Sorry to be so opaque. :-)
>>
>> cW
>>
>> Mario Winterer wrote:
>>> Christian W. Damus schrieb:
>>>> I, Mario,
>>>>
>>>> Yes, this JUnit test looks like a good Bugzilla contribution. :-)
>>>
>>> Should I file a bug for this purpose?
>>>
>>>> I have already fixed one bug in the undo/redo of RecordingCommands
>>>> that are used as triggers, but I think more work is needed.
>>>> Probably, the RecordingCommand will need to be able to create its
>>>> own nested transaction, as the AbstractEMFOperation can do, in the
>>>> appropriate circumstances.
>>>>
>>>> Cheers,
>>>>
>>>> Christian
>>
>> -----8<-----
Re: RecordingCommand causes headache [message #423954 is a reply to message #423953] Fri, 10 October 2008 14:13 Go to previous message
Mario Winterer is currently offline Mario WintererFriend
Messages: 136
Registered: July 2009
Senior Member
Christian W. Damus schrieb:
> Hi, Mario,
>
> This is good news! Why do you say "Oh, no?" Is there something else?

No, no, everything's fine - its just because my poor unit test has
nothing to do now... ;-)

> :-)
>
> Mario Winterer wrote:
>> Oh no!
>> My unit test case works with EMF Transaction Core 1.2.1 (Eclipse
>> 3.4.1). Yesterday, the same test failed with EMF Transaction Core
>> 1.2.0 (Eclipse 3.4.0). Your bugfix seems to work...
>>
>> Christian W. Damus schrieb:
>>> Hi, Mario,
>>>
>>> Yes, that is what I meant. Sorry to be so opaque. :-)
>>>
>>> cW
>>>
>>> Mario Winterer wrote:
>>>> Christian W. Damus schrieb:
>>>>> I, Mario,
>>>>>
>>>>> Yes, this JUnit test looks like a good Bugzilla contribution. :-)
>>>>
>>>> Should I file a bug for this purpose?
>>>>
>>>>> I have already fixed one bug in the undo/redo of RecordingCommands
>>>>> that are used as triggers, but I think more work is needed.
>>>>> Probably, the RecordingCommand will need to be able to create its
>>>>> own nested transaction, as the AbstractEMFOperation can do, in the
>>>>> appropriate circumstances.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> Christian
>>>
>>> -----8<-----
Previous Topic:[Teneo] EMF and legacy database
Next Topic:GenModel and file extension
Goto Forum:
  


Current Time: Fri Apr 26 10:02:22 GMT 2024

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

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

Back to the top