Merge changes I3eb497d7,Icfa329bf * changes: Add config option to disallow functions globally Persist inherited labels locally in case they are modified
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 035a2a8..178fadb 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
@@ -14,9 +14,10 @@ package com.googlesource.gerrit.plugins.simplesubmitrules.config; +import com.google.common.collect.ImmutableList; import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.data.LabelFunction; import com.google.gerrit.common.data.LabelType; -import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.server.config.PluginConfig; @@ -31,6 +32,7 @@ import com.googlesource.gerrit.plugins.simplesubmitrules.api.SubmitConfig; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Map; /** Codec class used to convert {@link SubmitConfig} from/to a Gerrit config */ @@ -106,27 +108,57 @@ } void applyTo(SubmitConfig inConfig, ProjectState projectState) throws BadRequestException { - PluginConfig pluginConfig = pluginConfigFactory.getFromProjectConfig(projectState, pluginName); - applyCommentRulesTo(inConfig.comments, pluginConfig); - applyLabelsTo(inConfig.labels, projectState.getLabelTypes()); + PluginConfig hostPluginConfig = pluginConfigFactory.getFromGerritConfig(pluginName); + PluginConfig projectPluginConfig = + pluginConfigFactory.getFromProjectConfig(projectState, pluginName); + applyCommentRulesTo(inConfig.comments, projectPluginConfig); + applyLabelsTo(inConfig.labels, projectState, hostPluginConfig); } - private static void applyLabelsTo(Map<String, LabelDefinition> labels, LabelTypes labelTypes) + private static void applyLabelsTo( + Map<String, LabelDefinition> labels, ProjectState projectState, PluginConfig hostPluginConfig) throws BadRequestException { + if (labels.isEmpty()) { + return; + } + for (Map.Entry<String, LabelDefinition> entry : labels.entrySet()) { + if (!projectState.getConfig().getLabelSections().containsKey(entry.getKey())) { + // The current project does not have this label. Try to copy it down from the inherited + // labels to be able to modify it locally. + Map<String, LabelType> copiedLabelTypes = projectState.getConfig().getLabelSections(); + projectState + .getLabelTypes() + .getLabelTypes() + .stream() + .filter(l -> l.getName().equals(entry.getKey())) + .filter(l -> l.canOverride()) + .forEach(l -> copiedLabelTypes.put(l.getName(), copyLabelType(l))); + } + String label = entry.getKey(); LabelDefinition definition = entry.getValue(); - LabelType labelType = labelTypes.byLabel(label); + LabelType labelType = projectState.getConfig().getLabelSections().get(label); if (labelType == null) { throw new BadRequestException( "The label " + label + " does not exist. You can't change its config."); } - definition.getFunction().ifPresent(labelType::setFunction); if (definition.ignoreSelfApproval != null) { labelType.setIgnoreSelfApproval(definition.ignoreSelfApproval); } + + if (definition.getFunction().isPresent()) { + List<String> disallowedLabelFunctions = + ImmutableList.copyOf( + hostPluginConfig.getStringList("disallowedLabelFunctions-" + label)); + LabelFunction function = definition.getFunction().get(); + if (disallowedLabelFunctions.contains(function.getFunctionName())) { + throw new BadRequestException(function.getFunctionName() + " disallowed"); + } + labelType.setFunction(function); + } applyCopyScoresTo(definition.copyScores, labelType); } } @@ -152,4 +184,23 @@ 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/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md index 15c0e18..33f465c 100644 --- a/src/main/resources/Documentation/about.md +++ b/src/main/resources/Documentation/about.md
@@ -53,8 +53,10 @@ The comments section defines the rules to apply to comments (can the change be submitted with unresolved comments, …). The labels section defines each label. -It is reasonable to consider that a label missing from the labels section won't be reset, but -consumers should not rely upon it. +When reading labels on the API, the result includes both local and inherited labels. +When the configuration is modified through the API, the plugin will check if there are +local label configurations. If the request modifies an inherited label, it will be copied +down so that it can be modified locally. ### CommentsRules @@ -104,3 +106,21 @@ Example: `MaxWithBlock`, `AnyWithBlock`... See the Labels documentation page for more information. + +### Configuration in gerrit.config + +The following is a list of configuration options that can be changed +in gerrit.config. All configs have to be nested under plugin.@PLUGIN@: + +#### disallowedLabelFunctions-<label-name> + +This config will prevent users from changing the function in the +label configuration that is referenced in the name to a matching value. +However, the config does not effect existing labels that already have +the forbidden value. + +Example: +``` +[plugin "simple-submit"] + disallowedLabelFunctions-Code-Review = MaxNoBlock +```
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 d41ed1c..2ddbba7 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/PluginIT.java
@@ -25,6 +25,7 @@ import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestPlugin; import com.google.gerrit.common.RawInputUtil; +import com.google.gerrit.common.data.LabelFunction; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.client.Side; @@ -37,7 +38,6 @@ import com.googlesource.gerrit.plugins.simplesubmitrules.api.SubmitConfig; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; -import org.junit.Before; import org.junit.Test; @TestPlugin( @@ -47,16 +47,6 @@ public class PluginIT extends LightweightPluginDaemonTest { private static final String JSON_TYPE = "application/json"; - @Before - public void setUpCodeReviewLabel() throws Exception { - // TODO(hiesel): Remove once copy-down logic is in place - try (ProjectConfigUpdate u = updateProject(project)) { - LabelType codeReview = projectCache.getAllProjects().getLabelTypes().byLabel("Code-Review"); - u.getConfig().getLabelSections().put("Code-Review", codeReview); - u.save(); - } - } - @Test public void singleApprovalIsSufficientByDefault() throws Exception { PushOneCommit.Result r = createChange(); @@ -145,6 +135,16 @@ assertThat(parsedConfig.comments).isEqualTo(new CommentsRules(true)); } + @Test + public void pluginPersistsLabelInCurrentProjectWhenOverrideIsNeeded() throws Exception { + LabelDefinition codeReview = new LabelDefinition("MaxNoBlock", false, null); + SubmitConfig config = new SubmitConfig(ImmutableMap.of("Code-Review", codeReview), null); + postConfig(project, config); + + LabelType myLabel = projectCache.get(project).getConfig().getLabelSections().get("Code-Review"); + assertThat(myLabel.getFunction()).isEqualTo(LabelFunction.MAX_NO_BLOCK); + } + private void postConfig(Project.NameKey project, SubmitConfig config) throws Exception { RawInput rawInput = RawInputUtil.create(newGson().toJson(config).getBytes(Charsets.UTF_8), JSON_TYPE);
diff --git a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java index 815198e..0c078c7 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java +++ b/src/test/java/com/googlesource/gerrit/plugins/simplesubmitrules/config/ConfigServletIT.java
@@ -14,9 +14,11 @@ package com.googlesource.gerrit.plugins.simplesubmitrules.config; +import static com.google.common.truth.Truth.assertThat; import static com.google.gerrit.server.project.testing.Util.value; import com.google.common.base.Charsets; +import com.google.gerrit.acceptance.GerritConfig; import com.google.gerrit.acceptance.LightweightPluginDaemonTest; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.TestPlugin; @@ -68,6 +70,16 @@ r.assertForbidden(); } + @Test + @GerritConfig( + name = "plugin.my-plugin.disallowedLabelFunctions-Code-Review", + value = "MaxWithBlock") + public void disallowedFunctionThrowsBadRequestException() throws Exception { + RawInput rawInput = createConfig(); + RestResponse r = adminRestSession.putRaw(endpointUrl(project), rawInput); + assertThat(r.getEntityContent()).isEqualTo("MaxWithBlock disallowed"); + } + private static RawInput createConfig() { return RawInputUtil.create( ("{\n"