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 75b9f4eb7a..50eb9ad547 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 @@ -24,6 +24,8 @@ import java.awt.event.KeyEvent; import java.beans.PropertyChangeListener; import java.beans.PropertyEditorSupport; import java.io.File; +import java.text.SimpleDateFormat; +import java.time.LocalDate; import java.util.*; import javax.swing.JComponent; @@ -34,7 +36,7 @@ import org.junit.*; import generic.test.AbstractGuiTest; import generic.theme.GThemeDefaults.Colors.Palette; -import ghidra.util.HelpLocation; +import ghidra.util.*; import ghidra.util.bean.opteditor.OptionsVetoException; import ghidra.util.exception.InvalidInputException; import gui.event.MouseBinding; @@ -454,12 +456,133 @@ public class OptionsTest extends AbstractGuiTest { assertTrue(options.contains("Foo")); assertTrue(options.contains("Bar")); - options.registerOption("Bar", 0, null, "foo"); - + options.registerOption("Bar", 0, null, "Bar description"); options.removeUnusedOptions(); - assertFalse(options.contains("Foo")); + // registered; should stay around assertTrue(options.contains("Bar")); + // not registered, but not aged-off; should stay around + assertTrue(options.contains("Foo")); + + // Save, edit the date of the options to be older than 1 year, which is our age cutoff. + // Make sure any unregistered option is then removed. + Element savedRoot = options.getXmlRoot(false); + LocalDate twoYearsAgo = LocalDate.now().minusYears(2); + Set optionNames = Set.of("Bar", "Foo"); + setLastUsedDate(savedRoot, twoYearsAgo, optionNames); + + options = new ToolOptions(savedRoot); + assertTrue(options.contains("Foo")); + assertTrue(options.contains("Bar")); + + options.registerOption("Bar", 0, null, "Bar description"); + options.removeUnusedOptions(); + + // registered; should stay around + assertTrue(options.contains("Bar")); + // not registered, and older than 1 year; should be removed + assertFalse(options.contains("Foo")); + } + + @Test + public void testRemoveUnusedOption_WrappedOption() throws Exception { + + Date date = new Date(); + options.setDate("Foo", date); + options.setDate("Bar", date); + saveAndRestoreOptions(); + assertEquals(date, options.getDate("Foo", null)); + assertEquals(date, options.getDate("Bar", null)); + + Date defaultDate = new SimpleDateFormat("yyyy-MM-dd").parse("2025-09-12"); + + options.registerOption("Bar", defaultDate, null, "Bar description"); + options.removeUnusedOptions(); + + // registered; should stay around + assertTrue(options.contains("Bar")); + // not registered, but not aged-off; should stay around + assertTrue(options.contains("Foo")); + + // Save, edit the date of the options to be older than 1 year, which is our age cutoff. + // Make sure any unregistered option is then removed. + Element savedRoot = options.getXmlRoot(false); + LocalDate twoYearsAgo = LocalDate.now().minusYears(2); + Set optionNames = Set.of("Bar", "Foo"); + setLastUsedDate(savedRoot, twoYearsAgo, optionNames); + + options = new ToolOptions(savedRoot); + assertTrue(options.contains("Foo")); + assertTrue(options.contains("Bar")); + + options.registerOption("Bar", defaultDate, null, "Bar description"); + options.removeUnusedOptions(); + + // registered; should stay around + assertTrue(options.contains("Bar")); + // not registered, and older than 1 year; should be removed + assertFalse(options.contains("Foo")); + } + + @Test + public void testUnregisteredWarningMessage() { + + /* + Test that options access, but not registered, will trigger a warning message. + */ + + SpyErrorLogger spyLogger = new SpyErrorLogger(); + Msg.setErrorLogger(spyLogger); + + options.getInt("Foo", 1); + assertTrue(spyLogger.isEmtpy()); + + options.validateOptions(); + spyLogger.assertLogMessage("Unregistered", "property", "Foo"); + + spyLogger.reset(); + options.registerOption("Foo", 0, null, "Description"); + options.getInt("Foo", 2); + options.validateOptions(); + assertTrue(spyLogger.isEmtpy()); + } + + private void setLastUsedDate(Element savedRoot, LocalDate lastRegisteredDate, + Set names) { + + /* + + Msg.debug(this, XmlUtilities.toString(savedRoot)); + + + + + + + */ + + String dateString = lastRegisteredDate.format(ToolOptions.LAST_REGISTERED_DATE_FORMATTER); + + List elements = getElementsByName(savedRoot, names); + for (Element element : elements) { // + element.setAttribute(ToolOptions.LAST_REGISTERED_DATE_ATTIBUTE, dateString); + } + + } + + @SuppressWarnings("unchecked") + private List getElementsByName(Element savedRoot, Set names) { + + List matches = new ArrayList<>(); + List children = savedRoot.getChildren(); + for (Element saveState : children) { + String name = saveState.getAttributeValue("NAME"); + if (names.contains(name)) { + matches.add(saveState); + } + } + + return matches; } @Test diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java index ffd97c4953..55453956c7 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java @@ -538,7 +538,7 @@ public class DecompileOptions { grabFromProgram(program); - // assuming if one is not registered, then none area + // assuming if one is not registered, then none are if (!opt.isRegistered(PREDICATE_OPTIONSTRING)) { return; } diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AttributedSaveState.java b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AttributedSaveState.java new file mode 100644 index 0000000000..97a1ad097a --- /dev/null +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/AttributedSaveState.java @@ -0,0 +1,154 @@ +/* ### + * 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.options; + +import java.util.*; +import java.util.Map.Entry; + +import org.jdom.Attribute; +import org.jdom.Element; + +/** + * A version of {@link SaveState} that allows clients to add attributes to properties in this save + * state. The following code shows how to use this class: + * 
+ * AttributedSaveState ss = new AttributedSaveState();
+ * ss.putBoolean("Happy", true);
+ * 
+ * Map attrs = Map.of("MyAttribute", "AttributeValue");
+ * ss.addAttrbibutes("Happy", attrs);
+ *
+ * + *

In this example, the property "Happy" will be given the attribute "MyAttribute" with the value + * of "AttributeValue". This is useful for clients that wish to add attributes to individual + * properties, such as a date for tracking usage. + * + *

Usage Note: The given attributes are only supported when writing and reading xml. Json + * is not supported. + */ +public class AttributedSaveState extends SaveState { + + private Map> propertyAttributes; + + public AttributedSaveState() { + super(); + } + + public AttributedSaveState(String name) { + super(name); + } + + public AttributedSaveState(Element root) { + super(root); + } + + /** + * Adds the given map of attribute name/value pairs to this save state. + * @param propertyName the property name within this save state that will be attributed + * @param attributes the attributes + */ + public void addAttributes(String propertyName, Map attributes) { + getPropertyAttributes().put(propertyName, attributes); + } + + /** + * Removes all attributes associated with the given property name. + * @param propertyName the property name within this save state that has the given attributes + */ + public void removeAttributes(String propertyName) { + getPropertyAttributes().remove(propertyName); + } + + /** + * Gets the attributes currently associated with the given property name + * @param propertyName the property name for which to get attributes + * @return the attributes or null + */ + public Map getAttributes(String propertyName) { + return getPropertyAttributes().get(propertyName); + } + + private Map> getPropertyAttributes() { + if (propertyAttributes == null) { + propertyAttributes = new HashMap<>(); + } + return propertyAttributes; + } + + @Override + protected SaveState createSaveState() { + return new AttributedSaveState(); + } + + @Override + protected void initializeElement(Element e) { + + String name = e.getAttributeValue(NAME); + if (name == null) { + return; // sub-element; properties not supported + } + + // + // Overridden to add our attributes to the newly created element, used to create xml + // + Map attrs = getPropertyAttributes().get(name); + if (attrs != null) { + Set> entries = attrs.entrySet(); + for (Entry entry : entries) { + String key = entry.getKey(); + String value = entry.getValue(); + e.setAttribute(key, value); + } + } + } + + @Override + protected void processElement(Element element) { + super.processElement(element); + + String name = element.getAttributeValue(NAME); + if (name == null) { + return; // sub-element; properties not supported + } + + // + // Overridden to extract non-standard attributes from the given element. The element was + // created after restoring from xml. We extract the attributes that we added above in + // initializeElement(). + // + Map newAttrs = new HashMap<>(); + + @SuppressWarnings("unchecked") + List attrs = element.getAttributes(); + for (Attribute attr : attrs) { + + String attrName = attr.getName(); + String attrValue = switch (attrName) { + // ignore standard attributes, as they are managed by the parent class + case NAME, TYPE, VALUE -> null; + default -> attr.getValue(); + }; + + if (attrValue != null) { + newAttrs.put(attrName, attrValue); + } + } + + if (!newAttrs.isEmpty()) { + getPropertyAttributes().put(name, newAttrs); + } + } +} diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/GProperties.java b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/GProperties.java index 7cd4fa87ac..081a9bb19a 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/GProperties.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/GProperties.java @@ -78,7 +78,7 @@ public class GProperties { private static final String STATE = "STATE"; protected static final String TYPE = "TYPE"; protected static final String NAME = "NAME"; - private static final String VALUE = "VALUE"; + protected static final String VALUE = "VALUE"; public static DateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); private static final String ARRAY_ELEMENT_NAME = "A"; protected TreeMap map; // use ordered map for deterministic serialization @@ -505,35 +505,34 @@ public class GProperties { return root; } - protected Element createElement(String key, Object value) { + protected Element createElement(String propertyName, Object value) { Element elem = null; if (value instanceof Element) { - elem = createElementFromElement(key, (Element) value); + elem = createElementFromElement(propertyName, (Element) value); } else if (value instanceof Byte) { - elem = setAttributes(key, "byte", ((Byte) value).toString()); + elem = setAttributes(propertyName, "byte", ((Byte) value).toString()); } else if (value instanceof Short) { - elem = setAttributes(key, "short", ((Short) value).toString()); + elem = setAttributes(propertyName, "short", ((Short) value).toString()); } else if (value instanceof Integer) { - elem = setAttributes(key, "int", ((Integer) value).toString()); + elem = setAttributes(propertyName, "int", ((Integer) value).toString()); } else if (value instanceof Long) { - elem = setAttributes(key, "long", ((Long) value).toString()); + elem = setAttributes(propertyName, "long", ((Long) value).toString()); } else if (value instanceof Float) { - elem = setAttributes(key, "float", ((Float) value).toString()); + elem = setAttributes(propertyName, "float", ((Float) value).toString()); } else if (value instanceof Double) { - elem = setAttributes(key, "double", ((Double) value).toString()); + elem = setAttributes(propertyName, "double", ((Double) value).toString()); } else if (value instanceof Boolean) { - elem = setAttributes(key, "boolean", ((Boolean) value).toString()); + elem = setAttributes(propertyName, "boolean", ((Boolean) value).toString()); } else if (value instanceof String) { - elem = new Element(STATE); - elem.setAttribute(NAME, key); + elem = createElement(STATE, propertyName); elem.setAttribute(TYPE, "string"); if (XmlUtilities.hasInvalidXMLCharacters((String) value)) { elem.setAttribute("ENCODED_VALUE", NumericUtilities @@ -544,71 +543,66 @@ public class GProperties { } } else if (value instanceof Color) { - elem = setAttributes(key, "Color", Integer.toString(((Color) value).getRGB())); + elem = setAttributes(propertyName, "Color", Integer.toString(((Color) value).getRGB())); } else if (value instanceof Date) { - elem = setAttributes(key, "Date", DATE_FORMAT.format((Date) value)); + elem = setAttributes(propertyName, "Date", DATE_FORMAT.format((Date) value)); } else if (value instanceof File) { - elem = setAttributes(key, "File", ((File) value).getAbsolutePath()); + elem = setAttributes(propertyName, "File", ((File) value).getAbsolutePath()); } else if (value instanceof KeyStroke) { - elem = setAttributes(key, "KeyStroke", value.toString()); + elem = setAttributes(propertyName, "KeyStroke", value.toString()); } else if (value instanceof Font font) { - elem = setAttributes(key, "Font", toFontString(font)); + elem = setAttributes(propertyName, "Font", toFontString(font)); } else if (value instanceof byte[]) { - elem = new Element("BYTES"); - elem.setAttribute(NAME, key); + elem = createElement("BYTES", propertyName); elem.setAttribute(VALUE, NumericUtilities.convertBytesToString((byte[]) value)); } else if (value instanceof short[]) { - elem = setArrayAttributes(key, "short", value); + elem = setArrayAttributes(propertyName, "short", value); } else if (value instanceof int[]) { - elem = setArrayAttributes(key, "int", value); + elem = setArrayAttributes(propertyName, "int", value); } else if (value instanceof long[]) { - elem = setArrayAttributes(key, "long", value); + elem = setArrayAttributes(propertyName, "long", value); } else if (value instanceof float[]) { - elem = setArrayAttributes(key, "float", value); + elem = setArrayAttributes(propertyName, "float", value); } else if (value instanceof double[]) { - elem = setArrayAttributes(key, "double", value); + elem = setArrayAttributes(propertyName, "double", value); } else if (value instanceof boolean[]) { - elem = setArrayAttributes(key, "boolean", value); + elem = setArrayAttributes(propertyName, "boolean", value); } else if (value instanceof String[]) { - elem = setArrayAttributes(key, "string", value); + elem = setArrayAttributes(propertyName, "string", value); } else if (value instanceof Enum) { Enum e = (Enum) value; - elem = new Element("ENUM"); - elem.setAttribute(NAME, key); + elem = createElement("ENUM", propertyName); elem.setAttribute(TYPE, "enum"); elem.setAttribute("CLASS", e.getClass().getName()); elem.setAttribute(VALUE, e.name()); } else if (value instanceof GProperties) { Element savedElement = ((GProperties) value).saveToXml(); - elem = new Element(GPROPERTIES_TAG); - elem.setAttribute(NAME, key); + elem = createElement(GPROPERTIES_TAG, propertyName); elem.setAttribute(TYPE, G_PROPERTIES_TYPE); elem.addContent(savedElement); } else { - elem = new Element("NULL"); - elem.setAttribute(NAME, key); + elem = createElement("NULL", propertyName); } return elem; } - private Element setArrayAttributes(String key, String type, Object values) { - Element elem = new Element("ARRAY"); - elem.setAttribute(NAME, key); + private Element setArrayAttributes(String propertyName, String type, Object values) { + Element elem = createElement("ARRAY", propertyName); elem.setAttribute(TYPE, type); for (int i = 0; i < Array.getLength(values); i++) { Object value = Array.get(values, i); @@ -621,10 +615,19 @@ public class GProperties { return elem; } - private Element setAttributes(String key, String type, String value) { - Element elem; - elem = new Element(STATE); - elem.setAttribute(NAME, key); + protected Element createElement(String tag, String name) { + Element e = new Element(tag); + e.setAttribute(NAME, name); + initializeElement(e); + return e; + } + + protected void initializeElement(Element e) { + // subclasses may override + } + + private Element setAttributes(String propertyName, String type, String value) { + Element elem = createElement(STATE, propertyName); elem.setAttribute(TYPE, type); elem.setAttribute(VALUE, value); return elem; @@ -792,8 +795,7 @@ public class GProperties { } protected Element createElementFromElement(String internalKey, Element internalElement) { - Element newElement = new Element("XML"); - newElement.setAttribute(NAME, internalKey); + Element newElement = createElement("XML", internalKey); Element internalElementClone = (Element) internalElement.clone(); newElement.addContent(internalElementClone); @@ -1420,11 +1422,11 @@ public class GProperties { return true; } - private String toFontString(Font font) { + private static String toFontString(Font font) { return String.format("%s-%s-%s", font.getName(), getStyleString(font), font.getSize()); } - private String getStyleString(Font font) { + private static String getStyleString(Font font) { boolean bold = font.isBold(); boolean italic = font.isItalic(); if (bold && italic) { diff --git a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/SaveState.java b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/SaveState.java index e90e3c28b6..af93e7413a 100644 --- a/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/SaveState.java +++ b/Ghidra/Framework/Generic/src/main/java/ghidra/framework/options/SaveState.java @@ -17,6 +17,7 @@ package ghidra.framework.options; import java.io.File; import java.io.IOException; +import java.util.List; import org.jdom.Element; @@ -114,31 +115,86 @@ public class SaveState extends XmlProperties { return getAsType(name, null, SaveState.class); } + @SuppressWarnings("unchecked") @Override protected void processElement(Element element) { String tag = element.getName(); - - if (tag.equals("SAVE_STATE")) { - Element child = (Element) element.getChildren().get(0); - if (child != null) { - String name = element.getAttributeValue(NAME); - map.put(name, new SaveState(child)); - return; - } + if (!tag.equals("SAVE_STATE")) { + super.processElement(element); + return; } - super.processElement(element); + + /* + When using a SaveState inside of a SaveState, we produce xml that looks like this: + + + + + */ + + SaveState saveState = createSaveState(); + + List children = element.getChildren(); + if (children.isEmpty()) { + return; + } + + Element child = (Element) element.getChildren().get(0); + String childTag = child.getName(); + if (childTag.equals("SAVE_STATE")) { + /* + Old style tag, with one level of extra nesting + + + + + + + + */ + children = child.getChildren(); + } + + for (Element e : children) { + saveState.processElement(e); + } + + String parentName = element.getAttributeValue(NAME); + map.put(parentName, saveState); } + @SuppressWarnings("unchecked") @Override protected Element createElement(String key, Object value) { - if (value instanceof SaveState saveState) { - Element savedElement = saveState.saveToXml(); - Element element = new Element("SAVE_STATE"); - element.setAttribute(NAME, key); - element.setAttribute(TYPE, "SaveState"); - element.addContent(savedElement); - return element; + if (!(value instanceof SaveState saveState)) { + return super.createElement(key, value); } - return super.createElement(key, value); + + /* + When using a SaveState inside of a SaveState, we produce xml that looks like this: + + + + + */ + + Element savedElement = saveState.saveToXml(); + Element element = new Element("SAVE_STATE"); + element.setAttribute(NAME, key); + element.setAttribute(TYPE, "SaveState"); + + // do not write an extra intermediate node + List children = savedElement.getChildren(); + for (Element e : children) { + Element newElement = (Element) e.clone(); + element.addContent(newElement); + } + + return element; + } + + // allows subclasses to override how sub-save states are created + protected SaveState createSaveState() { + return new SaveState(); } } diff --git a/Ghidra/Framework/Generic/src/test/java/ghidra/util/SpyErrorLogger.java b/Ghidra/Framework/Generic/src/test/java/ghidra/util/SpyErrorLogger.java index e98b648da4..5c3fb54606 100644 --- a/Ghidra/Framework/Generic/src/test/java/ghidra/util/SpyErrorLogger.java +++ b/Ghidra/Framework/Generic/src/test/java/ghidra/util/SpyErrorLogger.java @@ -4,9 +4,9 @@ * 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. @@ -15,7 +15,7 @@ */ package ghidra.util; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import java.util.Arrays; import java.util.Iterator; @@ -88,6 +88,10 @@ public class SpyErrorLogger implements ErrorLogger, Iterable { messages.clear(); } + public boolean isEmtpy() { + return messages.isEmpty(); + } + public void assertLogMessage(String... words) { for (String message : this) { if (StringUtilities.containsAllIgnoreCase(message, words)) { diff --git a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/AbstractOptions.java b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/AbstractOptions.java index 93a99f05b4..adb2d50894 100644 --- a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/AbstractOptions.java +++ b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/AbstractOptions.java @@ -260,8 +260,11 @@ public abstract class AbstractOptions implements Options { if (option == null) { return null; } - if (option.getOptionType() != type) { - Msg.error(this, "Registered option incompatible with existing option: " + optionName, + OptionType existingType = option.getOptionType(); + if (existingType != type) { + Msg.error(this, "Registered option incompatible with existing option: '%s'. " + + "Existing type '%s'; registered type '%s'.".formatted(optionName, existingType, + type), new AssertException()); return null; } diff --git a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/Option.java b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/Option.java index 580ee354c2..d51d878fa7 100644 --- a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/Option.java +++ b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/Option.java @@ -16,6 +16,7 @@ package ghidra.framework.options; import java.beans.PropertyEditor; +import java.time.LocalDate; import java.util.Objects; import ghidra.util.HelpLocation; @@ -23,11 +24,15 @@ import ghidra.util.SystemUtilities; import utilities.util.reflection.ReflectionUtilities; public abstract class Option { + public static final String UNREGISTERED_OPTION = "Unregistered Option"; + private static final LocalDate ONE_YEAR_AGO = LocalDate.now().minusYears(1); + private final String name; private Object defaultValue; private boolean isRegistered; + private LocalDate lastRegisteredDate; private String description; private HelpLocation helpLocation; private OptionType optionType; @@ -45,7 +50,10 @@ public abstract class Option { this.defaultValue = defaultValue; this.isRegistered = isRegistered; this.propertyEditor = editor; - if (!isRegistered) { + if (isRegistered) { + lastRegisteredDate = LocalDate.now(); + } + else { recordInception(); } } @@ -67,6 +75,7 @@ public abstract class Option { defaultValue = defaultValue != null ? defaultValue : updatedDefaultValue; propertyEditor = propertyEditor != null ? propertyEditor : updatedEditor; isRegistered = true; + lastRegisteredDate = LocalDate.now(); } public abstract Object getCurrentValue(); @@ -74,7 +83,8 @@ public abstract class Option { public abstract void doSetCurrentValue(Object value); public void setCurrentValue(Object value) { - this.isRegistered = true; + isRegistered = true; + lastRegisteredDate = LocalDate.now(); doSetCurrentValue(value); } @@ -109,10 +119,44 @@ public abstract class Option { return value; } + public boolean wasRegisteredInPreviousSession() { + return lastRegisteredDate != null; + } + public boolean isRegistered() { return isRegistered; } + public void setLastRegisteredDate(LocalDate date) { + lastRegisteredDate = date; + } + + public LocalDate getLastRegisteredDate() { + LocalDate date = lastRegisteredDate; + if (date == null) { + // This implies a new option that has not been registered and was not in a saved tool. + // Pick an old date so the option will not linger. + date = ONE_YEAR_AGO; + } + return date; + } + + /** + * Returns true if the last registered date for this option is older than 1 year ago. That + * means that the option has not been registered and is likely no longer valid. This may not be + * true, if the given option still exists, but is only active on-demand by the user. If the + * option has expired, it will be removed. If it is still an existing on-demand option, it can + * again be saved when the user loads the owning provider and changes the option. In this case, + * it will remain in the tool for at least another year. + * @return true if expired + */ + public boolean hasExpired() { + if (lastRegisteredDate == null) { + return false; + } + return lastRegisteredDate.isBefore(ONE_YEAR_AGO); + } + public void restoreDefault() { setCurrentValue(defaultValue); } @@ -151,4 +195,5 @@ public abstract class Option { public OptionType getOptionType() { return optionType; } + } diff --git a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/ToolOptions.java b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/ToolOptions.java index 6cd8ea33b9..071c840c89 100644 --- a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/ToolOptions.java +++ b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/ToolOptions.java @@ -20,6 +20,8 @@ import java.awt.Font; import java.beans.PropertyEditor; import java.io.File; import java.lang.reflect.Constructor; +import java.time.LocalDate; +import java.time.format.DateTimeFormatter; import java.util.*; import javax.swing.KeyStroke; @@ -45,10 +47,16 @@ import ghidra.util.exception.AssertException; *

The Options Dialog shows the delimited hierarchy in tree format. */ public class ToolOptions extends AbstractOptions { + private static final String CLASS_ATTRIBUTE = "CLASS"; private static final String NAME_ATTRIBUTE = "NAME"; private static final String WRAPPED_OPTION_NAME = "WRAPPED_OPTION"; private static final String CLEARED_VALUE_ELEMENT_NAME = "CLEARED_VALUE"; + + public static final String LAST_REGISTERED_DATE_ATTIBUTE = "LAST_REGISTERED"; + public static final DateTimeFormatter LAST_REGISTERED_DATE_FORMATTER = + DateTimeFormatter.ISO_LOCAL_DATE; + public static final Set> PRIMITIVE_CLASSES = buildPrimitiveClassSet(); public static final Set> WRAPPABLE_CLASSES = buildWrappableClassSet(); @@ -91,7 +99,7 @@ public class ToolOptions extends AbstractOptions { public ToolOptions(Element root) { this(root.getAttributeValue(NAME_ATTRIBUTE)); - SaveState saveState = new SaveState(root); + AttributedSaveState saveState = new AttributedSaveState(root); readNonWrappedOptions(saveState); @@ -103,12 +111,23 @@ public class ToolOptions extends AbstractOptions { } } - private void readNonWrappedOptions(SaveState saveState) { + private void readNonWrappedOptions(AttributedSaveState saveState) { for (String optionName : saveState.getNames()) { Object object = saveState.getObject(optionName); - Option option = - createUnregisteredOption(optionName, OptionType.getOptionType(object), null); + + // Get the last registered date. No date implies an old xml format. + LocalDate date = LocalDate.now(); + Map attrs = saveState.getAttributes(optionName); + String dateString = attrs.get(LAST_REGISTERED_DATE_ATTIBUTE); + if (dateString != null) { + date = LocalDate.parse(dateString, LAST_REGISTERED_DATE_FORMATTER); + } + + OptionType type = OptionType.getOptionType(object); + Option option = createUnregisteredOption(optionName, type, null); option.doSetCurrentValue(object); // use doSet versus set so that it is not registered + option.setLastRegisteredDate(date); + valueMap.put(optionName, option); } } @@ -124,19 +143,19 @@ public class ToolOptions extends AbstractOptions { continue; // shouldn't happen } - String optionName = element.getAttributeValue(NAME_ATTRIBUTE); Class c = Class.forName(element.getAttributeValue(CLASS_ATTRIBUTE)); Constructor constructor = c.getDeclaredConstructor(); WrappedOption wo = (WrappedOption) constructor.newInstance(); wo.readState(new SaveState(element)); - if (wo instanceof WrappedCustomOption wrappedCustom && !wrappedCustom.isValid()) { continue; } + if (wo instanceof WrappedKeyStroke wrappedKs) { wo = wrappedKs.toWrappedActionTrigger(); } + String optionName = element.getAttributeValue(NAME_ATTRIBUTE); Option option = createUnregisteredOption(optionName, wo.getOptionType(), null); valueMap.put(optionName, option); @@ -149,6 +168,15 @@ public class ToolOptions extends AbstractOptions { else { option.doSetCurrentValue(wo.getObject()); // use doSet so that it is not registered } + + // Get the last registered date. No date implies an old xml format. + LocalDate date = LocalDate.now(); + String dateString = element.getAttributeValue(LAST_REGISTERED_DATE_ATTIBUTE); + if (dateString != null) { + date = LocalDate.parse(dateString, LAST_REGISTERED_DATE_FORMATTER); + } + + option.setLastRegisteredDate(date); } } @@ -162,7 +190,7 @@ public class ToolOptions extends AbstractOptions { */ public Element getXmlRoot(boolean includeDefaultBindings) { - SaveState saveState = new SaveState(XML_ELEMENT_NAME); + AttributedSaveState saveState = new AttributedSaveState(XML_ELEMENT_NAME); writeNonWrappedOptions(includeDefaultBindings, saveState); @@ -174,18 +202,31 @@ public class ToolOptions extends AbstractOptions { return root; } - private void writeNonWrappedOptions(boolean includeDefaultBindings, SaveState saveState) { + private void writeNonWrappedOptions(boolean includeDefaultBindings, + AttributedSaveState saveState) { for (String optionName : valueMap.keySet()) { - Option optionValue = valueMap.get(optionName); - if (includeDefaultBindings || !optionValue.isDefault()) { - Object value = optionValue.getValue(null); - if (isSupportedBySaveState(value)) { - saveState.putObject(optionName, value); - } + Option option = valueMap.get(optionName); + if (includeDefaultBindings || !option.isDefault()) { + writeNonWrappedOption(saveState, optionName, option); } } } + private void writeNonWrappedOption(AttributedSaveState saveState, String optionName, + Option option) { + Object value = option.getValue(null); + if (!isSupportedBySaveState(value)) { + return; + } + + saveState.putObject(optionName, value); + + LocalDate date = option.getLastRegisteredDate(); + String dateString = date.format(LAST_REGISTERED_DATE_FORMATTER); + Map attrs = Map.of(LAST_REGISTERED_DATE_ATTIBUTE, dateString); + saveState.addAttributes(optionName, attrs); + } + private void writeWrappedOptions(boolean includeDefaultBindings, Element root) { for (String optionName : valueMap.keySet()) { Option option = valueMap.get(optionName); @@ -195,36 +236,47 @@ public class ToolOptions extends AbstractOptions { } if (includeDefaultBindings || !option.isDefault()) { - Object value = option.getCurrentValue(); - if (isSupportedBySaveState(value)) { - continue; // handled above - } - - WrappedOption wrappedOption = wrapOption(option); - if (wrappedOption == null) { - continue; // cannot write an option without a value to determine its type - } - - SaveState ss = new SaveState(WRAPPED_OPTION_NAME); - Element elem = null; - if (value == null) { - // Handle the null case ourselves, not using the wrapped option (and when - // reading from xml) so the logic does not need to be in each wrapped option - elem = ss.saveToXml(); - elem.addContent(new Element(CLEARED_VALUE_ELEMENT_NAME)); - } - else { - wrappedOption.writeState(ss); - elem = ss.saveToXml(); - } - - elem.setAttribute(NAME_ATTRIBUTE, optionName); - elem.setAttribute(CLASS_ATTRIBUTE, wrappedOption.getClass().getName()); - root.addContent(elem); + writeWrappedOption(root, optionName, option); } } } + private void writeWrappedOption(Element root, String optionName, Option option) { + Object value = option.getCurrentValue(); + if (isSupportedBySaveState(value)) { + return; // handled above + } + + WrappedOption wrappedOption = wrapOption(option); + if (wrappedOption == null) { + return; // cannot write an option without a value to determine its type + } + + SaveState ss = new SaveState(WRAPPED_OPTION_NAME); + Element element = null; + if (value == null) { + // Handle the null case ourselves, not using the wrapped option (and when + // reading from xml) so the logic does not need to be in each wrapped option + element = ss.saveToXml(); + element.addContent(new Element(CLEARED_VALUE_ELEMENT_NAME)); + } + else { + wrappedOption.writeState(ss); + element = ss.saveToXml(); + } + + element.setAttribute(NAME_ATTRIBUTE, optionName); + + String className = wrappedOption.getClass().getName(); + element.setAttribute(CLASS_ATTRIBUTE, className); + + LocalDate date = option.getLastRegisteredDate(); + String dateString = date.format(LAST_REGISTERED_DATE_FORMATTER); + element.setAttribute(LAST_REGISTERED_DATE_ATTIBUTE, dateString); + + root.addContent(element); + } + private boolean isSupportedBySaveState(Object obj) { if (obj == null) { return false; @@ -310,7 +362,7 @@ public class ToolOptions extends AbstractOptions { List optionNames = new ArrayList<>(valueMap.keySet()); for (String optionName : optionNames) { Option optionState = valueMap.get(optionName); - if (!optionState.isRegistered()) { + if (optionState.hasExpired()) { removeOption(optionName); } } @@ -377,11 +429,15 @@ public class ToolOptions extends AbstractOptions { Set keySet = valueMap.keySet(); for (String propertyName : keySet) { Option optionState = valueMap.get(propertyName); - if (optionState.isRegistered()) { + if (optionState.isRegistered() || optionState.wasRegisteredInPreviousSession()) { continue; } - Msg.warn(this, "Unregistered property \"" + propertyName + "\" in Options \"" + name + - "\"\n " + optionState.getInceptionInformation()); + + // getting here means that this option was used in the current tool session, but was not + // registered this session or in a previous session + Msg.warn(this, + "Unregistered property \"" + propertyName + "\" in Options \"" + name + + "\"\n " + optionState.getInceptionInformation()); } } diff --git a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/WrappedOption.java b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/WrappedOption.java index 71bae742b6..8a438afd14 100644 --- a/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/WrappedOption.java +++ b/Ghidra/Framework/Gui/src/main/java/ghidra/framework/options/WrappedOption.java @@ -1,13 +1,12 @@ /* ### * 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. * 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. @@ -17,30 +16,29 @@ package ghidra.framework.options; /** - * Wrapper class for an object that represents a property value and is - * saved as a set of primitives. + * Wrapper class for an object that represents a property value and is saved as a set of primitives. */ public interface WrappedOption { /** - * Get the object that is the property value. + * {@return Get the object that is the property value} */ public abstract Object getObject(); /** - * Concrete subclass of WrappedOption should read all of its - * state from the given saveState object. + * Subclasses of WrappedOption should read all state from the given save state object. * @param saveState container of state information */ public abstract void readState(SaveState saveState); /** - * Concrete subclass of WrappedOption should write all of its - * state to the given saveState object. + * Subclasses of WrappedOption should write all state to the given save state object. * @param saveState container of state information */ public abstract void writeState(SaveState saveState); + /** + * {@return the option type} + */ public abstract OptionType getOptionType(); - }