Support project specific plugin list parameters for edit in UI

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

Documentation/rest-api-projects.txt

@@ -1295,10 +1295,12 @@ parameter.
 |`display_name`    |optional|
 The display name of the configuration parameter.
 |`type`            ||
-The type of the configuration parameter, can be `STRING`, `INT`, `LONG`
+The type of the configuration parameter, can be `STRING`, `INT`,
-or `BOOLEAN`.
+`LONG`, `BOOLEAN` or `LIST`.
 |`value`           |optional|
 The value of the configuration parameter as string.
+|`permitted_values`|optional|
+The list of permitted values, only set if the `type` is `LIST`.
 |===============================
 
 [[dashboard-info]]

gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java

@@ -371,6 +371,9 @@ public class ProjectInfoScreen extends ProjectScreen {
           w = renderTextBox(g, param, true);
         } else if ("BOOLEAN".equals(param.type())) {
           w = renderCheckBox(g, param);
+        } else if ("LIST".equals(param.type())
+            && param.permittedValues() != null) {
+          w = renderListBox(g, param);
         } else {
           continue;
         }
@@ -399,6 +402,21 @@ public class ProjectInfoScreen extends ProjectScreen {
     return checkBox;
   }
 
+  private ListBox renderListBox(LabeledWidgetsGrid g,
+      ConfigParameterInfo param) {
+    ListBox listBox = new ListBox();
+    for (int i = 0; i < param.permittedValues().length(); i++) {
+      String sv = param.permittedValues().get(i);
+      listBox.addItem(sv);
+      if (sv.equals(param.value())) {
+        listBox.setSelectedIndex(i);
+      }
+    }
+    g.add(getDisplayName(param), listBox);
+    saveEnabler.listenTo(listBox);
+    return listBox;
+  }
+
   private String getDisplayName(ConfigParameterInfo param) {
     return param.displayName() != null ? param.displayName() : param.name();
   }
@@ -460,6 +478,10 @@ public class ProjectInfoScreen extends ProjectScreen {
           values.put(e2.getKey(), ((TextBox) widget).getValue().trim());
         } else if (widget instanceof CheckBox) {
           values.put(e2.getKey(), Boolean.toString(((CheckBox) widget).getValue()));
+        } else if (widget instanceof ListBox) {
+          ListBox listBox = (ListBox) widget;
+          String value = listBox.getValue(listBox.getSelectedIndex());
+          values.put(e2.getKey(), value);
         }
       }
     }

gerrit-gwtui/src/main/java/com/google/gerrit/client/projects/ConfigInfo.java

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.client.Project.InheritableBoolean;
 import com.google.gerrit.reviewdb.client.Project.SubmitType;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.JsArray;
+import com.google.gwt.core.client.JsArrayString;
 import com.google.gwtexpui.safehtml.client.FindReplace;
 import com.google.gwtexpui.safehtml.client.LinkFindReplace;
 import com.google.gwtexpui.safehtml.client.RawFindReplace;
@@ -150,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 JsArrayString permittedValues()  /*-{ return this.permitted_values; }-*/;
 
     protected ConfigParameterInfo() {
     }

gerrit-server/src/main/java/com/google/gerrit/server/config/ProjectConfigEntry.java

@@ -14,38 +14,63 @@
 
 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 java.util.Arrays;
