Merge "Allow plugins to define custom data in SubmitRequirement"

Documentation/json.txt

@@ -185,14 +185,16 @@ in order for the change to be submittable.
 
 [[requirement]]
 == requirement
-Information about a requirement (not met) in order to submit a change.
+Information about a requirement in order to submit a change.
 
-shortReason:: A short description of the requirement (a hint).
+fallbackText:: A human readable description of the requirement.
 
-fullReason:: A longer and descriptive message explaining what needs to
-be changed to meet the requirement.
+type:: Alphanumerical (plus hyphens or underscores) string to identify what the requirement is and
+why it was triggered. Can be seen as a class: requirements sharing the same type were created for a
+similar reason, and the data structure will follow one set of rules.
 
-label:: (Optional) The name of the linked label, if set by a pre-submit rule.
+data:: (Optional) Additional key-value data linked to this requirement. This is used in templates to
+render rich status messages.
 
 [[label]]
 == label

java/com/google/gerrit/common/BUILD

@@ -22,6 +22,7 @@ gwt_module(
         "//lib:guava",
         "//lib:gwtorm_client",
         "//lib:servlet-api-3_1",
+        "//lib/auto:auto-value",
         "//lib/jgit/org.eclipse.jgit:jgit",
         "//lib/log:api",
     ],
@@ -46,6 +47,7 @@ java_library(
         "//lib:gwtjsonrpc",
         "//lib:gwtorm",
         "//lib:servlet-api-3_1",
+        "//lib/auto:auto-value",
         "//lib/jgit/org.eclipse.jgit:jgit",
         "//lib/log:api",
     ],

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.common.data;
 
+import com.google.common.annotations.GwtIncompatible;
 import com.google.gerrit.reviewdb.client.Account;
 import java.util.Collection;
 import java.util.List;
