Do not store legacy submit records as submit requirements upon merge
We have a submit operation (StoreSubmitRequirementsOp) that stores
submit requirements in NoteDb when the change is merged. This relies on
SubmitRequirementsEvaluator to compute submit requirements that are then
stored. If the "enable_legacy_submit_requirements" experiment feature is
enabled, SubmitRequirementsEvaluator also retrieves legacy submit
records and converts them to submit requirements. This was necessary so
that we can surface submit requirements to the users based on legacy
submit records.
We don't want these legacy "converted" submit records to
be stored as submit requirements in NoteDb when the change is being
merged, hence we are doing it in this change. When surfing submit
requirements for closed changes to users we will:
1) Retrieve stored submit requirements from NoteDb.
2) Retrieve stored legacy submit records from NoteDb and convert them to
submit requirements. We will implement this in a follow up change.
Change-Id: If4fa6aa4a07cfd1bedc89cc6834270fb590a919f
diff --git a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
index 57a3cd7..1a7d5af 100644
--- a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -50,7 +50,9 @@
// patchset to the user before it was merged.
ChangeData changeData = changeDataFactory.create(ctx.getProject(), ctx.getChange().getId());
ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
- update.putSubmitRequirementResults(evaluator.evaluateAllRequirements(changeData).values());
+ // We do not want to store submit requirements in NoteDb for legacy submit records
+ update.putSubmitRequirementResults(
+ evaluator.evaluateAllRequirements(changeData, /* includeLegacy= */ false).values());
return !changeData.submitRequirements().isEmpty();
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
index b3ac380..402bb51 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -26,8 +26,13 @@
/**
* Evaluate and return all submit requirement results for a change. Submit requirements are read
* from the project config of the project containing the change as well as parent projects.
+ *
+ * @param cd change data corresponding to a specific gerrit change
+ * @param includeLegacy if set to true, evaluate legacy {@link
+ * com.google.gerrit.entities.SubmitRecord}s and convert them to submit requirements.
*/
- Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(ChangeData cd);
+ Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ ChangeData cd, boolean includeLegacy);
/** Evaluate a single {@link SubmitRequirement} using change data. */
SubmitRequirementResult evaluateRequirement(SubmitRequirement sr, ChangeData cd);
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 0f91df2..151ee7b 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -75,11 +75,13 @@
}
@Override
- public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(ChangeData cd) {
+ public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(
+ ChangeData cd, boolean includeLegacy) {
Map<SubmitRequirement, SubmitRequirementResult> result = getRequirements(cd);
- if (experimentFeatures.isFeatureEnabled(
- ExperimentFeaturesConstants
- .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
+ if (includeLegacy
+ && experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
result.putAll(SubmitRequirementsAdapter.getLegacyRequirements(legacyEvaluator, cd));
}
return ImmutableMap.copyOf(result);
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index c551cd2..4086877 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -951,7 +951,8 @@
notes().getSubmitRequirementsResult().stream()
.collect(Collectors.toMap(r -> r.submitRequirement(), Function.identity()));
} else {
- submitRequirements = submitRequirementsEvaluator.evaluateAllRequirements(this);
+ submitRequirements =
+ submitRequirementsEvaluator.evaluateAllRequirements(this, /* includeLegacy= */ true);
}
}
return submitRequirements;
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 4c17800..6a0408a 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4582,11 +4582,8 @@
// 4. Merge the change. Submit requirements status is presented from NoteDb.
gApi.changes().id(changeId).current().submit();
change = gApi.changes().id(changeId).get();
- assertThat(change.submitRequirements).hasSize(2);
- assertSubmitRequirementStatus(
- change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ true);
- assertSubmitRequirementStatus(
- change.submitRequirements, "build-cop-override", Status.SATISFIED, /* isLegacy= */ true);
+ // Legacy submit records are not stored as submit requirements in NoteDb.
+ assertThat(change.submitRequirements).isEmpty();
}
@Test
