Merge branch 'stable-3.3' * stable-3.3: Use InternalUser for checking projects list Add test for setups without AnonymousRead Remove Eclipse warnings Change-Id: I0b6532194d5f818c694458f86fde22c5fef3fced
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; + } + }); } }