From b3eb73e4ff4162967649272440b1138d35df9fc0 Mon Sep 17 00:00:00 2001
From: Brian Major <major.brian@gmail.com>
Date: Mon, 14 Nov 2016 15:49:03 -0800
Subject: [PATCH] issue-10 - reverted API changes in GMSClient

---
 cadc-access-control-server/build.gradle       |   4 +-
 cadc-access-control/build.gradle              |   2 +-
 .../java/ca/nrc/cadc/ac/client/GMSClient.java | 144 +++++++++---------
 .../client/{GMSClientMain.java => Main.java}  |  70 ++++-----
 .../ca/nrc/cadc/ac/client/GMSClientTest.java  |  52 ++++---
 5 files changed, 133 insertions(+), 139 deletions(-)
 rename cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/{GMSClientMain.java => Main.java} (85%)

diff --git a/cadc-access-control-server/build.gradle b/cadc-access-control-server/build.gradle
index d0d66f7b..8685386b 100644
--- a/cadc-access-control-server/build.gradle
+++ b/cadc-access-control-server/build.gradle
@@ -13,7 +13,7 @@ repositories {
 sourceCompatibility = 1.7
 group = 'org.opencadc'
 
-version = '1.1.0'
+version = '1.1.1'
 
 dependencies {
     compile 'log4j:log4j:1.2.+'
@@ -22,7 +22,7 @@ dependencies {
     compile 'xerces:xercesImpl:2.+'
     compile 'com.unboundid:unboundid-ldapsdk:2.3.+'
 
-    compile 'org.opencadc:cadc-access-control:1.1.+'
+    compile 'org.opencadc:cadc-access-control:[1.1.1,)'
     compile 'org.opencadc:cadc-util:1.+'
     compile 'org.opencadc:cadc-log:1.+'
     compile 'org.opencadc:cadc-registry:1.+'
diff --git a/cadc-access-control/build.gradle b/cadc-access-control/build.gradle
index 1ff4624c..3a5e919c 100644
--- a/cadc-access-control/build.gradle
+++ b/cadc-access-control/build.gradle
@@ -15,7 +15,7 @@ sourceCompatibility = 1.7
 
 group = 'org.opencadc'
 
-version = '1.1.0'
+version = '1.1.1'
 
 mainClassName = 'ca.nrc.cadc.ac.client.GMSClientMain'
 
diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
index 2e58a8a5..68d94dd1 100755
--- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClient.java
@@ -94,7 +94,6 @@ import org.apache.log4j.Logger;
 import ca.nrc.cadc.ac.Group;
 import ca.nrc.cadc.ac.GroupAlreadyExistsException;
 import ca.nrc.cadc.ac.GroupNotFoundException;
-import ca.nrc.cadc.ac.GroupURI;
 import ca.nrc.cadc.ac.Role;
 import ca.nrc.cadc.ac.UserNotFoundException;
 import ca.nrc.cadc.ac.WriterException;
@@ -129,11 +128,20 @@ public class GMSClient implements TransferListener
     private SSLSocketFactory sslSocketFactory;
     private SSLSocketFactory mySocketFactory;
 
+    private URI serviceID;
+
     /**
      * Constructor.
+     *
+     * @param serviceID            The service ID.
      */
-    public GMSClient()
+    public GMSClient(URI serviceID)
     {
+        if (serviceID == null)
+            throw new IllegalArgumentException("invalid serviceID: " + serviceID);
+        if (serviceID.getFragment() != null)
+            throw new IllegalArgumentException("invalid serviceID (fragment not allowed): " + serviceID);
+        this.serviceID = serviceID;
     }
 
     public void transferEvent(TransferEvent te)
@@ -173,9 +181,8 @@ public class GMSClient implements TransferListener
         throws GroupAlreadyExistsException, AccessControlException,
                UserNotFoundException, WriterException, IOException
     {
-
         URL createGroupURL = getRegistryClient()
-            .getServiceURL(group.getID().getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         log.debug("createGroupURL request to " + createGroupURL.toString());
 
         // reset the state of the cache
@@ -237,19 +244,18 @@ public class GMSClient implements TransferListener
     /**
      * Get the group object.
      *
-     * @param groupID Identifies the group to get.
+     * @param groupName Identifies the group to get.
      * @return The group.
      * @throws GroupNotFoundException If the group was not found.
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public Group getGroup(GroupURI groupID)
+    public Group getGroup(String groupName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
-
         URL groupsURL = getRegistryClient()
-            .getServiceURL(groupID.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
-        URL getGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupID.getName());
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+        URL getGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupName);
         log.debug("getGroup request to " + getGroupURL.toString());
 
         ByteArrayOutputStream out = new ByteArrayOutputStream();
@@ -300,11 +306,11 @@ public class GMSClient implements TransferListener
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public List<String> getGroupNames(URI serviceID)
+    public List<String> getGroupNames()
         throws AccessControlException, IOException
     {
         URL getGroupNamesURL = getRegistryClient()
-            .getServiceURL(serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
 
         log.debug("getGroupNames request to " + getGroupNamesURL.toString());
 
@@ -382,7 +388,7 @@ public class GMSClient implements TransferListener
                AccessControlException, WriterException, IOException
     {
         URL groupsURL = getRegistryClient()
-            .getServiceURL(group.getID().getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL updateGroupURL = new URL(groupsURL.toExternalForm() + "/" + group.getID().getName());
         log.debug("updateGroup request to " + updateGroupURL.toString());
 
@@ -442,17 +448,17 @@ public class GMSClient implements TransferListener
     /**
      * Delete the group.
      *
-     * @param groupID Identifies the group to delete.
+     * @param groupName Identifies the group to delete.
      * @throws GroupNotFoundException If the group was not found.
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public void deleteGroup(GroupURI groupID)
+    public void deleteGroup(String groupName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
         URL groupsURL = getRegistryClient()
-            .getServiceURL(groupID.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
-        URL deleteGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupID.getName());
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
+        URL deleteGroupURL = new URL(groupsURL.toExternalForm() + "/" + groupName);
         log.debug("deleteGroup request to " + deleteGroupURL.toString());
 
         // reset the state of the cache
@@ -506,21 +512,21 @@ public class GMSClient implements TransferListener
     /**
      * Add a group as a member of another group.
      *
-     * @param targetGroup The group in which to add the group member.
+     * @param targetGroupName The group in which to add the group member.
      * @param groupMemberName The group member to add.
      * @throws IllegalArgumentException If cyclical membership is detected.
      * @throws GroupNotFoundException If the group was not found.
      * @throws AccessControlException If unauthorized to perform this operation.
      * @throws java.io.IOException
      */
-    public void addGroupMember(GroupURI targetGroup, String groupMemberName)
+    public void addGroupMember(String targetGroupName, String groupMemberName)
         throws IllegalArgumentException, GroupNotFoundException,
                AccessControlException, IOException
     {
 
-        String path = "/" + targetGroup.getName() + "/groupMembers/" + groupMemberName;
+        String path = "/" + targetGroupName + "/groupMembers/" + groupMemberName;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL addGroupMemberURL = new URL(groupsURL.toExternalForm() + path);
         log.debug("addGroupMember request to " + addGroupMemberURL.toString());
 
@@ -560,28 +566,28 @@ public class GMSClient implements TransferListener
     /**
      * Add a user as a member of a group.
      *
-     * @param targetGroup The group in which to add the group member.
+     * @param targetGroupName The group in which to add the group member.
      * @param userID The user to add.
      * @throws GroupNotFoundException If the group was not found.
      * @throws UserNotFoundException If the member was not found.
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void addUserMember(GroupURI targetGroup, Principal userID)
+    public void addUserMember(String targetGroupName, Principal userID)
         throws GroupNotFoundException, UserNotFoundException, AccessControlException, IOException
     {
-        if (targetGroup == null)
-            throw new IllegalArgumentException("targetGroup required");
+        if (targetGroupName == null)
+            throw new IllegalArgumentException("targetGroupName required");
 
         if (userID == null)
             throw new IllegalArgumentException("userID required");
 
-        log.debug("addUserMember: " + targetGroup + " + " + userID.getName());
+        log.debug("addUserMember: " + targetGroupName + " + " + userID.getName());
 
         String userIDType = AuthenticationUtil.getPrincipalType(userID);
-        String path = "/" + targetGroup.getName() + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
+        String path = "/" + targetGroupName + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL addUserMemberURL = new URL(groupsURL.toExternalForm() + path);
 
         log.debug("addUserMember request to " + addUserMemberURL.toString());
@@ -625,20 +631,20 @@ public class GMSClient implements TransferListener
     /**
      * Remove a group as a member of another group.
      *
-     * @param targetGroup The group from which to remove the group member.
+     * @param targetGroupName The group from which to remove the group member.
      * @param groupMemberName The group member to remove.
      * @throws GroupNotFoundException If the group was not found.
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void removeGroupMember(GroupURI targetGroup,
+    public void removeGroupMember(String targetGroupName,
                                   String groupMemberName)
         throws GroupNotFoundException, AccessControlException, IOException
     {
 
-        String path = "/" + targetGroup.getName() + "/groupMembers/" + groupMemberName;
+        String path = "/" + targetGroupName + "/groupMembers/" + groupMemberName;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL removeGroupMemberURL = new URL(groupsURL.toExternalForm() + path);
         log.debug("removeGroupMember request to " +
                   removeGroupMemberURL.toString());
@@ -692,22 +698,22 @@ public class GMSClient implements TransferListener
     /**
      * Remove a user as a member of a group.
      *
-     * @param targetGroup The group from which to remove the group member.
+     * @param targetGroupName The group from which to remove the group member.
      * @param userID The user to remove.
      * @throws GroupNotFoundException If the group was not found.
      * @throws UserNotFoundException If the member was not found.
      * @throws java.io.IOException
      * @throws AccessControlException If unauthorized to perform this operation.
      */
-    public void removeUserMember(GroupURI targetGroup, Principal userID)
+    public void removeUserMember(String targetGroupName, Principal userID)
         throws GroupNotFoundException, UserNotFoundException, AccessControlException, IOException
     {
         String userIDType = AuthenticationUtil.getPrincipalType(userID);
 
-        log.debug("removeUserMember: " + targetGroup + " - " + userID.getName() + " type: " + userIDType);
-        String path = "/" + targetGroup.getName() + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
+        log.debug("removeUserMember: " + targetGroupName + " - " + userID.getName() + " type: " + userIDType);
+        String path = "/" + targetGroupName + "/userMembers/" + NetUtil.encode(userID.getName()) + "?idType=" + userIDType;
         URL groupsURL = getRegistryClient()
-            .getServiceURL(targetGroup.getServiceID(), Standards.GMS_GROUPS_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT);
         URL removeUserMemberURL = new URL(groupsURL.toExternalForm() + path);
 
         log.debug("removeUserMember: " + removeUserMemberURL.toString());
@@ -783,14 +789,14 @@ public class GMSClient implements TransferListener
      * @throws ca.nrc.cadc.ac.UserNotFoundException
      * @throws java.io.IOException
      */
-    public List<Group> getMemberships(URI serviceID, Role role)
+    public List<Group> getMemberships(Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return getMemberships(serviceID, null, role);
+        return getMemberships(null, role);
     }
 
 
-    private List<Group> getMemberships(URI serviceID, Principal ignore, Role role)
+    private List<Group> getMemberships(Principal ignore, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
         if (role == null)
@@ -801,7 +807,7 @@ public class GMSClient implements TransferListener
         Principal userID = getCurrentUserID();
         if (userID != null)
         {
-            List<Group> cachedGroups = getCachedGroups(serviceID, userID, role, true);
+            List<Group> cachedGroups = getCachedGroups(userID, role, true);
             if (cachedGroups != null)
             {
                 return cachedGroups;
@@ -819,7 +825,7 @@ public class GMSClient implements TransferListener
         searchGroupPath.append("&ROLE=").append(NetUtil.encode(roleString));
 
         URL searchURL = getRegistryClient()
-            .getServiceURL(serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
         URL getMembershipsURL = new URL(searchURL.toExternalForm() + searchGroupPath.toString());
 
         log.debug("getMemberships request to " + getMembershipsURL.toString());
@@ -857,7 +863,7 @@ public class GMSClient implements TransferListener
             log.debug("getMemberships returned: " + groupsXML);
             GroupListReader groupListReader = new GroupListReader();
             List<Group> groups = groupListReader.read(groupsXML);
-            setCachedGroups(serviceID, userID, groups, role);
+            setCachedGroups(userID, groups, role);
             return groups;
         }
         catch (Exception bug)
@@ -874,17 +880,17 @@ public class GMSClient implements TransferListener
      *
      * This call is identical to getMemberShip(userID, groupName, Role.MEMBER)
      *
-     * @param groupID Identifies the group.
+     * @param groupName Identifies the group.
      * @return The group or null of the user is not a member.
      * @throws UserNotFoundException If the user does not exist.
      * @throws AccessControlException If not allowed to peform the search.
      * @throws IllegalArgumentException If a parameter is null.
      * @throws IOException If an unknown error occured.
      */
-    public Group getMembership(GroupURI groupID)
+    public Group getMembership(String groupName)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return getMembership(groupID, Role.MEMBER);
+        return getMembership(groupName, Role.MEMBER);
     }
 
     /**
@@ -892,7 +898,7 @@ public class GMSClient implements TransferListener
      * identified by userID, is a member (of type role) of that group.
      * Return null otherwise.
      *
-     * @param groupID Identifies the group.
+     * @param groupName Identifies the group.
      * @param role The membership role to search.
      * @return The group or null of the user is not a member.
      * @throws UserNotFoundException If the user does not exist.
@@ -900,10 +906,10 @@ public class GMSClient implements TransferListener
      * @throws IllegalArgumentException If a parameter is null.
      * @throws IOException If an unknown error occured.
      */
-    public Group getMembership(GroupURI groupID, Role role)
+    public Group getMembership(String groupName, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        if (groupID == null || role == null)
+        if (groupName == null || role == null)
         {
             throw new IllegalArgumentException("groupName and role are required.");
         }
@@ -911,7 +917,7 @@ public class GMSClient implements TransferListener
         Principal userID = getCurrentUserID();
         if (userID != null)
         {
-            Group cachedGroup = getCachedGroup(userID, groupID, role);
+            Group cachedGroup = getCachedGroup(userID, groupName, role);
             if (cachedGroup != null)
             {
                 return cachedGroup;
@@ -927,10 +933,10 @@ public class GMSClient implements TransferListener
         //searchGroupURL.append("ID=").append(NetUtil.encode(id));
         //searchGroupURL.append("&IDTYPE=").append(NetUtil.encode(idType));
         searchGroupPath.append("&ROLE=").append(NetUtil.encode(roleString));
-        searchGroupPath.append("&GROUPID=").append(NetUtil.encode(groupID.getName()));
+        searchGroupPath.append("&GROUPID=").append(NetUtil.encode(groupName));
 
         URL searchURL = getRegistryClient()
-            .getServiceURL(groupID.getServiceID(), Standards.GMS_SEARCH_01, AuthMethod.CERT);
+            .getServiceURL(this.serviceID, Standards.GMS_SEARCH_01, AuthMethod.CERT);
         URL getMembershipURL = new URL(searchURL.toExternalForm() + searchGroupPath.toString());
 
         log.debug("getMembership request to " + getMembershipURL.toString());
@@ -979,7 +985,7 @@ public class GMSClient implements TransferListener
                 return ret;
             }
             throw new IllegalStateException(
-                    "Duplicate membership for " + userID + " in group " + groupID);
+                    "Duplicate membership for " + userID + " in group " + groupName);
         }
         catch (Exception bug)
         {
@@ -991,37 +997,37 @@ public class GMSClient implements TransferListener
     /**
      * Check group membership of the current Subject.
      *
-     * @param groupID
+     * @param groupName
      * @return true if the current Subject is a member of the group, false otherwise
      * @throws UserNotFoundException
      * @throws AccessControlException
      * @throws IOException
      */
-    public boolean isMember(GroupURI groupID)
+    public boolean isMember(String groupName)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return isMember(groupID, Role.MEMBER);
+        return isMember(groupName, Role.MEMBER);
     }
 
     /**
      *
-     * @param groupID
+     * @param groupName
      * @param role
      * @return true if the current Subject is a member of the group with the specified role, false otherwise
      * @throws UserNotFoundException
      * @throws AccessControlException
      * @throws IOException
      */
-    public boolean isMember(GroupURI groupID, Role role)
+    public boolean isMember(String groupName, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        return isMember(getCurrentUserID(), groupID, role);
+        return isMember(getCurrentUserID(), groupName, role);
     }
 
-    private boolean isMember(Principal userID, GroupURI groupID, Role role)
+    private boolean isMember(Principal userID, String groupName, Role role)
         throws UserNotFoundException, AccessControlException, IOException
     {
-        Group group = getMembership(groupID, role);
+        Group group = getMembership(groupName, role);
         return group != null;
     }
 
@@ -1077,7 +1083,7 @@ public class GMSClient implements TransferListener
         }
     }
 
-    protected GroupMemberships getGroupCache(URI serviceID, Principal userID)
+    protected GroupMemberships getGroupCache(Principal userID)
     {
         AccessControlContext acContext = AccessController.getContext();
         Subject subject = Subject.getSubject(acContext);
@@ -1107,21 +1113,21 @@ public class GMSClient implements TransferListener
         return null; // no cache
     }
 
-    protected Group getCachedGroup(Principal userID, GroupURI groupID, Role role)
+    protected Group getCachedGroup(Principal userID, String groupID, Role role)
     {
-        List<Group> groups = getCachedGroups(groupID.getServiceID(), userID, role, false);
+        List<Group> groups = getCachedGroups(userID, role, false);
         if (groups == null)
             return null; // no cache
         for (Group g : groups)
         {
-            if (g.getID().equals(groupID))
+            if (g.getID().getName().equals(groupID))
                 return g;
         }
         return null;
     }
-    protected List<Group> getCachedGroups(URI serviceID, Principal userID, Role role, boolean complete)
+    protected List<Group> getCachedGroups(Principal userID, Role role, boolean complete)
     {
-        GroupMemberships mems = getGroupCache(serviceID, userID);
+        GroupMemberships mems = getGroupCache(userID);
         if (mems == null)
             return null; // no cache
 
@@ -1135,16 +1141,16 @@ public class GMSClient implements TransferListener
 
     protected void addCachedGroup(Principal userID, Group group, Role role)
     {
-        GroupMemberships mems = getGroupCache(group.getID().getServiceID(), userID);
+        GroupMemberships mems = getGroupCache(userID);
         if (mems == null)
             return; // no cache
 
         mems.add(group, role);
     }
 
-    protected void setCachedGroups(URI serviceID, Principal userID, List<Group> groups, Role role)
+    protected void setCachedGroups(Principal userID, List<Group> groups, Role role)
     {
-        GroupMemberships mems = getGroupCache(serviceID, userID);
+        GroupMemberships mems = getGroupCache(userID);
         if (mems == null)
             return; // no cache
 
diff --git a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/Main.java
similarity index 85%
rename from cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java
rename to cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/Main.java
index 14e8db6e..87bfca84 100644
--- a/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/GMSClientMain.java
+++ b/cadc-access-control/src/main/java/ca/nrc/cadc/ac/client/Main.java
@@ -69,6 +69,7 @@
 
 package ca.nrc.cadc.ac.client;
 
+import java.net.URI;
 import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.Principal;
@@ -95,10 +96,10 @@ import ca.nrc.cadc.util.Log4jInit;
  * only used for testing.  Should not be used for production
  * work.
  */
-public class GMSClientMain implements PrivilegedAction<Object>
+public class Main implements PrivilegedAction<Object>
 {
 
-    private static Logger log = Logger.getLogger(GMSClientMain.class);
+    private static Logger log = Logger.getLogger(Main.class);
 
     public static final String ARG_ADD_MEMBER = "add-member";
     public static final String ARG_DEL_MEMBER = "remove-member";
@@ -118,12 +119,11 @@ public class GMSClientMain implements PrivilegedAction<Object>
     public static final String ARG_V = "v";
     public static final String ARG_D = "d";
 
-    private GMSClient client;
     private ArgumentMap argMap;
 
-    private GMSClientMain()
+    private Main(ArgumentMap args)
     {
-        client = new GMSClient();
+        this.argMap = args;
     }
 
     public static void main(String[] args)
@@ -149,8 +149,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
         else
             Log4jInit.setLevel("ca", Level.WARN);
 
-        GMSClientMain main = new GMSClientMain();
-        main.argMap = argMap;
+        Main main = new Main(argMap);
 
         Subject subject = CertCmdArgUtil.initSubject(argMap, true);
 
@@ -192,15 +191,15 @@ public class GMSClientMain implements PrivilegedAction<Object>
 
     private static void usage()
     {
-        System.out.println("--create --group=<g>");
-        System.out.println("--get --group=<g>");
-        System.out.println("--delete --group=<g>");
+        System.out.println("--create --group=<uri>");
+        System.out.println("--get --group=<uri>");
+        System.out.println("--delete --group=<uri>");
         System.out.println();
-        System.out.println("--add-member --group=<g> --userid=<u>");
-        System.out.println("--remove-member --group=<g> --userid=<u>");
+        System.out.println("--add-member --group=<uri> --userid=<u>");
+        System.out.println("--remove-member --group=<uri> --userid=<u>");
         System.out.println();
-        System.out.println("--add-admin --group=<g> --userid=<u>");
-        System.out.println("--remove-admin --group=<g> --userid=<u>");
+        System.out.println("--add-admin --group=<uri> --userid=<u>");
+        System.out.println("--remove-admin --group=<uri> --userid=<u>");
     }
 
     @Override
@@ -209,12 +208,14 @@ public class GMSClientMain implements PrivilegedAction<Object>
         try
         {
             String command = getCommand();
-
-            GroupURI groupID = null;
+            String suri = argMap.getValue(ARG_GROUP);
+            GroupURI guri = new GroupURI(new URI(suri));
+            GMSClient client = new GMSClient(guri.getServiceID());
+            String group = guri.getName();
 
             if (command.equals(ARG_ADD_MEMBER))
             {
-                String group = argMap.getValue(ARG_GROUP);
+
                 String userID = argMap.getValue(ARG_USERID);
 
                 if (group == null)
@@ -223,11 +224,10 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (userID == null)
                     throw new IllegalArgumentException("No userid specified");
 
-                client.addUserMember(new GroupURI(group), new HttpPrincipal(userID));
+                client.addUserMember(group, new HttpPrincipal(userID));
             }
             else if (command.equals(ARG_DEL_MEMBER))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
@@ -235,11 +235,10 @@ public class GMSClientMain implements PrivilegedAction<Object>
                 if (member == null)
                     throw new IllegalArgumentException("No user specified");
 
-                client.removeUserMember(new GroupURI(group), new HttpPrincipal(member));
+                client.removeUserMember(group, new HttpPrincipal(member));
             }
             else if (command.equals(ARG_ADD_ADMIN))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 String userID = argMap.getValue(ARG_USERID);
 
                 if (group == null)
@@ -249,7 +248,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                     throw new IllegalArgumentException("No userid specified");
                 HttpPrincipal hp = new HttpPrincipal(userID);
 
-                Group cur = client.getGroup(new GroupURI(group));
+                Group cur = client.getGroup(group);
                 boolean update = true;
                 for (User admin : cur.getUserAdmins())
                 {
@@ -280,7 +279,6 @@ public class GMSClientMain implements PrivilegedAction<Object>
             }
             else if (command.equals(ARG_DEL_ADMIN))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
@@ -289,7 +287,7 @@ public class GMSClientMain implements PrivilegedAction<Object>
                     throw new IllegalArgumentException("No user specified");
                 HttpPrincipal hp = new HttpPrincipal(userID);
 
-                Group cur = client.getGroup(new GroupURI(group));
+                Group cur = client.getGroup(group);
                 boolean update = false;
                 Iterator<User> iter = cur.getUserAdmins().iterator();
                 while (iter.hasNext())
@@ -319,29 +317,15 @@ public class GMSClientMain implements PrivilegedAction<Object>
             }
             else if (command.equals(ARG_CREATE_GROUP))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
-                GroupURI groupURI = null;
-                try
-                {
-                    groupURI = new GroupURI(group);
-                }
-                catch (Exception e)
-                {
-                    String message = "Invalid group URI format '" +
-                        group + "': " + e.getMessage();
-                    log.debug(message, e);
-                    throw new IllegalArgumentException(message);
-                }
-
                 AccessControlContext accessControlContext = AccessController.getContext();
                 Subject subject = Subject.getSubject(accessControlContext);
                 Set<X500Principal> principals = subject.getPrincipals(X500Principal.class);
                 X500Principal p = principals.iterator().next();
 
-                Group g = new Group(groupURI);
+                Group g = new Group(guri);
 
                 User member = new User();
                 member.getIdentities().add(p);
@@ -350,11 +334,10 @@ public class GMSClientMain implements PrivilegedAction<Object>
             }
             else if (command.equals(ARG_GET_GROUP))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
-                Group g = client.getGroup(new GroupURI(group));
+                Group g = client.getGroup(group);
                 System.out.println("found: " + g.getID());
                 System.out.println("\t" + g.description);
                 System.out.println("owner: " + g.getOwner());
@@ -374,11 +357,10 @@ public class GMSClientMain implements PrivilegedAction<Object>
             }
             else if (command.equals(ARG_DELETE_GROUP))
             {
-                String group = argMap.getValue(ARG_GROUP);
                 if (group == null)
                     throw new IllegalArgumentException("No group specified");
 
-                client.deleteGroup(new GroupURI(group));
+                client.deleteGroup(group);
             }
 
             return null;
@@ -389,4 +371,4 @@ public class GMSClientMain implements PrivilegedAction<Object>
             return t;
         }
     }
-}
+}
\ No newline at end of file
diff --git a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
index cfdc3a18..d68ca7f6 100644
--- a/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
+++ b/cadc-access-control/src/test/java/ca/nrc/cadc/ac/client/GMSClientTest.java
@@ -115,11 +115,13 @@ public class GMSClientTest
         final RegistryClient mockRegistryClient =
                 createMock(RegistryClient.class);
 
-//        expect(mockRegistryClient.getServiceURL(serviceID, Standards.UMS_USERS_01, AuthMethod.CERT))
-//            .andReturn(new URL("http://mysite.com/users"));
+        final URI serviceID = URI.create("ivo://mysite.com/users");
+
+        expect(mockRegistryClient.getServiceURL(serviceID, Standards.UMS_USERS_01, AuthMethod.CERT))
+            .andReturn(new URL("http://mysite.com/users"));
 
         replay(mockRegistryClient);
-        GMSClient client = new GMSClient()
+        GMSClient client = new GMSClient(serviceID)
         {
             @Override
             protected RegistryClient getRegistryClient()
@@ -149,15 +151,15 @@ public class GMSClientTest
         final HttpPrincipal test1UserID = new HttpPrincipal("test");
         subject.getPrincipals().add(test1UserID);
 
-        final URI serviceID = URI.create("ivo://example.org/gms");
+        final URI serviceID = URI.create("ivo://mysite.com/users");
         final RegistryClient mockRegistryClient =
                 createMock(RegistryClient.class);
 
         expect(mockRegistryClient.getServiceURL(serviceID, Standards.GMS_GROUPS_01, AuthMethod.CERT ))
-            .andReturn(new URL("http://example.org/gms"));
+            .andReturn(new URL("http://mysite.com/users"));
 
         replay(mockRegistryClient);
-        final GMSClient client = new GMSClient()
+        final GMSClient client = new GMSClient(serviceID)
         {
             @Override
             protected RegistryClient getRegistryClient()
@@ -173,42 +175,46 @@ public class GMSClientTest
             {
 
                 List<Group> initial = client
-                        .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
+                        .getCachedGroups(test1UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", initial);
 
                 // add single group as isMember might do
                 GroupURI group0uri = new GroupURI("ivo://example.org/gms?0");
                 Group group0 = new Group(group0uri);
                 client.addCachedGroup(test1UserID, group0, Role.MEMBER);
-                List<Group> actual = client.getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
+                List<Group> actual = client
+                        .getCachedGroups(test1UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
-                Group g = client.getCachedGroup(test1UserID, group0uri, Role.MEMBER);
+                Group g = client
+                        .getCachedGroup(test1UserID, "0", Role.MEMBER);
                 Assert.assertNotNull("cached group from incomplete cache", g);
 
                 // add all groups like getMemberships might do
                 List<Group> expected = new ArrayList<Group>();
-                Group group1 = new Group(new GroupURI("ivo://example.org/gms?1"));
-                Group group2 = new Group(new GroupURI("ivo://example.org/gms?2"));
+                GroupURI group1uri = new GroupURI("ivo://example.org/gms?1");
+                GroupURI group2uri = new GroupURI("ivo://example.org/gms?2");
+                Group group1 = new Group(group1uri);
+                Group group2 = new Group(group2uri);
                 expected.add(group0);
                 expected.add(group1);
                 expected.add(group2);
 
-                client.setCachedGroups(serviceID, test1UserID, expected, Role.MEMBER);
+                client.setCachedGroups(test1UserID, expected, Role.MEMBER);
 
                 actual = client
-                        .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
+                        .getCachedGroups(test1UserID, Role.MEMBER, true);
                 Assert.assertEquals("Wrong cached groups", expected, actual);
 
                 // check against another role
                 actual = client
-                        .getCachedGroups(serviceID, test1UserID, Role.OWNER, true);
+                        .getCachedGroups(test1UserID, Role.OWNER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 // check against another userid
                 final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
                 actual = client
-                        .getCachedGroups(serviceID, anotherUserID, Role.MEMBER, true);
+                        .getCachedGroups(anotherUserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 return null;
@@ -228,7 +234,7 @@ public class GMSClientTest
             {
 
                 List<Group> initial = client
-                        .getCachedGroups(serviceID, test2UserID, Role.MEMBER, true);
+                        .getCachedGroups(test2UserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", initial);
 
                 List<Group> expected = new ArrayList<Group>();
@@ -237,21 +243,21 @@ public class GMSClientTest
                 expected.add(group1);
                 expected.add(group2);
 
-                client.setCachedGroups(serviceID, test2UserID, expected, Role.MEMBER);
+                client.setCachedGroups(test2UserID, expected, Role.MEMBER);
 
                 List<Group> actual = client
-                        .getCachedGroups(serviceID, test2UserID, Role.MEMBER, true);
+                        .getCachedGroups(test2UserID, Role.MEMBER, true);
                 Assert.assertEquals("Wrong cached groups", expected, actual);
 
                 // check against another role
                 actual = client
-                        .getCachedGroups(serviceID, test2UserID, Role.OWNER, true);
+                        .getCachedGroups(test2UserID, Role.OWNER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 // check against another userid
                 final HttpPrincipal anotherUserID = new HttpPrincipal("anotheruser");
                 actual = client
-                        .getCachedGroups(serviceID, anotherUserID, Role.MEMBER, true);
+                        .getCachedGroups(anotherUserID, Role.MEMBER, true);
                 Assert.assertNull("Cache should be null", actual);
 
                 return null;
@@ -261,7 +267,7 @@ public class GMSClientTest
         // do the same without a subject
 
         List<Group> initial = client
-                .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
+                .getCachedGroups(test1UserID, Role.MEMBER, true);
         Assert.assertNull("Cache should be null", initial);
 
         List<Group> newgroups = new ArrayList<Group>();
@@ -270,10 +276,10 @@ public class GMSClientTest
         newgroups.add(group1);
         newgroups.add(group2);
 
-        client.setCachedGroups(serviceID, test1UserID, newgroups, Role.MEMBER);
+        client.setCachedGroups(test1UserID, newgroups, Role.MEMBER);
 
         List<Group> actual = client
-                .getCachedGroups(serviceID, test1UserID, Role.MEMBER, true);
+                .getCachedGroups(test1UserID, Role.MEMBER, true);
         Assert.assertNull("Cache should still be null", actual);
     }
 
-- 
GitLab