Merged 5.2.N (5.2.2) to HEAD (5.2)

134807 mward: Merged mward/repo-1600-zonesfilter (5.2.1) to 5.2.N (5.2.1)
      134742 mward: REPO-1583: added isRoot filtering.


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

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

@@ -181,23 +181,21 @@ public class GroupsImpl implements Groups
         Pair<String, Boolean> sortProp = getGroupsSortProp(parameters);
 
         // Parse where clause properties.
-        Boolean isRootParam = null;
         Query q = parameters.getQuery();
-
+        Boolean isRootParam = null;
         String zoneFilter = null;
         if (q != null)
         {
-            GroupsQueryWalker propertyWalker = new GroupsQueryWalker(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null);
+            GroupsQueryWalker propertyWalker = new GroupsQueryWalker();
             QueryHelper.walk(q, propertyWalker);
 
-            isRootParam = propertyWalker.getProperty(PARAM_IS_ROOT, WhereClauseParser.EQUALS, Boolean.class);
+            isRootParam = propertyWalker.getIsRoot();
             List<String> zonesParam = propertyWalker.getZones();
             if (zonesParam != null)
             {
                 validateZonesParam(zonesParam);
                 zoneFilter = zonesParam.get(0);
             }
-
         }
 
         final AuthorityType authorityType = AuthorityType.GROUP;
@@ -272,18 +270,21 @@ public class GroupsImpl implements Groups
         }
 
         Query q = parameters.getQuery();
-        List<String> zonesParam = null;
+        Boolean isRootParam = null;
+        String zoneFilter = null;
         if (q != null)
         {
-            GroupsQueryWalker propertyWalker = new GroupsQueryWalker(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null);
+            GroupsQueryWalker propertyWalker = new GroupsQueryWalker();
             QueryHelper.walk(q, propertyWalker);
-            zonesParam = propertyWalker.getZones();
+
+            isRootParam = propertyWalker.getIsRoot();
+            List<String> zonesParam = propertyWalker.getZones();
             if (zonesParam != null)
             {
                 validateZonesParam(zonesParam);
+                zoneFilter = zonesParam.get(0);
             }
         }
-        final String zoneFilter = zonesParam != null ? zonesParam.get(0) : null;
 
         final List<String> includeParam = parameters.getInclude();
         Paging paging = parameters.getPaging();
@@ -296,11 +297,16 @@ public class GroupsImpl implements Groups
         Set<String> userAuthorities = runAsSystem(
                 () -> authorityService.getAuthoritiesForUser(personId));
 
+        final Set<String> rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP);
+
         // Filter, transform and sort the list of user authorities into
         // a suitable list of AuthorityInfo objects.
+        final String finalZoneFilter = zoneFilter;
+        final Boolean finalIsRootParam = isRootParam;
         List<AuthorityInfo> groupAuthorities = userAuthorities.stream().
                 filter(a -> a.startsWith(AuthorityType.GROUP.getPrefixString())).
-                filter(a -> zonePredicate(a, zoneFilter)).
+                filter(a -> isRootPredicate(finalIsRootParam, rootAuthorities, a)).
+                filter(a -> zonePredicate(a, finalZoneFilter)).
                 map(this::getAuthorityInfo).
                 sorted(new AuthorityInfoComparator(sortProp.getFirst(), sortProp.getSecond())).
                 collect(Collectors.toList());
@@ -312,8 +318,6 @@ public class GroupsImpl implements Groups
         int totalItems = pagingResult.getTotalResultCount().getFirst();
 
         // Transform the page of results into Group objects
-        final Set<String> rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP);
-
         List<Group> groups = page.stream().
                 map(authority -> getGroup(authority, includeParam, rootAuthorities)).
                 collect(Collectors.toList());
@@ -407,6 +411,27 @@ public class GroupsImpl implements Groups
         return zonePredicate(zones, zone);
     }
 
+    /**
+     * Test to see if a result should be included, using the isRoot parameter.
+     * <p>
+     * If isRootParam is null, then no results will be filtered. Otherwise
+     * results will be filtered to return only those that are root or non-root,
+     * depending on the value of isRootParam.
+     *
+     * @param isRootParam
+     * @param rootAuthorities
+     * @param authority
+     * @return
+     */
+    private boolean isRootPredicate(Boolean isRootParam, Set<String> rootAuthorities, String authority)
+    {
+        if (isRootParam != null)
+        {
+            return isRootParam == isRootAuthority(rootAuthorities, authority);
+        }
+        return true;
+    }
+
     /**
      * Checks a list of zones to see if it matches the supplied zone filter
      * ({@code requiredZone} parameter).
@@ -957,6 +982,11 @@ public class GroupsImpl implements Groups
     {
         private List<String> zones;
 
+        public GroupsQueryWalker()
+        {
+            super(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null);
+        }
+
         @Override
         public void and()
         {
@@ -972,11 +1002,6 @@ public class GroupsImpl implements Groups
             }
         }
 
-        public GroupsQueryWalker(Set<String> supportedEqualsParameters, Set<String> supportedMatchesParameters)
-        {
-            super(supportedEqualsParameters, supportedMatchesParameters);
-        }
-
         /**
          * The list of zones specified in the where clause.
          *
@@ -986,5 +1011,15 @@ public class GroupsImpl implements Groups
         {
             return zones;
         }
+
+        /**
+         * Get the value of the isRoot clause, if specified.
+         *
+         * @return The isRoot param if specified, null if not.
+         */
+        public Boolean getIsRoot()
+        {
+            return getProperty(PARAM_IS_ROOT, WhereClauseParser.EQUALS, Boolean.class);
+        }
     }
 }

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

