Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
Use InternalUser for checking projects list
Add test for setups without AnonymousRead
Remove Eclipse warnings
Change-Id: I48b8c780231562767eb4d49ff742b7fbc1a68bca
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
index dcf2f0e..8e64593 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ActiveWorkersCheck.java
@@ -34,7 +34,6 @@
private Integer threshold;
private Integer interactiveThreadsMaxPoolSize;
private MetricRegistry metricRegistry;
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
@Inject
public ActiveWorkersCheck(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java
index cd0274d..ce9f8d9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/AuthHealthCheck.java
@@ -36,7 +36,6 @@
private final AccountCache byIdCache;
private final String username;
private final String password;
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
@Inject
public AuthHealthCheck(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java
index a368246..6a6ade6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/DeadlockCheck.java
@@ -29,7 +29,6 @@
"proc/jvm/thread/num_deadlocked_threads";
private final MetricRegistry metricRegistry;
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
@Inject
public DeadlockCheck(
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
index d107588..9196cdb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/JGitHealthCheck.java
@@ -31,8 +31,6 @@
private final GitRepositoryManager repositoryManager;
private final Set<Project.NameKey> repositoryNameKeys;
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
-
@Inject
public JGitHealthCheck(
ListeningExecutorService executor,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
index 9850a10..78cff2a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/ProjectsListHealthCheck.java
@@ -19,7 +19,10 @@
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.server.restapi.project.ListProjects;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckConfig;
import com.googlesource.gerrit.plugins.healthcheck.HealthCheckMetrics;
@@ -31,36 +34,40 @@
public class ProjectsListHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(ProjectsListHealthCheck.class);
private static final int PROJECTS_LIST_LIMIT = 100;
- private final ListProjects listProjects;
-
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
+ private final Provider<ListProjects> listProjectsProvider;
+ private final OneOffRequestContext oneOffCtx;
@Inject
public ProjectsListHealthCheck(
ListeningExecutorService executor,
HealthCheckConfig config,
- ListProjects listProjects,
+ Provider<ListProjects> listProjectsProvider,
+ OneOffRequestContext oneOffCtx,
HealthCheckMetrics.Factory healthCheckMetricsFactory) {
super(executor, config, PROJECTSLIST, healthCheckMetricsFactory);
- this.listProjects = listProjects;
+ this.listProjectsProvider = listProjectsProvider;
+ this.oneOffCtx = oneOffCtx;
}
@Override
protected Result doCheck() {
- listProjects.setStart(0);
- listProjects.setLimit(PROJECTS_LIST_LIMIT);
- listProjects.setShowDescription(true);
- listProjects.setMatchPrefix("All-");
- try {
- SortedMap<String, ProjectInfo> projects = listProjects.apply();
- if (projects != null && projects.size() > 0) {
- return Result.PASSED;
+ try (ManualRequestContext ctx = oneOffCtx.open()) {
+ ListProjects listProjects = listProjectsProvider.get();
+ listProjects.setStart(0);
+ listProjects.setLimit(PROJECTS_LIST_LIMIT);
+ listProjects.setShowDescription(true);
+ listProjects.setMatchPrefix("All-");
+ try {
+ SortedMap<String, ProjectInfo> projects = listProjects.apply();
+ if (projects != null && projects.size() > 0) {
+ return Result.PASSED;
+ }
+ log.warn("Empty or null projects list: Gerrit should always have at least 1 project");
+ } catch (Exception e) {
+ log.warn("Unable to list projects", e);
}
- log.warn("Empty or null projects list: Gerrit should always have at least 1 project");
- } catch (Exception e) {
- log.warn("Unable to list projects", e);
+ return Result.FAILED;
}
- return Result.FAILED;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
index 9b6825f..a786606 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/healthcheck/check/QueryChangesHealthCheck.java
@@ -33,12 +33,9 @@
public class QueryChangesHealthCheck extends AbstractHealthCheck {
private static final Logger log = LoggerFactory.getLogger(QueryChangesHealthCheck.class);
private final Provider<QueryChanges> queryChangesProvider;
- private final HealthCheckConfig config;
private final int limit;
private final OneOffRequestContext oneOffCtx;
- private HealthCheckMetrics.Factory healthCheckMetricsFactory;
-
@Inject
public QueryChangesHealthCheck(
ListeningExecutorService executor,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
index f67802e..fada3bb 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/AbstractHealthCheckTest.java
@@ -16,14 +16,12 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.metrics.Counter0;
import com.google.gerrit.metrics.Description;
import com.google.gerrit.metrics.DisabledMetricMaker;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.metrics.Timer0;
-import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.healthcheck.check.AbstractHealthCheck;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck;
import java.util.concurrent.Executors;
@@ -33,7 +31,6 @@
public class AbstractHealthCheckTest {
- private ListeningExecutorService executor;
MetricMaker testMetricMaker;
DummyHealthCheckMetricsFactory healthCheckMetricsFactory = new DummyHealthCheckMetricsFactory();
@@ -104,17 +101,13 @@
private TestCheck createTestCheckWithStatus(HealthCheck.Result result) {
return new TestCheck(
- executor, HealthCheckConfig.DEFAULT_CONFIG, "testCheck", healthCheckMetricsFactory, result);
+ HealthCheckConfig.DEFAULT_CONFIG, "testCheck", healthCheckMetricsFactory, result);
}
private static class TestCheck extends AbstractHealthCheck {
- @Inject private ListeningExecutorService executor;
private final Result finalResult;
- private DummyHealthCheckMetricsFactory healthCheckMetricsFactory =
- new DummyHealthCheckMetricsFactory();
public TestCheck(
- ListeningExecutorService executor,
HealthCheckConfig config,
String name,
HealthCheckMetrics.Factory healthCheckMetricsFactory,
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
index 79a4bc4..aa880a3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/DeadlockCheckTest.java
@@ -45,9 +45,6 @@
@Test
public void shouldPassCheckWhenNoDeadlock() {
-
- MetricRegistry metricRegistry = createMetricRegistry(0);
-
Injector injector = testInjector(new TestModule(createMetricRegistry(0)));
DeadlockCheck check = createCheck(injector);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
index a372509..e9901a9 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckIT.java
@@ -70,6 +70,12 @@
}
@Test
+ public void shouldReturnOkWhenHealthyAndAnonymousReadIsBlocked() throws Exception {
+ blockAnonymousRead();
+ getHealthCheckStatus().assertOK();
+ }
+
+ @Test
public void shouldReturnAJsonPayload() throws Exception {
assertThat(getHealthCheckStatus().getHeader(CONTENT_TYPE)).contains("application/json");
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
index b2fb862..b971054 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/HealthCheckStatusEndpointTest.java
@@ -95,7 +95,7 @@
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
}
@@ -128,7 +128,7 @@
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
@@ -171,7 +171,7 @@
HealthCheckStatusEndpoint healthCheckApi =
injector.getInstance(HealthCheckStatusEndpoint.class);
- Response<?> resp = (Response<?>) healthCheckApi.apply(null);
+ Response<?> resp = healthCheckApi.apply(null);
assertThat(resp.statusCode()).isEqualTo(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
index c8d25b6..58009ea 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/healthcheck/ProjectsListHealthCheckTest.java
@@ -21,8 +21,11 @@
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.server.restapi.project.ListProjects;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Guice;
import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
import com.googlesource.gerrit.plugins.healthcheck.check.HealthCheck.Result;
import com.googlesource.gerrit.plugins.healthcheck.check.ProjectsListHealthCheck;
import java.util.SortedMap;
@@ -30,7 +33,11 @@
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+@RunWith(MockitoJUnitRunner.class)
public class ProjectsListHealthCheckTest {
@Inject private ListeningExecutorService executor;
@@ -38,6 +45,8 @@
private Config gerritConfig = new Config();
+ @Mock private OneOffRequestContext mockOneOffCtx;
+
@Before
public void setUp() throws Exception {
Guice.createInjector(new HealthCheckModule()).injectMembers(this);
@@ -47,7 +56,11 @@
public void shouldBeHealthyWhenListProjectsWorks() {
ProjectsListHealthCheck jGitHealthCheck =
new ProjectsListHealthCheck(
- executor, DEFAULT_CONFIG, getWorkingProjectList(0), healthCheckMetricsFactory);
+ executor,
+ DEFAULT_CONFIG,
+ getWorkingProjectList(0),
+ mockOneOffCtx,
+ healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.PASSED);
}
@@ -55,7 +68,11 @@
public void shouldBeUnhealthyWhenListProjectsIsFailing() {
ProjectsListHealthCheck jGitHealthCheck =
new ProjectsListHealthCheck(
- executor, DEFAULT_CONFIG, getFailingProjectList(), healthCheckMetricsFactory);
+ executor,
+ DEFAULT_CONFIG,
+ getFailingProjectList(),
+ mockOneOffCtx,
+ healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.FAILED);
}
@@ -66,34 +83,37 @@
executor,
DEFAULT_CONFIG,
getWorkingProjectList(DEFAULT_CONFIG.getTimeout() * 2),
+ mockOneOffCtx,
healthCheckMetricsFactory);
assertThat(jGitHealthCheck.run().result).isEqualTo(Result.TIMEOUT);
}
- private ListProjects getFailingProjectList() {
- return new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
+ private Provider<ListProjects> getFailingProjectList() {
+ return Providers.of(
+ new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
- @Override
- public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
- throw new IllegalArgumentException("Unable to return project list");
- }
- };
+ @Override
+ public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
+ throw new IllegalArgumentException("Unable to return project list");
+ }
+ });
}
- private ListProjects getWorkingProjectList(long execTime) {
- return new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
+ private Provider<ListProjects> getWorkingProjectList(long execTime) {
+ return Providers.of(
+ new ListProjects(null, null, null, null, null, null, null, null, null, gerritConfig) {
- @Override
- public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
- SortedMap<String, ProjectInfo> projects = new TreeMap<>();
- projects.put("testproject", new ProjectInfo());
- try {
- Thread.sleep(execTime);
- } catch (InterruptedException e) {
- throw new IllegalStateException(e);
- }
- return projects;
- }
- };
+ @Override
+ public SortedMap<String, ProjectInfo> apply() throws BadRequestException {
+ SortedMap<String, ProjectInfo> projects = new TreeMap<>();
+ projects.put("testproject", new ProjectInfo());
+ try {
+ Thread.sleep(execTime);
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(e);
+ }
+ return projects;
+ }
+ });
}
}
