Merge "Drop LabelType.getPosition()"

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

@@ -37,7 +37,6 @@ public class LabelType {
     lt.setAbbreviatedName(ac.getAbbreviatedName());
     lt.setFunctionName(ac.getFunctionName());
     lt.setCopyMinScore(ac.isCopyMinScore());
-    lt.setPosition(ac.getPosition());
     return lt;
   }
 
@@ -92,7 +91,6 @@ public class LabelType {
   protected String abbreviatedName;
   protected String functionName;
   protected boolean copyMinScore;
-  protected short position;
 
   protected List<LabelValue> values;
   protected short maxNegative;
@@ -153,14 +151,6 @@ public class LabelType {
     this.functionName = functionName;
   }
 
-  public short getPosition() {
-    return position;
-  }
-
-  public void setPosition(short position) {
-    this.position = position;
-  }
-
   public List<LabelValue> getValues() {
     return values;
   }

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.common.data;
 
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -22,6 +23,7 @@ public class LabelTypes {
   protected List<LabelType> labelTypes;
   private transient Map<String, LabelType> byId;
   private transient Map<String, LabelType> byLabel;
+  private transient Map<String, Integer> positions;
 
   protected LabelTypes() {
   }
@@ -71,4 +73,38 @@ public class LabelTypes {
   public String toString() {
     return labelTypes.toString();
   }
+
+  public Comparator<String> nameComparator() {
+    final Map<String, Integer> positions = positions();
+    return new Comparator<String>() {
+      @Override
+      public int compare(String left, String right) {
+        int lp = position(left);
+        int rp = position(right);
+        int cmp = lp - rp;
+        if (cmp == 0) {
+          cmp = left.compareTo(right);
+        }
+        return cmp;
+      }
+
+      private int position(String name) {
+        Integer p = positions.get(name);
+        return p != null ? p : positions.size();
+      }
+    };
+  }
+
+  private Map<String, Integer> positions() {
+    if (positions == null) {
+      positions = new HashMap<String, Integer>();
+      if (labelTypes != null) {
+        int i = 0;
+        for (LabelType t : labelTypes) {
+          positions.put(t.getName(), i++);
+        }
+      }
+    }
+    return positions;
+  }
 }

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

@@ -352,7 +352,7 @@ public class ChangeJson {
       LabelTypes labelTypes, boolean standard) throws OrmException {
     // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
     Map<String, LabelInfo> labels =
-        new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes));
+        new TreeMap<String, LabelInfo>(labelTypes.nameComparator());
     for (SubmitRecord rec : submitRecords(cd)) {
       if (rec.labels == null) {
         continue;
@@ -485,7 +485,7 @@ public class ChangeJson {
     //
     // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
     Map<String, LabelInfo> labels =
-        new TreeMap<String, LabelInfo>(LabelOrdering.create(labelTypes));
+        new TreeMap<String, LabelInfo>(labelTypes.nameComparator());
     for (ApprovalCategory.Id id : categories) {
       LabelType type = labelTypes.byId(id.get());
       if (type != null) {

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

@@ -1,36 +0,0 @@
-// 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.change;
-
-import com.google.common.base.Function;
-import com.google.common.collect.Ordering;
-import com.google.gerrit.common.data.LabelType;
-import com.google.gerrit.common.data.LabelTypes;
-
-class LabelOrdering {
-  public static Ordering<String> create(final LabelTypes labelTypes) {
-    return Ordering.natural().nullsLast().onResultOf(
-        new Function<String, Short>() {
-          @Override
-          public Short apply(String n) {
-            LabelType lt = labelTypes.byLabel(n);
-            return lt != null ? lt.getPosition() : null;
-          }
-        }).compound(Ordering.natural());
-  }
-
-  private LabelOrdering() {
-  }
-}

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

@@ -88,8 +88,7 @@ public class ReviewerJson {
     }
 
     // Don't use Maps.newTreeMap(Comparator) due to OpenJDK bug 100167.
-    out.approvals = new TreeMap<String,String>(LabelOrdering.create(
+    out.approvals = new TreeMap<String,String>(labelTypes.nameComparator());
-        labelTypes));
     for (PatchSetApproval ca : approvals) {
       for (PermissionRange pr : ctl.getLabelRanges()) {
         if (!pr.isEmpty()) {

gerrit-server/src/test/java/com/google/gerrit/rules/GerritCommonTest.java

@@ -39,13 +39,13 @@ public class GerritCommonTest extends PrologTestCase {
 
     LabelTypes types =
         new LabelTypes(Arrays.asList(
-            category(0, "CRVW", "Code-Review",
+            category("CRVW", "Code-Review",
                 value(2, "Looks good to me, approved"),
                 value(1, "Looks good to me, but someone else must approve"),
                 value(0, "No score"),
                 value(-1, "I would prefer that you didn't submit this"),
                 value(-2, "Do not submit")),
-            category(1, "VRIF", "Verified", value(1, "Verified"),
+            category("VRIF", "Verified", value(1, "Verified"),
                 value(0, "No score"), value(-1, "Fails"))));
     ProjectConfig config = new ProjectConfig(new Project.NameKey("myproject"));
     config.createInMemory();
@@ -71,11 +71,9 @@ public class GerritCommonTest extends PrologTestCase {
     return new LabelValue((short) value, text);
   }
 
-  private static LabelType category(int pos, String id, String name,
+  private static LabelType category(String id, String name,
       LabelValue... values) {
-    LabelType type = new LabelType(id, name, Arrays.asList(values));
+    return new LabelType(id, name, Arrays.asList(values));
-    type.setPosition((short) pos);
-    return type;
   }
 
   private static class Projects implements ProjectCache {