ALF-2404 Correct the site membership role listing for user with all-numeric usernames

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@30903 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/templates/webscripts/org/alfresco/repository/site/membership/memberships.get.js

@@ -1,74 +0,0 @@
-/**
- * Implementation of list memberships
- */
-
-function main()
-{
-   // Get the site id
-   var shortName = url.extension.split("/")[0];
-   var site = siteService.getSite(shortName);
-   
-   // get the filters
-   var nameFilter = args["nf"];
-   var roleFilter = args["rf"];
-   var authorityType = args["authorityType"];
-   var sizeString = args["size"];
-   var collapseGroups = false;
-   
-   if (authorityType != null)
-   {
-      if (authorityType.match("USER|GROUP") == null)
-      {
-         status.setCode(status.STATUS_BAD_REQUEST, "The 'authorityType' argument must be either USER or GROUP.");
-         return;
-      }
-      if (authorityType == "USER")
-      {
-         collapseGroups = true;
-      }
-   }
-   
-   var sizeSearch = 0;
-   if(sizeString != null)
-   {
-      sizeSearch = parseInt(sizeString);
-   }
-   
-   // Get the filtered memberships
-   // Comes back as a Map<String, String> containing the full authority name and role
-   var memberships = site.listMembers(nameFilter, roleFilter, sizeSearch, collapseGroups);
-   
-   // Get a list of all the users resolved to person nodes
-   var authorities = Array(memberships.length);
-   var roles = {};
-   
-   for (userName in memberships)
-   {
-      var membershipRole = memberships[userName];
-      if (userName.match("^GROUP_"))
-      {
-         if (authorityType == null || authorityType == "GROUP")
-         {
-            // make sure the keys are strings - as usernames may be all numbers!
-            authorities['_' + userName] = groups.getGroupForFullAuthorityName(userName);
-            roles['_' + userName] = membershipRole;
-         }
-      }
-      else
-      {
-         if (authorityType == null || authorityType == "USER")
-         {
-            // make sure the keys are strings - as usernames may be all numbers!
-            authorities['_' + userName] = people.getPerson(userName);
-            roles['_' + userName] = membershipRole;
-         }
-      }
-   }
-   
-   // Pass the information to the template
-   model.site = site;
-   model.roles = roles;
-   model.authorities = authorities;
-}
-
-main();

config/alfresco/web-scripts-application-context.xml

@@ -712,15 +712,30 @@
    <!-- Site Service REST API -->
    <!--                         -->
    
+   <!-- abstract site web script -->
+   <bean id="abstractSiteWebScript"
+          class="org.alfresco.repo.web.scripts.site.AbstractSiteWebscript"
+          parent="webscript" abstract="true">
+      <property name="siteService" ref="SiteService"/>
+      <property name="authorityService" ref="AuthorityService"/>
+   </bean>
+
+   <!-- Lists the membership of a site -->
+   <bean id="webscript.org.alfresco.repository.site.membership.memberships.get" class="org.alfresco.repo.web.scripts.site.SiteMembershipsGet" parent="abstractSiteWebScript">
+      <property name="personService" ref="PersonService"/>
+      <property name="scriptAuthorityService" ref="authorityServiceScript"/>
+   </bean>
+
    <!-- Exports a site's contents as ACP+AVM dumps -->
    <bean id="webscript.org.alfresco.repository.site.site-export.get" class="org.alfresco.repo.web.scripts.site.SiteExportGet" parent="webscript">
-      <property name="authorityService" ref="AuthorityService"/>
+      <property name="siteService" ref="SiteService"/>
       <property name="mimetypeService" ref="MimetypeService"/>
       <property name="exporterService" ref="ExporterService"/>
-      <property name="siteService" ref="SiteService"/>
+      <property name="authorityService" ref="AuthorityService"/>
       <property name="authenticationContextManager" ref="Authentication"/>
    </bean>
 
+
    <!--                         -->
    <!-- Person Service REST API -->
    <!--                         -->

