Merge pull request #10 from protegeproject/fix-improperly-disposed-pr…

…ojects Fix improperly disposed projects and only respond to selection changes for the project that the selection model belongs to
protegeproject · Jun 4, 2024 · 8713c7d · 8713c7d
2 parents 04abdd5 + 34178e4
commit 8713c7d
Show file tree

Hide file tree

Showing 11 changed files with 128 additions and 24 deletions.
diff --git a/...t/src/main/java/edu/stanford/bmir/protege/web/client/dispatch/DispatchServiceManager.java b/...t/src/main/java/edu/stanford/bmir/protege/web/client/dispatch/DispatchServiceManager.java
@@ -6,6 +6,7 @@
 import com.google.gwt.place.shared.Place;
 import com.google.gwt.place.shared.PlaceController;
 import com.google.gwt.user.client.Timer;
+import com.google.gwt.user.client.Window;
 import com.google.gwt.user.client.rpc.AsyncCallback;
 import com.google.gwt.user.client.rpc.InvocationException;
 import com.google.web.bindery.event.shared.EventBus;
@@ -31,6 +32,7 @@
 import java.util.*;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -126,6 +128,7 @@ public <A extends Action<R>, R extends Result> void execute(A action, final Disp
         callback.handleSubmittedForExecution();
         if(action instanceof HasProjectId) {
             ProjectId projectId = ((HasProjectId) action).getProjectId();
+            checkMakingACallForCurrentProject(projectId);
             ResultCache resultCache = getResultCache(projectId, eventBus);
             Optional<R> result = resultCache.getCachedResult(action);
             if (result.isPresent()) {
@@ -145,6 +148,24 @@ public <A extends Action<R>, R extends Result> void execute(A action, final Disp
 //        }
     }
 
+    private void checkMakingACallForCurrentProject(ProjectId projectId) {
+        Place place = placeController.getWhere();
+        if(place instanceof HasProjectId) {
+            ProjectId placeProjectId = ((HasProjectId) place).getProjectId();
+            if(!placeProjectId.equals(projectId)) {
+                logCurrentStack();
+            }
+        }
+    }
+
+    private static void logCurrentStack() {
+        try {
+            throw new RuntimeException("Mismatch");
+        } catch (RuntimeException e) {
+            logger.log(Level.WARNING, "Mismatch of project Ids.  If not deliberate this may indicated a bug somewhere.", e);
+        }
+    }
+
     @SuppressWarnings("unchecked")
     private <A extends Action<R>, R extends Result> void execAction(A action, DispatchServiceCallback<R> callback) {
         requestCount++;

diff --git a/...rc/main/java/edu/stanford/bmir/protege/web/client/editor/ClassEditorPortletPresenter.java b/...rc/main/java/edu/stanford/bmir/protege/web/client/editor/ClassEditorPortletPresenter.java
@@ -49,4 +49,10 @@ public void startPortlet(PortletUi portletUi, WebProtegeEventBus eventBus) {
     protected void handleReloadRequest() {
         editorPresenter.handleReloadRequest();
     }
+
+    @Override
+    public void dispose() {
+        editorPresenter.dispose();
+        super.dispose();
+    }
 }
diff --git a/...ent/src/main/java/edu/stanford/bmir/protege/web/client/inject/ClientProjectComponent.java b/...ent/src/main/java/edu/stanford/bmir/protege/web/client/inject/ClientProjectComponent.java
@@ -10,8 +10,10 @@
 import edu.stanford.bmir.protege.web.client.project.ProjectPresenter;
 import edu.stanford.bmir.protege.web.client.projectsettings.ProjectSettingsPresenter;
 import edu.stanford.bmir.protege.web.client.search.EntitySearchSettingsPresenter;
+import edu.stanford.bmir.protege.web.client.selection.SelectionModel;
 import edu.stanford.bmir.protege.web.client.sharing.SharingSettingsPresenter;
 import edu.stanford.bmir.protege.web.client.tag.ProjectTagsPresenter;
+import edu.stanford.bmir.protege.web.shared.HasDispose;
 import edu.stanford.bmir.protege.web.shared.inject.ProjectSingleton;
 import edu.stanford.bmir.protege.web.shared.project.ProjectId;
 
@@ -28,7 +30,7 @@
         }
 )
 @ProjectSingleton
-public interface ClientProjectComponent {
+public interface ClientProjectComponent extends HasDispose {
 
     ProjectId getProjectId();
 
@@ -51,4 +53,11 @@ public interface ClientProjectComponent {
     EntitySearchSettingsPresenter getEntitySearchSettingsPresenter();
 
     PerspectivesManagerPresenter getPerspectivesManagerPresenter();
+
+    SelectionModel getProjectSelectionModel();
+
+    default void dispose() {
+        getProjectPresenter().dispose();
+        getProjectSelectionModel().dispose();
+    }
 }
diff --git a/...lient/src/main/java/edu/stanford/bmir/protege/web/client/perspective/PerspectiveImpl.java b/...lient/src/main/java/edu/stanford/bmir/protege/web/client/perspective/PerspectiveImpl.java
@@ -138,6 +138,7 @@ public void onResize() {
 
     @Override
     public void dispose() {
+        widgetMapper.dispose();
     }
 
 

diff --git a/.../src/main/java/edu/stanford/bmir/protege/web/client/perspective/PerspectivePresenter.java b/.../src/main/java/edu/stanford/bmir/protege/web/client/perspective/PerspectivePresenter.java
@@ -6,6 +6,7 @@
 import com.google.gwt.user.client.ui.AcceptsOneWidget;
 import com.google.gwt.user.client.ui.Label;
 import com.google.web.bindery.event.shared.EventBus;
+import com.google.web.bindery.event.shared.HandlerRegistration;
 import edu.stanford.bmir.protege.web.client.dispatch.DispatchServiceManager;
 import edu.stanford.bmir.protege.web.client.form.LanguageMapCurrentLocaleMapper;
 import edu.stanford.bmir.protege.web.client.library.msgbox.MessageBox;
@@ -66,6 +67,8 @@ public class PerspectivePresenter implements HasDispose {
 
     private final LanguageMapCurrentLocaleMapper localeMapper;
 
+    private HandlerRegistration placeChangedHandlerRegistration;
+
 
     @Inject
     public PerspectivePresenter(final PerspectiveView perspectiveView,
@@ -91,8 +94,8 @@ public PerspectivePresenter(final PerspectiveView perspectiveView,
 
     public void start(AcceptsOneWidget container, EventBus eventBus, ProjectViewPlace place) {
         GWT.log("[PerspectivePresenter] Starting at place " + place);
-        eventBus.addHandler(PlaceChangeEvent.TYPE, event -> {
-            if(event.getNewPlace() instanceof ProjectViewPlace) {
+        placeChangedHandlerRegistration = eventBus.addHandler(PlaceChangeEvent.TYPE, event -> {
+            if (event.getNewPlace() instanceof ProjectViewPlace) {
                 displayPerspective(((ProjectViewPlace) event.getNewPlace()).getPerspectiveId());
             }
         });
@@ -213,6 +216,9 @@ private void savePerspectiveLayout(PerspectiveId perspectiveId, Optional<Node> n
 
     @Override
     public void dispose() {
+        if(placeChangedHandlerRegistration != null) {
+            placeChangedHandlerRegistration.removeHandler();
+        }
         for(Perspective tab : perspectiveCache.values()) {
             tab.dispose();
         }

diff --git a/...t/src/main/java/edu/stanford/bmir/protege/web/client/perspective/PortletWidgetMapper.java b/...t/src/main/java/edu/stanford/bmir/protege/web/client/perspective/PortletWidgetMapper.java
@@ -4,6 +4,7 @@
 import com.google.gwt.user.client.ui.IsWidget;
 import com.google.gwt.user.client.ui.Label;
 import edu.stanford.bmir.protege.web.client.portlet.*;
+import edu.stanford.bmir.protege.web.shared.HasDispose;
 import edu.stanford.bmir.protege.web.shared.PortletId;
 import edu.stanford.bmir.protege.web.shared.event.WebProtegeEventBus;
 import edu.stanford.protege.widgetmap.client.HasFixedPrimaryAxisSize;
@@ -13,14 +14,14 @@
 import edu.stanford.protege.widgetmap.shared.node.NodeProperties;
 import edu.stanford.protege.widgetmap.shared.node.TerminalNode;
 import edu.stanford.protege.widgetmap.shared.node.TerminalNodeId;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nonnull;
 import javax.inject.Inject;
 import javax.inject.Provider;
-import java.util.HashMap;
-import java.util.Map;
-import java.util.Optional;
+import java.util.*;
 import java.util.function.Consumer;
+import java.util.logging.Logger;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -29,7 +30,9 @@
  * Stanford Center for Biomedical Informatics Research
  * 17/02/16
  */
-public class PortletWidgetMapper implements WidgetMapper {
+public class PortletWidgetMapper implements WidgetMapper, HasDispose {
+
+    private final Logger logger = Logger.getLogger("PortletWidgetMapper");
 
     private static final String DROP_ZONE = "drop-zone";
 
@@ -45,6 +48,8 @@ public class PortletWidgetMapper implements WidgetMapper {
 
     private Consumer<TerminalNode> nodePropertiesChangedHandler = node -> {};
 
+    private List<HasDispose> disposables = new ArrayList<>();
+
     @Inject
     public PortletWidgetMapper(@Nonnull PortletFactory portletFactory,
                                Provider<PortletUi> portletUiProvider,
@@ -69,19 +74,19 @@ public void setNodePropertiesChangedHandler(Consumer<TerminalNode> nodePropertie
 
     @Override
     public IsWidget getWidget(TerminalNode terminalNode) {
-        GWT.log("[PortletWidgetMapper] Getting widget for TerminalNode: " + terminalNode);
+        logger.info("logger.infoGetting widget for TerminalNode: " + terminalNode);
         ViewHolder cachedViewHolder = nodeId2ViewHolderMap.get(terminalNode.getNodeId());
         if (cachedViewHolder != null) {
-            GWT.log("[PortletWidgetMapper] Using cached view: " + terminalNode);
+            logger.info("logger.infoUsing cached view: " + terminalNode);
             return cachedViewHolder;
         }
         String portletClass = terminalNode.getNodeProperties().getPropertyValue("portlet", "");
-        GWT.log("[PortletWidgetMapper] Instantiate portlet: " + portletClass);
+        logger.info("logger.infoInstantiate portlet: " + portletClass);
         ViewHolder viewHolder;
         if (!portletClass.isEmpty()) {
             Optional<WebProtegePortletComponents> thePortlet = portletFactory.createPortlet(new PortletId(portletClass));
             if (thePortlet.isPresent()) {
-                GWT.log("[PortletWidgetMapper] Created portlet from auto-generated factory");
+                logger.info("logger.infoCreated portlet from auto-generated factory");
                 WebProtegePortletComponents portletComponents = thePortlet.get();
                 WebProtegePortletPresenter portletPresenter = portletComponents.getPresenter();
                 viewHolder = createViewHolder(terminalNode,
@@ -115,8 +120,10 @@ private ViewHolder createViewHolder(@Nonnull TerminalNode node,
             nodePropertiesChangedHandler.accept(node);
         });
         WebProtegeEventBus eventBus = eventBusProvider.get();
+        disposables.add(eventBus);
         portletUi.setTitle(portlet.getPortletDescriptor().getTitle());
         WebProtegePortletPresenter portletPresenter = portlet.getPresenter();
+        disposables.add(portletPresenter);
         portletPresenter.start(portletUi, eventBus);
         ViewHolder viewHolder;
         if (portletPresenter instanceof HasFixedPrimaryAxisSize) {
@@ -127,10 +134,17 @@ private ViewHolder createViewHolder(@Nonnull TerminalNode node,
         }
         viewHolder.addStyleName(DROP_ZONE);
         viewHolder.addCloseHandler(event -> {
+            disposables.remove(eventBus);
             eventBus.dispose();
+            disposables.remove(portletPresenter);
             portletPresenter.dispose();
             nodeId2ViewHolderMap.remove(node.getNodeId());
         });
         return viewHolder;
     }
+
+    @Override
+    public void dispose() {
+        disposables.forEach(HasDispose::dispose);
+    }
 }
diff --git a/...nt/src/main/java/edu/stanford/bmir/protege/web/client/place/WebProtegeActivityMapper.java b/...nt/src/main/java/edu/stanford/bmir/protege/web/client/place/WebProtegeActivityMapper.java
@@ -22,6 +22,7 @@
 import edu.stanford.bmir.protege.web.client.projectsettings.ProjectSettingsActivity;
 import edu.stanford.bmir.protege.web.client.search.EntitySearchSettingsActivity;
 import edu.stanford.bmir.protege.web.client.search.EntitySearchSettingsPresenter;
+import edu.stanford.bmir.protege.web.client.selection.SelectionModel;
 import edu.stanford.bmir.protege.web.client.sharing.SharingSettingsActivity;
 import edu.stanford.bmir.protege.web.client.sharing.SharingSettingsPresenter;
 import edu.stanford.bmir.protege.web.client.signup.SignUpPresenter;
@@ -44,6 +45,7 @@
  * 12/02/16
  */
 public class WebProtegeActivityMapper implements ActivityMapper {
+
     Logger logger = Logger.getLogger("WebProtegeActivityMapper");
 
     private final ClientApplicationComponent applicationComponent;
@@ -86,9 +88,9 @@ public WebProtegeActivityMapper(LoggedInUserProvider loggedInUserProvider,
     }
 
     public void start() {
-        GWT.log("[WebProtegeActivityMapper] Started activity mapper.");
+        logger.info("Started activity mapper.");
         eventBus.addHandler(UserLoggedOutEvent.ON_USER_LOGGED_OUT, event -> {
-            GWT.log("[WebProtegeActivityMapper] User logged out.  Going to the Login Place.");
+            logger.info("User logged out.  Going to the Login Place.");
             LoginPlace loginPlace;
             Place currentPlace = placeController.getWhere();
             if (!(currentPlace instanceof LoginPlace)) {
@@ -102,16 +104,18 @@ public void start() {
     }
 
     private ClientProjectComponent getClientProjectComponentForProjectAndLoggedInUser(@Nonnull ProjectId projectId) {
+        logger.info("Getting project component for " + projectId + ".  The current project is " + currentClientProjectComponent);
         if(currentClientProjectComponent.isPresent()) {
             ClientProjectComponent projectComponent = currentClientProjectComponent.get();
             if(isProjectComponentForProject(projectComponent, projectId) && isLastUserSameAsLoggedInUser()) {
                 return projectComponent;
             }
-            projectComponent.getProjectPresenter().dispose();
+            logger.info("Disposing of project component for " + projectComponent.getProjectId());
+            projectComponent.dispose();
         }
         ClientProjectComponent nextProjectComponent = instantiateClientProjectComponent(projectId);
         // Reset project component and user
-        GWT.log("[WebProtegeActivityMapper] Instantiating new project component");
+        logger.info("Instantiating new project component");
         lastUser = Optional.of(loggedInUserProvider.getCurrentUserId());
         currentClientProjectComponent = Optional.of(nextProjectComponent);
         return nextProjectComponent;
@@ -131,10 +135,10 @@ private ClientProjectComponent instantiateClientProjectComponent(@Nonnull Projec
     }
 
     public Activity getActivity(final Place place) {
-        GWT.log("[WebProtegeActivityMapper] Map place: " + place);
+        logger.info("Map place: " + place);
         if (shouldRedirectToLogin(place)) {
-            GWT.log("[WebProtegeActivityMapper] User is not logged in.  Redirecting to login.");
-            logger.info("[WebProtegeActivityMapper] User is not logged in.  Redirecting to login.");
+            logger.info("User is not logged in.  Redirecting to login.");
+            logger.info("User is not logged in.  Redirecting to login.");
             loginPresenter.setNextPlace(place);
             Scheduler.get().scheduleFinally(() -> placeController.goTo(new LoginPlace(place)));
             return new LoginActivity(loginPresenter);
@@ -168,7 +172,7 @@ public Activity getActivity(final Place place) {
         }
         if (place instanceof LoginPlace) {
             if (!loggedInUserProvider.getCurrentUserId().isGuest()) {
-                logger.info("[WebProtegeActivityMapper] Schedule to project list after login.");
+                logger.info("Schedule to project list after login.");
 
                 Scheduler.get().scheduleFinally(() -> placeController.goTo(new ProjectListPlace()));
             }
@@ -193,7 +197,7 @@ public Activity getActivity(final Place place) {
         }
 
         if (place instanceof ProjectListPlace) {
-            logger.info("[WebProtegeActivityMapper] Route to project list activity");
+            logger.info("Route to project list activity");
             return new ProjectListActivity(projectManagerPresenter);
         }
 

diff --git a/...java/edu/stanford/bmir/protege/web/client/portlet/AbstractWebProtegePortletPresenter.java b/...java/edu/stanford/bmir/protege/web/client/portlet/AbstractWebProtegePortletPresenter.java
@@ -17,11 +17,13 @@
 import edu.stanford.bmir.protege.web.shared.lang.DisplayNameSettingsChangedEvent;
 import edu.stanford.bmir.protege.web.shared.place.ProjectViewPlace;
 import edu.stanford.bmir.protege.web.shared.project.ProjectId;
+import edu.stanford.protege.widgetmap.client.view.HasViewTitle;
 import org.semanticweb.owlapi.model.OWLEntity;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import java.util.Optional;
+import java.util.logging.Logger;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -30,6 +32,8 @@
 
 public abstract class AbstractWebProtegePortletPresenter implements WebProtegePortletPresenter, EntityDisplay {
 
+    private static final Logger logger = Logger.getLogger("AbstractWebProtegePortletPresenter");
+
     private final SelectionModel selectionModel;
 
     private final ProjectId projectId;
@@ -51,6 +55,8 @@ public abstract class AbstractWebProtegePortletPresenter implements WebProtegePo
     @Nullable
     private Place lastPlace = null;
 
+    private boolean disposed = false;
+
     public AbstractWebProtegePortletPresenter(@Nonnull SelectionModel selectionModel,
                                               @Nonnull ProjectId projectId,
                                               @Nonnull DisplayNameRenderer displayNameRenderer,
@@ -60,6 +66,7 @@ public AbstractWebProtegePortletPresenter(@Nonnull SelectionModel selectionModel
         this.projectId = checkNotNull(projectId);
         this.displayNameRenderer = displayNameRenderer;
         selectionModelHandlerRegistration = selectionModel.addSelectionChangedHandler(e -> {
+            logger.info("Handling selection changed in " + projectId + " (disposed=" + disposed  + ") in " + portletUi.map(HasViewTitle::getViewTitle).orElse("(not set)"));
                                                                                           if (trackSelection) {
                                                                                               handleBeforeSetEntity(e.getPreviousSelection());
                                                                                               handleAfterSetEntity(e.getLastSelection());
@@ -201,6 +208,8 @@ public String toString() {
 
     @Override
     public void dispose() {
+        disposed = true;
+        logger.info("Disposing of portlet " + getClass().getSimpleName() + " in project " + projectId);
         selectionModelHandlerRegistration.removeHandler();
         portletUi.ifPresent(HasDispose::dispose);
     }

diff --git a/...i-client/src/main/java/edu/stanford/bmir/protege/web/client/project/ProjectPresenter.java b/...i-client/src/main/java/edu/stanford/bmir/protege/web/client/project/ProjectPresenter.java
@@ -143,6 +143,7 @@ public void dispose() {
         linkBarPresenter.dispose();
         perspectivePresenter.dispose();
         eventPollingManager.stop();
+        eventBus.dispose();
     }
 
     @Override
-Original file line number
+Diff line change
@@ Expand Up / @@ -138,6 +138,7 @@ public void onResize() { @@
         @Override
         public void dispose() {
+            widgetMapper.dispose();
         }
@@ Expand Down @@