Community
Participate
Working Groups
Build Identifier: 20110916-0149 Root elements cannot be deleted from projects. Actually, they deletes, but appears again after eclipse relaunch (i.e., no actual changes written to storage). I've made some investigation and found that in case of deleting root model element state persister doesn't save resource, because dirtyResources is empty after deleting. May be it is because of condition in file /org.eclipse.emf.emfstore.common.model/src/org/eclipse/emf/emfstore/common/model/util/EObjectChangeNotifier.java on line 181. I.e. if notifier is project, project change observers doesn't notified about element removal, and state persister then doesn't add project's resource to dirtyResources. reproducible on svn, git & current update site versions. Reproducible: Always Steps to Reproduce: 1. Create project 2. Create an model element in project 3. Delete this element 4. Relaunch Eclipse 5. Element still exists in project
Yes, removing condition part !(notifier instanceof Project) solved problem, but I don't know if it's safe to do that. Comment pls!
Thanks for pinning down this bug! I think the change you suggested is potentially dangerous since receivers might not expect notifications of Project. Maybe they should be aware of that, but for the time being I would suggest the following change. I had no time to test it, I am on the EclipseCon currenly, could you please try? Also it would be great if you could supply a test case. A potential starting point for a test is CompositeOperationTest.abortSmallComposite() in client.test. Index: src/org/eclipse/emf/emfstore/client/model/impl/StatePersister.java =================================================================== --- src/org/eclipse/emf/emfstore/client/model/impl/StatePersister.java (revision 12442) +++ src/org/eclipse/emf/emfstore/client/model/impl/StatePersister.java (working copy) @@ -5,6 +5,7 @@ import org.eclipse.emf.common.command.Command; import org.eclipse.emf.common.notify.Notification; +import org.eclipse.emf.common.notify.impl.AdapterImpl; import org.eclipse.emf.common.util.BasicEMap; import org.eclipse.emf.common.util.TreeIterator; import org.eclipse.emf.common.util.URI; @@ -27,7 +28,7 @@ import org.eclipse.emf.emfstore.common.model.util.ModelUtil; import org.eclipse.emf.emfstore.common.model.util.ProjectChangeObserver; -public class StatePersister implements CommandObserver, ProjectChangeObserver { +public class StatePersister extends AdapterImpl implements CommandObserver, ProjectChangeObserver { private IdEObjectCollection collection; Index: src/org/eclipse/emf/emfstore/client/model/impl/ProjectSpaceImpl.java =================================================================== --- src/org/eclipse/emf/emfstore/client/model/impl/ProjectSpaceImpl.java (revision 12442) +++ src/org/eclipse/emf/emfstore/client/model/impl/ProjectSpaceImpl.java (working copy) @@ -1682,6 +1682,8 @@ // TODO: initialization order important this.getProject().addProjectChangeObserver(this.operationRecorder); this.getProject().addProjectChangeObserver(statePersister); + // FIXME: BUGFIX move me into StatePersister + this.getProject().eAdapters().add(this.statePersister); if (project instanceof ProjectImpl) { ((ProjectImpl) this.getProject()).setUndetachable(operationRecorder); (In reply to comment #1) > Yes, removing condition part !(notifier instanceof Project) solved problem, but > I don't know if it's safe to do that. > > Comment pls!
Hi! Thanks for quick reply. Tried patch, it doesn't work. I've got your point, but notification doesn't passed to notify method of StatePersister.
We found a fix and it seems to work, we will do a code review tomorrow and then check it in. I will let you know.
Created attachment 206943 [details] fixes #362649
The patch attached should solve this bug. As Maximilian pointed out, some receivers do not expect notifications from projects, thus, additionally to removing the check in EObjectChangeNotifier, we added a new filter to the default FilterStack. NotifiableIdEObjectCollectionFilter makes sure that notifications from Projects only come through if that is the desired behavior. Another instanceof-check for project is required in the StatePersisters to prevent it from trying to get an ID from a project and trying to set it in the resource. Otherwise StatePersister will throw a RuntimeException.
The proposed patch is OK and has been applied to the master.
great news!
