Merge changes from topic 'submit-rule-evaluator'

* changes:
  Disallow current_user/1 predicate in submit type rules
  Use SubmitRuleEvaluator to get submit type from Mergeable
  Remove ChangeControl.getSubmitTypeRecord methods
  ChangeJson: Remove unused ChangeControl arg from submitRecords
  Remove ChangeControl.canSubmit(...) methods
  Avoid throwing OrmException from SubmitRuleEvaluator methods
  Move boolean args from canSubmit to SubmitRuleEvaluator
  Move conversion to SubmitTypeRecords into SubmitRuleEvaluator
  Move conversion to SubmitRecords into SubmitRuleEvaluator
  SubmitRuleEvaluator: Check state in getSubmitRule()
  Refactor SubmitRuleEvaluator to reduce confusing arguments
  Construct ChangeDatas with ChangeControls in REST handlers

gerrit-server/src/main/java/com/google/gerrit/rules/StoredValues.java

@@ -25,6 +25,7 @@ import com.google.gerrit.reviewdb.client.PatchSetInfo;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.patch.PatchList;
@@ -50,7 +51,13 @@ public final class StoredValues {
   public static final StoredValue<ReviewDb> REVIEW_DB = create(ReviewDb.class);
   public static final StoredValue<ChangeData> CHANGE_DATA = create(ChangeData.class);
   public static final StoredValue<PatchSet> PATCH_SET = create(PatchSet.class);
+
+  // Note: no guarantees are made about the user passed in the ChangeControl; do
+  // not depend on this directly. Either use .forUser(otherUser) to get a
+  // control for a specific known user, or use CURRENT_USER, which may be null
+  // for rule types that may not depend on the current user.
   public static final StoredValue<ChangeControl> CHANGE_CONTROL = create(ChangeControl.class);
+  public static final StoredValue<CurrentUser> CURRENT_USER = create(CurrentUser.class);
 
   public static Change getChange(Prolog engine) throws SystemException {
     ChangeData cd = CHANGE_DATA.get(engine);

gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java

@@ -89,6 +89,7 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeData.ChangedLines;
 import com.google.gwtorm.server.OrmException;
@@ -352,19 +353,18 @@ public class ChangeJson {
     return c.isMergeable();
   }
 
-  private List<SubmitRecord> submitRecords(ChangeControl ctl, ChangeData cd)
+  private List<SubmitRecord> submitRecords(ChangeData cd) throws OrmException {
-      throws OrmException {
     if (cd.getSubmitRecords() != null) {
       return cd.getSubmitRecords();
     }
-    if (ctl == null) {
-      return ImmutableList.of();
-    }
     PatchSet ps = cd.currentPatchSet();
     if (ps == null) {
       return ImmutableList.of();
     }
-    cd.setSubmitRecords(ctl.canSubmit(db.get(), ps, cd, true, false, true));
+    cd.setSubmitRecords(new SubmitRuleEvaluator(cd).setPatchSet(ps)
+        .setFastEvalLabels(true)
+        .setAllowDraft(true)
+        .canSubmit());
     return cd.getSubmitRecords();
   }
 
@@ -418,7 +418,7 @@ public class ChangeJson {
       LabelTypes labelTypes, boolean standard) throws OrmException {
     // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
     Map<String, LabelInfo> labels = new TreeMap<>(labelTypes.nameComparator());
-    for (SubmitRecord rec : submitRecords(ctl, cd)) {
+    for (SubmitRecord rec : submitRecords(cd)) {
       if (rec.labels == null) {
         continue;
       }
@@ -619,7 +619,7 @@ public class ChangeJson {
 
     LabelTypes labelTypes = ctl.getLabelTypes();
     SetMultimap<String, String> permitted = LinkedHashMultimap.create();
-    for (SubmitRecord rec : submitRecords(ctl, cd)) {
+    for (SubmitRecord rec : submitRecords(cd)) {
       if (rec.labels == null) {
         continue;
       }

gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.change;
 
 import com.google.common.collect.Sets;
+import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.extensions.common.SubmitType;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -32,6 +33,8 @@ import com.google.gerrit.server.git.strategy.SubmitStrategyFactory;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.AtomicUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -79,9 +82,9 @@ public class Mergeable implements RestReadView<RevisionResource> {
     this.force = force;
   }
 
-  private final TestSubmitType.Get submitType;
   private final GitRepositoryManager gitManager;
   private final ProjectCache projectCache;
+  private final ChangeData.Factory changeDataFactory;
   private final SubmitStrategyFactory submitStrategyFactory;
   private final Provider<ReviewDb> db;
   private final ChangeIndexer indexer;
@@ -89,15 +92,15 @@ public class Mergeable implements RestReadView<RevisionResource> {
   private boolean force;
 
   @Inject
-  Mergeable(TestSubmitType.Get submitType,
+  Mergeable(GitRepositoryManager gitManager,
-      GitRepositoryManager gitManager,
       ProjectCache projectCache,
+      ChangeData.Factory changeDataFactory,
       SubmitStrategyFactory submitStrategyFactory,
       Provider<ReviewDb> db,
       ChangeIndexer indexer) {
-    this.submitType = submitType;
     this.gitManager = gitManager;
     this.projectCache = projectCache;
+    this.changeDataFactory = changeDataFactory;
     this.submitStrategyFactory = submitStrategyFactory;
     this.db = db;
     this.indexer = indexer;
@@ -117,7 +120,14 @@ public class Mergeable implements RestReadView<RevisionResource> {
       return result;
     }
 
-    result.submitType = submitType.apply(resource);
+    ChangeData cd = changeDataFactory.create(db.get(), resource.getControl());
+    SubmitTypeRecord rec = new SubmitRuleEvaluator(cd)
+        .setPatchSet(ps)
+        .getSubmitType();
+    if (rec.status != SubmitTypeRecord.Status.OK) {
+      throw new OrmException("Submit type rule failed: " + rec);
+    }
+    result.submitType = rec.type;
     result.mergeable = change.isMergeable();
 
     Repository git = gitManager.openRepository(change.getProject());

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -289,7 +289,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
         in.entrySet().iterator();
     Set<String> filePaths =
         Sets.newHashSet(changeDataFactory.create(
-            db.get(), revision.getChange()).filePaths(
+            db.get(), revision.getControl()).filePaths(
                 revision.getPatchSet()));
     while (mapItr.hasNext()) {
       Map.Entry<String, List<CommentInput>> ent = mapItr.next();

gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java

@@ -31,6 +31,7 @@ import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.account.AccountInfo;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -109,8 +110,11 @@ public class ReviewerJson {
     ChangeData cd = changeDataFactory.create(db.get(), ctl);
     PatchSet ps = cd.currentPatchSet();
     if (ps != null) {
-      for (SubmitRecord rec :
+      for (SubmitRecord rec : new SubmitRuleEvaluator(cd)
-          ctl.canSubmit(db.get(), ps, cd, true, false, true)) {
+          .setPatchSet(ps)
+          .setFastEvalLabels(true)
+          .setAllowDraft(true)
+          .canSubmit()) {
         if (rec.labels == null) {
           continue;
         }

gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java

@@ -60,6 +60,8 @@ import com.google.gerrit.server.git.VersionedMetaData.BatchMetaDataUpdate;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.AtomicUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -102,6 +104,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   private final Provider<ReviewDb> dbProvider;
   private final GitRepositoryManager repoManager;
   private final IdentifiedUser.GenericFactory userFactory;
+  private final ChangeData.Factory changeDataFactory;
   private final ChangeUpdate.Factory updateFactory;
   private final ApprovalsUtil approvalsUtil;
   private final ChangeMessagesUtil cmUtil;
@@ -118,6 +121,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
       Provider<ReviewDb> dbProvider,
       GitRepositoryManager repoManager,
       IdentifiedUser.GenericFactory userFactory,
+      ChangeData.Factory changeDataFactory,
       ChangeUpdate.Factory updateFactory,
       ApprovalsUtil approvalsUtil,
       ChangeMessagesUtil cmUtil,
@@ -131,6 +135,7 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
     this.dbProvider = dbProvider;
     this.repoManager = repoManager;
     this.userFactory = userFactory;
+    this.changeDataFactory = changeDataFactory;
     this.updateFactory = updateFactory;
     this.approvalsUtil = approvalsUtil;
     this.cmUtil = cmUtil;
@@ -374,10 +379,12 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
   }
 
   private List<SubmitRecord> checkSubmitRule(RevisionResource rsrc,
-      boolean force) throws ResourceConflictException {
+      boolean force) throws ResourceConflictException, OrmException {
-    List<SubmitRecord> results = rsrc.getControl().canSubmit(
+    ChangeData cd =
-        dbProvider.get(),
+        changeDataFactory.create(dbProvider.get(), rsrc.getControl());
-        rsrc.getPatchSet());
+    List<SubmitRecord> results = new SubmitRuleEvaluator(cd)
+        .setPatchSet(rsrc.getPatchSet())
+        .canSubmit();
     Optional<SubmitRecord> ok = findOkRecord(results);
     if (ok.isPresent()) {
       // Rules supplied a valid solution.

gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitRule.java

@@ -14,13 +14,7 @@
 
 package com.google.gerrit.server.change;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
-import com.google.common.base.Function;
-import com.google.common.base.Joiner;
 import com.google.common.base.MoreObjects;
-import com.google.common.base.Throwables;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.SubmitRecord;
@@ -32,18 +26,14 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.rules.RulesCache;
 import com.google.gerrit.server.account.AccountInfo;
 import com.google.gerrit.server.change.TestSubmitRule.Input;
-import com.google.gerrit.server.project.RuleEvalException;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-import com.googlecode.prolog_cafe.lang.Term;
-
 import org.kohsuke.args4j.Option;
 
-import java.io.ByteArrayInputStream;
 import java.util.List;
 import java.util.Map;
 
@@ -87,45 +77,14 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> {
       throw new AuthException("project rules are disabled");
     }
     input.filters = MoreObjects.firstNonNull(input.filters, filters);
-
     SubmitRuleEvaluator evaluator = new SubmitRuleEvaluator(
-        db.get(),
+        changeDataFactory.create(db.get(), rsrc.getControl()));
-        rsrc.getPatchSet(),
-        rsrc.getControl().getProjectControl(),
-        rsrc.getControl(),
-        rsrc.getChange(),
-        changeDataFactory.create(db.get(), rsrc.getChange()),
-        false,
-        "locate_submit_rule", "can_submit",
-        "locate_submit_filter", "filter_submit_results",
-        input.filters == Filters.SKIP,
-        input.rule != null
-          ? new ByteArrayInputStream(input.rule.getBytes(UTF_8))
-          : null);
 
-    List<Term> results;
+    List<SubmitRecord> records = evaluator.setPatchSet(rsrc.getPatchSet())
-    try {
+          .setLogErrors(false)
-      results = eval(evaluator);
+          .setSkipSubmitFilters(input.filters == Filters.SKIP)
-    } catch (RuleEvalException e) {
+          .setRule(input.rule)
-      String msg = Joiner.on(": ").skipNulls().join(Iterables.transform(
+          .canSubmit();
-          Throwables.getCausalChain(e),
-          new Function<Throwable, String>() {
-            @Override
-            public String apply(Throwable in) {
-              return in.getMessage();
-            }
-          }));
-      throw new BadRequestException("rule failed: " + msg);
-    }
-    if (results.isEmpty()) {
-      throw new BadRequestException(String.format(
-          "rule %s has no solutions",
-          evaluator.getSubmitRule().toString()));
-    }
-
-    List<SubmitRecord> records = rsrc.getControl().resultsToSubmitRecord(
-        evaluator.getSubmitRule(),
-        results);
     List<Record> out = Lists.newArrayListWithCapacity(records.size());
     AccountInfo.Loader accounts = accountInfoFactory.create(true);
     for (SubmitRecord r : records) {
@@ -135,11 +94,6 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, Input> {
     return out;
   }
 
-  private static List<Term> eval(SubmitRuleEvaluator evaluator)
-      throws RuleEvalException {
-    return evaluator.evaluate();
-  }
-
   static class Record {
     SubmitRecord.Status status;
     String errorMessage;

gerrit-server/src/main/java/com/google/gerrit/server/change/TestSubmitType.java

@@ -14,9 +14,8 @@
 
 package com.google.gerrit.server.change;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
-
 import com.google.common.base.MoreObjects;
+import com.google.gerrit.common.data.SubmitTypeRecord;
 import com.google.gerrit.extensions.common.SubmitType;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -26,20 +25,14 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.rules.RulesCache;
 import com.google.gerrit.server.change.TestSubmitRule.Filters;
 import com.google.gerrit.server.change.TestSubmitRule.Input;
-import com.google.gerrit.server.project.RuleEvalException;
 import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 
-import com.googlecode.prolog_cafe.lang.SymbolTerm;
-import com.googlecode.prolog_cafe.lang.Term;
-
 import org.kohsuke.args4j.Option;
 
-import java.io.ByteArrayInputStream;
-import java.util.List;
-
 public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
   private final Provider<ReviewDb> db;
   private final ChangeData.Factory changeDataFactory;
@@ -59,7 +52,7 @@ public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
 
   @Override
   public SubmitType apply(RevisionResource rsrc, Input input)
-      throws AuthException, BadRequestException {
+      throws AuthException, BadRequestException, OrmException {
     if (input == null) {
       input = new Input();
     }
@@ -67,52 +60,21 @@ public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
       throw new AuthException("project rules are disabled");
     }
     input.filters = MoreObjects.firstNonNull(input.filters, filters);
-
     SubmitRuleEvaluator evaluator = new SubmitRuleEvaluator(
-        db.get(),
+          changeDataFactory.create(db.get(), rsrc.getControl()));
-        rsrc.getPatchSet(),
-        rsrc.getControl().getProjectControl(),
-        rsrc.getControl(),
-        rsrc.getChange(),
-        changeDataFactory.create(db.get(), rsrc.getChange()),
-        false,
-        "locate_submit_type", "get_submit_type",
-        "locate_submit_type_filter", "filter_submit_type_results",
-        input.filters == Filters.SKIP,
-        input.rule != null
-          ? new ByteArrayInputStream(input.rule.getBytes(UTF_8))
-          : null);
 
-    List<Term> results;
+    SubmitTypeRecord rec = evaluator.setPatchSet(rsrc.getPatchSet())
-    try {
+        .setLogErrors(false)
-      results = evaluator.evaluate();
+        .setSkipSubmitFilters(input.filters == Filters.SKIP)
-    } catch (RuleEvalException e) {
+        .setRule(input.rule)
-      throw new BadRequestException(String.format(
+        .getSubmitType();
-          "rule failed with exception: %s",
+    if (rec.status != SubmitTypeRecord.Status.OK) {
-          e.getMessage()));
-    }
-    if (results.isEmpty()) {
-      throw new BadRequestException(String.format(
-          "rule %s has no solution",
-          evaluator.getSubmitRule()));
-    }
-    Term type = results.get(0);
-    if (!type.isSymbol()) {
       throw new BadRequestException(String.format(
           "rule %s produced invalid result: %s",
-          evaluator.getSubmitRule().toString(),
+          evaluator.getSubmitRule(), rec));
-          type));
     }
 
-    String typeName = ((SymbolTerm) type).name();
+    return rec.type;
-    try {
-      return SubmitType.valueOf(typeName.toUpperCase());
-    } catch (IllegalArgumentException e) {
-      throw new BadRequestException(String.format(
-          "rule %s produced invalid result: %s",
-          evaluator.getSubmitRule().toString(),
-          type));
-    }
   }
 
   static class Get implements RestReadView<RevisionResource> {
@@ -125,7 +87,7 @@ public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
 
     @Override
     public SubmitType apply(RevisionResource resource)
-        throws AuthException, BadRequestException {
+        throws AuthException, BadRequestException, OrmException {
       return test.apply(resource, null);
     }
   }

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

@@ -64,6 +64,8 @@ import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
+import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.util.RequestScopePropagator;
 import com.google.gwtorm.server.AtomicUpdate;
 import com.google.gwtorm.server.OrmException;
@@ -137,6 +139,7 @@ public class MergeOp {
   private final PatchSetInfoFactory patchSetInfoFactory;
   private final IdentifiedUser.GenericFactory identifiedUserFactory;
   private final ChangeControl.GenericFactory changeControlFactory;
+  private final ChangeData.Factory changeDataFactory;
   private final ChangeUpdate.Factory updateFactory;
   private final MergeQueue mergeQueue;
   private final MergeValidators.Factory mergeValidatorsFactory;
@@ -175,6 +178,7 @@ public class MergeOp {
       final MergeFailSender.Factory mfsf,
       final PatchSetInfoFactory psif, final IdentifiedUser.GenericFactory iuf,
       final ChangeControl.GenericFactory changeControlFactory,
+      final ChangeData.Factory changeDataFactory,
       final ChangeUpdate.Factory updateFactory,
       final MergeQueue mergeQueue, @Assisted final Branch.NameKey branch,
       final ChangeHooks hooks, final AccountCache accountCache,
@@ -197,6 +201,7 @@ public class MergeOp {
     patchSetInfoFactory = psif;
     identifiedUserFactory = iuf;
     this.changeControlFactory = changeControlFactory;
+    this.changeDataFactory = changeDataFactory;
     this.updateFactory = updateFactory;
     this.mergeQueue = mergeQueue;
     this.hooks = hooks;
@@ -557,7 +562,12 @@ public class MergeOp {
         }
       }
 
-      SubmitType submitType = getSubmitType(commit.getControl(), ps);
+      SubmitType submitType;
+      try {
+        submitType = getSubmitType(commit.getControl(), ps);
+      } catch (OrmException err) {
+        throw new MergeException("Cannot check submit type", err);
+      }
       if (submitType == null) {
         commit.setStatusCode(CommitMergeStatus.NO_SUBMIT_TYPE);
         toUpdate.add(chg);
@@ -571,13 +581,21 @@ public class MergeOp {
     return toSubmit;
   }
 
-  private SubmitType getSubmitType(ChangeControl ctl, PatchSet ps) {
+  private SubmitType getSubmitType(ChangeControl ctl, PatchSet ps)
-    SubmitTypeRecord r = ctl.getSubmitTypeRecord(db, ps);
+      throws OrmException {
-    if (r.status != SubmitTypeRecord.Status.OK) {
+    try {
-      log.error("Failed to get submit type for " + ctl.getChange().getKey());
+      ChangeData cd = changeDataFactory.create(db, ctl);
+      SubmitTypeRecord r = new SubmitRuleEvaluator(cd).setPatchSet(ps)
+          .getSubmitType();
+      if (r.status != SubmitTypeRecord.Status.OK) {
+        log.error("Failed to get submit type for " + ctl.getChange().getKey());
+        return null;
+      }
+      return r.type;
+    } catch (OrmException e) {
+      log.error("Failed to get submit type for " + ctl.getChange().getKey(), e);
       return null;
     }
-    return r.type;
   }
 
   private RefUpdate updateBranch(final SubmitStrategy strategy,

gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java

@@ -20,9 +20,6 @@ import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.RefConfigSection;
-import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.common.data.SubmitTypeRecord;
-import com.google.gerrit.extensions.common.SubmitType;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -39,28 +36,13 @@ import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 import com.google.inject.assistedinject.AssistedInject;
 
-import com.googlecode.prolog_cafe.lang.IntegerTerm;
-import com.googlecode.prolog_cafe.lang.ListTerm;
-import com.googlecode.prolog_cafe.lang.Prolog;
-import com.googlecode.prolog_cafe.lang.StructureTerm;
-import com.googlecode.prolog_cafe.lang.SymbolTerm;
-import com.googlecode.prolog_cafe.lang.Term;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.List;
 
 
 /** Access control management for a user accessing a single change. */
 public class ChangeControl {
-  private static final Logger log = LoggerFactory
-      .getLogger(ChangeControl.class);
-
   public static class GenericFactory {
     private final ProjectControl.GenericFactory projectControl;
     private final Provider<ReviewDb> db;
@@ -175,19 +157,6 @@ public class ChangeControl {
     ChangeControl create(RefControl refControl, ChangeNotes notes);
   }
 
-  /**
-   * Exception thrown when the label term of a submit record
-   * unexpectedly didn't contain a user term.
-   */
-  private static class UserTermExpected extends Exception {
-    private static final long serialVersionUID = 1L;
-
-    public UserTermExpected(SubmitRecord.Label label) {
-      super(String.format("A label with the status %s must contain a user.",
-          label.toString()));
-    }
-  }
-
   private final ChangeData.Factory changeDataFactory;
   private final RefControl refControl;
   private final ChangeNotes notes;
@@ -422,11 +391,6 @@ public class ChangeControl {
           || getRefControl().canEditHashtags(); // user can edit hashtag on a specific ref
   }
 
-
-  public List<SubmitRecord> getSubmitRecords(ReviewDb db, PatchSet patchSet) {
-    return canSubmit(db, patchSet, null, false, true, false);
-  }
-
   public boolean canSubmit() {
     return getRefControl().canSubmit();
   }
@@ -435,306 +399,17 @@ public class ChangeControl {
     return getRefControl().canSubmitAs();
   }
 
-  public List<SubmitRecord> canSubmit(ReviewDb db, PatchSet patchSet) {
-    return canSubmit(db, patchSet, null, false, false, false);
-  }
-
-  public List<SubmitRecord> canSubmit(ReviewDb db, PatchSet patchSet,
-      @Nullable ChangeData cd, boolean fastEvalLabels, boolean allowClosed,
-      boolean allowDraft) {
-    if (!allowClosed && getChange().getStatus().isClosed()) {
-      SubmitRecord rec = new SubmitRecord();
-      rec.status = SubmitRecord.Status.CLOSED;
-      return Collections.singletonList(rec);
-    }
-
-    if (!patchSet.getId().equals(getChange().currentPatchSetId())) {
-      return ruleError("Patch set " + patchSet.getPatchSetId() + " is not current");
-    }
-
-    cd = changeData(db, cd);
-    if ((getChange().getStatus() == Change.Status.DRAFT || patchSet.isDraft())
-        && !allowDraft) {
-      return cannotSubmitDraft(db, patchSet, cd);
-    }
-
-    List<Term> results;
-    SubmitRuleEvaluator evaluator;
-    try {
-      evaluator = new SubmitRuleEvaluator(db, patchSet,
-          getProjectControl(),
-          this, getChange(), cd,
-          fastEvalLabels,
-          "locate_submit_rule", "can_submit",
-          "locate_submit_filter", "filter_submit_results");
-      results = evaluator.evaluate();
-    } catch (RuleEvalException e) {
-      return logRuleError(e.getMessage(), e);
-    }
-
-    if (results.isEmpty()) {
-      // This should never occur. A well written submit rule will always produce
-      // at least one result informing the caller of the labels that are
-      // required for this change to be submittable. Each label will indicate
-      // whether or not that is actually possible given the permissions.
-      log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
-          + getChange().getId() + " of " + getProject().getName()
-          + " has no solution.");
-      return ruleError("Project submit rule has no solution");
-    }
-
-    return resultsToSubmitRecord(evaluator.getSubmitRule(), results);
-  }
-
   private boolean match(String destBranch, String refPattern) {
     return RefPatternMatcher.getMatcher(refPattern).match(destBranch,
         this.getRefControl().getCurrentUser().getUserName());
   }
 
-  private List<SubmitRecord> cannotSubmitDraft(ReviewDb db, PatchSet patchSet,
-      @Nullable ChangeData cd) {
-    try {
-      if (!isDraftVisible(db, cd)) {
-        return ruleError("Patch set " + patchSet.getPatchSetId() + " not found");
-      } else if (patchSet.isDraft()) {
-        return ruleError("Cannot submit draft patch sets");
-      } else {
-        return ruleError("Cannot submit draft changes");
-      }
-    } catch (OrmException err) {
-      return logRuleError("Cannot read patch set " + patchSet.getId(), err);
-    }
-  }
-
-  /**
-   * Convert the results from Prolog Cafe's format to Gerrit's common format.
-   *
-   * can_submit/1 terminates when an ok(P) record is found. Therefore walk
-   * the results backwards, using only that ok(P) record if it exists. This
-   * skips partial results that occur early in the output. Later after the loop
-   * the out collection is reversed to restore it to the original ordering.
-   */
-  public List<SubmitRecord> resultsToSubmitRecord(Term submitRule, List<Term> results) {
-    List<SubmitRecord> out = new ArrayList<>(results.size());
-    for (int resultIdx = results.size() - 1; 0 <= resultIdx; resultIdx--) {
-      Term submitRecord = results.get(resultIdx);
-      SubmitRecord rec = new SubmitRecord();
-      out.add(rec);
-
-      if (!submitRecord.isStructure() || 1 != submitRecord.arity()) {
-        return logInvalidResult(submitRule, submitRecord);
-      }
-
-      if ("ok".equals(submitRecord.name())) {
-        rec.status = SubmitRecord.Status.OK;
-
-      } else if ("not_ready".equals(submitRecord.name())) {
-        rec.status = SubmitRecord.Status.NOT_READY;
-
-      } else {
-        return logInvalidResult(submitRule, submitRecord);
-      }
-
-      // Unpack the one argument. This should also be a structure with one
-      // argument per label that needs to be reported on to the caller.
-      //
-      submitRecord = submitRecord.arg(0);
-
-      if (!submitRecord.isStructure()) {
-        return logInvalidResult(submitRule, submitRecord);
-      }
-
-      rec.labels = new ArrayList<>(submitRecord.arity());
-
-      for (Term state : ((StructureTerm) submitRecord).args()) {
-        if (!state.isStructure() || 2 != state.arity() || !"label".equals(state.name())) {
-          return logInvalidResult(submitRule, submitRecord);
-        }
-
-        SubmitRecord.Label lbl = new SubmitRecord.Label();
-        rec.labels.add(lbl);
-
-        lbl.label = state.arg(0).name();
-        Term status = state.arg(1);
-
-        try {
-          if ("ok".equals(status.name())) {
-            lbl.status = SubmitRecord.Label.Status.OK;
-            appliedBy(lbl, status);
-
-          } else if ("reject".equals(status.name())) {
-            lbl.status = SubmitRecord.Label.Status.REJECT;
-            appliedBy(lbl, status);
-
-          } else if ("need".equals(status.name())) {
-            lbl.status = SubmitRecord.Label.Status.NEED;
-
-          } else if ("may".equals(status.name())) {
-            lbl.status = SubmitRecord.Label.Status.MAY;
-
-          } else if ("impossible".equals(status.name())) {
-            lbl.status = SubmitRecord.Label.Status.IMPOSSIBLE;
-
-          } else {
-            return logInvalidResult(submitRule, submitRecord);
-          }
-        } catch (UserTermExpected e) {
-          return logInvalidResult(submitRule, submitRecord, e.getMessage());
-        }
-      }
-
-      if (rec.status == SubmitRecord.Status.OK) {
-        break;
-      }
-    }
-    Collections.reverse(out);
-
-    return out;
-  }
-
-  public SubmitTypeRecord getSubmitTypeRecord(ReviewDb db, PatchSet patchSet) {
-    return getSubmitTypeRecord(db, patchSet, null);
-  }
-
-  public SubmitTypeRecord getSubmitTypeRecord(ReviewDb db, PatchSet patchSet,
-      @Nullable ChangeData cd) {
-    cd = changeData(db, cd);
-    try {
-      if (getChange().getStatus() == Change.Status.DRAFT
-          && !isDraftVisible(db, cd)) {
-        return typeRuleError("Patch set " + patchSet.getPatchSetId()
-            + " not found");
-      }
-      if (patchSet.isDraft() && !isDraftVisible(db, cd)) {
-        return typeRuleError("Patch set " + patchSet.getPatchSetId()
-            + " not found");
-      }
-    } catch (OrmException err) {
-      return logTypeRuleError("Cannot read patch set " + patchSet.getId(),
-          err);
-    }
-
-    List<Term> results;
-    SubmitRuleEvaluator evaluator;
-    try {
-      evaluator = new SubmitRuleEvaluator(db, patchSet,
-          getProjectControl(), this, getChange(), cd,
-          false,
-          "locate_submit_type", "get_submit_type",
-          "locate_submit_type_filter", "filter_submit_type_results");
-      results = evaluator.evaluate();
-    } catch (RuleEvalException e) {
-      return logTypeRuleError(e.getMessage(), e);
-    }
-
-    if (results.isEmpty()) {
-      // Should never occur for a well written rule
-      log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
-          + getChange().getId() + " of " + getProject().getName()
-          + " has no solution.");
-      return typeRuleError("Project submit rule has no solution");
-    }
-
-    Term typeTerm = results.get(0);
-    if (!typeTerm.isSymbol()) {
-      log.error("Submit rule '" + evaluator.getSubmitRule() + "' for change "
-          + getChange().getId() + " of " + getProject().getName()
-          + " did not return a symbol.");
-      return typeRuleError("Project submit rule has invalid solution");
-    }
-
-    String typeName = ((SymbolTerm)typeTerm).name();
-    try {
-      return SubmitTypeRecord.OK(
-          SubmitType.valueOf(typeName.toUpperCase()));
-    } catch (IllegalArgumentException e) {
-      return logInvalidType(evaluator.getSubmitRule(), typeName);
-    }
-  }
-
-  private List<SubmitRecord> logInvalidResult(Term rule, Term record, String reason) {
-    return logRuleError("Submit rule " + rule + " for change " + getChange().getId()
-        + " of " + getProject().getName() + " output invalid result: " + record
-        + (reason == null ? "" : ". Reason: " + reason));
-  }
-
-  private List<SubmitRecord> logInvalidResult(Term rule, Term record) {
-    return logInvalidResult(rule, record, null);
-  }
-
-  private List<SubmitRecord> logRuleError(String err, Exception e) {
-    log.error(err, e);
-    return ruleError("Error evaluating project rules, check server log");
-  }
-
-  private List<SubmitRecord> logRuleError(String err) {
-    log.error(err);
-    return ruleError("Error evaluating project rules, check server log");
-  }
-
-  private List<SubmitRecord> ruleError(String err) {
-    SubmitRecord rec = new SubmitRecord();
-    rec.status = SubmitRecord.Status.RULE_ERROR;
-    rec.errorMessage = err;
-    return Collections.singletonList(rec);
-  }
-
-  private SubmitTypeRecord logInvalidType(Term rule, String record) {
-    return logTypeRuleError("Submit type rule " + rule + " for change "
-        + getChange().getId() + " of " + getProject().getName()
-        + " output invalid result: " + record);
-  }
-
-  private SubmitTypeRecord logTypeRuleError(String err, Exception e) {
-    log.error(err, e);
-    return typeRuleError("Error evaluating project type rules, check server log");
-  }
-
-  private SubmitTypeRecord logTypeRuleError(String err) {
-    log.error(err);
-    return typeRuleError("Error evaluating project type rules, check server log");
-  }
-
-  private SubmitTypeRecord typeRuleError(String err) {
-    SubmitTypeRecord rec = new SubmitTypeRecord();
-    rec.status = SubmitTypeRecord.Status.RULE_ERROR;
-    rec.errorMessage = err;
-    return rec;
-  }
-
   private ChangeData changeData(ReviewDb db, @Nullable ChangeData cd) {
     return cd != null ? cd : changeDataFactory.create(db, this);
   }
 
-  private void appliedBy(SubmitRecord.Label label, Term status)
+  public boolean isDraftVisible(ReviewDb db, ChangeData cd)
-      throws UserTermExpected {
-    if (status.isStructure() && status.arity() == 1) {
-      Term who = status.arg(0);
-      if (isUser(who)) {
-        label.appliedBy = new Account.Id(((IntegerTerm) who.arg(0)).intValue());
-      } else {
-        throw new UserTermExpected(label);
-      }
-    }
-  }
-
-  private boolean isDraftVisible(ReviewDb db, ChangeData cd)
       throws OrmException {
     return isOwner() || isReviewer(db, cd) || getRefControl().canViewDrafts();
   }
-
-  private static boolean isUser(Term who) {
-    return who.isStructure()
-        && who.arity() == 1
-        && who.name().equals("user")
-        && who.arg(0).isInteger();
-  }
-
-  public static Term toListTerm(List<Term> terms) {
-    Term list = Prolog.Nil;
-    for (int i = terms.size() - 1; i >= 0; i--) {
-      list = new ListTerm(terms.get(i), list);
-    }
-    return list;
-  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java

@@ -14,24 +14,38 @@
 
 package com.google.gerrit.server.project;
 
-import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkState;
+import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.collect.Lists;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.common.data.SubmitTypeRecord;
+import com.google.gerrit.extensions.common.SubmitType;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.rules.PrologEnvironment;
 import com.google.gerrit.rules.StoredValues;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
 
 import com.googlecode.prolog_cafe.compiler.CompileException;
+import com.googlecode.prolog_cafe.lang.IntegerTerm;
 import com.googlecode.prolog_cafe.lang.ListTerm;
 import com.googlecode.prolog_cafe.lang.Prolog;
 import com.googlecode.prolog_cafe.lang.PrologException;
+import com.googlecode.prolog_cafe.lang.StructureTerm;
+import com.googlecode.prolog_cafe.lang.SymbolTerm;
 import com.googlecode.prolog_cafe.lang.Term;
 import com.googlecode.prolog_cafe.lang.VariableTerm;
 
-import java.io.InputStream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -42,155 +56,467 @@ import java.util.List;
  * all the way up to All-Projects.
  */
 public class SubmitRuleEvaluator {
-  private final ReviewDb db;
+  private static final Logger log = LoggerFactory
-  private final PatchSet patchSet;
+      .getLogger(SubmitRuleEvaluator.class);
-  private final ProjectControl projectControl;
+
-  private final ChangeControl changeControl;
+  private static final String DEFAULT_MSG =
-  private final Change change;
+      "Error evaluating project rules, check server log";
+
+  public static List<SubmitRecord> defaultRuleError() {
+    return createRuleError(DEFAULT_MSG);
+  }
+
+  public static List<SubmitRecord> createRuleError(String err) {
+    SubmitRecord rec = new SubmitRecord();
+    rec.status = SubmitRecord.Status.RULE_ERROR;
+    rec.errorMessage = err;
+    return Collections.singletonList(rec);
+  }
+
+  public static SubmitTypeRecord defaultTypeError() {
+    return createTypeError(DEFAULT_MSG);
+  }
+
+  public static SubmitTypeRecord createTypeError(String err) {
+    SubmitTypeRecord rec = new SubmitTypeRecord();
+    rec.status = SubmitTypeRecord.Status.RULE_ERROR;
+    rec.errorMessage = err;
+    return rec;
+  }
+
+  /**
+   * Exception thrown when the label term of a submit record
+   * unexpectedly didn't contain a user term.
+   */
+  private static class UserTermExpected extends Exception {
+    private static final long serialVersionUID = 1L;
+
+    public UserTermExpected(SubmitRecord.Label label) {
+      super(String.format("A label with the status %s must contain a user.",
+          label.toString()));
+    }
+  }
+
   private final ChangeData cd;
-  private final boolean fastEvalLabels;
+  private final ChangeControl control;
-  private final String userRuleLocatorName;
+
-  private final String userRuleWrapperName;
+  private PatchSet patchSet;
-  private final String filterRuleLocatorName;
+  private boolean fastEvalLabels;
-  private final String filterRuleWrapperName;
+  private boolean allowDraft;
-  private final boolean skipFilters;
+  private boolean allowClosed;
-  private final InputStream rulesInputStream;
+  private boolean skipFilters;
+  private String rule;
+  private boolean logErrors = true;
 
   private Term submitRule;
-  private String projectName;
 
-  /**
+  public SubmitRuleEvaluator(ChangeData cd) throws OrmException {
-   * @param userRuleLocatorName The name of the rule used to locate the
+    this.cd = cd;
-   *        user-supplied rule.
+    this.control = cd.changeControl();
-   * @param userRuleWrapperName The name of the wrapper rule used to evaluate
-   *        the user-supplied rule.
-   * @param filterRuleLocatorName The name of the rule used to locate the filter
-   *        rule.
-   * @param filterRuleWrapperName The name of the rule used to evaluate the
-   *        filter rule.
-   */
-  public SubmitRuleEvaluator(ReviewDb db, PatchSet patchSet,
-      ProjectControl projectControl,
-      ChangeControl changeControl, Change change, ChangeData cd,
-      boolean fastEvalLabels,
-      String userRuleLocatorName, String userRuleWrapperName,
-      String filterRuleLocatorName, String filterRuleWrapperName) {
-    this(db, patchSet, projectControl, changeControl, change, cd,
-        fastEvalLabels, userRuleLocatorName, userRuleWrapperName,
-        filterRuleLocatorName, filterRuleWrapperName, false, null);
   }
 
   /**
-   * @param userRuleLocatorName The name of the rule used to locate the
+   * @param ps patch set of the change to evaluate. If not set, the current
-   *        user-supplied rule.
+   * patch set will be loaded from {@link #canSubmit()} or {@link
-   * @param userRuleWrapperName The name of the wrapper rule used to evaluate
+   * #getSubmitType}.
-   *        the user-supplied rule.
+   * @return this
-   * @param filterRuleLocatorName The name of the rule used to locate the filter
-   *        rule.
-   * @param filterRuleWrapperName The name of the rule used to evaluate the
-   *        filter rule.
-   * @param skipSubmitFilters if {@code true} submit filter will not be
-   *        applied
-   * @param rules when non-null the rules will be read from this input stream
-   *        instead of refs/meta/config:rules.pl file
    */
-  public SubmitRuleEvaluator(ReviewDb db, PatchSet patchSet,
+  public SubmitRuleEvaluator setPatchSet(PatchSet ps) {
-      ProjectControl projectControl,
+    checkArgument(ps.getId().getParentKey().equals(cd.getId()),
-      ChangeControl changeControl, Change change, ChangeData cd,
+        "Patch set %s does not match change %s", ps.getId(), cd.getId());
-      boolean fastEvalLabels,
+    patchSet = ps;
-      String userRuleLocatorName, String userRuleWrapperName,
+    return this;
-      String filterRuleLocatorName, String filterRuleWrapperName,
-      boolean skipSubmitFilters, InputStream rules) {
-    this.db = db;
-    this.patchSet = patchSet;
-    this.projectControl = projectControl;
-    this.changeControl = changeControl;
-    this.change = change;
-    this.cd = checkNotNull(cd, "ChangeData");
-    this.fastEvalLabels = fastEvalLabels;
-    this.userRuleLocatorName = userRuleLocatorName;
-    this.userRuleWrapperName = userRuleWrapperName;
-    this.filterRuleLocatorName = filterRuleLocatorName;
-    this.filterRuleWrapperName = filterRuleWrapperName;
-    this.skipFilters = skipSubmitFilters;
-    this.rulesInputStream = rules;
   }
 
   /**
-   * Evaluates the given rule and filters.
+   * @param fast if true, infer label information from rules rather than reading
-   *
+   *     from project config.
-   * Sets the {@link #submitRule} to the Term found by the
+   * @return this
-   * {@link #userRuleLocatorName}. This can be used when reporting error(s) on
-   * unexpected return value of this method.
-   *
-   * @return List of {@link Term} objects returned from the evaluated rules.
-   * @throws RuleEvalException
    */
-  public List<Term> evaluate() throws RuleEvalException {
+  public SubmitRuleEvaluator setFastEvalLabels(boolean fast) {
-    PrologEnvironment env = getPrologEnvironment();
+    fastEvalLabels = fast;
+    return this;
+  }
+
+  /**
+   * @param allow whether to allow {@link #canSubmit()} on closed changes.
+   * @return this
+   */
+  public SubmitRuleEvaluator setAllowClosed(boolean allow) {
+    allowClosed = allow;
+    return this;
+  }
+
+  /**
+   * @param allow whether to allow {@link #canSubmit()} on closed changes.
+   * @return this
+   */
+  public SubmitRuleEvaluator setAllowDraft(boolean allow) {
+    allowDraft = allow;
+    return this;
+  }
+
+  /**
+   * @param skip if true, submit filter will not be applied.
+   * @return this
+   */
+  public SubmitRuleEvaluator setSkipSubmitFilters(boolean skip) {
+    skipFilters = skip;
+    return this;
+  }
+
+  /**
+   * @param rule custom rule to use, or null to use refs/meta/config:rules.pl.
+   * @return this
+   */
+  public SubmitRuleEvaluator setRule(@Nullable String rule) {
+    this.rule = rule;
+    return this;
+  }
+
+  /**
+   * @param log whether to log error messages in addition to returning error
+   *     records. If true, error record messages will be less descriptive.
+   */
+  public SubmitRuleEvaluator setLogErrors(boolean log) {
+    logErrors = log;
+    return this;
+  }
+
+  /**
+   * Evaluate the submit rules.
+   *
+   * @return List of {@link SubmitRecord} objects returned from the evaluated
+   *     rules, including any errors.
+   */
+  public List<SubmitRecord> canSubmit() {
     try {
-      submitRule = env.once("gerrit", userRuleLocatorName, new VariableTerm());
+      initPatchSet();
+    } catch (OrmException e) {
+      return ruleError("Error looking up patch set "
+          + control.getChange().currentPatchSetId());
+    }
+    Change c = control.getChange();
+    if (!allowClosed && c.getStatus().isClosed()) {
+      SubmitRecord rec = new SubmitRecord();
+      rec.status = SubmitRecord.Status.CLOSED;
+      return Collections.singletonList(rec);
+    }
+    if ((c.getStatus() == Change.Status.DRAFT || patchSet.isDraft())
+        && !allowDraft) {
+      return cannotSubmitDraft();
+    }
+
+    List<Term> results;
+    try {
+      results = evaluateImpl("locate_submit_rule", "can_submit",
+          "locate_submit_filter", "filter_submit_results",
+          control.getCurrentUser());
+    } catch (RuleEvalException e) {
+      return ruleError(e.getMessage(), e);
+    }
+
+    if (results.isEmpty()) {
+      // This should never occur. A well written submit rule will always produce
+      // at least one result informing the caller of the labels that are
+      // required for this change to be submittable. Each label will indicate
+      // whether or not that is actually possible given the permissions.
+      return ruleError(String.format("Submit rule '%s' for change %s of %s has "
+            + "no solution.", getSubmitRule(), cd.getId(), getProjectName()));
+    }
+
+    return resultsToSubmitRecord(getSubmitRule(), results);
+  }
+
+  private List<SubmitRecord> cannotSubmitDraft() {
+    try {
+      if (!control.isDraftVisible(cd.db(), cd)) {
+        return createRuleError("Patch set " + patchSet.getId() + " not found");
+      } else if (patchSet.isDraft()) {
+        return createRuleError("Cannot submit draft patch sets");
+      } else {
+        return createRuleError("Cannot submit draft changes");
+      }
+    } catch (OrmException err) {
+      String msg = "Cannot check visibility of patch set " + patchSet.getId();
+      log.error(msg, err);
+      return createRuleError(msg);
+    }
+  }
+
+  /**
+   * Convert the results from Prolog Cafe's format to Gerrit's common format.
+   *
+   * can_submit/1 terminates when an ok(P) record is found. Therefore walk
+   * the results backwards, using only that ok(P) record if it exists. This
+   * skips partial results that occur early in the output. Later after the loop
+   * the out collection is reversed to restore it to the original ordering.
+   */
+  private List<SubmitRecord> resultsToSubmitRecord(
+      Term submitRule, List<Term> results) {
+    List<SubmitRecord> out = new ArrayList<>(results.size());
+    for (int resultIdx = results.size() - 1; 0 <= resultIdx; resultIdx--) {
+      Term submitRecord = results.get(resultIdx);
+      SubmitRecord rec = new SubmitRecord();
+      out.add(rec);
+
+      if (!submitRecord.isStructure() || 1 != submitRecord.arity()) {
+        return invalidResult(submitRule, submitRecord);
+      }
+
+      if ("ok".equals(submitRecord.name())) {
+        rec.status = SubmitRecord.Status.OK;
+
+      } else if ("not_ready".equals(submitRecord.name())) {
+        rec.status = SubmitRecord.Status.NOT_READY;
+
+      } else {
+        return invalidResult(submitRule, submitRecord);
+      }
+
+      // Unpack the one argument. This should also be a structure with one
+      // argument per label that needs to be reported on to the caller.
+      //
+      submitRecord = submitRecord.arg(0);
+
+      if (!submitRecord.isStructure()) {
+        return invalidResult(submitRule, submitRecord);
+      }
+
+      rec.labels = new ArrayList<>(submitRecord.arity());
+
+      for (Term state : ((StructureTerm) submitRecord).args()) {
+        if (!state.isStructure() || 2 != state.arity() || !"label".equals(state.name())) {
+          return invalidResult(submitRule, submitRecord);
+        }
+
+        SubmitRecord.Label lbl = new SubmitRecord.Label();
+        rec.labels.add(lbl);
+
+        lbl.label = state.arg(0).name();
+        Term status = state.arg(1);
+
+        try {
+          if ("ok".equals(status.name())) {
+            lbl.status = SubmitRecord.Label.Status.OK;
+            appliedBy(lbl, status);
+
+          } else if ("reject".equals(status.name())) {
+            lbl.status = SubmitRecord.Label.Status.REJECT;
+            appliedBy(lbl, status);
+
+          } else if ("need".equals(status.name())) {
+            lbl.status = SubmitRecord.Label.Status.NEED;
+
+          } else if ("may".equals(status.name())) {
+            lbl.status = SubmitRecord.Label.Status.MAY;
+
+          } else if ("impossible".equals(status.name())) {
+            lbl.status = SubmitRecord.Label.Status.IMPOSSIBLE;
+
+          } else {
+            return invalidResult(submitRule, submitRecord);
+          }
+        } catch (UserTermExpected e) {
+          return invalidResult(submitRule, submitRecord, e.getMessage());
+        }
+      }
+
+      if (rec.status == SubmitRecord.Status.OK) {
+        break;
+      }
+    }
+    Collections.reverse(out);
+
+    return out;
+  }
+
+  private List<SubmitRecord> invalidResult(Term rule, Term record, String reason) {
+    return ruleError(String.format("Submit rule %s for change %s of %s output "
+        + "invalid result: %s%s", rule, cd.getId(), getProjectName(), record,
+        (reason == null ? "" : ". Reason: " + reason)));
+  }
+
+  private List<SubmitRecord> invalidResult(Term rule, Term record) {
+    return invalidResult(rule, record, null);
+  }
+
+  private List<SubmitRecord> ruleError(String err) {
+    return ruleError(err, null);
+  }
+
+  private List<SubmitRecord> ruleError(String err, Exception e) {
+    if (logErrors) {
+      if (e == null) {
+        log.error(err);
+      } else {
+        log.error(err, e);
+      }
+      return defaultRuleError();
+    } else {
+      return createRuleError(err);
+    }
+  }
+
+  /**
+   * Evaluate the submit type rules to get the submit type.
+   *
+   * @return record from the evaluated rules.
+   */
+  public SubmitTypeRecord getSubmitType() {
+    try {
+      initPatchSet();
+    } catch (OrmException e) {
+      return typeError("Error looking up patch set "
+          + control.getChange().currentPatchSetId());
+    }
+
+    try {
+      if (control.getChange().getStatus() == Change.Status.DRAFT
+          && !control.isDraftVisible(cd.db(), cd)) {
+        return createTypeError("Patch set " + patchSet.getId() + " not found");
+      }
+      if (patchSet.isDraft() && !control.isDraftVisible(cd.db(), cd)) {
+        return createTypeError("Patch set " + patchSet.getId() + " not found");
+      }
+    } catch (OrmException err) {
+      String msg = "Cannot read patch set " + patchSet.getId();
+      log.error(msg, err);
+      return createTypeError(msg);
+    }
+
+    List<Term> results;
+    try {
+      results = evaluateImpl("locate_submit_type", "get_submit_type",
+          "locate_submit_type_filter", "filter_submit_type_results",
+          // Do not include current user in submit type evaluation. This is used
+          // for mergeability checks, which are stored persistently and so must
+          // have a consistent view of the submit type.
+          null);
+    } catch (RuleEvalException e) {
+      return typeError(e.getMessage(), e);
+    }
+
+    if (results.isEmpty()) {
+      // Should never occur for a well written rule
+      return typeError("Submit rule '" + getSubmitRule() + "' for change "
+          + cd.getId() + " of " + getProjectName() + " has no solution.");
+    }
+
+    Term typeTerm = results.get(0);
+    if (!typeTerm.isSymbol()) {
+      return typeError("Submit rule '" + getSubmitRule() + "' for change "
+          + cd.getId() + " of " + getProjectName()
+          + " did not return a symbol.");
+    }
+
+    String typeName = ((SymbolTerm) typeTerm).name();
+    try {
+      return SubmitTypeRecord.OK(
+          SubmitType.valueOf(typeName.toUpperCase()));
+    } catch (IllegalArgumentException e) {
+      return typeError("Submit type rule " + getSubmitRule() + " for change "
+          + cd.getId() + " of " + getProjectName() + " output invalid result: "
+          + typeName);
+    }
+  }
+
+  private SubmitTypeRecord typeError(String err) {
+    return typeError(err, null);
+  }
+
+  private SubmitTypeRecord typeError(String err, Exception e) {
+    if (logErrors) {
+      if (e == null) {
+        log.error(err);
+      } else {
+        log.error(err, e);
+      }
+      return defaultTypeError();
+    } else {
+      return createTypeError(err);
+    }
+  }
+
+  private List<Term> evaluateImpl(
+      String userRuleLocatorName,
+      String userRuleWrapperName,
+      String filterRuleLocatorName,
+      String filterRuleWrapperName,
+      CurrentUser user) throws RuleEvalException {
+    PrologEnvironment env = getPrologEnvironment(user);
+    try {
+      Term sr = env.once("gerrit", userRuleLocatorName, new VariableTerm());
       if (fastEvalLabels) {
         env.once("gerrit", "assume_range_from_label");
       }
 
       List<Term> results = new ArrayList<>();
       try {
-        for (Term[] template : env.all("gerrit", userRuleWrapperName,
+        for (Term[] template : env.all("gerrit", userRuleWrapperName, sr,
-            submitRule, new VariableTerm())) {
+              new VariableTerm())) {
           results.add(template[1]);
         }
-      } catch (PrologException err) {
-        throw new RuleEvalException("Exception calling " + submitRule
-            + " on change " + change.getId() + " of " + getProjectName(),
-            err);
       } catch (RuntimeException err) {
-        throw new RuleEvalException("Exception calling " + submitRule
+        throw new RuleEvalException("Exception calling " + sr
-            + " on change " + change.getId() + " of " + getProjectName(),
+            + " on change " + cd.getId() + " of " + getProjectName(),
             err);
       }
 
       Term resultsTerm = toListTerm(results);
       if (!skipFilters) {
-        resultsTerm = runSubmitFilters(resultsTerm, env);
+        resultsTerm = runSubmitFilters(
+            resultsTerm, env, filterRuleLocatorName, filterRuleWrapperName);
       }
+      List<Term> r;
       if (resultsTerm.isList()) {
-        List<Term> r = Lists.newArrayList();
+        r = Lists.newArrayList();
         for (Term t = resultsTerm; t.isList();) {
           ListTerm l = (ListTerm) t;
           r.add(l.car().dereference());
           t = l.cdr().dereference();
         }
-        return r;
+      } else {
+        r = Collections.emptyList();
       }
-      return Collections.emptyList();
+      submitRule = sr;
+      return r;
     } finally {
       env.close();
     }
   }
 
-  private PrologEnvironment getPrologEnvironment() throws RuleEvalException {
+  private PrologEnvironment getPrologEnvironment(CurrentUser user)
-    ProjectState projectState = projectControl.getProjectState();
+      throws RuleEvalException {
+    checkState(patchSet != null,
+        "getPrologEnvironment() called before initPatchSet()");
+    ProjectState projectState = control.getProjectControl().getProjectState();
     PrologEnvironment env;
     try {
-      if (rulesInputStream == null) {
+      if (rule == null) {
         env = projectState.newPrologEnvironment();
       } else {
-        env = projectState.newPrologEnvironment("stdin", rulesInputStream);
+        env = projectState.newPrologEnvironment(
+            "stdin", new ByteArrayInputStream(rule.getBytes(UTF_8)));
       }
     } catch (CompileException err) {
       throw new RuleEvalException("Cannot consult rules.pl for "
           + getProjectName(), err);
     }
-    env.set(StoredValues.REVIEW_DB, db);
+    env.set(StoredValues.REVIEW_DB, cd.db());
     env.set(StoredValues.CHANGE_DATA, cd);
     env.set(StoredValues.PATCH_SET, patchSet);
-    env.set(StoredValues.CHANGE_CONTROL, changeControl);
+    env.set(StoredValues.CHANGE_CONTROL, control);
+    if (user != null) {
+      env.set(StoredValues.CURRENT_USER, user);
+    }
     return env;
   }
 
-  private Term runSubmitFilters(Term results, PrologEnvironment env) throws RuleEvalException {
+  private Term runSubmitFilters(Term results, PrologEnvironment env,
-    ProjectState projectState = projectControl.getProjectState();
+      String filterRuleLocatorName, String filterRuleWrapperName)
+      throws RuleEvalException {
+    ProjectState projectState = control.getProjectControl().getProjectState();
     PrologEnvironment childEnv = env;
     for (ProjectState parentState : projectState.parents()) {
       PrologEnvironment parentEnv;
@@ -215,11 +541,11 @@ public class SubmitRuleEvaluator {
         results = template[2];
       } catch (PrologException err) {
         throw new RuleEvalException("Exception calling " + filterRule
-            + " on change " + change.getId() + " of "
+            + " on change " + cd.getId() + " of "
             + parentState.getProject().getName(), err);
       } catch (RuntimeException err) {
         throw new RuleEvalException("Exception calling " + filterRule
-            + " on change " + change.getId() + " of "
+            + " on change " + cd.getId() + " of "
             + parentState.getProject().getName(), err);
       }
       childEnv = parentEnv;
@@ -235,14 +561,37 @@ public class SubmitRuleEvaluator {
     return list;
   }
 
+  private void appliedBy(SubmitRecord.Label label, Term status)
+      throws UserTermExpected {
+    if (status.isStructure() && status.arity() == 1) {
+      Term who = status.arg(0);
+      if (isUser(who)) {
+        label.appliedBy = new Account.Id(((IntegerTerm) who.arg(0)).intValue());
+      } else {
+        throw new UserTermExpected(label);
+      }
+    }
+  }
+
+  private static boolean isUser(Term who) {
+    return who.isStructure()
+        && who.arity() == 1
+        && who.name().equals("user")
+        && who.arg(0).isInteger();
+  }
+
   public Term getSubmitRule() {
+    checkState(submitRule != null, "getSubmitRule() invalid before evaluation");
     return submitRule;
   }
 
-  private String getProjectName() {
+  private void initPatchSet() throws OrmException {
-    if (projectName == null) {
+    if (patchSet == null) {
-      projectName = projectControl.getProjectState().getProject().getName();
+      patchSet = cd.currentPatchSet();
     }
-    return projectName;
+  }
+
+  private String getProjectName() {
+    return control.getProjectControl().getProjectState().getProject().getName();
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java

@@ -270,6 +270,10 @@ public class ChangeData {
     notes = c.getNotes();
   }
 
+  public ReviewDb db() {
+    return db;
+  }
+
   public boolean isFromSource(ChangeDataSource s) {
     return s == returnedBySource;
   }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/ConflictsPredicate.java

@@ -23,10 +23,10 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.MergeException;
 import com.google.gerrit.server.git.strategy.SubmitStrategy;
-import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.OperatorPredicate;
 import com.google.gerrit.server.query.OrPredicate;
 import com.google.gerrit.server.query.Predicate;
@@ -148,18 +148,11 @@ class ConflictsPredicate extends OrPredicate<ChangeData> {
         }
 
         private SubmitType getSubmitType(Change change, ChangeData cd) throws OrmException {
-          try {
+          SubmitTypeRecord r = new SubmitRuleEvaluator(cd).getSubmitType();
-            final SubmitTypeRecord r =
+          if (r.status != SubmitTypeRecord.Status.OK) {
-                args.changeControlGenericFactory.controlFor(change,
-                    args.userFactory.create(change.getOwner()))
-                    .getSubmitTypeRecord(db.get(), cd.currentPatchSet(), cd);
-            if (r.status != SubmitTypeRecord.Status.OK) {
-              return null;
-            }
-            return r.type;
-          } catch (NoSuchChangeException e) {
             return null;
           }
+          return r.type;
         }
 
         private Set<RevCommit> getAlreadyAccepted(Repository repo, RevWalk rw,

gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java

@@ -20,9 +20,7 @@ import com.google.common.collect.Lists;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.GlobalCapability;
 import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.config.TrackingFooters;
 import com.google.gerrit.server.data.ChangeAttribute;
@@ -31,13 +29,13 @@ import com.google.gerrit.server.data.QueryStatsAttribute;
 import com.google.gerrit.server.events.EventFactory;
 import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.Predicate;
 import com.google.gerrit.server.query.QueryParseException;
 import com.google.gson.Gson;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
-import com.google.inject.Provider;
 
 import org.eclipse.jgit.util.io.DisabledOutputStream;
 import org.slf4j.Logger;
@@ -97,7 +95,6 @@ public class QueryProcessor {
   private final EventFactory eventFactory;
   private final ChangeQueryBuilder queryBuilder;
   private final ChangeQueryRewriter queryRewriter;
-  private final Provider<ReviewDb> db;
   private final TrackingFooters trackingFooters;
   private final CurrentUser user;
   private final int maxLimit;
@@ -124,12 +121,11 @@ public class QueryProcessor {
   @Inject
   QueryProcessor(EventFactory eventFactory,
       ChangeQueryBuilder.Factory queryBuilder, CurrentUser currentUser,
-      ChangeQueryRewriter queryRewriter, Provider<ReviewDb> db,
+      ChangeQueryRewriter queryRewriter,
       TrackingFooters trackingFooters) {
     this.eventFactory = eventFactory;
     this.queryBuilder = queryBuilder.create(currentUser);
     this.queryRewriter = queryRewriter;
-    this.db = db;
     this.trackingFooters = trackingFooters;
     this.user = currentUser;
     this.maxLimit = currentUser.getCapabilities()
@@ -325,11 +321,10 @@ public class QueryProcessor {
           }
 
           if (includeSubmitRecords) {
-            PatchSet.Id psId = d.change().currentPatchSetId();
+            eventFactory.addSubmitRecords(c, new SubmitRuleEvaluator(d)
-            PatchSet patchSet = db.get().patchSets().get(psId);
+                .setAllowClosed(true)
-            List<SubmitRecord> submitResult = cc.canSubmit( //
+                .setAllowDraft(true)
-                db.get(), patchSet, null, false, true, true);
+                .canSubmit());
-            eventFactory.addSubmitRecords(c, submitResult);
           }
 
           if (includeCommitMessage) {

gerrit-server/src/main/java/gerrit/PRED_current_user_1.java

@@ -20,7 +20,6 @@ import com.google.gerrit.server.AnonymousUser;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PeerDaemonUser;
-import com.google.gerrit.server.project.ChangeControl;
 
 import com.googlecode.prolog_cafe.lang.EvaluationException;
 import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -47,8 +46,11 @@ public class PRED_current_user_1 extends Predicate.P1 {
     engine.setB0();
     Term a1 = arg1.dereference();
 
-    ChangeControl cControl = StoredValues.CHANGE_CONTROL.get(engine);
+    CurrentUser curUser = StoredValues.CURRENT_USER.getOrNull(engine);
-    CurrentUser curUser = cControl.getCurrentUser();
+    if (curUser == null) {
+      throw new EvaluationException(
+          "Current user not available in this rule type");
+    }
     Term resultTerm;
 
     if (curUser.isIdentifiedUser()) {
@@ -67,4 +69,4 @@ public class PRED_current_user_1 extends Predicate.P1 {
     }
     return cont;
   }
-}
+}