Add a legacy flag to submit requirements results
This is to differentiate between results that are created from legacy
submit records from results created by evaluating submit requirements
that are stored in the project config.
Change-Id: Id00abed12a3ddc46bdcbe2732f78c2cae8e68c12
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index b376a74..f2271d9 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8198,6 +8198,10 @@
|`status`||
Status describing the result of evaluating the submit requirement. The status
is one of (`SATISFIED`, `UNSATISFIED`, `OVERRIDDEN`, `NOT_APPLICABLE`).
+|`is_legacy`||
+If true, this submit requirement result was created from a legacy
+link:#submit-record[SubmitRecord]. Otherwise, it was created by evaluating a
+submit requirement.
|`applicability_expression_result`|optional|
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the applicability expression. Not set if the
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
index e1d5f39..4f2f2f6 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -41,6 +41,12 @@
/** SHA-1 of the patchset commit ID for which the submit requirement was evaluated. */
public abstract ObjectId patchSetCommitId();
+ /**
+ * Whether this result was created from a legacy {@link SubmitRecord}, or by evaluating a {@link
+ * SubmitRequirement}.
+ */
+ public abstract boolean legacy();
+
@Memoized
public Status status() {
if (assertError(submittabilityExpressionResult())
@@ -109,6 +115,8 @@
public abstract Builder patchSetCommitId(ObjectId value);
+ public abstract Builder legacy(boolean value);
+
public abstract SubmitRequirementResult build();
}
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
index 685e81a..d17da0a 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
@@ -47,6 +47,9 @@
/** Overall result (status) of evaluating this submit requirement. */
public Status status;
+ /** Whether this result was created from a legacy submit record. */
+ public boolean isLegacy;
+
/** Result of evaluating the applicability expression. */
public SubmitRequirementExpressionInfo applicabilityExpressionResult;
diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java
index 43f1475..5efcf59 100644
--- a/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/java/com/google/gerrit/server/change/ChangeJson.java
@@ -402,6 +402,7 @@
submitRequirementExpressionToInfo(
req.submittabilityExpression(), result.submittabilityExpressionResult());
info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
+ info.isLegacy = result.legacy();
return info;
}
diff --git a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
index 416b850..9bf56d8 100644
--- a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
+++ b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
@@ -40,6 +40,7 @@
SubmitRequirementResultProto.Builder builder = SubmitRequirementResultProto.newBuilder();
builder
.setSubmitRequirement(SubmitRequirementSerializer.serialize(r.submitRequirement()))
+ .setLegacy(r.legacy())
.setCommit(ObjectIdConverter.create().toByteString(r.patchSetCommitId()));
if (r.applicabilityExpressionResult().isPresent()) {
builder.setApplicabilityExpressionResult(
@@ -60,6 +61,7 @@
public SubmitRequirementResult fromProto(SubmitRequirementResultProto proto) {
SubmitRequirementResult.Builder builder =
SubmitRequirementResult.builder()
+ .legacy(proto.getLegacy())
.patchSetCommitId(ObjectIdConverter.create().fromByteString(proto.getCommit()))
.submitRequirement(
SubmitRequirementSerializer.deserialize(proto.getSubmitRequirement()));
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java b/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
index a9ae4a1..f43e6b4 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsAdapter.java
@@ -58,6 +58,7 @@
.setAllowOverrideInChildProjects(labelType.isCanOverride());
result.add(
SubmitRequirementResult.builder()
+ .legacy(true)
.submitRequirement(req.build())
.submittabilityExpressionResult(
createExpressionResult(toExpression(atoms), mapStatus(label), atoms))
@@ -79,6 +80,7 @@
.build();
return ImmutableList.of(
SubmitRequirementResult.builder()
+ .legacy(true)
.submitRequirement(sr)
.submittabilityExpressionResult(
createExpressionResult(
@@ -99,6 +101,7 @@
.build();
result.add(
SubmitRequirementResult.builder()
+ .legacy(true)
.submitRequirement(sr)
.submittabilityExpressionResult(
createExpressionResult(
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 0f38fcc..36377e9 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -91,6 +91,7 @@
: Optional.empty();
return SubmitRequirementResult.builder()
+ .legacy(false)
.submitRequirement(sr)
.patchSetCommitId(cd.currentPatchSet().commitId())
.submittabilityExpressionResult(blockingResult)
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 5c5d127..7bc393f 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4048,12 +4048,14 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4073,19 +4075,22 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
// Requirement is satisfied because there are no votes
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", -1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
// Requirement is still satisfied because -1 is not the max negative value
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", -2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
// Requirement is now unsatisfied because -2 is the max negative value
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4104,12 +4109,14 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4135,15 +4142,19 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
- assertSubmitRequirementStatus(change.submitRequirements, "verified", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "verified", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(2);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
- assertSubmitRequirementStatus(change.submitRequirements, "verified", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "verified", Status.UNSATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4163,7 +4174,8 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.NOT_APPLICABLE);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.NOT_APPLICABLE, /* isLegacy= */ false);
}
@Test
@@ -4192,13 +4204,15 @@
String changeId = r.getChangeId();
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "build-cop-override", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.OVERRIDDEN);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.OVERRIDDEN, /* isLegacy= */ false);
}
@Test
@@ -4227,17 +4241,20 @@
String changeId = r.getChangeId();
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4255,12 +4272,14 @@
String changeId = r.getChangeId();
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4290,13 +4309,15 @@
String changeId = r.getChangeId();
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 1);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
// +1 was enough to fulfill the requirement: override in child project was ignored
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4316,7 +4337,8 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
revision.review(ReviewInput.approve());
@@ -4348,13 +4370,15 @@
ChangeInfo change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.UNSATISFIED, /* isLegacy= */ false);
voteLabel(changeId, "code-review", 2);
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
gApi.changes().id(changeId).current().submit();
@@ -4370,7 +4394,8 @@
// The new "verified" submit requirement is not returned, since this change is closed
change = gApi.changes().id(changeId).get();
assertThat(change.submitRequirements).hasSize(1);
- assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "code-review", Status.SATISFIED, /* isLegacy= */ false);
}
@Test
@@ -4397,7 +4422,10 @@
.get();
assertThat(changeInfos).hasSize(1);
assertSubmitRequirementStatus(
- changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ changeInfos.get(0).submitRequirements,
+ "code-review",
+ Status.SATISFIED,
+ /* isLegacy= */ false);
}
@Test
@@ -4425,7 +4453,10 @@
.get();
assertThat(changeInfos).hasSize(1);
assertSubmitRequirementStatus(
- changeInfos.get(0).submitRequirements, "code-review", Status.SATISFIED);
+ changeInfos.get(0).submitRequirements,
+ "code-review",
+ Status.SATISFIED,
+ /* isLegacy= */ false);
}
@Test
@@ -5077,9 +5108,12 @@
private void assertSubmitRequirementStatus(
Collection<SubmitRequirementResultInfo> results,
String requirementName,
- SubmitRequirementResultInfo.Status status) {
+ SubmitRequirementResultInfo.Status status,
+ boolean isLegacy) {
for (SubmitRequirementResultInfo result : results) {
- if (result.name.equals(requirementName) && result.status == status) {
+ if (result.name.equals(requirementName)
+ && result.status == status
+ && result.isLegacy == isLegacy) {
return;
}
}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
index d35b022..048e3fd 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java
@@ -684,6 +684,7 @@
.submitRequirementsResult(
ImmutableList.of(
SubmitRequirementResult.builder()
+ .legacy(true)
.patchSetCommitId(
ObjectId.fromString("26e50c7d315a33a13e5cc00902781fa876bc36cd"))
.submitRequirement(
@@ -713,6 +714,7 @@
newProtoBuilder()
.addSubmitRequirementResult(
SubmitRequirementResultProto.newBuilder()
+ .setLegacy(true)
.setCommit(
ObjectIdConverter.create()
.toByteString(
diff --git a/proto/cache.proto b/proto/cache.proto
index aa04555..4f4c838 100644
--- a/proto/cache.proto
+++ b/proto/cache.proto
@@ -483,7 +483,7 @@
}
// Serialized form of com.google.gerrit.entities.SubmitRequirementResult.
-// Next ID: 6
+// Next ID: 7
message SubmitRequirementResultProto {
SubmitRequirementProto submit_requirement = 1;
SubmitRequirementExpressionResultProto applicability_expression_result = 2;
@@ -492,6 +492,9 @@
// Patchset commit ID at which the submit requirements are evaluated.
bytes commit = 5;
+
+ // Whether this result was created from a legacy submit record.
+ bool legacy = 6;
}
// Serialized form of com.google.gerrit.entities.SubmitRequirementExpressionResult.
