Merged mward/repo-1600-zonesfilter (5.2.1) to 5.2.N (5.2.1)

134741 mward: REPO-1600: added support for zone filtering to GET /people/{personId}/groups
   Also brought in-line with spec to return an empty list rather than a 404 if the zone is non-existent.


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@134806 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/impl/GroupsImpl.java

@@ -42,6 +42,7 @@ import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.alfresco.query.CannedQueryPageDetails;
+import org.alfresco.query.EmptyPagingResults;
 import org.alfresco.query.PagingRequest;
 import org.alfresco.query.PagingResults;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
@@ -193,22 +194,7 @@ public class GroupsImpl implements Groups
             List<String> zonesParam = propertyWalker.getZones();
             if (zonesParam != null)
             {
-                if (zonesParam.size() > MAX_ZONES)
+                validateZonesParam(zonesParam);
-                {
-                    throw new IllegalArgumentException("A maximum of " + MAX_ZONES + " zones may be specified.");
-                }
-                else if (zonesParam.isEmpty())
-                {
-                    throw new IllegalArgumentException("Zones filter list cannot be empty.");
-                }
-                // Validate each zone name
-                zonesParam.forEach(zone -> {
-                    if (zone.isEmpty())
-                    {
-                        throw new IllegalArgumentException("Zone name cannot be empty (i.e. '')");
-                    }
-                });
-
                 zoneFilter = zonesParam.get(0);
             }
 
@@ -224,7 +210,8 @@ public class GroupsImpl implements Groups
         }
         catch (UnknownAuthorityException e)
         {
-            throw new EntityNotFoundException("Zone "+zoneFilter+" does not exist.");
+            // Non-existent zone
+            pagingResult = new EmptyPagingResults<>();
         }
 
         // Create response.
@@ -249,6 +236,25 @@ public class GroupsImpl implements Groups
         return CollectionWithPagingInfo.asPaged(paging, groups, pagingResult.hasMoreItems(), totalItems);
     }
 
+    private void validateZonesParam(List<String> zonesParam)
+    {
+        if (zonesParam.size() > MAX_ZONES)
+        {
+            throw new IllegalArgumentException("A maximum of " + MAX_ZONES + " zones may be specified.");
+        }
+        else if (zonesParam.isEmpty())
+        {
+            throw new IllegalArgumentException("Zones filter list cannot be empty.");
+        }
+        // Validate each zone name
+        zonesParam.forEach(zone -> {
+            if (zone.isEmpty())
+            {
+                throw new IllegalArgumentException("Zone name cannot be empty (i.e. '')");
+            }
+        });
+    }
+
     @Override
     public CollectionWithPagingInfo<Group> getGroupsByPersonId(String requestedPersonId, Parameters parameters)
     {
@@ -265,6 +271,20 @@ public class GroupsImpl implements Groups
             throw new PermissionDeniedException();
         }
 
+        Query q = parameters.getQuery();
+        List<String> zonesParam = null;
+        if (q != null)
+        {
+            GroupsQueryWalker propertyWalker = new GroupsQueryWalker(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null);
+            QueryHelper.walk(q, propertyWalker);
+            zonesParam = propertyWalker.getZones();
+            if (zonesParam != null)
+            {
+                validateZonesParam(zonesParam);
+            }
+        }
+        final String zoneFilter = zonesParam != null ? zonesParam.get(0) : null;
+
         final List<String> includeParam = parameters.getInclude();
         Paging paging = parameters.getPaging();
 
@@ -280,6 +300,7 @@ public class GroupsImpl implements Groups
         // a suitable list of AuthorityInfo objects.
         List<AuthorityInfo> groupAuthorities = userAuthorities.stream().
                 filter(a -> a.startsWith(AuthorityType.GROUP.getPrefixString())).
+                filter(a -> zonePredicate(a, zoneFilter)).
                 map(this::getAuthorityInfo).
                 sorted(new AuthorityInfoComparator(sortProp.getFirst(), sortProp.getSecond())).
                 collect(Collectors.toList());
@@ -292,8 +313,7 @@ public class GroupsImpl implements Groups
 
         // Transform the page of results into Group objects
         final Set<String> rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP);
