Private URL: changed namespace from ":privateUrl" to "#" #1012

src/main/java/edu/harvard/iq/dataverse/RoleAssigneeServiceBean.java

@@ -10,7 +10,6 @@
 import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroup;
 import edu.harvard.iq.dataverse.authorization.groups.impl.explicit.ExplicitGroupServiceBean;
 import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
-import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser;
 import edu.harvard.iq.dataverse.authorization.users.GuestUser;
 import edu.harvard.iq.dataverse.privateurl.PrivateUrlUtil;
 import java.util.ArrayList;
@@ -63,38 +62,28 @@ void setup() {
     }
 
     /**
-     * @param identifier An identifier beginning with ":", "@", or "&".
+     * @param identifier An identifier beginning with ":" (builtin), "@"
+     * ({@link AuthenticatedUser}), "&" ({@link Group}), or "#"
+     * ({@link PrivateUrlUser}).
+     *
      * @return A RoleAssignee (User or Group) or null.
-     * @throws IllegalArgumentException
+     *
+     * @throws IllegalArgumentException if you pass null, empty string, or an
+     * identifier that doesn't start with one of the supported characters.
      */
     public RoleAssignee getRoleAssignee(String identifier) {
         if (identifier == null || identifier.isEmpty()) {
             throw new IllegalArgumentException("Identifier cannot be null or empty string.");
         }
         switch (identifier.charAt(0)) {
             case ':':
-                /**
-                 * This "startsWith" code in identifier2roleAssignee is here to
-                 * support a functional requirement to display the Private URL
-                 * role assignment when looking at permissions at the dataset
-                 * level in the GUI and allow for revoking the role from that
-                 * page. Interestingly, if you remove the "startsWith" code,
-                 * null will be returned for Private URL but the assignment is
-                 * still visible from the API. When null is returned
-                 * ManagePermissionsPage cannot list the assignment.
-                 *
-                 * "startsWith" is the moral equivalent of
-                 * "identifier.charAt(0)". :)
-                 */
-                if (identifier.startsWith(PrivateUrlUser.PREFIX)) {
-                    return PrivateUrlUtil.identifier2roleAssignee(identifier);
-                } else {
-                    return predefinedRoleAssignees.get(identifier);
-                }
+                return predefinedRoleAssignees.get(identifier);
             case '@':
                 return authSvc.getAuthenticatedUser(identifier.substring(1));
             case '&':
                 return groupSvc.getGroup(identifier.substring(1));
+            case '#':
+                return PrivateUrlUtil.identifier2roleAssignee(identifier);
             default:
                 throw new IllegalArgumentException("Unsupported assignee identifier '" + identifier + "'");
         }

src/main/java/edu/harvard/iq/dataverse/authorization/users/PrivateUrlUser.java

@@ -12,7 +12,7 @@
  */
 public class PrivateUrlUser implements User {
 
-    public static final String PREFIX = ":privateUrl";
+    public static final String PREFIX = "#";
 
     /**
      * In the future, this could probably be dvObjectId rather than datasetId,

src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtil.java

@@ -23,32 +23,13 @@ public class PrivateUrlUtil {
 
     /**
      * Use of this method should be limited to
-     * RoleAssigneeServiceBean.getRoleAssignee, which is the centralized place
-     * to return a RoleAssignee (which can be either a User or a Group) when all
-     * you have is the string that is their identifier.
+     * RoleAssigneeServiceBean.getRoleAssignee. If you have the
+     * {@link RoleAssignment} in your hand, just instantiate a
+     * {@link PrivateUrlUser} using the definitionPoint.
      *
-     * @todo Consider using a new character such as "#" (as suggested by
-     * Michael) as the unique namespace for PrivateUrlUser rather than ":" which
-     * means "builtin". Before the introduction of the Private URL feature, the
-     * prefix ":" was only used for a short list of unchanging
-     * "predefinedRoleAssignees" which consisted of the groups
-     * ":authenticated-users" and ":AllUsers" and the user ":guest". A
-     * PrivateUrlUser is something of a different animal in that its identifier
-     * will vary based on the dataset that it is associated with
-     * (":privateUrl42" for dataset 42, for example). The prefix we're using now
-     * is ":privateUrl". If we switch to "#" I guess we would just make it
-     * "#42"? Or would it be "#privateUrl42?" See also getRoleAssignee in
-     * RoleAssigneeServiceBean which is where the code would be cleaner if we
-     * use "#".
-     *
-     * @param identifier The identifier is expected to start with the
-     * PrivateUrlUser.PREFIX and end with a number for a dataset,
-     * ":privateUrl42", for example. The ":" indicates that this is a "builtin"
-     * role assignee. The number at the end of the identifier of a
-     * PrivateUrlUser is all we have to associate the role assignee identifier
-     * with a dataset. If we had the role assignment itself in our hands, we
-     * would simply get the dataset id from RoleAssignment.getDefinitionPoint
-     * and then use it to instantiate a PrivateUrlUser.
+     * @param identifier For example, "#42". The identifier is expected to start
+     * with "#" (the namespace for a PrivateUrlUser and its corresponding
+     * RoleAssignment) and end with the dataset id.
      *
      * @return A valid PrivateUrlUser (which like any User or Group is a
      * RoleAssignee) if a valid identifier is provided or null.

src/test/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtilTest.java

@@ -63,7 +63,7 @@ public void testIdentifier2roleAssignee() {
         String validIdentifier = PrivateUrlUser.PREFIX + 42;
         returnFromValidIdentifier = PrivateUrlUtil.identifier2roleAssignee(validIdentifier);
         assertNotNull(returnFromValidIdentifier);
-        assertEquals(":privateUrl42", returnFromValidIdentifier.getIdentifier());
+        assertEquals("#42", returnFromValidIdentifier.getIdentifier());
         assertEquals("Private URL Enabled", returnFromValidIdentifier.getDisplayInfo().getTitle());
         Assert.assertTrue(returnFromValidIdentifier instanceof PrivateUrlUser);
         PrivateUrlUser privateUrlUser42 = (PrivateUrlUser) returnFromValidIdentifier;
@@ -107,7 +107,7 @@ public void testGetDatasetFromRoleAssignmentSuccess() {
         String privateUrlToken = null;
         RoleAssignment ra = new RoleAssignment(aRole, anAssignee, dataset, privateUrlToken);
         assertNotNull(PrivateUrlUtil.getDatasetFromRoleAssignment(ra));
-        assertEquals(":privateUrl42", ra.getAssigneeIdentifier());
+        assertEquals("#42", ra.getAssigneeIdentifier());
     }
 
     @Test
@@ -159,7 +159,7 @@ public void testGetDraftDatasetVersionFromRoleAssignmentSuccess() {
         RoleAssignment ra = new RoleAssignment(aRole, anAssignee, dataset, privateUrlToken);
         DatasetVersion datasetVersionOut = PrivateUrlUtil.getDraftDatasetVersionFromRoleAssignment(ra);
         assertNotNull(datasetVersionOut);
-        assertEquals(":privateUrl42", ra.getAssigneeIdentifier());
+        assertEquals("#42", ra.getAssigneeIdentifier());
     }
 
     @Test

src/test/java/edu/harvard/iq/dataverse/util/json/JsonPrinterTest.java

@@ -26,7 +26,7 @@ public void testJson_RoleAssignment() {
         JsonObjectBuilder job = JsonPrinter.json(ra);
         assertNotNull(job);
         JsonObject jsonObject = job.build();
-        assertEquals(":privateUrl42", jsonObject.getString("assignee"));
+        assertEquals("#42", jsonObject.getString("assignee"));
         assertEquals(123, jsonObject.getInt("definitionPointId"));
         assertEquals("e1d53cf6-794a-457a-9709-7c07629a8267", jsonObject.getString("privateUrlToken"));
     }
@@ -47,7 +47,7 @@ public void testJson_PrivateUrl() {
         assertEquals("e1d53cf6-794a-457a-9709-7c07629a8267", jsonObject.getString("token"));
         assertEquals("https://dataverse.example.edu/privateurl.xhtml?token=e1d53cf6-794a-457a-9709-7c07629a8267", jsonObject.getString("link"));
         assertEquals("e1d53cf6-794a-457a-9709-7c07629a8267", jsonObject.getJsonObject("roleAssignment").getString("privateUrlToken"));
-        assertEquals(":privateUrl42", jsonObject.getJsonObject("roleAssignment").getString("assignee"));
+        assertEquals("#42", jsonObject.getJsonObject("roleAssignment").getString("assignee"));
     }
 
 }