@@ -724,6 +724,25 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             assertEquals("GROUP_ALFRESCO_ADMINISTRATORS", it.next().getId());
         }
 
+        // Filter by isRoot
+        {
+            Map<String, String> params = new HashMap<>();
+
+            params.put("where", "(isRoot=true)");
+            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 be root groups.
+            groups.forEach(group -> assertTrue(group.getIsRoot()));
+
+            params.put("where", "(isRoot=false)");
+            response = groupsProxy.getGroupsByPersonId("-me-", params, "Couldn't get user's groups", 200);
+            groups = response.getList();
+            assertFalse(groups.isEmpty());
+            // All groups should be non-root groups.
+            groups.forEach(group -> assertFalse(group.getIsRoot()));
+        }
+
         // -ve test: attempt to get groups for non-existent person
         {
             groupsProxy.getGroupsByPersonId("i-do-not-exist", null, "Incorrect response", 404);
@@ -794,6 +813,52 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             checkList(respOrderAsc.getList(), resp.getPaging(), resp);
         }
 
+        // Sorting should be the same regardless of implementation (canned query
+        // or postprocessing).
+        {
+            // paging
+            Paging paging = getPaging(0, Integer.MAX_VALUE);
+
+            Map<String, String> otherParams = new HashMap<>();
+            addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, null);
+
+            // Get and sort groups using canned query.
+            ListResponse<Group> respCannedQuery = getGroupsByPersonId(personAlice.getId(), paging, otherParams);
+
+            // Get and sort groups using postprocessing.
+            otherParams.put("where", "(isRoot=true)");
+            ListResponse<Group> respPostProcess = getGroupsByPersonId(personAlice.getId(), paging, otherParams);
+
+            List<Group> expected = respCannedQuery.getList();
+            expected.retainAll(respPostProcess.getList());
+
+            // If this assertion fails, then the tests aren't providing any value - change them!
+            assertTrue("List doesn't contain enough items for test to be conclusive.", expected.size() > 0);
+            checkList(expected, respPostProcess.getPaging(), respPostProcess);
+        }
+
+        {
+            // paging
+            Paging paging = getPaging(0, Integer.MAX_VALUE);
+
+            Map<String, String> otherParams = new HashMap<>();
+            addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_ID, null);
+
+            // Get and sort groups using canned query.
+            ListResponse<Group> respCannedQuery = getGroupsByPersonId(personAlice.getId(), paging, otherParams);
+
+            // Get and sort groups using postprocessing.
+            otherParams.put("where", "(isRoot=true)");
+            ListResponse<Group> respPostProcess = getGroupsByPersonId(personAlice.getId(), paging, otherParams);
+
+            List<Group> expected = respCannedQuery.getList();
+            expected.retainAll(respPostProcess.getList());
+
+            // If this assertion fails, then the tests aren't providing any value - change them!
+            assertTrue("List doesn't contain enough items for test to be conclusive.", expected.size() > 0);
+            checkList(expected, respPostProcess.getPaging(), respPostProcess);
+        }
+
         // Sort by id.
         {
             // paging
@@ -867,13 +932,55 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             groups.forEach(group -> assertNull(group.getZones()));
         }
 
+        // Filter zones while using where isRoot=true
+        // (this causes a different query path to be used)
         {
-            // Zone that doesn't exist.
-            Map<String, String> params = new HashMap<>();
-            params.put("where", "(isRoot=false AND zones in ('I.DO.NOT.EXIST'))");
+            Map<String, String> otherParams = new HashMap<>();
+            // Ensure predictable result ordering
+            addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, true);
+            otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES);
+
+            otherParams.put("where", "(isRoot=true AND zones in ('APITEST.MYZONE'))");
+
             ListResponse<Group> response = groupsProxy.
-                    getGroupsByPersonId(personAlice.getId(), params, "Incorrect response", 200);
+                    getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200);
             List<Group> groups = response.getList();
+
+            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 = groupsProxy.
+                    getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200);
+            groups = response.getList();
+            assertTrue(groups.isEmpty());
+        }
+
+        // Filter zones while using where isRoot=false
+        {
+            Map<String, String> otherParams = new HashMap<>();
+            // Ensure predictable result ordering
+            addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, true);
+
+            otherParams.put("where", "(isRoot=false AND zones in ('APITEST.MYZONE'))");
+
+            ListResponse<Group> response = groupsProxy.
+                    getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200);
+            List<Group> groups = response.getList();
+
+            assertEquals(2, groups.size());
+            assertEquals(groupA, groups.get(0));
+            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 = groupsProxy.
+                    getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200);
+            groups = response.getList();
             assertTrue(groups.isEmpty());
         }