-        // Zone filter not supported currently.
+
-        final String zoneFilter = null;
         List<Group> groups = page.stream().
                 map(authority -> getGroup(authority, includeParam, rootAuthorities)).
                 collect(Collectors.toList());
@@ -316,7 +336,7 @@ public class GroupsImpl implements Groups
                 // list of root authorities.
                 List<AuthorityInfo> authorities = rootAuthorities.stream().
                         map(this::getAuthorityInfo).
-                        filter(auth -> matchZoneFilter(auth, zoneFilter)).
+                        filter(auth -> zonePredicate(auth.getAuthorityName(), zoneFilter)).
                         collect(Collectors.toList());
                 groupList = new ArrayList<>(rootAuthorities.size());
                 groupList.addAll(authorities);
@@ -367,12 +387,44 @@ public class GroupsImpl implements Groups
         return pagingResult;
     }
 
-    private boolean matchZoneFilter(AuthorityInfo authority, String zone)
+    /**
+     * Checks to see if the named group authority should be included in results
+     * when filtered by zone.
+     *
+     * @see #zonePredicate(Set, String)
+     * @param groupName
+     * @param zone
+     * @return true if result should be included.
+     */
+    private boolean zonePredicate(String groupName, String zone)
     {
+        Set<String> zones = null;
         if (zone != null)
         {
-            Set<String> zones = authorityService.getAuthorityZones(authority.getAuthorityName());
+            // Don't bother doing this lookup unless the zone filter is non-null.
-            return zones.contains(zone);
+            zones = authorityService.getAuthorityZones(groupName);
+        }
+        return zonePredicate(zones, zone);
+    }
+
+    /**
+     * Checks a list of zones to see if it matches the supplied zone filter
+     * ({@code requiredZone} parameter).
+     * <p>
+     * If the requiredZone parameter is null, then the filter will not be applied (returns true.)
+     * <p>
+     * If the requiredZone parameter is not null (i.e. a filter must be applied) and the
+     * {@code zones}) list is {@code null} then the predicate will return false.
+     *
+     * @param zones
+     * @param requiredZone
+     * @return true if result should be included.
+     */
+    private boolean zonePredicate(Set<String> zones, String requiredZone)
+    {
+        if (requiredZone != null)
+        {
+            return zones != null && zones.contains(requiredZone);
         }
         return true;
     }

source/test-java/org/alfresco/rest/api/tests/GroupsTest.java

@@ -25,16 +25,6 @@
  */
 package org.alfresco.rest.api.tests;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-
-import java.util.*;
-
-import javax.servlet.http.HttpServletResponse;
-
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.rest.AbstractSingleNetworkSiteTest;
 import org.alfresco.rest.api.tests.client.PublicApiClient;
