ImageGalleryConmtroller listener management changes

ImageGallery/src/org/sleuthkit/autopsy/imagegallery/ImageGalleryController.java

@@ -90,10 +90,11 @@ import org.sleuthkit.datamodel.TskCoreException;
 import org.sleuthkit.datamodel.TskData;
 
 /**
- * This class is responsible for the controller role in an MVC pattern
+ * Instances of this class are responsible for fulfilling the controller role in
- * implementation where the model is the drawables database for the case plus
+ * an MVC pattern implementation where the model is the drawables database for a
- * the image gallery tables in the case database, and the view is the image
+ * case plus the image gallery tables in the case database, and the view is the
- * gallery top component. There is a per case Singleton instance of this class.
+ * image gallery top component. The controller, the model, and the child
+ * components of the view change every time a new case is opened.
  */
 public final class ImageGalleryController {
 
@@ -114,26 +115,17 @@ public final class ImageGalleryController {
     );
 
     /*
-     * There is Singleton instance of this class per case. It is created during
+     * There is an instance of this class per case. It is created during the
-     * the opening of case resources and destroyed during the closing of case
+     * opening of case resources and destroyed during the closing of case
      * resources.
      */
-    private static final Object controllerLock = new Object();
+    private static final Object controllersByCaseLock = new Object();
-    @GuardedBy("controllerLock")
+    @GuardedBy("controllersByCaseLock")
     private static final Map<String, ImageGalleryController> controllersByCase = new HashMap<>();
 
     /**
-     * A flag that controls whether or not the controller is handling various
+     * A flag that controls whether or not the image gallery controller is
-     * application events in "real time." Set to true by default. If the flag is
+     * handling various application events. Set to true by default.
-     * not set then:
-     *
-     * - All ingest module events are ignored.
-     *
-     * - Data source added events are ignored.
-     *
-     * -
-     * RJCTODO: Finish this RJCTODO: Why is this perceived as speeding up
-     * ingest?
      */
     private final SimpleBooleanProperty listeningEnabled;
 
@@ -167,11 +159,12 @@ public final class ImageGalleryController {
      *
      * @param theCase The case.
      *
-     * @throws TskCoreException If there is an issue creating/opening the model
+     * @throws TskCoreException If there is an issue creating/opening a local
-     *                          for the case.
+     *                          drawables database for the case or the image
+     *                          gallery tables in the case database.
      */
     static void createController(Case theCase) throws TskCoreException {
-        synchronized (controllerLock) {
+        synchronized (controllersByCaseLock) {
             if (!controllersByCase.containsKey(theCase.getName())) {
                 ImageGalleryController controller = new ImageGalleryController(theCase);
                 controller.startUp();
@@ -185,29 +178,30 @@ public final class ImageGalleryController {
      *
      * @param theCase The case.
      *
-     * @return The image gallery controller or null if it does not exist.
+     * @return The controller or null if it does not exist.
      */
     public static ImageGalleryController getController(Case theCase) {
-        synchronized (controllerLock) {
+        synchronized (controllersByCaseLock) {
             return controllersByCase.get(theCase.getName());
         }
     }
 
     /**
-     * Shuts down the image gallery controller for a case. The controller will
+     * Shuts down the image gallery controller for a case. The controller closes
-     * close the model for the case.
+     * the model for the case: a local drawables database and the image gallery
+     * tables in the case database.
      *
      * @param theCase The case.
      */
     static void shutDownController(Case theCase) {
         ImageGalleryController controller = null;
-        synchronized (controllerLock) {
+        synchronized (controllersByCaseLock) {
             if (controllersByCase.containsKey(theCase.getName())) {
                 controller = controllersByCase.remove(theCase.getName());
             }
-        }
+            if (controller != null) {
-        if (controller != null) {
+                controller.shutDown();
-            controller.shutDown();
+            }
         }
     }
 
@@ -256,30 +250,59 @@ public final class ImageGalleryController {
     }
 
     /**
+     * Sets a flag indicating whether the model is "stale" for any data source
+     * in the current case. The model is a local drawables database and the
+     * image gallery tables in the case database.
      *
-     * @param b True if any data source in the case is stale
+     * @param isStale True if the model is "stale" for any data source in the
+     *                current case.
      */
     @ThreadConfined(type = ThreadConfined.ThreadType.ANY)
-    void setCaseStale(Boolean b) {
+    void setCaseStale(Boolean isStale) {
         Platform.runLater(() -> {
-            isCaseStale.set(b);
+            isCaseStale.set(isStale);
         });
     }
 
+    /**
+     * Gets the boolean property that is set to true if the model is "stale" for
+     * any data source in the current case. The model is a local drawables
+     * database and the image gallery tables in the case database.
+     *
+     * @return The property that is set to true if the model is "stale" for any
+     *         data source in the current case.
+     */
     public ReadOnlyBooleanProperty staleProperty() {
         return isCaseStale.getReadOnlyProperty();
     }
 
     /**
+     * Gets the state of the flag that is set if the Model is "stale" for any
+     * data source in the case. The model is a local drawables database and the
+     * image gallery tables in the case database.
      *
-     * @return true if any data source in the case is stale
+     * @return True if the model is "stale" for any data source in the current
+     *         case.
      */
     @ThreadConfined(type = ThreadConfined.ThreadType.JFX)
     boolean isCaseStale() {
         return isCaseStale.get();
     }
 
-    ImageGalleryController(@Nonnull Case newCase) throws TskCoreException {
+    /**
+     * Constructs an object that is responsible for fulfilling the controller
+     * role in an MVC pattern implementation where the model is the drawables
+     * database for a case plus the image gallery tables in the case database,
+     * and the view is the image gallery top component. The controller, the
+     * model, and the child components of the view change every time a new case
+     * is opened.
+     *
+     * @param theCase The case.
+     *
+     * @throws TskCoreException If there is an error constructing the
+     *                          controller.
+     */
+    ImageGalleryController(@Nonnull Case theCase) throws TskCoreException {
         listeningEnabled = new SimpleBooleanProperty(false);
         isCaseStale = new ReadOnlyBooleanWrapper(false);
         metaDataCollapsed = new ReadOnlyBooleanWrapper(false);
@@ -288,9 +311,9 @@ public final class ImageGalleryController {
         dbTaskQueueSize = new ReadOnlyIntegerWrapper(0);
         historyManager = new History<>();
         undoManager = new UndoRedoManager();
-        autopsyCase = Objects.requireNonNull(newCase);
+        autopsyCase = Objects.requireNonNull(theCase);
-        sleuthKitCase = newCase.getSleuthkitCase();
+        sleuthKitCase = theCase.getSleuthkitCase();
-        setListeningEnabled(ImageGalleryModule.isEnabledforCase(newCase));
+        setListeningEnabled(ImageGalleryModule.isEnabledforCase(theCase));
         caseEventListener = new CaseEventListener();
         ingestJobEventListener = new IngestJobEventListener();
         ingestModuleEventListener = new IngestModuleEventListener();
@@ -301,7 +324,9 @@ public final class ImageGalleryController {
         thumbnailCache = new ThumbnailCache(this);
 
         /*
-         * These two lines need to be executed in this order. RJCTODO: Why?
+         * The next two lines need to be executed in this order.
+         *
+         * RJCTODO: Why?
          */
         groupManager = new GroupManager(this);
         drawableDB = DrawableDB.getDrawableDB(this);
@@ -316,12 +341,13 @@ public final class ImageGalleryController {
         dbExecutor = getNewDBExecutor();
 
         /*
-         * Add a listener for changes to the Image Gallery enabled property that
+         * Add a listener for changes to the flag property for listening to
-         * is set by a user via the options panel. For single-user cases, the
+         * application events. The property is set by the user via the options
-         * listener queues drawables database rebuild tasks if the drawables
+         * panel. For single-user cases, the listener queues drawables database
-         * database for the current case is stale. For multi-user cases, thw
+         * rebuild tasks if the drawables database for the current case is
-         * listener does nothing, because rebuilding the drawables database is
+         * stale. For multi-user cases, the listener does nothing, because
-         * deferred until the Image Gallery tool is opened.
+         * rebuilding the drawables database is deferred until the Image Gallery
+         * tool is opened.
          */
         listeningEnabled.addListener((observable, wasPreviouslyEnabled, isEnabled) -> {
             try {
@@ -337,30 +363,22 @@ public final class ImageGalleryController {
 
         /*
          * Add a listener for changes to the view state property that clears the
-         * current selection and flush the undo/redo history.
+         * current selection and flushes the undo/redo history.
          */
         viewStateProperty().addListener((Observable observable) -> {
             selectionModel.clearSelection();
             undoManager.clear();
         });
 
-        /*
-         * Add a listener for ingest manager ingest module and ingest job events
-         * that enables/disables regrouping based on the drawables database task
-         * queue size and whether or not ingest is running. Note that execution
-         * of this logic needs to be dispatched to the JFX thread since the
-         * listener's event handler will be invoked in the ingest manager's
-         * event publishing thread.
-         */
-        PropertyChangeListener ingestEventHandler = propertyChangeEvent -> Platform.runLater(this::updateRegroupDisabled);
-        IngestManager ingestManager = IngestManager.getInstance();
-        ingestManager.addIngestModuleEventListener(ingestEventHandler);
-        ingestManager.addIngestJobEventListener(ingestEventHandler);
-
         /*
          * Add a listener to the size of the drawables database task queue that
          * enables/disables regrouping based on the drawables database task
          * queue size and whether or not ingest is running.
+         *
+         * RJCTODO: Why do we need to call updateRegroupDisabled both if the
+         * drawables database task queue size changes and if an ingest job or
+         * ingest module application event is published? Look at the two ingest
+         * event listeners to see the other places we call this method.
          */
         dbTaskQueueSize.addListener(obs -> this.updateRegroupDisabled());
 
@@ -719,6 +737,19 @@ public final class ImageGalleryController {
 
         @Override
         public void propertyChange(PropertyChangeEvent event) {
+            /*
+             * For all ingest module events, call a method that enables/disables
+             * regrouping based on the drawables database task queue size and
+             * whether or not ingest is running.
+             *
+             * RJCTODO: Why do we need to call updateRegroupDisabled both if the
+             * drawables database task queue size changes and if an ingest job
+             * or ingest module application event is published? Look at the
+             * IngestJobEventListener and look at the the startUp method to see
+             * where we add a listener to the drawables database task queue.
+             */
+            Platform.runLater(ImageGalleryController.this::updateRegroupDisabled);
+
             if (isListeningEnabled() == false) {
                 return;
             }
@@ -775,11 +806,7 @@ public final class ImageGalleryController {
         public void propertyChange(PropertyChangeEvent event) {
             Case.Events eventType = Case.Events.valueOf(event.getPropertyName());
             if (eventType == Case.Events.CURRENT_CASE) {
-                if (event.getOldValue() != null) {
+                if (event.getOldValue() != null) { // Case closed event
-                    /*
-                     * The old value is set, then the CURRENT_CASE event is a
-                     * case closed event.
-                     */
                     SwingUtilities.invokeLater(ImageGalleryTopComponent::closeTopComponent);
                 }
             } else {
@@ -827,6 +854,19 @@ public final class ImageGalleryController {
         })
         @Override
         public void propertyChange(PropertyChangeEvent event) {
+            /*
+             * For all ingest job events, call a method that enables/disables
+             * regrouping based on the drawables database task queue size and
+             * whether or not ingest is running.
+             *
+             * RJCTODO: Why do we need to call updateRegroupDisabled both if the
+             * drawables database task queue size changes and if an ingest job
+             * or ingest module application event is published? Look at the
+             * IngestModuleEventListener and look at the the startUp method to
+             * see where we add a listener to the drawables database task queue.
+             */
+            Platform.runLater(ImageGalleryController.this::updateRegroupDisabled);
+
             /*
              * Only handling data source analysis events.
              */

ImageGallery/src/org/sleuthkit/autopsy/imagegallery/ImageGalleryTopComponent.java

@@ -25,7 +25,9 @@ import java.util.Optional;
 import java.util.logging.Level;
 import java.util.stream.Collectors;
 import javafx.application.Platform;
-import javafx.beans.Observable;
+import javafx.beans.value.ChangeListener;
+import javafx.beans.value.ObservableValue;
+import javafx.collections.ListChangeListener;
 import javafx.concurrent.Task;
 import javafx.embed.swing.JFXPanel;
 import javafx.geometry.Insets;
@@ -49,7 +51,6 @@ import javafx.scene.layout.StackPane;
 import javafx.scene.layout.VBox;
 import javafx.scene.paint.Color;
 import javafx.stage.Modality;
-import javax.annotation.concurrent.GuardedBy;
 import javax.swing.JComponent;
 import javax.swing.SwingUtilities;
 import static org.apache.commons.collections4.CollectionUtils.isNotEmpty;
@@ -68,6 +69,7 @@ import org.sleuthkit.autopsy.coreutils.Logger;
 import org.sleuthkit.autopsy.coreutils.ThreadConfined;
 import org.sleuthkit.autopsy.directorytree.ExternalViewerShortcutAction;
 import org.sleuthkit.autopsy.imagegallery.datamodel.DrawableAttribute;
+import org.sleuthkit.autopsy.imagegallery.datamodel.grouping.DrawableGroup;
 import org.sleuthkit.autopsy.imagegallery.datamodel.grouping.GroupManager;
 import org.sleuthkit.autopsy.imagegallery.gui.DataSourceCell;
 import org.sleuthkit.autopsy.imagegallery.gui.GuiUtils;
@@ -101,13 +103,15 @@ import org.sleuthkit.datamodel.TskCoreException;
 public final class ImageGalleryTopComponent extends TopComponent implements ExplorerManager.Provider, Lookup.Provider {
 
     private static final long serialVersionUID = 1L;
-    private final static String PREFERRED_ID = "ImageGalleryTopComponent"; // NON-NLS
+    private static final String PREFERRED_ID = "ImageGalleryTopComponent"; // NON-NLS
     private static final Logger logger = Logger.getLogger(ImageGalleryTopComponent.class.getName());
 
     private final ExplorerManager em = new ExplorerManager();
     private final Lookup lookup = (ExplorerUtils.createLookup(em, getActionMap()));
 
     private volatile ImageGalleryController controller;
+    private volatile ControllerListener controllerListener;
+    private volatile GroupManagerListener groupManagerListener;
 
     private SplitPane splitPane;
     private StackPane centralStack;
@@ -157,7 +161,7 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
         if (topComponent.isOpened()) {
             showTopComponent();
         } else {
-            topComponent.getCurrentControllerAndOpen();
+            topComponent.openForCurrentCase();
         }
     }
 
@@ -220,8 +224,10 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
      */
     public static void closeTopComponent() {
         // RJCTODO: Could add the flag that used to be used for the busy wait on 
-        // the initial JavaFX thread task to avoid superfluous construction here. 
+        // the initial JavaFX thread task to avoid superfluous construction here.
-        getTopComponent().close();
+        ImageGalleryTopComponent topComponent = getTopComponent();
+        topComponent.closeForCurrentCase();
+        topComponent.close();
     }
 
     /**
@@ -236,9 +242,9 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
     }
 
     /**
-     * Gets the current controller, allows the user to select the data sources
+     * Gets the controller for the current case, allows the user to select the
-     * for which images are to be displayed and opens the top component's
+     * data sources for which images are to be displayed and opens the top
-     * window.
+     * component's window.
      *
      * @throws TskCoreException If there is an error getting the current
      *                          controller.
@@ -248,7 +254,7 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
         "ImageGalleryTopComponent.chooseDataSourceDialog.contentText=Data source:",
         "ImageGalleryTopComponent.chooseDataSourceDialog.all=All",
         "ImageGalleryTopComponent.chooseDataSourceDialog.titleText=Image Gallery",})
-    private void getCurrentControllerAndOpen() throws TskCoreException {
+    private void openForCurrentCase() throws TskCoreException {
         Case currentCase = Case.getCurrentCase();
         ImageGalleryController currentController = ImageGalleryController.getController(currentCase);
 
@@ -308,12 +314,15 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
                     splitPane.setDividerPositions(0.1, 1.0);
 
                     /*
-                     * Set up for a call to checkForGroups to happen whenever
+                     * Set up listeners to update the UI when the controller's
-                     * the controller's regrouping disabled property or the
+                     * grouping enabled/disabled property changes or the
-                     * group manager's analyzed groups property changes.
+                     * contents of the group managers list of drawable groups
+                     * changes.
                      */
-                    controller.regroupDisabledProperty().addListener((Observable unused) -> Platform.runLater(() -> checkForAnalyzedGroupsForCurrentGroupBy()));
+                    controllerListener = new ControllerListener();
-                    controller.getGroupManager().getAnalyzedGroupsForCurrentGroupBy().addListener((Observable unused) -> Platform.runLater(() -> checkForAnalyzedGroupsForCurrentGroupBy()));
+                    controller.regroupDisabledProperty().addListener(controllerListener);
+                    groupManagerListener = new GroupManagerListener();
+                    controller.getGroupManager().getAnalyzedGroupsForCurrentGroupBy().addListener(groupManagerListener);
 
                     /*
                      * Dispatch a later task to call check for groups. Note that
@@ -383,6 +392,11 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
         });
     }
 
+     void closeForCurrentCase() {
+        controller.regroupDisabledProperty().removeListener(controllerListener);
+        controller.getGroupManager().getAnalyzedGroupsForCurrentGroupBy().removeListener(groupManagerListener);
+    }
+
     /**
      * This method is called from within the constructor to initialize the form.
      * WARNING: Do NOT modify this code. The content of this method is always
@@ -440,6 +454,8 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
      * manager and removes the blocking progress spinner if there are analyzed
      * groups; otherwise adds a blocking progress spinner with an appropriate
      * message.
+     *
+     * RJCTODO: Is this an accurate method description?
      */
     @ThreadConfined(type = ThreadConfined.ThreadType.JFX)
     @NbBundle.Messages({
@@ -538,4 +554,37 @@ public final class ImageGalleryTopComponent extends TopComponent implements Expl
             setOpacity(.4);
         }
     }
+
+    /**
+     * Instances of this class are used to listen for changes to the
+     * controller's grouping enabled property. If the value of the property
+     * changes, a call to the top component's
+     * checkForAnalyzedGroupsForCurrentGroupBy method is queued for the JavaFX
+     * thread.
+     */
+    private class ControllerListener implements ChangeListener<Boolean> {
+
+        @Override
+        public void changed(ObservableValue<? extends Boolean> observable, Boolean oldValue, Boolean newValue) {
+            Platform.runLater(() -> checkForAnalyzedGroupsForCurrentGroupBy());
+        }
+
+    }
+
+    /**
+     * Instances of this class are used to listen for changes to the group
+     * manager's list of drawable groups for the user's currently selected
+     * "group by" choice. If the contents of the list change, a call to the top
+     * component's checkForAnalyzedGroupsForCurrentGroupBy method is queued for
+     * the JavaFX thread.
+     */
+    private class GroupManagerListener implements ListChangeListener<DrawableGroup> {
+
+        @Override
+        public void onChanged(Change<? extends DrawableGroup> c) {
+            Platform.runLater(() -> checkForAnalyzedGroupsForCurrentGroupBy());
+        }
+
+    }
+
 }