Extract opening repos to its own class
Move opening the repos (with caching and translating URL -> repoName) to
its own class.
Providers can offer different implementations adjusted to
their backends, and supporting e.g. cross-host reading. The injection
code comes in follow up changes.
Change-Id: I9f346e6733bd35170a2a94a27d7acbd6390679bf
diff --git a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
index 11e1233..c82c830 100644
--- a/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
+++ b/java/com/googlesource/gerrit/plugins/supermanifest/SuperManifestRefUpdatedListener.java
@@ -52,7 +52,6 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
-import java.util.Map;
import java.util.Set;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.RefNotFoundException;
@@ -330,7 +329,9 @@
String.format("invalid toolType: %s", c.getToolType().name()));
}
try (GerritRemoteReader reader =
- new GerritRemoteReader(repoManager, canonicalWebUrl.toString())) {
+ new GerritRemoteReader(
+ new GerritSuperManifestRepoManager(repoManager, canonicalWebUrl.toString()),
+ canonicalWebUrl.toString())) {
subModuleUpdater.update(reader, c, refName);
}
}
@@ -356,12 +357,11 @@
// GerritRemoteReader is for injecting Gerrit's Git implementation into JGit.
static class GerritRemoteReader implements RepoCommand.RemoteReader, AutoCloseable {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final Map<Project.NameKey, Repository> repos;
- private final GitRepositoryManager repoManager;
private final String canonicalWebUrl;
+ private final SuperManifestRepoManager repoManager;
- GerritRemoteReader(GitRepositoryManager repoManager, @CanonicalWebUrl String canonicalWebUrl) {
- this.repos = new HashMap<>();
+ GerritRemoteReader(
+ SuperManifestRepoManager repoManager, @CanonicalWebUrl String canonicalWebUrl) {
this.repoManager = repoManager;
this.canonicalWebUrl = canonicalWebUrl;
}
@@ -378,7 +378,9 @@
// When the remote is fetch="<relative path>" the manifest parser uses a repoName as URI.
// Do a poor man's guessing if we have a repoName or URI
Repository repo =
- uriStr.contains("://") ? doOpenByUri(uriStr) : doOpenByName(Project.nameKey(uriStr));
+ uriStr.contains("://")
+ ? repoManager.openByUri(uriStr)
+ : repoManager.openByName(Project.nameKey(uriStr));
Ref ref = repo.findRef(refName);
if (ref == null || ref.getObjectId() == null) {
logger.atWarning().log(
@@ -406,7 +408,9 @@
// When the remote is fetch="<relative path>" the manifest parser uses a repoName as URI.
// Do a poor man's guessing if we have a repoName or URI
Repository repo =
- uriStr.contains("://") ? doOpenByUri(uriStr) : doOpenByName(Project.nameKey(uriStr));
+ uriStr.contains("://")
+ ? repoManager.openByUri(uriStr)
+ : repoManager.openByName(Project.nameKey(uriStr));
Ref r = repo.findRef(ref);
ObjectId objectId = r == null ? repo.resolve(ref) : r.getObjectId();
if (objectId == null) {
@@ -421,10 +425,40 @@
}
public Repository openRepository(String name) throws IOException {
- return doOpenByName(Project.nameKey(name));
+ return repoManager.openByName(Project.nameKey(name));
}
- private Repository doOpenByName(Project.NameKey name) throws IOException {
+ @Override
+ public void close() {
+ try {
+ repoManager.close();
+ } catch (Exception e) {
+ logger.atWarning().log("Error closing the repoManager");
+ }
+ }
+ }
+
+ // AutoCloseable so implementations can keep a cache
+ public interface SuperManifestRepoManager extends AutoCloseable {
+ Repository openByUri(String uriStr) throws IOException;
+
+ Repository openByName(Project.NameKey repoName) throws IOException;
+ }
+
+ static class GerritSuperManifestRepoManager implements SuperManifestRepoManager {
+ private final HashMap<Project.NameKey, Repository> repos;
+ private final GitRepositoryManager repoManager;
+ private final String canonicalWebUrl;
+
+ GerritSuperManifestRepoManager(
+ GitRepositoryManager repoManager, @CanonicalWebUrl String canonicalWebUrl) {
+ this.repos = new HashMap<>();
+ this.repoManager = repoManager;
+ this.canonicalWebUrl = canonicalWebUrl;
+ }
+
+ @Override
+ public Repository openByName(Project.NameKey name) throws IOException {
if (repos.containsKey(name)) {
return repos.get(name);
}
@@ -434,7 +468,8 @@
return repo;
}
- private Repository doOpenByUri(String uriStr) throws IOException {
+ @Override
+ public Repository openByUri(String uriStr) throws IOException {
// A URL in this host is <canonicalWebUrl>/<repoName>.
if (!uriStr.startsWith(canonicalWebUrl)) {
// Cannot open repos in another host.
@@ -450,7 +485,7 @@
repoName = repoName.substring(1);
}
- return doOpenByName(Project.nameKey(repoName));
+ return openByName(Project.nameKey(repoName));
}
@Override
diff --git a/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritRemoteReaderTest.java b/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritRemoteReaderTest.java
index 539bdc4..bf32da6 100644
--- a/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritRemoteReaderTest.java
+++ b/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritRemoteReaderTest.java
@@ -25,8 +25,12 @@
private static final String PROJECT_URL = CANONICAL_WEB_URL + "/" + PROJECT_REPONAME;
private GitRepositoryManager repoManager = new InMemoryRepositoryManager();
+ SuperManifestRefUpdatedListener.GerritSuperManifestRepoManager superManifestRepoManager =
+ new SuperManifestRefUpdatedListener.GerritSuperManifestRepoManager(
+ repoManager, CANONICAL_WEB_URL);
private SuperManifestRefUpdatedListener.GerritRemoteReader reader =
- new SuperManifestRefUpdatedListener.GerritRemoteReader(repoManager, CANONICAL_WEB_URL);
+ new SuperManifestRefUpdatedListener.GerritRemoteReader(
+ superManifestRepoManager, CANONICAL_WEB_URL);
private Repository projectX;
private RevCommit projectXTip;
diff --git a/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritSuperManifestRepoManagerTest.java b/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritSuperManifestRepoManagerTest.java
new file mode 100644
index 0000000..08980fe
--- /dev/null
+++ b/javatests/com/googlesource/gerrit/plugins/supermanifest/GerritSuperManifestRepoManagerTest.java
@@ -0,0 +1,47 @@
+package com.googlesource.gerrit.plugins.supermanifest;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import java.io.IOException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+public class GerritSuperManifestRepoManagerTest {
+ private static final String CANONICAL_WEB_URL = "https://example.com/gerrit/";
+
+ SuperManifestRefUpdatedListener.GerritSuperManifestRepoManager superManifestRepoManager;
+
+ @Before
+ public void setUp() throws IOException {
+ GitRepositoryManager repoManager = new InMemoryRepositoryManager();
+ repoManager.createRepository(Project.nameKey("project/x"));
+ superManifestRepoManager =
+ new SuperManifestRefUpdatedListener.GerritSuperManifestRepoManager(
+ repoManager, CANONICAL_WEB_URL);
+ }
+
+ @Test
+ public void openByRepoName() throws Exception {
+ Repository repo = superManifestRepoManager.openByName(Project.nameKey("project/x"));
+ assertThat(repo).isNotNull();
+ }
+
+ @Test
+ public void openByUri_canonical_repoNameExcludingCanonical() throws Exception {
+ Repository repo = superManifestRepoManager.openByUri(CANONICAL_WEB_URL + "project/x");
+ assertThat(repo).isNotNull();
+ }
+
+ @Test
+ public void openByUri_nonCanonical_uriPathIsRepoName() {
+ assertThrows(
+ RepositoryNotFoundException.class,
+ () -> superManifestRepoManager.openByUri("https://otherhost.com/project/x"));
+ }
+}
