Add Java implementation of the label functions

Add unit tests for the labels functions.
Check if prolog rules are defined for this project or its parents, and
if not default to the added Java implementations of LabelFunctons.

Before this commit, the Prolog rules engine was always invoked to check
wether a change can be submitted or not, even if no prolog rules were
defined.
Doing so should also make it easier to extract Prolog as a plugin
without losing any of its currently offered features (label functions
and default rules implementation).

The LabelFunction code is inspired by Saša Živkov's change Iffe1567,
adjusted to live directly in the enum.

Change-Id: I5e18b0d494be3f0423bb533ed84a63ea4b8a31df

Documentation/config-labels.txt

@@ -217,9 +217,9 @@ enabled. To permit blocking submits, ensure a negative value is defined.
 
 * `AnyWithBlock`
 +
-The lowest possible negative value, if present, blocks a submit, Any
-other value enables a submit. To permit blocking submits, ensure
-that a negative value is defined.
+The label is not mandatory but the lowest possible negative value,
+if present, blocks a submit. To permit blocking submits, ensure that a
+negative value is defined.
 
 * `MaxNoBlock`
 +

java/com/google/gerrit/common/data/LabelFunction.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.common.data;
 
 import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -27,15 +28,15 @@ import java.util.Optional;
  * rules, in which case the choice of function in the project config is ignored.
  *
  * <p>Function semantics are documented in {@code config-labels.txt}, and actual behavior is
