Clean up preference spy

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/PreferenceSpyConfiguration.java

@@ -21,14 +21,13 @@
  */
 public class PreferenceSpyConfiguration {
 
-	private static String bundleId = "org.eclipse.pde.spy.preferences";
+	private static final String BUNDLE_ID = "org.eclipse.pde.spy.preferences";
 
 	private static IEclipsePreferences preferenceStore;
 
 	public static IEclipsePreferences getPreferenceStore() {
-		// Create the preference store lazily.
 		if (preferenceStore == null) {
-			preferenceStore = 	InstanceScope.INSTANCE.getNode(bundleId);;
+			preferenceStore = InstanceScope.INSTANCE.getNode(BUNDLE_ID);
 		}
 		return preferenceStore;
 	}

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/addon/PreferenceSpyAddon.java

@@ -58,7 +58,7 @@ public class PreferenceSpyAddon {
 
 	@Inject
 	@Optional
-	public void initialzePreferenceSpy(
+	public void initializePreferenceSpy(
 			@Preference(value = PreferenceConstants.TRACE_PREFERENCES) boolean tracePreferences) {
 		if (tracePreferences) {
 			registerVisitors();

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/handler/CollapseAllHandler.java

@@ -21,9 +21,8 @@ public class CollapseAllHandler {
 
 	@Execute
 	public void execute(MPart part) {
-		Object partImpl = part.getObject();
-		if (partImpl instanceof PreferenceSpyPart) {
-			((PreferenceSpyPart) partImpl).getViewer().collapseAll();
+		if (part.getObject() instanceof PreferenceSpyPart spy) {
+			spy.getViewer().collapseAll();
 		}
 	}
 

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/handler/ExpandAllHandler.java

@@ -21,9 +21,8 @@ public class ExpandAllHandler {
 
 	@Execute
 	public void execute(MPart part) {
-		Object partImpl = part.getObject();
-		if (partImpl instanceof PreferenceSpyPart) {
-			((PreferenceSpyPart) partImpl).getViewer().expandAll();
+		if (part.getObject() instanceof PreferenceSpyPart spy) {
+			spy.getViewer().expandAll();
 		}
 	}
 

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/handler/ShowAllPreferencesHandler.java

@@ -32,6 +32,9 @@
 import org.osgi.service.prefs.BackingStoreException;
 
 public class ShowAllPreferencesHandler {
+
+	private static final String DEFAULT_VALUE_MARKER = "*default*";
+
 	@Execute
 	public void execute(Shell shell, IEventBroker eventBroker) {
 		Map<String, PreferenceNodeEntry> preferenceEntries = new HashMap<>();
@@ -44,7 +47,7 @@ public void execute(Shell shell, IEventBroker eventBroker) {
 			PreferenceNodeEntry preferenceNodeEntry = preferenceEntries.computeIfAbsent(node.absolutePath(),
 					PreferenceNodeEntry::new);
 			for (String key : keys) {
-				String value = node.get(key, "*default*");
+				String value = node.get(key, DEFAULT_VALUE_MARKER);
 				preferenceNodeEntry.addChildren(new PreferenceEntry(node.absolutePath(), key, value, value));
 			}
 			return true;

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/model/PreferenceEntry.java

@@ -13,6 +13,8 @@
  *******************************************************************************/
 package org.eclipse.pde.spy.preferences.model;
 
+import java.util.Objects;
+
 public class PreferenceEntry extends AbstractModelObject {
 
 	public enum Fields {
@@ -48,13 +50,6 @@ public PreferenceEntry(String nodePath, String key, String oldValue, String newV
 		this.newValue = newValue;
 	}
 
-	public PreferenceEntry(PreferenceEntry parent, String nodePath, String key, String oldValue, String newValue) {
-		this.nodePath = nodePath;
-		this.key = key;
-		this.oldValue = oldValue;
-		this.newValue = newValue;
-	}
-
 	public PreferenceEntry getParent() {
 		return parent;
 	}
@@ -103,70 +98,23 @@ public void setRecentlyChanged(boolean recentlyChanged) {
 		firePropertyChange("recentlyChanged", this.recentlyChanged, this.recentlyChanged = recentlyChanged);
 	}
 
+	// Identity is the (nodePath, key) pair. Mutable fields like oldValue/newValue/recentlyChanged
+	// must not participate, because instances are stored in a WritableSet and mutated in place.
 	@Override
 	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + ((key == null) ? 0 : key.hashCode());
-		result = prime * result + ((newValue == null) ? 0 : newValue.hashCode());
-		result = prime * result + ((nodePath == null) ? 0 : nodePath.hashCode());
-		result = prime * result + ((oldValue == null) ? 0 : oldValue.hashCode());
-		result = prime * result + ((parent == null) ? 0 : parent.hashCode());
-		result = prime * result + (recentlyChanged ? 1231 : 1237);
-		return result;
+		return Objects.hash(nodePath, key);
 	}
 
 	@Override
 	public boolean equals(Object obj) {
 		if (this == obj) {
 			return true;
 		}
-		if (obj == null) {
-			return false;
-		}
-		if (getClass() != obj.getClass()) {
+		if (obj == null || getClass() != obj.getClass()) {
 			return false;
 		}
 		PreferenceEntry other = (PreferenceEntry) obj;
-		if (key == null) {
-			if (other.key != null) {
-				return false;
-			}
-		} else if (!key.equals(other.key)) {
-			return false;
-		}
-		if (newValue == null) {
-			if (other.newValue != null) {
-				return false;
-			}
-		} else if (!newValue.equals(other.newValue)) {
-			return false;
-		}
-		if (nodePath == null) {
-			if (other.nodePath != null) {
-				return false;
-			}
-		} else if (!nodePath.equals(other.nodePath)) {
-			return false;
-		}
-		if (oldValue == null) {
-			if (other.oldValue != null) {
-				return false;
-			}
-		} else if (!oldValue.equals(other.oldValue)) {
-			return false;
-		}
-		if (parent == null) {
-			if (other.parent != null) {
-				return false;
-			}
-		} else if (!parent.equals(other.parent)) {
-			return false;
-		}
-		if (recentlyChanged != other.recentlyChanged) {
-			return false;
-		}
-		return true;
+		return Objects.equals(nodePath, other.nodePath) && Objects.equals(key, other.key);
 	}
 
 	public long getTime() {

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/model/PreferenceEntryManager.java

@@ -27,10 +27,6 @@ public PreferenceNodeEntry getRecentPreferenceNodeEntry(String nodePath) {
 		return recentPreferenceEntries.get(nodePath);
 	}
 
-	public PreferenceNodeEntry removeRecentPreferenceNodeEntry(String nodePath) {
-		return recentPreferenceEntries.remove(nodePath);
-	}
-
 	public void clearRecentPreferenceNodeEntry() {
 		recentPreferenceEntries.clear();
 	}

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/model/PreferenceNodeEntry.java

@@ -21,7 +21,7 @@
 
 public class PreferenceNodeEntry extends PreferenceEntry {
 
-	private IObservableSet<Object> preferenceEntries = new WritableSet<>();
+	private final IObservableSet<PreferenceEntry> preferenceEntries = new WritableSet<>();
 
 	public PreferenceNodeEntry() {
 		super();
@@ -32,27 +32,23 @@ public PreferenceNodeEntry(String nodePath) {
 	}
 
 	public void addChildren(Collection<PreferenceEntry> entries) {
-		getPreferenceEntries().addAll(entries);
+		preferenceEntries.addAll(entries);
 	}
 
 	public boolean addChildren(PreferenceEntry... entry) {
-		return getPreferenceEntries().addAll(Arrays.asList(entry));
+		return preferenceEntries.addAll(Arrays.asList(entry));
 	}
 
 	public void removeChildren(Collection<PreferenceEntry> entries) {
-		getPreferenceEntries().removeAll(entries);
+		preferenceEntries.removeAll(entries);
 	}
 
 	public void removeChildren(PreferenceEntry... entry) {
-		getPreferenceEntries().removeAll(Arrays.asList(entry));
+		preferenceEntries.removeAll(Arrays.asList(entry));
 	}
 
-	public IObservableSet<Object> getPreferenceEntries() {
+	public IObservableSet<PreferenceEntry> getPreferenceEntries() {
 		return preferenceEntries;
 	}
 
-	public void setPreferenceEntries(IObservableSet<Object> preferenceEntries) {
-		this.preferenceEntries = preferenceEntries;
-	}
-
 }

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/parts/PreferenceSpyPart.java

@@ -19,7 +19,6 @@
 
 import org.eclipse.core.databinding.beans.typed.BeanProperties;
 import org.eclipse.core.databinding.observable.Realm;
-import org.eclipse.core.databinding.observable.set.IObservableSet;
 import org.eclipse.core.databinding.property.Properties;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences.PreferenceChangeEvent;
 import org.eclipse.e4.core.di.annotations.Optional;
@@ -32,7 +31,6 @@
 import org.eclipse.jface.databinding.swt.DisplayRealm;
 import org.eclipse.jface.resource.FontDescriptor;
 import org.eclipse.jface.viewers.ColumnLabelProvider;
-import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.jface.viewers.TreeViewer;
 import org.eclipse.jface.viewers.TreeViewerColumn;
@@ -77,10 +75,8 @@ public void postConstruct(Composite parent, final ESelectionService selectionSer
 		table.setLinesVisible(true);
 
 		filteredTree.getViewer().addSelectionChangedListener(event -> {
-			ISelection selection = event.getSelection();
-			if (selection instanceof IStructuredSelection) {
-				ArrayList<PreferenceEntry> preferenceEntries = new ArrayList<>(
-						((IStructuredSelection) selection).toList());
+			if (event.getSelection() instanceof IStructuredSelection structured) {
+				ArrayList<PreferenceEntry> preferenceEntries = new ArrayList<>(structured.toList());
 				selectionService.setSelection(preferenceEntries);
 			}
 		});
@@ -146,7 +142,6 @@ public void preferenceChanged(
 			preferenceNodeEntry.addChildren(preferenceEntry);
 			preferenceEntry.setParent(preferenceNodeEntry);
 			preferenceEntryManager.addChildren(preferenceNodeEntry);
-			filteredTree.getViewer().setInput(preferenceEntryManager);
 			preferenceEntryManager.putRecentPreferenceEntry(event.getNode().absolutePath(), preferenceNodeEntry);
 		} else {
 			preferenceEntry.setParent(preferenceNodeEntry);
@@ -168,14 +163,10 @@ public void preferenceChanged(
 	}
 
 	private PreferenceEntry findPreferenceEntry(PreferenceEntry preferenceEntry) {
-		PreferenceEntry parent = preferenceEntry.getParent();
-		if (parent instanceof PreferenceNodeEntry) {
-			IObservableSet<Object> preferenceEntries = ((PreferenceNodeEntry) parent).getPreferenceEntries();
-			for (Object object : preferenceEntries) {
-				if (object instanceof PreferenceEntry existingPreferenceEntry) {
-					if (existingPreferenceEntry.getKey().equals(preferenceEntry.getKey())) {
-						return existingPreferenceEntry;
-					}
+		if (preferenceEntry.getParent() instanceof PreferenceNodeEntry parentNode) {
+			for (PreferenceEntry existing : parentNode.getPreferenceEntries()) {
+				if (existing.equals(preferenceEntry)) {
+					return existing;
 				}
 			}
 		}
@@ -192,20 +183,24 @@ public void preferenceChanged(
 
 	@Inject
 	@Optional
-	public void DeletePreferenceEntries(
+	public void deletePreferenceEntries(
 			@UIEventTopic(PreferenceSpyEventTopics.PREFERENCESPY_PREFERENCE_ENTRIES_DELETE) List<PreferenceEntry> preferenceEntries) {
 		if (preferenceEntries != null && !preferenceEntries.isEmpty()) {
 			for (PreferenceEntry preferenceEntry : preferenceEntries) {
-				preferenceEntryManager.removeChildren(preferenceEntry);
+				PreferenceEntry parent = preferenceEntry.getParent();
+				if (parent instanceof PreferenceNodeEntry parentNode) {
+					parentNode.removeChildren(preferenceEntry);
+				} else {
+					preferenceEntryManager.removeChildren(preferenceEntry);
+				}
 			}
-			preferenceEntryManager.removeChildren(preferenceEntries);
 			filteredTree.getViewer().refresh();
 		}
 	}
 
 	@Inject
 	@Optional
-	public void DeleteAllPreferenceEntries(
+	public void deleteAllPreferenceEntries(
 			@UIEventTopic(PreferenceSpyEventTopics.PREFERENCESPY_PREFERENCE_ENTRIES_DELETE_ALL) List<PreferenceEntry> preferenceEntries) {
 		if (preferenceEntryManager != null) {
 			preferenceEntryManager.clearRecentPreferenceNodeEntry();

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/viewer/PreferenceEntriesContentProvider.java

@@ -17,18 +17,16 @@
 import java.util.List;
 
 import org.eclipse.core.databinding.observable.masterdetail.IObservableFactory;
-import org.eclipse.core.databinding.observable.set.IObservableSet;
 import org.eclipse.jface.databinding.viewers.ObservableSetTreeContentProvider;
 import org.eclipse.jface.databinding.viewers.TreeStructureAdvisor;
 import org.eclipse.pde.spy.preferences.model.PreferenceEntry;
 import org.eclipse.pde.spy.preferences.model.PreferenceNodeEntry;
 
-@SuppressWarnings("rawtypes")
+@SuppressWarnings({ "rawtypes", "unchecked" })
 public class PreferenceEntriesContentProvider extends ObservableSetTreeContentProvider {
 
 	private boolean hierarchicalLayout;
 
-	@SuppressWarnings("unchecked")
 	public PreferenceEntriesContentProvider(IObservableFactory setFactory, TreeStructureAdvisor structureAdvisor) {
 		super(setFactory, structureAdvisor);
 	}
@@ -43,20 +41,19 @@ public Object[] getElements(Object inputElement) {
 		List<PreferenceEntry> childList = new ArrayList<>();
 
 		for (Object object : children) {
-			getChildren(object, childList);
+			collectLeaves(object, childList);
 		}
 
 		return childList.toArray();
 	}
 
-	private void getChildren(Object element, List<PreferenceEntry> childList) {
-		if (element instanceof PreferenceNodeEntry) {
-			IObservableSet preferenceEntries = ((PreferenceNodeEntry) element).getPreferenceEntries();
-			for (Object object : preferenceEntries) {
-				getChildren(object, childList);
+	private void collectLeaves(Object element, List<PreferenceEntry> childList) {
+		if (element instanceof PreferenceNodeEntry nodeEntry) {
+			for (PreferenceEntry child : nodeEntry.getPreferenceEntries()) {
+				collectLeaves(child, childList);
 			}
-		} else if (element instanceof PreferenceEntry) {
-			childList.add((PreferenceEntry) element);
+		} else if (element instanceof PreferenceEntry preferenceEntry) {
+			childList.add(preferenceEntry);
 		}
 	}
 

ui/org.eclipse.pde.spy.preferences/src/org/eclipse/pde/spy/preferences/viewer/PreferenceEntryViewerComparator.java

@@ -35,7 +35,7 @@ public int compare(Viewer viewer, Object e1, Object e2) {
 			long time2 = entry2.getTime();
 
 			if (time != 0 && time2 != 0) {
-				return (int) (time2 - time);
+				return Long.compare(time2, time);
 			}
 		}