CreateLabel/SetLabel: Reject duplicate values

Formatting a label as JSON fails if it has duplicate values [1].

Likely it's still possible to create a label with duplicate values by
pushing to refs/meta/config. We should add a check there too, but that
can be done in a follow-up change.

[1]
java.lang.IllegalStateException: Duplicate key 1 (attempted merging values Looks Good and Looks Good)
        at java.util.stream.Collectors.duplicateKeyException(Collectors.java:131)
        at java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:178)
        at java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
        at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1383)
        at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
        at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
        at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
        at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
        at com.google.gerrit.server.project.ProjectJson.format(ProjectJson.java:52)
        at com.google.gerrit.server.restapi.project.GetProject.apply(GetProject.java:37)
        at com.google.gerrit.server.restapi.project.GetProject.apply(GetProject.java:25)
        at com.google.gerrit.httpd.restapi.RestApiServlet.lambda$invokeRestReadViewWithRetry$3(RestApiServlet.java:723)
        at com.github.rholder.retry.AttemptTimeLimiters$NoAttemptTimeLimit.call(AttemptTimeLimiters.java:78)
        at com.github.rholder.retry.Retryer.call(Retryer.java:160)
        at com.google.gerrit.server.update.RetryHelper.executeWithTimeoutCount(RetryHelper.java:417)
        at com.google.gerrit.server.update.RetryHelper.executeWithAttemptAndTimeoutCount(RetryHelper.java:368)
        at com.google.gerrit.server.update.RetryHelper.execute(RetryHelper.java:271)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestEndpointWithRetry(RestApiServlet.java:820)
        at com.google.gerrit.httpd.restapi.RestApiServlet.invokeRestReadViewWithRetry(RestApiServlet.java:718)
        at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:501)
        at javax.servlet.http.HttpServlet.service(HttpServlet.java:717)
        ...

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I09c4bdd93f543f5f4da62da43c7825163c4e5523

java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java

@@ -24,10 +24,12 @@ import com.google.gerrit.exceptions.InvalidNameException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.server.project.RefPattern;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 
 public class LabelDefinitionInputParser {
   public static LabelFunction parseFunction(String functionString) throws BadRequestException {
@@ -39,6 +41,7 @@ public class LabelDefinitionInputParser {
   public static List<LabelValue> parseValues(Map<String, String> values)
       throws BadRequestException {
     List<LabelValue> valueList = new ArrayList<>();
+    Set<Short> allValues = new HashSet<>();
     for (Entry<String, String> e : values.entrySet()) {
       short value;
       try {
@@ -46,6 +49,9 @@ public class LabelDefinitionInputParser {
       } catch (NumberFormatException ex) {
         throw new BadRequestException("invalid value: " + e.getKey(), ex);
       }
+      if (!allValues.add(value)) {
+        throw new BadRequestException("duplicate value: " + value);
+      }
       String valueDescription = e.getValue().trim();
       if (valueDescription.isEmpty()) {
         throw new BadRequestException("description for value '" + e.getKey() + "' cannot be empty");

javatests/com/google/gerrit/acceptance/rest/project/CreateLabelIT.java

@@ -168,6 +168,21 @@ public class CreateLabelIT extends AbstractDaemonTest {
     assertThat(thrown).hasMessageThat().contains("description for value '+1' cannot be empty");
   }
 
+  @Test
+  public void cannotCreateLabelWithDuplicateValues() throws Exception {
+    LabelDefinitionInput input = new LabelDefinitionInput();
+    // Positive values can be specified as '<value>' or '+<value>'.
+    input.values =
+        ImmutableMap.of(
+            "+1", "Looks Good", "1", "Looks Good", "0", "Don't Know", "-1", "Looks Bad");
+
+    BadRequestException thrown =
+        assertThrows(
+            BadRequestException.class,
+            () -> gApi.projects().name(allProjects.get()).label("Foo").create(input));
+    assertThat(thrown).hasMessageThat().contains("duplicate value: 1");
+  }
+
   @Test
   public void cannotCreateLabelWithInvalidDefaultValue() throws Exception {
     LabelDefinitionInput input = new LabelDefinitionInput();

javatests/com/google/gerrit/acceptance/rest/project/SetLabelIT.java

@@ -322,6 +322,21 @@ public class SetLabelIT extends AbstractDaemonTest {
     assertThat(thrown).hasMessageThat().contains("description for value '+1' cannot be empty");
   }
 
+  @Test
+  public void cannotSetDuplicateValues() throws Exception {
+    LabelDefinitionInput input = new LabelDefinitionInput();
+    // Positive values can be specified as '<value>' or '+<value>'.
+    input.values =
+        ImmutableMap.of(
+            "+1", "Looks Good", "1", "Looks Good", "0", "Don't Know", "-1", "Looks Bad");
+
+    BadRequestException thrown =
+        assertThrows(
+            BadRequestException.class,
+            () -> gApi.projects().name(allProjects.get()).label("Code-Review").update(input));
+    assertThat(thrown).hasMessageThat().contains("duplicate value: 1");
+  }
+
   @Test
   public void updateDefaultValue() throws Exception {
     LabelDefinitionInput input = new LabelDefinitionInput();