Merge "Submit requirements evaluator: return results for legacy submit records"
diff --git a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
index 65662ba..5e9ce97 100644
--- a/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
+++ b/java/com/google/gerrit/server/experiments/ExperimentFeaturesConstants.java
@@ -32,6 +32,14 @@
public static final String GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_PUSH_CANCELLATION =
"GerritBackendRequestFeature__enable_push_cencallation";
+ /**
+ * Allow legacy {@link com.google.gerrit.entities.SubmitRecord}s to be converted and returned as
+ * submit requirements by the {@link
+ * com.google.gerrit.server.project.SubmitRequirementsEvaluator}.
+ */
+ public static final String GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS =
+ "GerritBackendRequestFeature__enable_legacy_submit_requirements";
+
/** Features, enabled by default in the current release. */
public static final ImmutableSet<String> DEFAULT_ENABLED_FEATURES =
ImmutableSet.of(UI_FEATURE_PATCHSET_COMMENTS);
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 36377e9..9be50c7 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -17,6 +17,8 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
@@ -24,6 +26,8 @@
import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.experiments.ExperimentFeatures;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.inject.AbstractModule;
@@ -31,13 +35,21 @@
import com.google.inject.Module;
import com.google.inject.Provider;
import com.google.inject.Scopes;
+import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.ObjectId;
/** Evaluates submit requirements for different change data. */
public class SubmitRequirementsEvaluatorImpl implements SubmitRequirementsEvaluator {
+
private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
private final ProjectCache projectCache;
+ private final SubmitRuleEvaluator.Factory legacyEvaluator;
+ private final ExperimentFeatures experimentFeatures;
public static Module module() {
return new AbstractModule() {
@@ -52,9 +64,14 @@
@Inject
private SubmitRequirementsEvaluatorImpl(
- Provider<ChangeQueryBuilder> changeQueryBuilderProvider, ProjectCache projectCache) {
+ Provider<ChangeQueryBuilder> changeQueryBuilderProvider,
+ ProjectCache projectCache,
+ SubmitRuleEvaluator.Factory legacyEvaluator,
+ ExperimentFeatures experimentFeatures) {
this.changeQueryBuilderProvider = changeQueryBuilderProvider;
this.projectCache = projectCache;
+ this.legacyEvaluator = legacyEvaluator;
+ this.experimentFeatures = experimentFeatures;
}
@Override
@@ -65,14 +82,13 @@
@Override
public Map<SubmitRequirement, SubmitRequirementResult> evaluateAllRequirements(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, evaluateRequirement(requirement, cd));
+ Map<SubmitRequirement, SubmitRequirementResult> result = getRequirements(cd);
+ if (experimentFeatures.isFeatureEnabled(
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)) {
+ result.putAll(getLegacyRequirements(cd));
}
- return result.build();
+ return ImmutableMap.copyOf(result);
}
@Override
@@ -113,6 +129,34 @@
}
}
+ /** Evaluate and return submit requirements stored in this project's config and its parents. */
+ private Map<SubmitRequirement, SubmitRequirementResult> getRequirements(ChangeData cd) {
+ ProjectState state = projectCache.get(cd.project()).orElseThrow(illegalState(cd.project()));
+ Map<String, SubmitRequirement> requirements = state.getSubmitRequirements();
+ Map<SubmitRequirement, SubmitRequirementResult> result = new HashMap<>();
+ for (SubmitRequirement requirement : requirements.values()) {
+ result.put(requirement, evaluateRequirement(requirement, cd));
+ }
+ return result;
+ }
+
+ /**
+ * Convert and return legacy submit records (created by label functions and other {@link
+ * com.google.gerrit.server.rules.SubmitRule}s to submit requirement results.
+ */
+ private Map<SubmitRequirement, SubmitRequirementResult> getLegacyRequirements(ChangeData cd) {
+ // We use SubmitRuleOptions.defaults() which does not recompute submit rules for closed changes.
+ // This doesn't have an effect since we never call this class (i.e. to evaluate submit
+ // requirements) for closed changes.
+ List<SubmitRecord> records = legacyEvaluator.create(SubmitRuleOptions.defaults()).evaluate(cd);
+ List<LabelType> labelTypes = cd.getLabelTypes().getLabelTypes();
+ ObjectId commitId = cd.currentPatchSet().commitId();
+ return records.stream()
+ .map(r -> SubmitRequirementsAdapter.createResult(r, labelTypes, commitId))
+ .flatMap(List::stream)
+ .collect(Collectors.toMap(sr -> sr.submitRequirement(), Function.identity()));
+ }
+
/** Evaluate the predicate recursively using change data. */
private PredicateResult evaluatePredicateTree(
Predicate<ChangeData> predicate, ChangeData changeData) {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 9d9c411..ac43078 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -169,6 +169,7 @@
import com.google.gerrit.server.change.ChangeMessages;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.testing.TestChangeETagComputation;
+import com.google.gerrit.server.experiments.ExperimentFeaturesConstants;
import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
@@ -4404,6 +4405,62 @@
}
@Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value =
+ ExperimentFeaturesConstants
+ .GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_LEGACY_SUBMIT_REQUIREMENTS)
+ public void submitRequirements_ReturnForLegacySubmitRecords_IfEnabled() throws Exception {
+ configLabel("build-cop-override", LabelFunction.MAX_WITH_BLOCK);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(
+ allowLabel("build-cop-override")
+ .ref("refs/heads/master")
+ .group(REGISTERED_USERS)
+ .range(-1, 1))
+ .update();
+
+ // 1. Project has two legacy requirements: Code-Review and bco. Both unsatisfied.
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "build-cop-override", Status.UNSATISFIED, /* isLegacy= */ true);
+
+ // 2. Vote +1 on bco. bco becomes satisfied
+ voteLabel(changeId, "build-cop-override", 1);
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(2);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "Code-Review", Status.UNSATISFIED, /* isLegacy= */ true);
+ assertSubmitRequirementStatus(
+ change.submitRequirements, "build-cop-override", Status.SATISFIED, /* isLegacy= */ true);
+
+ // 3. Vote +1 on Code-Review. Code-Review becomes satisfied
+ voteLabel(changeId, "Code-Review", 2);
+ 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);
+
+ // 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);
+ }
+
+ @Test
public void submitRequirement_backFilledFromIndexForActiveChanges() throws Exception {
configSubmitRequirement(
project,
