From 9480a63ec41e117c7868786ed44fb2276f62469a Mon Sep 17 00:00:00 2001
From: Philip Durbin <philip_durbin@harvard.edu>
Date: Wed, 22 Jun 2016 15:17:45 -0400
Subject: [PATCH] Private URL: clarify javadoc identfier todo #1012

---
 .../dataverse/privateurl/PrivateUrlUtil.java  | 35 +++++++++++--------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtil.java b/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtil.java
index 15a5bdd1993..fde46308f84 100644
--- a/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtil.java
+++ b/src/main/java/edu/harvard/iq/dataverse/privateurl/PrivateUrlUtil.java
@@ -24,26 +24,31 @@ public class PrivateUrlUtil {
      * to return a RoleAssignee (which can be either a User or a Group) when all
      * you have is the string that is their identifier.
      *
-     * @todo Consider using a new character, something other than ":" as a
-     * namespace for PrivateUrlUser rather than ":" which is for a short list of
-     * unchanging "predefinedRoleAssignees" which consists of
-     * :authenticated-users, :AllUsers, and :guest. A PrivateUrlUser is
-     * something of a different animal in that its identifier will vary based on
-     * the dataset that it is associated with. The number at the end of the
-     * identifier will vary.
+     * @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 User
-     * rather than a Group (groups start with "&"). 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.
+     * ":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.
      *
      * @return A valid PrivateUrlUser (which like any User or Group is a
-     * RoleAssignee) if a valid identifer is provided or null.
+     * RoleAssignee) if a valid identifier is provided or null.
      */
     public static RoleAssignee identifier2roleAssignee(String identifier) {
         String[] parts = identifier.split(PrivateUrlUser.PREFIX);