- * implemented in Prolog in {@code gerrit_common.pl}.
+ * implemented both in Prolog in {@code gerrit_common.pl} and in the {@link #check} method.
  */
 public enum LabelFunction {
-  MAX_WITH_BLOCK("MaxWithBlock", true),
-  ANY_WITH_BLOCK("AnyWithBlock", true),
-  MAX_NO_BLOCK("MaxNoBlock", false),
-  NO_BLOCK("NoBlock", false),
-  NO_OP("NoOp", false),
-  PATCH_SET_LOCK("PatchSetLock", false);
+  ANY_WITH_BLOCK("AnyWithBlock", true, false, false),
+  MAX_WITH_BLOCK("MaxWithBlock", true, true, true),
+  MAX_NO_BLOCK("MaxNoBlock", false, true, true),
+  NO_BLOCK("NoBlock"),
+  NO_OP("NoOp"),
+  PATCH_SET_LOCK("PatchSetLock");
 
   public static final Map<String, LabelFunction> ALL;
 
@@ -53,10 +54,18 @@ public enum LabelFunction {
 
   private final String name;
   private final boolean isBlock;
+  private final boolean isRequired;
+  private final boolean requiresMaxValue;
 
-  private LabelFunction(String name, boolean isBlock) {
+  LabelFunction(String name) {
+    this(name, false, false, false);
+  }
+
+  LabelFunction(String name, boolean isBlock, boolean isRequired, boolean requiresMaxValue) {
     this.name = name;
     this.isBlock = isBlock;
+    this.isRequired = isRequired;
+    this.requiresMaxValue = requiresMaxValue;
   }
 
   /** The function name as defined in documentation and {@code project.config}. */
@@ -68,4 +77,47 @@ public enum LabelFunction {
   public boolean isBlock() {
     return isBlock;
   }
+
+  /** Whether the label is a mandatory label, meaning absence of votes will prevent submission. */
+  public boolean isRequired() {
+    return isRequired;
+  }
+
+  /** Whether the label requires a vote with the maximum value to allow submission. */
+  public boolean isMaxValueRequired() {
+    return requiresMaxValue;
+  }
+
+  public SubmitRecord.Label check(LabelType labelType, Iterable<PatchSetApproval> approvals) {
+    SubmitRecord.Label submitRecordLabel = new SubmitRecord.Label();
+    submitRecordLabel.label = labelType.getName();
+
+    submitRecordLabel.status = SubmitRecord.Label.Status.MAY;
+    if (isRequired) {
+      submitRecordLabel.status = SubmitRecord.Label.Status.NEED;
+    }
+
+    for (PatchSetApproval a : approvals) {
+      if (a.getValue() == 0) {
+        continue;
+      }
+
+      if (isBlock && labelType.isMaxNegative(a)) {
+        submitRecordLabel.appliedBy = a.getAccountId();
+        submitRecordLabel.status = SubmitRecord.Label.Status.REJECT;
+        return submitRecordLabel;
+      }
+
+      if (labelType.isMaxPositive(a) || !requiresMaxValue) {
+        submitRecordLabel.appliedBy = a.getAccountId();
+
+        submitRecordLabel.status = SubmitRecord.Label.Status.MAY;
+        if (isRequired) {
+          submitRecordLabel.status = SubmitRecord.Label.Status.OK;
+        }
+      }
+    }
+
+    return submitRecordLabel;
+  }
 }

java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -72,6 +72,7 @@ import com.google.gerrit.server.project.SubmitRuleEvaluator;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gerrit.server.restapi.group.GroupModule;
+import com.google.gerrit.server.rules.DefaultSubmitRule;
 import com.google.gerrit.server.rules.PrologModule;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.inject.Inject;
@@ -172,6 +173,7 @@ public class BatchProgramModule extends FactoryModule {
     // Submit rule evaluator
     factory(SubmitRuleEvaluator.Factory.class);
     install(new PrologModule());
+    install(new DefaultSubmitRule.Module());
 
     bind(ChangeJson.Factory.class).toProvider(Providers.<ChangeJson.Factory>of(null));
     bind(EventUtil.class).toProvider(Providers.<EventUtil>of(null));

java/com/google/gerrit/server/config/GerritGlobalModule.java

@@ -175,6 +175,7 @@ import com.google.gerrit.server.query.change.ChangeQueryProcessor;
 import com.google.gerrit.server.query.change.ConflictsCacheImpl;
 import com.google.gerrit.server.restapi.config.ConfigRestModule;
 import com.google.gerrit.server.restapi.group.GroupModule;
+import com.google.gerrit.server.rules.DefaultSubmitRule;
 import com.google.gerrit.server.rules.PrologModule;
 import com.google.gerrit.server.rules.RulesCache;
 import com.google.gerrit.server.rules.SubmitRule;
@@ -246,6 +247,7 @@ public class GerritGlobalModule extends FactoryModule {
     install(new GroupModule());
     install(new NoteDbModule(cfg));
     install(new PrologModule());
+    install(new DefaultSubmitRule.Module());
     install(new ReceiveCommitsModule());
     install(new SshAddressesModule());
     install(ThreadLocalRequestContext.module());

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

@@ -67,6 +67,7 @@ import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Ref;
@@ -197,6 +198,24 @@ public class ProjectState {
     return capabilities;
   }
 
+  /**
+   * Returns true if the Prolog engine is expected to run for this project, that is if this project
+   * or a parent possesses a rules.pl file.
+   */
+  public boolean hasPrologRules() {
+    // We check if this project has a rules.pl file
+    if (getConfig().getRulesId() != null) {
+      return true;
+    }
+
+    // If not, we check the parents.
+    return parents()
+        .stream()
+        .map(ProjectState::getConfig)
+        .map(ProjectConfig::getRulesId)
+        .anyMatch(Objects::nonNull);
+  }
+
   /** @return Construct a new PrologEnvironment for the calling thread. */
   public PrologEnvironment newPrologEnvironment() throws CompileException {
     PrologMachineCopy pmc = rulesMachine;

java/com/google/gerrit/server/rules/DefaultSubmitRule.java

@@ -0,0 +1,132 @@
+// Copyright (C) 2018 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.rules;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.gerrit.common.data.LabelFunction;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+import com.google.gerrit.server.project.SubmitRuleOptions;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Java implementation of Gerrit's default pre-submit rules behavior: check if the labels have the
+ * correct values, according to the {@link LabelFunction} they are attached to.
+ *
+ * <p>As this behavior is also implemented by the Prolog rules system, we skip it if at least one
+ * project in the hierarchy has a {@code rules.pl} file.
+ */
+@Singleton
+public final class DefaultSubmitRule implements SubmitRule {
+  private static final Logger log = LoggerFactory.getLogger(DefaultSubmitRule.class);
+
+  public static class Module extends FactoryModule {
+    @Override
+    public void configure() {
+      bind(SubmitRule.class)
+          .annotatedWith(Exports.named("DefaultRules"))
+          .to(DefaultSubmitRule.class);
+    }
+  }
+
+  private final ProjectCache projectCache;
+
+  @Inject
+  DefaultSubmitRule(ProjectCache projectCache) {
+    this.projectCache = projectCache;
+  }
+
+  @Override
+  public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions options) {
+    ProjectState projectState = projectCache.get(cd.project());
+
+    // In case at least one project has a rules.pl file, we let Prolog handle it.
+    // The Prolog rules engine will also handle the labels for us.
+    if (projectState == null || projectState.hasPrologRules()) {
+      return Collections.emptyList();
+    }
+
+    SubmitRecord submitRecord = new SubmitRecord();
+    submitRecord.status = SubmitRecord.Status.OK;
+
+    List<LabelType> labelTypes;
+    List<PatchSetApproval> approvals;
+    try {
+      labelTypes = cd.getLabelTypes().getLabelTypes();
+      approvals = cd.currentApprovals();
+    } catch (OrmException e) {
+      log.error("Unable to fetch labels and approvals for change {}", cd.getId(), e);
+
+      submitRecord.errorMessage = "Unable to fetch labels and approvals for the change";
+      submitRecord.status = SubmitRecord.Status.RULE_ERROR;
+      return Collections.singletonList(submitRecord);
+    }
+
+    submitRecord.labels = new ArrayList<>(labelTypes.size());
+
+    for (LabelType t : labelTypes) {
+      LabelFunction labelFunction = t.getFunction();
+      if (labelFunction == null) {
+        log.error(
+            "Unable to find the LabelFunction for label {}, change {}", t.getName(), cd.getId());
+
+        submitRecord.errorMessage = "Unable to find the LabelFunction for label " + t.getName();
+        submitRecord.status = SubmitRecord.Status.RULE_ERROR;
+        return Collections.singletonList(submitRecord);
+      }
+
+      Collection<PatchSetApproval> approvalsForLabel = getApprovalsForLabel(approvals, t);
+      SubmitRecord.Label label = labelFunction.check(t, approvalsForLabel);
+      submitRecord.labels.add(label);
+
+      switch (label.status) {
+        case OK:
+        case MAY:
+          break;
+
+        case NEED:
+        case REJECT:
+        case IMPOSSIBLE:
+          submitRecord.status = SubmitRecord.Status.NOT_READY;
+          break;
+      }
+    }
+
+    return Collections.singletonList(submitRecord);
+  }
+
+  private static List<PatchSetApproval> getApprovalsForLabel(
+      List<PatchSetApproval> approvals, LabelType t) {
+    return approvals
+        .stream()
+        .filter(input -> input.getLabel().equals(t.getLabelId().get()))
+        .collect(toImmutableList());
+  }
+}

java/com/google/gerrit/server/rules/PrologRule.java

@@ -16,25 +16,34 @@ package com.google.gerrit.server.rules;
 
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.common.data.SubmitTypeRecord;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.SubmitRuleOptions;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.rules.PrologRuleEvaluator.Factory;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
 import java.util.Collection;
+import java.util.Collections;
 
 @Singleton
 public class PrologRule implements SubmitRule {
-
-  private final Factory factory;
+  private final PrologRuleEvaluator.Factory factory;
+  private final ProjectCache projectCache;
 
   @Inject
-  private PrologRule(PrologRuleEvaluator.Factory factory) {
+  private PrologRule(PrologRuleEvaluator.Factory factory, ProjectCache projectCache) {
     this.factory = factory;
+    this.projectCache = projectCache;
   }
 
   @Override
   public Collection<SubmitRecord> evaluate(ChangeData cd, SubmitRuleOptions opts) {
+    ProjectState projectState = projectCache.get(cd.project());
+    // We only want to run the prolog engine if we have at least one rules.pl file to use.
+    if (projectState == null || !projectState.hasPrologRules()) {
+      return Collections.emptyList();
+    }
+
     return getEvaluator(cd, opts).evaluate();
   }
 

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

@@ -231,6 +231,23 @@ public class CustomLabelIT extends AbstractDaemonTest {
     assertThat(q.blocking).isNull();
   }
 
+  @Test
+  public void customLabelMaxWithBlock_MaxVoteNegativeVoteBlock() throws Exception {
+    label.setFunction(MAX_WITH_BLOCK);
+    saveLabelConfig();
+    PushOneCommit.Result r = createChange();
+    revision(r).review(new ReviewInput().label(label.getName(), 1));
+    revision(r).review(new ReviewInput().label(label.getName(), -1));
+    ChangeInfo c = getWithLabels(r);
+    LabelInfo q = c.labels.get(label.getName());
+    assertThat(q.all).hasSize(1);
+    assertThat(q.approved).isNull();
+    assertThat(q.recommended).isNull();
+    assertThat(q.disliked).isNull();
+    assertThat(q.rejected).isNotNull();
+    assertThat(q.blocking).isTrue();
+  }
+
   @Test
   public void customLabel_DisallowPostSubmit() throws Exception {
     label.setFunction(NO_OP);

javatests/com/google/gerrit/common/data/LabelFunctionTest.java

@@ -0,0 +1,148 @@
+// Copyright (C) 2018 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.common.data;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.LabelId;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.PatchSet.Id;
+import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import java.sql.Date;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Test;
+
+public class LabelFunctionTest {
+  private static final String LABEL_NAME = "Verified";
+  private static final LabelId LABEL_ID = new LabelId(LABEL_NAME);
+  private static final Change.Id CHANGE_ID = new Change.Id(100);
+  private static final PatchSet.Id PS_ID = new PatchSet.Id(CHANGE_ID, 1);
+  private static final LabelType VERIFIED_LABEL = makeLabel();
+  private static final PatchSetApproval APPROVAL_2 = makeApproval(2);
+  private static final PatchSetApproval APPROVAL_1 = makeApproval(1);
+  private static final PatchSetApproval APPROVAL_0 = makeApproval(0);
+  private static final PatchSetApproval APPROVAL_M1 = makeApproval(-1);
+  private static final PatchSetApproval APPROVAL_M2 = makeApproval(-2);
+
+  @Test
+  public void checkLabelNameIsCorrect() {
+    for (LabelFunction function : LabelFunction.values()) {
+      SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, ImmutableList.of());
+      assertThat(myLabel.label).isEqualTo("Verified");
+    }
+  }
+
+  @Test
+  public void checkFunctionDoesNothing() {
+    checkNothingHappens(LabelFunction.NO_BLOCK);
+    checkNothingHappens(LabelFunction.NO_OP);
+    checkNothingHappens(LabelFunction.PATCH_SET_LOCK);
+    checkNothingHappens(LabelFunction.ANY_WITH_BLOCK);
+
+    checkLabelIsRequired(LabelFunction.MAX_WITH_BLOCK);
+    checkLabelIsRequired(LabelFunction.MAX_NO_BLOCK);
+  }
+
+  @Test
+  public void checkBlockWorks() {
+    checkBlockWorks(LabelFunction.ANY_WITH_BLOCK);
+    checkBlockWorks(LabelFunction.MAX_WITH_BLOCK);
+  }
+
+  @Test
+  public void checkMaxWorks() {
+    checkMaxIsEnforced(LabelFunction.MAX_NO_BLOCK);
+    checkMaxIsEnforced(LabelFunction.MAX_WITH_BLOCK);
+
+    checkMaxValidatesTheLabel(LabelFunction.MAX_NO_BLOCK);
+    checkMaxValidatesTheLabel(LabelFunction.MAX_WITH_BLOCK);
+  }
+
+  @Test
+  public void checkMaxNoBlockIgnoresMin() {
+    List<PatchSetApproval> approvals = ImmutableList.of(APPROVAL_M2, APPROVAL_2, APPROVAL_M2);
+
+    SubmitRecord.Label myLabel = LabelFunction.MAX_NO_BLOCK.check(VERIFIED_LABEL, approvals);
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK);
+    assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.getAccountId());
+  }
+
+  private static LabelType makeLabel() {
+    List<LabelValue> values = new ArrayList<>();
+    // The label text is irrelevant here, only the numerical value is used
+    values.add(new LabelValue((short) -2, "Great job, please fix compilation."));
+    values.add(new LabelValue((short) -1, "Really good, please make some minor changes."));
+    values.add(new LabelValue((short) 0, "No vote."));
+    values.add(new LabelValue((short) 1, "Closest thing perfection."));
+    values.add(new LabelValue((short) 2, "Perfect!"));
+    return new LabelType(LABEL_NAME, values);
+  }
+
+  private static PatchSetApproval makeApproval(int value) {
+    Account.Id accountId = new Account.Id(10000 + value);
+    PatchSetApproval.Key key = makeKey(PS_ID, accountId, LABEL_ID);
+    return new PatchSetApproval(key, (short) value, Date.from(Instant.now()));
+  }
+
+  private static PatchSetApproval.Key makeKey(Id psId, Account.Id accountId, LabelId labelId) {
+    return new PatchSetApproval.Key(psId, accountId, labelId);
+  }
+
+  private static void checkBlockWorks(LabelFunction function) {
+    List<PatchSetApproval> approvals = ImmutableList.of(APPROVAL_1, APPROVAL_M2, APPROVAL_2);
+
+    SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals);
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.REJECT);
+    assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_M2.getAccountId());
+  }
+
+  private static void checkNothingHappens(LabelFunction function) {
+    SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, ImmutableList.of());
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.MAY);
+    assertThat(myLabel.appliedBy).isNull();
+  }
+
+  private static void checkLabelIsRequired(LabelFunction function) {
+    SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, ImmutableList.of());
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.NEED);
+    assertThat(myLabel.appliedBy).isNull();
+  }
+
+  private static void checkMaxIsEnforced(LabelFunction function) {
+    List<PatchSetApproval> approvals = ImmutableList.of(APPROVAL_1, APPROVAL_0);
+
+    SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals);
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.NEED);
+  }
+
+  private static void checkMaxValidatesTheLabel(LabelFunction function) {
+    List<PatchSetApproval> approvals = ImmutableList.of(APPROVAL_1, APPROVAL_2, APPROVAL_M1);
+
+    SubmitRecord.Label myLabel = function.check(VERIFIED_LABEL, approvals);
+
+    assertThat(myLabel.status).isEqualTo(SubmitRecord.Label.Status.OK);
+    assertThat(myLabel.appliedBy).isEqualTo(APPROVAL_2.getAccountId());
+  }
+}