@@ -60,7 +61,7 @@ public class SubmitRecord {
 
   public Status status;
   public List<Label> labels;
-  public List<SubmitRequirement> requirements;
+  @GwtIncompatible public List<SubmitRequirement> requirements;
   public String errorMessage;
 
   public static class Label {
@@ -131,6 +132,7 @@ public class SubmitRecord {
     }
   }
 
+  @GwtIncompatible
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
@@ -158,6 +160,7 @@ public class SubmitRecord {
     return sb.toString();
   }
 
+  @GwtIncompatible
   @Override
   public boolean equals(Object o) {
     if (o instanceof SubmitRecord) {
@@ -170,6 +173,7 @@ public class SubmitRecord {
     return false;
   }
 
+  @GwtIncompatible
   @Override
   public int hashCode() {
     return Objects.hash(status, labels, errorMessage, requirements);

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

@@ -14,67 +14,65 @@
 
 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;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.GwtIncompatible;
+import com.google.common.base.CharMatcher;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import java.util.Map;
 
 /** Describes a requirement to submit a change. */
-public final class SubmitRequirement {
-  private final String shortReason;
-  private final String fullReason;
-  @Nullable private final String label;
+@GwtIncompatible
+@AutoValue
+@AutoValue.CopyAnnotations
+public abstract class SubmitRequirement {
+  private static final CharMatcher TYPE_MATCHER =
+      CharMatcher.inRange('a', 'z')
+          .or(CharMatcher.inRange('A', 'Z'))
+          .or(CharMatcher.inRange('0', '9'))
+          .or(CharMatcher.anyOf("-_"));
 
-  public SubmitRequirement(String shortReason, String fullReason, @Nullable String label) {
-    this.shortReason = requireNonNull(shortReason);
-    this.fullReason = requireNonNull(fullReason);
-    this.label = label;
-  }
+  @AutoValue.Builder
+  public abstract static class Builder {
+    public abstract Builder setType(String value);
 
-  public String shortReason() {
-    return shortReason;
-  }
+    public abstract Builder setFallbackText(String value);
 
-  public String fullReason() {
-    return fullReason;
-  }
-
-  public Optional<String> label() {
-    return Optional.ofNullable(label);
-  }
-
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) {
-      return true;
+    public Builder setData(Map<String, String> value) {
+      return setData(ImmutableMap.copyOf(value));
     }
-    if (o instanceof SubmitRequirement) {
-      SubmitRequirement that = (SubmitRequirement) o;
-      return Objects.equals(shortReason, that.shortReason)
-          && Objects.equals(fullReason, that.fullReason)
-          && Objects.equals(label, that.label);
+
+    public Builder addCustomValue(String key, String value) {
+      dataBuilder().put(key, value);
+      return this;
     }
-    return false;
+
+    public SubmitRequirement build() {
+      SubmitRequirement requirement = autoBuild();
+      Preconditions.checkState(
+          validateType(requirement.type()),
+          "SubmitRequirement's type contains non alphanumerical symbols.");
+      return requirement;
+    }
+
+    abstract Builder setData(ImmutableMap<String, String> value);
+
+    abstract ImmutableMap.Builder<String, String> dataBuilder();
+
+    abstract SubmitRequirement autoBuild();
   }
 
-  @Override
-  public int hashCode() {
-    return Objects.hash(shortReason, fullReason, label);
+  public abstract String fallbackText();
+
+  public abstract String type();
+
+  public abstract ImmutableMap<String, String> data();
+
+  public static Builder builder() {
+    return new AutoValue_SubmitRequirement.Builder();
   }
 
-  @Override
-  public String toString() {
-    return "SubmitRequirement{"
-        + "shortReason='"
-        + shortReason
-        + '\''
-        + ", fullReason='"
-        + fullReason
-        + '\''
-        + ", label='"
-        + label
-        + '\''
-        + '}';
+  private static boolean validateType(String type) {
+    return TYPE_MATCHER.matchesAllOf(type);
   }
 }

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

@@ -14,12 +14,14 @@
 
 package com.google.gerrit.server.data;
 
+import java.util.Map;
+
 /**
  * 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;
+  public Map<String, String> data;
+  public String type;
+  public String fallbackText;
 }

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

@@ -261,9 +261,9 @@ public class EventFactory {
       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);
+        re.fallbackText = req.fallbackText();
+        re.type = req.type();
+        re.data = req.data();
         sa.requirements.add(re);
       }
     }

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

@@ -36,7 +36,6 @@ 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;
@@ -77,6 +76,7 @@ import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -657,9 +657,9 @@ public class ChangeField {
     }
 
     static class StoredRequirement {
-      String shortReason;
-      String fullReason;
-      @Nullable String label;
+      String fallbackText;
+      String type;
+      Map<String, String> data;
     }
 
     SubmitRecord.Status status;
@@ -684,9 +684,9 @@ public class ChangeField {
         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);
+          sr.type = requirement.type();
+          sr.fallbackText = requirement.fallbackText();
+          sr.data = requirement.data();
           this.requirements.add(sr);
         }
       }
@@ -708,10 +708,13 @@ public class ChangeField {
       }
       if (requirements != null) {
         rec.requirements = new ArrayList<>(requirements.size());
-        for (StoredRequirement requirement : requirements) {
+        for (StoredRequirement req : requirements) {
           SubmitRequirement sr =
-              new SubmitRequirement(
-                  requirement.shortReason, requirement.fullReason, requirement.label);
+              SubmitRequirement.builder()
+                  .setType(req.type)
+                  .setFallbackText(req.fallbackText)
+                  .setData(req.data)
+                  .build();
           rec.requirements.add(sr);
         }
       }

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

@@ -93,7 +93,7 @@ public class ChangeFieldTest extends GerritBaseTests {
   }
 
   @Test
-  public void storedSubmitRecordsWithRequirements() {
+  public void storedSubmitRecordsWithRequirement() {
     SubmitRecord r =
         record(
             SubmitRecord.Status.OK,
@@ -101,10 +101,27 @@ public class ChangeFieldTest extends GerritBaseTests {
             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);
+        SubmitRequirement.builder()
+            .setType("short_type")
+            .setFallbackText("Fallback text may contain special symbols like < > \\ / ; :")
+            .addCustomValue("custom_data", "my value")
+            .build();
+    r.requirements = Collections.singletonList(sr);
+
+    assertStoredRecordRoundTrip(r);
+  }
+
+  @Test
+  public void storedSubmitRequirementWithoutCustomData() {
+    SubmitRecord r =
+        record(
+            SubmitRecord.Status.OK,
+            label(SubmitRecord.Label.Status.MAY, "Label-1", null),
+            label(SubmitRecord.Label.Status.OK, "Label-2", 1));
+
+    // Doesn't have any custom data value
+    SubmitRequirement sr =
+        SubmitRequirement.builder().setFallbackText("short_type").setType("ci_status").build();
     r.requirements = Collections.singletonList(sr);
 
     assertStoredRecordRoundTrip(r);