From aa8dc6a491fb09a4a8ec2c9e41ba2cb4b9969b07 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Wed, 8 May 2019 17:59:39 -0400 Subject: [PATCH] GT-2763 - Table Sorting - review fixes --- .../app/plugin/core/symtable/SymbolPanel.java | 34 ++++++----------- .../markuptable/VTMarkupItemsTableModel.java | 37 ++++--------------- .../gui/util/AbstractVTMatchTableModel.java | 35 ++++-------------- .../filter/AbstractPatternTextFilter.java | 33 +++++++++++------ .../widgets/filter/InvertedTextFilter.java | 6 +-- .../filter/MatchesPatternTextFilter.java | 7 +--- .../widgets/table/CombinedTableFilter.java | 6 +-- .../table/DefaultRowFilterTransformer.java | 11 ++---- .../widgets/table/InvertedTableFilter.java | 6 +-- .../table/MultiTextFilterTableFilter.java | 8 +--- .../widgets/table/TableTextFilter.java | 7 +--- .../widgets/table/threaded/AddRemoveJob.java | 8 ++-- .../widgets/table/threaded/LoadJob.java | 3 +- .../table/threaded/LoadSpecificDataJob.java | 3 +- .../widgets/table/threaded/SortJob.java | 10 ++--- .../table/threaded/TableUpdateJob.java | 4 +- .../threaded/ThreadedTableModelUpdateMgr.java | 8 ++-- 17 files changed, 81 insertions(+), 145 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolPanel.java index 7d7691ed7b..f651b9dbf9 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolPanel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolPanel.java @@ -16,7 +16,8 @@ package ghidra.app.plugin.core.symtable; import java.awt.BorderLayout; -import java.util.*; +import java.util.ArrayList; +import java.util.List; import javax.swing.*; import javax.swing.event.TableModelListener; @@ -118,7 +119,7 @@ class SymbolPanel extends JPanel { protected RowFilterTransformer updateRowDataTransformer(boolean nameOnly) { if (nameOnly) { - return new NameOnlyRowTransformer(); + return new NameOnlyRowTransformer(tableModel); } return new DefaultRowFilterTransformer<>(tableModel, symTable.getColumnModel()); @@ -200,13 +201,18 @@ class SymbolPanel extends JPanel { // Inner Classes //================================================================================================== - private class NameOnlyRowTransformer implements RowFilterTransformer { + private static class NameOnlyRowTransformer implements RowFilterTransformer { private List list = new ArrayList<>(); + private SymbolTableModel model; + + NameOnlyRowTransformer(SymbolTableModel model) { + this.model = model; + } @Override public List transform(SymbolRowObject rowObject) { list.clear(); - Symbol symbol = tableModel.getSymbolForRowObject(rowObject); + Symbol symbol = model.getSymbolForRowObject(rowObject); if (symbol != null) { list.add(symbol.getName()); } @@ -215,11 +221,8 @@ class SymbolPanel extends JPanel { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + getEnclosingInstance().hashCode(); - result = prime * result + ((list == null) ? 0 : list.hashCode()); - return result; + // not meant to put in hashing structures; the data for equals may change over time + throw new UnsupportedOperationException(); } @Override @@ -233,20 +236,7 @@ class SymbolPanel extends JPanel { if (getClass() != obj.getClass()) { return false; } - - NameOnlyRowTransformer other = (NameOnlyRowTransformer) obj; - if (!getEnclosingInstance().equals(other.getEnclosingInstance())) { - return false; - } - - if (!Objects.equals(list, other.list)) { - return false; - } return true; } - - private SymbolPanel getEnclosingInstance() { - return SymbolPanel.this; - } } } diff --git a/Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/provider/markuptable/VTMarkupItemsTableModel.java b/Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/provider/markuptable/VTMarkupItemsTableModel.java index 117880b2aa..5e6865a19b 100644 --- a/Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/provider/markuptable/VTMarkupItemsTableModel.java +++ b/Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/provider/markuptable/VTMarkupItemsTableModel.java @@ -177,7 +177,7 @@ public class VTMarkupItemsTableModel extends AddressBasedTableModel { + private static class MarkupTablePassthroughFilter implements TableFilter { private List> appliedFilters; @@ -246,40 +246,17 @@ public class VTMarkupItemsTableModel extends AddressBasedTableModel { + private static class MatchTablePassthroughFilter implements TableFilter { private List> appliedFilters; @@ -206,37 +206,16 @@ public abstract class AbstractVTMatchTableModel extends AddressBasedTableModel implements TableFilter { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((filters == null) ? 0 : filters.hashCode()); - return result; + // not meant to put in hashing structures; the data for equals may change over time + throw new UnsupportedOperationException(); } @Override diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/DefaultRowFilterTransformer.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/DefaultRowFilterTransformer.java index 6671106bd1..e56d9e9a56 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/DefaultRowFilterTransformer.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/DefaultRowFilterTransformer.java @@ -131,12 +131,8 @@ public class DefaultRowFilterTransformer implements RowFilterTransfo @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((columnData == null) ? 0 : columnData.hashCode()); - result = prime * result + ((columnModel == null) ? 0 : columnModel.hashCode()); - result = prime * result + ((model == null) ? 0 : model.hashCode()); - return result; + // not meant to put in hashing structures; the data for equals may change over time + throw new UnsupportedOperationException(); } @Override @@ -160,7 +156,8 @@ public class DefaultRowFilterTransformer implements RowFilterTransfo return false; } - if (!Objects.equals(model, other.model)) { + // use '==' so that we don't end up comparing all table data + if (model != other.model) { return false; } return true; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/InvertedTableFilter.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/InvertedTableFilter.java index 25347c05d0..d8a1dee6e5 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/InvertedTableFilter.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/InvertedTableFilter.java @@ -38,10 +38,8 @@ public class InvertedTableFilter implements TableFilter @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((filter == null) ? 0 : filter.hashCode()); - return result; + // not meant to put in hashing structures; the data for equals may change over time + throw new UnsupportedOperationException(); } @Override diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/MultiTextFilterTableFilter.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/MultiTextFilterTableFilter.java index 1ad2ba47f1..5e800bca78 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/MultiTextFilterTableFilter.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/MultiTextFilterTableFilter.java @@ -91,12 +91,8 @@ public class MultiTextFilterTableFilter implements TableFilter implements TableFilter { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((textFilter == null) ? 0 : textFilter.hashCode()); - result = prime * result + ((transformer == null) ? 0 : transformer.hashCode()); - return result; + // not meant to put in hashing structures; the data for equals may change over time + throw new UnsupportedOperationException(); } @Override diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/AddRemoveJob.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/AddRemoveJob.java index 6dcb2bffc5..0c1664a0d7 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/AddRemoveJob.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/AddRemoveJob.java @@ -29,16 +29,16 @@ public class AddRemoveJob extends TableUpdateJob { } @Override - public synchronized boolean filter() { + public synchronized boolean requestFilter() { // // This is a request to fully filter the table's data (like when the filter changes). // In this case, we had disabled 'force filter', as the sorting did not need it. // However, when the client asks to filter, make sure we filter. // - boolean jobIsStillRunning = super.filter(); - if (jobIsStillRunning) { + boolean canFilter = super.requestFilter(); + if (canFilter) { setForceFilter(true); // reset, since we had turned it off above; now we have to filter } - return jobIsStillRunning; + return canFilter; } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadJob.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadJob.java index 6bb5a2d620..39c204ce11 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadJob.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadJob.java @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +21,6 @@ public class LoadJob extends TableUpdateJob { LoadJob(ThreadedTableModel model, TaskMonitor monitor) { super(model, monitor); reload(); // set job to totally reload data; - sort(model.getSortingContext(), false); // set the comparator so the data will be sorted + requestSort(model.getSortingContext(), false); // set the comparator so the data will be sorted } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadSpecificDataJob.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadSpecificDataJob.java index bd9a0bc3b8..05dc7bc3aa 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadSpecificDataJob.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/LoadSpecificDataJob.java @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +25,6 @@ public class LoadSpecificDataJob extends TableUpdateJob { // set the comparator so the data will be sorted; always force a sort, since we just // loaded the data - sort(model.getSortingContext(), true); + requestSort(model.getSortingContext(), true); } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/SortJob.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/SortJob.java index 3c40a297a9..f1e953efb7 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/SortJob.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/SortJob.java @@ -24,20 +24,20 @@ public class SortJob extends TableUpdateJob { TableSortingContext sortingContext, boolean forceSort) { super(model, monitor); setForceFilter(false); // only sorting - sort(sortingContext, forceSort); + requestSort(sortingContext, forceSort); } @Override - public synchronized boolean filter() { + public synchronized boolean requestFilter() { // // This is a request to fully filter the table's data (like when the filter changes). // In this case, we had disabled 'force filter', as the sorting did not need it. // However, when the client asks to filter, make sure we filter. // - boolean jobIsStillRunning = super.filter(); - if (jobIsStillRunning) { + boolean canFilter = super.requestFilter(); + if (canFilter) { setForceFilter(true); // reset, since we had turned it off above; now we have to filter } - return jobIsStillRunning; + return canFilter; } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java index e4d0e72447..fd457a1a38 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java @@ -204,7 +204,7 @@ public class TableUpdateJob { * @return true if the sort can be processed by this job, false if this job is essentially already * completed and therefor cannot perform the sort job. */ - public synchronized boolean sort(TableSortingContext newSortingContext, boolean forceSort) { + public synchronized boolean requestSort(TableSortingContext newSortingContext, boolean forceSort) { if (currentState == DONE) { return false; } @@ -235,7 +235,7 @@ public class TableUpdateJob { * @return true if the filter can be processed by this job, false if this job is essentially already * completed and therefor cannot perform the filter job. */ - public synchronized boolean filter() { + public synchronized boolean requestFilter() { if (currentState == DONE) { return false; } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java index b150d89438..5852f9ad53 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java @@ -211,12 +211,12 @@ class ThreadedTableModelUpdateMgr { void sort(TableSortingContext sortingContext, boolean forceSort) { synchronized (updateManager) { if (currentJob != null && pendingJob == null && - currentJob.sort(sortingContext, forceSort)) { + currentJob.requestSort(sortingContext, forceSort)) { return; } if (pendingJob != null) { - pendingJob.sort(sortingContext, forceSort); + pendingJob.requestSort(sortingContext, forceSort); } else { pendingJob = new SortJob<>(model, monitor, sortingContext, forceSort); @@ -240,11 +240,11 @@ class ThreadedTableModelUpdateMgr { */ void filter() { synchronized (updateManager) { - if (currentJob != null && pendingJob == null && currentJob.filter()) { + if (currentJob != null && pendingJob == null && currentJob.requestFilter()) { return; } if (pendingJob != null) { - pendingJob.filter(); + pendingJob.requestFilter(); } else { pendingJob = new FilterJob<>(model, monitor);