SubmitRecord: Add a field with the rule name that created the record
We need this field since we're going to wrap the existing submit records
into submit requirements to be served with other submit requirements
from project.config.
We need this for a couple of reasons:
1) For submit records containing one or more labels, we want to create
submit requirement results per label of the form
`label:$name=$rule_name`. For submit records containing no labels, we
will create one submit requirement result whose name = rule name.
2) We want to identify the submit record created by the default submit
rule. We need to inspect the label function so that we can create a
proper blocking condition accordingly. For example, for a label with a
MAX_WITH_BLOCK function we want to create the blocking condition as
`label:$name=MAX -label:$name=MIN`.
We write the ruleName field in SubmitRequirementsEvaluator after
evaluating each submit rule. Rule name has the form of
'$plugin_name~$ruleClassName' for example 'gerrit~DefaultSubmitRule'. We
also update the storage of submit records in NoteDb to include the
rule name field.
Change-Id: I3f08f189dd154a36a84ebdd7aec6a553ce6a044c
diff --git a/java/com/google/gerrit/entities/SubmitRecord.java b/java/com/google/gerrit/entities/SubmitRecord.java
index 860997f..95ad9f8 100644
--- a/java/com/google/gerrit/entities/SubmitRecord.java
+++ b/java/com/google/gerrit/entities/SubmitRecord.java
@@ -68,6 +68,8 @@
}
}
+ // Name of the rule that created this submit record, formatted as '$pluginName~$ruleName'
+ public String ruleName;
public Status status;
public List<Label> labels;
public List<LegacySubmitRequirement> requirements;
@@ -158,6 +160,7 @@
*/
public SubmitRecord deepCopy() {
SubmitRecord copy = new SubmitRecord();
+ copy.ruleName = ruleName;
copy.status = status;
copy.errorMessage = errorMessage;
if (labels != null) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 349c106..b827fe1 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -947,12 +947,14 @@
@Deprecated Map<String, String> data;
}
+ String ruleName;
SubmitRecord.Status status;
List<StoredLabel> labels;
List<StoredRequirement> requirements;
String errorMessage;
public StoredSubmitRecord(SubmitRecord rec) {
+ this.ruleName = rec.ruleName;
this.status = rec.status;
this.errorMessage = rec.errorMessage;
if (rec.labels != null) {
@@ -984,6 +986,7 @@
public SubmitRecord toSubmitRecord() {
SubmitRecord rec = new SubmitRecord();
+ rec.ruleName = ruleName;
rec.status = status;
rec.errorMessage = errorMessage;
if (labels != null) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index e2ed7c0..a9182d6 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -162,7 +162,10 @@
* The computation of {@link ChangeField#LABEL} has changed: We added the non_uploader arg to the
* label field.
*/
- static final Schema<ChangeData> V66 = schema(V65, false);
+ @Deprecated static final Schema<ChangeData> V66 = schema(V65, false);
+
+ /** Updated submit records: store the rule name that created the submit record. */
+ static final Schema<ChangeData> V67 = schema(V66, false);
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
index 87ad646..f82349a 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesParser.java
@@ -917,6 +917,11 @@
}
} else {
checkFooter(rec != null, FOOTER_SUBMITTED_WITH, line);
+ if (line.startsWith("Rule-Name: ")) {
+ String ruleName = line.split(": ")[1];
+ rec.ruleName = ruleName;
+ continue;
+ }
SubmitRecord.Label label = new SubmitRecord.Label();
if (rec.labels == null) {
rec.labels = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 971e0a8..004ad40 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -730,7 +730,10 @@
msg.append(' ').append(sanitizeFooter(rec.errorMessage));
}
msg.append('\n');
-
+ if (rec.ruleName != null) {
+ addFooter(msg, FOOTER_SUBMITTED_WITH).append("Rule-Name: ").append(rec.ruleName);
+ msg.append('\n');
+ }
if (rec.labels != null) {
for (SubmitRecord.Label label : rec.labels) {
// Label names/values are safe to append without sanitizing.
@@ -745,7 +748,6 @@
msg.append('\n');
}
}
- // TODO(maximeg) We might want to list plugins that validated this submission.
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
index 6345cdb..cb8b686 100644
--- a/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRuleEvaluator.java
@@ -125,7 +125,17 @@
projectState.hasPrologRules()
? rule -> !(rule.get() instanceof DefaultSubmitRule)
: rule -> true)
- .map(c -> c.call(s -> s.evaluate(cd)))
+ .map(
+ c ->
+ c.call(
+ s -> {
+ Optional<SubmitRecord> evaluate = s.evaluate(cd);
+ if (evaluate.isPresent()) {
+ evaluate.get().ruleName =
+ c.getPluginName() + "~" + s.getClass().getSimpleName();
+ }
+ return evaluate;
+ }))
.filter(Optional::isPresent)
.map(Optional::get)
.collect(toImmutableList());
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
index bc9f50a5..636b71d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRuleIT.java
@@ -27,6 +27,7 @@
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.inject.Inject;
import java.util.List;
+import java.util.stream.Collectors;
import org.junit.Test;
public class SubmitRuleIT extends AbstractDaemonTest {
@@ -39,6 +40,11 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
List<SubmitRecord> recordsBeforeSubmission = submitRuleEvaluator.evaluate(r.getChange());
+ assertThat(
+ recordsBeforeSubmission.stream()
+ .map(record -> record.ruleName)
+ .collect(Collectors.toList()))
+ .containsExactly("gerrit~DefaultSubmitRule");
gApi.changes().id(r.getChangeId()).current().submit();
// Add a new label that blocks submission if not granted. In case we reevaluate the rules,
// this would show up as blocking submission.
@@ -57,6 +63,11 @@
PushOneCommit.Result r = createChange();
approve(r.getChangeId());
List<SubmitRecord> recordsBeforeSubmission = submitRuleEvaluator.evaluate(r.getChange());
+ assertThat(
+ recordsBeforeSubmission.stream()
+ .map(record -> record.ruleName)
+ .collect(Collectors.toList()))
+ .containsExactly("gerrit~DefaultSubmitRule");
gApi.changes().id(r.getChangeId()).current().submit();
// Add a new label that blocks submission if not granted. In case we reevaluate the rules,
// this would show up as blocking submission.
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index 66d9e4b5..d35b022 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -1035,6 +1035,8 @@
assertThatSerializedClass(SubmitRecord.class)
.hasFields(
ImmutableMap.of(
+ "ruleName",
+ new TypeLiteral<String>() {}.getType(),
"status",
SubmitRecord.Status.class,
"labels",
