fix categorization ui update errors

- remove old listeners when updating treecell item.  cleanup GroupTreeCell
- invalidateHashSetHitsCount before adding/removing files to make sure it is seen during listner involation
- cleanup GroupTreeItem and NavPanel
- add comment to GroupManager
This commit is contained in:
jmillman 2015-05-27 13:30:25 -04:00
parent 09aa1b243e
commit b04fa81b53
5 changed files with 208 additions and 216 deletions

View File

@ -25,14 +25,12 @@ import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import org.sleuthkit.autopsy.coreutils.Logger;
import org.sleuthkit.autopsy.imagegallery.ImageGalleryController;
import org.sleuthkit.datamodel.BlackboardArtifact;
import org.sleuthkit.datamodel.TskCoreException;
/**
* Represents a set of image/video files in a group. The UI listens to changes
* to the group membership and updates itself accordingly.
*/
public class DrawableGroup implements Comparable<DrawableGroup>{
public class DrawableGroup implements Comparable<DrawableGroup> {
private static final Logger LOGGER = Logger.getLogger(DrawableGroup.class.getName());
@ -69,7 +67,7 @@ public class DrawableGroup implements Comparable<DrawableGroup>{
* Call to indicate that an image has been added or removed from the group,
* so the hash counts may not longer be accurate.
*/
synchronized public void invalidateHashSetHitsCount(){
synchronized public void invalidateHashSetHitsCount() {
filesWithHashSetHitsCount = -1;
}
@ -80,7 +78,7 @@ public class DrawableGroup implements Comparable<DrawableGroup>{
for (Long fileID : fileIds()) {
try {
if(ImageGalleryController.getDefault().getDatabase().isInHashSet(fileID)){
if (ImageGalleryController.getDefault().getDatabase().isInHashSet(fileID)) {
filesWithHashSetHitsCount++;
}
} catch (IllegalStateException | NullPointerException ex) {
@ -117,20 +115,20 @@ public class DrawableGroup implements Comparable<DrawableGroup>{
}
synchronized public void addFile(Long f) {
invalidateHashSetHitsCount();
if (fileIDs.contains(f) == false) {
fileIDs.add(f);
}
invalidateHashSetHitsCount();
}
synchronized public void removeFile(Long f) {
fileIDs.removeAll(f);
invalidateHashSetHitsCount();
fileIDs.removeAll(f);
}
// By default, sort by group key name
@Override
public int compareTo(DrawableGroup other){
public int compareTo(DrawableGroup other) {
return this.groupKey.getValueDisplayName().compareTo(other.groupKey.getValueDisplayName());
}
}

View File

@ -283,7 +283,9 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
group.removeFile(fileID);
// If we're grouping by category, we don't want to remove empty groups.
if(! group.groupKey.getValueDisplayName().startsWith("CAT-")){
if (group.groupKey.getValueDisplayName().startsWith("CAT-")) {
return;
} else {
if (group.fileIds().isEmpty()) {
synchronized (groupMap) {
groupMap.remove(groupKey, group);
@ -456,7 +458,7 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
}
return values;
} catch(TskCoreException ex){
} catch (TskCoreException ex) {
LOGGER.log(Level.WARNING, "TSK error getting list of type " + groupBy.getDisplayName());
return new ArrayList<A>();
}
@ -520,8 +522,11 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
* Count the number of files with the given category.
* This is faster than getFileIDsWithCategory and should be used if only the
* counts are needed and not the file IDs.
*
* @param category Category to match against
*
* @return Number of files with the given category
*
* @throws TskCoreException
*/
public int countFilesWithCategory(Category category) throws TskCoreException {
@ -581,7 +586,7 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
*/
public <A extends Comparable<A>> void regroup(final DrawableAttribute<A> groupBy, final GroupSortBy sortBy, final SortOrder sortOrder, Boolean force) {
if(! Case.isCaseOpen()){
if (!Case.isCaseOpen()) {
return;
}
@ -613,15 +618,11 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
}
}
/**
* an executor to submit async ui related background tasks to.
*/
final ExecutorService regroupExecutor = Executors.newSingleThreadExecutor(new BasicThreadFactory.Builder().namingPattern("ui task -%d").build());
public ReadOnlyDoubleProperty regroupProgress() {
return regroupProgress.getReadOnlyProperty();
}
@ -630,6 +631,8 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
* handle {@link FileUpdateEvent} sent from Db when files are
* inserted/updated
*
* TODO: why isn't this just two methods!
*
* @param evt
*/
@Override
@ -646,7 +649,7 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
DrawableGroup g = getGroupForKey(gk);
if (g == null){
if (g == null) {
// It may be that this was the last unanalyzed file in the group, so test
// whether the group is now fully analyzed.
//TODO: use method in groupmanager ?
@ -654,8 +657,7 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
if (checkAnalyzed != null) { // => the group is analyzed, so add it to the ui
populateAnalyzedGroup(gk, checkAnalyzed);
}
}
else{
} else {
g.invalidateHashSetHitsCount();
}
}
@ -698,13 +700,13 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
}
}
}
Category.fireChange(fileIDs);
if (evt.getChangedAttribute() == DrawableAttribute.CATEGORY) {
Category.fireChange(fileIDs);
}
if (evt.getChangedAttribute() == DrawableAttribute.TAGS) {
TagUtils.fireChange(fileIDs);
}
break;
}
}
@ -789,7 +791,7 @@ public class GroupManager implements FileUpdateEvent.FileUpdateListener {
Collections.sort(groups, sortBy.getGrpComparator(sortOrder));
// Officially add all groups in order
for(DrawableGroup g:groups){
for (DrawableGroup g : groups) {
populateAnalyzedGroup(g, ReGroupTask.this);
}

View File

@ -1,8 +1,28 @@
/*
* Autopsy Forensic Browser
*
* Copyright 2013-15 Basis Technology Corp.
* Contact: carrier <at> sleuthkit <dot> org
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.sleuthkit.autopsy.imagegallery.gui.navpanel;
import static java.util.Objects.isNull;
import java.util.Optional;
import javafx.application.Platform;
import javafx.beans.InvalidationListener;
import javafx.beans.Observable;
import javafx.scene.Node;
import javafx.scene.control.OverrunStyle;
import javafx.scene.control.Tooltip;
import javafx.scene.control.TreeCell;
@ -13,103 +33,95 @@ import org.sleuthkit.autopsy.imagegallery.datamodel.DrawableAttribute;
import org.sleuthkit.autopsy.imagegallery.grouping.DrawableGroup;
/**
* A {@link Node} in the tree that listens to its associated group. Manages
* visual representation of TreeNode in Tree. Listens to properties of group
* that don't impact hierarchy and updates ui to reflect them
* A cell in the NavPanel tree that listens to its associated group's fileids.
* Manages visual representation of TreeNode in Tree. Listens to properties of
* group that don't impact hierarchy and updates ui to reflect them
*/
class GroupTreeCell extends TreeCell<TreeNode> {
/**
* icon to use if this cell's TreeNode doesn't represent a group but just a
* folder(with no DrawableFiles) in the hierarchy.
* folder(with no DrawableFiles) in the file system hierarchy.
*/
private static final Image EMPTY_FOLDER_ICON = new Image("org/sleuthkit/autopsy/imagegallery/images/folder.png");
/**
* reference to listener that allows us to remove it from a group when a new
* group is assigned to this Cell
*/
private InvalidationListener listener;
public GroupTreeCell() {
//TODO: move this to .css file
//adjust indent, default is 10 which uses up a lot of space.
setStyle("-fx-indent:5;");
//since end of path is probably more interesting put ellipsis at front
setTextOverrun(OverrunStyle.LEADING_ELLIPSIS);
Platform.runLater(() -> {
prefWidthProperty().bind(getTreeView().widthProperty().subtract(15));
});
}
@Override
synchronized protected void updateItem(final TreeNode tNode, boolean empty) {
super.updateItem(tNode, empty);
prefWidthProperty().bind(getTreeView().widthProperty().subtract(15));
protected synchronized void updateItem(final TreeNode tNode, boolean empty) {
//if there was a previous group, remove the listener
Optional.ofNullable(getItem())
.map(TreeNode::getGroup)
.ifPresent((DrawableGroup t) -> {
t.fileIds().removeListener(listener);
});
if (tNode == null || empty) {
super.updateItem(tNode, empty);
if (isNull(tNode) || empty) {
Platform.runLater(() -> {
setTooltip(null);
setText(null);
setGraphic(null);
});
} else {
final String name = StringUtils.defaultIfBlank(tNode.getPath(), DrawableGroup.UNKNOWN);
Platform.runLater(() -> {
setTooltip(new Tooltip(name));
});
final String groupName = StringUtils.defaultIfBlank(tNode.getPath(), DrawableGroup.UNKNOWN);
if (tNode.getGroup() == null) {
if (isNull(tNode.getGroup())) {
//"dummy" group in file system tree <=> a folder with no drawables
Platform.runLater(() -> {
setText(name);
setTooltip(new Tooltip(groupName));
setText(groupName);
setGraphic(new ImageView(EMPTY_FOLDER_ICON));
});
} else {
//if number of files in this group changes (eg file is recategorized), update counts
tNode.getGroup().fileIds().addListener((Observable o) -> {
listener = (Observable o) -> {
final String countsText = getCountsText();
Platform.runLater(() -> {
String groupName; // The "name" variable set earlier is generally not correct at this point
if((getItem() == null) || (getItem().getGroup() == null) ||
(getItem().getGroup().groupKey == null)){
groupName = "";
}
else{
groupName = getItem().getGroup().groupKey.getValueDisplayName();
}
setText(groupName + " (" + getNumerator() + getDenominator() + ")");
setText(groupName + countsText);
});
});
};
//if number of files in this group changes (eg file is recategorized), update counts via listener
tNode.getGroup().fileIds().addListener(listener);
//... and use icon corresponding to group type
final Image icon = tNode.getGroup().groupKey.getAttribute().getIcon();
final String countsText = getCountsText();
Platform.runLater(() -> {
//this TreeNode has a group so append counts to name ...
setText(name + " (" + getNumerator() + getDenominator() + ")");
//... and use icon corresponding to group type
setGraphic(new ImageView(tNode.getGroup().groupKey.getAttribute().getIcon()));
setTooltip(new Tooltip(groupName));
setGraphic(new ImageView(icon));
setText(groupName + countsText);
});
}
}
}
/**
* @return the Numerator of the count to append to the group name = number
* of hashset hits + "/"
*/
synchronized private String getNumerator() {
try {
final String numerator = (getItem().getGroup().groupKey.getAttribute() != DrawableAttribute.HASHSET)
? getItem().getGroup().getFilesWithHashSetHitsCount() + "/"
: "";
return numerator;
} catch (NullPointerException e) {
//instead of this try catch block, remove the listener when assigned a null treeitem / group
return "";
}
}
private synchronized String getCountsText() {
final String counts = Optional.ofNullable(getItem())
.map(TreeNode::getGroup)
.map((DrawableGroup t) -> {
return " (" + ((t.groupKey.getAttribute() == DrawableAttribute.HASHSET)
? Integer.toString(t.getSize())
: t.getFilesWithHashSetHitsCount() + "/" + t.getSize()) + ")";
}).orElse(""); //if item is null or group is null
/**
* @return the Denominator of the count to append to the group name = number
* of files in group
*/
synchronized private Integer getDenominator() {
try {
return getItem().getGroup().getSize();
} catch (NullPointerException ex) {
return 0;
}
return counts;
}
}

View File

@ -37,10 +37,28 @@ import org.sleuthkit.autopsy.imagegallery.grouping.DrawableGroup;
*/
class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeItem> {
static GroupTreeItem getTreeItemForGroup(GroupTreeItem root, DrawableGroup grouping) {
if (Objects.equals(root.getValue().getGroup(), grouping)) {
return root;
} else {
synchronized (root.getChildren()) {
for (TreeItem<TreeNode> child : root.getChildren()) {
final GroupTreeItem childGTI = (GroupTreeItem) child;
GroupTreeItem val = getTreeItemForGroup(childGTI, grouping);
if (val != null) {
return val;
}
}
}
}
return null;
}
/**
* maps a path segment to the child item of this item with that path segment
*/
private Map<String, GroupTreeItem> childMap = new HashMap<>();
private final Map<String, GroupTreeItem> childMap = new HashMap<>();
/**
* the comparator if any used to sort the children of this item
*/
@ -68,7 +86,7 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
* Recursive method to add a grouping at a given path.
*
* @param path Full path (or subset not yet added) to add
* @param g Group to add
* @param g Group to add
* @param tree True if it is part of a tree (versus a list)
*/
void insert(String path, DrawableGroup g, Boolean tree) {
@ -88,12 +106,9 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
prefixTreeItem = newTreeItem;
childMap.put(prefix, prefixTreeItem);
Platform.runLater(new Runnable() {
@Override
public void run() {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
}
Platform.runLater(() -> {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
}
});
@ -109,14 +124,11 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
newTreeItem.setExpanded(true);
childMap.put(path, newTreeItem);
Platform.runLater(new Runnable() {
@Override
public void run() {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
if (comp != null) {
FXCollections.sort(getChildren(), comp);
}
Platform.runLater(() -> {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
if (comp != null) {
FXCollections.sort(getChildren(), comp);
}
}
});
@ -129,7 +141,7 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
* Recursive method to add a grouping at a given path.
*
* @param path Full path (or subset not yet added) to add
* @param g Group to add
* @param g Group to add
* @param tree True if it is part of a tree (versus a list)
*/
void insert(List<String> path, DrawableGroup g, Boolean tree) {
@ -147,12 +159,9 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
prefixTreeItem = newTreeItem;
childMap.put(prefix, prefixTreeItem);
Platform.runLater(new Runnable() {
@Override
public void run() {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
}
Platform.runLater(() -> {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
}
});
@ -169,14 +178,11 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
newTreeItem.setExpanded(true);
childMap.put(path.get(0), newTreeItem);
Platform.runLater(new Runnable() {
@Override
public void run() {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
if (comp != null) {
FXCollections.sort(getChildren(), comp);
}
Platform.runLater(() -> {
synchronized (getChildren()) {
getChildren().add(newTreeItem);
if (comp != null) {
FXCollections.sort(getChildren(), comp);
}
}
});
@ -189,24 +195,6 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
return comp.compare(this, o);
}
static GroupTreeItem getTreeItemForGroup(GroupTreeItem root, DrawableGroup grouping) {
if (Objects.equals(root.getValue().getGroup(), grouping)) {
return root;
} else {
synchronized (root.getChildren()) {
for (TreeItem<TreeNode> child : root.getChildren()) {
final GroupTreeItem childGTI = (GroupTreeItem) child;
GroupTreeItem val = getTreeItemForGroup(childGTI, grouping);
if (val != null) {
return val;
}
}
}
}
return null;
}
GroupTreeItem getTreeItemForPath(List<String> path) {
// end of recursion
if (path.isEmpty()) {
@ -253,4 +241,5 @@ class GroupTreeItem extends TreeItem<TreeNode> implements Comparable<GroupTreeIt
ti.resortChildren(comp);
}
}
}

View File

@ -1,7 +1,7 @@
/*
* Autopsy Forensic Browser
*
* Copyright 2013 Basis Technology Corp.
* Copyright 2013-15 Basis Technology Corp.
* Contact: carrier <at> sleuthkit <dot> org
*
* Licensed under the Apache License, Version 2.0 (the "License");
@ -20,7 +20,6 @@ package org.sleuthkit.autopsy.imagegallery.gui.navpanel;
import java.net.URL;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
import java.util.ResourceBundle;
import javafx.application.Platform;
@ -41,23 +40,21 @@ import javafx.scene.control.TreeView;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import org.apache.commons.lang3.StringUtils;
import org.openide.util.Exceptions;
import org.sleuthkit.autopsy.coreutils.ThreadConfined;
import org.sleuthkit.autopsy.coreutils.ThreadConfined.ThreadType;
import org.sleuthkit.autopsy.imagegallery.FXMLConstructor;
import org.sleuthkit.autopsy.imagegallery.ImageGalleryController;
import org.sleuthkit.autopsy.imagegallery.datamodel.DrawableAttribute;
import org.sleuthkit.autopsy.imagegallery.datamodel.DrawableFile;
import org.sleuthkit.autopsy.imagegallery.grouping.DrawableGroup;
import org.sleuthkit.autopsy.imagegallery.grouping.GroupKey;
import org.sleuthkit.autopsy.imagegallery.grouping.GroupSortBy;
import org.sleuthkit.autopsy.imagegallery.grouping.GroupViewState;
import org.sleuthkit.datamodel.TskCoreException;
/**
* Display two trees. one shows all folders (groups) and calls out folders with
* images. the user can select folders with images to see them in the main
* GroupPane The other shows folders with hash set hits.
*
* //TODO: there is too much code duplication between the navTree and the
* hashTree. Extract the common code to some new class.
*/
public class NavPanel extends TabPane {
@ -170,24 +167,15 @@ public class NavPanel extends TabPane {
removeFromNavTree(g);
removeFromHashTree(g);
}
if(change.wasPermutated()){
if (change.wasPermutated()) {
// Handle this afterward
wasPermuted = true;
}
}
if(wasPermuted){
// Remove everything and add it again in the new order
for(DrawableGroup g:controller.getGroupManager().getAnalyzedGroups()){
removeFromNavTree(g);
removeFromHashTree(g);
}
for(DrawableGroup g:controller.getGroupManager().getAnalyzedGroups()){
insertIntoNavTree(g);
if (g.getFilesWithHashSetHitsCount() > 0) {
insertIntoHashTree(g);
}
}
if (wasPermuted) {
rebuildTrees();
}
});
@ -205,6 +193,27 @@ public class NavPanel extends TabPane {
});
}
private void rebuildTrees() {
navTreeRoot = new GroupTreeItem("", null, sortByBox.getSelectionModel().selectedItemProperty().get());
hashTreeRoot = new GroupTreeItem("", null, sortByBox.getSelectionModel().selectedItemProperty().get());
ObservableList<DrawableGroup> groups = controller.getGroupManager().getAnalyzedGroups();
for (DrawableGroup g : groups) {
insertIntoNavTree(g);
if (g.getFilesWithHashSetHitsCount() > 0) {
insertIntoHashTree(g);
}
}
Platform.runLater(() -> {
navTree.setRoot(navTreeRoot);
navTreeRoot.setExpanded(true);
hashTree.setRoot(hashTreeRoot);
hashTreeRoot.setExpanded(true);
});
}
private void updateControllersGroup() {
final TreeItem<TreeNode> selectedItem = activeTreeProperty.get().getSelectionModel().getSelectedItem();
if (selectedItem != null && selectedItem.getValue() != null && selectedItem.getValue().getGroup() != null) {
@ -216,11 +225,6 @@ public class NavPanel extends TabPane {
hashTreeRoot.resortChildren(sortByBox.getSelectionModel().getSelectedItem());
}
private void insertIntoHashTree(DrawableGroup g) {
initHashTree();
hashTreeRoot.insert(g.groupKey.getValueDisplayName(), g, false);
}
/**
* Set the tree to the passed in group
*
@ -235,26 +239,28 @@ public class NavPanel extends TabPane {
if (treeItemForGroup != null) {
/* When we used to run the below code on the FX thread, it would
* get into infinite loops when the next group button was pressed quickly
* because the udpates became out of order and History could not keep
* track of what was current. Currently (4/2/15), this method is
* already on the FX thread, so it is OK. */
* get into infinite loops when the next group button was pressed
* quickly because the udpates became out of order and History could
* not
* keep track of what was current.
*
* Currently (4/2/15), this method is already on the FX thread, so
* it is OK. */
//Platform.runLater(() -> {
TreeItem<TreeNode> ti = treeItemForGroup;
while (ti != null) {
ti.setExpanded(true);
ti = ti.getParent();
}
int row = activeTreeProperty.get().getRow(treeItemForGroup);
if (row != -1) {
activeTreeProperty.get().getSelectionModel().select(treeItemForGroup);
activeTreeProperty.get().scrollTo(row);
}
//});
TreeItem<TreeNode> ti = treeItemForGroup;
while (ti != null) {
ti.setExpanded(true);
ti = ti.getParent();
}
int row = activeTreeProperty.get().getRow(treeItemForGroup);
if (row != -1) {
activeTreeProperty.get().getSelectionModel().select(treeItemForGroup);
activeTreeProperty.get().scrollTo(row);
}
//}); //end Platform.runLater
}
}
@SuppressWarnings("fallthrough")
private static List<String> groupingToPath(DrawableGroup g) {
if (g.groupKey.getAttribute() == DrawableAttribute.PATH) {
@ -269,11 +275,14 @@ public class NavPanel extends TabPane {
}
}
private void insertIntoHashTree(DrawableGroup g) {
initHashTree();
hashTreeRoot.insert(groupingToPath(g), g, false);
}
private void insertIntoNavTree(DrawableGroup g) {
initNavTree();
List<String> path = groupingToPath(g);
navTreeRoot.insert(path, g, true);
navTreeRoot.insert(groupingToPath(g), g, true);
}
private void removeFromNavTree(DrawableGroup g) {
@ -313,22 +322,4 @@ public class NavPanel extends TabPane {
});
}
}
//these are not used anymore, but could be usefull at some point
//TODO: remove them or find a use and undeprecate
@Deprecated
private void rebuildNavTree() {
navTreeRoot = new GroupTreeItem("", null, sortByBox.getSelectionModel().selectedItemProperty().get());
ObservableList<DrawableGroup> groups = controller.getGroupManager().getAnalyzedGroups();
for (DrawableGroup g : groups) {
insertIntoNavTree(g);
}
Platform.runLater(() -> {
navTree.setRoot(navTreeRoot);
navTreeRoot.setExpanded(true);
});
}
}