Add SubmitRequirement to hold the presubmit conditions not met

Currently, labels are (mis-) used to communicate with the user regarding
the presubmit failure reason. This leads to weird situations where the
Gerrit interface tells "Needs labels: Not-Author-Review,
CI-Build-Failure" which are not labels (and don't look like labels).

The introduced class and variable, SubmitRequirement, contains two
fields describing the issue and an optional label. This allows to
implement the two former examples: the CI doesn't need a label, but the
Non-Author review condition might be related to a Verified label, for
instance.

Change-Id: I83ab3fe52cc689416a26f9d4f91ebafb7337d428

Documentation/json.txt

@@ -178,6 +178,21 @@ status:: Current submit status.
 labels:: This describes the state of each code review
 <<label,label attribute>>, unless the status is RULE_ERROR.
 
+requirements:: Each <<requirement>> describes what needs to be changed
+in order for the change to be submittable.
+
+
+[[requirement]]
+== requirement
+Information about a requirement (not met) in order to submit a change.
+
+shortReason:: A short description of the requirement (a hint).
+
+fullReason:: A longer and descriptive message explaining what needs to
+be changed to meet the requirement.
+
+label:: (Optional) The name of the linked label, if set by a pre-submit rule.
+
 [[label]]
 == label
 Information about a code review label for a change.

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

@@ -20,7 +20,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
-/** Describes the state required to submit a change. */
+/** Describes the state and edits required to submit a change. */
 public class SubmitRecord {
   public static Optional<SubmitRecord> findOkRecord(Collection<SubmitRecord> in) {
     if (in == null) {
@@ -36,7 +36,7 @@ public class SubmitRecord {
     /** The change is ready for submission. */
     OK,
 
-    /** The change is missing a required label. */
+    /** Something is preventing this change from being submitted. */
     NOT_READY,
 
     /** The change has been closed. */
@@ -55,6 +55,7 @@ public class SubmitRecord {
 
   public Status status;
   public List<Label> labels;
+  public List<SubmitRequirement> requirements;
   public String errorMessage;
 
   public static class Label {
@@ -140,6 +141,14 @@ public class SubmitRecord {
         delimiter = ", ";
       }
     }
+    sb.append("],[");
+    if (requirements != null) {
+      String delimiter = "";
+      for (SubmitRequirement requirement : requirements) {
+        sb.append(delimiter).append(requirement);
+        delimiter = ", ";
+      }
+    }
     sb.append(']');
     return sb.toString();
   }
@@ -150,13 +159,14 @@ public class SubmitRecord {
       SubmitRecord r = (SubmitRecord) o;
       return Objects.equals(status, r.status)
           && Objects.equals(labels, r.labels)
-          && Objects.equals(errorMessage, r.errorMessage);
+          && Objects.equals(errorMessage, r.errorMessage)
+          && Objects.equals(requirements, r.requirements);
     }
     return false;
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(status, labels, errorMessage);
+    return Objects.hash(status, labels, errorMessage, requirements);
   }
 }

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

@@ -0,0 +1,80 @@
+// 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 java.util.Objects.requireNonNull;
+
+import com.google.gerrit.common.Nullable;
+import java.util.Objects;
+import java.util.Optional;
+
+/** Describes a requirement to submit a change. */
+public final class SubmitRequirement {
+  private final String shortReason;
+  private final String fullReason;
+  @Nullable private final String label;
+
+  public SubmitRequirement(String shortReason, String fullReason, @Nullable String label) {
+    this.shortReason = requireNonNull(shortReason);
+    this.fullReason = requireNonNull(fullReason);
+    this.label = label;
+  }
+
+  public String shortReason() {
+    return shortReason;
+  }
+
+  public String fullReason() {
+    return fullReason;
+  }
+
+  public Optional<String> label() {
+    return Optional.ofNullable(label);
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o instanceof SubmitRequirement) {
+      SubmitRequirement that = (SubmitRequirement) o;
+      return Objects.equals(shortReason, that.shortReason)
+          && Objects.equals(fullReason, that.fullReason)
+          && Objects.equals(label, that.label);
+    }
+    return false;
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(shortReason, fullReason, label);
+  }
+
+  @Override
+  public String toString() {
+    return "SubmitRequirement{"
+        + "shortReason='"
+        + shortReason
+        + '\''
+        + ", fullReason='"
+        + fullReason
+        + '\''
+        + ", label='"
+        + label
+        + '\''
+        + '}';
+  }
+}

java/com/google/gerrit/server/data/SubmitLabelAttribute.java

@@ -14,6 +14,10 @@
 
 package com.google.gerrit.server.data;
 
+/**
+ * Represents a {@link com.google.gerrit.common.data.SubmitRecord.Label} that does not depend on
+ * Gerrit internal classes, to be serialized.
+ */
 public class SubmitLabelAttribute {
   public String label;
   public String status;

java/com/google/gerrit/server/data/SubmitRecordAttribute.java

@@ -16,7 +16,12 @@ package com.google.gerrit.server.data;
 
 import java.util.List;
 
+/**
+ * Represents a {@link com.google.gerrit.common.data.SubmitRecord} that does not depend on Gerrit
+ * internal classes, to be serialized.
+ */
 public class SubmitRecordAttribute {
   public String status;
   public List<SubmitLabelAttribute> labels;
+  public List<SubmitRequirementAttribute> requirements;
 }

java/com/google/gerrit/server/data/SubmitRequirementAttribute.java

@@ -0,0 +1,25 @@
+// 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.data;
+
+/**
+ * Represents a {@link com.google.gerrit.common.data.SubmitRequirement} that does not depend on
+ * Gerrit internal classes, to be serialized
+ */
+public class SubmitRequirementAttribute {
+  public String shortReason;
+  public String fullReason;
+  public String label;
+}

java/com/google/gerrit/server/events/EventFactory.java

@@ -23,6 +23,7 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.common.data.SubmitRequirement;
 import com.google.gerrit.index.IndexConfig;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
@@ -52,6 +53,7 @@ import com.google.gerrit.server.data.PatchSetCommentAttribute;
 import com.google.gerrit.server.data.RefUpdateAttribute;
 import com.google.gerrit.server.data.SubmitLabelAttribute;
 import com.google.gerrit.server.data.SubmitRecordAttribute;
+import com.google.gerrit.server.data.SubmitRequirementAttribute;
 import com.google.gerrit.server.data.TrackingIdAttribute;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.patch.PatchList;
@@ -229,6 +231,7 @@ public class EventFactory {
       sa.status = submitRecord.status.name();
       if (submitRecord.status != SubmitRecord.Status.RULE_ERROR) {
         addSubmitRecordLabels(submitRecord, sa);
+        addSubmitRecordRequirements(submitRecord, sa);
       }
       ca.submitRecords.add(sa);
     }
@@ -253,6 +256,19 @@ public class EventFactory {
     }
   }
 
+  private void addSubmitRecordRequirements(SubmitRecord submitRecord, SubmitRecordAttribute sa) {
+    if (submitRecord.requirements != null && !submitRecord.requirements.isEmpty()) {
+      sa.requirements = new ArrayList<>();
+      for (SubmitRequirement req : submitRecord.requirements) {
+        SubmitRequirementAttribute re = new SubmitRequirementAttribute();
+        re.shortReason = req.shortReason();
+        re.fullReason = req.fullReason();
+        re.label = req.label().orElse(null);
+        sa.requirements.add(re);
+      }
+    }
+  }
+
   public void addDependencies(RevWalk rw, ChangeAttribute ca, Change change, PatchSet currentPs) {
     if (change == null || currentPs == null) {
       return;

java/com/google/gerrit/server/index/change/ChangeField.java

@@ -36,7 +36,9 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Table;
 import com.google.common.primitives.Longs;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.common.data.SubmitRequirement;
 import com.google.gerrit.index.FieldDef;
 import com.google.gerrit.index.SchemaUtil;
 import com.google.gerrit.reviewdb.client.Account;
@@ -657,8 +659,15 @@ public class ChangeField {
       Integer appliedBy;
     }
 
+    static class StoredRequirement {
+      String shortReason;
+      String fullReason;
+      @Nullable String label;
+    }
+
     SubmitRecord.Status status;
     List<StoredLabel> labels;
+    List<StoredRequirement> requirements;
     String errorMessage;
 
     StoredSubmitRecord(SubmitRecord rec) {
@@ -674,6 +683,16 @@ public class ChangeField {
           this.labels.add(sl);
         }
       }
+      if (rec.requirements != null) {
+        this.requirements = new ArrayList<>(rec.requirements.size());
+        for (SubmitRequirement requirement : rec.requirements) {
+          StoredRequirement sr = new StoredRequirement();
+          sr.shortReason = requirement.shortReason();
+          sr.fullReason = requirement.fullReason();
+          sr.label = requirement.label().orElse(null);
+          this.requirements.add(sr);
+        }
+      }
     }
 
     private SubmitRecord toSubmitRecord() {
@@ -690,6 +709,15 @@ public class ChangeField {
           rec.labels.add(srl);
         }
       }
+      if (requirements != null) {
+        rec.requirements = new ArrayList<>(requirements.size());
+        for (StoredRequirement requirement : requirements) {
+          SubmitRequirement sr =
+              new SubmitRequirement(
+                  requirement.shortReason, requirement.fullReason, requirement.label);
+          rec.requirements.add(sr);
+        }
+      }
       return rec;
     }
   }

javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java

@@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Table;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.common.data.SubmitRequirement;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.ReviewerSet;
@@ -30,6 +31,7 @@ import com.google.gerrit.server.notedb.ReviewerStateInternal;
 import com.google.gerrit.testing.GerritBaseTests;
 import com.google.gerrit.testing.TestTimeUtil;
 import java.sql.Timestamp;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import org.junit.After;
@@ -80,11 +82,32 @@ public class ChangeFieldTest extends GerritBaseTests {
   @Test
   public void storedSubmitRecords() {
     assertStoredRecordRoundTrip(record(SubmitRecord.Status.CLOSED));
-    assertStoredRecordRoundTrip(
+
+    SubmitRecord r =
         record(
             SubmitRecord.Status.OK,
             label(SubmitRecord.Label.Status.MAY, "Label-1", null),
-            label(SubmitRecord.Label.Status.OK, "Label-2", 1)));
+            label(SubmitRecord.Label.Status.OK, "Label-2", 1));
+
+    assertStoredRecordRoundTrip(r);
+  }
+
+  @Test
+  public void storedSubmitRecordsWithRequirements() {
+    SubmitRecord r =
+        record(
+            SubmitRecord.Status.OK,
+            label(SubmitRecord.Label.Status.MAY, "Label-1", null),
+            label(SubmitRecord.Label.Status.OK, "Label-2", 1));
+
+    SubmitRequirement sr =
+        new SubmitRequirement(
+            "short reason",
+            "Full reason can be a long string with special symbols like < > \\ / ; :",
+            null);
+    r.requirements = Collections.singletonList(sr);
+
+    assertStoredRecordRoundTrip(r);
   }
 
   private static SubmitRecord record(SubmitRecord.Status status, SubmitRecord.Label... labels) {