Implement REST endpoint to reindex a project

We recently added a project index to Gerrit. This complements the
existing set of indices (change, account, group). For this new index we
want to offer a REST endpoint as well to reindex a project with the
option to recusively reindex the children as well.

Unfortunately, we previously bound /index to the endpoint that reindexes
all changes in the project. This commit changes that binding to
/index.changes.
We make an effort to keep the REST API stable, but this is just an
administrative endpoint and it seems to be a better fit to just
repurpose the endpoint name.

This commit adds tests and methods for the Java extension API.

Change-Id: I2b3bb9f38c94938b98059da193e80289996a4a41

Documentation/cmd-index-changes-in-project.txt

@@ -1,12 +1,12 @@
-= gerrit index project
+= gerrit index changes in project
 
 == NAME
-gerrit index project - Index all the changes in one or more projects.
+gerrit index changes in project - Index all the changes in one or more projects.
 
 == SYNOPSIS
 [verse]
 --
-_ssh_ -p <port> <host> _gerrit index project_ <PROJECT> [<PROJECT> ...]
+_ssh_ -p <port> <host> _gerrit index changes-in-project_ <PROJECT> [<PROJECT> ...]
 --
 
 == DESCRIPTION
@@ -26,7 +26,7 @@ This command is intended to be used in scripts.
 Index all changes in projects MyProject and NiceProject.
 
 ----
-    $ ssh -p 29418 user@review.example.com gerrit index project MyProject NiceProject
+    $ ssh -p 29418 user@review.example.com gerrit index changes-in-project MyProject NiceProject
 ----
 
 GERRIT

Documentation/cmd-index.txt

@@ -136,7 +136,7 @@ link:cmd-index-start.html[gerrit index start]::
 link:cmd-index-changes.html[gerrit index changes]::
 	Index one or more changes.
 
-link:cmd-index-project.html[gerrit index project]::
+link:cmd-index-changes-in-project.html[gerrit index changes-in-project]::
 	Index all the changes in one or more projects.
 
 link:cmd-logging-ls-level.html[gerrit logging ls-level]::

Documentation/rest-api-projects.txt

@@ -1354,6 +1354,31 @@ parameters `perm`, `account` and `ref`, for example:
 
 
 [[index]]
+=== Index project
+
+Adds or updates the current project (and children, if specified) in the secondary index.
+The indexing task is executed asynchronously in background, so this command
+returns immediately.
+
+As an input, a link:#index-project-input[IndexProjectInput] entity can be provided.
+
+.Request
+----
+  POST /projects/MyProject/index HTTP/1.0
+  Content-Type: application/json; charset=UTF-8
+
+  {
+    "index_children": "true"
+  }
+----
+
+.Response
+----
+  HTTP/1.1 202 Accepted
+  Content-Disposition: attachment
+----
+
+[[index.changes]]
 === Index all changes in a project
 
 Adds or updates all the changes belonging to a project in the secondary index.
@@ -1362,7 +1387,7 @@ returns immediately.
 
 .Request
 ----
-  POST /projects/MyProject/index HTTP/1.0
+  POST /projects/MyProject/index.changes HTTP/1.0
 ----
 
 .Response
@@ -3128,6 +3153,17 @@ The ref to which `HEAD` should be set, the `refs/heads` prefix can be
 omitted.
 |============================
 
+[[index-project-input]]
+=== IndexProjectInput
+The `IndexProjectInput` contains parameters for indexing a project.
+
+[options="header",cols="1,^2,4"]
+|================================
+|Field Name         ||Description
+|`index_children`   ||
+If children should be indexed recursively.
+|================================
+
 [[inherited-boolean-info]]
 === InheritedBooleanInfo
 A boolean value that can also be inherited.

java/com/google/gerrit/extensions/api/projects/IndexProjectInput.java

@@ -0,0 +1,19 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.projects;
+
+public class IndexProjectInput {
+  public Boolean indexChildren;
+}

java/com/google/gerrit/extensions/api/projects/ProjectApi.java

