Consolidate parsing and formatting for label votes

We currently support two styles of parsing, Label-Value and
Label=Value. Consolidate these into a single LabelVote class.
(Unfortunately for backwards-compatibility reasons we can't just
eliminate Label=Value in ReviewCommand.)

Change-Id: I9e5255b184d786226e6b11fe059001608c00673d

gerrit-common/src/main/java/com/google/gerrit/common/data/LabelType.java

@@ -33,14 +33,18 @@ public class LabelType {
     return new LabelType(name, values);
   }
 
-  private static String checkName(String name) {
+  public static String checkName(String name) {
+    if (name == null || name.isEmpty()) {
+      throw new IllegalArgumentException("Empty label name");
+    }
     if ("SUBM".equals(name)) {
       throw new IllegalArgumentException(
           "Reserved label name \"" + name + "\"");
     }
     for (int i = 0; i < name.length(); i++) {
       char c = name.charAt(i);
-      if (!((c >= 'a' && c <= 'z') ||
+      if ((i == 0 && c == '-') ||
+          !((c >= 'a' && c <= 'z') ||
             (c >= 'A' && c <= 'Z') ||
             (c >= '0' && c <= '9') ||
             c == '-')) {

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -49,6 +49,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountsCollection;
 import com.google.gerrit.server.index.ChangeIndexer;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.server.util.TimeUtil;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -387,7 +388,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
         // User requested delete of this label.
         if (c != null) {
           if (c.getValue() != 0) {
-            labelDelta.add("-" + normName);
+            addLabelDelta(normName, (short) 0);
           }
           del.add(c);
         }
@@ -395,7 +396,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
         c.setValue(ent.getValue());
         c.setGranted(timestamp);
         upd.add(c);
-        labelDelta.add(format(normName, c.getValue()));
+        addLabelDelta(normName, c.getValue());
         categories.put(normName, c.getValue());
       } else if (c != null && c.getValue() == ent.getValue()) {
         current.put(normName, c);
@@ -407,7 +408,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
             ent.getValue(), TimeUtil.nowTs());
         c.setGranted(timestamp);
         ins.add(c);
-        labelDelta.add(format(normName, c.getValue()));
+        addLabelDelta(normName, c.getValue());
         categories.put(normName, c.getValue());
       }
     }
@@ -467,14 +468,8 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
     return current;
   }
 
-  private static String format(String name, short value) {
-    StringBuilder sb = new StringBuilder(name.length() + 2);
-    sb.append(name);
-    if (value >= 0) {
-      sb.append('+');
-    }
-    sb.append(value);
-    return sb.toString();
+  private void addLabelDelta(String name, short value) {
+    labelDelta.add(new LabelVote(name, value).format());
   }
 
   private boolean insertMessage(RevisionResource rsrc, String msg)

gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java

@@ -23,6 +23,7 @@ import com.google.gerrit.server.project.ChangeControl;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.query.OrPredicate;
 import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.util.LabelVote;
 import com.google.inject.Provider;
 
 import java.util.List;
@@ -75,25 +76,39 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
       ProjectCache projectCache, ChangeControl.GenericFactory ccFactory,
       IdentifiedUser.GenericFactory userFactory, Provider<ReviewDb> dbProvider,
       String value, Set<Account.Id> accounts, AccountGroup.UUID group) {
-    String label;
-    Test test;
-    int expVal;
-    Matcher m1 = Pattern.compile("(=|>=|<=)([+-]?\\d+)$").matcher(value);
-    Matcher m2 = Pattern.compile("([+-]\\d+)$").matcher(value);
-    if (m1.find()) {
-      label = value.substring(0, m1.start());
-      test = Test.op(m1.group(1));
-      expVal = value(m1.group(2));
+    String label = null;
+    Test test = null;
+    int expVal = 0;
 
-    } else if (m2.find()) {
-      label = value.substring(0, m2.start());
+    try {
+      LabelVote v = LabelVote.parse(value);
       test = Test.EQ;
-      expVal = value(m2.group(1));
+      label = v.getLabel();
+      expVal = v.getValue();
+    } catch (IllegalArgumentException e) {
+      // Try next format.
+    }
 
-    } else {
-      label = value;
+    try {
+      LabelVote v = LabelVote.parseWithEquals(value);
       test = Test.EQ;
-      expVal = 1;
+      label = v.getLabel();
+      expVal = v.getValue();
+    } catch (IllegalArgumentException e) {
+      // Try next format.
+    }
+
+    if (label == null) {
+      Matcher m = Pattern.compile("(>=|<=)([+-]?\\d+)$").matcher(value);
+      if (m.find()) {
+        label = value.substring(0, m.start());
+        test = Test.op(m.group(1));
+        expVal = value(m.group(2));
+      } else {
+        label = value;
+        test = Test.EQ;
+        expVal = 1;
+      }
     }
 
     List<Predicate<ChangeData>> r = Lists.newArrayListWithCapacity(2 * MAX_LABEL_VALUE);

gerrit-server/src/main/java/com/google/gerrit/server/util/LabelVote.java

@@ -0,0 +1,109 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.util;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.common.base.Objects;
+import com.google.common.base.Strings;
+import com.google.gerrit.common.data.LabelType;
+
+/** A single vote on a label, consisting of a label name and a value. */
+public class LabelVote {
+  public static LabelVote parse(String text) {
+    checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
+    if (text.charAt(0) == '-') {
+      return new LabelVote(text.substring(1), (short) 0);
+    }
+    short sign = 0;
+    int i;
+    for (i = text.length() - 1; i >= 0; i--) {
+      int c = text.charAt(i);
+      if (c == '-') {
+        sign = (short) -1;
+        break;
+      } else if (c == '+') {
+        sign = (short) 1;
+        break;
+      } else if (!('0' <= c && c <= '9')) {
+        break;
+      }
+    }
+    if (sign == 0) {
+      return new LabelVote(text, (short) 1);
+    }
+    return new LabelVote(text.substring(0, i),
+        (short)(sign * Short.parseShort(text.substring(i + 1))));
+  }
+
+  public static LabelVote parseWithEquals(String text) {
+    checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
+    int e = text.lastIndexOf('=');
+    checkArgument(e >= 0, "Label vote missing '=': %s", text);
+    return new LabelVote(text.substring(0, e),
+        Short.parseShort(text.substring(e + 1), text.length()));
+  }
+
+  private final String name;
+  private final short value;
+
+  public LabelVote(String name, short value) {
+    this.name = LabelType.checkName(name);
+    this.value = value;
+  }
+
+  public String getLabel() {
+    return name;
+  }
+
+  public short getValue() {
+    return value;
+  }
+
+  public String format() {
+    if (value == (short) 0) {
+      return '-' + name;
+    } else if (value == (short) 1) {
+      return name;
+    } else if (value < 0) {
+      return name + value;
+    } else {
+      return name + '+' + value;
+    }
+  }
+
+  public String formatWithEquals() {
+    if (value <= (short) 0) {
+      return name + '=' + value;
+    } else {
+      return name + "=+" + value;
+    }
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (o instanceof LabelVote) {
+      LabelVote l = (LabelVote) o;
+      return Objects.equal(name, l.name)
+          && value == l.value;
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return format();
+  }
+}

gerrit-server/src/test/java/com/google/gerrit/server/util/LabelVoteTest.java

@@ -0,0 +1,90 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.util;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public class LabelVoteTest {
+  @Test
+  public void parse() {
+    LabelVote l;
+    l = LabelVote.parse("Code-Review-2");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) -2, l.getValue());
+    l = LabelVote.parse("Code-Review-1");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) -1, l.getValue());
+    l = LabelVote.parse("-Code-Review");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 0, l.getValue());
+    l = LabelVote.parse("Code-Review");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 1, l.getValue());
+    l = LabelVote.parse("Code-Review+2");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 2, l.getValue());
+  }
+
+  @Test
+  public void format() {
+    assertEquals("Code-Review-2", LabelVote.parse("Code-Review-2").format());
+    assertEquals("Code-Review-1", LabelVote.parse("Code-Review-1").format());
+    assertEquals("-Code-Review", LabelVote.parse("-Code-Review").format());
+    assertEquals("Code-Review", LabelVote.parse("Code-Review").format());
+    assertEquals("Code-Review+2", LabelVote.parse("Code-Review+2").format());
+  }
+
+  @Test
+  public void parseWithEquals() {
+    LabelVote l;
+    l = LabelVote.parseWithEquals("Code-Review=-2");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) -2, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=-1");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) -1, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=0");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 0, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=1");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 1, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=+1");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 1, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=2");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 2, l.getValue());
+    l = LabelVote.parseWithEquals("Code-Review=+2");
+    assertEquals("Code-Review", l.getLabel());
+    assertEquals((short) 2, l.getValue());
+  }
+
+  @Test
+  public void formatWithEquals() {
+    assertEquals("Code-Review=-2",
+        LabelVote.parseWithEquals("Code-Review=-2").formatWithEquals());
+    assertEquals("Code-Review=-1",
+        LabelVote.parseWithEquals("Code-Review=-1").formatWithEquals());
+    assertEquals("Code-Review=0",
+        LabelVote.parseWithEquals("Code-Review=0").formatWithEquals());
+    assertEquals("Code-Review=+1",
+        LabelVote.parseWithEquals("Code-Review=+1").formatWithEquals());
+    assertEquals("Code-Review=+2",
+        LabelVote.parseWithEquals("Code-Review=+2").formatWithEquals());
+  }
+}

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java

@@ -14,9 +14,7 @@
 
 package com.google.gerrit.sshd.commands;
 
-import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelValue;
@@ -41,6 +39,7 @@ import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
 import com.google.gerrit.server.project.NoSuchProjectException;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.sshd.CommandMetaData;
 import com.google.gerrit.sshd.SshCommand;
 import com.google.gerrit.util.cli.CmdLineParser;
@@ -117,18 +116,8 @@ public class ReviewCommand extends SshCommand {
 
   @Option(name = "--label", aliases = "-l", usage = "custom label(s) to assign", metaVar = "LABEL=VALUE")
   void addLabel(final String token) {
-    List<String> parts = ImmutableList.copyOf(Splitter.on('=').split(token));
-    if (parts.size() != 2) {
-      throw new IllegalArgumentException("invalid custom label " + token);
-    }
-    short value;
-    try {
-      value = Short.parseShort(parts.get(1));
-    } catch (IllegalArgumentException e) {
-      throw new IllegalArgumentException("invalid custom label value "
-          + parts.get(1));
-    }
-    customLabels.put(parts.get(0), value);
+    LabelVote v = LabelVote.parse(token);
+    customLabels.put(v.getLabel(), v.getValue());
   }
 
   @Inject