Run submit requirement validation in ManualRequestContext

Before, SRs were validated on push, but the validation ran as the
pushing user. This means that user-dependent operators (in particular:
predicates that resolve account IDs) would parse differently in
validation and execution.

Test using a plugin-provided predicate. Also test directly that
the evaluation uses the server identity.

Change-Id: I8ab993459d00f6dd4d94aa068fada4a656d185c6
Bug: Google b/269077185
Release-Notes: n/a

java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java

diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 0991f20..39e12c4 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -77,7 +77,9 @@
   @Override
   public void validateExpression(SubmitRequirementExpression expression)
       throws QueryParseException {
-    queryBuilder.get().parse(expression.expressionString());
+    try (ManualRequestContext ignored = requestContext.open()) {
+      queryBuilder.get().parse(expression.expressionString());
+    }
   }
 
   @Override

javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java

diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
index a643d56..63068c6 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
@@ -19,13 +19,32 @@
 import static com.google.gerrit.acceptance.GitUtil.fetch;
 import static com.google.gerrit.acceptance.GitUtil.pushHead;
 
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
+import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.OperatorPredicate;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.List;
 import java.util.Locale;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.transport.PushResult;
@@ -447,6 +466,98 @@
     r.assertOkStatus();
   }
 
+  protected static class IsOperatorModule extends AbstractModule {
+    @Override
+    public void configure() {
+      bind(ChangeQueryBuilder.ChangeIsOperandFactory.class)
+          .annotatedWith(Exports.named("changeNumberEven"))
+          .to(SampleIsOperand.class);
+    }
+  }
+
+  private static class SampleIsOperand implements ChangeQueryBuilder.ChangeIsOperandFactory {
+    final Provider<CurrentUser> currentUserProvider;
+
+    @Inject
+    SampleIsOperand(Provider<CurrentUser> currentUserProvider) {
+      this.currentUserProvider = currentUserProvider;
+    }
+
+    @Override
+    public Predicate<ChangeData> create(ChangeQueryBuilder builder) throws QueryParseException {
+      return new IsSamplePredicate(currentUserProvider.get());
+    }
+  }
+
+  private static class IsSamplePredicate extends OperatorPredicate<ChangeData>
+      implements Matchable<ChangeData> {
+
+    CurrentUser currentUser;
+
+    public IsSamplePredicate(CurrentUser currentUser) {
+      super("is", "changeNumberEven");
+      this.currentUser = currentUser;
+      assertServerUser();
+    }
+
+    private void assertServerUser() {
+      try {
+        currentUser.asIdentifiedUser();
+        throw new IllegalStateException("is an identified user");
+      } catch (UnsupportedOperationException e) {
+        // as expected.
+      }
+    }
+
+    @Override
+    public boolean match(ChangeData changeData) {
+      assertServerUser();
+      return true;
+    }
+
+    @Override
+    public int getCost() {
+      return 0;
+    }
+  }
+
+  @Test
+  public void submitRequirementValidationRunsAsServer() throws Exception {
+    try (TestRepository<Repository> testRepo =
+        new TestRepository<>(repoManager.openRepository(project))) {
+      testRepo.delete(RefNames.REFS_CONFIG);
+    }
+
+    PushOneCommit.Result r = createChange();
+    String changeId = r.getChangeId();
+
+    try (AutoCloseable ignored = installPlugin("myplugin", IsOperatorModule.class)) {
+      PushOneCommit push =
+          pushFactory
+              .create(
+                  admin.newIdent(),
+                  testRepo,
+                  "Test Change",
+                  ProjectConfig.PROJECT_CONFIG,
+                  "[submit-requirement \"SAMPLE\"]\n"
+                      + "  submittableIf = is:changeNumberEven_myplugin\n")
+              .setParents(ImmutableList.of());
+      PushOneCommit.Result cfgPush = push.to(RefNames.REFS_CONFIG);
+      cfgPush.assertOkStatus();
+
+      ChangeInfo info = gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+      List<SubmitRequirementResultInfo> results =
+          info.submitRequirements.stream()
+              .filter(x -> x.name.equals("SAMPLE"))
+              .collect(Collectors.toList());
+      assertThat(results).hasSize(1);
+      assertThat(results.get(0).status).isNotEqualTo(SubmitRequirementResultInfo.Status.ERROR);
+    }
+
+    // TODO(hanwen): should return 500 ISE for
+    //  gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+  }
+
   @Test
   public void invalidSubmitRequirementIsRejectedWhenPushingForReview() throws Exception {
     fetchRefsMetaConfig();