Bug 335344 - [External Settings Provider] Settings lost after changing language IDs

3 files changed, 124 insertions, 11 deletions

diff --git a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
index 9b79c80f43c..7dce5fa0d77 100644
--- a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
+++ b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/ExternalSettingsProviderTests.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2009 Intel Corporation and others.
+ * Copyright (c) 2007, 2011 Intel Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -8,11 +8,12 @@
  * Contributors:
  * Intel Corporation - Initial API and implementation
  * James Blackburn (Broadcom Corp.)
+ * Christian Walther (Indel AG) - [335344] test for changing language IDs
  *******************************************************************************/
 package org.eclipse.cdt.core.settings.model;
 
 import java.util.Arrays;
-
+import java.util.HashMap;
 import junit.framework.TestSuite;
 
 import org.eclipse.cdt.core.dom.IPDOMManager;
@@ -28,7 +29,7 @@ import org.eclipse.core.runtime.Path;
 
 public class ExternalSettingsProviderTests extends BaseTestCase{
 	private static final String PROJ_NAME_PREFIX = "espt_";
-	ICProject p1, p2, p3, p4;
+	ICProject p1, p2, p3, p4, p5;
 	
 	public static TestSuite suite() {
 		return suite(ExternalSettingsProviderTests.class, "_");
@@ -39,6 +40,7 @@ public class ExternalSettingsProviderTests extends BaseTestCase{
 		p2 = CProjectHelper.createNewStileCProject(PROJ_NAME_PREFIX + "b", IPDOMManager.ID_NO_INDEXER);
 		p3 = CProjectHelper.createNewStileCProject(PROJ_NAME_PREFIX + "c", IPDOMManager.ID_NO_INDEXER);
 		p4 = CProjectHelper.createNewStileCProject(PROJ_NAME_PREFIX + "d", IPDOMManager.ID_NO_INDEXER);
+		p5 = CProjectHelper.createNewStileCProject(PROJ_NAME_PREFIX + "e", IPDOMManager.ID_NO_INDEXER);
 	}
 	
 	/**
@@ -350,6 +352,79 @@ public class ExternalSettingsProviderTests extends BaseTestCase{
 		return out;
 	}
 
+	/**
+	 * Test if changing only the language IDs of an external setting works
+	 */
+	public void testChangeLanguageSet() throws CoreException {
+		TestExtSettingsProvider.setVariantNum(2);
+
+		CoreModel model = CoreModel.getDefault();
+		ICProjectDescriptionManager mngr = model.getProjectDescriptionManager();
+		IProject project = p5.getProject();
+		
+		// add external settings provider
+		ICProjectDescription des = model.getProjectDescription(project);
+		ICConfigurationDescription cfgDes = des.getConfigurations()[0];
+		String[] extPIds = new String[]{TestExtSettingsProvider.TEST_EXTERNAL_PROVIDER_ID};
+		cfgDes.setExternalSettingsProviderIds(extPIds);
+		model.setProjectDescription(project, des);
+		
+		// read out the settings it caused
+		des = model.getProjectDescription(project, false);
+		cfgDes = des.getConfigurations()[0];
+		ICFolderDescription root = cfgDes.getRootFolderDescription();
+		HashMap<String, ICLanguageSetting> languageSettingsById = new HashMap<String, ICLanguageSetting>();
+		for (ICLanguageSetting s: root.getLanguageSettings()) {
+			languageSettingsById.put(s.getLanguageId(), s);
+		}
+		
+		ICLanguageSetting ls;
+		ICLanguageSettingEntry[] entries;
+		ICLanguageSettingEntry[] expectedEntriesSet = new ICLanguageSettingEntry[]{
+				new CMacroEntry("m_c", "mv_c", 0)
+		};
+		ICLanguageSettingEntry[] expectedEntriesUnset = new ICLanguageSettingEntry[] {};
+		
+		// setting should be present for assembly but not for C
+		ls = languageSettingsById.get("org.eclipse.cdt.core.assembly");
+		assertNotNull(ls);
+		entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+		assertEquals(1, entries.length);
+		assertTrue(Arrays.equals(expectedEntriesSet, entries));
+		
+		ls = languageSettingsById.get("org.eclipse.cdt.core.gcc");
+		assertNotNull(ls);
+		entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+		assertEquals(0, entries.length);
+		assertTrue(Arrays.equals(expectedEntriesUnset, entries));
+		
+		// update settings provider
+		TestExtSettingsProvider.setVariantNum(3);
+		mngr.updateExternalSettingsProviders(extPIds, null);
+		
+		// read out the settings it caused
+		des = model.getProjectDescription(project, false);
+		cfgDes = des.getConfigurations()[0];
+		root = cfgDes.getRootFolderDescription();
+		languageSettingsById = new HashMap<String, ICLanguageSetting>();
+		for (ICLanguageSetting s: root.getLanguageSettings()) {
+			languageSettingsById.put(s.getLanguageId(), s);
+		}
+		
+		// setting should be present for both now
+		ls = languageSettingsById.get("org.eclipse.cdt.core.gcc");
+		assertNotNull(ls);
+		entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+		assertEquals(1, entries.length);
+		assertTrue(Arrays.equals(expectedEntriesSet, entries));
+		
+		ls = languageSettingsById.get("org.eclipse.cdt.core.assembly");
+		assertNotNull(ls);
+		entries = ls.getSettingEntries(ICSettingEntry.MACRO);
+		assertEquals(1, entries.length);
+		assertTrue(Arrays.equals(expectedEntriesSet, entries));
+	}
+
 	protected void tearDown() throws Exception {
 		try {
 			p1.getProject().delete(true, null);
@@ -367,5 +442,9 @@ public class ExternalSettingsProviderTests extends BaseTestCase{
 			p4.getProject().delete(true, null);
 		} catch (CoreException e){
 		}
+		try {
+			p5.getProject().delete(true, null);
+		} catch (CoreException e){
+		}
 	}
 }
diff --git a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
index 8b7dbede7bc..f9540663774 100644
--- a/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
+++ b/core/org.eclipse.cdt.core.tests/model/org/eclipse/cdt/core/settings/model/TestExtSettingsProvider.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2009 Intel Corporation and others.
+ * Copyright (c) 2007, 2011 Intel Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  * Intel Corporation - Initial API and implementation
+ * Christian Walther (Indel AG) - [335344] test for changing language IDs
  *******************************************************************************/
 package org.eclipse.cdt.core.settings.model;
 
@@ -60,9 +61,36 @@ public class TestExtSettingsProvider extends CExternalSettingProvider {
 		})
 	};
 
