core: allow custom roles (#2863)

Role previously mandated that all role strings starts with "role/".
If the given string doesn't start with the prefix, Role prepends it.

This is incorrect, since custom roles have format "projects/X/roles/Y".
This PR makes Role a little smarter at prepending:
we only prepend if the given string contains no slash; this keeps the
previously-valid use valid.

Fixes #2687.

Author: pongad

google-cloud-core/src/main/java/com/google/cloud/Role.java

@@ -82,15 +82,22 @@ public static Role owner() {
   /**
    * Returns a new role given its string value.
    *
-   * @param value the string value for the role, for example, {@code "roles/viewer"},
-   *     {@code "roles/editor"}, or {@code "roles/owner"}. If this value does not start with the
-   *     role prefix {@code roles/}, the prefix is prepended.
+   * <p>If the value contains no slash character ({@code '/'}), the prefix {@code "roles/""} is
+   * prepended. This slightly simplifies usage for <a
+   * href="https://cloud.google.com/iam/docs/understanding-roles>"predefined roles</a>. For <a
+   * href="https://cloud.google.com/iam/docs/creating-custom-roles">custom roles</a>, call this
+   * method with the fully-qualified name, eg {@code "projects/XXX/roles/YYY"}.
+   *
+   * @param value the string value for the role
    * @see <a href="https://cloud.google.com/iam/docs/viewing-grantable-roles">Viewing the Grantable
    *     Roles on Resources</a>
    */
   public static Role of(String value) {
     checkNotNull(value);
-    return new Role(value.startsWith(ROLE_PREFIX) ? value : ROLE_PREFIX + value);
+    if (!value.contains("/")) {
+      value = ROLE_PREFIX + value;
+    }
+    return new Role(value);
   }
 
   @Override

google-cloud-core/src/test/java/com/google/cloud/RoleTest.java

@@ -16,7 +16,7 @@
 
 package com.google.cloud;
 
-import static org.junit.Assert.assertEquals;
+import static com.google.common.truth.Truth.assertThat;
 
 import org.junit.Test;
 
@@ -28,28 +28,31 @@ public class RoleTest {
 
   @Test
   public void testOf() {
-    assertEquals("roles/viewer", VIEWER.getValue());
-    assertEquals("roles/editor", EDITOR.getValue());
-    assertEquals("roles/owner", OWNER.getValue());
+    assertThat(VIEWER.getValue()).isEqualTo("roles/viewer");
+    assertThat(EDITOR.getValue()).isEqualTo("roles/editor");
+    assertThat(OWNER.getValue()).isEqualTo("roles/owner");
     compareRoles(VIEWER, Role.of("roles/viewer"));
     compareRoles(EDITOR, Role.of("roles/editor"));
     compareRoles(OWNER, Role.of("roles/owner"));
+
+    String customRole = "projects/foo/roles/bar";
+    assertThat(Role.of(customRole).getValue()).isEqualTo(customRole);
   }
 
 
   @Test
   public void testViewer() {
-    assertEquals("roles/viewer", Role.viewer().getValue());
+    assertThat(Role.viewer().getValue()).isEqualTo("roles/viewer");
   }
 
   @Test
   public void testEditor() {
-    assertEquals("roles/editor", Role.editor().getValue());
+    assertThat(Role.editor().getValue()).isEqualTo("roles/editor");
   }
 
   @Test
   public void testOwner() {
-    assertEquals("roles/owner", Role.owner().getValue());
+    assertThat(Role.owner().getValue()).isEqualTo("roles/owner");
   }
 
   @Test(expected = NullPointerException.class)
@@ -58,9 +61,9 @@ public void testOfNullValue() {
   }
 
   private void compareRoles(Role expected, Role actual) {
-    assertEquals(expected, actual);
-    assertEquals(expected.getValue(), actual.getValue());
-    assertEquals(expected.hashCode(), actual.hashCode());
-    assertEquals(expected.toString(), actual.toString());
+    assertThat(actual).isEqualTo(expected);
+    assertThat(actual.getValue()).isEqualTo(expected.getValue());
+    assertThat(actual.hashCode()).isEqualTo(expected.hashCode());
+    assertThat(actual.toString()).isEqualTo(expected.toString());
   }
 }