[452962] Improve Session resources cleanup on close

    
- clear DAnlysisSessionEObject.activatedViewpoints to avoid potential
Viewpoint reloading on proxy resolution.
- Give Session in constructor of RepresentationNameListener to avoid get
Session from SessionManager.
- Remove useless try/catch NPE in RepresentationsChangeAdapter as it was
for a CDO bug which is now fixed.
- Have DAnalysisSessionImpl.Saver registration/unregistration for both
ResourceSetListener and Lifecycle in constructor/dispose().
- Have DAnalysisSessionImpl.removeAdaptersOnAnalysis() remove
SemanticResourcesUpdater as it is added in
DAnalysisSessionImpl.addAdaptersOnAnalysis()

Bug: 452962
Change-Id: I8942bdd7168f4cc3c5981dd0b5c2c4b438ac0359
Signed-off-by: Esteban Dugueperoux <esteban.dugueperoux@obeo.fr>

3 files changed, 64 insertions, 70 deletions

diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/RepresentationNameListener.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/RepresentationNameListener.java
index 984e493283..a720b6b9d3 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/RepresentationNameListener.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/RepresentationNameListener.java
@@ -11,15 +11,11 @@
 package org.eclipse.sirius.business.internal.session;
 
 import org.eclipse.emf.common.notify.Notification;
-import org.eclipse.emf.ecore.EObject;
 import org.eclipse.emf.transaction.ResourceSetChangeEvent;
 import org.eclipse.emf.transaction.ResourceSetListenerImpl;
 import org.eclipse.sirius.business.api.session.Session;
 import org.eclipse.sirius.business.api.session.SessionManager;
-import org.eclipse.sirius.ext.base.Option;
-import org.eclipse.sirius.ext.base.Options;
 import org.eclipse.sirius.viewpoint.DRepresentation;
-import org.eclipse.sirius.viewpoint.DSemanticDecorator;
 import org.eclipse.sirius.viewpoint.ViewpointPackage;
 
 import com.google.common.collect.Iterables;
@@ -31,44 +27,43 @@ import com.google.common.collect.Iterables;
  * @author pcdavid
  */
 public class RepresentationNameListener extends ResourceSetListenerImpl {
+
+    private Session session;
+
     /**
-     * {@inheritDoc}
+     * Default constructor.
+     * 
+     * @param session
+     *            the {@link Session}
      */
+    public RepresentationNameListener(Session session) {
+        this.session = session;
+        session.getTransactionalEditingDomain().addResourceSetListener(this);
+    }
+
     @Override
     public boolean isPostcommitOnly() {
         return true;
     }
 
-    /**
-     * {@inheritDoc}
-     */
     @Override
     public void resourceSetChanged(ResourceSetChangeEvent event) {
         for (Notification notif : Iterables.filter(event.getNotifications(), Notification.class)) {
             if (isRepresentationNameChange(notif)) {
-                Option<Session> session = getSessionFromRepresentation((DRepresentation) notif.getNotifier());
-                if (session.some()) {
-                    SessionManager.INSTANCE.notifyRepresentationRenamed(session.get());
-                }
+                SessionManager.INSTANCE.notifyRepresentationRenamed(session);
             }
         }
     }
 
-    private Option<Session> getSessionFromRepresentation(DRepresentation representation) {
-        EObject semanticElement = null;
-        if (representation instanceof DSemanticDecorator) {
-            semanticElement = ((DSemanticDecorator) representation).getTarget();
-        }
-        if (semanticElement != null) {
-            Session session = SessionManager.INSTANCE.getSession(semanticElement);
-            if (session != null) {
-                return Options.newSome(session);
-            }
-        }
-        return Options.newNone();
-    }
-
     private boolean isRepresentationNameChange(Notification notif) {
         return notif.getNotifier() instanceof DRepresentation && notif.getFeatureID(DRepresentation.class) == ViewpointPackage.DREPRESENTATION__NAME;
     }
+
+    /**
+     * Dispose this resource.
+     */
+    public void dispose() {
+        getTarget().removeResourceSetListener(this);
+        session = null;
+    }
 }
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/DAnalysisSessionImpl.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/DAnalysisSessionImpl.java
index eb34aa1669..447ccdf6ff 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/DAnalysisSessionImpl.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/DAnalysisSessionImpl.java
@@ -52,11 +52,11 @@ import org.eclipse.emf.ecore.resource.impl.ResourceImpl;
 import org.eclipse.emf.ecore.util.ECrossReferenceAdapter;
 import org.eclipse.emf.ecore.util.EcoreUtil;
 import org.eclipse.emf.transaction.ResourceSetChangeEvent;
-import org.eclipse.emf.transaction.ResourceSetListener;
 import org.eclipse.emf.transaction.ResourceSetListenerImpl;
 import org.eclipse.emf.transaction.RunnableWithResult;
 import org.eclipse.emf.transaction.Transaction;
 import org.eclipse.emf.transaction.TransactionalEditingDomain;