@@ -190,6 +190,13 @@ public interface ProjectApi {
    */
   void parent(String parent) throws RestApiException;
 
+  /**
+   * Reindex the project and children in case {@code indexChildren} is specified.
+   *
+   * @param indexChildren decides if children should be indexed recursively
+   */
+  void index(boolean indexChildren) throws RestApiException;
+
   /**
    * A default implementation which allows source compatibility when adding new methods to the
    * interface.
@@ -344,5 +351,10 @@ public interface ProjectApi {
     public void parent(String parent) throws RestApiException {
       throw new NotImplementedException();
     }
+
+    @Override
+    public void index(boolean indexChildren) throws RestApiException {
+      throw new NotImplementedException();
+    }
   }
 }

java/com/google/gerrit/server/api/projects/ProjectApiImpl.java

@@ -34,6 +34,7 @@ import com.google.gerrit.extensions.api.projects.DeleteBranchesInput;
 import com.google.gerrit.extensions.api.projects.DeleteTagsInput;
 import com.google.gerrit.extensions.api.projects.DescriptionInput;
 import com.google.gerrit.extensions.api.projects.HeadInput;
+import com.google.gerrit.extensions.api.projects.IndexProjectInput;
 import com.google.gerrit.extensions.api.projects.ParentInput;
 import com.google.gerrit.extensions.api.projects.ProjectApi;
 import com.google.gerrit.extensions.api.projects.ProjectInput;
@@ -64,6 +65,7 @@ import com.google.gerrit.server.restapi.project.GetConfig;
 import com.google.gerrit.server.restapi.project.GetDescription;
 import com.google.gerrit.server.restapi.project.GetHead;
 import com.google.gerrit.server.restapi.project.GetParent;
+import com.google.gerrit.server.restapi.project.Index;
 import com.google.gerrit.server.restapi.project.ListBranches;
 import com.google.gerrit.server.restapi.project.ListChildProjects;
 import com.google.gerrit.server.restapi.project.ListDashboards;
@@ -118,6 +120,7 @@ public class ProjectApiImpl implements ProjectApi {
   private final SetHead setHead;
   private final GetParent getParent;
   private final SetParent setParent;
+  private final Index index;
 
   @AssistedInject
   ProjectApiImpl(
@@ -150,6 +153,7 @@ public class ProjectApiImpl implements ProjectApi {
       SetHead setHead,
       GetParent getParent,
       SetParent setParent,
+      Index index,
       @Assisted ProjectResource project) {
     this(
         permissionBackend,
@@ -182,6 +186,7 @@ public class ProjectApiImpl implements ProjectApi {
         setHead,
         getParent,
         setParent,
+        index,
         null);
   }
 
@@ -216,6 +221,7 @@ public class ProjectApiImpl implements ProjectApi {
       SetHead setHead,
       GetParent getParent,
       SetParent setParent,
+      Index index,
       @Assisted String name) {
     this(
         permissionBackend,
@@ -248,6 +254,7 @@ public class ProjectApiImpl implements ProjectApi {
         setHead,
         getParent,
         setParent,
+        index,
         name);
   }
 
@@ -282,6 +289,7 @@ public class ProjectApiImpl implements ProjectApi {
       SetHead setHead,
       GetParent getParent,
       SetParent setParent,
+      Index index,
       String name) {
     this.permissionBackend = permissionBackend;
     this.createProject = createProject;
@@ -314,6 +322,7 @@ public class ProjectApiImpl implements ProjectApi {
     this.getParent = getParent;
     this.setParent = setParent;
     this.name = name;
+    this.index = index;
   }
 
   @Override
@@ -595,6 +604,17 @@ public class ProjectApiImpl implements ProjectApi {
     }
   }
 
+  @Override
+  public void index(boolean indexChildren) throws RestApiException {
+    try {
+      IndexProjectInput input = new IndexProjectInput();
+      input.indexChildren = indexChildren;
+      index.apply(checkExists(), input);
+    } catch (Exception e) {
+      throw asRestApiException("Cannot index project", e);
+    }
+  }
+
   private ProjectResource checkExists() throws ResourceNotFoundException {
     if (project == null) {
       throw new ResourceNotFoundException(name);

java/com/google/gerrit/server/restapi/project/Index.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2017 The Android Open Source Project
+// Copyright (C) 2018 The Android Open Source Project
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -16,57 +16,76 @@ package com.google.gerrit.server.restapi.project;
 
 import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
 
-import com.google.common.io.ByteStreams;
+import com.google.common.flogger.FluentLogger;
 import com.google.common.util.concurrent.ListeningExecutorService;
-import com.google.gerrit.common.data.GlobalCapability;
-import com.google.gerrit.extensions.annotations.RequiresCapability;
-import com.google.gerrit.extensions.api.projects.ProjectInput;
+import com.google.gerrit.extensions.api.projects.IndexProjectInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.index.project.ProjectIndexer;
 import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.git.MultiProgressMonitor;
-import com.google.gerrit.server.git.MultiProgressMonitor.Task;
 import com.google.gerrit.server.index.IndexExecutor;
-import com.google.gerrit.server.index.change.AllChangesIndexer;
-import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectResource;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
+import java.io.IOException;
 import java.util.concurrent.Future;
-import org.eclipse.jgit.util.io.NullOutputStream;
 
-@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
 @Singleton
-public class Index implements RestModifyView<ProjectResource, ProjectInput> {
+public class Index implements RestModifyView<ProjectResource, IndexProjectInput> {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  private final Provider<AllChangesIndexer> allChangesIndexerProvider;
-  private final ChangeIndexer indexer;
+  private final PermissionBackend permissionBackend;
+  private final ProjectIndexer indexer;
   private final ListeningExecutorService executor;
+  private final Provider<ListChildProjects> listChildProjectsProvider;
 
   @Inject
   Index(
-      Provider<AllChangesIndexer> allChangesIndexerProvider,
-      ChangeIndexer indexer,
-      @IndexExecutor(BATCH) ListeningExecutorService executor) {
-    this.allChangesIndexerProvider = allChangesIndexerProvider;
+      PermissionBackend permissionBackend,
+      ProjectIndexer indexer,
+      @IndexExecutor(BATCH) ListeningExecutorService executor,
+      Provider<ListChildProjects> listChildProjectsProvider) {
+    this.permissionBackend = permissionBackend;
     this.indexer = indexer;
     this.executor = executor;
+    this.listChildProjectsProvider = listChildProjectsProvider;
   }
 
   @Override
-  public Response.Accepted apply(ProjectResource resource, ProjectInput input) {
-    Project.NameKey project = resource.getNameKey();
-    Task mpt =
-        new MultiProgressMonitor(ByteStreams.nullOutputStream(), "Reindexing project")
-            .beginSubTask("", MultiProgressMonitor.UNKNOWN);
-    AllChangesIndexer allChangesIndexer = allChangesIndexerProvider.get();
-    allChangesIndexer.setVerboseOut(NullOutputStream.INSTANCE);
-    // The REST call is just a trigger for async reindexing, so it is safe to ignore the future's
-    // return value.
+  public Response.Accepted apply(ProjectResource rsrc, IndexProjectInput input)
+      throws IOException, AuthException, OrmException, PermissionBackendException,
+          ResourceConflictException {
+    permissionBackend.currentUser().check(GlobalPermission.MAINTAIN_SERVER);
+    String response = "Project " + rsrc.getName() + " submitted for reindexing";
+
+    reindexAsync(rsrc.getNameKey());
+    if (Boolean.TRUE.equals(input.indexChildren)) {
+      ListChildProjects listChildProjects = listChildProjectsProvider.get();
+      listChildProjects.setRecursive(true);
+      listChildProjects.apply(rsrc).forEach(p -> reindexAsync(new Project.NameKey(p.name)));
+
+      response += " (indexing children recursively)";
+    }
+    return Response.accepted(response);
+  }
+
+  private void reindexAsync(Project.NameKey project) {
     @SuppressWarnings("unused")
-    Future<Void> ignored =
-        executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt));
-    return Response.accepted("Project " + project + " submitted for reindexing");
+    Future<?> possiblyIgnoredError =
+        executor.submit(
+            () -> {
+              try {
+                indexer.index(project);
+              } catch (IOException e) {
+                logger.atWarning().withCause(e).log("reindexing project %s failed", project);
+              }
+            });
   }
 }

java/com/google/gerrit/server/restapi/project/IndexChanges.java

@@ -0,0 +1,72 @@
+// Copyright (C) 2017 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
+
+import com.google.common.io.ByteStreams;
+import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.common.data.GlobalCapability;
+import com.google.gerrit.extensions.annotations.RequiresCapability;
+import com.google.gerrit.extensions.api.projects.ProjectInput;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.git.MultiProgressMonitor;
+import com.google.gerrit.server.git.MultiProgressMonitor.Task;
+import com.google.gerrit.server.index.IndexExecutor;
+import com.google.gerrit.server.index.change.AllChangesIndexer;
+import com.google.gerrit.server.index.change.ChangeIndexer;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.util.concurrent.Future;
+import org.eclipse.jgit.util.io.NullOutputStream;
+
+@RequiresCapability(GlobalCapability.ADMINISTRATE_SERVER)
+@Singleton
+public class IndexChanges implements RestModifyView<ProjectResource, ProjectInput> {
+
+  private final Provider<AllChangesIndexer> allChangesIndexerProvider;
+  private final ChangeIndexer indexer;
+  private final ListeningExecutorService executor;
+
+  @Inject
+  IndexChanges(
+      Provider<AllChangesIndexer> allChangesIndexerProvider,
+      ChangeIndexer indexer,
+      @IndexExecutor(BATCH) ListeningExecutorService executor) {
+    this.allChangesIndexerProvider = allChangesIndexerProvider;
+    this.indexer = indexer;
+    this.executor = executor;
+  }
+
+  @Override
+  public Response.Accepted apply(ProjectResource resource, ProjectInput input) {
+    Project.NameKey project = resource.getNameKey();
+    Task mpt =
+        new MultiProgressMonitor(ByteStreams.nullOutputStream(), "Reindexing project")
+            .beginSubTask("", MultiProgressMonitor.UNKNOWN);
+    AllChangesIndexer allChangesIndexer = allChangesIndexerProvider.get();
+    allChangesIndexer.setVerboseOut(NullOutputStream.INSTANCE);
+    // The REST call is just a trigger for async reindexing, so it is safe to ignore the future's
+    // return value.
+    @SuppressWarnings("unused")
+    Future<?> possiblyIgnoredError =
+        executor.submit(allChangesIndexer.reindexProject(indexer, project, mpt, mpt));
+    return Response.accepted("Project " + project + " submitted for reindexing");
+  }
+}

java/com/google/gerrit/server/restapi/project/Module.java

@@ -68,6 +68,7 @@ public class Module extends RestApiModule {
     get(PROJECT_KIND, "statistics.git").to(GetStatistics.class);
     post(PROJECT_KIND, "gc").to(GarbageCollect.class);
     post(PROJECT_KIND, "index").to(Index.class);
+    post(PROJECT_KIND, "index.changes").to(IndexChanges.class);
 
     child(PROJECT_KIND, "branches").to(BranchesCollection.class);
     create(BRANCH_KIND).to(CreateBranch.class);

java/com/google/gerrit/sshd/commands/IndexChangesInProjectCommand.java

@@ -19,7 +19,7 @@ import static com.google.gerrit.common.data.GlobalCapability.MAINTAIN_SERVER;
 import com.google.gerrit.extensions.annotations.RequiresAnyCapability;
 import com.google.gerrit.server.project.ProjectResource;
 import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.restapi.project.Index;
+import com.google.gerrit.server.restapi.project.IndexChanges;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
 import com.google.inject.Inject;
@@ -28,10 +28,10 @@ import java.util.List;
 import org.kohsuke.args4j.Argument;
 
 @RequiresAnyCapability({MAINTAIN_SERVER})
-@CommandMetaData(name = "project", description = "Index changes of a project")
-final class IndexProjectCommand extends SshCommand {
+@CommandMetaData(name = "changes-in-project", description = "Index changes of a project")
+final class IndexChangesInProjectCommand extends SshCommand {
 
-  @Inject private Index index;
+  @Inject private IndexChanges index;
 
   @Argument(
       index = 0,

java/com/google/gerrit/sshd/commands/IndexCommandsModule.java

@@ -40,6 +40,6 @@ public class IndexCommandsModule extends CommandModule {
       command(index, IndexStartCommand.class);
     }
     command(index, IndexChangesCommand.class);
-    command(index, IndexProjectCommand.class);
+    command(index, IndexChangesInProjectCommand.class);
   }
 }

javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java

@@ -15,10 +15,13 @@
 package com.google.gerrit.acceptance.api.project;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.server.git.QueueProvider.QueueType.BATCH;
 import static java.util.stream.Collectors.toSet;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.util.concurrent.AtomicLongMap;
+import com.google.common.util.concurrent.ListeningExecutorService;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.GitUtil;
 import com.google.gerrit.acceptance.NoHttpd;
@@ -39,7 +42,9 @@ import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.index.IndexExecutor;
 import com.google.inject.Inject;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.PushResult;
@@ -52,6 +57,10 @@ import org.junit.Test;
 public class ProjectIT extends AbstractDaemonTest {
   @Inject private DynamicSet<ProjectIndexedListener> projectIndexedListeners;
 
+  @Inject
+  @IndexExecutor(BATCH)
+  private ListeningExecutorService executor;
+
   private ProjectIndexedCounter projectIndexedCounter;
   private RegistrationHandle projectIndexedCounterHandle;
 
@@ -381,6 +390,26 @@ public class ProjectIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void reindexProject() throws Exception {
+    createProject("child", project);
+    projectIndexedCounter.clear();
+
+    gApi.projects().name(allProjects.get()).index(false);
+    projectIndexedCounter.assertReindexOf(allProjects.get());
+  }
+
+  @Test
+  public void reindexProjectWithChildren() throws Exception {
+    Project.NameKey middle = createProject("middle", project);
+    Project.NameKey leave = createProject("leave", middle);
+    projectIndexedCounter.clear();
+
+    gApi.projects().name(project.get()).index(true);
+    projectIndexedCounter.assertReindexExactly(
+        ImmutableMap.of(project.get(), 1L, middle.get(), 1L, leave.get(), 1L));
+  }
+
   private ConfigInput createTestConfigInput() {
     ConfigInput input = new ConfigInput();
     input.description = "some description";
@@ -427,5 +456,10 @@ public class ProjectIT extends AbstractDaemonTest {
     void assertNoReindex() {
       assertThat(countsByProject).isEmpty();
     }
+
+    void assertReindexExactly(ImmutableMap<String, Long> expected) {
+      assertThat(countsByProject.asMap()).containsExactlyEntriesIn(expected);
+      clear();
+    }
   }
 }

javatests/com/google/gerrit/acceptance/ssh/AbstractIndexTests.java

@@ -102,7 +102,7 @@ public abstract class AbstractIndexTests extends AbstractDaemonTest {
     enableChangeIndexWrites();
 
     changeIndexedCounter.clear();
-    String cmd = Joiner.on(" ").join("gerrit", "index", "project", project.get());
+    String cmd = Joiner.on(" ").join("gerrit", "index", "changes-in-project", project.get());
     adminSshSession.exec(cmd);
     adminSshSession.assertSuccess();
 

javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java

@@ -93,7 +93,8 @@ public class SshCommandsIT extends AbstractDaemonTest {
                 }
               }),
           "index",
-          ImmutableList.of("changes", "project"), // "activate" and "start" are not included
+          ImmutableList.of(
+              "changes", "changes-in-project"), // "activate" and "start" are not included
           "logging",
           ImmutableList.of("ls", "set"),
           "plugin",