Model group ids as indexes

4 files changed, 61 insertions, 29 deletions

diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
index 1bf690749bb..d8ad7295b1c 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterMembership.java
@@ -31,7 +31,7 @@ public class ClusterMembership {
 
         String[] components = restValue.split("/");
 
-        if ( components.length == 3)
+        if ( components.length == 3) // Aug 2016: This should never happen any more
             initWithoutGroup(components, dockerImage);
         else if (components.length == 4)
             initWithGroup(components, dockerImage);
@@ -50,20 +50,21 @@ public class ClusterMembership {
     }
 
     private void initWithoutGroup(String[] components, Optional<String> dockerImage) {
-        this.cluster = ClusterSpec.from(ClusterSpec.Type.valueOf(components[0]), ClusterSpec.Id.from(components[1]),
-                                        Optional.empty(), dockerImage);
+        this.cluster = ClusterSpec.request(ClusterSpec.Type.valueOf(components[0]), 
+                                           ClusterSpec.Id.from(components[1]),
+                                           dockerImage);
         this.index = Integer.parseInt(components[2]);
     }
 
     private void initWithGroup(String[] components, Optional<String> dockerImage) {
         this.cluster = ClusterSpec.from(ClusterSpec.Type.valueOf(components[0]), ClusterSpec.Id.from(components[1]),
-                                        Optional.of(ClusterSpec.Group.from(components[2])), dockerImage);
+                                        ClusterSpec.Group.from(Integer.valueOf(components[2])), dockerImage);
         this.index = Integer.parseInt(components[3]);
     }
 
     protected String toStringValue() {
         return cluster.type().name() + "/" + cluster.id().value() +
-                ( cluster.group().isPresent() ? "/" + cluster.group().get().value() : "") + "/" + index +
+                ( cluster.group().isPresent() ? "/" + cluster.group().get().index() : "") + "/" + index +
                 ( retired ? "/retired" : "");
     }
 
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
index da6c6baa417..706e1313ddd 100644
--- a/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
+++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ClusterSpec.java
@@ -14,7 +14,10 @@ public final class ClusterSpec {
 
     private final Type type;
     private final Id id;
+
+    /** The group id of these hosts, or empty if this is represents a request for hosts */
     private final Optional<Group> groupId;
+
     private final Optional<String> dockerImage;
 
     private ClusterSpec(Type type, Id id, Optional<Group> groupId, Optional<String> dockerImage) {
@@ -37,18 +40,34 @@ public final class ClusterSpec {
 
     public ClusterSpec changeGroup(Optional<Group> newGroup) { return new ClusterSpec(type, id, newGroup, dockerImage); }
 
+    /** @deprecated pass a docker image or empty. TODO: Remove when no model older than 6.29 is in use */
+    @Deprecated
     public static ClusterSpec from(Type type, Id id) {
         return new ClusterSpec(type, id, Optional.empty(), Optional.empty());
     }
 
+    /** @deprecated either pass a group or not. TODO: Remove when no model older than 6.29 is in use */
+    @Deprecated 
     public static ClusterSpec from(Type type, Id id, Optional<Group> groupId) {
         return new ClusterSpec(type, id, groupId, Optional.empty());
     }
 
+    /** @deprecated pass a docker image or empty. TODO: Remove when no model older than 6.29 is in use */
+    @Deprecated
     public static ClusterSpec from(Type type, Id id, Optional<Group> groupId, Optional<String> dockerImage) {
         return new ClusterSpec(type, id, groupId, dockerImage);
     }
 
+    /** Create a specification <b>specifying</b> an existing cluster group having these attributes */
+    public static ClusterSpec from(Type type, Id id, Group groupId, Optional<String> dockerImage) {
+        return new ClusterSpec(type, id, Optional.of(groupId), dockerImage);
+    }
+
+    /** Create a specification <b>requesting</b> a cluster with these attributes */
+    public static ClusterSpec request(Type type, Id id, Optional<String> dockerImage) {
+        return new ClusterSpec(type, id, Optional.empty(), dockerImage);
+    }
+
     @Override
     public String toString() {
         return type + " " + id + (groupId.isPresent() ? " " + groupId.get() : "");
@@ -132,33 +151,45 @@ public final class ClusterSpec {
     }
 
     /** Identifier of a group within a cluster */
+    @SuppressWarnings("deprecation")
     public static final class Group {
 
-        private final String id;
+        private final int index;
 
+        /** @deprecated pass a group index instead. TODO: Remove when no older config models than 6.29 remains */
+        @Deprecated
         public Group(String id) {
-            Objects.requireNonNull(id, "Group id cannot be null");
-            this.id = id;
+            this(Integer.parseInt(id));
         }
-
-        public static Group from(String id) {
-            return new Group(id);
+        
+        private Group(int index) {
+            this.index = index;
         }
 
-        public String value() { return id; }
+        /** @deprecated pass a group index instead. TODO: Remove when no older config models than 6.29 remains */
+        @Deprecated
+        public static Group from(String id) { return new Group(id); }
+        
+        public static Group from(int index) { return new Group(index); }
+
+        /** @deprecated use index() instead. TODO: Remove when no older config models than 6.29 remains */
+        @Deprecated
+        public String value() { return String.valueOf(index); }
+        
+        public int index() { return index; }
 
         @Override
-        public String toString() { return "group '" + id + "'"; }
+        public String toString() { return "group " + index; }
 
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
-            return ((Group)o).id.equals(this.id);
+            return ((Group)o).index == this.index;
         }
 
         @Override
-        public int hashCode() { return id.hashCode(); }
+        public int hashCode() { return index; }
 
     }
 
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
index 6c2565ba3a0..7cf0abecc2d 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterMembershipTest.java
@@ -16,7 +16,7 @@ public class ClusterMembershipTest {
 
     @Test
     public void testContainerServiceInstance() {
-        ClusterSpec cluster = ClusterSpec.from(ClusterSpec.Type.container, ClusterSpec.Id.from("id1"), Optional.empty());
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.container, ClusterSpec.Id.from("id1"), Optional.empty());
         assertContainerService(ClusterMembership.from(cluster, 3));
     }
 
@@ -27,7 +27,7 @@ public class ClusterMembershipTest {
 
     @Test
     public void testServiceInstance() {
-        ClusterSpec cluster = ClusterSpec.from(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"), Optional.empty());
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"), Optional.empty());
         assertContentService(ClusterMembership.from(cluster, 37));
     }
 
@@ -39,18 +39,18 @@ public class ClusterMembershipTest {
     @Test
     public void testServiceInstanceWithGroup() {
         ClusterSpec cluster = ClusterSpec.from(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"),
-                                               Optional.of(ClusterSpec.Group.from("gr4")));
+                                               ClusterSpec.Group.from(4), Optional.empty());
         assertContentServiceWithGroup(ClusterMembership.from(cluster, 37));
     }
 
     @Test
     public void testServiceInstanceWithGroupFromString() {
-        assertContentServiceWithGroup(ClusterMembership.from("content/id1/gr4/37", Optional.empty()));
+        assertContentServiceWithGroup(ClusterMembership.from("content/id1/4/37", Optional.empty()));
     }
 
     @Test
     public void testServiceInstanceWithRetire() {
-        ClusterSpec cluster = ClusterSpec.from(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"), Optional.empty());
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"), Optional.empty());
         assertContentServiceWithRetire(ClusterMembership.retiredFrom(cluster, 37));
     }
 
@@ -62,13 +62,13 @@ public class ClusterMembershipTest {
     @Test
     public void testServiceInstanceWithGroupAndRetire() {
         ClusterSpec cluster = ClusterSpec.from(ClusterSpec.Type.content, ClusterSpec.Id.from("id1"),
-                Optional.of(ClusterSpec.Group.from("gr4")));
+                                               ClusterSpec.Group.from(4), Optional.empty());
         assertContentServiceWithGroupAndRetire(ClusterMembership.retiredFrom(cluster, 37));
     }
 
     @Test
     public void testServiceInstanceWithGroupAndRetireFromString() {
-        assertContentServiceWithGroupAndRetire(ClusterMembership.from("content/id1/gr4/37/retired", Optional.empty()));
+        assertContentServiceWithGroupAndRetire(ClusterMembership.from("content/id1/4/37/retired", Optional.empty()));
     }
 
     private void assertContainerService(ClusterMembership instance) {
@@ -91,10 +91,10 @@ public class ClusterMembershipTest {
     private void assertContentServiceWithGroup(ClusterMembership instance) {
         assertEquals(ClusterSpec.Type.content, instance.cluster().type());
         assertEquals("id1", instance.cluster().id().value());
-        assertEquals("gr4", instance.cluster().group().get().value());
+        assertEquals(4, instance.cluster().group().get().index());
         assertEquals(37, instance.index());
         assertFalse(instance.retired());
-        assertEquals("content/id1/gr4/37", instance.stringValue());
+        assertEquals("content/id1/4/37", instance.stringValue());
     }
 
     private void assertContentServiceWithRetire(ClusterMembership instance) {
@@ -108,10 +108,10 @@ public class ClusterMembershipTest {
     private void assertContentServiceWithGroupAndRetire(ClusterMembership instance) {
         assertEquals(ClusterSpec.Type.content, instance.cluster().type());
         assertEquals("id1", instance.cluster().id().value());
-        assertEquals("gr4", instance.cluster().group().get().value());
+        assertEquals(4, instance.cluster().group().get().index());
         assertEquals(37, instance.index());
         assertTrue(instance.retired());
-        assertEquals("content/id1/gr4/37/retired", instance.stringValue());
+        assertEquals("content/id1/4/37/retired", instance.stringValue());
     }
 
 }
diff --git a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
index d51d97e36c0..3b07b79a5f2 100644
--- a/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
+++ b/config-provisioning/src/test/java/com/yahoo/config/provision/ClusterSpecTest.java
@@ -21,9 +21,9 @@ public class ClusterSpecTest {
     @Test
     public void testGroupEquals() {
         new EqualsTester()
-                .addEqualityGroup(ClusterSpec.Group.from("id1"), ClusterSpec.Group.from("id1"))
-                .addEqualityGroup(ClusterSpec.Group.from("id2"))
-                .addEqualityGroup(ClusterSpec.Group.from("id3"))
+                .addEqualityGroup(ClusterSpec.Group.from(1), ClusterSpec.Group.from(1))
+                .addEqualityGroup(ClusterSpec.Group.from(2))
+                .addEqualityGroup(ClusterSpec.Group.from(3))
                 .testEquals();
     }