From b6760828c11963e9b116c890e2aef00fefd8ef6a Mon Sep 17 00:00:00 2001 From: jmillman Date: Tue, 12 Apr 2016 14:34:55 -0400 Subject: [PATCH 1/4] distinguish datasource filters by datsource id and not displayname in expansion map. fix bugs in disabled and selected state of filter copies --- .../timeline/filters/AbstractFilter.java | 2 ++ .../timeline/filters/CompoundFilter.java | 9 +++-- .../timeline/filters/DataSourceFilter.java | 4 +-- .../timeline/filters/DataSourcesFilter.java | 23 +++++++------ .../timeline/filters/HashHitsFilter.java | 5 +-- .../autopsy/timeline/filters/RootFilter.java | 6 +++- .../autopsy/timeline/filters/TagsFilter.java | 5 +-- .../timeline/ui/filtering/FilterSetPanel.java | 8 ++--- .../timeline/ui/filtering/FilterTreeItem.java | 33 ++++++++++++++----- 9 files changed, 61 insertions(+), 34 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java index 6c9e1b00ed..16f7228b50 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java @@ -68,10 +68,12 @@ public abstract class AbstractFilter implements Filter { return "[" + (isSelected() ? "x" : " ") + "]"; // NON-NLS } + @Override public boolean isActive() { return activeProperty.get(); } + @Override public BooleanBinding activeProperty() { return activeProperty; } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java index 055c1c8748..140574076a 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java @@ -62,15 +62,18 @@ public abstract class CompoundFilter extends Abstr while (c.next()) { addSubFilterListeners(c.getAddedSubList()); } + setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); }); - this.subFilters.setAll(subFilters); - - this.selectedProperty().addListener(activeProperty -> { + + this.selectedProperty().addListener(activeProperty -> { getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); }); this.disabledProperty().addListener(activeProperty -> { getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); }); + + this.subFilters.setAll(subFilters); + getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); } private void addSubFilterListeners(List newSubfilters) { diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourceFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourceFilter.java index 2cd3f93d9e..c713965933 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourceFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourceFilter.java @@ -43,7 +43,7 @@ public class DataSourceFilter extends AbstractFilter { @Override synchronized public DataSourceFilter copyOf() { - DataSourceFilter filterCopy = new DataSourceFilter(getDisplayName(), getDataSourceID()); + DataSourceFilter filterCopy = new DataSourceFilter(getDataSourceName(), getDataSourceID()); filterCopy.setSelected(isSelected()); filterCopy.setDisabled(isDisabled()); return filterCopy; @@ -51,7 +51,7 @@ public class DataSourceFilter extends AbstractFilter { @Override public String getDisplayName() { - return dataSourceName; + return getDataSourceName(); } @Override diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java index baeb3b7e61..4150134aef 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java @@ -21,6 +21,7 @@ package org.sleuthkit.autopsy.timeline.filters; import java.util.function.Predicate; import java.util.stream.Collectors; import javafx.beans.binding.Bindings; +import javafx.beans.binding.BooleanBinding; import javafx.beans.value.ObservableBooleanValue; import org.openide.util.NbBundle; @@ -30,18 +31,17 @@ import org.openide.util.NbBundle; public class DataSourcesFilter extends UnionFilter { public DataSourcesFilter() { - setSelected(false); } @Override public DataSourcesFilter copyOf() { final DataSourcesFilter filterCopy = new DataSourcesFilter(); - filterCopy.setSelected(isSelected()); - filterCopy.setDisabled(isDisabled()); //add a copy of each subfilter getSubFilters().forEach(dataSourceFilter -> filterCopy.addSubFilter(dataSourceFilter.copyOf()) ); + filterCopy.setSelected(isSelected()); + filterCopy.setDisabled(isDisabled()); return filterCopy; } @@ -65,13 +65,6 @@ public class DataSourcesFilter extends UnionFilter { return string; } - public void addSubFilter(DataSourceFilter dataSourceFilter) { - super.addSubFilter(dataSourceFilter); - if (getSubFilters().size() > 1) { - setSelected(Boolean.TRUE); - } - } - @Override public boolean equals(Object obj) { if (obj == null) { @@ -100,6 +93,16 @@ public class DataSourcesFilter extends UnionFilter { return Bindings.or(super.disabledProperty(), Bindings.size(getSubFilters()).lessThanOrEqualTo(1)); } + @Override + public BooleanBinding activeProperty() { + return super.activeProperty().and(Bindings.not(disabledProperty())); + } + + @Override + public boolean isActive() { + return activeProperty().get(); + } + @Override Predicate getDuplicatePredicate(DataSourceFilter subfilter) { return dataSourcefilter -> dataSourcefilter.getDataSourceID() == subfilter.getDataSourceID(); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java index c2eb196b79..13284683cc 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java @@ -42,12 +42,13 @@ public class HashHitsFilter extends UnionFilter { @Override public HashHitsFilter copyOf() { HashHitsFilter filterCopy = new HashHitsFilter(); - filterCopy.setSelected(isSelected()); - filterCopy.setDisabled(isDisabled()); //add a copy of each subfilter this.getSubFilters().forEach(hashSetFilter -> filterCopy.addSubFilter(hashSetFilter.copyOf()) ); + filterCopy.setSelected(isSelected()); + filterCopy.setDisabled(isDisabled()); + return filterCopy; } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/RootFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/RootFilter.java index 2b31978d7b..8b26ed1c71 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/RootFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/RootFilter.java @@ -1,7 +1,7 @@ /* * Autopsy Forensic Browser * - * Copyright 2015 Basis Technology Corp. + * Copyright 2015-16 Basis Technology Corp. * Contact: carrier sleuthkit org * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -122,4 +122,8 @@ public class RootFilter extends IntersectionFilter { } }; } + + public TypeFilter getTypeFilter() { + return typeFilter; + } } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java index 973b82a19d..52c0b3b559 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java @@ -44,12 +44,13 @@ public class TagsFilter extends UnionFilter { @Override public TagsFilter copyOf() { TagsFilter filterCopy = new TagsFilter(); - filterCopy.setSelected(isSelected()); - filterCopy.setDisabled(isDisabled()); //add a copy of each subfilter getSubFilters().forEach(tagNameFilter -> filterCopy.addSubFilter(tagNameFilter.copyOf()) ); + filterCopy.setSelected(isSelected()); + filterCopy.setDisabled(isDisabled()); + return filterCopy; } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java index 760f0a9bc3..53dceb8a6f 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java @@ -1,7 +1,7 @@ /* * Autopsy Forensic Browser * - * Copyright 2013-15 Basis Technology Corp. + * Copyright 2013-16 Basis Technology Corp. * Contact: carrier sleuthkit org * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -44,12 +44,10 @@ import org.sleuthkit.autopsy.timeline.TimeLineController; import org.sleuthkit.autopsy.timeline.VisualizationMode; import org.sleuthkit.autopsy.timeline.actions.ResetFilters; import org.sleuthkit.autopsy.timeline.datamodel.FilteredEventsModel; -import org.sleuthkit.autopsy.timeline.datamodel.eventtype.RootEventType; import org.sleuthkit.autopsy.timeline.filters.AbstractFilter; import org.sleuthkit.autopsy.timeline.filters.DescriptionFilter; import org.sleuthkit.autopsy.timeline.filters.Filter; import org.sleuthkit.autopsy.timeline.filters.RootFilter; -import org.sleuthkit.autopsy.timeline.filters.TypeFilter; /** * The FXML controller for the filter ui. @@ -86,7 +84,7 @@ final public class FilterSetPanel extends BorderPane { private final FilteredEventsModel filteredEvents; private final TimeLineController controller; - private final ObservableMap expansionMap = FXCollections.observableHashMap(); + private final ObservableMap expansionMap = FXCollections.observableHashMap(); private double dividerPosition; @NbBundle.Messages({ @@ -114,7 +112,7 @@ final public class FilterSetPanel extends BorderPane { legendColumn.setCellValueFactory(cellDataFeatures -> cellDataFeatures.getValue().valueProperty()); legendColumn.setCellFactory(col -> new LegendCell(this.controller)); - expansionMap.put(new TypeFilter(RootEventType.getInstance()).getDisplayName(), true); + expansionMap.put(controller.getEventsModel().getFilter().getTypeFilter(), true); this.filteredEvents.eventTypeZoomProperty().addListener((Observable observable) -> applyFilters()); this.filteredEvents.descriptionLODProperty().addListener((Observable observable1) -> applyFilters()); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java index adc57dc646..86b030aadf 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java @@ -1,6 +1,23 @@ +/* + * Autopsy Forensic Browser + * + * Copyright 2014-16 Basis Technology Corp. + * Contact: carrier sleuthkit 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.timeline.ui.filtering; -import javafx.beans.Observable; import javafx.collections.ListChangeListener; import javafx.collections.MapChangeListener; import javafx.collections.ObservableMap; @@ -22,22 +39,20 @@ final public class FilterTreeItem extends TreeItem { * be made for them added added to the children of this * FilterTreeItem */ - public FilterTreeItem(Filter f, ObservableMap expansionMap) { + public FilterTreeItem(Filter f, ObservableMap expansionMap) { super(f); - expansionMap.addListener((MapChangeListener.Change change) -> { - if (change.getKey() == f.getDisplayName()) { + expansionMap.addListener((MapChangeListener.Change change) -> { + if (change.getKey().equals(f)) { setExpanded(expansionMap.get(change.getKey())); } }); - if (expansionMap.get(f.getDisplayName()) != null) { - setExpanded(expansionMap.get(f.getDisplayName())); + if (expansionMap.containsKey(f)) { + setExpanded(expansionMap.get(f)); } - expandedProperty().addListener((Observable observable) -> { - expansionMap.put(f.getDisplayName(), isExpanded()); - }); + expandedProperty().addListener(expandedProperty -> expansionMap.put(f, isExpanded())); if (f instanceof CompoundFilter) { CompoundFilter compoundFilter = (CompoundFilter) f; From 1076ef3d81d0eef0039793fa56d71b24a54dc3f1 Mon Sep 17 00:00:00 2001 From: jmillman Date: Fri, 15 Apr 2016 16:08:36 -0400 Subject: [PATCH 2/4] fix filter disabling bug by moving listener outside of constructor. other misc cleanup: - keep references to disabled and active properties in DataSourcesFilter so they and any listeners are not GC'd - do parallel fix to ordering of TypeFilter copyOf as was done to other CopyOf implementations --- .../timeline/filters/AbstractFilter.java | 2 +- .../timeline/filters/CompoundFilter.java | 22 ++--- .../timeline/filters/DataSourcesFilter.java | 14 ++-- .../autopsy/timeline/filters/Filter.java | 80 ++++++++++++++++--- .../autopsy/timeline/filters/TypeFilter.java | 4 +- .../timeline/ui/filtering/FilterTreeItem.java | 8 +- 6 files changed, 92 insertions(+), 38 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java index 16f7228b50..14394546ba 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/AbstractFilter.java @@ -70,7 +70,7 @@ public abstract class AbstractFilter implements Filter { @Override public boolean isActive() { - return activeProperty.get(); + return activeProperty().get(); } @Override diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java index 140574076a..51fa6decb6 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java @@ -33,9 +33,9 @@ import javafx.collections.ObservableList; * a {@link CompoundFilter} uses listeners to enforce the following * relationships between it and its sub-filters: *
    - * if a filter becomes inactive disable all of its subfilters - * if a sub-filter changes active state set the parent filter active if any - * of its sub-filters are active. + * if a compound filter becomes inactive disable all of its sub-filters + * if all of a compound filter's sub-filters become un-selected, un-select + * the compound filter. *
*/ public abstract class CompoundFilter extends AbstractFilter { @@ -57,30 +57,22 @@ public abstract class CompoundFilter extends Abstr public CompoundFilter(List subFilters) { super(); - //listen to changes in list of subfilters and add active state listener to newly added filters + //listen to changes in list of subfilters and this.subFilters.addListener((ListChangeListener.Change c) -> { - while (c.next()) { + while (c.next()) { //add active state listener to newly added filters addSubFilterListeners(c.getAddedSubList()); } setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); }); - this.selectedProperty().addListener(activeProperty -> { - getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); - }); - this.disabledProperty().addListener(activeProperty -> { - getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); - }); - this.subFilters.setAll(subFilters); - getSubFilters().forEach(subFilter -> subFilter.setDisabled(isActive() == false)); } private void addSubFilterListeners(List newSubfilters) { for (SubFilterType sf : newSubfilters) { - //if a subfilter changes active state + //if a subfilter changes selected state sf.selectedProperty().addListener((Observable observable) -> { - //set this filter acttive af any of the subfilters are active. + //set this filter selected af any of the subfilters are selected. setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); }); } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java index 4150134aef..c36868a787 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java @@ -30,7 +30,12 @@ import org.openide.util.NbBundle; */ public class DataSourcesFilter extends UnionFilter { + private final BooleanBinding activePropertyOverride; + private BooleanBinding disabledPropertyOverride; + public DataSourcesFilter() { + disabledPropertyOverride = Bindings.or(super.disabledProperty(), Bindings.size(getSubFilters()).lessThanOrEqualTo(1)); + activePropertyOverride = super.activeProperty().and(Bindings.not(disabledProperty())); } @Override @@ -90,17 +95,12 @@ public class DataSourcesFilter extends UnionFilter { @Override public ObservableBooleanValue disabledProperty() { - return Bindings.or(super.disabledProperty(), Bindings.size(getSubFilters()).lessThanOrEqualTo(1)); + return disabledPropertyOverride; } @Override public BooleanBinding activeProperty() { - return super.activeProperty().and(Bindings.not(disabledProperty())); - } - - @Override - public boolean isActive() { - return activeProperty().get(); + return activePropertyOverride; } @Override diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/Filter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/Filter.java index 435998a183..92e04dcf81 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/Filter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/Filter.java @@ -25,11 +25,16 @@ import javafx.collections.FXCollections; import javafx.collections.ObservableList; /** - * Interface for Filters + * Interface for Filters. Filters are given to the EventDB who interpretes them + * a appropriately for all db queries. Since the filters are primarily + * configured in the UI, this interface provides selected, disabled and active + * (selected and not disabled) properties. */ public interface Filter { /** + * get a filter that is the intersection of the given filters + * * @param filters a set of filters to intersect * * @return a filter that is the intersection of the given filters @@ -39,50 +44,101 @@ public interface Filter { } /** + * get a filter that is the intersection of the given filters + * * @param filters a set of filters to intersect * * @return a filter that is the intersection of the given filters */ public static IntersectionFilter intersect(Filter[] filters) { - return new IntersectionFilter<>(FXCollections.observableArrayList(filters)); + return intersect(FXCollections.observableArrayList(filters)); } /** - * since filters have mutable state (active) and are observed in various - * places, we need a mechanism to copy the current state to keep in history. + * since filters have mutable state (selected/disabled/active) and are + * observed in various places, we need a mechanism to copy the current state + * to keep in the history. * - * Concrete subtasks should implement this in a way that preserves the - * active state and any subfilters. + * Concrete sub classes should implement this in a way that preserves the + * state and any sub-filters. * * @return a copy of this filter. */ Filter copyOf(); + /** + * get the display name of this filter + * + * @return a name for this filter to show in the UI + */ String getDisplayName(); + /** + * get a representation of this filter (and it's state) as a HTML string + * + * @return a html representation of this filter + */ String getHTMLReportString(); + /** + * get an Ascii representation of this filter's selected state: ie [x] for + * selected or [ ] for not selected + * + * @return an Ascii representation of this filter's selected state + */ String getStringCheckBox(); + /** + * is this filter selected + * + * @return true if this filter is selected + */ boolean isSelected(); - void setSelected(Boolean act); + /** + * set this filter selected + * + * @param selected true to selecte, false to un-select + */ + void setSelected(Boolean selected); + /** + * observable selected property + * + * @return the observable selected property for this filter + */ SimpleBooleanProperty selectedProperty(); - /* - * TODO: disabled state only affects the state of the checkboxes in the ui - * and not the actual filters and shouldn't be implemented here, but it was - * too hard to figure out how it should be implemented without intruding on - * the ui-ignorant filters + /** + * set the filter disabled */ void setDisabled(Boolean act); + /** + * observable disabled property + * + * @return the observable disabled property for this filter + */ ObservableBooleanValue disabledProperty(); + /** + * is this filter disabled + * + * @return true if this filter is disabled + */ boolean isDisabled(); + /** + * is this filter active (selected and not disabled) + * + * @return true if this filter is active + */ boolean isActive(); + /** + * observable active property + * + * @return the observable active property for this filter + */ BooleanBinding activeProperty(); } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java index d3dc05985e..9af52e99fb 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java @@ -101,13 +101,13 @@ public class TypeFilter extends UnionFilter { public TypeFilter copyOf() { //make a nonrecursive copy of this filter final TypeFilter filterCopy = new TypeFilter(eventType, false); - filterCopy.setSelected(isSelected()); - filterCopy.setDisabled(isDisabled()); //add a copy of each subfilter getSubFilters().forEach(typeFilter -> filterCopy.addSubFilter(typeFilter.copyOf(), comparator) ); + filterCopy.setSelected(isSelected()); + filterCopy.setDisabled(isDisabled()); return filterCopy; } diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java index 86b030aadf..ba06bfd404 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java @@ -55,7 +55,7 @@ final public class FilterTreeItem extends TreeItem { expandedProperty().addListener(expandedProperty -> expansionMap.put(f, isExpanded())); if (f instanceof CompoundFilter) { - CompoundFilter compoundFilter = (CompoundFilter) f; + final CompoundFilter compoundFilter = (CompoundFilter) f; for (Filter subFilter : compoundFilter.getSubFilters()) { getChildren().add(new FilterTreeItem(subFilter, expansionMap)); @@ -69,6 +69,12 @@ final public class FilterTreeItem extends TreeItem { } } }); + + compoundFilter.activeProperty().addListener(activeProperty -> { + compoundFilter.getSubFilters().forEach(subFilter -> subFilter.setDisabled(compoundFilter.isActive() == false)); + }); + } + } } From ff3810df09edbec09b74bd323706193f558ca39e Mon Sep 17 00:00:00 2001 From: jmillman Date: Fri, 15 Apr 2016 16:40:25 -0400 Subject: [PATCH 3/4] comments and minor cleanup --- .../timeline/filters/CompoundFilter.java | 35 ++++++---------- .../timeline/filters/DataSourcesFilter.java | 10 ++--- .../timeline/filters/HashHitsFilter.java | 5 +-- .../autopsy/timeline/filters/TagsFilter.java | 5 +-- .../autopsy/timeline/filters/TypeFilter.java | 6 +-- .../timeline/ui/filtering/FilterSetPanel.java | 5 +++ .../timeline/ui/filtering/FilterTreeItem.java | 42 +++++++++++-------- 7 files changed, 53 insertions(+), 55 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java index 51fa6decb6..285e2186d6 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/CompoundFilter.java @@ -20,7 +20,6 @@ package org.sleuthkit.autopsy.timeline.filters; import java.util.List; import java.util.Objects; -import javafx.beans.Observable; import javafx.collections.FXCollections; import javafx.collections.ListChangeListener; import javafx.collections.ObservableList; @@ -31,12 +30,8 @@ import javafx.collections.ObservableList; * implementations can decide how to combine the sub-filters. * * a {@link CompoundFilter} uses listeners to enforce the following - * relationships between it and its sub-filters: - *
    - * if a compound filter becomes inactive disable all of its sub-filters - * if all of a compound filter's sub-filters become un-selected, un-select - * the compound filter. - *
+ * relationships between it and its sub-filters: if all of a compound filter's + * sub-filters become un-selected, un-select the compound filter. */ public abstract class CompoundFilter extends AbstractFilter { @@ -57,27 +52,23 @@ public abstract class CompoundFilter extends Abstr public CompoundFilter(List subFilters) { super(); - //listen to changes in list of subfilters and - this.subFilters.addListener((ListChangeListener.Change c) -> { - while (c.next()) { //add active state listener to newly added filters - addSubFilterListeners(c.getAddedSubList()); + //listen to changes in list of subfilters + this.subFilters.addListener((ListChangeListener.Change change) -> { + while (change.next()) { + //add a listener to the selected property of each added subfilter + change.getAddedSubList().forEach(addedSubFilter -> { + //if a subfilter's selected property changes... + addedSubFilter.selectedProperty().addListener(selectedProperty -> { + //set this compound filter selected af any of the subfilters are selected. + setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); + }); + }); } - setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); }); this.subFilters.setAll(subFilters); } - private void addSubFilterListeners(List newSubfilters) { - for (SubFilterType sf : newSubfilters) { - //if a subfilter changes selected state - sf.selectedProperty().addListener((Observable observable) -> { - //set this filter selected af any of the subfilters are selected. - setSelected(getSubFilters().parallelStream().anyMatch(Filter::isSelected)); - }); - } - } - static boolean areSubFiltersEqual(final CompoundFilter oneFilter, final CompoundFilter otherFilter) { if (oneFilter.getSubFilters().size() != otherFilter.getSubFilters().size()) { return false; diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java index c36868a787..3395982ad7 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/DataSourcesFilter.java @@ -30,21 +30,21 @@ import org.openide.util.NbBundle; */ public class DataSourcesFilter extends UnionFilter { + //keep references to the overridden properties so they don't get GC'd private final BooleanBinding activePropertyOverride; - private BooleanBinding disabledPropertyOverride; + private final BooleanBinding disabledPropertyOverride; public DataSourcesFilter() { disabledPropertyOverride = Bindings.or(super.disabledProperty(), Bindings.size(getSubFilters()).lessThanOrEqualTo(1)); - activePropertyOverride = super.activeProperty().and(Bindings.not(disabledProperty())); + activePropertyOverride = super.activeProperty().and(Bindings.not(disabledPropertyOverride)); } @Override public DataSourcesFilter copyOf() { final DataSourcesFilter filterCopy = new DataSourcesFilter(); //add a copy of each subfilter - getSubFilters().forEach(dataSourceFilter -> - filterCopy.addSubFilter(dataSourceFilter.copyOf()) - ); + getSubFilters().forEach(dataSourceFilter -> filterCopy.addSubFilter(dataSourceFilter.copyOf())); + //these need to happen after the listeners fired by adding the subfilters filterCopy.setSelected(isSelected()); filterCopy.setDisabled(isDisabled()); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java index 13284683cc..ba2791ac1e 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/HashHitsFilter.java @@ -43,9 +43,8 @@ public class HashHitsFilter extends UnionFilter { public HashHitsFilter copyOf() { HashHitsFilter filterCopy = new HashHitsFilter(); //add a copy of each subfilter - this.getSubFilters().forEach(hashSetFilter -> - filterCopy.addSubFilter(hashSetFilter.copyOf()) - ); + this.getSubFilters().forEach(hashSetFilter -> filterCopy.addSubFilter(hashSetFilter.copyOf())); + //these need to happen after the listeners fired by adding the subfilters filterCopy.setSelected(isSelected()); filterCopy.setDisabled(isDisabled()); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java index 52c0b3b559..7d23c49916 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/TagsFilter.java @@ -45,9 +45,8 @@ public class TagsFilter extends UnionFilter { public TagsFilter copyOf() { TagsFilter filterCopy = new TagsFilter(); //add a copy of each subfilter - getSubFilters().forEach(tagNameFilter -> - filterCopy.addSubFilter(tagNameFilter.copyOf()) - ); + getSubFilters().forEach(tagNameFilter -> filterCopy.addSubFilter(tagNameFilter.copyOf())); + //these need to happen after the listeners fired by adding the subfilters filterCopy.setSelected(isSelected()); filterCopy.setDisabled(isDisabled()); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java b/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java index 9af52e99fb..b897791985 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/filters/TypeFilter.java @@ -102,10 +102,8 @@ public class TypeFilter extends UnionFilter { //make a nonrecursive copy of this filter final TypeFilter filterCopy = new TypeFilter(eventType, false); //add a copy of each subfilter - getSubFilters().forEach(typeFilter -> - filterCopy.addSubFilter(typeFilter.copyOf(), comparator) - ); - + getSubFilters().forEach(typeFilter -> filterCopy.addSubFilter(typeFilter.copyOf(), comparator)); + //these need to happen after the listeners fired by adding the subfilters filterCopy.setSelected(isSelected()); filterCopy.setDisabled(isDisabled()); return filterCopy; diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java index 53dceb8a6f..8512c3f819 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterSetPanel.java @@ -84,6 +84,10 @@ final public class FilterSetPanel extends BorderPane { private final FilteredEventsModel filteredEvents; private final TimeLineController controller; + /** + * map from filter to its expansion state in the ui, used to restore the + * expansion state as we navigate back and forward in the history + */ private final ObservableMap expansionMap = FXCollections.observableHashMap(); private double dividerPosition; @@ -112,6 +116,7 @@ final public class FilterSetPanel extends BorderPane { legendColumn.setCellValueFactory(cellDataFeatures -> cellDataFeatures.getValue().valueProperty()); legendColumn.setCellFactory(col -> new LegendCell(this.controller)); + //type is the only filter expanded initialy expansionMap.put(controller.getEventsModel().getFilter().getTypeFilter(), true); this.filteredEvents.eventTypeZoomProperty().addListener((Observable observable) -> applyFilters()); diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java index ba06bfd404..59368eeab6 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java @@ -31,50 +31,56 @@ import org.sleuthkit.autopsy.timeline.filters.Filter; final public class FilterTreeItem extends TreeItem { /** - * recursively construct a tree of treeitems to parallel the filter tree of + * recursively construct a tree of TreeItems to parallel the filter tree of * the given filter * * - * @param f the filter for this item. if f has sub-filters, tree items will - * be made for them added added to the children of this - * FilterTreeItem + * @param filter the filter for this item. if f has sub-filters, tree items + * will be made for them added added to the children of this + * FilterTreeItem */ - public FilterTreeItem(Filter f, ObservableMap expansionMap) { - super(f); + public FilterTreeItem(Filter filter, ObservableMap expansionMap) { + super(filter); + //listen to changes in the expansion map, and update expansion state of filter object expansionMap.addListener((MapChangeListener.Change change) -> { - if (change.getKey().equals(f)) { + if (change.getKey().equals(filter)) { setExpanded(expansionMap.get(change.getKey())); } }); - if (expansionMap.containsKey(f)) { - setExpanded(expansionMap.get(f)); + if (expansionMap.containsKey(filter)) { + setExpanded(expansionMap.get(filter)); } - expandedProperty().addListener(expandedProperty -> expansionMap.put(f, isExpanded())); + //keep expanion map upto date if user expands/collapses filter + expandedProperty().addListener(expandedProperty -> expansionMap.put(filter, isExpanded())); - if (f instanceof CompoundFilter) { - final CompoundFilter compoundFilter = (CompoundFilter) f; + //if the filter is a compound filter, add its subfilters to the tree + if (filter instanceof CompoundFilter) { + final CompoundFilter compoundFilter = (CompoundFilter) filter; - for (Filter subFilter : compoundFilter.getSubFilters()) { - getChildren().add(new FilterTreeItem(subFilter, expansionMap)); - } + //add all sub filters + compoundFilter.getSubFilters().forEach(subFilter -> getChildren().add(new FilterTreeItem(subFilter, expansionMap))); + //listen to changes in sub filters and keep tree in sync compoundFilter.getSubFilters().addListener((ListChangeListener.Change c) -> { while (c.next()) { for (Filter subfFilter : c.getAddedSubList()) { - setExpanded(true); + setExpanded(true); //emphasize new filters by expanding parent to make sure they are visible getChildren().add(new FilterTreeItem(subfFilter, expansionMap)); } } }); + /* + * enforce the following relationship between a compound filter and + * its subfilters: if a compound filter's active property changes, + * disable the subfilters if the compound filter is not active. + */ compoundFilter.activeProperty().addListener(activeProperty -> { compoundFilter.getSubFilters().forEach(subFilter -> subFilter.setDisabled(compoundFilter.isActive() == false)); }); - } - } } From ddcf547f22b01ea79678b45e1eda9f71424cd3fb Mon Sep 17 00:00:00 2001 From: jmillman Date: Fri, 15 Apr 2016 16:54:43 -0400 Subject: [PATCH 4/4] set initial disabled state of sub filters based on active state of compound filter --- .../timeline/ui/filtering/FilterTreeItem.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java index 59368eeab6..58be2a2554 100644 --- a/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java +++ b/Core/src/org/sleuthkit/autopsy/timeline/ui/filtering/FilterTreeItem.java @@ -79,8 +79,20 @@ final public class FilterTreeItem extends TreeItem { * disable the subfilters if the compound filter is not active. */ compoundFilter.activeProperty().addListener(activeProperty -> { - compoundFilter.getSubFilters().forEach(subFilter -> subFilter.setDisabled(compoundFilter.isActive() == false)); + disableSubFiltersIfNotActive(compoundFilter); }); + + disableSubFiltersIfNotActive(compoundFilter); } } + + /** + * disable the sub-filters of the given compound filter if it is not active + * + * @param compoundFilter the compound filter + */ + static private void disableSubFiltersIfNotActive(CompoundFilter compoundFilter) { + boolean inactive = compoundFilter.isActive() == false; + compoundFilter.getSubFilters().forEach(subFilter -> subFilter.setDisabled(inactive)); + } }