commit b23cb34b63d1b405695fae4724e2f532f3603aa2
author Edwin Kempin <ekempin@google.com> Wed Nov 23 13:29:24 2022 +0100
committer Edwin Kempin <ekempin@google.com> Wed Nov 23 13:29:24 2022 +0100
tree acfab777311b1f3054ff2d5d8d73987306b9fe56
parent 415c40ea241a87d28694d07995aeea2213e30d30

Do not fail with ISE when users try to create a label with an invalid function

ProjectConfig already detected invalid label functions correctly and
produced a corresponding user error. However then we failed with an
internal server error while trying to set null for the function of the
label, which is a non-nullable field. Fix this by not setting the
function in this case.

Release-Notes: skip
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Iba2202a019debcb4e85cf3a5dd0eabed31073567

java/com/google/gerrit/server/project/ProjectConfig.java

diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index a964ee1..df1ba61 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -1145,7 +1145,7 @@
                 "Invalid %s for label \"%s\". Valid names are: %s",
                 KEY_FUNCTION, name, Joiner.on(", ").join(LabelFunction.ALL.keySet())));
       }
-      label.setFunction(function.orElse(null));
+      function.ifPresent(label::setFunction);
       label.setCopyCondition(rc.getString(LABEL, name, KEY_COPY_CONDITION));
 
       if (!values.isEmpty()) {

javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java

diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
index 52207db..d5395f4 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectConfigIT.java
@@ -134,6 +134,28 @@
   }
 
   @Test
+  public void rejectCreatingLabelWithInvalidFunction() throws Exception {
+    fetchRefsMetaConfig();
+    PushOneCommit push =
+        pushFactory.create(
+            admin.newIdent(),
+            testRepo,
+            "Test Change",
+            ProjectConfig.PROJECT_CONFIG,
+            "[label \"Foo\"]\n  function = INVALID");
+    PushOneCommit.Result r = push.to(RefNames.REFS_CONFIG);
+    r.assertErrorStatus(
+        String.format("commit %s: invalid project configuration", abbreviateName(r.getCommit())));
+    r.assertMessage(
+        String.format(
+            "ERROR: commit %s: invalid project configuration:\n"
+                + "ERROR: commit %s:   project.config: Invalid function for label \"foo\"."
+                + " Valid names are: AnyWithBlock, MaxWithBlock, MaxNoBlock, NoBlock, NoOp,"
+                + " PatchSetLock",
+            abbreviateName(r.getCommit()), abbreviateName(r.getCommit())));
+  }
+
+  @Test
   public void rejectSettingCopyMinScore() throws Exception {
     testRejectSettingLabelFlag(
         LabelConfigValidator.KEY_COPY_MIN_SCORE, /* value= */ true, "is:MIN");