diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/data/PairedTransactionTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/data/PairedTransactionTest.java index 87389fe9e7..06aa17f6ee 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/data/PairedTransactionTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/framework/data/PairedTransactionTest.java @@ -22,7 +22,6 @@ import java.util.*; import org.junit.*; -import db.DBHandle; import generic.test.AbstractGenericTest; import ghidra.framework.model.*; import ghidra.framework.options.Options; @@ -82,23 +81,6 @@ public class PairedTransactionTest extends AbstractGenericTest { } } - class DummyDomainObject extends DomainObjectAdapterDB { - - protected DummyDomainObject(String name, Object consumer) throws IOException { - super(new DBHandle(), name, 10, 1, consumer); - } - - @Override - public String getDescription() { - return "Test object: " + testName.getMethodName(); - } - - @Override - public boolean isChangeable() { - return true; - } - } - private static String START = "Start"; private static String END = "End"; private static String UNDO_STATE_CHANGE1 = "UndoRedo1"; @@ -106,7 +88,7 @@ public class PairedTransactionTest extends AbstractGenericTest { class MyListener implements TransactionListener { - private List events = new ArrayList(); + private List events = new ArrayList<>(); private Transaction lastTransaction; private DomainObjectAdapterDB obj; @@ -122,7 +104,8 @@ public class PairedTransactionTest extends AbstractGenericTest { } @Override - public synchronized void transactionStarted(DomainObjectAdapterDB domainObj, Transaction tx) { + public synchronized void transactionStarted(DomainObjectAdapterDB domainObj, + Transaction tx) { assertEquals(obj, domainObj); events.add(START); lastTransaction = tx; @@ -176,8 +159,8 @@ public class PairedTransactionTest extends AbstractGenericTest { assertTrue(obj1.canUndo()); assertTrue(obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); Transaction tx = obj1Listener.getLastTransaction(); obj1Listener.getEvents(); @@ -204,15 +187,15 @@ public class PairedTransactionTest extends AbstractGenericTest { assertEquals(obj1, synchronizedDomainObjects[0]); assertEquals(obj2, synchronizedDomainObjects[1]); - assertEquals(synchronizedDomainObjects, obj2.getSynchronizedDomainObjects()); + assertArrayEquals(synchronizedDomainObjects, obj2.getSynchronizedDomainObjects()); assertEquals(0, obj1.getUndoStackDepth()); assertEquals(0, obj2.getUndoStackDepth()); - assertTrue(!obj1.canUndo()); - assertTrue(!obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canUndo()); + assertFalse(obj2.canUndo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); String[] events1 = obj1Listener.getEvents(); assertEquals(UNDO_STATE_CHANGE1, events1[events1.length - 1]); @@ -269,10 +252,10 @@ public class PairedTransactionTest extends AbstractGenericTest { assertEquals(0, obj1.getUndoStackDepth()); assertEquals(0, obj2.getUndoStackDepth()); - assertTrue(!obj1.canUndo()); - assertTrue(!obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canUndo()); + assertFalse(obj2.canUndo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); events1 = obj1Listener.getEvents(); events2 = obj2Listener.getEvents(); @@ -329,8 +312,8 @@ public class PairedTransactionTest extends AbstractGenericTest { assertTrue(obj1.canUndo()); assertTrue(obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); events1 = obj1Listener.getEvents(); events2 = obj2Listener.getEvents(); @@ -344,8 +327,8 @@ public class PairedTransactionTest extends AbstractGenericTest { obj1.undo(); - assertTrue(!obj1.canUndo()); - assertTrue(!obj2.canUndo()); + assertFalse(obj1.canUndo()); + assertFalse(obj2.canUndo()); assertTrue(obj1.canRedo()); assertTrue(obj2.canRedo()); @@ -366,8 +349,8 @@ public class PairedTransactionTest extends AbstractGenericTest { assertTrue(obj1.canUndo()); assertTrue(obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); events1 = obj1Listener.getEvents(); events2 = obj2Listener.getEvents(); @@ -388,10 +371,10 @@ public class PairedTransactionTest extends AbstractGenericTest { assertEquals(0, obj1.getUndoStackDepth()); assertEquals(0, obj2.getUndoStackDepth()); - assertTrue(!obj1.canUndo()); - assertTrue(!obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canUndo()); + assertFalse(obj2.canUndo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); events1 = obj1Listener.getEvents(); events2 = obj2Listener.getEvents(); @@ -449,10 +432,10 @@ public class PairedTransactionTest extends AbstractGenericTest { assertEquals(0, obj1.getUndoStackDepth()); assertEquals(1, obj2.getUndoStackDepth()); - assertTrue(!obj1.canUndo()); + assertFalse(obj1.canUndo()); assertTrue(obj2.canUndo()); - assertTrue(!obj1.canRedo()); - assertTrue(!obj2.canRedo()); + assertFalse(obj1.canRedo()); + assertFalse(obj2.canRedo()); events1 = obj1Listener.getEvents(); events2 = obj2Listener.getEvents(); diff --git a/Ghidra/Features/Base/src/test/java/ghidra/framework/plugintool/mgr/BackgroundCommandTaskTest.java b/Ghidra/Features/Base/src/test/java/ghidra/framework/plugintool/mgr/BackgroundCommandTaskTest.java new file mode 100644 index 0000000000..0c539d6c62 --- /dev/null +++ b/Ghidra/Features/Base/src/test/java/ghidra/framework/plugintool/mgr/BackgroundCommandTaskTest.java @@ -0,0 +1,184 @@ +/* ### + * 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 ghidra.framework.plugintool.mgr; + +import static org.junit.Assert.*; + +import java.io.IOException; + +import org.junit.Before; +import org.junit.Test; + +import generic.test.AbstractGenericTest; +import ghidra.framework.cmd.BackgroundCommand; +import ghidra.framework.data.DummyDomainObject; +import ghidra.framework.model.*; +import ghidra.test.DummyTool; +import ghidra.util.UniversalIdGenerator; +import ghidra.util.exception.RollbackException; +import ghidra.util.task.TaskMonitor; + +public class BackgroundCommandTaskTest extends AbstractGenericTest { + + @Before + public void setUp() { + UniversalIdGenerator.initialize(); + } + + @Test + public void testSuccessfulCommand() throws Exception { + + DummyTool tool = new DummyTool(); + ToolTaskManager taskManager = new ToolTaskManager(tool); + SpyDomainObject domainObject = new SpyDomainObject(this); + SuccessfulDummyCommand cmd = new SuccessfulDummyCommand(); + taskManager.executeCommand(cmd, domainObject); + + waitFor(taskManager); + assertTrue(domainObject.wasCommitted()); + } + + @Test + public void testExceptionalCommand_NonRollbackException() throws Exception { + + DummyTool tool = new DummyTool(); + ToolTaskManager taskManager = new ToolTaskManager(tool); + SpyDomainObject domainObject = new SpyDomainObject(this); + NullPointerExceptionCommand cmd = new NullPointerExceptionCommand(); + + setErrorsExpected(true); + taskManager.executeCommand(cmd, domainObject); + waitFor(taskManager); + setErrorsExpected(false); + + assertTrue(domainObject.wasCommitted()); + } + + @Test + public void testExceptionalCommand_RollbackException() throws Exception { + + DummyTool tool = new DummyTool(); + ToolTaskManager taskManager = new ToolTaskManager(tool); + SpyDomainObject domainObject = new SpyDomainObject(this); + RollbackExceptionCommand cmd = new RollbackExceptionCommand(); + + setErrorsExpected(true); + taskManager.executeCommand(cmd, domainObject); + waitFor(taskManager); + setErrorsExpected(false); + + assertFalse(domainObject.wasCommitted()); + } + + @Test + public void testExceptionalCommand_DomainObjectLockedException() throws Exception { + + DummyTool tool = new DummyTool(); + ToolTaskManager taskManager = new ToolTaskManager(tool); + SpyDomainObject domainObject = new SpyDomainObject(this); + DomainObjectLockedExceptionCommand cmd = new DomainObjectLockedExceptionCommand(); + + setErrorsExpected(true); + taskManager.executeCommand(cmd, domainObject); + waitFor(taskManager); + setErrorsExpected(false); + + assertFalse(domainObject.wasCommitted()); + } + + private void waitFor(ToolTaskManager taskManager) { + waitFor(() -> !taskManager.isBusy()); + } + + private class SpyDomainObject extends DummyDomainObject { + + private static final int ID = 1; + private boolean transactionCommited; + + protected SpyDomainObject(Object consumer) throws IOException { + super(consumer); + } + + @Override + public int startTransaction(String description) { + return startTransaction(description, null); + } + + @Override + public int startTransaction(String description, AbortedTransactionListener listener) { + return ID; + } + + @Override + public void endTransaction(int transactionID, boolean commit) { + + assertEquals(ID, transactionID); + transactionCommited = commit; + } + + boolean wasCommitted() { + return transactionCommited; + } + } + + private class SuccessfulDummyCommand extends BackgroundCommand { + + SuccessfulDummyCommand() { + super("Dummy", true, true, false); + } + + @Override + public boolean applyTo(DomainObject obj, TaskMonitor monitor) { + return true; + } + + } + + private class NullPointerExceptionCommand extends BackgroundCommand { + + NullPointerExceptionCommand() { + super("Dummy", true, true, false); + } + + @Override + public boolean applyTo(DomainObject obj, TaskMonitor monitor) { + throw new NullPointerException(); + } + + } + + private class RollbackExceptionCommand extends BackgroundCommand { + RollbackExceptionCommand() { + super("Dummy", true, true, false); + } + + @Override + public boolean applyTo(DomainObject obj, TaskMonitor monitor) { + throw new RollbackException("This is a rollback exception"); + } + } + + private class DomainObjectLockedExceptionCommand extends BackgroundCommand { + DomainObjectLockedExceptionCommand() { + super("Dummy", true, true, false); + } + + @Override + public boolean applyTo(DomainObject obj, TaskMonitor monitor) { + throw new DomainObjectLockedException("Unable to connect"); + } + } +} diff --git a/Ghidra/Features/VersionTracking/src/test/java/ghidra/feature/vt/gui/filters/TagFilterTest.java b/Ghidra/Features/VersionTracking/src/test/java/ghidra/feature/vt/gui/filters/TagFilterTest.java index 48f56eca9d..3a698ac060 100644 --- a/Ghidra/Features/VersionTracking/src/test/java/ghidra/feature/vt/gui/filters/TagFilterTest.java +++ b/Ghidra/Features/VersionTracking/src/test/java/ghidra/feature/vt/gui/filters/TagFilterTest.java @@ -15,15 +15,13 @@ */ package ghidra.feature.vt.gui.filters; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.io.IOException; import java.util.*; import org.junit.*; -import db.DBHandle; import ghidra.feature.vt.api.db.VTSessionDB; import ghidra.feature.vt.api.impl.VTChangeManager; import ghidra.feature.vt.api.impl.VersionTrackingChangeRecord; @@ -32,7 +30,7 @@ import ghidra.feature.vt.db.DummyTestProgramCorrelator; import ghidra.feature.vt.db.VTBaseTestCase; import ghidra.feature.vt.gui.plugin.VTController; import ghidra.feature.vt.gui.plugin.VTControllerListener; -import ghidra.framework.data.DomainObjectAdapterDB; +import ghidra.framework.data.DummyDomainObject; import ghidra.framework.model.*; import ghidra.framework.plugintool.ServiceProvider; import ghidra.program.model.listing.Program; @@ -433,23 +431,6 @@ public class TagFilterTest extends VTBaseTestCase { } } - class DummyDomainObject extends DomainObjectAdapterDB { - - protected DummyDomainObject(Object consumer) throws IOException { - super(new DBHandle(), "Dummy", 10, 1, consumer); - } - - @Override - public String getDescription() { - return "Test object: " + getName(); - } - - @Override - public boolean isChangeable() { - return true; - } - } - public static VTProgramCorrelator createProgramCorrelator(ServiceProvider serviceProvider, Program sourceProgram, Program destinationProgram) { return new DummyTestProgramCorrelator(serviceProvider, sourceProgram, destinationProgram); diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/BackgroundCommandTask.java b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/BackgroundCommandTask.java index 09020b936a..a5d51e6a15 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/BackgroundCommandTask.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/BackgroundCommandTask.java @@ -56,6 +56,7 @@ class BackgroundCommandTask extends Task implements AbortedTransactionListener { /** * Returns the Domain Object associated with this Task + * @return the object */ public UndoableDomainObject getDomainObject() { return obj; @@ -96,13 +97,12 @@ class BackgroundCommandTask extends Task implements AbortedTransactionListener { } boolean success = false; - boolean error = true; + boolean commit = true; try { success = cmd.applyTo(obj, monitor); if (success) { taskMgr.taskCompleted(obj, this, monitor); } - error = false; } catch (Throwable t) { synchronized (taskMgr) { @@ -114,26 +114,28 @@ class BackgroundCommandTask extends Task implements AbortedTransactionListener { t = t.getCause(); } - if (ignoreException(t)) { + commit = shouldKeepData(t); + + if (isUnrecoverableException(t)) { monitor.cancel(); taskMgr.clearTasks(obj); return; } else if (!(t instanceof RollbackException)) { - Msg.showError(this, null, "Command Failure", - "An unexpected error occurred while processing the command: " + cmd.getName(), - t); + String message = + "An unexpected error occurred while processing the command: " + cmd.getName(); + Msg.showError(this, null, "Command Failure", message, t); } } finally { TaskUtilities.removeTrackedTask(this); try { - obj.endTransaction(id, !error); + obj.endTransaction(id, commit); } catch (DomainObjectException e) { Throwable cause = e.getCause(); - if (!error && !(cause instanceof ClosedException)) { - Msg.showError(this, null, null, null, cause); + if (commit && !(cause instanceof ClosedException)) { + Msg.error(this, "Transaction error", cause); success = false; } } @@ -144,7 +146,13 @@ class BackgroundCommandTask extends Task implements AbortedTransactionListener { } } - private boolean ignoreException(Throwable t) { + private boolean shouldKeepData(Throwable t) { + // unrecoverable exceptions are really bad; rollback exceptions signal to abort + boolean reallyBad = isUnrecoverableException(t) || t instanceof RollbackException; + return !reallyBad; + } + + private boolean isUnrecoverableException(Throwable t) { //@formatter:off return t instanceof ConnectException || diff --git a/Ghidra/Framework/Project/src/test/java/ghidra/framework/data/DummyDomainObject.java b/Ghidra/Framework/Project/src/test/java/ghidra/framework/data/DummyDomainObject.java new file mode 100644 index 0000000000..87cd61175e --- /dev/null +++ b/Ghidra/Framework/Project/src/test/java/ghidra/framework/data/DummyDomainObject.java @@ -0,0 +1,44 @@ +/* ### + * 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 ghidra.framework.data; + +import java.io.IOException; + +import db.DBHandle; + +/** + * Simple dummy version of DomainObjectAdapterDB + */ +public class DummyDomainObject extends DomainObjectAdapterDB { + + public DummyDomainObject(Object consumer) throws IOException { + this("Dummy", consumer); + } + + public DummyDomainObject(String name, Object consumer) throws IOException { + super(new DBHandle(), name, 10, 1, consumer); + } + + @Override + public String getDescription() { + return "Test object: " + getName(); + } + + @Override + public boolean isChangeable() { + return true; + } +} diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/test/DummyTool.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/test/DummyTool.java index 4e70428db7..9c44e7864a 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/test/DummyTool.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/test/DummyTool.java @@ -20,7 +20,7 @@ import java.beans.PropertyChangeListener; import java.util.Collections; import java.util.Set; -import javax.swing.ImageIcon; +import javax.swing.*; import javax.swing.event.ChangeListener; import org.jdom.Element; @@ -436,4 +436,19 @@ public class DummyTool extends PluginTool { public PluginClassManager getPluginClassManager() { return null; } + + @Override + public void addStatusComponent(JComponent c, boolean addBorder, boolean rightSide) { + //do nothing + } + + @Override + public void removeStatusComponent(JComponent c) { + //do nothing + } + + @Override + public JFrame getToolFrame() { + return null; + } }