Merge branch 'stable-3.1' into stable-3.2

* stable-3.1:
  Use InternalUser for checking projects list
  Add test for setups without AnonymousRead
  Remove Eclipse warnings

Change-Id: I6327dfd09a545cef91765124c257a5afc89224bf
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;
+          }
+        });
   }
 }