Remove ID from LabelType

Labels are completely uniquely identified by name; IDs were only left
so ID or label name would be recognized in PatchSetApproval's
categoryId to facilitate online data migration. By this commit,
migration must have finished.

Change-Id: I761cd39cf02fe8c63f246dd588500d53a4acc3b0

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

@@ -33,16 +33,6 @@ public class LabelType {
     return new LabelType(name, values);
   }
 
-  private static String checkId(String id) {
-    if ("SUBM".equals(id)) {
-      throw new IllegalArgumentException("Reserved label ID \"" + id + "\"");
-    }
-    if (id != null && id.length() > 4) {
-      throw new IllegalArgumentException("Illegal label ID \"" + id + "\"");
-    }
-    return id;
-  }
-
   private static String checkName(String name) {
     if ("SUBM".equals(name)) {
       throw new IllegalArgumentException(
@@ -102,7 +92,6 @@ public class LabelType {
   }
 
   protected String name;
-  protected String id;
 
   protected String abbreviatedName;
   protected String functionName;
@@ -143,20 +132,7 @@ public class LabelType {
     return name;
   }
 
-  @Deprecated
-  public String getId() {
-    return id;
-  }
-
-  @Deprecated
-  public void setId(String id) {
-    this.id = checkId(id);
-  }
-
   public boolean matches(PatchSetApproval psa) {
-    if (id != null && psa.getLabelId().get().equals(id)) {
-      return true;
-    }
     return psa.getLabelId().get().equalsIgnoreCase(name);
   }
 

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

@@ -26,7 +26,6 @@ import java.util.Map;
 public class LabelTypes {
   protected List<LabelType> labelTypes;
   private transient Map<String, LabelType> byLabel;
-  private transient Map<String, LabelType> byId;
   private transient Map<String, Integer> positions;
 
   protected LabelTypes() {
@@ -42,8 +41,7 @@ public class LabelTypes {
   }
 
   public LabelType byLabel(LabelId labelId) {
-    LabelType t = byId().get(labelId.get());
+    return byLabel().get(labelId.get().toLowerCase());
-    return t != null ? t : byLabel().get(labelId.get().toLowerCase());
   }
 
   public LabelType byLabel(String labelName) {
@@ -62,20 +60,6 @@ public class LabelTypes {
     return byLabel;
   }
 
-  private Map<String, LabelType> byId() {
-    if (byId == null) {
-      byId = new HashMap<String, LabelType>();
-      if (labelTypes != null) {
-        for (LabelType t : labelTypes) {
-          if (t.getId() != null) {
-            byId.put(t.getId(), t);
-          }
-        }
-      }
-    }
-    return byId;
-  }
-
   @Override
   public String toString() {
     return labelTypes.toString();

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

@@ -109,7 +109,6 @@ public class ProjectConfig extends VersionedMetaData {
   private static final String KEY_LOCAL_DEFAULT = "local-default";
 
   private static final String LABEL = "label";
-  private static final String KEY_ID = "id";
   private static final String KEY_ABBREVIATION = "abbreviation";
   private static final String KEY_FUNCTION = "function";
   private static final String KEY_COPY_MIN_SCORE = "copyMinScore";
@@ -565,7 +564,6 @@ public class ProjectConfig extends VersionedMetaData {
             "Invalid label \"%s\"", name)));
         continue;
       }
-      label.setId(rc.getString(LABEL, name, KEY_ID));
       String abbr = rc.getString(LABEL, name, KEY_ABBREVIATION);
       if (abbr != null) {
         label.setAbbreviatedName(abbr);

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_53.java

@@ -29,7 +29,6 @@ import static com.google.gerrit.common.data.Permission.SUBMIT;
 import com.google.gerrit.common.data.AccessSection;
 import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.data.LabelType;
-import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRule;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -44,6 +43,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.schema.Schema_77.LegacyLabelTypes;
 import com.google.gwtorm.jdbc.JdbcSchema;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -70,7 +70,7 @@ class Schema_53 extends SchemaVersion {
 
   private SystemConfig systemConfig;
   private Map<AccountGroup.Id, GroupReference> groupMap;
-  private LabelTypes labelTypes;
+  private LegacyLabelTypes labelTypes;
   private GroupReference projectOwners;
 
   private Map<Project.NameKey, Project.NameKey> parentsByProject;

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_77.java

@@ -14,10 +14,14 @@
 
 package com.google.gerrit.server.schema;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
 import com.google.gerrit.common.data.LabelValue;
+import com.google.gerrit.reviewdb.client.PatchSetApproval.LabelId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.config.AllProjectsName;
@@ -64,7 +68,7 @@ public class Schema_77 extends SchemaVersion {
   @Override
   protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException {
     try {
-      LabelTypes labelTypes = getLegacyTypes(db);
+      LegacyLabelTypes labelTypes = getLegacyTypes(db);
       SqlDialect dialect = ((JdbcSchema) db).getDialect();
       if (dialect instanceof DialectH2) {
         alterTable(db, "ALTER TABLE %s ALTER COLUMN %s varchar(255)");
@@ -100,9 +104,9 @@ public class Schema_77 extends SchemaVersion {
     }
   }
 
-  private void migrateLabelsToAllProjects(ReviewDb db, LabelTypes labelTypes)
+  private void migrateLabelsToAllProjects(ReviewDb db,
-      throws SQLException, RepositoryNotFoundException, IOException,
+      LegacyLabelTypes labelTypes) throws SQLException,
-      ConfigInvalidException {
+      RepositoryNotFoundException, IOException, ConfigInvalidException {
     Repository git = mgr.openRepository(allProjects);
 
     try {
@@ -114,10 +118,7 @@ public class Schema_77 extends SchemaVersion {
       ProjectConfig config = ProjectConfig.read(md);
       Map<String, LabelType> configTypes = config.getLabelSections();
       List<LabelType> newTypes = Lists.newArrayList();
-      for (LabelType type : labelTypes.getLabelTypes()) {
+      for (LegacyLabelType type : labelTypes.getLegacyLabelTypes()) {
-        // Don't include IDs for this migration, since we are also updating all
-        // existing PatchSetApprovals.
-        type.setId(null);
         if (!configTypes.containsKey(type.getName())) {
           newTypes.add(type);
         }
@@ -134,12 +135,12 @@ public class Schema_77 extends SchemaVersion {
     }
   }
 
-  private void migratePatchSetApprovals(ReviewDb db, LabelTypes labelTypes)
+  private void migratePatchSetApprovals(ReviewDb db,
-      throws SQLException {
+      LegacyLabelTypes labelTypes) throws SQLException {
     PreparedStatement stmt = ((JdbcSchema) db).getConnection().prepareStatement(
         "UPDATE patch_set_approvals SET category_id = ? WHERE category_id = ?");
     try {
-      for (LabelType type : labelTypes.getLabelTypes()) {
+      for (LegacyLabelType type : labelTypes.getLegacyLabelTypes()) {
         stmt.setString(1, type.getName());
         stmt.setString(2, type.getId());
         stmt.addBatch();
@@ -150,8 +151,54 @@ public class Schema_77 extends SchemaVersion {
     }
   }
 
-  static LabelTypes getLegacyTypes(ReviewDb db) throws SQLException {
+  static class LegacyLabelType extends LabelType {
-    List<LabelType> types = Lists.newArrayListWithCapacity(2);
+    private String id;
+
+    private LegacyLabelType(String name, List<LabelValue> values) {
+      super(name, values);
+    }
+
+    String getId() {
+      return id;
+    }
+
+    private void setId(String id) {
+      checkArgument(id.length() <= 4, "Invalid legacy label ID: \"%s\"", id);
+      this.id = id;
+    }
+  }
+
+  static class LegacyLabelTypes extends LabelTypes {
+    private final List<LegacyLabelType> legacyTypes;
+
+    private final Map<String, LegacyLabelType> byId;
+
+    private LegacyLabelTypes(List<LegacyLabelType> types) {
+      super(types);
+      legacyTypes = types;
+      byId = Maps.newHashMap();
+      for (LegacyLabelType type : types) {
+        byId.put(type.getId(), type);
+      }
+    }
+
+    List<LegacyLabelType> getLegacyLabelTypes() {
+      return legacyTypes;
+    }
+
+    @Override
+    public LegacyLabelType byLabel(LabelId labelId) {
+      LegacyLabelType t = byId.get(labelId.get());
+      return t != null ? t : (LegacyLabelType) super.byLabel(labelId);
+    }
+
+    LegacyLabelType byId(LabelId id) {
+      return byId.get(id.get());
+    }
+  }
+
+  static LegacyLabelTypes getLegacyTypes(ReviewDb db) throws SQLException {
+    List<LegacyLabelType> types = Lists.newArrayListWithCapacity(2);
     Statement catStmt = null;
     PreparedStatement valStmt = null;
     ResultSet catRs = null;
@@ -179,8 +226,8 @@ public class Schema_77 extends SchemaVersion {
         } finally {
           valRs.close();
         }
-        LabelType type =
+        LegacyLabelType type =
-            new LabelType(getLabelName(catRs.getString("name")), values);
+            new LegacyLabelType(getLabelName(catRs.getString("name")), values);
         type.setId(id);
         type.setAbbreviatedName(catRs.getString("abbreviated_name"));
         type.setFunctionName(catRs.getString("function_name"));
@@ -198,7 +245,7 @@ public class Schema_77 extends SchemaVersion {
         catStmt.close();
       }
     }
-    return new LabelTypes(types);
+    return new LegacyLabelTypes(types);
   }
 
   private static String getLabelName(String name) {