Community
Participate
Working Groups
Provide a ChangeRecorder that can adapt itself lazily to CDO models with the goal to make the use of a CommandStack with undo/redo capability available to partly loaded CDO models. --- A org.eclipse.emf.ecore.change.util.ChangeRecorder cannot be used together with partly loaded CDO models since the self adapting mechanism follows all references recursively until the whole containment graph has been loaded into memory. CDO provides a CDOLazyContentAdapter that dynamically self-adapts to all objects that are loaded into memory by CDO. Referenced objects are loaded only when they are needed, hence the term "lazy"-loaded. For large CDO-models to take advantage of a CommandStack with undo/redo there has to be a ChangeRecorder that uses such a lazy adapting mechanism. It would be favourable if the proposed "lazy" ChangeRecorder was compatible with the existing org.eclipse.emf.ecore.change.util.ChangeRecorder and so also with ChangeCommand. But ChangeCommand, ChangeRecorder and its adaption mechanism are currently "soldered" together in a way that makes reuse of the code in a CDO setting difficult. Ideas: - Make org.eclipse.emf.ecore.change.util.ChangeRecorder delegate the object adaption to a pluggable adapter (let it default to current behaviour). In this way a CDOLazyContentAdapter can be used when needed and all related EMF functionality can be reused by CDO-models. - IF the org.eclipse.emf.common.notify.Adapter interface doesn't provide enough functionality for the ChangeRecorder, maybe a new interface with the functionality needed by a pluggable adapter can be introduced and then be implemented by CDOLazyContentAdapter. There may be similarities with this bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=323792
The only place in EMF core where ChangeRecorder is used is in ChangeCommand and that's not used anywhere else in EMF core. I can't break the ChangeCommand API, so unless CDO extends ChangeRecorder, it can't reuse ChangeCommand. CDO could certainly provide its own ChangeCommand that uses its own version of a change recorder, but then the problem just moves to downstream frameworks like EMF Transaction, and that code base is basically unsupported these days. It's not clear that CDO can't just extend ChangeRecorder, perhaps specializing mostly the setTarget method to not aggressively adapt the whole containment tree. Until there is an answer to that questions, I'll assign this to CDO pending concrete suggestions for non-breaking changes I can make in EMF core to support greater flexibility. If CDO can implement what it needs by extending ChangeRecorder, we could add some facility so that the following method in ChangeCommand can find an appropriate implementation, perhaps by looking for an adapter on the resource set... protected ChangeRecorder createChangeRecorder() { return new ChangeRecorder(); }
Based on the 2 previous comments, I come with the following idea: a) Create a subclass of ChangeRecorder, let's say EContentAdapterDelegatingChangeRecorder. b) This class overrides the original behavior that adds the adapter recursively on children. E.g. override .setTarget(), .addAdapter(), etc, in order to do nothing. c) The new class would have an attribute of type EContentAdapter that is simply set on the root object when beginning recording. When EContentAdapter receives notifications (i.e. its notifyChanged() method), it would delegate to the changeRecorder.notifyChanged() d) The above attribute could by a CDOLazyContentAdapter Does this proposition has any flaws? I would like to begin this kind of implementation, but I'm wondering if there are cases/scenarios that I'm missing.
I don't immediately see that something is missing and it seems cheap enough to try it. Ed, do you have some hints for Cristian? Anders, what about your implementation, do you still consider to attach it? And if it's "just" as a reference ;-)
The subclassing of ChangeRecorder seems to be the right approach but I am not sure that it is as straight forward as proposed by Cristian (I may be wrong). What you would want is a separation of the two concerns: - Adding and removing the (ChangeRecorder) adapter - The actual recording of changes As it is these concerns don't seem to be separated at all (and probably for good reason). For example the targetObjects and originalTargetObject attributes in ChangeRecorder are accessed both in consolidateChanges() (which is change recording) and in setTarget() (which is adapting). I haven't wrapped my head around all this but simply overriding setTarget() will hardly work. Maybe there is a way to factor out the change recorder functionality from setTarget(). Then the ChangeRecorder subclass (i.e. the EContentAdapterDelegatingChangeRecorder from comment 2) would still need to *be* the adapter and intercept all the setTarget() etc calls in order to first(?) call the (delegate) EcontentAdapter and after(?) that the factored out change recorder functionality. Also, there is a bug in CDOLazyContentAdapter that needs fixing: https://bugs.eclipse.org/bugs/show_bug.cgi?id=405483 ///Anders
Anders, what about your implementation, do you still consider to attach it? And if it's "just" as a reference ;-)
My "implementation" has so many problems, partly caused by a desire to make it work without changes to the EMF code and to work around bugs (like 323792). It doesn't subclass ChangeRecorder, instead it subclasses CDOLazyContentAdapter and delegates to ChangeRecorder. And so it is not compatible with... anything. Sure I can attach it, but, besides being really hard to follow, it just wouldn't be a step in the right direction. On the other hand, I have decided to try and contribute something more like what is discussed in this bug, i.e. subclassing ChangeRecorder and delegating to an Adapter.
Moving all outstanding enhancements to 4.3
Moving all open enhancement requests to 4.4
By default ChangeRecorder & its descendant TransactionChangeRecorder are lazy according to EMF API, i.e. using EObject.eIsProxy() and InternalEList.basicIterator(). Then if we had EMF proxy support in CDO, we would have a lazy ChangeRecorder for free. See Bug 392720
Moving all open bugzillas to 4.5.
Moving all unaddressed bugzillas to 4.6.
Moving all open bugs to 4.7
Moving all unresolved issues to version 4.8-
Moving all unresolved issues to version 4.9
Moving to 4.13.
