Bug 579372 - Removing project preference values doesn't work until
restart

Make sure the deleted nodes in file are also deleted from the memory
preferences.

Change-Id: Ib2788e9f0be5eac8d0ef92acec46fb015d53a688
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.resources/+/192164
Tested-by: Platform Bot <platform-bot@eclipse.org>
diff --git a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/ProjectPreferences.java b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/ProjectPreferences.java
index fce5725..3d76270 100644
--- a/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/ProjectPreferences.java
+++ b/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/ProjectPreferences.java
@@ -19,6 +19,7 @@
 import java.io.*;
 import java.text.MessageFormat;
 import java.util.*;
+import java.util.Map.Entry;
 import org.eclipse.core.internal.preferences.*;
 import org.eclipse.core.internal.utils.*;
 import org.eclipse.core.resources.*;
@@ -40,6 +41,8 @@
 public class ProjectPreferences extends EclipsePreferences {
 	static final String PREFS_REGULAR_QUALIFIER = ResourcesPlugin.PI_RESOURCES;
 	static final String PREFS_DERIVED_QUALIFIER = PREFS_REGULAR_QUALIFIER + ".derived"; //$NON-NLS-1$
+	static final String PLACEHOLDER = "<temporary_value_placeholder>"; //$NON-NLS-1$
+
 	/**
 	 * Cache which nodes have been loaded from disk
 	 */
@@ -198,14 +201,11 @@
 				Policy.debug("Unable to determine preference file or file does not exist for node: " + node.absolutePath()); //$NON-NLS-1$
 			return;
 		}
-		Properties fromDisk = loadProperties(file);
-		// no work to do
-		if (fromDisk.isEmpty())
-			return;
-		// create a new node to store the preferences in.
-		IExportedPreferences myNode = (IExportedPreferences) ExportedPreferences.newRoot().node(node.absolutePath());
-		convertFromProperties((EclipsePreferences) myNode, fromDisk, false);
-		//flag that we are currently reading, to avoid unnecessary writing
+
+		// Create special "overriding" preferences to be applied
+		ExportedPreferences myNode = overridingPreferences(node, file);
+
+		// flag that we are currently reading, to avoid unnecessary writing
 		boolean oldIsReading = node.isReading;
 		node.isReading = true;
 		try {
@@ -215,6 +215,66 @@
 		}
 	}
 
