Changeset 35299

Timestamp:

2021-08-16T20:38:05+12:00 (3 years ago)

Author:

anupama

Message:

1. UserTermInfo.toString() now displays expandedGroups instead of compactedGroups (or the earlier, original user-entered groups). Still need to check that this is not used somewhere important, and if so, then that it is only used in DB related contexts where it needs to be expandedGroups now. 2. Tidied up: origGroups and getOrigGroups() are now compactedGroups and getCompactedGroups() and cleaned up unused/commented out code I had added.

Location:

main/trunk/greenstone3/src/java/org/greenstone/gsdl3

Files:

• service/Authentication.java (modified) (3 diffs)
• util/UserTermInfo.java (modified) (7 diffs)

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/service/Authentication.java

@@ -1084 +1084 @@
             for (int i = 0; i < userQueryResult.getSize(); i++)
             {
-                // can call either getExpandedGroups() or getOrigGroups() to check admin group is in there:
-                if (((UserTermInfo) userInfo.get(i)).getOrigGroups() != null && ((UserTermInfo) userInfo.get(i)).getOrigGroups().matches(".*\\badministrator\\b.*"))
+                // can call either getExpandedGroups() or getCompactedGroups() to check admin group is in there:
+                if (((UserTermInfo) userInfo.get(i)).getCompactedGroups() != null && ((UserTermInfo) userInfo.get(i)).getCompactedGroups().matches(".*\\badministrator\\b.*"))
                 {
                     adminFound = true;
@@ -1217 +1217 @@
                 else if (dataType.equals(COMPACTED_GROUPS))
                 {
-                    data = ((UserTermInfo) userInfo.get(i)).getOrigGroups();
+                    data = ((UserTermInfo) userInfo.get(i)).getCompactedGroups();
                     break;
                 }
@@ -1260 +1260 @@
             Element user_node = doc.createElement(GSXML.USER_NODE_ELEM);
             String username = ((UserTermInfo) userInfo.get(i)).getUsername();
-            String compactedGroups = ((UserTermInfo) userInfo.get(i)).getOrigGroups();
+            String compactedGroups = ((UserTermInfo) userInfo.get(i)).getCompactedGroups();
             // Since this is used for display in admin pages, get compacted (more similar to user-entered) groups
             // and not expanded groups, as getUserNodeList only used in addUserInformationToNode() which

main/trunk/greenstone3/src/java/org/greenstone/gsdl3/util/UserTermInfo.java

@@ -27 +27 @@
     private String username;
     private String password;
-        private String origGroups = null; // TODO: rename to compactedGroups and get-method, and update method calls accordingly
+        /** The user-entered value of groups listing sorted into natural ordering
+     * (alphabetic/ASCII order in this case) and with duplicates removed */
+        private String compactedGroups;
+        /** The user-entered value of groups listing with hierarchical groups expanded, sorted and duplicates removed
+     * e.g. if groups="nz.ac.waikato.toto,nz.ac.waikato.cs.pinky"; expandedGroups="nz,nz.ac,nz.ac.waikato,nz.ac.waikato.cs,nz.ac.waikato.cs.pinky,nz.ac.waikato.toto" */
         private String expandedGroups;
     private String accountstatus;
@@ -34 +38 @@
 
     // This constructor is called to instantiate a UserTermInfo object from entries in the userDB
-    // which now stores expandedGroups instead of the user-entered origGroups
-    // Use the other constructor if you know the user-entered groups
+    // which now stores expandedGroups instead of the user-entered groups or the latter's more compactedGroups form
     public UserTermInfo(String username, String password, String expandedGroups,
             String accountStatus, String comment, String email) {
@@ -41 +44 @@
     this.password = password;
 
-
+    // Ensure these 2 representations of the groups listing are always in sync
     this.expandedGroups = expandedGroups;
-    this.origGroups = UserTermInfo.compactGroups(this.expandedGroups); // alternative: don't set here and do lazy evaluation when getOrigGroups() is called
+    this.compactedGroups = UserTermInfo.compactGroups(this.expandedGroups); // alternative: don't set here and do lazy evaluation when getCompactedGroups() is called
+    
     this.accountstatus = accountStatus;
     this.comment = comment;
     this.email = email;
     }
-
-    
-    // Not used, for completion's sake:
-    // If you only have user-entered groups at hand, pass in null or "" for expandedGroups
-    // to create a UserTermInfo object, which will work out the expandedGroups.
-    /*
-    public UserTermInfo(String username, String password, String userEnteredGroups, String expandedGroups,
-            String accountStatus, String comment, String email) {
-    this.username = username;
-    this.password = password;
-    
-    this.origGroups = (userEnteredGroups == null) ? "" : userEnteredGroups;
-    if(expandedGroups == null || expandedGroups.equals("")) {
-        this.expandedGroups = UserTermInfo.expandGroups(this.origGroups);
-    }
-    this.accountstatus = accountStatus;
-    this.comment = comment;
-    this.email = email; 
-    }
-    */
     
     public String toString()
@@ -74 +58 @@
         result += "<username = " + username + ">";
         result += "<password = " + password + ">";
-        result += "<groups = " + origGroups + ">";
+        result += "<groups = " + expandedGroups + ">"; // compactedGroups?
         result += "<enable = " + accountstatus + ">";
         result += "<comment = " + comment + ">";
@@ -90 +74 @@
     }
 
-    public String getOrigGroups() {
-    return origGroups;
-    //return UserTermInfo.compactGroups(this.expandedGroups); // not storing, evaluating on demand
-
-    // lazy evaluation: getExpandedGroups() doesn't get called on every enquiry about
-    // a UserTermInfo struct. It only gets called on authenticated-ping system-action
-    // (including via ServletRealmCheck being run from run from commandline or gliserver.pl)
-    // So for a whole list of users and the UserTermInfo struct of each,
-    // don't pre-calculate and store the expanded groups.
-    // Is it meaningful to store this at all Ever or better to always evaluate on demand?
-    
+    public String getCompactedGroups() {
+    return compactedGroups;
+    // could do lazy evaluation by calling UserTermInfo.compactGroups(this.expandedGroups) here and returning it
+    // or 1st storing answer in compactedGroups member var before returning to avoid reevaluating multiple times.
     }    
     
     public String getExpandedGroups() {
     return expandedGroups;
-    //return UserTermInfo.expandGroups(this.origGroups); // not storing, evaluating on demand
-    
-    // lazy evaluation: getExpandedGroups() doesn't get called on every enquiry about
-    // a UserTermInfo struct. It only gets called on authenticated-ping system-action
-    // (including via ServletRealmCheck being run from run from commandline or gliserver.pl)
-    // So for a whole list of users and the UserTermInfo struct of each,
-    // don't pre-calculate and store the expanded groups.
-    // Is it meaningful to store this at all Ever or better to always evaluate on demand?
-    ///if(expandedGroups == null) {
-    ///    expandedGroups = UserTermInfo.expandGroups(this.origGroups);
-    ///}
-    ///return expandedGroups;
     }
 
@@ -196 +161 @@
         return compactGroupList.toString();
     }
-    
-    
-    /*
-    else {
-     
-
-        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of whitespace surrounding commas
-        if(indivGroups.length == 0) {
-        return groups;
-        }
-        Set<String> compactGroups = new TreeSet<String>();
-        String currGroup = indivGroups[indivGroups.length-1];
-        for(int i = indivGroups.length-1; i--; i > 0) {
-        String prevGroup = indivGroups[i-1];
-        
-        int indexOfPeriod = currGroup.indexOf('.');
-        if(indexOfPeriod == -1) {
-           compactGroups.add(currGroup);
-           currGroup = prevGroup;
-        }
-        else {
-        
-            if(currGroup.startsWith(prevGroup)) {
-            compactGroups.add(currGroup);
-            if(i == 0) {
-                break;              
-            } else {
-                i--;
-                currGroup = indivGroups[i];
-            }
-            }
-            else {
-            compactGroups.add(currGroup);
-            currGroup = prevGroup;
-            }
-        }
-        }
-    }
-    */
     } 
 
@@ -311 +237 @@
 
 
-     /*
-    public static String expandGroups(String groups) {
-    if(groups.indexOf('.') == -1) {
-        return groups;
-    }
-    else {
-        // Build up the string again
-        StringBuilder expandedGroupList = new StringBuilder();      
-
-        String[] indivGroups = groups.split("\\s*,\\s*"); // simultaneously get rid of any whitespace surrounding commas
-        for(String s : indivGroups) {
-        int indexOfPeriod = s.indexOf('.');
-
-        if(indexOfPeriod != -1) {
-            do {
-            String expandedGroup = s.substring(0, indexOfPeriod);
-            expandedGroupList.append(expandedGroup);
-            expandedGroupList.append(',');
-            indexOfPeriod += 1;         
-            indexOfPeriod = s.indexOf('.', indexOfPeriod);
-            } while(indexOfPeriod != -1);
-        }
-
-        // Whether this particular group s contained a period or not,
-        // remember to add this group string, s, back in in its entirety too        
-        expandedGroupList.append(s);
-        expandedGroupList.append(',');
-        }
-        expandedGroupList.deleteCharAt(expandedGroupList.length()-1); // remove extraneous comma        
-        return expandedGroupList.toString();
-    }
-    }
-    */
-    
 }