Remove RoleId and move serialisation etc to UserRoles

7 files changed, 193 insertions, 197 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java
new file mode 100644
index 00000000000..e37c866b97f
--- /dev/null
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/stubs/MockUserManagement.java
@@ -0,0 +1,5 @@
+package com.yahoo.vespa.hosted.controller.api.integration.stubs;
+
+public class MockUserManagement {
+
+}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java
deleted file mode 100644
index fea4892cd8f..00000000000
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleId.java
+++ /dev/null
@@ -1,114 +0,0 @@
-package com.yahoo.vespa.hosted.controller.api.integration.user;
-
-import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
-import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
-import com.yahoo.vespa.hosted.controller.api.role.Role;
-import com.yahoo.vespa.hosted.controller.api.role.Roles;
-import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
-
-import java.util.Objects;
-
-/**
- * An identifier for a role which users identified by {@link UserId}s can be members of, corresponding to a bound {@link Role}.
- *
- * @author jonmv
- */
-public class RoleId { // TODO jvenstad: Move serlialisation part to CloudRoles utility class.
-
-    private final String value;
-
-    private RoleId(String value) {
-        if (value.isBlank())
-            throw new IllegalArgumentException("Id value must be non-blank.");
-        this.value = value;
-    }
-
-    public static RoleId fromRole(TenantRole role) {
-        return new RoleId(valueOf(role));
-    }
-
-    public static RoleId fromRole(ApplicationRole role) {
-        return new RoleId(valueOf(role));
-    }
-
-    public static RoleId fromValue(String value) {
-        return new RoleId(value);
-    }
-
-    public String value() {
-        return value;
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        if (this == o) return true;
-        if (o == null || getClass() != o.getClass()) return false;
-        RoleId id = (RoleId) o;
-        return Objects.equals(value, id.value);
-    }
-
-    @Override
-    public int hashCode() {
-        return Objects.hash(value);
-    }
-
-    @Override
-    public String toString() {
-        return value;
-    }
-
-    /** Returns the {@link Role} this represent. */
-    public Role toRole(Roles roles) {
-        String[] parts = value.split("\\.");
-        if (parts.length == 2) switch (parts[1]) {
-            case "tenantOwner":    return roles.tenantOwner(TenantName.from(parts[0]));
-            case "tenantAdmin":    return roles.tenantAdmin(TenantName.from(parts[0]));
-            case "tenantOperator": return roles.tenantOperator(TenantName.from(parts[0]));
-        }
-        if (parts.length == 3) switch (parts[2]) {
-            case "applicationAdmin":     return roles.applicationAdmin(TenantName.from(parts[0]), ApplicationName.from(parts[1]));
-            case "applicationOperator":  return roles.applicationOperator(TenantName.from(parts[0]), ApplicationName.from(parts[1]));
-            case "applicationDeveloper": return roles.applicationDeveloper(TenantName.from(parts[0]), ApplicationName.from(parts[1]));
-            case "applicationReader":    return roles.applicationReader(TenantName.from(parts[0]), ApplicationName.from(parts[1]));
-        }
-        throw new IllegalArgumentException("Malformed or illegal role value '" + value + "'.");
-    }
-
-    private static String valueOf(TenantRole role) {
-        return valueOf(role.tenant()) + "." + valueOf(role.definition());
-    }
-
-    private static String valueOf(ApplicationRole role) {
-        return valueOf(role.tenant()) + "." + valueOf(role.application()) + "." + valueOf(role.definition());
-    }
-
-    private static String valueOf(TenantName tenant) {
-        if (tenant.value().contains("."))
-            throw new IllegalArgumentException("Tenant names may not contain '.'.");
-
-        return tenant.value();
-    }
-
-    private static String valueOf(ApplicationName application) {
-        if (application.value().contains("."))
-            throw new IllegalArgumentException("Application names may not contain '.'.");
-
-        return application.value();
-    }
-
-    private static String valueOf(RoleDefinition role) {
-        switch (role) {
-            case tenantOwner:          return "tenantOwner";
-            case tenantAdmin:          return "tenantAdmin";
-            case tenantOperator:       return "tenantOperator";
-            case applicationAdmin:     return "applicationAdmin";
-            case applicationOperator:  return "applicationOperator";
-            case applicationDeveloper: return "applicationDeveloper";
-            case applicationReader:    return "applicationReader";
-            default: throw new IllegalArgumentException("No value defined for role '" + role + "'.");
-        }
-    }
-
-}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserManagement.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserManagement.java
index 2ae8c0b7f31..ff25e779d30 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserManagement.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserManagement.java
@@ -6,28 +6,25 @@ import java.util.Collection;
 import java.util.List;
 
 /**
- * Management of {@link UserId}s and {@link RoleId}s, used for access control with {@link Role}s.
+ * Management of {@link UserId}s as members of {@link Role}s.
  *
  * @author jonmv
  */
 public interface UserManagement {
 
     /** Creates the given role, or throws if the role already exists. */
-    void createRole(RoleId role);
+    void createRole(Role role);
 
     /** Ensures the given role does not exist. */
-    void deleteRole(RoleId role);
+    void deleteRole(Role role);
 
     /** Ensures the given users exist, and are part of the given role, or throws if the role does not exist. */
-    void addUsers(RoleId role, Collection<UserId> users);
+    void addUsers(Role role, Collection<UserId> users);
 
     /** Ensures none of the given users are part of the given role, or throws if the role does not exist. */
-    void removeUsers(RoleId role, Collection<UserId> users);
-
-    /** Returns all known roles. */
-    default List<RoleId> listRoles() { throw new UnsupportedOperationException("To be removed."); }
+    void removeUsers(Role role, Collection<UserId> users);
 
     /** Returns all users in the given role, or throws if the role does not exist. */
-    List<UserId> listUsers(RoleId role);
+    List<UserId> listUsers(Role role);
 
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
new file mode 100644
index 00000000000..424f5a1d8a5
--- /dev/null
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRoles.java
@@ -0,0 +1,115 @@
+package com.yahoo.vespa.hosted.controller.api.integration.user;
+
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
+import com.yahoo.vespa.hosted.controller.api.role.Role;
+import com.yahoo.vespa.hosted.controller.api.role.RoleDefinition;
+import com.yahoo.vespa.hosted.controller.api.role.Roles;
+import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
+
+import java.util.List;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Validation, utility and serialization methods for roles used in user management.
+ *
+ * @author jonmv
+ */
+public class UserRoles {
+
+    private final Roles roles;
+
+    /** Creates a new UserRoles which can be used for serialisation and listing of bound user roles. */
+    public UserRoles(Roles roles) {
+        this.roles = requireNonNull(roles);
+    }
+
+    /** Returns the list of {@link TenantRole}s a {@link UserId} may be a member of. */
+    public List<TenantRole> tenantRoles(TenantName tenant) {
+        return List.of(roles.tenantOperator(tenant),
+                       roles.tenantAdmin(tenant),
+                       roles.tenantOwner(tenant));
+    }
+
+    /** Returns the list of {@link ApplicationRole}s a {@link UserId} may be a member of. */
+    public List<ApplicationRole> applicationRoles(TenantName tenant, ApplicationName application) {
+        return List.of(roles.applicationReader(tenant, application),
+                       roles.applicationDeveloper(tenant, application),
+                       roles.applicationOperator(tenant, application),
+                       roles.applicationAdmin(tenant, application));
+    }
+
+    /** Returns the {@link Role} the given value represents. */
+    public Role toRole(String value) {
+        String[] parts = value.split("\\.");
+        if (parts.length == 2) return toRole(TenantName.from(parts[0]), parts[1]);
+        if (parts.length == 3) return toRole(TenantName.from(parts[0]), ApplicationName.from(parts[1]), parts[2]);
+        throw new IllegalArgumentException("Malformed or illegal role value '" + value + "'.");
+    }
+
+    /** Returns the {@link Role} the given tenant and role names correspond to. */
+    public Role toRole(TenantName tenant, ApplicationName application, String roleName) {
+        switch (roleName) {
+            case "applicationAdmin": return roles.applicationAdmin(tenant, application);
+            case "applicationOperator": return roles.applicationOperator(tenant, application);
+            case "applicationDeveloper": return roles.applicationDeveloper(tenant, application);
+            case "applicationReader": return roles.applicationReader(tenant, application);
+            default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
+        }
+    }
+
+    /** Returns the {@link Role} the given tenant, application and role names correspond to. */
+    public Role toRole(TenantName tenant, String roleName) {
+        switch (roleName) {
+            case "tenantOwner": return roles.tenantOwner(tenant);
+            case "tenantAdmin": return roles.tenantAdmin(tenant);
+            case "tenantOperator": return roles.tenantOperator(tenant);
+            default: throw new IllegalArgumentException("Malformed or illegal role name '" + roleName + "'.");
+        }
+    }
+
+    /** Returns a serialised representation the given role. */
+    public static String valueOf(Role role) {
+        if (role instanceof TenantRole) return valueOf((TenantRole) role);
+        if (role instanceof ApplicationRole) return valueOf((ApplicationRole) role);
+        throw new IllegalArgumentException("Unexpected role type '" + role.getClass().getName() + "'.");
+    }
+
+    private static String valueOf(TenantRole role) {
+        return valueOf(role.tenant()) + "." + valueOf(role.definition());
+    }
+
+    private static String valueOf(ApplicationRole role) {
+        return valueOf(role.tenant()) + "." + valueOf(role.application()) + "." + valueOf(role.definition());
+    }
+
+    private static String valueOf(RoleDefinition role) {
+        switch (role) {
+            case tenantOwner:          return "tenantOwner";
+            case tenantAdmin:          return "tenantAdmin";
+            case tenantOperator:       return "tenantOperator";
+            case applicationAdmin:     return "applicationAdmin";
+            case applicationOperator:  return "applicationOperator";
+            case applicationDeveloper: return "applicationDeveloper";
+            case applicationReader:    return "applicationReader";
+            default: throw new IllegalArgumentException("No value defined for role '" + role + "'.");
+        }
+    }
+
+    private static String valueOf(TenantName tenant) {
+        if (tenant.value().contains("."))
+            throw new IllegalArgumentException("Tenant names may not contain '.'.");
+
+        return tenant.value();
+    }
+
+    private static String valueOf(ApplicationName application) {
+        if (application.value().contains("."))
+            throw new IllegalArgumentException("Application names may not contain '.'.");
+
+        return application.value();
+    }
+
+}
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
index 9811b26d873..bfdb0be4378 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Roles.java
@@ -19,7 +19,6 @@ public class Roles {
 
     private final SystemName system;
 
-
     @Inject
     public Roles(ZoneRegistry zones) {
         this(zones.system());
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleIdTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleIdTest.java
deleted file mode 100644
index 12adcbbae89..00000000000
--- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/RoleIdTest.java
+++ /dev/null
@@ -1,73 +0,0 @@
-package com.yahoo.vespa.hosted.controller.api.integration.user;
-
-import com.yahoo.config.provision.ApplicationName;
-import com.yahoo.config.provision.SystemName;
-import com.yahoo.config.provision.TenantName;
-import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
-import com.yahoo.vespa.hosted.controller.api.role.Roles;
-import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
-import org.junit.Test;
-
-import java.util.List;
-
-import static org.junit.Assert.assertEquals;
-
-/**
- * @author jonmv
- */
-public class RoleIdTest {
-
-    @Test
-    public void testSerialization() {
-        Roles roles = new Roles(SystemName.main);
-
-        TenantName tenant = TenantName.from("my-tenant");
-        for (TenantRole role : List.of(roles.tenantOwner(tenant),
-                                       roles.tenantAdmin(tenant),
-                                       roles.tenantOperator(tenant)))
-            assertEquals(role, RoleId.fromRole(role).toRole(roles));
-
-        ApplicationName application = ApplicationName.from("my-application");
-        for (ApplicationRole role : List.of(roles.applicationAdmin(tenant, application),
-                                            roles.applicationOperator(tenant, application),
-                                            roles.applicationDeveloper(tenant, application),
-                                            roles.applicationReader(tenant, application)))
-            assertEquals(role, RoleId.fromRole(role).toRole(roles));
-
-        assertEquals(roles.tenantOperator(tenant),
-                     RoleId.fromValue("my-tenant.tenantOperator").toRole(roles));
-        assertEquals(roles.applicationReader(tenant, application),
-                     RoleId.fromValue("my-tenant.my-application.applicationReader").toRole(roles));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalTenantName() {
-        RoleId.fromRole(new Roles(SystemName.main).tenantAdmin(TenantName.from("my.tenant")));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalApplicationName() {
-        RoleId.fromRole(new Roles(SystemName.main).applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalRole() {
-        RoleId.fromRole(new Roles(SystemName.main).tenantPipeline(TenantName.from("my-tenant"), ApplicationName.from("my-app")));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalRoleValue() {
-        RoleId.fromValue("my-tenant.awesomePerson").toRole(new Roles(SystemName.cd));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalCombination() {
-        RoleId.fromValue("my-tenant.my-application.tenantOwner").toRole(new Roles(SystemName.cd));
-    }
-
-    @Test(expected = IllegalArgumentException.class)
-    public void illegalValue() {
-        RoleId.fromValue("hostedOperator").toRole(new Roles(SystemName.Public));
-    }
-
-}
diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java
new file mode 100644
index 00000000000..262245a3366
--- /dev/null
+++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/user/UserRolesTest.java
@@ -0,0 +1,67 @@
+package com.yahoo.vespa.hosted.controller.api.integration.user;
+
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.vespa.hosted.controller.api.role.ApplicationRole;
+import com.yahoo.vespa.hosted.controller.api.role.Roles;
+import com.yahoo.vespa.hosted.controller.api.role.TenantRole;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author jonmv
+ */
+public class UserRolesTest {
+
+    private static final Roles roles = new Roles(SystemName.main);
+    private static final UserRoles userRoles = new UserRoles(roles);
+
+    @Test
+    public void testSerialization() {
+        TenantName tenant = TenantName.from("my-tenant");
+        for (TenantRole role : userRoles.tenantRoles(tenant))
+            assertEquals(role, userRoles.toRole(UserRoles.valueOf(role)));
+
+        ApplicationName application = ApplicationName.from("my-application");
+        for (ApplicationRole role : userRoles.applicationRoles(tenant, application))
+            assertEquals(role, userRoles.toRole(UserRoles.valueOf(role)));
+
+        assertEquals(roles.tenantOperator(tenant),
+                     userRoles.toRole("my-tenant.tenantOperator"));
+        assertEquals(roles.applicationReader(tenant, application),
+                     userRoles.toRole("my-tenant.my-application.applicationReader"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalTenantName() {
+        UserRoles.valueOf(roles.tenantAdmin(TenantName.from("my.tenant")));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalApplicationName() {
+        UserRoles.valueOf(roles.applicationOperator(TenantName.from("my-tenant"), ApplicationName.from("my.app")));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalRole() {
+        UserRoles.valueOf(roles.tenantPipeline(TenantName.from("my-tenant"), ApplicationName.from("my-app")));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalRoleValue() {
+        userRoles.toRole("my-tenant.awesomePerson");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalCombination() {
+        userRoles.toRole("my-tenant.my-application.tenantOwner");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void illegalValue() {
+        userRoles.toRole("hostedOperator");
+    }
+
+}