Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 452653

Summary:	TrackingModificationTrigger is called several times
Product:	[Modeling] Sirius	Reporter:	Esteban DUGUEPEROUX <esteban.dugueperoux>
Component:	Core	Assignee:	Esteban DUGUEPEROUX <esteban.dugueperoux>
Status:	CLOSED FIXED	QA Contact:	Maxime Porhel <maxime.porhel>
Severity:	normal
Priority:	P3	CC:	belqassim.djafer, cedric.brun, laurent.redor, maxime.porhel, pierre-charles.david
Version:	2.0.0	Keywords:	triaged
Target Milestone:	3.0.0M4
Hardware:	PC
OS:	Linux
Whiteboard:	needtest
Bug Depends on:
Bug Blocks:	454016

Description Esteban DUGUEPEROUX

2014-11-21 08:52:32 EST

With Bug 452558 we have seen that a same precommit can be called several times, for example we have DanglingRefRemovalTrigger which can clean semantic dangling references then trigger a second times TrackingModificationTrigger.
In addition in case of rollBack in the second precommit iteration loop, the work of TrackingModificationTrigger will be useless.

As the change on Resource.modified field can be done outside write transaction, we could move TrackingModificationTrigger logic to a ValidateEditSupport implementation to be sure to be called once.

Comment 1 Esteban DUGUEPEROUX

2014-11-21 08:54:07 EST

A draft of gerrit : https://git.eclipse.org/r/36844

Comment 2 Cedric Brun

2014-11-21 09:12:22 EST

You might wnat to check that ResourceSetSync gets called with the isModified=true change anyway as all the workspace sync will rely on this.

Another implication would be that we can't "rollback" anymore based on the fact that isModified gets set to true. I don't see a use case for this but I'd like you to confirm it gets not possible then.

Logic which would rely on the isModified flag being set *during* the transaction will not work anymore. You'll have to check that a session.save() which gets requested while a transaction is in process and ends up being trigger in postcommit() will always have this information up to date or the saving policies can't rely on it.

Comment 3 Esteban DUGUEPEROUX

2014-11-28 11:14:46 EST

ResourceSetSync is correctly notified of this change in postcommit as before.

Indeed we can't anymore rollback on this feature change.

Code in postcommit can rely on it, as ValidateEditSupport.finalizeForCommit() is called before postcommits.

Comment 4 Esteban DUGUEPEROUX

2014-12-01 03:57:12 EST

Currently, i.e. with the current TrackingModificationTrigger, there is an issue :

1. do a change
2. save => all Resources return false al Resource.isModified() call OK
3. undo => all Resources return false al Resource.isModified() call KO while the editor is visible as dirty with a *
This is because Resource.RESOURCE__IS_MODIFIED feature is not undoed.

Comment 5 Esteban DUGUEPEROUX

2014-12-01 04:03:44 EST

We don't have this issue with Resource.setTrackingModification() has on undo we receive notifications. The advantage of ValidateEditSupport and that we are notified of changed resource even on undo through ValidateEditSupport.handleResourceChange() method.

Comment 6 Esteban DUGUEPEROUX

2014-12-01 08:49:53 EST

Fixed as 5ca028139011f2c30380af9c58158e8821bc26c5 .

Comment 7 Pierre-Charles David

2014-12-02 04:48:12 EST

The previous commit caused a regression in our tests. Reopening until the fix in https://git.eclipse.org/r/#/c/37441/ is merged.

Comment 8 Esteban DUGUEPEROUX

2014-12-03 03:20:25 EST

Last Gerrit merged as 2c8add9824179a1e70bacbdb1870ba8710ee545a.

Comment 9 Pierre-Charles David

2014-12-03 09:01:06 EST

Cloned as bug 454016 to backport into Sirius 2.0.x.

Comment 10 Belqassim Djafer

2015-04-08 05:32:13 EDT

Verified as technical issue on Sirius 3.0.0M6

Comment 11 Maxime Porhel

2015-05-22 10:16:08 EDT

Validated on Sirius 3.0.0 RC1

Comment 12 Pierre-Charles David

2015-06-24 11:15:10 EDT

Available in Sirius 3.0.0. See https://wiki.eclipse.org/Sirius/3.0.0.