+import org.eclipse.emf.transaction.TransactionalEditingDomain.Lifecycle;
 import org.eclipse.emf.transaction.TransactionalEditingDomainEvent;
 import org.eclipse.emf.transaction.TransactionalEditingDomainListener;
 import org.eclipse.emf.transaction.TransactionalEditingDomainListenerImpl;
@@ -195,20 +195,26 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
             }
         };
 
-        public void initialize() {
-            TransactionalEditingDomain ted = getTransactionalEditingDomain();
-            if (ted instanceof TransactionalEditingDomain.Lifecycle) {
-                TransactionalEditingDomain.Lifecycle lc = (TransactionalEditingDomain.Lifecycle) ted;
-                lc.addTransactionalEditingDomainListener(domainListener);
+        /**
+         * Default constructor.
+         * 
+         * @param domain
+         *            the {@link TransactionalEditingDomain}
+         */
+        public Saver(TransactionalEditingDomain domain) {
+            domain.addResourceSetListener(this);
+            Lifecycle lifecycle = TransactionUtil.getAdapter(getTarget(), Lifecycle.class);
+            if (lifecycle != null) {
+                lifecycle.addTransactionalEditingDomainListener(domainListener);
             }
         }
 
         public void dispose() {
-            TransactionalEditingDomain ted = getTransactionalEditingDomain();
-            if (ted instanceof TransactionalEditingDomain.Lifecycle) {
-                TransactionalEditingDomain.Lifecycle lc = (TransactionalEditingDomain.Lifecycle) ted;
-                lc.removeTransactionalEditingDomainListener(domainListener);
+            Lifecycle lifecycle = TransactionUtil.getAdapter(getTarget(), Lifecycle.class);
+            if (lifecycle != null) {
+                lifecycle.removeTransactionalEditingDomainListener(domainListener);
             }
+            getTarget().removeResourceSetListener(this);
             disarm();
         }
 
@@ -271,7 +277,7 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
     }
 
     /** The {@link TransactionalEditingDomain} associated to this Session. */
-    protected final TransactionalEditingDomain transactionalEditingDomain;
+    private TransactionalEditingDomain transactionalEditingDomain;
 
     // Session's state and helpers for its maintenance.
     // See also the following fields inherited from DAnalysisSessionEObject:
@@ -292,7 +298,7 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
     /** The semantic resources collection updater. */
     protected SemanticResourcesUpdater semanticResourcesUpdater;
 
-    private final ControlledResourcesDetector controlledResourcesDetector;
+    private ControlledResourcesDetector controlledResourcesDetector;
 
     private DAnalysisRefresher dAnalysisRefresher;
 
@@ -301,7 +307,7 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
     /** The custom saving policy the session should use. */
     protected SavingPolicy savingPolicy;
 
-    private final Saver saver = new Saver();
+    private Saver saver;
 
     private ReloadingPolicy reloadingPolicy;
 
@@ -333,9 +339,9 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
     /** The listener suitable for refresh the opened viewpoint editors. */
     protected RefreshEditorsPrecommitListener refreshEditorsListeners;
 
-    private final RepresentationsChangeAdapter representationsChangeAdapter;
+    private RepresentationsChangeAdapter representationsChangeAdapter;
 
-    private final ResourceSetListener representationNameListener;
+    private RepresentationNameListener representationNameListener;
 
     /**
      * Create a new session.
@@ -349,11 +355,10 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
         Preconditions.checkNotNull(this.sessionResource, "A session must be inside a resource.");
         this.transactionalEditingDomain = Preconditions.checkNotNull(TransactionUtil.getEditingDomain(mainDAnalysis), "A session must be associated to an EditingDomain");
         this.mainDAnalysis = mainDAnalysis;
-
         this.interpreter = new ODesignGenericInterpreter();
         this.representationsChangeAdapter = new RepresentationsChangeAdapter(this);
-        this.representationNameListener = new RepresentationNameListener();
         this.controlledResourcesDetector = new ControlledResourcesDetector(this);
+        saver = new Saver(getTransactionalEditingDomain());
         super.getAnalyses().add(mainDAnalysis);
         super.getResources().add(sessionResource);
         setAnalysisSelector(DAnalysisSelectorService.getSelector(this));
@@ -507,9 +512,8 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
 
             ResourceSetSync.getOrInstallResourceSetSync(transactionalEditingDomain).registerClient(this);
             monitor.worked(1);
-            transactionalEditingDomain.addResourceSetListener(representationNameListener);
+            this.representationNameListener = new RepresentationNameListener(this);
             monitor.worked(1);
-            saver.initialize();
 
             final Collection<DAnalysis> allAnalyses = allAnalyses();
             if (allAnalyses.isEmpty()) {
@@ -1438,6 +1442,9 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
         if (this.representationsChangeAdapter != null) {
             this.representationsChangeAdapter.unregisterAnalysis(analysis);
         }
+        if (semanticResourcesUpdater != null && analysis.eAdapters().contains(semanticResourcesUpdater)) {
+            analysis.eAdapters().remove(semanticResourcesUpdater);
+        }
     }
 
     @Override
@@ -1875,9 +1882,6 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
         if (!isOpen()) {
             return;
         }
-        if (saver != null && getTransactionalEditingDomain() != null) {
-            getTransactionalEditingDomain().removeResourceSetListener(saver);
-        }
         if (Movida.isEnabled()) {
             org.eclipse.sirius.business.internal.movida.registry.ViewpointRegistry registry = (org.eclipse.sirius.business.internal.movida.registry.ViewpointRegistry) ViewpointRegistry.getInstance();
             registry.removeListener((ViewpointRegistryListener) this);
@@ -1889,12 +1893,16 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
 
         if (controlledResourcesDetector != null) {
             controlledResourcesDetector.dispose();
+            controlledResourcesDetector = null;
         }
         if (dAnalysisRefresher != null) {
             dAnalysisRefresher.dispose();
             dAnalysisRefresher = null;
         }
-
+        removeListener(getRefreshEditorsListener());
+        refreshEditorsListeners = null;
+        reloadingPolicy = null;
+        savingPolicy = null;
         if (interpreter != null) {
             interpreter.dispose();
         }
@@ -1912,8 +1920,9 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
             currentResourceCollector = null;
         }
         interpreter = null;
-        crossReferencer = null;
-        transactionalEditingDomain.removeResourceSetListener(representationNameListener);
+        representationNameListener.dispose();
+        representationNameListener = null;
+        representationsChangeAdapter = null;
         // dispose the SessionEventBroker
         if (broker != null) {
             broker.dispose();
@@ -1940,12 +1949,18 @@ public class DAnalysisSessionImpl extends DAnalysisSessionEObjectImpl implements
             semanticResources.clear();
         }
         reenableECrossReferenceAdaptersBeforeEndOfClosing();
+        crossReferencer = null;
         saver.dispose();
-
+        saver = null;
         if (disposeEditingDomainOnClose) {
             transactionalEditingDomain.dispose();
             doDisposePermissionAuthority(resourceSet);
+            transactionalEditingDomain = null;
         }
+        getActivatedViewpoints().clear();
+        movidaSupport = null;
+        services = null;
+        sessionResource = null;
         mainDAnalysis = null;
     }
 
diff --git a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/RepresentationsChangeAdapter.java b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/RepresentationsChangeAdapter.java
index d6ae272830..200e8df85d 100644
--- a/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/RepresentationsChangeAdapter.java
+++ b/plugins/org.eclipse.sirius/src/org/eclipse/sirius/business/internal/session/danalysis/RepresentationsChangeAdapter.java
@@ -12,15 +12,12 @@ package org.eclipse.sirius.business.internal.session.danalysis;
 
 import java.util.Collection;
 
-import org.eclipse.core.runtime.IStatus;
-import org.eclipse.core.runtime.Status;
 import org.eclipse.emf.common.notify.Notification;
 import org.eclipse.emf.common.notify.impl.AdapterImpl;
 import org.eclipse.sirius.business.api.session.Session;
 import org.eclipse.sirius.business.api.session.SessionManager;
 import org.eclipse.sirius.viewpoint.DAnalysis;
 import org.eclipse.sirius.viewpoint.DView;
-import org.eclipse.sirius.viewpoint.SiriusPlugin;
 import org.eclipse.sirius.viewpoint.ViewpointPackage;
 
 import com.google.common.collect.Iterables;
@@ -116,12 +113,7 @@ public class RepresentationsChangeAdapter extends AdapterImpl {
      */
     public void unregisterAnalysis(final DAnalysis analysis) {
         if (analysis.eAdapters().contains(this)) {
-            // TODO remove this try/catch once the offline mode will be supported
-            try {
-                analysis.eAdapters().remove(this);
-            } catch (NullPointerException e) {
-                SiriusPlugin.getDefault().getLog().log(new Status(IStatus.WARNING, SiriusPlugin.ID, "Error while closing the representation, the remote server may be unreachable."));
-            }
+            analysis.eAdapters().remove(this);
         }
         for (final DView view : analysis.getOwnedViews()) {
             unregisterView(view);
@@ -146,15 +138,7 @@ public class RepresentationsChangeAdapter extends AdapterImpl {
      */
     private void unregisterView(final DView view) {
         if (view.eAdapters().contains(this)) {
-            // TODO remove this try/catch once the offline mode will be supported
-            try {
-                view.eAdapters().remove(this);
-            } catch (NullPointerException e) {
-                if (SiriusPlugin.getDefault().isDebugging()) {
-                    SiriusPlugin.getDefault().getLog().log(new Status(IStatus.WARNING, SiriusPlugin.ID, "Error while closing the representation, the remote server may be unreachable."));
-                }
-            }
-
+            view.eAdapters().remove(this);
         }
     }
 }