+	/**
+	 * Creates new preferences node from given file that can be used to apply via
+	 * preferences service and override the current in-memory preferences state.
+	 *
+	 * @param current in-memory state
+	 * @param file    new state on the disk to be loaded
+	 * @return new node that contains everything required to apply new state
+	 * @throws BackingStoreException
+	 * @see PreferencesService#applyPreferences(IExportedPreferences)
+	 */
+	private static ExportedPreferences overridingPreferences(ProjectPreferences current, IFile file)
+			throws BackingStoreException {
+		Properties fromDisk = loadProperties(file);
+
+		Properties fromMemory = new Properties();
+		current.convertToProperties(fromMemory, ""); //$NON-NLS-1$
+
+		// Re-create all the child elements existing in memory but not in the file
+		// in the node to be applied to preferences, so we can delete previously
+		// existing node values
+		Set<Entry<Object, Object>> set = fromMemory.entrySet();
+		for (Entry<Object, Object> entry : set) {
+			String key = (String) entry.getKey();
+			// Only touch nodes that are not in the file
+			if (!fromDisk.containsKey(key)) {
+				// and only touch "children" nodes, like encoding/<project>
+				if (key.indexOf('/') > 0) {
+					fromDisk.put(key, PLACEHOLDER);
+				}
+			}
+		}
+
+		// create a new node to store the preferences in.
+		ExportedPreferences myNode = (ExportedPreferences) ExportedPreferences.newRoot().node(current.absolutePath());
+		convertFromProperties(myNode, fromDisk, false);
+
+		// Makes sure the properties are overridden, not merged by marking children
+		// via setExportRoot() - this will remove & recreate them in memory,
+		// so only new values from the file are kept, old ones are removed
+		// See PreferencesService#applyPreferences(IExportedPreferences)
+		myNode.accept(child -> {
+			String[] keys = child.keys();
+			boolean nodeShouldBeRemoved = false;
+			// Remove our placeholders, we don't need them, only nodes
+			for (String key : keys) {
+				if (PLACEHOLDER.equals(child.get(key, null))) {
+					child.remove(key);
+					nodeShouldBeRemoved = true;
+				}
+			}
+			// Only mark child nodes for deletion, if we do that for the root
+			// node, the preferences file will be re-created (which leads to errors)
+			if (child != myNode && nodeShouldBeRemoved) {
+				((ExportedPreferences) child).setExportRoot();
+			}
+			return true;
+		});
+		return myNode;
+	}
+
 	static void removeNode(Preferences node) throws CoreException {
 		String message = NLS.bind(Messages.preferences_removeNodeException, node.absolutePath());
 		try {
diff --git a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/resources/ProjectPreferencesTest.java b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/resources/ProjectPreferencesTest.java
index 168b0e8..3d91bf9 100644
--- a/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/resources/ProjectPreferencesTest.java
+++ b/tests/org.eclipse.core.tests.resources/src/org/eclipse/core/tests/internal/resources/ProjectPreferencesTest.java
@@ -696,6 +696,72 @@
 	}
 
 	/*
+	 * Bug 579372 - property removals are not detected.
+	 */
+	public void test_579372() throws Exception {
+		IProject project = getProject(getUniqueString());
+		ensureExistsInWorkspace(project, true);
+		Preferences node = new ProjectScope(project).getNode(ResourcesPlugin.PI_RESOURCES);
+		node.put("key1", "value1");
+		node.node("child").put("key", "childValue1");
+		node.node("child").node("node").put("key", "childValue2");
+		node.flush();
+		IFile prefFile = getFileInWorkspace(project, ResourcesPlugin.PI_RESOURCES);
+		assertTrue("Preferences missing", prefFile.exists());
+		Properties properties = new Properties();
+		try (InputStream contents = prefFile.getContents()) {
+			properties.load(contents);
+		}
+		assertEquals("value1", properties.get("key1"));
+		assertEquals("childValue1", properties.get("child/key"));
+		assertEquals("childValue2", properties.get("child/node/key"));
+		assertEquals("UTF-8", properties.get("encoding/<project>"));
+
+		// adds property
+		properties.put("key0", "value0");
+		// adds property
+		properties.put("child2/key", "childValue3");
+		// changes property
+		properties.put("key1", "value2");
+		// removes a property
+		properties.remove("child/key");
+		properties.remove("child/node/key");
+		properties.remove("encoding/<project>");
+
+		ByteArrayOutputStream tempOutput = new ByteArrayOutputStream();
+		properties.store(tempOutput, null);
+		ByteArrayInputStream tempInput = new ByteArrayInputStream(tempOutput.toByteArray());
+		prefFile.setContents(tempInput, false, false, getMonitor());
+
+		// here, project preferences should have caught up with the changes
+		node = new ProjectScope(project).getNode(ResourcesPlugin.PI_RESOURCES);
+		// property was added
+		assertEquals("value0", node.get("key0", null));
+		// property value changed
+		assertEquals("value2", node.get("key1", null));
+
+		List<String> children = List.of(node.childrenNames());
+		assertTrue(children.contains("child"));
+		assertTrue(children.contains("child2"));
+		assertTrue(children.contains("encoding"));
+
+		// Added "child2/key" property
+		assertEquals(Arrays.asList("key"), Arrays.asList(node.node("child2").keys()));
+		assertEquals("childValue3", node.node("child2").get("key", null));
+
+		// Removed "child/key" and "child/node/key" have no values anymore (only
+		// structure)
+		assertEquals(Arrays.asList(), Arrays.asList(node.node("child").keys()));
+		assertEquals(Arrays.asList("node"), Arrays.asList(node.node("child").childrenNames()));
+		assertEquals(Arrays.asList(), Arrays.asList(node.node("child").node("node").keys()));
+		assertEquals(Arrays.asList(), Arrays.asList(node.node("child").node("node").childrenNames()));
+
+		// Removed "encoding/<project>" has no value anymore (only structure)
+		assertEquals(Arrays.asList(), Arrays.asList(node.node("encoding").keys()));
+		assertEquals(Arrays.asList(), Arrays.asList(node.node("encoding").childrenNames()));
+	}
+
+	/*
 	 * Bug 256900 - When the preferences file is copied between projects, the corresponding preferences
 	 * should be updated.
 	 */