Store submit requirements in NoteDb when the change is abandoned
We already store submit requirements in NoteDb when the change is merged
(implemented in change I2a63f725). In this change we do the same for
abandoned changes. We store SR results in NoteDb upon merge/abandon
because project's SRs might change in the future; for example project
admin might add or delete SRs. We want to present to the user the SR
results that applied to the latest patchset of the change when it was
merged.
Storage for both merged/abandoned changes is currently gated with the
"store_submit_requirements_on_merge" experiment. We will remove this
experiment soon before SRs are launched.
Bug: Google b/204286761
Change-Id: I7485485f0b8bf94eeb8b96ff28505e6cda9508bb
diff --git a/java/com/google/gerrit/server/change/BatchAbandon.java b/java/com/google/gerrit/server/change/BatchAbandon.java
index e0a72ac4..9a3c388 100644
--- a/java/com/google/gerrit/server/change/BatchAbandon.java
+++ b/java/com/google/gerrit/server/change/BatchAbandon.java
@@ -20,6 +20,7 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.config.ChangeCleanupConfig;
+import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
import com.google.gerrit.server.plugincontext.PluginItemContext;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdate;
@@ -34,15 +35,18 @@
private final AbandonOp.Factory abandonOpFactory;
private final ChangeCleanupConfig cfg;
private final PluginItemContext<AccountPatchReviewStore> accountPatchReviewStore;
+ private final StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory;
@Inject
BatchAbandon(
AbandonOp.Factory abandonOpFactory,
ChangeCleanupConfig cfg,
- PluginItemContext<AccountPatchReviewStore> accountPatchReviewStore) {
+ PluginItemContext<AccountPatchReviewStore> accountPatchReviewStore,
+ StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory) {
this.abandonOpFactory = abandonOpFactory;
this.cfg = cfg;
this.accountPatchReviewStore = accountPatchReviewStore;
+ this.storeSubmitRequirementsOpFactory = storeSubmitRequirementsOpFactory;
}
/**
@@ -74,6 +78,7 @@
change.project().get(), project.get()));
}
u.addOp(change.getId(), abandonOpFactory.create(accountState, msgTxt));
+ u.addOp(change.getId(), storeSubmitRequirementsOpFactory.create());
}
u.execute();
diff --git a/java/com/google/gerrit/server/restapi/change/Abandon.java b/java/com/google/gerrit/server/restapi/change/Abandon.java
index 2cfc3f5..affe947 100644
--- a/java/com/google/gerrit/server/restapi/change/Abandon.java
+++ b/java/com/google/gerrit/server/restapi/change/Abandon.java
@@ -30,6 +30,7 @@
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.update.BatchUpdate;
@@ -48,6 +49,7 @@
private final AbandonOp.Factory abandonOpFactory;
private final NotifyResolver notifyResolver;
private final PatchSetUtil patchSetUtil;
+ private final StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory;
@Inject
Abandon(
@@ -55,12 +57,14 @@
ChangeJson.Factory json,
AbandonOp.Factory abandonOpFactory,
NotifyResolver notifyResolver,
- PatchSetUtil patchSetUtil) {
+ PatchSetUtil patchSetUtil,
+ StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory) {
this.updateFactory = updateFactory;
this.json = json;
this.abandonOpFactory = abandonOpFactory;
this.notifyResolver = notifyResolver;
this.patchSetUtil = patchSetUtil;
+ this.storeSubmitRequirementsOpFactory = storeSubmitRequirementsOpFactory;
}
@Override
@@ -119,7 +123,9 @@
AbandonOp op = abandonOpFactory.create(accountState, msgTxt);
try (BatchUpdate u = updateFactory.create(notes.getProjectName(), user, TimeUtil.nowTs())) {
u.setNotify(notify);
- u.addOp(notes.getChangeId(), op).execute();
+ u.addOp(notes.getChangeId(), op);
+ u.addOp(notes.getChangeId(), storeSubmitRequirementsOpFactory.create());
+ u.execute();
}
return op.getChange();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 74407c0..2e1bd54 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4642,6 +4642,131 @@
ExperimentFeaturesConstants
.GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE
})
+ public void submitRequirement_storedForAbandonedChanges() throws Exception {
+ for (SubmitType submitType : SubmitType.values()) {
+ Project.NameKey project = createProjectForPush(submitType);
+ TestRepository<InMemoryRepository> repo = cloneProject(project);
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r =
+ createChange(repo, "master", "Add a file", "foo", "content", "topic");
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+
+ gApi.changes().id(r.getChangeId()).abandon();
+ ChangeNotes notes = notesFactory.create(project, r.getChange().getId());
+ SubmitRequirementResult result =
+ notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.submittabilityExpressionResult().status())
+ .isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
+ assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ .isEqualTo("label:Code-Review=+2");
+ }
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {
+ ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS,
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE
+ })
+ public void submitRequirement_retrievedFromNoteDbForAbandonedChanges() throws Exception {
+ for (SubmitType submitType : SubmitType.values()) {
+ Project.NameKey project = createProjectForPush(submitType);
+ TestRepository<InMemoryRepository> repo = cloneProject(project);
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r =
+ createChange(repo, "master", "Add a file", "foo", "content", "topic");
+ String changeId = r.getChangeId();
+ voteLabel(changeId, "Code-Review", 2);
+ gApi.changes().id(changeId).abandon();
+
+ // Add another submit requirement. This will not get returned for the abandoned change, since
+ // we return the state of the SR results when the change was abandoned.
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("New-Requirement")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("-has:unresolved"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+ ChangeInfo changeInfo =
+ gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+ assertThat(changeInfo.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements,
+ "Code-Review",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:Code-Review=+2");
+
+ // Restore the change, the new requirement will show up
+ gApi.changes().id(changeId).restore();
+ changeInfo = gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+ assertThat(changeInfo.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements,
+ "Code-Review",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:Code-Review=+2");
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements,
+ "New-Requirement",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "-has:unresolved");
+
+ // Abandon again, make sure the new requirement was persisted
+ gApi.changes().id(changeId).abandon();
+ changeInfo = gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+ assertThat(changeInfo.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements,
+ "Code-Review",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "label:Code-Review=+2");
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements,
+ "New-Requirement",
+ Status.SATISFIED,
+ /* isLegacy= */ false,
+ /* submittabilityCondition= */ "-has:unresolved");
+ }
+ }
+
+ @Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ values = {
+ ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS,
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_STORE_SUBMIT_REQUIREMENTS_ON_MERGE
+ })
public void submitRequirement_retrievedFromNoteDbForClosedChanges() throws Exception {
configSubmitRequirement(
project,