+import java.util.List;
+
 @ExtensionPoint
 public class ProjectConfigEntry {
   public enum Type {
-    STRING, INT, LONG, BOOLEAN
+    STRING, INT, LONG, BOOLEAN, LIST
   }
 
   private final String displayName;
   private final String defaultValue;
   private final Type type;
+  private final List<String> permittedValues;
 
   public ProjectConfigEntry(String displayName, String defaultValue) {
-    this(displayName, defaultValue, Type.STRING);
+    this(displayName, defaultValue, Type.STRING, null);
   }
 
   public ProjectConfigEntry(String displayName, int defaultValue) {
-    this(displayName, Integer.toString(defaultValue), Type.INT);
+    this(displayName, Integer.toString(defaultValue), Type.INT, null);
   }
 
   public ProjectConfigEntry(String displayName, long defaultValue) {
-    this(displayName, Long.toString(defaultValue), Type.LONG);
+    this(displayName, Long.toString(defaultValue), Type.LONG, null);
   }
 
   public ProjectConfigEntry(String displayName, boolean defaultValue) {
-    this(displayName, Boolean.toString(defaultValue), Type.BOOLEAN);
+    this(displayName, Boolean.toString(defaultValue), Type.BOOLEAN, null);
   }
 
-  private ProjectConfigEntry(String displayName, String defaultValue, Type type) {
+  public ProjectConfigEntry(String displayName, String defaultValue,
+      List<String> permittedValues) {
+    this(displayName, defaultValue, Type.LIST, permittedValues);
+  }
+
+  public <T extends Enum<?>> ProjectConfigEntry(String displayName,
+      T defaultValue, Class<T> permittedValues) {
+    this(displayName, defaultValue.name(), Type.LIST, Lists.transform(
+        Arrays.asList(permittedValues.getEnumConstants()),
+        new Function<Enum<?>, String>() {
+          @Override
+          public String apply(Enum<?> e) {
+            return e.name();
+          }
+        }));
+  }
+
+  private ProjectConfigEntry(String displayName, String defaultValue,
+      Type type, List<String> permittedValues) {
     this.displayName = displayName;
     this.defaultValue = defaultValue;
     this.type = type;
+    this.permittedValues = permittedValues;
   }
 
   public String getDisplayName() {
@@ -59,4 +84,8 @@ public class ProjectConfigEntry {
   public Type getType() {
     return type;
   }
+
+  public List<String> getPermittedValues() {
+    return permittedValues;
+  }
 }

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

@@ -67,6 +67,11 @@ public enum CommitMergeStatus {
   /** */
   INVALID_PROJECT_CONFIGURATION("Change contains an invalid project configuration."),
 
+  /** */
+  INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED(
+      "Change contains an invalid project configuration:\n"
+          + "One of the plugin configuration parameters has a value that is not permitted."),
+
   /** */
   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

@@ -686,6 +686,7 @@ public class MergeOp {
           case CANNOT_CHERRY_PICK_ROOT:
           case NOT_FAST_FORWARD:
           case INVALID_PROJECT_CONFIGURATION:
+          case INVALID_PROJECT_CONFIGURATION_PLUGIN_VALUE_NOT_PERMITTED:
           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

@@ -51,6 +51,8 @@ import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.Capable;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.registration.DynamicMap.Entry;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
@@ -75,6 +77,8 @@ import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.change.Submit;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.CanonicalWebUrl;
+import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.ProjectConfigEntry;
 import com.google.gerrit.server.events.CommitReceivedEvent;
 import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.MultiProgressMonitor.Task;
@@ -305,6 +309,7 @@ public class ReceiveCommits {
   private final SubmoduleOp.Factory subOpFactory;
   private final Provider<Submit> submitProvider;
   private final MergeQueue mergeQueue;
+  private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
 
   private final List<CommitValidationMessage> messages = new ArrayList<CommitValidationMessage>();
   private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@@ -352,7 +357,8 @@ public class ReceiveCommits {
       @Assisted final Repository repo,
       final SubmoduleOp.Factory subOpFactory,
       final Provider<Submit> submitProvider,
-      final MergeQueue mergeQueue) throws IOException {
+      final MergeQueue mergeQueue,
+      final DynamicMap<ProjectConfigEntry> pluginConfigEntries) throws IOException {
     this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
     this.db = db;
     this.changeDataFactory = changeDataFactory;
@@ -395,6 +401,7 @@ public class ReceiveCommits {
     this.subOpFactory = subOpFactory;
     this.submitProvider = submitProvider;
     this.mergeQueue = mergeQueue;
+    this.pluginConfigEntries = pluginConfigEntries;
 
     this.messageSender = new ReceivePackMessageSender();
 
@@ -871,6 +878,20 @@ public class ReceiveCommits {
                   continue;
                 }
               }
+
+              for (Entry<ProjectConfigEntry> e : pluginConfigEntries) {
+                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()));
+                  }
+                }
+              }
             } catch (Exception e) {
               reject(cmd, "invalid project configuration");
               log.error("User " + currentUser.getUserName()

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

@@ -15,7 +15,9 @@
 package com.google.gerrit.server.git.validators;
 
 import com.google.common.collect.Lists;
+import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.DynamicMap.Entry;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -25,6 +27,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ApprovalsUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.ProjectConfigEntry;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CommitMergeStatus;
 import com.google.gerrit.server.git.ProjectConfig;
@@ -32,8 +36,10 @@ import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.Inject;
 
+import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.lib.Repository;
 
+import java.io.IOException;
 import java.util.List;
 
 public class MergeValidators {
@@ -74,6 +80,7 @@ public class MergeValidators {
     private final ProjectCache projectCache;
     private final IdentifiedUser.GenericFactory identifiedUserFactory;
     private final ApprovalsUtil approvalsUtil;
+    private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
 
     public interface Factory {
       ProjectConfigValidator create();
@@ -83,12 +90,14 @@ public class MergeValidators {
     public ProjectConfigValidator(AllProjectsName allProjectsName,
         ReviewDb db, ProjectCache projectCache,
         IdentifiedUser.GenericFactory iuf,
-        ApprovalsUtil approvalsUtil) {
+        ApprovalsUtil approvalsUtil,
+        DynamicMap<ProjectConfigEntry> pluginConfigEntries) {
       this.allProjectsName = allProjectsName;
       this.db = db;
       this.projectCache = projectCache;
       this.identifiedUserFactory = iuf;
       this.approvalsUtil = approvalsUtil;
+      this.pluginConfigEntries = pluginConfigEntries;
     }
 
     @Override
@@ -105,39 +114,51 @@ public class MergeValidators {
               new ProjectConfig(destProject.getProject().getNameKey());
           cfg.load(repo, commit);
           newParent = cfg.getProject().getParent(allProjectsName);
-        } catch (Exception e) {
+          final Project.NameKey oldParent =
+              destProject.getProject().getParent(allProjectsName);
+          if (oldParent == null) {
+            // update of the 'All-Projects' project
+            if (newParent != null) {
+              throw new MergeValidationException(CommitMergeStatus.
+                  INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT);
+            }
+          } else {
+            if (!oldParent.equals(newParent)) {
+              PatchSetApproval psa =
+                  approvalsUtil.getSubmitter(db, commit.notes, patchSetId);
+              if (psa == null) {
+                throw new MergeValidationException(CommitMergeStatus.
+                    SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
+              }
+              final IdentifiedUser submitter =
+                  identifiedUserFactory.create(psa.getAccountId());
+              if (!submitter.getCapabilities().canAdministrateServer()) {
+                throw new MergeValidationException(CommitMergeStatus.
+                    SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
+              }
+
+              if (projectCache.get(newParent) == null) {
+                throw new MergeValidationException(CommitMergeStatus.
+                    INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND);
+              }
+            }
+          }
+
+          for (Entry<ProjectConfigEntry> e : pluginConfigEntries) {
+            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);
+              }
+            }
+          }
+        } catch (ConfigInvalidException | IOException e) {
           throw new MergeValidationException(CommitMergeStatus.
               INVALID_PROJECT_CONFIGURATION);
         }
-        final Project.NameKey oldParent =
-            destProject.getProject().getParent(allProjectsName);
-        if (oldParent == null) {
-          // update of the 'All-Projects' project
-          if (newParent != null) {
-            throw new MergeValidationException(CommitMergeStatus.
-                INVALID_PROJECT_CONFIGURATION_ROOT_PROJECT_CANNOT_HAVE_PARENT);
-          }
-        } else {
-          if (!oldParent.equals(newParent)) {
-            PatchSetApproval psa =
-                approvalsUtil.getSubmitter(db, commit.notes, patchSetId);
-            if (psa == null) {
-              throw new MergeValidationException(CommitMergeStatus.
-                  SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
-            }
-            final IdentifiedUser submitter =
-                identifiedUserFactory.create(psa.getAccountId());
-            if (!submitter.getCapabilities().canAdministrateServer()) {
-              throw new MergeValidationException(CommitMergeStatus.
-                  SETTING_PARENT_PROJECT_ONLY_ALLOWED_BY_ADMIN);
-            }
-
-            if (projectCache.get(newParent) == null) {
-              throw new MergeValidationException(CommitMergeStatus.
-                  INVALID_PROJECT_CONFIGURATION_PARENT_PROJECT_NOT_FOUND);
-            }
-          }
-        }
       }
     }
   }

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

@@ -32,6 +32,7 @@ import com.google.gerrit.server.extensions.webui.UiActions;
 import com.google.gerrit.server.git.TransferConfig;
 import com.google.inject.util.Providers;
 
+import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -135,6 +136,7 @@ public class ConfigInfo {
       p.displayName = configEntry.getDisplayName();
       p.type = configEntry.getType();
       p.value = cfg.getString(e.getExportName(), configEntry.getDefaultValue());
+      p.permittedValues = configEntry.getPermittedValues();
       Map<String, ConfigParameterInfo> pc = pluginConfig.get(e.getPluginName());
       if (pc == null) {
         pc = new TreeMap<>();
@@ -161,5 +163,6 @@ public class ConfigInfo {
     public String displayName;
     public ProjectConfigEntry.Type type;
     public String value;
+    public List<String> permittedValues;
   }
 }

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

@@ -201,6 +201,13 @@ public class PutConfig implements RestModifyView<ProjectResource, Input> {
                 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;