Remove session properties when disconnecting a project

Disconnecting a project left the session properties for the
RepositoryMappings still attached to the resource info in the Eclipse
resource tree.

This prevents timely garbage collection of no longer used repository
instances because the mappings contain hard references to repositories.

Moreover, potentially stale properties might become active again when
the project is re-connected.

Change-Id: I845c708d3d98ba285c8db94edd2ed14deebd29fb
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

5 files changed, 64 insertions, 3 deletions

diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/GitProvider.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/GitProvider.java
index 732e482722..159822994a 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/GitProvider.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/GitProvider.java
@@ -66,7 +66,7 @@ public class GitProvider extends RepositoryProvider {
 	@Override
 	public void deconfigure() throws CoreException {
 		try {
-			GitProjectData.delete(getProject());
+			GitProjectData.deconfigure(getProject());
 		} catch (IOException e) {
 			throw new CoreException(new Status(IStatus.ERROR,
 					Activator.getPluginId(), e.getMessage(), e));
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
index ddf681d3cc..ad899ffa87 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/CoreText.java
@@ -176,6 +176,9 @@ public class CoreText extends NLS {
 	public static String GitProjectData_failedToCacheRepoMapping;
 
 	/** */
+	public static String GitProjectData_failedToUnmapRepoMapping;
+
+	/** */
 	public static String GitProjectData_FailedToMarkTeamPrivate;
 
 	/** */
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
index 48fc7bc6aa..b4690a89b0 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/internal/coretext.properties
@@ -66,6 +66,7 @@ GitFileHistory_noHeadRevisionAvailable=No HEAD revision available from Git for p
 GitProjectData_mappedResourceGone=Remove repository mapping of Git mapped resource for which project or mapped git repository has disappeared: ''{0}''
 GitProjectData_failedFindingRepoMapping=Failed finding RepositoryMapping
 GitProjectData_failedToCacheRepoMapping=Failed to cache RepositoryMapping
+GitProjectData_failedToUnmapRepoMapping=Failed to unmap {0}
 GitProjectData_FailedToMarkTeamPrivate=Failed to mark {0} as team private
 GitProjectData_missing=Git team provider configuration has gone missing.
 GitProjectData_saveFailed=Saving Git team provider data to {0} failed.
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/project/GitProjectData.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/project/GitProjectData.java
index f337a0e8b4..3deb986aab 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/project/GitProjectData.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/project/GitProjectData.java
@@ -240,6 +240,26 @@ public class GitProjectData {
 	}
 
 	/**
+	 * Drop the Eclipse project from our association of projects/repositories
+	 * and remove all RepositoryMappings.
+	 *
+	 * @param p
+	 *            to deconfigure
+	 * @throws IOException
+	 *             if the property file cannot be removed.
+	 */
+	public static void deconfigure(final IProject p) throws IOException {
+		trace("deconfigure(" + p.getName() + ")"); //$NON-NLS-1$ //$NON-NLS-2$
+		GitProjectData d = lookup(p);
+		if (d == null) {
+			deletePropertyFiles(p);
+		} else {
+			d.deletePropertyFilesAndUncache();
+			unmap(d);
+		}
+	}
+
+	/**
 	 * Add the Eclipse project to our association of projects/repositories
 	 *
 	 * @param p
@@ -360,6 +380,23 @@ public class GitProjectData {
 		}
 	}
 
+	private static void unmap(GitProjectData data) {
+		for (RepositoryMapping m : data.mappings.values()) {
+			IContainer c = m.getContainer();
+			if (c != null && c.isAccessible()) {
+				try {
+					c.setSessionProperty(MAPPING_KEY, null);
+					// Team private members are re-set in
+					// DisconnectProviderOperation
+				} catch (CoreException e) {
+					Activator.logWarning(MessageFormat.format(
+							CoreText.GitProjectData_failedToUnmapRepoMapping,
+							c.getFullPath()), e);
+				}
+			}
+		}
+	}
+
 	private synchronized static GitProjectData lookup(final IProject p) {
 		return projectDataCache.get(p);
 	}
@@ -628,8 +665,6 @@ public class GitProjectData {
 			return;
 		}
 
-		fireRepositoryChanged(m);
-
 		trace("map "  //$NON-NLS-1$
 				+ c
 				+ " -> "  //$NON-NLS-1$
@@ -641,6 +676,8 @@ public class GitProjectData {
 					CoreText.GitProjectData_failedToCacheRepoMapping, err);
 		}
 
+		fireRepositoryChanged(m);
+
 		dotGit = c.findMember(Constants.DOT_GIT);
 		if (dotGit != null) {
 			protect(dotGit);
diff --git a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
index ece1f6ab95..9fde54af5e 100644
--- a/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
+++ b/org.eclipse.egit.ui.test/src/org/eclipse/egit/ui/internal/submodules/SubmoduleFolderTest.java
@@ -13,6 +13,7 @@ import static org.eclipse.swtbot.eclipse.finder.waits.Conditions.waitForEditor;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
@@ -24,10 +25,12 @@ import org.eclipse.core.resources.IProject;
 import org.eclipse.core.resources.IProjectDescription;
 import org.eclipse.core.resources.IResource;
 import org.eclipse.core.resources.ResourcesPlugin;
+import org.eclipse.core.runtime.QualifiedName;
 import org.eclipse.core.runtime.jobs.Job;
 import org.eclipse.egit.core.Activator;
 import org.eclipse.egit.core.JobFamilies;
 import org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry;
+import org.eclipse.egit.core.project.GitProjectData;
 import org.eclipse.egit.core.project.RepositoryMapping;
 import org.eclipse.egit.core.test.TestRepository;
 import org.eclipse.egit.ui.common.LocalRepositoryTestCase;
@@ -263,6 +266,23 @@ public class SubmoduleFolderTest extends LocalRepositoryTestCase {
 	}
 
 	@Test
+	public void testDisconnect() throws Exception {
+		SWTBotTree projectExplorerTree = TestUtil.getExplorerTree();
+		getProjectItem(projectExplorerTree, PROJ1).select();
+		String menuString = util
+				.getPluginLocalizedValue("DisconnectAction_label");
+		ContextMenuHelper.clickContextMenuSync(projectExplorerTree, "Team",
+				menuString);
+		ResourcesPlugin.getWorkspace().getRoot()
+				.refreshLocal(IResource.DEPTH_INFINITE, null);
+		// Access the session property directly: RepositoryMapping.getMapping()
+		// checks whether the project is shared with git.
+		Object mapping = childFolder.getSessionProperty(new QualifiedName(
+				GitProjectData.class.getName(), "RepositoryMapping"));
+		assertNull("Should have no RepositoryMapping", mapping);
+	}
+
+	@Test
 	public void testDecoration() throws Exception {
 		SWTBotTree projectExplorerTree = TestUtil.getExplorerTree();
 		SWTBotTreeItem node = TestUtil.navigateTo(projectExplorerTree,