diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 9bfbd4d2e0..61f2c15042 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -1301,6 +1301,8 @@ The type of the configuration parameter, can be `STRING`, `INT`, The value of the configuration parameter as string. If the parameter is inheritable this is the effective value which is deduced from `configured_value` and `inherited_value`. +`editable` |`false` if not set| +Whether the value is editable. |`permitted_values`|optional| The list of permitted values, only set if the `type` is `LIST`. |`inheritable` |`false` if not set| diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java index d836cb523e..75de91526e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java @@ -377,7 +377,11 @@ public class ProjectInfoScreen extends ProjectScreen { } else { continue; } - widgetMap.put(param.name(), w); + if (param.editable()) { + widgetMap.put(param.name(), w); + } else { + w.setEnabled(false); + } } } @@ -441,8 +445,30 @@ public class ProjectInfoScreen extends ProjectScreen { } } - g.add(getDisplayName(param), listBox); - saveEnabler.listenTo(listBox); + if (param.editable()) { + saveEnabler.listenTo(listBox); + g.add(getDisplayName(param), listBox); + } else { + listBox.setEnabled(false); + + if (param.inheritable() && listBox.getSelectedIndex() != 0) { + // the inherited value is not selected, + // since the listBox is disabled the inherited value cannot be + // seen and we have to display it explicitly + Label inheritedLabel = + new Label(Util.M.pluginProjectInheritedValue(param + .inheritedValue())); + inheritedLabel.setStyleName(Gerrit.RESOURCES.css() + .pluginProjectConfigInheritedValue()); + HorizontalPanel p = new HorizontalPanel(); + p.add(listBox); + p.add(inheritedLabel); + g.add(getDisplayName(param), p); + } else { + g.add(getDisplayName(param), listBox); + } + } + return listBox; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java index 381c189356..3f2b90c117 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java @@ -151,6 +151,7 @@ public class ConfigInfo extends JavaScriptObject { public final native String displayName() /*-{ return this.display_name; }-*/; public final native String type() /*-{ return this.type; }-*/; public final native String value() /*-{ return this.value; }-*/; + public final native boolean editable() /*-{ return this.editable ? true : false; }-*/; public final native boolean inheritable() /*-{ return this.inheritable ? true : false; }-*/; public final native String configuredValue() /*-{ return this.configured_value; }-*/; public final native String inheritedValue() /*-{ return this.inherited_value; }-*/; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ProjectConfigEntry.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ProjectConfigEntry.java index 8076eaa5c3..c7cd48efdc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ProjectConfigEntry.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ProjectConfigEntry.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.config; import com.google.common.base.Function; import com.google.common.collect.Lists; import com.google.gerrit.extensions.annotations.ExtensionPoint; +import com.google.gerrit.server.project.ProjectState; import java.util.Arrays; import java.util.List; @@ -121,4 +122,8 @@ public class ProjectConfigEntry { public List getPermittedValues() { return permittedValues; } + + public boolean isEditable(ProjectState project) { + return true; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java index ee0669026c..4c3e5f44fe 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/CommitMergeStatus.java @@ -72,6 +72,11 @@ public enum CommitMergeStatus { "Change contains an invalid project configuration:\n" + "One of the plugin configuration parameters has a value that is not permitted."), + /** */ + INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE( + "Change contains an invalid project configuration:\n" + + "One of the plugin configuration parameters is not editable."), + /** */ INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND( "Change contains an invalid project configuration:\n" diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index cba5f1ae27..82ab25a21a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -687,6 +687,7 @@ public class MergeOp { case NOT_FAST_FORWARD: case INVALID_PROJECT_CONFIGURATION: case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED: + case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE: case INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND: case INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT: case SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN: diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 43bf1618cf..ef432465e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -880,16 +880,29 @@ public class ReceiveCommits { } for (Entry e : pluginConfigEntries) { + PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName()); ProjectConfigEntry configEntry = e.getProvider().get(); - if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType())) { - PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName()); - String value = pluginCfg.getString(e.getExportName()); - if (value != null && !configEntry.getPermittedValues().contains(value)) { - reject(cmd, String.format( - "invalid project configuration: The value '%s' is " - + "not permitted for parameter '%s' of plugin '%s'.", - value, e.getExportName(), e.getPluginName())); - } + String value = pluginCfg.getString(e.getExportName()); + String oldValue = + projectControl.getProjectState().getConfig() + .getPluginConfig(e.getPluginName()) + .getString(e.getExportName()); + + if ((value == null ? oldValue != null : !value.equals(oldValue)) && + !configEntry.isEditable(projectControl.getProjectState())) { + reject(cmd, String.format( + "invalid project configuration: Not allowed to set parameter" + + " '%s' of plugin '%s' on project '%s'.", + e.getExportName(), e.getPluginName(), project.getName())); + continue; + } + + if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType()) + && value != null && !configEntry.getPermittedValues().contains(value)) { + reject(cmd, String.format( + "invalid project configuration: The value '%s' is " + + "not permitted for parameter '%s' of plugin '%s'.", + value, e.getExportName(), e.getPluginName())); } } } catch (Exception e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java index 6d9a0cdf3d..519fc413ff 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java @@ -145,14 +145,24 @@ public class MergeValidators { } for (Entry e : pluginConfigEntries) { + PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName()); ProjectConfigEntry configEntry = e.getProvider().get(); - if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType())) { - PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName()); - String value = pluginCfg.getString(e.getExportName()); - if (value != null && !configEntry.getPermittedValues().contains(value)) { - throw new MergeValidationException(CommitMergeStatus. - INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED); - } + + String value = pluginCfg.getString(e.getExportName()); + String oldValue = destProject.getConfig() + .getPluginConfig(e.getPluginName()) + .getString(e.getExportName()); + + if ((value == null ? oldValue != null : !value.equals(oldValue)) && + !configEntry.isEditable(destProject)) { + throw new MergeValidationException(CommitMergeStatus. + INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_EDITABLE); + } + + if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType()) + && value != null && !configEntry.getPermittedValues().contains(value)) { + throw new MergeValidationException(CommitMergeStatus. + INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED); } } } catch (ConfigInvalidException | IOException e) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java index 18e713c241..5197b93484 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java @@ -132,14 +132,15 @@ public class ConfigInfo { PluginConfigFactory cfgFactory, AllProjectsNameProvider allProjects) { TreeMap> pluginConfig = new TreeMap<>(); for (Entry e : pluginConfigEntries) { + ProjectConfigEntry configEntry = e.getProvider().get(); PluginConfig cfg = cfgFactory.getFromProjectConfig(project, e.getPluginName()); - ProjectConfigEntry configEntry = e.getProvider().get(); String configuredValue = cfg.getString(e.getExportName()); ConfigParameterInfo p = new ConfigParameterInfo(); p.displayName = configEntry.getDisplayName(); p.type = configEntry.getType(); p.permittedValues = configEntry.getPermittedValues(); + p.editable = configEntry.isEditable(project) ? true : null; if (configEntry.isInheritable() && !allProjects.get().equals(project.getProject().getNameKey())) { PluginConfig cfgWithInheritance = @@ -194,6 +195,7 @@ public class ConfigInfo { public String displayName; public ProjectConfigEntry.Type type; public String value; + public Boolean editable; public Boolean inheritable; public String configuredValue; public String inheritedValue; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java index e50c751d2d..bff7a30181 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java @@ -148,7 +148,8 @@ public class PutConfig implements RestModifyView { } if (input.pluginConfigValues != null) { - setPluginConfigValues(projectConfig, input.pluginConfigValues); + setPluginConfigValues(rsrc.getControl().getProjectState(), + projectConfig, input.pluginConfigValues); } md.setMessage("Modified project settings\n"); @@ -177,8 +178,8 @@ public class PutConfig implements RestModifyView { } } - private void setPluginConfigValues(ProjectConfig projectConfig, - Map> pluginConfigValues) + private void setPluginConfigValues(ProjectState projectState, + ProjectConfig projectConfig, Map> pluginConfigValues) throws BadRequestException { for (Entry> e : pluginConfigValues.entrySet()) { String pluginName = e.getKey(); @@ -192,40 +193,51 @@ public class PutConfig implements RestModifyView { "Parameter name '%s' must match '^[a-zA-Z0-9]+[a-zA-Z0-9-]*$'", v.getKey())); continue; } + String oldValue = cfg.getString(v.getKey()); if (v.getValue() != null) { - try { - switch (projectConfigEntry.getType()) { - case BOOLEAN: - cfg.setBoolean(v.getKey(), Boolean.parseBoolean(v.getValue())); - break; - case INT: - cfg.setInt(v.getKey(), Integer.parseInt(v.getValue())); - break; - case LONG: - cfg.setLong(v.getKey(), Long.parseLong(v.getValue())); - break; - case LIST: - if (!projectConfigEntry.getPermittedValues() - .contains(v.getValue())) { - throw new BadRequestException(String.format( - "The value '%s' is not permitted for parameter '%s' of plugin '" - + pluginName + "'", v.getValue(), v.getKey())); - } - case STRING: - cfg.setString(v.getKey(), v.getValue()); - break; - default: - log.warn(String.format( - "The type '%s' of parameter '%s' is not supported.", - projectConfigEntry.getType().name(), v.getKey())); + if (!v.getValue().equals(oldValue)) { + validateProjectConfigEntryIsEditable(projectConfigEntry, + projectState, e.getKey(), pluginName); + try { + switch (projectConfigEntry.getType()) { + case BOOLEAN: + boolean newBooleanValue = Boolean.parseBoolean(v.getValue()); + cfg.setBoolean(v.getKey(), newBooleanValue); + break; + case INT: + int newIntValue = Integer.parseInt(v.getValue()); + cfg.setInt(v.getKey(), newIntValue); + break; + case LONG: + long newLongValue = Long.parseLong(v.getValue()); + cfg.setLong(v.getKey(), newLongValue); + break; + case LIST: + if (!projectConfigEntry.getPermittedValues().contains(v.getValue())) { + throw new BadRequestException(String.format( + "The value '%s' is not permitted for parameter '%s' of plugin '" + + pluginName + "'", v.getValue(), v.getKey())); + } + case STRING: + cfg.setString(v.getKey(), v.getValue()); + break; + default: + log.warn(String.format( + "The type '%s' of parameter '%s' is not supported.", + projectConfigEntry.getType().name(), v.getKey())); + } + } catch (NumberFormatException ex) { + throw new BadRequestException(String.format( + "The value '%s' of config parameter '%s' of plugin '%s' is invalid: %s", + v.getValue(), v.getKey(), pluginName, ex.getMessage())); } - } catch (NumberFormatException ex) { - throw new BadRequestException(String.format( - "The value '%s' of config parameter '%s' of plugin '%s' is invalid: %s", - v.getValue(), v.getKey(), pluginName, ex.getMessage())); } } else { - cfg.unset(v.getKey()); + if (oldValue != null) { + validateProjectConfigEntryIsEditable(projectConfigEntry, + projectState, e.getKey(), pluginName); + cfg.unset(v.getKey()); + } } } else { throw new BadRequestException(String.format( @@ -236,6 +248,16 @@ public class PutConfig implements RestModifyView { } } + private static void validateProjectConfigEntryIsEditable( + ProjectConfigEntry projectConfigEntry, ProjectState projectState, + String parameterName, String pluginName) throws BadRequestException { + if (!projectConfigEntry.isEditable(projectState)) { + throw new BadRequestException(String.format( + "Not allowed to set parameter '%s' of plugin '%s' on project '%s'.", + parameterName, pluginName, projectState.getProject().getName())); + } + } + private static boolean isValidParameterName(String name) { return CharMatcher.JAVA_LETTER_OR_DIGIT .or(CharMatcher.is('-'))