diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functionwindow/FunctionWindowProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functionwindow/FunctionWindowProvider.java index a0172e27e2..274d48bc47 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functionwindow/FunctionWindowProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functionwindow/FunctionWindowProvider.java @@ -18,15 +18,17 @@ package ghidra.app.plugin.core.functionwindow; import java.awt.BorderLayout; import java.awt.Dimension; import java.awt.event.MouseEvent; +import java.util.HashSet; +import java.util.Set; import javax.swing.*; -import javax.swing.table.JTableHeader; +import javax.swing.table.*; import docking.ActionContext; import ghidra.app.services.GoToService; import ghidra.framework.plugintool.ComponentProviderAdapter; -import ghidra.program.model.listing.Function; -import ghidra.program.model.listing.Program; +import ghidra.program.model.address.Address; +import ghidra.program.model.listing.*; import ghidra.program.util.ProgramSelection; import ghidra.util.HelpLocation; import ghidra.util.table.*; @@ -161,16 +163,44 @@ public class FunctionWindowProvider extends ComponentProviderAdapter { } private void setFunctionTableRenderer() { - functionTable.getColumnModel() - .getColumn(FunctionTableModel.LOCATION_COL) - .setPreferredWidth( - FunctionTableModel.LOCATION_COL_WIDTH); + TableColumnModel columnModel = functionTable.getColumnModel(); + TableColumn column = columnModel.getColumn(FunctionTableModel.LOCATION_COL); + column.setPreferredWidth(FunctionTableModel.LOCATION_COL_WIDTH); } void update(Function function) { - if (isVisible()) { - functionModel.update(function); + if (!isVisible()) { + return; } + + Set functions = getRelatedFunctions(function); + for (Function f : functions) { + functionModel.update(f); + } + } + + /** + * Gathers this function and any functions that thunk it + * @param f the function + * @return the related functions + */ + private Set getRelatedFunctions(Function f) { + + Program program = f.getProgram(); + FunctionManager functionManager = program.getFunctionManager(); + Set functions = new HashSet<>(); + Address[] addresses = f.getFunctionThunkAddresses(true); + if (addresses != null) { + for (Address a : addresses) { + Function thunk = functionManager.getFunctionAt(a); + if (thunk != null) { + functions.add(thunk); + } + } + } + + functions.add(f); + return functions; } void functionAdded(Function function) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableModel.java index aaaf5990b6..162d891241 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableModel.java @@ -60,6 +60,8 @@ class SymbolTableModel extends AddressBasedTableModel { private ReferenceManager refMgr; private Symbol lastSymbol; private SymbolFilter filter; + + // TODO this can be removed after GP-2030 is finished private TableAddRemoveStrategy deletedDbObjectAddRemoveStrategy = new SymbolTableAddRemoveStrategy(); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/CoalescingAddRemoveStrategy.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/CoalescingAddRemoveStrategy.java new file mode 100644 index 0000000000..bb4f22d314 --- /dev/null +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/CoalescingAddRemoveStrategy.java @@ -0,0 +1,264 @@ +/* ### + * IP: GHIDRA + * + * 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 docking.widgets.table; + +import static docking.widgets.table.AddRemoveListItem.Type.*; + +import java.util.*; + +import docking.widgets.table.threaded.*; +import ghidra.util.exception.CancelledException; +import ghidra.util.task.TaskMonitor; + +/** + * The {@link ThreadedTableModel} does not correctly function with data that can change outside of + * the table. For example, if a table uses db objects as row objects, these db objects can be + * changed by the user and by analysis while table has already been loaded. The problem with this + * is that the table's sort can be broken when new items are to be added, removed or re-inserted, + * as this process requires a binary search, which will be broken if the criteria used to sort the + * data has changed. Effectively, a row object change can break the binary search if that item + * stays in a previously sorted position, but has updated data that would put the symbol in a new + * position if sorted again. For example, if the table is sorted on name and the name of an item + * changes, then future uses of the binary search will be broken while that item is still in the + * position that matches its old name. + *

+ * This issue has been around for quite some time. To completely fix this issue, each row object + * of the table would need to be immutable, at least on the sort criteria. We could fix this in + * the future if the *mostly correct* sorting behavior is not good enough. For now, the + * client can trigger a re-sort (e.g., by opening and closing the table) to fix the slightly + * out-of-sort data. + *

+ * The likelihood of the sort being inconsistent now relates directly to how many changed items are + * in the table at the time of an insert. The more changed items, the higher the chance of a + * stale/misplaced item being used during a binary search, thus producing an invalid insert + * position. + *

+ * This strategy is setup to mitigate the number of invalid items in the table at the time the + * inserts are applied. The basic workflow of this algorithm is: + * 

+ * 1) condense the add / remove requests to remove duplicate efforts
+ * 2) process all removes first
+ *    --all pure removes
+ *    --all removes as part of a re-insert
+ * 3) process all items that failed to remove due to the sort data changing
+ * 4) process all adds (this step will fail if the data contains mis-sorted items)
+ *    --all adds as part of a re-insert
+ *    --all pure adds
+ *
+ * + * Step 3, processing failed removals, is done to avoid a brute force lookup at each removal + * request. + * + *