@@ -54,6 +44,11 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import javax.servlet.http.HttpServletResponse;
+import java.util.*;
+
+import static org.junit.Assert.*;
+
 /**
  * V1 REST API tests for managing Groups
  *
@@ -415,6 +410,12 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             assertEquals(1, groups.size());
             assertEquals(rootGroup, groups.get(0));
             assertTrue(groups.get(0).getZones().contains("APITEST.MYZONE"));
+
+            // Zone that doesn't exist.
+            otherParams.put("where", "(isRoot=true AND zones in ('I.DO.NOT.EXIST'))");
+            response = getGroups(paging, otherParams, "Incorrect response", 200);
+            groups = response.getList();
+            assertTrue(groups.isEmpty());
         }
 
         // Filter zones while using where isRoot=false
@@ -434,6 +435,12 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             assertEquals(groupB, groups.get(1));
             // We haven't included the zones info.
             groups.forEach(group -> assertNull(group.getZones()));
+
+            // Zone that doesn't exist.
+            otherParams.put("where", "(isRoot=false AND zones in ('I.DO.NOT.EXIST'))");
+            response = getGroups(paging, otherParams, "Incorrect response", 200);
+            groups = response.getList();
+            assertTrue(groups.isEmpty());
         }
 
         // -ve test: invalid zones clause
@@ -458,10 +465,6 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             otherParams.put("where", "(zones in ('', 'APP.DEFAULT', '', 'APITEST.MYZONE'))");
             getGroups(paging, otherParams, "Incorrect response", 400);
 
-            // A zone that isn't in use or doesn't exist
-            otherParams.put("where", "(zones in ('NON.EXISTENT'))");
-            getGroups(paging, otherParams, "Incorrect response", 404);
-
             // OR operator not currently supported
             otherParams.put("where", "(isRoot=true OR zones in ('APP.DEFAULT'))");
             getGroups(paging, otherParams, "Incorrect response", 400);
@@ -815,6 +818,91 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
 
             getGroupsByPersonId(personAlice.getId(), paging, otherParams, HttpServletResponse.SC_BAD_REQUEST);
         }
+
+        // Filter by zone, use the -me- alias.
+        {
+            Map<String, String> params = new HashMap<>();
+            params.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES);
+
+            params.put("where", "(zones in ('APP.DEFAULT'))");
+
+            // Use the -me- alias
+            ListResponse<Group> response = groupsProxy.
+                    getGroupsByPersonId("-me-", params, "Couldn't get user's groups", 200);
+            List<Group> groups = response.getList();
+
+            assertFalse(groups.isEmpty());
+            // All groups should contain the selected zone.
+            groups.forEach(group -> assertTrue(group.getZones().contains("APP.DEFAULT")));
+        }
+
+        // Filter by zone, use the -me- alias.
+        {
+            Map<String, String> params = new HashMap<>();
+            params.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES);
+
+            params.put("where", "(zones in ('APITEST.MYZONE'))");
+
+            // Use the -me- alias
+            ListResponse<Group> response = groupsProxy.
+                    getGroupsByPersonId("-me-", params, "Couldn't get user's groups", 200);
+            List<Group> groups = response.getList();
+
+            assertEquals(3, groups.size());
+            // All groups should contain the selected zone.
+            groups.forEach(group -> assertTrue(group.getZones().contains("APITEST.MYZONE")));
+        }
+
+        // Filter by zone - use the person's ID, without "include"-ing zones
+        {
+            Map<String, String> params = new HashMap<>();
+            params.put("where", "(zones in ('APITEST.ANOTHER'))");
+
+            ListResponse<Group> response = groupsProxy.
+                    getGroupsByPersonId(personAlice.getId(), params, "Couldn't get user's groups", 200);
+            List<Group> groups = response.getList();
+
+            assertEquals(1, groups.size());
+            // We haven't included the zone info
+            groups.forEach(group -> assertNull(group.getZones()));
+        }
+
+        {
+            // Zone that doesn't exist.
+            Map<String, String> params = new HashMap<>();
+            params.put("where", "(isRoot=false AND zones in ('I.DO.NOT.EXIST'))");
+            ListResponse<Group> response = groupsProxy.
+                    getGroupsByPersonId(personAlice.getId(), params, "Incorrect response", 200);
+            List<Group> groups = response.getList();
+            assertTrue(groups.isEmpty());
+        }
+
+        // -ve test: invalid zones clause
+        {
+            Paging paging = getPaging(0, Integer.MAX_VALUE);
+            Map<String, String> otherParams = new HashMap<>();
+            otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES);
+
+            // Empty zone list
+            otherParams.put("where", "(zones in ())");
+            groupsProxy.getGroupsByPersonId(personAlice.getId(), otherParams, "Incorrect response", 400);
+
+            // Empty zone name
+            otherParams.put("where", "(zones in (''))");
+            groupsProxy.getGroupsByPersonId(personAlice.getId(), otherParams, "Incorrect response", 400);
+
+            // Too many zones
+            otherParams.put("where", "(zones in ('APP.DEFAULT', 'APITEST.MYZONE'))");
+            groupsProxy.getGroupsByPersonId(personAlice.getId(), otherParams, "Incorrect response", 400);
+
+            // "A series of unfortunate errors"
+            otherParams.put("where", "(zones in ('', 'APP.DEFAULT', '', 'APITEST.MYZONE'))");
+            groupsProxy.getGroupsByPersonId(personAlice.getId(), otherParams, "Incorrect response", 400);
+
+            // OR operator not currently supported
+            otherParams.put("where", "(isRoot=true OR zones in ('APP.DEFAULT'))");
+            groupsProxy.getGroupsByPersonId(personAlice.getId(), otherParams, "Incorrect response", 400);
+        }
     }
 
     private void testGetGroupMembersByGroupId() throws Exception