GT-2763 - Table Sorting - review fixes

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<SymbolRowObject> 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<SymbolRowObject> {
+	private static class NameOnlyRowTransformer implements RowFilterTransformer<SymbolRowObject> {
 		private List<String> list = new ArrayList<>();
+		private SymbolTableModel model;
+
+		NameOnlyRowTransformer(SymbolTableModel model) {
+			this.model = model;
+		}
 
 		@Override
 		public List<String> 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;
+			// not meant to put in hashing structures; the data for equals may change over time
-			int result = 1;
+			throw new UnsupportedOperationException();
-			result = prime * result + getEnclosingInstance().hashCode();
-			result = prime * result + ((list == null) ? 0 : list.hashCode());
-			return result;
 		}
 
 		@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;
-		}
 	}
 }

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/provider/markuptable/VTMarkupItemsTableModel.java

@@ -177,7 +177,7 @@ public class VTMarkupItemsTableModel extends AddressBasedTableModel<VTMarkupItem
 // Inner Classes
 //==================================================================================================   
 
-	private class MarkupTablePassthroughFilter implements TableFilter<VTMarkupItem> {
+	private static class MarkupTablePassthroughFilter implements TableFilter<VTMarkupItem> {
 
 		private List<Filter<VTMarkupItem>> appliedFilters;
 
@@ -246,40 +246,17 @@ public class VTMarkupItemsTableModel extends AddressBasedTableModel<VTMarkupItem
 
 		@Override
 		public int hashCode() {
-			final int prime = 31;
+			// not meant to put in hashing structures; the data for equals changes
-			int result = 1;
+			throw new UnsupportedOperationException();
-			result = prime * result + getEnclosingInstance().hashCode();
-			result = prime * result + ((appliedFilters == null) ? 0 : appliedFilters.hashCode());
-			return result;
 		}
 
 		@Override
 		public boolean equals(Object obj) {
-			if (this == obj) {
+			// For now we don't support equals(); if this filter gets re-created, 
-				return true;
+			// then the table must be re-filtered.  If we decide to implement this method, then 
-			}
+			// we must also implement equals() on the filters used by this filter.
-			if (obj == null) {
+			return this == obj;
-				return false;
-			}
-			if (getClass() != obj.getClass()) {
-				return false;
-			}
-
-			MarkupTablePassthroughFilter other = (MarkupTablePassthroughFilter) obj;
-			if (!getEnclosingInstance().equals(other.getEnclosingInstance())) {
-				return false;
-			}
-
-			if (!Objects.equals(appliedFilters, other.appliedFilters)) {
-				return false;
-			}
-			return true;
 		}
-
-		private VTMarkupItemsTableModel getEnclosingInstance() {
-			return VTMarkupItemsTableModel.this;
-		}
-
 	}
 
 	// column for selecting/editing?

Ghidra/Features/VersionTracking/src/main/java/ghidra/feature/vt/gui/util/AbstractVTMatchTableModel.java

@@ -137,7 +137,7 @@ public abstract class AbstractVTMatchTableModel extends AddressBasedTableModel<V
 //==================================================================================================
 // Inner Classes
 //==================================================================================================
-	private class MatchTablePassthroughFilter implements TableFilter<VTMatch> {
+	private static class MatchTablePassthroughFilter implements TableFilter<VTMatch> {
 
 		private List<Filter<VTMatch>> appliedFilters;
 
@@ -206,37 +206,16 @@ public abstract class AbstractVTMatchTableModel extends AddressBasedTableModel<V
 
 		@Override
 		public int hashCode() {
-			final int prime = 31;
+			// not meant to put in hashing structures; the data for equals changes
-			int result = 1;
+			throw new UnsupportedOperationException();
-			result = prime * result + getEnclosingInstance().hashCode();
-			result = prime * result + ((appliedFilters == null) ? 0 : appliedFilters.hashCode());
-			return result;
 		}
 
 		@Override
 		public boolean equals(Object obj) {
-			if (this == obj) {
+			// For now we don't support equals(); if this filter gets re-created, 
-				return true;
+			// then the table must be re-filtered.  If we decide to implement this method, then 
-			}
+			// we must also implement equals() on the filters used by this filter.
-			if (obj == null) {
+			return this == obj;
-				return false;
-			}
-			if (getClass() != obj.getClass()) {
-				return false;
-			}
-
-			MatchTablePassthroughFilter other = (MatchTablePassthroughFilter) obj;
-			if (!getEnclosingInstance().equals(other.getEnclosingInstance())) {
-				return false;
-			}
-			if (!Objects.equals(appliedFilters, other.appliedFilters)) {
-				return false;
-			}
-			return true;
-		}
-
-		private AbstractVTMatchTableModel getEnclosingInstance() {
-			return AbstractVTMatchTableModel.this;
 		}
 	}
 

Ghidra/Framework/Docking/src/main/java/docking/widgets/filter/AbstractPatternTextFilter.java

@@ -70,11 +70,8 @@ public abstract class AbstractPatternTextFilter implements TextFilter {
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((filterPattern == null) ? 0 : filterPattern.hashCode());
-		result = prime * result + ((filterText == null) ? 0 : filterText.hashCode());
-		return result;
 	}
 
 	@Override
@@ -90,12 +87,10 @@ public abstract class AbstractPatternTextFilter implements TextFilter {
 		}
 
 		AbstractPatternTextFilter other = (AbstractPatternTextFilter) obj;
-
+		if (!patternsEqual(createPattern(), other.createPattern())) {
-		String myPattern = getPatternString();
-		String otherPattern = other.getPatternString();
-		if (!myPattern.equals(otherPattern)) {
 			return false;
 		}
+
 		if (!Objects.equals(filterText, other.filterText)) {
 			return false;
 		}
@@ -103,8 +98,24 @@ public abstract class AbstractPatternTextFilter implements TextFilter {
 		return true;
 	}
 
-	private String getPatternString() {
+	private boolean patternsEqual(Pattern p1, Pattern p2) {
-		return filterPattern == null ? "" : filterPattern.pattern();
+		String myPattern = getPatternString(p1);
+		String otherPattern = getPatternString(p2);
+		if (!myPattern.equals(otherPattern)) {
+			return false;
+		}
+
+		int f1 = getPatternFlags(p1);
+		int f2 = getPatternFlags(p2);
+		return f1 == f2;
+	}
+
+	private String getPatternString(Pattern p) {
+		return p == null ? "" : p.pattern();
+	}
+
+	private int getPatternFlags(Pattern p) {
+		return p == null ? -1 : p.flags();
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/filter/InvertedTextFilter.java

@@ -43,10 +43,8 @@ public class InvertedTextFilter implements TextFilter {
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((filter == null) ? 0 : filter.hashCode());
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/filter/MatchesPatternTextFilter.java

@@ -40,11 +40,8 @@ public abstract class MatchesPatternTextFilter extends AbstractPatternTextFilter
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = super.hashCode();
+		throw new UnsupportedOperationException();
-		result = prime * result + (allowGlobbing ? 1231 : 1237);
-		result = prime * result + (caseSensitive ? 1231 : 1237);
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/CombinedTableFilter.java

@@ -103,10 +103,8 @@ public class CombinedTableFilter<T> implements TableFilter<T> {
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((filters == null) ? 0 : filters.hashCode());
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/DefaultRowFilterTransformer.java

@@ -131,12 +131,8 @@ public class DefaultRowFilterTransformer<ROW_OBJECT> implements RowFilterTransfo
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		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;
 	}
 
 	@Override
@@ -160,7 +156,8 @@ public class DefaultRowFilterTransformer<ROW_OBJECT> 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;

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/InvertedTableFilter.java

@@ -38,10 +38,8 @@ public class InvertedTableFilter<ROW_OBJECT> implements TableFilter<ROW_OBJECT>
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((filter == null) ? 0 : filter.hashCode());
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/MultiTextFilterTableFilter.java

@@ -91,12 +91,8 @@ public class MultiTextFilterTableFilter<ROW_OBJECT> implements TableFilter<ROW_O
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((evalMode == null) ? 0 : evalMode.hashCode());
-		result = prime * result + ((filters == null) ? 0 : filters.hashCode());
-		result = prime * result + ((transformer == null) ? 0 : transformer.hashCode());
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/TableTextFilter.java

@@ -59,11 +59,8 @@ public class TableTextFilter<ROW_OBJECT> implements TableFilter<ROW_OBJECT> {
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
+		// not meant to put in hashing structures; the data for equals may change over time
-		int result = 1;
+		throw new UnsupportedOperationException();
-		result = prime * result + ((textFilter == null) ? 0 : textFilter.hashCode());
-		result = prime * result + ((transformer == null) ? 0 : transformer.hashCode());
-		return result;
 	}
 
 	@Override

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/AddRemoveJob.java

@@ -29,16 +29,16 @@ public class AddRemoveJob<T> extends TableUpdateJob<T> {
 	}
 
 	@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();
+		boolean canFilter = super.requestFilter();
-		if (jobIsStillRunning) {
+		if (canFilter) {
 			setForceFilter(true); // reset, since we had turned it off above; now we have to filter
 		}
-		return jobIsStillRunning;
+		return canFilter;
 	}
 }

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<T> extends TableUpdateJob<T> {
 	LoadJob(ThreadedTableModel<T, ?> 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
 	}
 }

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<T> extends TableUpdateJob<T> {
 
 		// 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);
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/SortJob.java

@@ -24,20 +24,20 @@ public class SortJob<T> extends TableUpdateJob<T> {
 			TableSortingContext<T> 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();
+		boolean canFilter = super.requestFilter();
-		if (jobIsStillRunning) {
+		if (canFilter) {
 			setForceFilter(true); // reset, since we had turned it off above; now we have to filter
 		}
-		return jobIsStillRunning;
+		return canFilter;
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/TableUpdateJob.java

@@ -204,7 +204,7 @@ public class TableUpdateJob<T> {
 	 * @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<T> newSortingContext, boolean forceSort) {
+	public synchronized boolean requestSort(TableSortingContext<T> newSortingContext, boolean forceSort) {
 		if (currentState == DONE) {
 			return false;
 		}
@@ -235,7 +235,7 @@ public class TableUpdateJob<T> {
 	 * @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;
 		}

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModelUpdateMgr.java

@@ -211,12 +211,12 @@ class ThreadedTableModelUpdateMgr<T> {
 	void sort(TableSortingContext<T> 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<T> {
 	 */
 	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);