Adjust to changes in Gerrit core Change-Id: I08286eb53aac0af1b8f84669d576dccd30d5bb63
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(); } }