This strategy allows for the use of client proxy objects. The proxy objects should be coded + * such that the {@code hashCode()} and {@code equals()} methods will match those methods of the + * data's real objects. These proxy objects allow clients to search for an item without having a + * reference to the actual item. In this sense, the proxy object is equal to the existing row + * object in the table model, but is not the same instance as the row object. + * + * @param the row type + */ +public class CoalescingAddRemoveStrategy implements TableAddRemoveStrategy { + + @Override + public void process(List> addRemoveList, TableData tableData, + TaskMonitor monitor) throws CancelledException { + + Set> items = coalesceAddRemoveItems(addRemoveList); + + // + // Hash map the existing values so that we can use any object inside the add/remove list + // as a key into this map to get the matching existing value. Using the existing value + // enables the binary search to work when the add/remove item is a proxy object, but the + // existing item still has the data used to sort it. If the sort data has changed, then + // even this step will not allow the TableData to find the item in a search. + // + Map hashed = new HashMap<>(); + for (T t : tableData) { + hashed.put(t, t); + } + + Set failedToRemove = new HashSet<>(); + + int n = items.size(); + monitor.setMessage("Removing " + n + " items..."); + monitor.initialize(n); + + Iterator> it = items.iterator(); + while (it.hasNext()) { + AddRemoveListItem item = it.next(); + T value = item.getValue(); + if (item.isChange()) { + T toRemove = hashed.remove(value); + remove(tableData, toRemove, failedToRemove); + monitor.incrementProgress(1); + } + else if (item.isRemove()) { + T toRemove = hashed.remove(value); + remove(tableData, toRemove, failedToRemove); + it.remove(); + } + monitor.checkCanceled(); + } + + if (!failedToRemove.isEmpty()) { + int size = failedToRemove.size(); + String message = size == 1 ? "1 old symbol..." : size + " old symbols..."; + monitor.setMessage("Removing " + message); + + tableData.process((data, sortContext) -> { + return expungeLostItems(failedToRemove, data, sortContext); + }); + } + + n = items.size(); + monitor.setMessage("Adding " + n + " items..."); + it = items.iterator(); + while (it.hasNext()) { + AddRemoveListItem item = it.next(); + T value = item.getValue(); + if (item.isChange()) { + tableData.insert(value); + hashed.put(value, value); + } + else if (item.isAdd()) { + tableData.insert(value); + hashed.put(value, value); + } + monitor.checkCanceled(); + monitor.incrementProgress(1); + } + + monitor.setMessage("Done adding/removing"); + } + + private Set> coalesceAddRemoveItems( + List> addRemoveList) { + + Map> map = new HashMap<>(); + + for (AddRemoveListItem item : addRemoveList) { + + if (item.isChange()) { + handleChange(item, map); + } + else if (item.isAdd()) { + handleAdd(item, map); + } + else { + handleRemove(item, map); + } + } + + return new HashSet<>(map.values()); + } + + private void handleAdd(AddRemoveListItem item, Map> map) { + + T rowObject = item.getValue(); + AddRemoveListItem existing = map.get(rowObject); + if (existing == null) { + map.put(rowObject, item); + return; + } + + if (existing.isChange()) { + return; // change -> add; keep the change + } + if (existing.isAdd()) { + return; // already an add + } + + // remove -> add; make a change + map.put(rowObject, new AddRemoveListItem<>(CHANGE, existing.getValue())); + } + + private void handleRemove(AddRemoveListItem item, Map> map) { + + T rowObject = item.getValue(); + AddRemoveListItem existing = map.get(rowObject); + if (existing == null) { + map.put(rowObject, item); + return; + } + + if (existing.isChange()) { + map.put(rowObject, item); // change -> remove; just do the remove + return; + } + if (existing.isRemove()) { + return; + } + + // add -> remove; do no work + map.remove(rowObject); + } + + private void handleChange(AddRemoveListItem item, Map> map) { + + T rowObject = item.getValue(); + AddRemoveListItem existing = map.get(rowObject); + if (existing == null) { + map.put(rowObject, item); + return; + } + + if (!existing.isChange()) { + // either add or remove followed by a change; keep the change + map.put(rowObject, item); + } + + // otherwise, we had a change followed by a change; keep just 1 change + } + + private void remove(TableData tableData, T t, Set failedToRemove) { + if (t == null) { + return; + } + + if (!tableData.remove(t)) { + failedToRemove.add(t); + } + } + + /* + * Removes the given set of items that were unsuccessfully removed from the table as part of + * the add/remove process. These items could not be removed because some part of their state + * has changed such that the binary search performed during the normal remove process cannot + * locate the item in the table data. This algorithm will check the given set of items + * against the entire list of table data, locating the item to be removed. + */ + private List expungeLostItems(Set toRemove, List data, + TableSortingContext sortContext) { + + if (sortContext.isUnsorted()) { + // this can happen if the data is unsorted and we were asked to remove an item that + // was never in the table for some reason + return data; + } + + // Copy to a new list those items that are not marked for removal. This saves the + // list move its items every time a remove takes place + List newList = new ArrayList<>(data.size() - toRemove.size()); + for (int i = 0; i < data.size(); i++) { + T rowObject = data.get(i); + if (!toRemove.contains(rowObject)) { + newList.add(rowObject); + } + } + + return newList; + } +} diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/RowObject.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/RowObject.java index 1c34385bd6..698e363e93 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/RowObject.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/RowObject.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. @@ -16,15 +15,15 @@ */ package docking.widgets.table; -import ghidra.util.SystemUtilities; - import java.util.ArrayList; import java.util.List; import javax.swing.table.TableModel; +import ghidra.util.SystemUtilities; + /** - * An object that represents a row in a table. Most tables used in the system create tables that + * An object that represents a row in a table. Most tables used in the system create models that * use their own row objects (see {@link AbstractSortedTableModel}). This class exists to * compensate for those models that do not do this, but instead rely on the classic Java * {@link TableModel} method {@link TableModel#getValueAt(int, int)}. @@ -36,14 +35,18 @@ import javax.swing.table.TableModel; * row objects that created for non-{@link AbstractSortedTableModel}s will not be equal to * those created before the data change. This causes some features to break, such as selection * restoration after user edits. + * + * @deprecated this class is no longer used and will be removed */ +@Deprecated(forRemoval = true, since = "10.1") public class RowObject { /** * Factory method to create and initialize a row object. * * @param model the model required to gather data for the row object. - * @param row the row for which to create a row object * @return + * @param row the row for which to create a row object + * @return the row object */ public static RowObject createRowObject(TableModel model, int row) { RowObject rowObject = new RowObject(); @@ -54,7 +57,7 @@ public class RowObject { return rowObject; } - List values = new ArrayList(); + List values = new ArrayList<>(); int hash = -1; void addElement(Object object) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/DefaultAddRemoveStrategy.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/DefaultAddRemoveStrategy.java deleted file mode 100644 index c25957bfe2..0000000000 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/DefaultAddRemoveStrategy.java +++ /dev/null @@ -1,95 +0,0 @@ -/* ### - * IP: GHIDRA - * - * 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 docking.widgets.table.threaded; - -import java.util.*; - -import docking.widgets.table.AddRemoveListItem; -import docking.widgets.table.TableSortingContext; -import ghidra.util.exception.CancelledException; -import ghidra.util.task.TaskMonitor; - -/** - * A strategy that uses the table's sort state to perform a binary search of items to be added - * and removed. - * - *