source/java/org/alfresco/repo/web/scripts/site/AbstractSiteWebScript.java

@@ -0,0 +1,77 @@
+/*
+ * Copyright (C) 2005-2011 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.web.scripts.site;
+
+import java.util.Map;
+
+import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.site.SiteInfo;
+import org.alfresco.service.cmr.site.SiteService;
+import org.springframework.extensions.webscripts.Cache;
+import org.springframework.extensions.webscripts.DeclarativeWebScript;
+import org.springframework.extensions.webscripts.Status;
+import org.springframework.extensions.webscripts.WebScriptException;
+import org.springframework.extensions.webscripts.WebScriptRequest;
+
+/**
+ * @author Nick Burch
+ * @since 4.0
+ */
+public abstract class AbstractSiteWebScript extends DeclarativeWebScript
+{
+    protected SiteService siteService;
+    protected AuthorityService authorityService;
+    
+    public void setSiteService(SiteService siteService)
+    {
+        this.siteService = siteService;
+    }
+    
+    public void setAuthorityService(AuthorityService authorityService)
+    {
+        this.authorityService = authorityService;
+    }
+    
+    
+    protected static String buildSiteGroup(SiteInfo site)
+    {
+        return "GROUP_site_" + site.getShortName();
+    }
+
+    @Override
+    protected Map<String, Object> executeImpl(WebScriptRequest req,
+          Status status, Cache cache) 
+    {
+       // Grab the site
+       String siteName = 
+           req.getServiceMatch().getTemplateVars().get("shortname");
+       SiteInfo site = siteService.getSite(siteName);
+       if (site == null)
+       {
+           throw new WebScriptException(
+                   Status.STATUS_NOT_FOUND, 
+                   "No Site found with that short name");
+       }
+       
+       // Process
+       return executeImpl(site, req, status, cache);
+    }
+    protected abstract Map<String, Object> executeImpl(SiteInfo site,
+          WebScriptRequest req, Status status, Cache cache);
+}

source/java/org/alfresco/repo/web/scripts/site/SiteExportGet.java

