Merge "New extension to validate branch updates by submit strategies."

Documentation/config-validation.txt

@@ -45,6 +45,18 @@ are merged to the git repository.
 If the commit fails the validation, the plugin can throw an exception
 which will cause the merge to fail.
 
+[[on-submit-validation]]
+== On submit validation
+
+
+Plugins implementing the `OnSubmitValidationListener` interface can
+perform additional validation checks against ref operations resuling
+from execution of submit operation before they are applied to any git
+repositories (there could be more than one in case of topic submits).
+
+Plugin can throw an exception which will cause submit operation to be
+aborted.
+
 [[pre-upload-validation]]
 == Pre-upload validation
 

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java

@@ -29,6 +29,7 @@ import static org.junit.Assert.fail;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
@@ -45,6 +46,8 @@ import com.google.gerrit.extensions.client.ListChangesOption;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.common.LabelInfo;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BinaryResult;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -56,6 +59,7 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.change.RevisionResource;
@@ -63,8 +67,10 @@ import com.google.gerrit.server.change.Submit;
 import com.google.gerrit.server.git.BatchUpdate;
 import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
 import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.Util;
+import com.google.gerrit.server.validators.ValidationException;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gerrit.testutil.TestTimeUtil;
 import com.google.gwtorm.server.OrmException;
@@ -86,9 +92,13 @@ import org.junit.Before;
 import org.junit.Test;
 
 import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 
 @NoHttpd
@@ -110,6 +120,10 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
   @Inject
   private BatchUpdate.Factory updateFactory;
 
+  @Inject
+  private DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
+  private RegistrationHandle onSubmitValidatorHandle;
+
   private String systemTimeZone;
 
   @Before
@@ -129,6 +143,13 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     db.close();
   }
 
+  @After
+  public void removeOnSubmitValidator(){
+    if (onSubmitValidatorHandle != null){
+      onSubmitValidatorHandle.remove();
+    }
+  }
+
   protected abstract SubmitType getSubmitType();
 
   @Test
@@ -647,6 +668,94 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     assertThat(getRemoteHead()).isEqualTo(headAfterSubmit);
   }
 
