Allow to define per project if plugin config parameter is editable

It is now possible to decide for a project specific plugin
configuration parameter whether for a specific project it is editable
in the UI.

Change-Id: I29f55ca80d0c304ec61e5f7be10128629c7945cf
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>

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|

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);
+    if (param.editable()) {
-    saveEnabler.listenTo(listBox);
+      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;
   }
 

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; }-*/;

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<String> getPermittedValues() {
     return permittedValues;
   }
+
+  public boolean isEditable(ProjectState project) {
+    return true;
+  }
 }

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"

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:

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -880,16 +880,29 @@ public class ReceiveCommits {
               }
 
               for (Entry<ProjectConfigEntry> e : pluginConfigEntries) {
+                PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName());
                 ProjectConfigEntry configEntry = e.getProvider().get();
-                if (ProjectConfigEntry.Type.LIST.equals(configEntry.getType())) {
+                String value = pluginCfg.getString(e.getExportName());
-                  PluginConfig pluginCfg = cfg.getPluginConfig(e.getPluginName());
+                String oldValue =
-                  String value = pluginCfg.getString(e.getExportName());
+                    projectControl.getProjectState().getConfig()
-                  if (value != null && !configEntry.getPermittedValues().contains(value)) {
+                        .getPluginConfig(e.getPluginName())
-                    reject(cmd, String.format(
+                        .getString(e.getExportName());
-                        "invalid project configuration: The value '%s' is "
+
-                            + "not permitted for parameter '%s' of plugin '%s'.",
+                if ((value == null ? oldValue != null : !value.equals(oldValue)) &&
-                        value, e.getExportName(), e.getPluginName()));
+                    !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) {

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidators.java

@@ -145,14 +145,24 @@ public class MergeValidators {
           }
 
           for (Entry<ProjectConfigEntry> 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());
-              String value = pluginCfg.getString(e.getExportName());
+            String oldValue = destProject.getConfig()
-              if (value != null && !configEntry.getPermittedValues().contains(value)) {
+                .getPluginConfig(e.getPluginName())
-                throw new MergeValidationException(CommitMergeStatus.
+                .getString(e.getExportName());
-                    INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED);
+
-              }
+            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) {

gerrit-server/src/main/java/com/google/gerrit/server/project/ConfigInfo.java

@@ -132,14 +132,15 @@ public class ConfigInfo {
       PluginConfigFactory cfgFactory, AllProjectsNameProvider allProjects) {
     TreeMap<String, Map<String, ConfigParameterInfo>> pluginConfig = new TreeMap<>();
     for (Entry<ProjectConfigEntry> 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;

gerrit-server/src/main/java/com/google/gerrit/server/project/PutConfig.java

@@ -148,7 +148,8 @@ public class PutConfig implements RestModifyView<ProjectResource, Input> {
       }
 
       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<ProjectResource, Input> {
     }
   }
 
-  private void setPluginConfigValues(ProjectConfig projectConfig,
+  private void setPluginConfigValues(ProjectState projectState,
-      Map<String, Map<String, String>> pluginConfigValues)
+      ProjectConfig projectConfig, Map<String, Map<String, String>> pluginConfigValues)
       throws BadRequestException {
     for (Entry<String, Map<String, String>> e : pluginConfigValues.entrySet()) {
       String pluginName = e.getKey();
@@ -192,40 +193,51 @@ public class PutConfig implements RestModifyView<ProjectResource, Input> {
                 "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 {
+            if (!v.getValue().equals(oldValue)) {
-              switch (projectConfigEntry.getType()) {
+              validateProjectConfigEntryIsEditable(projectConfigEntry,
-                case BOOLEAN:
+                  projectState, e.getKey(), pluginName);
-                  cfg.setBoolean(v.getKey(), Boolean.parseBoolean(v.getValue()));
+              try {
-                  break;
+                switch (projectConfigEntry.getType()) {
-                case INT:
+                  case BOOLEAN:
-                  cfg.setInt(v.getKey(), Integer.parseInt(v.getValue()));
+                    boolean newBooleanValue = Boolean.parseBoolean(v.getValue());
-                  break;
+                    cfg.setBoolean(v.getKey(), newBooleanValue);
-                case LONG:
+                    break;
-                  cfg.setLong(v.getKey(), Long.parseLong(v.getValue()));
+                  case INT:
-                  break;
+                    int newIntValue = Integer.parseInt(v.getValue());
-                case LIST:
+                    cfg.setInt(v.getKey(), newIntValue);
-                  if (!projectConfigEntry.getPermittedValues()
+                    break;
-                      .contains(v.getValue())) {
+                  case LONG:
-                    throw new BadRequestException(String.format(
+                    long newLongValue = Long.parseLong(v.getValue());
-                        "The value '%s' is not permitted for parameter '%s' of plugin '"
+                    cfg.setLong(v.getKey(), newLongValue);
-                            + pluginName + "'", v.getValue(), v.getKey()));
+                    break;
-                  }
+                  case LIST:
-                case STRING:
+                    if (!projectConfigEntry.getPermittedValues().contains(v.getValue())) {
-                  cfg.setString(v.getKey(), v.getValue());
+                      throw new BadRequestException(String.format(
-                  break;
+                          "The value '%s' is not permitted for parameter '%s' of plugin '"
-                default:
+                              + pluginName + "'", v.getValue(), v.getKey()));
-                  log.warn(String.format(
+                    }
-                      "The type '%s' of parameter '%s' is not supported.",
+                  case STRING:
-                      projectConfigEntry.getType().name(), v.getKey()));
+                    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<ProjectResource, Input> {
     }
   }
 
+  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('-'))