Merge branch 'stable-2.15' into stable-2.16
* stable-2.15:
Upgrade testcontainers to 1.11.2
Fix the execution of GC evaluation tasks
Change-Id: If58a93cdfedcfb9f2ea79f3cdf133633d4a870eb
diff --git a/.gitignore b/.gitignore
index 72f041f..78857a0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@
/.settings/
/bazel-*
/eclipse-out/
+/.apt_generated/
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index 3d79e6d..aa23e31 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -153,28 +153,28 @@
sha1 = "ed8b772eb077a9cb50e44e90899c66a9a6c00e67",
)
- TEST_CONTAINERS_VERS = "1.11.1"
+ TEST_CONTAINERS_VERS = "1.11.2"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TEST_CONTAINERS_VERS,
- sha1 = "502692678fc90996a6c3c5392c229725cd5d4635",
+ sha1 = "eae47ed24bb07270d4b60b5e2c3444c5bf3c8ea9",
)
maven_jar(
name = "testcontainers-database-commons",
artifact = "org.testcontainers:database-commons:" + TEST_CONTAINERS_VERS,
- sha1 = "321b1d943cc3b2d3448d47a8d51478d73a5a207c",
+ sha1 = "f49cf9cd5f9c484f7c4cb24270d7cc8aae0bb188",
)
maven_jar(
name = "testcontainers-jdbc",
artifact = "org.testcontainers:jdbc:" + TEST_CONTAINERS_VERS,
- sha1 = "5432618ccd7033cc992d993028e52cb9391f0cde",
+ sha1 = "eaa9b0fa7a31bbfa03ad3c6d51808397b1fbdc40",
)
maven_jar(
name = "testcontainers-postgres",
artifact = "org.testcontainers:postgresql:" + TEST_CONTAINERS_VERS,
- sha1 = "cd2dc98053b50e743fb0e3a1aac2b069d77da066",
+ sha1 = "a48ea341efa0ec078ae4a717459252aebc7bff5d",
)
diff --git a/src/main/java/com/ericsson/gerrit/plugins/gcconductor/EvaluationTask.java b/src/main/java/com/ericsson/gerrit/plugins/gcconductor/EvaluationTask.java
index 387a59a..43803e4 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/gcconductor/EvaluationTask.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/gcconductor/EvaluationTask.java
@@ -96,7 +96,7 @@
return false;
}
EvaluationTask other = (EvaluationTask) obj;
- return repositoryPath == other.repositoryPath;
+ return repositoryPath.equals(other.repositoryPath);
}
private boolean isAlreadyInQueue() {
diff --git a/src/main/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/Evaluator.java b/src/main/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/Evaluator.java
index 07450c4..5ae7fac 100644
--- a/src/main/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/Evaluator.java
+++ b/src/main/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/Evaluator.java
@@ -15,6 +15,9 @@
package com.ericsson.gerrit.plugins.gcconductor.evaluator;
import com.ericsson.gerrit.plugins.gcconductor.EvaluationTask;
+import com.google.common.util.concurrent.JdkFutureAdapters;
+import com.google.common.util.concurrent.ListenableFuture;
+import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.GerritServerConfig;
@@ -28,6 +31,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Future;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
@@ -120,16 +124,19 @@
if (lastCheckExpired(repositoryPath)) {
EvaluationTask evaluationTask = evaluationTaskFactory.create(repositoryPath);
if (queuedEvaluationTasks.add(evaluationTask)) {
- try {
- executor.execute(evaluationTask);
- timestamps.put(repositoryPath, System.currentTimeMillis());
- } finally {
- queuedEvaluationTasks.remove(evaluationTask);
- }
+ Future<?> future = executor.submit(evaluationTask);
+ addTaskListener(future, evaluationTask);
+ timestamps.put(repositoryPath, System.currentTimeMillis());
}
}
}
+ private void addTaskListener(Future<?> future, EvaluationTask evaluationTask) {
+ ListenableFuture<?> listenableFuture = JdkFutureAdapters.listenInPoolThread(future);
+ listenableFuture.addListener(
+ () -> queuedEvaluationTasks.remove(evaluationTask), MoreExecutors.directExecutor());
+ }
+
private boolean lastCheckExpired(String repositoryPath) {
return !timestamps.containsKey(repositoryPath)
|| System.currentTimeMillis() >= timestamps.get(repositoryPath) + expireTime;
diff --git a/src/test/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/EvaluatorTest.java b/src/test/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/EvaluatorTest.java
index 53ed1d3..7dc403b 100644
--- a/src/test/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/EvaluatorTest.java
+++ b/src/test/java/com/ericsson/gerrit/plugins/gcconductor/evaluator/EvaluatorTest.java
@@ -28,6 +28,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import java.io.File;
import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config;
@@ -41,29 +42,49 @@
@RunWith(MockitoJUnitRunner.class)
public class EvaluatorTest {
private static final String REPOSITORY_PATH = "/path/someRepo.git";
+ private static final String REPOSITORY_PATH_OTHER = "/path/otherRepo.git";
private static final Project.NameKey NAME_KEY = new Project.NameKey("testProject");
@Mock private GitReferenceUpdatedListener.Event event;
@Mock private GitRepositoryManager repoManager;
@Mock private Repository repository;
+ @Mock private Repository repositoryOther;
@Mock private ScheduledThreadPoolExecutor executor;
@Mock private EvaluatorConfig config;
@Mock private Config gerritConfig;
private Evaluator evaluator;
- private EvaluationTask task;
+ private EvaluationTask taskSamePathCompleted;
+ private EvaluationTask taskSamePathNotCompleted;
+ private EvaluationTask taskDifferentPath;
@Before
public void createEvaluator() {
when(event.getProjectName()).thenReturn(NAME_KEY.get());
- task = new EvaluationTask(null, null, null, REPOSITORY_PATH);
- when(repository.getDirectory()).thenReturn(new File(REPOSITORY_PATH));
- Factory eventTaskFactory = mock(Factory.class);
- when(eventTaskFactory.create(REPOSITORY_PATH)).thenReturn(task);
+
when(config.getExpireTimeRecheck()).thenReturn(0L);
when(gerritConfig.getInt(
"receive", null, "threadPoolSize", Runtime.getRuntime().availableProcessors()))
.thenReturn(1);
+
+ when(repository.getDirectory()).thenReturn(new File(REPOSITORY_PATH));
+ when(repositoryOther.getDirectory()).thenReturn(new File(REPOSITORY_PATH_OTHER));
+
+ taskSamePathCompleted = new EvaluationTask(null, null, null, REPOSITORY_PATH);
+ taskSamePathNotCompleted = new EvaluationTask(null, null, null, REPOSITORY_PATH);
+ taskDifferentPath = new EvaluationTask(null, null, null, REPOSITORY_PATH_OTHER);
+
+ Factory eventTaskFactory = mock(Factory.class);
+ when(eventTaskFactory.create(REPOSITORY_PATH))
+ .thenReturn(taskSamePathNotCompleted)
+ .thenReturn(taskSamePathCompleted);
+ when(eventTaskFactory.create(REPOSITORY_PATH_OTHER)).thenReturn(taskDifferentPath);
+
+ when(executor.submit(taskSamePathCompleted))
+ .thenReturn(CompletableFuture.completedFuture(null));
+ when(executor.submit(taskSamePathNotCompleted)).thenReturn(new CompletableFuture<>());
+ when(executor.submit(taskDifferentPath)).thenReturn(CompletableFuture.completedFuture(null));
+
evaluator = new Evaluator(executor, eventTaskFactory, repoManager, config, gerritConfig);
}
@@ -71,7 +92,7 @@
public void onPostUploadShouldCreateTaskOnlyIfPreUploadCalled() {
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
- verify(executor).execute(task);
+ verify(executor).submit(taskSamePathCompleted);
}
@Test
@@ -81,7 +102,7 @@
when(repository.getDirectory()).thenReturn(fileMock);
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
- verify(executor, never()).execute(task);
+ verify(executor, never()).submit(taskSamePathCompleted);
}
@Test
@@ -89,37 +110,73 @@
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
evaluator.onPostUpload(null);
- verify(executor, times(1)).execute(task);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
}
@Test
public void onPostUploadShouldCreateTaskExpired() {
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
+ evaluator.onPreUpload(repositoryOther, null, null, null, null, null);
+ evaluator.onPostUpload(null);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
+ verify(executor, times(1)).submit(taskDifferentPath);
+ }
+
+ @Test
+ public void onPostUploadSameRepoShouldCreateSingleTaskOnly() {
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
- verify(executor, times(2)).execute(task);
+ evaluator.onPreUpload(repository, null, null, null, null, null);
+ evaluator.onPostUpload(null);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
+ }
+
+ @Test
+ public void onPostUploadCompletedTasksAreRemovedFromQueue() {
+ evaluator.onPreUpload(repositoryOther, null, null, null, null, null);
+ evaluator.onPostUpload(null);
+ evaluator.onPreUpload(repositoryOther, null, null, null, null, null);
+ evaluator.onPostUpload(null);
+ verify(executor, times(2)).submit(taskDifferentPath);
}
@Test
public void onPostUploadShouldNotCreateTaskNotExpired() {
when(config.getExpireTimeRecheck()).thenReturn(1000L);
Factory eventTaskFactory = mock(Factory.class);
- when(eventTaskFactory.create(REPOSITORY_PATH)).thenReturn(task);
+ when(eventTaskFactory.create(REPOSITORY_PATH)).thenReturn(taskSamePathCompleted);
evaluator = new Evaluator(executor, eventTaskFactory, repoManager, config, gerritConfig);
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
evaluator.onPreUpload(repository, null, null, null, null, null);
evaluator.onPostUpload(null);
- verify(executor, times(1)).execute(task);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
}
@Test
public void onGitReferenceUpdatedShouldCreateTaskExpired() throws Exception {
+ when(repoManager.openRepository(NAME_KEY)).thenReturn(repository).thenReturn(repositoryOther);
+ evaluator.onGitReferenceUpdated(event);
+ evaluator.onGitReferenceUpdated(event);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
+ verify(executor, times(1)).submit(taskDifferentPath);
+ }
+
+ @Test
+ public void onGitReferenceUpdatedSameRepoShouldCreateSingleTaskOnly() throws Exception {
when(repoManager.openRepository(NAME_KEY)).thenReturn(repository);
evaluator.onGitReferenceUpdated(event);
evaluator.onGitReferenceUpdated(event);
- verify(executor, times(2)).execute(task);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
+ }
+
+ @Test
+ public void onGitReferenceUpdatedCompletedTasksAreRemovedFromQueue() throws Exception {
+ when(repoManager.openRepository(NAME_KEY)).thenReturn(repositoryOther);
+ evaluator.onGitReferenceUpdated(event);
+ evaluator.onGitReferenceUpdated(event);
+ verify(executor, times(2)).submit(taskDifferentPath);
}
@Test
@@ -127,24 +184,24 @@
when(repoManager.openRepository(NAME_KEY)).thenReturn(repository);
when(config.getExpireTimeRecheck()).thenReturn(1000L);
Factory eventTaskFactory = mock(Factory.class);
- when(eventTaskFactory.create(REPOSITORY_PATH)).thenReturn(task);
+ when(eventTaskFactory.create(REPOSITORY_PATH)).thenReturn(taskSamePathCompleted);
evaluator = new Evaluator(executor, eventTaskFactory, repoManager, config, gerritConfig);
evaluator.onGitReferenceUpdated(event);
evaluator.onGitReferenceUpdated(event);
- verify(executor, times(1)).execute(task);
+ verify(executor, times(1)).submit(taskSamePathCompleted);
}
@Test
public void onGitReferenceUpdatedThrowsRepositoryNotFoundException() throws Exception {
doThrow(new RepositoryNotFoundException("")).when(repoManager).openRepository(NAME_KEY);
evaluator.onGitReferenceUpdated(event);
- verify(executor, never()).execute(task);
+ verify(executor, never()).submit(taskSamePathCompleted);
}
@Test
public void onGitReferenceUpdatedThrowsIOException() throws Exception {
doThrow(new IOException()).when(repoManager).openRepository(NAME_KEY);
evaluator.onGitReferenceUpdated(event);
- verify(executor, never()).execute(task);
+ verify(executor, never()).submit(taskSamePathCompleted);
}
}
