Only pass project name to UpdateGitMetricsTask
UpdateGitMetricsTask only needs the repo name to extract metrics.
Move the logic to build the repository in the task itself to avoid
opening repositories too early.
Change-Id: I26745dddc4d6bb70625060231b02a0c240e5bc13
diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
index 97bc7cf..ad970b5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/GitRepoUpdateListener.java
@@ -15,27 +15,19 @@
package com.googlesource.gerrit.plugins.gitrepometrics;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
-import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
-import java.io.IOException;
import java.util.concurrent.ExecutorService;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.Repository;
public class GitRepoUpdateListener implements GitReferenceUpdatedListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final GitRepositoryManager repoManager;
private final ExecutorService executor;
private final UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory;
@Inject
GitRepoUpdateListener(
- GitRepositoryManager repoManager,
@UpdateGitMetricsExecutor ExecutorService executor,
UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory) {
- this.repoManager = repoManager;
this.executor = executor;
this.updateGitMetricsTaskFactory = updateGitMetricsTaskFactory;
}
@@ -43,17 +35,8 @@
@Override
public void onGitReferenceUpdated(Event event) {
String projectName = event.getProjectName();
- Project.NameKey projectNameKey = Project.nameKey(projectName);
logger.atFine().log("Got an update for project %s", projectName);
- try (Repository repository = repoManager.openRepository(projectNameKey)) {
- UpdateGitMetricsTask updateGitMetricsTask =
- updateGitMetricsTaskFactory.create(repository, Project.builder(projectNameKey).build());
- executor.execute(updateGitMetricsTask);
- } catch (RepositoryNotFoundException e) {
- logger.atSevere().withCause(e).log("Cannot find repository for %s", projectName);
- } catch (IOException e) {
- logger.atSevere().withCause(e).log(
- "Something went wrong when reading from the repository for %s", projectName);
- }
+ UpdateGitMetricsTask updateGitMetricsTask = updateGitMetricsTaskFactory.create(projectName);
+ executor.execute(updateGitMetricsTask);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTask.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTask.java
index bee852e..2196a06 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTask.java
@@ -16,11 +16,14 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import com.googlesource.gerrit.plugins.gitrepometrics.collectors.GitStats;
+import java.io.IOException;
import java.util.Map;
import java.util.stream.Collectors;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.Repository;
@@ -28,35 +31,43 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
- UpdateGitMetricsTask create(Repository repository, Project project);
+ UpdateGitMetricsTask create(String projectName);
}
- private final Repository repository;
- private final Project project;
+ private final String projectName;
private GitRepoMetricsCache gitRepoMetricsCache;
+ private GitRepositoryManager repoManager;
@Inject
UpdateGitMetricsTask(
GitRepoMetricsCache gitRepoMetricsCache,
- @Assisted Repository repository,
- @Assisted Project project) {
- this.repository = repository;
- this.project = project;
+ GitRepositoryManager repoManager,
+ @Assisted String projectName) {
+ this.projectName = projectName;
this.gitRepoMetricsCache = gitRepoMetricsCache;
+ this.repoManager = repoManager;
}
@Override
public void run() {
- // TODO Might be a noop
- logger.atInfo().log(
- "Running task to collect stats: repo %s, project %s",
- repository.getIdentifier(), project.getName());
- // TODO Loop through all the collectors
- GitStats gitStats = new GitStats((FileRepository) repository, project);
- Map<String, Long> newMetrics = gitStats.get();
- logger.atInfo().log(
- "Here all the metrics for %s - %s", project.getName(), getStringFromMap(newMetrics));
- gitRepoMetricsCache.setMetrics(newMetrics);
+ Project.NameKey projectNameKey = Project.nameKey(projectName);
+ try (Repository repository = repoManager.openRepository(projectNameKey)) {
+ logger.atInfo().log(
+ "Running task to collect stats: repo %s, project %s",
+ repository.getIdentifier(), projectName);
+ // TODO Loop through all the collectors
+ Project project = Project.builder(projectNameKey).build();
+ GitStats gitStats = new GitStats((FileRepository) repository, project);
+ Map<String, Long> newMetrics = gitStats.get();
+ logger.atInfo().log(
+ "Here all the metrics for %s - %s", project.getName(), getStringFromMap(newMetrics));
+ gitRepoMetricsCache.setMetrics(newMetrics);
+ } catch (RepositoryNotFoundException e) {
+ logger.atSevere().withCause(e).log("Cannot find repository for %s", projectName);
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log(
+ "Something went wrong when reading from the repository for %s", projectName);
+ }
}
String getStringFromMap(Map<String, Long> m) {
@@ -67,6 +78,6 @@
@Override
public String toString() {
- return String.join(" - ", repository.toString(), project.getName());
+ return "UpdateGitMetricsTask " + projectName;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/collectors/GitStats.java b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/collectors/GitStats.java
index 059fa47..5a32a5b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/collectors/GitStats.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/gitrepometrics/collectors/GitStats.java
@@ -78,10 +78,7 @@
new GitRepoMetric(numberOfBitmaps, "Number of bitmaps", "Count"));
}
- private void putMetric(
- Map<String, Long> metrics,
- String metricName,
- long value) {
+ private void putMetric(Map<String, Long> metrics, String metricName, long value) {
metrics.put(GitRepoMetricsCache.getMetricName(metricName, p.getName()), value);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
index b47a42c..f215614 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/GitUpdateListenerTest.java
@@ -18,7 +18,6 @@
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
-import static org.mockito.Mockito.verifyNoInteractions;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
@@ -72,7 +71,7 @@
reset(mockedExecutorService);
gitRepoUpdateListener =
- new GitRepoUpdateListener(repoManager, mockedExecutorService, updateGitMetricsTaskFactory);
+ new GitRepoUpdateListener(mockedExecutorService, updateGitMetricsTaskFactory);
repository = repoManager.createRepository(testProjectNameKey);
}
@@ -81,17 +80,11 @@
doNothing().when(mockedExecutorService).execute(valueCapture.capture());
gitRepoUpdateListener.onGitReferenceUpdated(new TestEvent(testProject));
UpdateGitMetricsTask expectedUpdateGitMetricsTask =
- updateGitMetricsTaskFactory.create(repository, Project.builder(testProjectNameKey).build());
+ updateGitMetricsTaskFactory.create(testProject);
assertThat(valueCapture.getValue().toString())
.isEqualTo(expectedUpdateGitMetricsTask.toString());
}
- @Test
- public void shouldNotUpdateMetricsWhenRepoDoesntExists() {
- gitRepoUpdateListener.onGitReferenceUpdated(new TestEvent("InvalidProject"));
- verifyNoInteractions(mockedExecutorService);
- }
-
public static class TestEvent implements GitReferenceUpdatedListener.Event {
private final String projectName;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java
index 2cb70c3..79caa9e 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/gitrepometrics/UpdateGitMetricsTaskTest.java
@@ -22,11 +22,15 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.config.SitePath;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Inject;
import com.google.inject.Injector;
+import com.google.inject.Provides;
+import java.io.File;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
@@ -40,17 +44,21 @@
private final String pluginName = "git-repo-metrics";
private final String projectName = "testProject";
private final Project.NameKey projectNameKey = Project.nameKey(projectName);
- private Repository testRepository;
- private Project testProject;
private static PluginConfigFactory pluginConfigFactory = mock(PluginConfigFactory.class);
private GitRepoMetricsCache gitRepoMetricsCache;
+ private Repository testRepository;
+ Project testProject;
@Inject private UpdateGitMetricsTask.Factory updateGitMetricsTaskFactory;
@Before
public void setupRepository() throws Exception {
+ Path basePath = Files.createTempDirectory("git_repo_metrics_");
+ Path gitBasePath = new File(basePath.toFile(), "git").toPath();
+
Config c = new Config();
c.setStringList(pluginName, null, "project", Collections.singletonList("repo1"));
+ c.setString("gerrit", null, "basePath", gitBasePath.toString());
doReturn(c).when(pluginConfigFactory).getGlobalPluginConfig(any());
gitRepoMetricsCache =
@@ -63,17 +71,23 @@
protected void configure() {
install(new UpdateGitMetricsTaskModule());
bind(GitRepoMetricsCache.class).toInstance(gitRepoMetricsCache);
+ bind(Config.class).annotatedWith(GerritServerConfig.class).toInstance(c);
+ }
+
+ @Provides
+ @SitePath
+ Path getSitePath() {
+ return gitBasePath;
}
};
Injector injector = Guice.createInjector(m);
injector.injectMembers(this);
- Path p = Files.createTempDirectory("git_repo_metrics_");
try {
- testRepository = new FileRepository(p.toFile());
+ testRepository = new FileRepository(new File(gitBasePath.toFile(), projectName));
testRepository.create(true);
} catch (Exception e) {
- delete(p);
+ delete(gitBasePath);
throw e;
}
testProject = Project.builder(projectNameKey).build();
@@ -81,9 +95,16 @@
@Test
public void shouldUpdateMetrics() {
- UpdateGitMetricsTask updateGitMetricsTask =
- updateGitMetricsTaskFactory.create(testRepository, testProject);
+ UpdateGitMetricsTask updateGitMetricsTask = updateGitMetricsTaskFactory.create(projectName);
updateGitMetricsTask.run();
assertThat(gitRepoMetricsCache.getMetrics().keySet()).isNotEmpty();
}
+
+ @Test
+ public void shouldNotUpdateMetricsIfRepoDoesNotExist() {
+ UpdateGitMetricsTask updateGitMetricsTask =
+ updateGitMetricsTaskFactory.create("nonExistentProject");
+ updateGitMetricsTask.run();
+ assertThat(gitRepoMetricsCache.getMetrics().keySet()).isEmpty();
+ }
}
