From 0eafe444457d0bdcd739353f7fdf91ffb58fc96a Mon Sep 17 00:00:00 2001
From: dragonmacher <48328597+dragonmacher@users.noreply.github.com>
Date: Mon, 21 Nov 2022 09:54:08 -0500
Subject: [PATCH] Test fixes

---
 .../bean/opteditor/OptionsDialogTest.java     |  1 -
 .../ghidra/framework/options/OptionsTest.java | 20 +++++++++-----
 .../framework/options/AbstractOptions.java    | 16 +++++++----
 .../ghidra/framework/options/EditorState.java | 27 ++++++++++++-------
 4 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/util/bean/opteditor/OptionsDialogTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/util/bean/opteditor/OptionsDialogTest.java
index e9cb5362fd..c113dfb670 100644
--- a/Ghidra/Features/Base/src/test.slow/java/ghidra/util/bean/opteditor/OptionsDialogTest.java
+++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/util/bean/opteditor/OptionsDialogTest.java
@@ -649,7 +649,6 @@ public class OptionsDialogTest extends AbstractGhidraHeadedIntegrationTest {
 
 		Options options = tool.getOptions(ToolConstants.TOOL_OPTIONS);
 
-		Color c = options.getColor("Favorite Color", Palette.RED);
 		String currentValue = options.getString("Favorite String", null);
 		assertEquals("Bar", currentValue);
 
diff --git a/Ghidra/Features/Base/src/test/java/ghidra/framework/options/OptionsTest.java b/Ghidra/Features/Base/src/test/java/ghidra/framework/options/OptionsTest.java
index 4258e982f8..3c03ac9af8 100644
--- a/Ghidra/Features/Base/src/test/java/ghidra/framework/options/OptionsTest.java
+++ b/Ghidra/Features/Base/src/test/java/ghidra/framework/options/OptionsTest.java
@@ -246,15 +246,21 @@ public class OptionsTest extends AbstractGenericTest {
 		options.addOptionsChangeListener(listener1);
 		options.addOptionsChangeListener(listener2);
 
-		options.setColor("COLOR", Palette.BLUE);
+		try {
+			options.setColor("COLOR", Palette.BLUE);
+			fail("Expected an OptionsVetoExcepton");
+		}
+		catch (OptionsVetoException e) {
+			// expected
+		}
 
 		assertEquals(Palette.RED, options.getColor("COLOR", Palette.RED));
 
-		if (listener1.callOrder == 1) {
+		if (listener1.callCount == 1) {
 			assertEquals(Palette.RED, listener1.value);
 			assertEquals(null, listener2.value);
 		}
-		if (listener2.callOrder == 1) {
+		if (listener2.callCount == 1) {
 			assertEquals(Palette.RED, listener2.value);
 			assertEquals(null, listener1.value);
 		}
@@ -601,17 +607,17 @@ public class OptionsTest extends AbstractGenericTest {
 	private static class OptionsChangeListenerForTestVeto implements OptionsChangeListener {
 		static int count = 0;
 		private Object value;
-		private int callOrder = -1;
+		private int callCount = -1;
 
 		@Override
 		public void optionsChanged(ToolOptions options, String optionName, Object oldValue,
 				Object newValue) {
 
-			if (callOrder < 0) {
-				callOrder = ++count;
+			if (callCount < 0) {
+				callCount = ++count;
 			}
 
-			if (callOrder > 1) {
+			if (callCount > 1) {
 				throw new OptionsVetoException("Test");
 			}
 
diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AbstractOptions.java b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AbstractOptions.java
index ac9e9e523e..c1c4454790 100644
--- a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AbstractOptions.java
+++ b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AbstractOptions.java
@@ -175,9 +175,8 @@ public abstract class AbstractOptions implements Options {
 	}
 
 	private void warnShouldUseTheme(String optionType) {
-		Throwable throwable =
-			ReflectionUtilities.createThrowableWithStackOlderThan(AbstractOptions.class,
-				SubOptions.class);
+		Throwable throwable = ReflectionUtilities
+				.createThrowableWithStackOlderThan(AbstractOptions.class, SubOptions.class);
 		String call = throwable.getStackTrace()[0].toString();
 		Msg.warn(this, "Registering a direct " + optionType + " in the options is deprecated." +
 			" Use registerTheme" + optionType + "Binding() instead!\n Called from " + call + "\n");
@@ -343,8 +342,15 @@ public abstract class AbstractOptions implements Options {
 		Object oldValue = option.getCurrentValue();
 		option.setCurrentValue(newValue);
 
-		if (!notifyOptionChanged(optionName, oldValue, newValue)) {
-			option.setCurrentValue(oldValue);
+		boolean success = false;
+		try {
+			// this can throw an OptionsVetoException
+			success = notifyOptionChanged(optionName, oldValue, newValue);
+		}
+		finally {
+			if (!success) {
+				option.setCurrentValue(oldValue);
+			}
 		}
 	}
 
diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/EditorState.java b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/EditorState.java
index 6977680d8f..849e85e00c 100644
--- a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/EditorState.java
+++ b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/EditorState.java
@@ -117,16 +117,25 @@ public class EditorState implements PropertyChangeListener {
 			return;
 		}
 
-		options.putObject(name, currentValue);
-		Object newValue = options.getObject(name, null);
-		boolean success = Objects.equals(currentValue, newValue);
-		if (success) {
-			originalValue = newValue;
-			currentValue = newValue;
+		//
+		// The call to put() may throw an exception or may choose not to take the new value. Handle
+		// both cases using a finally block along with checking the value after making the put()
+		// call.
+		//
+		try {
+			options.putObject(name, currentValue);
 		}
-		else {
-			editor.setValue(originalValue);
-			currentValue = originalValue;
+		finally {
+			Object newValue = options.getObject(name, null);
+			boolean success = Objects.equals(currentValue, newValue);
+			if (success) {
+				originalValue = newValue;
+				currentValue = newValue;
+			}
+			else {
+				editor.setValue(originalValue);
+				currentValue = originalValue;
+			}
 		}
 	}