This strategy is aware that some items may not be correctly removed, such as database items - * that have been deleted externally to the table. These items may require a brute force update - * to achieve removal. - * - * @param the row type - */ -public class DefaultAddRemoveStrategy implements TableAddRemoveStrategy { - - @Override - public void process(List> addRemoveList, TableData tableData, - TaskMonitor monitor) throws CancelledException { - - int n = addRemoveList.size(); - monitor.setMessage("Adding/Removing " + n + " items..."); - monitor.initialize(n); - - Set failedToRemove = new HashSet<>(); - - // Note: this class does not directly perform a binary search, but instead relies on that - // work to be done by the call to TableData.remove() - for (int i = 0; i < n; i++) { - AddRemoveListItem item = addRemoveList.get(i); - T value = item.getValue(); - if (item.isChange()) { - tableData.remove(value); - tableData.insert(value); - } - else if (item.isRemove()) { - if (!tableData.remove(value)) { - failedToRemove.add(value); - } - } - else if (item.isAdd()) { - tableData.insert(value); - } - monitor.checkCanceled(); - monitor.setProgress(i); - } - - if (!failedToRemove.isEmpty()) { - int size = failedToRemove.size(); - String message = size == 1 ? "1 deleted item..." : size + " deleted items..."; - monitor.setMessage("Removing " + message); - - tableData.process((data, sortContext) -> { - return expungeLostItems(failedToRemove, data, sortContext); - }); - } - - monitor.setMessage("Done adding/removing"); - } - - private List expungeLostItems(Set toRemove, List data, - TableSortingContext sortContext) { - - List newList = new ArrayList<>(data.size() - toRemove.size()); - for (int i = 0; i < data.size(); i++) { - T rowObject = data.get(i); - if (!toRemove.contains(rowObject)) { - newList.add(rowObject); - } - } - - return newList; - } - -} diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModel.java index 43bd3557ff..2ca94acc82 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModel.java @@ -93,7 +93,7 @@ public abstract class ThreadedTableModel private int minUpdateDelayMillis; private int maxUpdateDelayMillis; private TableAddRemoveStrategy binarySearchAddRemoveStrategy = - new DefaultAddRemoveStrategy<>(); + new CoalescingAddRemoveStrategy<>(); protected ThreadedTableModel(String modelName, ServiceProvider serviceProvider) { this(modelName, serviceProvider, null); diff --git a/Ghidra/Framework/Docking/src/test/java/docking/widgets/table/CoalescingAddRemoveStrategyTest.java b/Ghidra/Framework/Docking/src/test/java/docking/widgets/table/CoalescingAddRemoveStrategyTest.java new file mode 100644 index 0000000000..c2eae82f58 --- /dev/null +++ b/Ghidra/Framework/Docking/src/test/java/docking/widgets/table/CoalescingAddRemoveStrategyTest.java @@ -0,0 +1,396 @@ +/* ### + * IP: GHIDRA + * + * 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 docking.widgets.table; + +import static docking.widgets.table.AddRemoveListItem.Type.*; +import static org.junit.Assert.*; + +import java.util.*; + +import org.junit.Before; +import org.junit.Test; + +import docking.widgets.table.threaded.TestTableData; +import ghidra.util.task.TaskMonitor; + +public class CoalescingAddRemoveStrategyTest { + + private CoalescingAddRemoveStrategy strategy; + private SpyTableData spyTableData; + private List modelData; + + @Before + public void setUp() throws Exception { + strategy = new CoalescingAddRemoveStrategy<>(); + modelData = createModelData(); + spyTableData = createTableData(); + } + + private SpyTableData createTableData() { + Comparator comparator = (s1, s2) -> { + return s1.getName().compareTo(s2.getName()); + }; + TableSortState sortState = TableSortState.createDefaultSortState(0); + TableSortingContext sortContext = + new TableSortingContext<>(sortState, comparator); + + modelData.sort(comparator); + + return new SpyTableData(modelData, sortContext); + } + + private List createModelData() { + List data = new ArrayList<>(); + data.add(new TestRowObject(1)); + data.add(new TestRowObject(2)); + data.add(new TestRowObject(3)); + data.add(new TestRowObject(4)); + return data; + } + + @Test + public void testRemove_DifferentInstance_SameId() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = new TestRowObject(1); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(0, spyTableData.getInsertCount()); + } + + @Test + public void testInsert_NewSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject newSymbol = new TestRowObject(10); + addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + assertEquals(0, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testInsertAndRemove_NewSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject newSymbol = new TestRowObject(10); + addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol)); + addRemoves.add(new AddRemoveListItem<>(REMOVE, newSymbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // no work was done, since the insert was followed by a remove + assertEquals(0, spyTableData.getRemoveCount()); + assertEquals(0, spyTableData.getInsertCount()); + } + + @Test + public void testChange_NewSymbol() throws Exception { + List> addRemoves = new ArrayList<>(); + + TestRowObject newSymbol = new TestRowObject(10); + addRemoves.add(new AddRemoveListItem<>(CHANGE, newSymbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // no remove, since the symbol was not in the table + assertEquals(0, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testChange_ExisingSymbol() throws Exception { + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // no remove, since the symbol was not in the table + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testRemoveAndInsert_NewSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject newSymbol = new TestRowObject(10); + addRemoves.add(new AddRemoveListItem<>(REMOVE, newSymbol)); + addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the remove does not happen, since the time was not in the table + assertEquals(0, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testRemoveAndInsert_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + addRemoves.add(new AddRemoveListItem<>(ADD, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the remove does not happen, since the time was not in the table + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testChangeAndInsert_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + addRemoves.add(new AddRemoveListItem<>(ADD, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the insert portions get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testChangeAndRemove_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the remove portions get coalesced; no insert takes place + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(0, spyTableData.getInsertCount()); + } + + @Test + public void testChangeAndChange_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the changes get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testRemoveAndRemove_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the removes get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(0, spyTableData.getInsertCount()); + } + + @Test + public void testInsertAndInsert_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(ADD, ro)); + addRemoves.add(new AddRemoveListItem<>(ADD, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the inserts get coalesced + assertEquals(0, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testInsertAndChange_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(ADD, ro)); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the insert portions get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testRemoveAndChange_ExistingSymbol() throws Exception { + + List> addRemoves = new ArrayList<>(); + + TestRowObject ro = modelData.get(0); + addRemoves.add(new AddRemoveListItem<>(REMOVE, ro)); + addRemoves.add(new AddRemoveListItem<>(CHANGE, ro)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the remove portions get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + + @Test + public void testLostItems_Remove() throws Exception { + + // + // Test that symbols get removed when the data up on which they are sorted changes before + // the removal takes place + // + List> addRemoves = new ArrayList<>(); + + TestRowObject symbol = modelData.get(0); + symbol.setName("UpdatedName"); + addRemoves.add(new AddRemoveListItem<>(REMOVE, symbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the insert portions get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(0, spyTableData.getInsertCount()); + } + + @Test + public void testLostItems_Change() throws Exception { + + // + // Test that symbols get removed when the data up on which they are sorted changes before + // the removal takes place + // + List> addRemoves = new ArrayList<>(); + + TestRowObject symbol = modelData.get(0); + symbol.setName("UpdatedName"); + addRemoves.add(new AddRemoveListItem<>(CHANGE, symbol)); + + strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY); + + // the insert portions get coalesced + assertEquals(1, spyTableData.getRemoveCount()); + assertEquals(1, spyTableData.getInsertCount()); + } + +//================================================================================================== +// Inner Classes +//================================================================================================== + + private class SpyTableData extends TestTableData { + + private int removeCount; + private int insertCount; + + SpyTableData(List data, TableSortingContext sortContext) { + super(data, sortContext); + } + + @Override + public boolean remove(TestRowObject t) { + removeCount++; + return super.remove(t); + } + + @Override + public void insert(TestRowObject value) { + insertCount++; + super.insert(value); + } + + int getRemoveCount() { + return removeCount; + } + + int getInsertCount() { + return insertCount; + } + } + + private class TestRowObject { + + private String name; + private long id; + + TestRowObject(long id) { + this.id = id; + this.name = Long.toString(id); + } + + String getName() { + return name; + } + + void setName(String newName) { + this.name = newName; + } + + @Override + public int hashCode() { + return (int) id; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + + TestRowObject other = (TestRowObject) obj; + if (id != other.id) { + return false; + } + return true; + } + + } +} diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/dialog/ExtensionTableModel.java b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/dialog/ExtensionTableModel.java index 8361138de1..01bfc27872 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/dialog/ExtensionTableModel.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/dialog/ExtensionTableModel.java @@ -18,8 +18,7 @@ package ghidra.framework.plugintool.dialog; import java.awt.Color; import java.awt.Component; import java.io.File; -import java.util.ArrayList; -import java.util.List; +import java.util.*; import docking.widgets.table.*; import docking.widgets.table.threaded.ThreadedTableModel; @@ -52,7 +51,7 @@ import utilities.util.FileUtilities; * is a checkbox allowing users to install/uninstall a particular extension. * */ -class ExtensionTableModel extends ThreadedTableModel> { +class ExtensionTableModel extends ThreadedTableModel { /** We don't care about the ordering of other columns, but the install/uninstall checkbox should be the first one and the name col is our initial sort column. */ @@ -60,7 +59,7 @@ class ExtensionTableModel extends ThreadedTableModel extensions = new ArrayList<>(); + private Set extensions; /** Indicates if the model has changed due to an install or uninstall. */ private boolean modelChanged = false; @@ -78,7 +77,7 @@ class ExtensionTableModel extends ThreadedTableModel createTableColumnDescriptor() { TableColumnDescriptor descriptor = - new TableColumnDescriptor(); + new TableColumnDescriptor<>(); descriptor.addVisibleColumn(new ExtensionInstalledColumn(), INSTALLED_COL, true); descriptor.addVisibleColumn(new ExtensionNameColumn(), NAME_COL, true); @@ -178,20 +177,33 @@ class ExtensionTableModel extends ThreadedTableModel getDataSource() { - return extensions; + public Object getDataSource() { + return null; } @Override protected void doLoad(Accumulator accumulator, TaskMonitor monitor) throws CancelledException { + if (extensions != null) { + accumulator.addAll(extensions); + return; + } + + try { + extensions = ExtensionUtils.getExtensions(); + } + catch (ExtensionException e) { + Msg.error(this, "Error loading extensions", e); + return; + } + accumulator.addAll(extensions); } /** - * Returns true if the model has changed as a result of installing or uninstalling an extension. + * Returns true if the model has changed as a result of installing or uninstalling an extension * - * @return true if the model has changed as a result of installing or uninstalling an extension. + * @return true if the model has changed as a result of installing or uninstalling an extension */ public boolean hasModelChanged() { return modelChanged; @@ -203,7 +215,7 @@ class ExtensionTableModel extends ThreadedTableModel model) { - extensions = model; + extensions = new HashSet<>(model); reload(); } @@ -211,12 +223,8 @@ class ExtensionTableModel extends ThreadedTableModel(ExtensionUtils.getExtensions())); - } - catch (ExtensionException e) { - Msg.error(this, "Error loading extensions", e); - } + extensions = null; + reload(); } /** @@ -236,7 +244,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { private ExtVersionRenderer renderer = new ExtVersionRenderer(); @@ -252,7 +260,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { return rowObject.getName(); } @@ -266,7 +274,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { private ExtVersionRenderer renderer = new ExtVersionRenderer(); @@ -282,7 +290,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { return rowObject.getDescription(); } @@ -296,7 +304,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { private ExtVersionRenderer renderer = new ExtVersionRenderer(); @@ -312,7 +320,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { String version = rowObject.getVersion(); @@ -335,7 +343,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { @Override public String getColumnName() { @@ -349,7 +357,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { return rowObject.isInstalled(); } } @@ -358,7 +366,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { @Override public String getColumnName() { @@ -372,7 +380,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { return rowObject.getInstallPath(); } } @@ -381,7 +389,7 @@ class ExtensionTableModel extends ThreadedTableModel> { + extends AbstractDynamicTableColumn { @Override public String getColumnName() { @@ -395,7 +403,7 @@ class ExtensionTableModel extends ThreadedTableModel data, ServiceProvider sp) throws IllegalArgumentException { + Object data, ServiceProvider sp) throws IllegalArgumentException { return rowObject.getArchivePath(); } }