+  @Test
+  public void submitWithValidation() throws Exception {
+    AtomicBoolean called = new AtomicBoolean(false);
+    this.addOnSubmitValidationListener(new OnSubmitValidationListener() {
+      @Override
+      public void preBranchUpdate(Arguments args) throws ValidationException {
+        called.set(true);
+        HashSet<String> refs = Sets.newHashSet(args.getCommands().keySet());
+        assertThat(refs).contains("refs/heads/master");
+        refs.remove("refs/heads/master");
+        if (!refs.isEmpty()){
+          // Some submit strategies need to insert new patchset.
+          assertThat(refs).hasSize(1);
+          assertThat(refs.iterator().next()).startsWith(RefNames.REFS_CHANGES);
+        }
+      }
+    });
+
+    PushOneCommit.Result change = createChange();
+    approve(change.getChangeId());
+    submit(change.getChangeId());
+    assertThat(called.get()).isTrue();
+  }
+
+  @Test
+  public void submitWithValidationMultiRepo() throws Exception {
+    assume().that(isSubmitWholeTopicEnabled()).isTrue();
+    String topic = "test-topic";
+
+    // Create test projects
+    TestRepository<?> repoA =
+        createProjectWithPush("project-a", null, getSubmitType());
+    TestRepository<?> repoB =
+        createProjectWithPush("project-b", null, getSubmitType());
+
+    // Create changes on project-a
+    PushOneCommit.Result change1 =
+        createChange(repoA, "master", "Change 1", "a.txt", "content", topic);
+    PushOneCommit.Result change2 =
+        createChange(repoA, "master", "Change 2", "b.txt", "content", topic);
+
+    // Create changes on project-b
+    PushOneCommit.Result change3 =
+        createChange(repoB, "master", "Change 3", "a.txt", "content", topic);
+    PushOneCommit.Result change4 =
+        createChange(repoB, "master", "Change 4", "b.txt", "content", topic);
+
+    List<PushOneCommit.Result> changes =
+        Lists.newArrayList(change1, change2, change3, change4);
+    for (PushOneCommit.Result change : changes) {
+      approve(change.getChangeId());
+    }
+
+    // Construct validator which will throw on a second call.
+    // Since there are 2 repos, first submit attempt will fail, the second will
+    // succeed.
+    List<String> projectsCalled = new ArrayList<>(4);
+    this.addOnSubmitValidationListener(new OnSubmitValidationListener() {
+      @Override
+      public void preBranchUpdate(Arguments args) throws ValidationException {
+        assertThat(args.getCommands().keySet()).contains("refs/heads/master");
+        try (RevWalk rw = args.newRevWalk()) {
+          rw.parseBody(rw.parseCommit(
+              args.getCommands().get("refs/heads/master").getNewId()));
+        } catch (IOException e) {
+          assertThat(e).isNull();
+        }
+        projectsCalled.add(args.getProject().get());
+        if (projectsCalled.size() == 2) {
+          throw new ValidationException("time to fail");
+        }
+      }
+    });
+    submitWithConflict(change4.getChangeId(), "time to fail");
+    assertThat(projectsCalled).containsExactly(name("project-a"),
+        name("project-b"));
+    for (PushOneCommit.Result change : changes) {
+      change.assertChange(Change.Status.NEW, name(topic), admin);
+    }
+
+    submit(change4.getChangeId());
+    assertThat(projectsCalled).containsExactly(name("project-a"),
+        name("project-b"), name("project-a"), name("project-b"));
+    for (PushOneCommit.Result change : changes) {
+      change.assertChange(Change.Status.MERGED, name(topic), admin);
+    }
+  }
+
   private void setChangeStatusToNew(PushOneCommit.Result... changes)
       throws Exception {
     for (PushOneCommit.Result change : changes) {
@@ -881,6 +990,11 @@ public abstract class AbstractSubmit extends AbstractDaemonTest {
     return getRemoteLog(project, "master");
   }
 
+  protected void addOnSubmitValidationListener(OnSubmitValidationListener listener){
+    assertThat(onSubmitValidatorHandle).isNull();
+    onSubmitValidatorHandle = onSubmitValidationListeners.add(listener);
+  }
+
   private String getLatestDiff(Repository repo) throws Exception {
     ObjectId oldTreeId = repo.resolve("HEAD~1^{tree}");
     ObjectId newTreeId = repo.resolve("HEAD^{tree}");

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -128,6 +128,8 @@ import com.google.gerrit.server.git.validators.MergeValidators;
 import com.google.gerrit.server.git.validators.MergeValidators.ProjectConfigValidator;
 import com.google.gerrit.server.git.validators.RefOperationValidationListener;
 import com.google.gerrit.server.git.validators.RefOperationValidators;
+import com.google.gerrit.server.git.validators.OnSubmitValidationListener;
+import com.google.gerrit.server.git.validators.OnSubmitValidators;
 import com.google.gerrit.server.git.validators.UploadValidationListener;
 import com.google.gerrit.server.git.validators.UploadValidators;
 import com.google.gerrit.server.group.GroupInfoCache;
@@ -350,6 +352,7 @@ public class GerritGlobalModule extends FactoryModule {
     DynamicSet.setOf(binder(), CommitValidationListener.class);
     DynamicSet.setOf(binder(), ChangeMessageModifier.class);
     DynamicSet.setOf(binder(), RefOperationValidationListener.class);
+    DynamicSet.setOf(binder(), OnSubmitValidationListener.class);
     DynamicSet.setOf(binder(), MergeValidationListener.class);
     DynamicSet.setOf(binder(), ProjectCreationValidationListener.class);
     DynamicSet.setOf(binder(), GroupCreationValidationListener.class);
@@ -391,6 +394,7 @@ public class GerritGlobalModule extends FactoryModule {
 
     factory(AbandonOp.Factory.class);
     factory(RefOperationValidators.Factory.class);
+    factory(OnSubmitValidators.Factory.class);
     factory(MergeValidators.Factory.class);
     factory(ProjectConfigValidator.Factory.class);
     factory(NotesBranchUtil.Factory.class);

gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java

@@ -51,6 +51,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.validators.OnSubmitValidators;
 import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
@@ -532,6 +533,7 @@ public class BatchUpdate implements AutoCloseable {
   private BatchRefUpdate batchRefUpdate;
   private boolean closeRepo;
   private Order order;
+  private OnSubmitValidators onSubmitValidators;
   private boolean updateChangesInParallel;
   private RequestId requestId;
 
@@ -606,6 +608,15 @@ public class BatchUpdate implements AutoCloseable {
     return this;
   }
 
+  /**
+   * Add a validation step for intended ref operations, which will be performed
+   * at the end of {@link RepoOnlyOp#updateRepo(RepoContext)} step.
+   */
+  BatchUpdate setOnSubmitValidators(OnSubmitValidators onSubmitValidators) {
+    this.onSubmitValidators = onSubmitValidators;
+    return this;
+  }
+
   /**
    * Execute {@link Op#updateChange(ChangeContext)} in parallel for each change.
    */
@@ -692,6 +703,18 @@ public class BatchUpdate implements AutoCloseable {
         op.updateRepo(ctx);
       }
 
+      if (onSubmitValidators != null && commands != null
+          && !commands.isEmpty()) {
+        // Validation of refs has to take place here and not at the beginning
+        // executeRefUpdates. Otherwise failing validation in a second
+        // BatchUpdate object will happen *after* first object's
+        // executeRefUpdates has finished, hence after first repo's refs have
+        // been updated, which is too late.
+        onSubmitValidators.validate(project,
+            new ReadOnlyRepository(getRepository()),
+            ctx.getInserter().newReader(), commands.getCommands());
+      }
+
       if (inserter != null) {
         logDebug("Flushing inserter");
         inserter.flush();

gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOpRepoManager.java

@@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
+import com.google.gerrit.server.git.validators.OnSubmitValidators;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
@@ -107,7 +108,8 @@ public class MergeOpRepoManager implements AutoCloseable {
       if (update == null) {
         update = batchUpdateFactory.create(db, getProjectName(), caller, ts)
             .setRepository(repo, rw, ins)
-            .setRequestId(submissionId);
+            .setRequestId(submissionId)
+            .setOnSubmitValidators(onSubmitValidatorsFactory.create());
       }
       return update;
     }
@@ -157,6 +159,7 @@ public class MergeOpRepoManager implements AutoCloseable {
 
   private final Map<Project.NameKey, OpenRepo> openRepos;
   private final BatchUpdate.Factory batchUpdateFactory;
+  private final OnSubmitValidators.Factory onSubmitValidatorsFactory;
   private final GitRepositoryManager repoManager;
   private final ProjectCache projectCache;
 
@@ -169,10 +172,12 @@ public class MergeOpRepoManager implements AutoCloseable {
   MergeOpRepoManager(
       GitRepositoryManager repoManager,
       ProjectCache projectCache,
-      BatchUpdate.Factory batchUpdateFactory) {
+      BatchUpdate.Factory batchUpdateFactory,
+      OnSubmitValidators.Factory onSubmitValidatorsFactory) {
     this.repoManager = repoManager;
     this.projectCache = projectCache;
     this.batchUpdateFactory = batchUpdateFactory;
+    this.onSubmitValidatorsFactory = onSubmitValidatorsFactory;
 
     openRepos = new HashMap<>();
   }

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/SubmitStrategy.java

@@ -46,6 +46,7 @@ import com.google.gerrit.server.git.MergeTip;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.SubmoduleOp;
 import com.google.gerrit.server.git.TagCache;
+import com.google.gerrit.server.git.validators.OnSubmitValidators;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
@@ -118,6 +119,7 @@ public abstract class SubmitStrategy {
     final ProjectCache projectCache;
     final PersonIdent serverIdent;
     final RebaseChangeOp.Factory rebaseFactory;
+    final OnSubmitValidators.Factory onSubmitValidatorsFactory;
     final TagCache tagCache;
 
     final Branch.NameKey destBranch;
@@ -158,6 +160,7 @@ public abstract class SubmitStrategy {
         @GerritPersonIdent PersonIdent serverIdent,
         ProjectCache projectCache,
         RebaseChangeOp.Factory rebaseFactory,
+        OnSubmitValidators.Factory onSubmitValidatorsFactory,
         TagCache tagCache,
         @Assisted Branch.NameKey destBranch,
         @Assisted CommitStatus commits,
@@ -212,6 +215,7 @@ public abstract class SubmitStrategy {
             "project not found: %s", destBranch.getParentKey());
       this.mergeSorter = new MergeSorter(rw, alreadyAccepted, canMergeFlag);
       this.mergeUtil = mergeUtilFactory.create(project);
+      this.onSubmitValidatorsFactory = onSubmitValidatorsFactory;
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/OnSubmitValidationListener.java

@@ -0,0 +1,85 @@
+// 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.git.validators;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.server.validators.ValidationException;
+
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+import java.util.Map;
+
+/**
+ * Listener to validate ref updates performed during submit operation.
+ *
+ * As submit strategies may generate new commits (e.g. Cherry Pick), this
+ * listener allows validation of resulting new commit before destination branch
+ * is updated and new patchset ref is created.
+ *
+ * If you only care about validating the change being submitted and not the
+ * resulting new commit, consider using {@link MergeValidationListener} instead.
+ */
+@ExtensionPoint
+public interface OnSubmitValidationListener {
+  public class Arguments {
+    private Project.NameKey project;
+    private Repository repository;
+    private ObjectReader objectReader;
+    private Map<String, ReceiveCommand> commands;
+
+    public Arguments(NameKey project, Repository repository,
+        ObjectReader objectReader, Map<String, ReceiveCommand> commands) {
+      this.project = project;
+      this.repository = repository;
+      this.objectReader = objectReader;
+      this.commands = commands;
+    }
+
+    public Project.NameKey getProject() {
+      return project;
+    }
+
+    /**
+     * @return a read only repository
+     */
+    public Repository getRepository() {
+      return repository;
+    }
+
+    public RevWalk newRevWalk() {
+      return new RevWalk(objectReader);
+    }
+
+    /**
+     * @return a map from ref to op on it covering all ref ops to be performed
+     *         on this repository as part of ongoing submit operation.
+     */
+    public Map<String, ReceiveCommand> getCommands(){
+      return commands;
+    }
+  }
+
+  /**
+   * Called right before branch is updated with new commit or commits as a
+   * result of submit.
+   *
+   * If ValidationException is thrown, submitting is aborted.
+   */
+  void preBranchUpdate(Arguments args) throws ValidationException;
+}

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/OnSubmitValidators.java

@@ -0,0 +1,53 @@
+// 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.git.validators;
+
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.git.IntegrationException;
+import com.google.gerrit.server.git.validators.OnSubmitValidationListener.Arguments;
+import com.google.gerrit.server.validators.ValidationException;
+import com.google.inject.Inject;
+
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+import java.util.Map;
+
+public class OnSubmitValidators {
+  public interface Factory {
+    OnSubmitValidators create();
+  }
+
+  private final DynamicSet<OnSubmitValidationListener> listeners;
+
+  @Inject
+  OnSubmitValidators(DynamicSet<OnSubmitValidationListener> listeners) {
+    this.listeners = listeners;
+  }
+
+  public void validate(Project.NameKey project, Repository repo,
+      ObjectReader objectReader, Map<String, ReceiveCommand> commands)
+      throws IntegrationException {
+    try {
+      for (OnSubmitValidationListener listener : this.listeners) {
+        listener.preBranchUpdate(
+            new Arguments(project, repo, objectReader, commands));
+      }
+    } catch (ValidationException e) {
+      throw new IntegrationException(e.getMessage());
+    }
+  }
+}