Merge "Adjust to changes in Gerrit core"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java
index bea7df0..0808bb1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslator.java
@@ -16,7 +16,6 @@
import static com.google.gerrit.server.project.ProjectCache.illegalState;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
@@ -81,7 +80,7 @@
}
static void applyCopyScoreRulesTo(
- Set<String> copyScoreRules, Set<String> disallowedCopyScoreRules, LabelType labelType)
+ Set<String> copyScoreRules, Set<String> disallowedCopyScoreRules, LabelType.Builder labelType)
throws BadRequestException {
Set<String> disallowed =
Sets.intersection(ImmutableSet.copyOf(copyScoreRules), disallowedCopyScoreRules);
@@ -150,23 +149,16 @@
.orElseThrow(illegalState(projectConfig.getName()));
projectState.getLabelTypes().getLabelTypes().stream()
.filter(l -> l.getName().equals(entry.getKey()))
- .filter(l -> l.canOverride())
- .forEach(l -> copiedLabelTypes.put(l.getName(), copyLabelType(l)));
+ .filter(l -> l.isCanOverride())
+ .forEach(l -> copiedLabelTypes.put(l.getName(), l));
}
String label = entry.getKey();
LabelDefinition definition = entry.getValue();
- LabelType labelType = projectConfig.getLabelSections().get(label);
-
- if (labelType == null) {
+ if (projectConfig.getLabelSections().get(label) == null) {
throw new BadRequestException(
"The label " + label + " does not exist. You can't change its config.");
}
-
- if (definition.ignoreSelfApproval != null) {
- labelType.setIgnoreSelfApproval(definition.ignoreSelfApproval);
- }
-
if (definition.getFunction().isPresent()) {
Set<String> disallowedLabelFunctions =
ImmutableSet.copyOf(
@@ -175,14 +167,25 @@
if (disallowedLabelFunctions.contains(function.getFunctionName())) {
throw new BadRequestException(function.getFunctionName() + " disallowed");
}
- labelType.setFunction(function);
}
+ projectConfig.updateLabelType(
+ label,
+ labelType -> {
+ if (definition.ignoreSelfApproval != null) {
+ labelType.setIgnoreSelfApproval(definition.ignoreSelfApproval);
+ }
+ if (definition.getFunction().isPresent()) {
+ labelType.setFunction(definition.getFunction().get());
+ }
+ });
if (definition.copyScoreRules != null) {
+ LabelType.Builder builder = projectConfig.getLabelSections().get(label).toBuilder();
Set<String> disallowedCopyScoreRules =
ImmutableSet.copyOf(
hostPluginConfig.getStringList("disallowedCopyScoreRules-" + label));
- applyCopyScoreRulesTo(definition.copyScoreRules, disallowedCopyScoreRules, labelType);
+ applyCopyScoreRulesTo(definition.copyScoreRules, disallowedCopyScoreRules, builder);
+ projectConfig.upsertLabelType(builder.build());
}
}
}
@@ -197,7 +200,7 @@
labelDefinition.function = labelType.getFunction().getFunctionName();
extractLabelCopyScoreRules(labelType, labelDefinition);
- labelDefinition.ignoreSelfApproval = labelType.ignoreSelfApproval();
+ labelDefinition.ignoreSelfApproval = labelType.isIgnoreSelfApproval();
}
private static void applyCommentRulesTo(@Nullable CommentsRules comments, PluginConfig config) {
@@ -208,23 +211,4 @@
SimpleSubmitRulesConfig.KEY_BLOCK_IF_UNRESOLVED_COMMENTS,
comments.blockIfUnresolvedComments);
}
-
- private static LabelType copyLabelType(LabelType label) {
- // TODO(hiesel) Move this to core
- LabelType copy = new LabelType(label.getName(), ImmutableList.copyOf(label.getValues()));
- if (label.getRefPatterns() != null) {
- copy.setRefPatterns(ImmutableList.copyOf(label.getRefPatterns()));
- }
- copy.setAllowPostSubmit(label.allowPostSubmit());
- copy.setCanOverride(label.canOverride());
- copy.setCopyAllScoresIfNoChange(label.isCopyAllScoresIfNoChange());
- copy.setCopyAllScoresIfNoCodeChange(label.isCopyAllScoresIfNoCodeChange());
- copy.setCopyAllScoresOnMergeFirstParentUpdate(label.isCopyAllScoresOnMergeFirstParentUpdate());
- copy.setCopyAllScoresOnTrivialRebase(label.isCopyAllScoresOnTrivialRebase());
- copy.setIgnoreSelfApproval(label.ignoreSelfApproval());
- copy.setCopyMaxScore(label.isCopyMaxScore());
- copy.setCopyMinScore(label.isCopyMinScore());
- copy.setFunction(label.getFunction());
- return copy;
- }
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
index b0d8be2..91276af 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
@@ -140,7 +140,11 @@
// Check that the label has the same configs besides the function, which we changed
LabelType allProjectsCR = projectCache.getAllProjects().getLabelTypes().byLabel("Code-Review");
- localCR.setFunction(allProjectsCR.getFunction());
+ projectCache
+ .get(project)
+ .get()
+ .getConfig()
+ .updateLabelType("Code-Review", lt -> lt.setFunction(allProjectsCR.getFunction()));
assertLabelTypeEquals(localCR, allProjectsCR);
}
@@ -167,15 +171,15 @@
}
private static void assertLabelTypeEquals(LabelType l1, LabelType l2) {
- assertThat(l1.allowPostSubmit()).isEqualTo(l2.allowPostSubmit());
- assertThat(l1.canOverride()).isEqualTo(l2.canOverride());
+ assertThat(l1.isAllowPostSubmit()).isEqualTo(l2.isAllowPostSubmit());
+ assertThat(l1.isCanOverride()).isEqualTo(l2.isCanOverride());
assertThat(l1.getDefaultValue()).isEqualTo(l2.getDefaultValue());
assertThat(l1.getLabelId()).isEqualTo(l2.getLabelId());
assertThat(l1.getMax()).isEqualTo(l2.getMax());
assertThat(l1.getMin()).isEqualTo(l2.getMin());
assertThat(l1.getName()).isEqualTo(l2.getName());
assertThat(l1.getRefPatterns()).isEqualTo(l2.getRefPatterns());
- assertThat(l1.ignoreSelfApproval()).isEqualTo(l2.ignoreSelfApproval());
+ assertThat(l1.isIgnoreSelfApproval()).isEqualTo(l2.isIgnoreSelfApproval());
assertThat(l1.isCopyAllScoresIfNoChange()).isEqualTo(l2.isCopyAllScoresIfNoChange());
assertThat(l1.isCopyAllScoresIfNoCodeChange()).isEqualTo(l2.isCopyAllScoresIfNoCodeChange());
assertThat(l1.isCopyAllScoresOnMergeFirstParentUpdate())
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
index 05b7bf0..a7c8f18 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigTranslatorTest.java
@@ -29,28 +29,28 @@
public class ConfigTranslatorTest {
@Test
public void checkLabelTranslation() throws Exception {
- checkTranslation("copyMinScore", LabelType::isCopyMinScore, LabelType::setCopyMinScore);
- checkTranslation("copyMaxScore", LabelType::isCopyMaxScore, LabelType::setCopyMaxScore);
+ checkTranslation("copyMinScore", LabelType::isCopyMinScore, LabelType.Builder::setCopyMinScore);
+ checkTranslation("copyMaxScore", LabelType::isCopyMaxScore, LabelType.Builder::setCopyMaxScore);
checkTranslation(
"copyAllScoresIfNoChange",
LabelType::isCopyAllScoresIfNoChange,
- LabelType::setCopyAllScoresIfNoChange);
+ LabelType.Builder::setCopyAllScoresIfNoChange);
checkTranslation(
"copyAllScoresIfNoCodeChange",
LabelType::isCopyAllScoresIfNoCodeChange,
- LabelType::setCopyAllScoresIfNoCodeChange);
+ LabelType.Builder::setCopyAllScoresIfNoCodeChange);
checkTranslation(
"copyAllScoresOnMergeFirstParentUpdate",
LabelType::isCopyAllScoresOnMergeFirstParentUpdate,
- LabelType::setCopyAllScoresOnMergeFirstParentUpdate);
+ LabelType.Builder::setCopyAllScoresOnMergeFirstParentUpdate);
checkTranslation(
"copyAllScoresOnTrivialRebase",
LabelType::isCopyAllScoresOnTrivialRebase,
- LabelType::setCopyAllScoresOnTrivialRebase);
+ LabelType.Builder::setCopyAllScoresOnTrivialRebase);
}
@Test
@@ -63,7 +63,7 @@
ImmutableSet.of(
"copyAllScoresIfNoChange", "copyAllScoresOnMergeFirstParentUpdate"),
ImmutableSet.of("copyAllScoresIfNoChange"),
- LabelType.withDefaultValues("Verified")));
+ LabelType.withDefaultValues("Verified").toBuilder()));
assertThat(thrown)
.hasMessageThat()
.contains("copy score rules [copyAllScoresIfNoChange] are forbidden");
@@ -73,7 +73,7 @@
private static void checkTranslation(
String copyScoreName,
Predicate<LabelType> functionToCheck,
- BiConsumer<LabelType, Boolean> functionToSet)
+ BiConsumer<LabelType.Builder, Boolean> functionToSet)
throws Exception {
checkLabelToGerritPresent(copyScoreName, functionToCheck);
checkLabelToGerritAbsent(copyScoreName, functionToCheck);
@@ -83,13 +83,13 @@
}
private static void checkLabelFromGerritPresent(
- String copyScoreName, BiConsumer<LabelType, Boolean> functionToSet) {
+ String copyScoreName, BiConsumer<LabelType.Builder, Boolean> functionToSet) {
- LabelType label = LabelType.withDefaultValues("Verified");
+ LabelType.Builder label = LabelType.withDefaultValues("Verified").toBuilder();
LabelDefinition labelDefinition = new LabelDefinition();
functionToSet.accept(label, false);
- ConfigTranslator.extractLabelCopyScoreRules(label, labelDefinition);
+ ConfigTranslator.extractLabelCopyScoreRules(label.build(), labelDefinition);
assertWithMessage("[case %s:false]", copyScoreName)
.that(labelDefinition.copyScoreRules)
@@ -97,13 +97,13 @@
}
private static void checkLabelFromGerritAbsent(
- String copyScoreName, BiConsumer<LabelType, Boolean> functionToSet) {
+ String copyScoreName, BiConsumer<LabelType.Builder, Boolean> functionToSet) {
- LabelType label = LabelType.withDefaultValues("Verified");
+ LabelType.Builder label = LabelType.withDefaultValues("Verified").toBuilder();
LabelDefinition labelDefinition = new LabelDefinition();
functionToSet.accept(label, true);
- ConfigTranslator.extractLabelCopyScoreRules(label, labelDefinition);
+ ConfigTranslator.extractLabelCopyScoreRules(label.build(), labelDefinition);
assertWithMessage("[case %s:true]", copyScoreName)
.that(labelDefinition.copyScoreRules)
@@ -112,18 +112,21 @@
private static void checkLabelToGerritPresent(
String copyScoreName, Predicate<LabelType> functionToCheck) throws Exception {
- LabelType label = LabelType.withDefaultValues("Verified");
-
+ LabelType.Builder label = LabelType.withDefaultValues("Verified").toBuilder();
ConfigTranslator.applyCopyScoreRulesTo(
ImmutableSet.of(copyScoreName), ImmutableSet.of(), label);
- assertWithMessage("[case %s:true]", copyScoreName).that(functionToCheck.test(label)).isTrue();
+ assertWithMessage("[case %s:true]", copyScoreName)
+ .that(functionToCheck.test(label.build()))
+ .isTrue();
}
private static void checkLabelToGerritAbsent(
String copyScoreName, Predicate<LabelType> functionToCheck) throws Exception {
- LabelType label = LabelType.withDefaultValues("Verified");
+ LabelType.Builder label = LabelType.withDefaultValues("Verified").toBuilder();
ConfigTranslator.applyCopyScoreRulesTo(ImmutableSet.of(), ImmutableSet.of(), label);
- assertWithMessage("[case %s:false]", copyScoreName).that(functionToCheck.test(label)).isFalse();
+ assertWithMessage("[case %s:false]", copyScoreName)
+ .that(functionToCheck.test(label.build()))
+ .isFalse();
}
}
