Community
Participate
Working Groups
Internally, the MSLResource implements modification tracking, setting its modified state when changes occur to any of its contents. It differs slightly from EMF's basic modification tracking, in that it only dirties a resource when a non-transient feature changes. However, the resource does not use the isTrackingModification() API to let clients know that it tracks its dirty state. The MSLResource should answer isModificationTracking() == true by default, and also allow clients to turn it off by setModificationTracking(false). Currently, the default value of isTrackingModification() is false and turning it on results in two content adapters trying to do the same job, but differently.
MSL should respect the state of the isTrackingModification() attribute on the Resource and *not* enable modification tracking _until_ the client calls setModificationTracking(true). It sounds like you're proposing a change to the basic semantics of setting this value to true by default. A better solution would be to leave the attribute (and the tracking mechanism) off by default and only enable your customized DirtyAdapter if setModificationTracking(true) is called by the client. The real key here though is that Resource.isModified() should return the correct value; Currently, an MSLResource which has a transient feature changed will return Resource.isModified() == true because the default behavior of the standard EMF modification tracking mechanism is to acknowledge all changes. If the MSLResource wants to customize the behavior, it should only do so while maintaining the original, expected semantics of EMF's Resource interface: (1) isTrackingModification() should equal false by default (2) Resource modification tracking should *only* be enabled if setTrackingModification(true) is invoked and should be disabled if setTrackingModification(false) is called. (3) Resource.isModified() should return true if and only if any persistable changes have been made on the resource. I understand that you use the "transient" feature capability in order to add things to the model which are not stored in the underlying persisted model. Many editors rely on Resource.isModified() to specify the dirty bit in their editor or to know when to save(); if you use the transient features in a way that should (a) not dirty an editor and (b) not be save()'d, then isModified() should respect this case.
It appears that we currently have a conflict between the the MSLObjectListener's management of the dirty state of a resource and the LogicalResource's internal DirtyAdapter. The former checks for the transient-ness of features, whereas the latter does not. Note that the DirtyAdapter's actual purpose is to maintain the dirty state of individual sub-units in a composite ("logical") resource. Unfortunately, it is overstepping its mandate in also updating the overall "modified" state of the resource. All of this is not good and needs to be fixed. This addresses item (3) in Comment #1. As for (1) and (2), one of the original requirements of the MSLResource was that it would always track modification. However, this is something that we should reconsider in our redesign of this "MSL" layer. I'm not sure that the Resource API stipulates that the default value of isModificationTracking should be false, but it makes sense for GMF where command-based editors may assume that they know all of the changes that they are making and, therefore, when a resource is dirty. This is not the kind of editor for which the MSL was originally designed. ;-) Of course, all of this may become moot as we anticipate that the support for cross-resource containment in EMF will obsolete the MSL's LogicalResource anyway.
Delivered a fix to the problem of the LogicalResource::DirtyAdapter reacting to notifications from transient features. The other problem of dirty-tracking not being configurable by the client remains.
Created attachment 31817 [details] Fixes modification setting for non-tracking resources This patch moves responsibility for tracking resource modification from the MSLObjectListener class to the MSL's custom resource implementation. Thus, if your application does not use the MResourceFactory to create its resources, this change ensures that your resource will track modifications only when isTrackingModification() is true and, in that case, will use the default ResourceImpl modification-tracking adapter. The MSLResource is changed to be isTrackingModification() = true by default, and extends the default adapter to setModified(true) only when: - the resource is not already isModified() - the notification is from a non-transient feature of an object that is not, recursively, contained by a transient reference Clients exist that depend on MSLResources tracking modification by default, so this behaviour is retained as an implicit API. However, for clients that do not use MSLResource (by not registering the MResourceFactory for their extensions), the MSL no longer imposes modification tracking externally.
Created attachment 31819 [details] Fixed patch (no extraneous files) The previous patch included two files with unrelated changes, that should not have been.
Applied Patch
[GMF Restructure] Bug 319140 : product GMF and component Runtime EMF was the original product and component for this bug
