Merge "Add integration test to validate submit requirements on upload"

3 files changed, 257 insertions, 10 deletions

diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 975ad52587..8f0b5357a4 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -943,8 +943,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       if (lowerNames.containsKey(lower)) {
         error(
             String.format(
-                "Submit requirement \"%s\" conflicts with \"%s\". Skipping the former.",
-                name, lowerNames.get(lower)));
+                "Submit requirement '%s' conflicts with '%s'.", name, lowerNames.get(lower)));
         continue;
       }
       lowerNames.put(lower, name);
@@ -958,9 +957,7 @@ public class ProjectConfig extends VersionedMetaData implements ValidationError.
       if (blockExpr == null) {
         error(
             String.format(
-                "Submit requirement \"%s\" does not define a submittability expression."
-                    + " Skipping this requirement.",
-                name));
+                "Submit requirement '%s' does not define a submittability expression.", name));
         continue;
       }
 
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
new file mode 100644
index 0000000000..4675bc0a0c
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
@@ -0,0 +1,252 @@
+// Copyright (C) 2021 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.acceptance.server.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.gerrit.acceptance.GitUtil.fetch;
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.server.project.ProjectConfig;
+import java.util.Locale;
+import java.util.function.Consumer;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
+import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.util.RawParseUtils;
+import org.junit.Test;
+
+/**
+ * Tests validating submit requirements on upload of {@code project.config} to {@code
+ * refs/meta/config}.
+ */
+public class SubmitRequirementsValidationIT extends AbstractDaemonTest {
+  @Test
+  public void validSubmitRequirementIsAccepted_optionalParametersNotSet() throws Exception {
+    fetchRefsMetaConfig();
+
+    String submitRequirementName = "Code-Review";
+    updateProjectConfig(
+        projectConfig ->
+            projectConfig.setString(
+                ProjectConfig.SUBMIT_REQUIREMENT,
+                /* subsection= */ submitRequirementName,
+                /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+                /* value= */ "label:\"code-review=+2\""));
+
+    PushResult r = pushRefsMetaConfig();
+    assertOkStatus(r);
+  }
+
+  @Test
+  public void validSubmitRequirementIsAccepted_allParametersSet() throws Exception {
+    fetchRefsMetaConfig();
+
+    String submitRequirementName = "Code-Review";
+    updateProjectConfig(
+        projectConfig -> {
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_DESCRIPTION,
+              /* value= */ "foo bar description");
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION,
+              /* value= */ "branch:refs/heads/master");
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+              /* value= */ "label:\"code-review=+2\"");
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION,
+              /* value= */ "label:\"override=+1\"");
+          projectConfig.setBoolean(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_OVERRIDE_IN_CHILD_PROJECTS,
+              /* value= */ false);
+        });
+
+    PushResult r = pushRefsMetaConfig();
+    assertOkStatus(r);
+  }
+
+  @Test
+  public void conflictingSubmitRequirementsAreRejected() throws Exception {
+    fetchRefsMetaConfig();
+
+    String submitRequirementName = "Code-Review";
+    updateProjectConfig(
+        projectConfig -> {
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName,
+              /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+              /* value= */ "label:\"code-review=+2\"");
+          projectConfig.setString(
+              ProjectConfig.SUBMIT_REQUIREMENT,
+              /* subsection= */ submitRequirementName.toLowerCase(Locale.US),
+              /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+              /* value= */ "label:\"code-review=+2\"");
+        });
+
+    PushResult r = pushRefsMetaConfig();
+    assertErrorStatus(
+        r,
+        "Invalid project configuration",
+        String.format(
+            "project.config: Submit requirement '%s' conflicts with '%s'.",
+            submitRequirementName.toLowerCase(Locale.US), submitRequirementName));
+  }
+
+  @Test
+  public void conflictingSubmitRequirementIsRejected() throws Exception {
+    fetchRefsMetaConfig();
+    String submitRequirementName = "Code-Review";
+    updateProjectConfig(
+        projectConfig ->
+            projectConfig.setString(
+                ProjectConfig.SUBMIT_REQUIREMENT,
+                /* subsection= */ submitRequirementName,
+                /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+                /* value= */ "label:\"code-review=+2\""));
+    PushResult r = pushRefsMetaConfig();
+    assertOkStatus(r);
+
+    updateProjectConfig(
+        projectConfig ->
+            projectConfig.setString(
+                ProjectConfig.SUBMIT_REQUIREMENT,
+                /* subsection= */ submitRequirementName.toLowerCase(Locale.US),
+                /* name= */ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION,
+                /* value= */ "label:\"code-review=+2\""));
+    r = pushRefsMetaConfig();
+    assertErrorStatus(
+        r,
+        "Invalid project configuration",
+        String.format(
+            "project.config: Submit requirement '%s' conflicts with '%s'.",
+            submitRequirementName.toLowerCase(Locale.US), submitRequirementName));
+  }
+
+  @Test
+  public void submitRequirementWithoutSubmittabilityExpressionIsRejected() throws Exception {
+    fetchRefsMetaConfig();
+
+    String submitRequirementName = "Code-Review";
+    updateProjectConfig(
+        projectConfig ->
+            projectConfig.setString(
+                ProjectConfig.SUBMIT_REQUIREMENT,
+                /* subsection= */ submitRequirementName,
+                /* name= */ ProjectConfig.KEY_SR_DESCRIPTION,
+                /* value= */ "foo bar description"));
+
+    PushResult r = pushRefsMetaConfig();
+    assertErrorStatus(
+        r,
+        "Invalid project configuration",
+        String.format(
+            "project.config: Submit requirement '%s' does not define a submittability expression.",
+            submitRequirementName));
+  }
+
+  private void fetchRefsMetaConfig() throws Exception {
+    fetch(testRepo, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
+    testRepo.reset(RefNames.REFS_CONFIG);
+  }
+
+  private PushResult pushRefsMetaConfig() throws Exception {
+    return pushHead(testRepo, RefNames.REFS_CONFIG);
+  }
+
+  private void updateProjectConfig(Consumer<Config> configUpdater) throws Exception {
+    RevCommit head = getHead(testRepo.getRepository(), RefNames.REFS_CONFIG);
+    Config projectConfig = readProjectConfig(head);
+    configUpdater.accept(projectConfig);
+    RevCommit commit =
+        testRepo.update(
+            RefNames.REFS_CONFIG,
+            testRepo
+                .commit()
+                .parent(head)
+                .message("Update project config")
+                .author(admin.newIdent())
+                .committer(admin.newIdent())
+                .add(ProjectConfig.PROJECT_CONFIG, projectConfig.toText()));
+
+    testRepo.reset(commit);
+  }
+
+  private Config readProjectConfig(RevCommit commit) throws Exception {
+    try (TreeWalk tw =
+        TreeWalk.forPath(
+            testRepo.getRevWalk().getObjectReader(),
+            ProjectConfig.PROJECT_CONFIG,
+            commit.getTree())) {
+      if (tw == null) {
+        throw new IllegalStateException(
+            String.format("%s does not exist", ProjectConfig.PROJECT_CONFIG));
+      }
+    }
+    RevObject blob = testRepo.get(commit.getTree(), ProjectConfig.PROJECT_CONFIG);
+    byte[] data = testRepo.getRepository().open(blob).getCachedBytes(Integer.MAX_VALUE);
+    String content = RawParseUtils.decode(data);
+
+    Config projectConfig = new Config();
+    projectConfig.fromText(content);
+    return projectConfig;
+  }
+
+  public void assertOkStatus(PushResult result) {
+    RemoteRefUpdate refUpdate = result.getRemoteUpdate(RefNames.REFS_CONFIG);
+    assertThat(refUpdate).isNotNull();
+    assertWithMessage(getMessage(result, refUpdate))
+        .that(refUpdate.getStatus())
+        .isEqualTo(Status.OK);
+  }
+
+  public void assertErrorStatus(PushResult result, String... expectedMessages) {
+    RemoteRefUpdate refUpdate = result.getRemoteUpdate(RefNames.REFS_CONFIG);
+    assertThat(refUpdate).isNotNull();
+    assertWithMessage(getMessage(result, refUpdate))
+        .that(refUpdate.getStatus())
+        .isEqualTo(Status.REJECTED_OTHER_REASON);
+    for (String expectedMessage : expectedMessages) {
+      assertThat(result.getMessages()).contains(expectedMessage);
+    }
+  }
+
+  private String getMessage(PushResult result, RemoteRefUpdate refUpdate) {
+    StringBuilder b = new StringBuilder();
+    if (refUpdate.getMessage() != null) {
+      b.append(refUpdate.getMessage());
+      b.append("\n");
+    }
+    b.append(result.getMessages());
+    return b.toString();
+  }
+}
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 7f0b685f38..9df59c2806 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -310,9 +310,7 @@ public class ProjectConfigTest {
     assertThat(cfg.getValidationErrors()).hasSize(1);
     assertThat(Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage())
         .isEqualTo(
-            "project.config: "
-                + "Submit requirement \"Code-Review\" conflicts with \"code-review\". "
-                + "Skipping the former.");
+            "project.config: Submit requirement 'Code-Review' conflicts with 'code-review'.");
   }
 
   @Test
@@ -332,8 +330,8 @@ public class ProjectConfigTest {
     assertThat(cfg.getValidationErrors()).hasSize(1);
     assertThat(Iterables.getOnlyElement(cfg.getValidationErrors()).getMessage())
         .isEqualTo(
-            "project.config: Submit requirement \"code-review\" does not define a submittability"
-                + " expression. Skipping this requirement.");
+            "project.config: Submit requirement 'code-review' does not define a submittability"
+                + " expression.");
   }
 
   @Test