@@ -167,7 +167,7 @@ public class SiteExportGet extends AbstractWebScript
     protected void doPeopleACPExport(SiteInfo site, CloseIgnoringOutputStream writeTo) throws IOException
     {
         // Find the root group
-        String siteGroup = buildSiteGroup(site);
+        String siteGroup = AbstractSiteWebScript.buildSiteGroup(site);
         
         // Now get all people in the child groups
         Set<String> siteUsers = authorityService.getContainedAuthorities(
@@ -205,7 +205,7 @@ public class SiteExportGet extends AbstractWebScript
     protected void doGroupExport(SiteInfo site, CloseIgnoringOutputStream writeTo) throws IOException
     {
         // Find the root group
-        String siteGroup = buildSiteGroup(site);
+        String siteGroup = AbstractSiteWebScript.buildSiteGroup(site);
         
         // Get all the child groups of the site (but not children of them)
         Set<String> siteGroups = authorityService.getContainedAuthorities(
@@ -265,7 +265,7 @@ public class SiteExportGet extends AbstractWebScript
         List<NodeRef> exportNodes = new ArrayList<NodeRef>();
         
         // Identify all the users
-        String siteGroup = buildSiteGroup(site);
+        String siteGroup = AbstractSiteWebScript.buildSiteGroup(site);
         Set<String> siteUsers = authorityService.getContainedAuthorities(
                 AuthorityType.USER, siteGroup, false);
         
@@ -302,11 +302,6 @@ public class SiteExportGet extends AbstractWebScript
         exporterService.exportView(handler, parameters, null);
     }
     
-    protected String buildSiteGroup(SiteInfo site)
-    {
-        return "GROUP_site_" + site.getShortName();
-    }
-    
     protected static class CloseIgnoringOutputStream extends FilterOutputStream
     {
         public CloseIgnoringOutputStream(OutputStream out)

source/java/org/alfresco/repo/web/scripts/site/SiteMembershipsGet.java

@@ -0,0 +1,150 @@
+/*
+ * Copyright (C) 2005-2011 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.web.scripts.site;
+
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.alfresco.repo.security.authority.script.ScriptAuthorityService;
+import org.alfresco.service.cmr.security.PersonService;
+import org.alfresco.service.cmr.site.SiteInfo;
+import org.springframework.extensions.webscripts.Cache;
+import org.springframework.extensions.webscripts.Status;
+import org.springframework.extensions.webscripts.WebScriptException;
+import org.springframework.extensions.webscripts.WebScriptRequest;
+
+/**
+ * Lists the members of a site, optionally filtering by name, role
+ *  or authority type.
+ * 
+ * Based on the old memberships.get.js webscript controller
+ * 
+ * @author Nick Burch
+ * @since 4.0
+ */
+public class SiteMembershipsGet extends AbstractSiteWebScript
+{
+    private PersonService personService;
+    private ScriptAuthorityService scriptAuthorityService;
+   
+    public void setPersonService(PersonService personService) 
+    {
+      this.personService = personService;
+    }
+
+    public void setScriptAuthorityService(ScriptAuthorityService scriptAuthorityService) 
+    {
+       this.scriptAuthorityService = scriptAuthorityService;
+    }
+
+   @Override
+    protected Map<String, Object> executeImpl(SiteInfo site,
+          WebScriptRequest req, Status status, Cache cache) 
+    {
+       // Grab out filters
+       String nameFilter = req.getParameter("nf");
+       String roleFilter = req.getParameter("rf");
+       String authorityType = req.getParameter("authorityType");
+       String sizeS = req.getParameter("size");
+       boolean collapseGroups = false;
+       
+       // Sanity check the types
+       if(authorityType != null)
+       {
+          if("USER".equals(authorityType))
+          {
+             collapseGroups = true;
+          }
+          else if("GROUP".equals(authorityType))
+          {
+             // No extra settings needed
+          }
+          else
+          {
+             throw new WebScriptException(Status.STATUS_BAD_REQUEST,
+                   "The Authority must be one of USER or GROUP");
+          }
+       }
+
+       // Figure out what to limit to, if anythign
+       int limit = 0;
+       if(sizeS != null)
+       {
+          try
+          {
+             limit = Integer.parseInt(sizeS);
+          }
+          catch(NumberFormatException e)
+          {
+             throw new WebScriptException(Status.STATUS_BAD_REQUEST,
+                   "Invalid size specified");
+          }
+       }
+       
+       
+       // Fetch the membership details of the site
+       Map<String, String> members =  this.siteService.listMembers(
+             site.getShortName(), nameFilter, roleFilter, limit, collapseGroups);
+       
+       
+       // Process it ready for FreeMarker
+       // Note that as usernames may be all numbers, we need to
+       //  prefix them with an underscore otherwise FTL can get confused!
+       Map<String,Object> authorities = new HashMap<String, Object>(members.size());
+       Map<String,String> roles = new LinkedHashMap<String, String>(members.size()); 
+       for(String authorityName : members.keySet())
+       {
+          String role = members.get(authorityName);
+          String ftlSafeName = "_" + authorityName;
+          
+          if(authorityName.startsWith("GROUP_"))
+          {
+             if(authorityType == null || authorityType.equals("GROUP"))
+             {
+                // Record the details
+                authorities.put(
+                      ftlSafeName,
+                      scriptAuthorityService.getGroupForFullAuthorityName(authorityName)
+                );
+                roles.put(ftlSafeName, role);
+             }
+          }
+          else
+          {
+             if(authorityType == null || authorityType.equals("USER"))
+             {
+                // Record the details
+                authorities.put(
+                      ftlSafeName,
+                      personService.getPerson(authorityName)
+                );
+                roles.put(ftlSafeName, role);
+             }
+          }
+       }
+       
+       // Pass the details to freemarker
+       Map<String,Object> model = new HashMap<String, Object>();
+       model.put("site", site);
+       model.put("roles", roles);
+       model.put("authorities", authorities);
+       return model;
+    }
+}

source/java/org/alfresco/repo/web/scripts/site/SiteServiceTest.java

@@ -70,6 +70,7 @@ public class SiteServiceTest extends BaseWebScriptTest
     private static final String USER_ONE = "SiteTestOne";
     private static final String USER_TWO = "SiteTestTwo";
     private static final String USER_THREE = "SiteTestThree";
+    private static final String USER_NUMERIC = "1234567890";
     
     private static final String URL_SITES = "/api/sites";
     private static final String URL_SITES_QUERY = URL_SITES + "/query";
@@ -96,6 +97,7 @@ public class SiteServiceTest extends BaseWebScriptTest
         createUser(USER_ONE);
         createUser(USER_TWO);
         createUser(USER_THREE);
+        createUser(USER_NUMERIC);
         
         // Do tests as user one
         this.authenticationComponent.setCurrentUser(USER_ONE);
@@ -117,6 +119,14 @@ public class SiteServiceTest extends BaseWebScriptTest
             this.personService.createPerson(ppOne);
         }
     }
+    private void deleteUser(String username)
+    {
+       this.personService.deletePerson(username);
+       if(this.authenticationService.authenticationExists(username))
+       {
+          this.authenticationService.deleteAuthentication(username);
+       }
+    }
     
     @Override
     protected void tearDown() throws Exception
@@ -132,6 +142,12 @@ public class SiteServiceTest extends BaseWebScriptTest
         
         // Clear the list
         this.createdSites.clear();
+        
+        // Clear the users
+        deleteUser(USER_ONE);
+        deleteUser(USER_TWO);
+        deleteUser(USER_THREE);
+        deleteUser(USER_NUMERIC);
     }
     
     public void testCreateSite() throws Exception
@@ -350,6 +366,42 @@ public class SiteServiceTest extends BaseWebScriptTest
         JSONArray result2 = new JSONArray(response.getContentAsString());
         assertNotNull(result2);
         assertEquals(2, result2.length());
+        
+        
+        // Add another user, with a fully numeric username
+        membership = new JSONObject();
+        membership.put("role", SiteModel.SITE_CONTRIBUTOR);
+        person = new JSONObject();
+        person.put("userName", USER_NUMERIC);
+        membership.put("person", person);
+        response = sendRequest(new PostRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS, membership.toString(), "application/json"), 200);
+        
+        // Check the details are correct for one user
+        membership = new JSONObject(response.getContentAsString());
+        assertEquals(SiteModel.SITE_CONTRIBUTOR, membership.get("role"));
+        assertEquals(USER_NUMERIC, membership.getJSONObject("authority").get("userName"));
+        
+        
+        // Check the membership list
+        response = sendRequest(new GetRequest(URL_SITES + "/" + shortName + URL_MEMBERSHIPS), 200);
+        String json = response.getContentAsString();
+        JSONArray result3 = new JSONArray(json);
+        assertNotNull(result3);
+        assertEquals(3, result3.length());
+        
+        // Check the everyone has the correct membership
+        // (The webscript returns the users in order to make testing easier)
+        membership = result3.getJSONObject(0);
+        assertEquals(SiteModel.SITE_MANAGER, membership.get("role"));
+        assertEquals(USER_ONE, membership.getJSONObject("authority").get("userName"));
+        
+        membership = result3.getJSONObject(1);
+        assertEquals(SiteModel.SITE_CONSUMER, membership.get("role"));
+        assertEquals(USER_TWO, membership.getJSONObject("authority").get("userName"));
+        
+        membership = result3.getJSONObject(2);
+        assertEquals(SiteModel.SITE_CONTRIBUTOR, membership.get("role"));
+        assertEquals(USER_NUMERIC, membership.getJSONObject("authority").get("userName"));
     }
     
     public void testGetMembership() throws Exception