Merge "Read submit requirements from NoteDb for closed changes"
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 076ba46..2d49884 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -166,6 +166,7 @@
import com.google.gerrit.server.mime.FileTypeRegistry;
import com.google.gerrit.server.mime.MimeUtilFileTypeRegistry;
import com.google.gerrit.server.notedb.NoteDbModule;
+import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp;
import com.google.gerrit.server.patch.DiffOperationsImpl;
import com.google.gerrit.server.patch.PatchListCacheImpl;
import com.google.gerrit.server.patch.PatchScriptFactory;
@@ -466,6 +467,7 @@
factory(MergedByPushOp.Factory.class);
factory(GitModules.Factory.class);
factory(VersionedAuthorizedKeys.Factory.class);
+ factory(StoreSubmitRequirementsOp.Factory.class);
bind(AccountManager.class);
bind(SubscriptionGraph.Factory.class).to(ConfiguredSubscriptionGraphFactory.class);
diff --git a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
index 47948d7..4d99f93 100644
--- a/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
+++ b/java/com/google/gerrit/server/notedb/StoreSubmitRequirementsOp.java
@@ -15,16 +15,26 @@
package com.google.gerrit.server.notedb;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
+import com.google.inject.Inject;
/** A {@link BatchUpdateOp} that stores the evaluated submit requirements of a change in NoteDb. */
public class StoreSubmitRequirementsOp implements BatchUpdateOp {
private final ChangeData.Factory changeDataFactory;
+ private final SubmitRequirementsEvaluator evaluator;
- public StoreSubmitRequirementsOp(ChangeData.Factory changeDataFactory) {
+ public interface Factory {
+ StoreSubmitRequirementsOp create();
+ }
+
+ @Inject
+ public StoreSubmitRequirementsOp(
+ ChangeData.Factory changeDataFactory, SubmitRequirementsEvaluator evaluator) {
this.changeDataFactory = changeDataFactory;
+ this.evaluator = evaluator;
}
@Override
@@ -32,7 +42,7 @@
Change change = ctx.getChange();
ChangeData changeData = changeDataFactory.create(change);
ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
- update.putSubmitRequirementResults(changeData.submitRequirements().values());
+ update.putSubmitRequirementResults(evaluator.getResults(changeData).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 65d9d9e..e66b974 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -14,6 +14,9 @@
package com.google.gerrit.server.project;
+import static com.google.gerrit.server.project.ProjectCache.illegalState;
+
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
@@ -26,16 +29,20 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.util.Map;
import java.util.Optional;
/** Evaluates submit requirements for different change data. */
@Singleton
public class SubmitRequirementsEvaluator {
private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
+ private final ProjectCache projectCache;
@Inject
- private SubmitRequirementsEvaluator(Provider<ChangeQueryBuilder> changeQueryBuilderProvider) {
+ private SubmitRequirementsEvaluator(
+ Provider<ChangeQueryBuilder> changeQueryBuilderProvider, ProjectCache projectCache) {
this.changeQueryBuilderProvider = changeQueryBuilderProvider;
+ this.projectCache = projectCache;
}
/**
@@ -50,6 +57,21 @@
changeQueryBuilderProvider.get().parse(expression.expressionString());
}
+ /**
+ * Evaluate and return all submit requirements for a change. Submit requirements are retrieved for
+ * the project containing the change and parent projects as well.
+ */
+ public Map<SubmitRequirement, SubmitRequirementResult> getResults(ChangeData cd) {
+ ProjectState state = projectCache.get(cd.project()).orElseThrow(illegalState(cd.project()));
+ Map<String, SubmitRequirement> requirements = state.getSubmitRequirements();
+ ImmutableMap.Builder<SubmitRequirement, SubmitRequirementResult> result =
+ ImmutableMap.builderWithExpectedSize(requirements.size());
+ for (SubmitRequirement requirement : requirements.values()) {
+ result.put(requirement, evaluate(requirement, cd));
+ }
+ return result.build();
+ }
+
/** Evaluate a {@link SubmitRequirement} on a given {@link ChangeData}. */
public SubmitRequirementResult evaluate(SubmitRequirement sr, ChangeData cd) {
SubmitRequirementExpressionResult blockingResult =
diff --git a/java/com/google/gerrit/server/query/change/ChangeData.java b/java/com/google/gerrit/server/query/change/ChangeData.java
index 9879f27..852387f 100644
--- a/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -106,6 +106,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
@@ -932,17 +933,27 @@
return messages;
}
- /** Get all submit requirements for this change, including those from parent projects. */
+ /**
+ * Get all evaluated submit requirements for this change, including those from parent projects.
+ * For closed changes, submit requirements are read from the change notes. For active changes,
+ * submit requirements are evaluated online.
+ *
+ * <p>For changes loaded from the index, the value will be set from index field {@link
+ * com.google.gerrit.server.index.change.ChangeField#STORED_SUBMIT_REQUIREMENTS}.
+ */
public Map<SubmitRequirement, SubmitRequirementResult> submitRequirements() {
if (submitRequirements == null) {
- ProjectState state = projectCache.get(project()).orElseThrow(illegalState(project()));
- Map<String, SubmitRequirement> requirements = state.getSubmitRequirements();
- ImmutableMap.Builder<SubmitRequirement, SubmitRequirementResult> result =
- ImmutableMap.builderWithExpectedSize(requirements.size());
- for (SubmitRequirement requirement : requirements.values()) {
- result.put(requirement, submitRequirementsEvaluator.evaluate(requirement, this));
+ if (!lazyload()) {
+ return Collections.emptyMap();
}
- submitRequirements = result.build();
+ Change c = change();
+ if (c != null && c.isClosed()) {
+ submitRequirements =
+ notes().getSubmitRequirementsResult().stream()
+ .collect(Collectors.toMap(r -> r.submitRequirement(), Function.identity()));
+ } else {
+ submitRequirements = submitRequirementsEvaluator.getResults(this);
+ }
}
return submitRequirements;
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 363cdca..942f024 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -241,6 +241,7 @@
private final NotifyResolver notifyResolver;
private final RetryHelper retryHelper;
private final ChangeData.Factory changeDataFactory;
+ private final StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory;
// Changes that were updated by this MergeOp.
private final Map<Change.Id, Change> updatedChanges;
@@ -274,7 +275,8 @@
NotifyResolver notifyResolver,
TopicMetrics topicMetrics,
RetryHelper retryHelper,
- ChangeData.Factory changeDataFactory) {
+ ChangeData.Factory changeDataFactory,
+ StoreSubmitRequirementsOp.Factory storeSubmitRequirementsOpFactory) {
this.cmUtil = cmUtil;
this.batchUpdateFactory = batchUpdateFactory;
this.internalUserFactory = internalUserFactory;
@@ -291,6 +293,7 @@
this.topicMetrics = topicMetrics;
this.changeDataFactory = changeDataFactory;
this.updatedChanges = new HashMap<>();
+ this.storeSubmitRequirementsOpFactory = storeSubmitRequirementsOpFactory;
}
@Override
@@ -665,7 +668,7 @@
Change.Id changeId = entry.getKey();
batchUpdatesByProject
.get(project)
- .addOp(changeId, new StoreSubmitRequirementsOp(changeDataFactory));
+ .addOp(changeId, storeSubmitRequirementsOpFactory.create());
}
try {
submissionExecutor.setAdditionalBatchUpdateListeners(
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d6ae16c..7785776 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4325,6 +4325,46 @@
}
@Test
+ public void submitRequirement_retrievedFromNoteDbForClosedChanges() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:code-review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+
+ voteLabel(changeId, "code-review", 2);
+
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+
+ gApi.changes().id(changeId).current().submit();
+
+ // Add new submit requirement
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("verified")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:verified=+1"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ // 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);
+ }
+
+ @Test
public void submitRequirement_backFilledFromIndexForActiveChanges() throws Exception {
configSubmitRequirement(
project,