+	private static CExternalSetting[] SETTINGS_3 = new CExternalSetting[]{
+		new CExternalSetting(
+				new String[]{
+						"org.eclipse.cdt.core.assembly"
+				},
+				null, null,
+				new ICSettingEntry[]{
+						new CMacroEntry("m_c", "mv_c", 0)
+				}
+		)
+	};
+	
+	private static CExternalSetting[] SETTINGS_4 = new CExternalSetting[]{
+		new CExternalSetting(
+				new String[]{
+						"org.eclipse.cdt.core.assembly",
+						"org.eclipse.cdt.core.gcc"
+				},
+				null, null,
+				new ICSettingEntry[]{
+						new CMacroEntry("m_c", "mv_c", 0)
+				}
+		)
+	};
+
 	public static final CExternalSetting[][] SETTINGS_VARIANTS = new CExternalSetting[][]{
 								SETTINGS_1, 
-								SETTINGS_2};
+								SETTINGS_2,
+								SETTINGS_3,
+								SETTINGS_4};
 
 	private static int variantNum;
 	
diff --git a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
index 136107d7c65..0662ce9cd23 100644
--- a/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
+++ b/core/org.eclipse.cdt.core/model/org/eclipse/cdt/internal/core/settings/model/CExternalSettinsDeltaCalculator.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2007, 2010 Intel Corporation and others.
+ * Copyright (c) 2007, 2011 Intel Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -7,6 +7,7 @@
  *
  * Contributors:
  * Intel Corporation - Initial API and implementation
+ * Christian Walther (Indel AG) - [335344] changing language IDs
  *******************************************************************************/
 package org.eclipse.cdt.internal.core.settings.model;
 
@@ -15,10 +16,11 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import org.eclipse.cdt.core.settings.model.CExternalSetting;
 import org.eclipse.cdt.core.settings.model.ICExternalSetting;
@@ -232,7 +234,7 @@ class CExternalSettinsDeltaCalculator {
 		if(oldSettings == null || oldSettings.length == 0)
 			return createDeltas(newSettings, true);
 
-		List<ExtSettingsDelta> deltaList = new ArrayList<ExtSettingsDelta>();
+		LinkedList<ExtSettingsDelta> deltaList = new LinkedList<ExtSettingsDelta>();
 
 		Map<ExtSettingMapKey, ICExternalSetting> newMap= toSettingsKeyMap(newSettings);
 		Map<ExtSettingMapKey, ICExternalSetting> oldMap = toSettingsKeyMap(oldSettings);
@@ -240,16 +242,20 @@ class CExternalSettinsDeltaCalculator {
 			CExternalSetting newSetting = (CExternalSetting)entry.getValue();
 			CExternalSetting oldSetting = (CExternalSetting)oldMap.remove(entry.getKey());
 			if(oldSetting == null){
-				deltaList.add(new ExtSettingsDelta(newSetting, true));
+				deltaList.addLast(new ExtSettingsDelta(newSetting, true));
 			} else {
 				ExtSettingsDelta delta = createDelta(newSetting, oldSetting);
 				if(delta != null)
-					deltaList.add(delta);
+					deltaList.addLast(delta);
 			}
 		}
 
 		for (ICExternalSetting oldSettng : oldMap.values()) {
-			deltaList.add(new ExtSettingsDelta((CExternalSetting)oldSettng, false));
+			// removals must be prepended to the list so that they are applied before additions,
+			// otherwise a setting that was just added might be immediately removed again in
+			// CExternalSettingsDeltaProcessor.applyDelta(ICLanguageSetting, ExtSettingsDelta[], int)
+			// if the old and new setting only differ in their language sets and these overlap
+			deltaList.addFirst(new ExtSettingsDelta((CExternalSetting)oldSettng, false));
 		}
 
 		if(deltaList.size() == 0)