Migrate label fns to SRs: Adapt logic for 'branch' label attribute
If the label definition contained a 'branch' field, the label was only
applicable and voteable on these specific branches (see [1]).
Adapting the label fns to SRs migrator logic to handle this case: add an
OR'ed `applicableIf` expression to the SR of all branches occurring in
the label def. This should also work with branches with regex ref
pattern.
[1]
https://gerrit-review.googlesource.com/Documentation/config-labels.html#label_branch
Google-Bug-Id: b/264859003
Release-Notes: skip
Change-Id: I8ddd6111079744e021d069df09e5e8c51752aa0e
diff --git a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
index 491b5cd..f3c741f 100644
--- a/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
+++ b/java/com/google/gerrit/server/schema/MigrateLabelFunctionsToSubmitRequirement.java
@@ -37,6 +37,7 @@
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.stream.Collectors;
import javax.inject.Inject;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Config;
@@ -216,12 +217,19 @@
Arrays.asList(
cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_VALUE)))
.build();
+ ImmutableList<String> refPatterns =
+ ImmutableList.<String>builder()
+ .addAll(
+ Arrays.asList(
+ cfg.getStringList(ProjectConfig.LABEL, labelName, ProjectConfig.KEY_BRANCH)))
+ .build();
LabelAttributes attributes =
LabelAttributes.create(
function == null ? "MaxWithBlock" : function,
canOverride,
ignoreSelfApproval,
- values);
+ values,
+ refPatterns);
labelTypes.put(labelName, attributes);
}
return labelTypes;
@@ -320,6 +328,15 @@
default:
break;
}
+ if (!attributes.refPatterns().isEmpty()) {
+ builder.setApplicabilityExpression(
+ SubmitRequirementExpression.of(
+ String.join(
+ " OR ",
+ attributes.refPatterns().stream()
+ .map(b -> "branch:\\\"" + b + "\\\"")
+ .collect(Collectors.toList()))));
+ }
return builder.build();
}
@@ -435,13 +452,16 @@
abstract ImmutableList<String> values();
+ abstract ImmutableList<String> refPatterns();
+
static LabelAttributes create(
String function,
boolean canOverride,
boolean ignoreSelfApproval,
- ImmutableList<String> values) {
+ ImmutableList<String> values,
+ ImmutableList<String> refPatterns) {
return new AutoValue_MigrateLabelFunctionsToSubmitRequirement_LabelAttributes(
- function, canOverride, ignoreSelfApproval, values);
+ function, canOverride, ignoreSelfApproval, values, refPatterns);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
index 74bfe0f..9d37497 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/MigrateLabelFunctionsToSubmitRequirementIT.java
@@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -260,6 +261,96 @@
}
@Test
+ public void migrateBlockingLabel_withBranchAttribute() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/master"));
+
+ assertNonExistentSr(/* srName = */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName */ "Foo",
+ /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withMultipleBranchAttributes() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/master", "refs/heads/develop"));
+
+ assertNonExistentSr(/* srName = */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName */ "Foo",
+ /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
+ + "OR branch:\\\"refs/heads/develop\\\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withRegexBranchAttribute() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("^refs/heads/main-.*"));
+
+ assertNonExistentSr(/* srName = */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName */ "Foo",
+ /* applicabilityExpression= */ "branch:\\\"^refs/heads/main-.*\\\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
+ public void migrateBlockingLabel_withRegexAndNonRegexBranchAttributes() throws Exception {
+ createLabelWithBranch(
+ "Foo",
+ "MaxWithBlock",
+ /* ignoreSelfApproval= */ false,
+ ImmutableList.of("refs/heads/master", "^refs/heads/main-.*"));
+
+ assertNonExistentSr(/* srName = */ "Foo");
+
+ TestUpdateUI updateUI = runMigration(/* expectedResult= */ Status.MIGRATED);
+ assertThat(updateUI.newlyCreatedSrs).isEqualTo(1);
+ assertThat(updateUI.existingSrsMismatchingWithMigration).isEqualTo(0);
+
+ assertExistentSr(
+ /* srName */ "Foo",
+ /* applicabilityExpression= */ "branch:\\\"refs/heads/master\\\" "
+ + "OR branch:\\\"^refs/heads/main-.*\\\"",
+ /* submittabilityExpression= */ "label:Foo=MAX AND -label:Foo=MIN",
+ /* canOverride= */ true);
+ assertLabelFunction("Foo", "NoBlock");
+ }
+
+ @Test
public void migrationIsIdempotent() throws Exception {
String oldRefsConfigId;
try (Repository repo = repoManager.openRepository(project)) {
@@ -381,6 +472,21 @@
gApi.projects().name(project.get()).label(labelName).create(input);
}
+ private void createLabelWithBranch(
+ String labelName,
+ String function,
+ boolean ignoreSelfApproval,
+ ImmutableList<String> refPatterns)
+ throws Exception {
+ LabelDefinitionInput input = new LabelDefinitionInput();
+ input.name = labelName;
+ input.function = function;
+ input.ignoreSelfApproval = ignoreSelfApproval;
+ input.values = ImmutableMap.of("+1", "Looks Good", " 0", "Don't Know", "-1", "Looks Bad");
+ input.branches = refPatterns;
+ gApi.projects().name(project.get()).label(labelName).create(input);
+ }
+
@CanIgnoreReturnValue
private SubmitRequirementApi createSubmitRequirement(
String name, String submitExpression, boolean canOverride) throws Exception {
