Prune deleted repositories from RepositoryCache

RepositoryCache is not removing repositories that were deleted. When
queried it will deliver old entries for a new repo created at the very
same path.
I now remove dead repos when they're looked up and when all repos are
requested.

Bug: 387617
Change-Id: I8c27f63ccb3b8d6465d3153d77a7459ca909c0f9
Signed-off-by: Andre Dietisheim <adietish@redhat.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

2 files changed, 66 insertions, 12 deletions

diff --git a/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/RepositoryCacheTest.java b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/RepositoryCacheTest.java
new file mode 100644
index 0000000000..c392eca56b
--- /dev/null
+++ b/org.eclipse.egit.core.test/src/org/eclipse/egit/core/test/RepositoryCacheTest.java
@@ -0,0 +1,55 @@
+/*******************************************************************************
+ * Copyright (c) 2012 Red Hat, Inc. Distributed under license by Red Hat, Inc.
+ * 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Andre Dietisheim - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.egit.core.test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.isIn;
+import static org.hamcrest.core.IsNot.not;
+
+import java.io.IOException;
+
+import org.eclipse.egit.core.Activator;
+import org.eclipse.egit.core.RepositoryCache;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.util.FileUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class RepositoryCacheTest extends GitTestCase {
+
+	private TestRepository testRepository;
+	private Repository repository;
+	private RepositoryCache cache;
+
+	@Before
+	public void setUp() throws Exception {
+		super.setUp();
+		this.testRepository = new TestRepository(gitDir);
+		this.repository = testRepository.getRepository();
+		this.cache = Activator.getDefault().getRepositoryCache();
+	}
+
+	@After
+	public void tearDown() throws Exception {
+		testRepository.dispose();
+		repository = null;
+		super.tearDown();
+	}
+
+	@Test
+	public void shouldNotContainDeletedRepository() throws IOException {
+		cache.lookupRepository(repository.getDirectory());
+		assertThat(repository, isIn(cache.getAllRepositories()));
+		FileUtils.delete(repository.getDirectory(), FileUtils.RECURSIVE);
+		assertThat(repository, not(isIn(cache.getAllRepositories())));
+	}
+}
diff --git a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
index 4027ee95c9..733551d8f3 100644
--- a/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
+++ b/org.eclipse.egit.core/src/org/eclipse/egit/core/RepositoryCache.java
@@ -16,7 +16,6 @@ import java.io.IOException;
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -46,13 +45,13 @@ public class RepositoryCache {
 	 */
 	public synchronized Repository lookupRepository(final File gitDir)
 			throws IOException {
+		prune(repositoryCache);
 		Reference<Repository> r = repositoryCache.get(gitDir);
 		Repository d = r != null ? r.get() : null;
 		if (d == null) {
 			d = new FileRepository(gitDir);
 			repositoryCache.put(gitDir, new WeakReference<Repository>(d));
 		}
-		prune(repositoryCache);
 		return d;
 	}
 
@@ -60,20 +59,20 @@ public class RepositoryCache {
 	 * @return all Repository instances contained in the cache
 	 */
 	public synchronized Repository[] getAllRepositories() {
-		List<Repository> result = new ArrayList<Repository>();
-		Collection<Reference<Repository>> values = repositoryCache.values();
-		for(Reference<Repository> ref:values) {
-			Repository repo = ref.get();
-			if(repo!=null)
-				result.add(repo);
+		prune(repositoryCache);
+		List<Repository> repositories = new ArrayList<Repository>();
+		for (Reference<Repository> reference : repositoryCache.values()) {
+			repositories.add(reference.get());
 		}
-		return result.toArray(new Repository[result.size()]);
+		return repositories.toArray(new Repository[repositories.size()]);
 	}
 
-	private static <K, V> void prune(Map<K, Reference<V>> map) {
-		for (final Iterator<Map.Entry<K, Reference<V>>> i = map.entrySet()
+	private static void prune(Map<File, Reference<Repository>> map) {
+		for (final Iterator<Map.Entry<File, Reference<Repository>>> i = map.entrySet()
 				.iterator(); i.hasNext();) {
-			if (i.next().getValue().get() == null)
+			Repository repository = i.next().getValue().get();
+			if (repository == null
+					|| !repository.getDirectory().exists())
 				i.remove();
 		}
 	}