Move conversion to SubmitTypeRecords into SubmitRuleEvaluator

Similar to TestSubmitRule/canSubmit, much logic was shared between
TestSubmitType and getSubmitType.

Change-Id: I212a4f225a741932474659bcf0133f086240fb7d

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

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.change;
 
 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;
@@ -24,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.util.List;
-
 public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
   private final Provider<ReviewDb> db;
   private final ChangeData.Factory changeDataFactory;
@@ -66,41 +61,20 @@ public class TestSubmitType implements RestModifyView<RevisionResource, Input> {
     }
     input.filters = MoreObjects.firstNonNull(input.filters, filters);
     SubmitRuleEvaluator evaluator = new SubmitRuleEvaluator(
-        changeDataFactory.create(db.get(), rsrc.getControl()));
+          changeDataFactory.create(db.get(), rsrc.getControl()));
 
-    List<Term> results;
-    try {
-      results = evaluator.setPatchSet(rsrc.getPatchSet())
-          .setSkipSubmitFilters(input.filters == Filters.SKIP)
-          .setRule(input.rule)
-          .evaluateSubmitType();
-    } catch (RuleEvalException e) {
-      throw new BadRequestException(String.format(
-          "rule failed with exception: %s",
-          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()) {
+    SubmitTypeRecord rec = evaluator.setPatchSet(rsrc.getPatchSet())
+        .setLogErrors(false)
+        .setSkipSubmitFilters(input.filters == Filters.SKIP)
+        .setRule(input.rule)
+        .getSubmitType();
+    if (rec.status != SubmitTypeRecord.Status.OK) {
       throw new BadRequestException(String.format(
           "rule %s produced invalid result: %s",
-          evaluator.getSubmitRule().toString(),
-          type));
+          evaluator.getSubmitRule(), rec));
     }
 
-    String typeName = ((SymbolTerm) type).name();
-    try {
-      return SubmitType.valueOf(typeName.toUpperCase());
-    } catch (IllegalArgumentException e) {
-      throw new BadRequestException(String.format(
-          "rule %s produced invalid result: %s",
-          evaluator.getSubmitRule().toString(),
-          type));
-    }
+    return rec.type;
   }
 
   static class Get implements RestReadView<RevisionResource> {

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

@@ -24,7 +24,6 @@ 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;
@@ -487,73 +486,26 @@ public class ChangeControl {
     try {
       if (getChange().getStatus() == Change.Status.DRAFT
           && !isDraftVisible(db, cd)) {
-        return typeRuleError("Patch set " + patchSet.getPatchSetId()
-            + " not found");
+        return SubmitRuleEvaluator.createTypeError(
+            "Patch set " + patchSet.getPatchSetId() + " not found");
       }
       if (patchSet.isDraft() && !isDraftVisible(db, cd)) {
-        return typeRuleError("Patch set " + patchSet.getPatchSetId()
-            + " not found");
+        return SubmitRuleEvaluator.createTypeError(
+            "Patch set " + patchSet.getPatchSetId() + " not found");
       }
     } catch (OrmException err) {
-      return logTypeRuleError("Cannot read patch set " + patchSet.getId(),
-          err);
+      String msg = "Cannot read patch set " + patchSet.getId();
+      log.error(msg, err);
+      return SubmitRuleEvaluator.createTypeError(msg);
     }
 
-    List<Term> results;
-    SubmitRuleEvaluator evaluator;
     try {
-      evaluator = new SubmitRuleEvaluator(cd);
-      results = evaluator.evaluateSubmitType();
-    } catch (OrmException | RuleEvalException e) {
-      return logTypeRuleError(e.getMessage(), e);
+      return new SubmitRuleEvaluator(cd).setPatchSet(patchSet)
+          .getSubmitType();
+    } catch (OrmException e) {
+      log.error(e.getMessage(), e);
+      return SubmitRuleEvaluator.defaultTypeError();
     }
-
-    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 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) {

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

@@ -21,6 +21,8 @@ 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.PatchSet;
 import com.google.gerrit.rules.PrologEnvironment;
@@ -34,6 +36,7 @@ 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;
 
@@ -54,8 +57,11 @@ public class SubmitRuleEvaluator {
   private static final Logger log = LoggerFactory
       .getLogger(SubmitRuleEvaluator.class);
 
+  private static final String DEFAULT_MSG =
+      "Error evaluating project rules, check server log";
+
   public static List<SubmitRecord> defaultRuleError() {
-    return createRuleError("Error evaluating project rules, check server log");
+    return createRuleError(DEFAULT_MSG);
   }
 
   public static List<SubmitRecord> createRuleError(String err) {
@@ -65,6 +71,17 @@ public class SubmitRuleEvaluator {
     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.
@@ -283,14 +300,58 @@ public class SubmitRuleEvaluator {
   }
 
   /**
-   * Evaluate the submit type rules.
+   * Evaluate the submit type rules to get the submit type.
    *
-   * @return List of {@link Term} objects returned from the evaluated rules.
-   * @throws RuleEvalException
+   * @return record from the evaluated rules.
    */
-  public List<Term> evaluateSubmitType() throws RuleEvalException {
-    return evaluateImpl("locate_submit_type", "get_submit_type",
+  public SubmitTypeRecord getSubmitType() {
+    List<Term> results;
+    try {
+      results = evaluateImpl("locate_submit_type", "get_submit_type",
           "locate_submit_type_filter", "filter_submit_type_results");
+    } 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(