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

134804 mward: Merged mward/repo-1600-zonesfilter (5.2.1) to 5.2.N (5.2.1)
      134686 mward: REPO-1600: implemented logical conjunction for where clause
      e.g.
          isRoot=true AND zones in ('MY.ZONE')


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

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

@@ -64,6 +64,7 @@ import org.alfresco.rest.framework.resource.parameters.Parameters;
 import org.alfresco.rest.framework.resource.parameters.SortColumn;
 import org.alfresco.rest.framework.resource.parameters.where.Query;
 import org.alfresco.rest.framework.resource.parameters.where.QueryHelper;
+import org.alfresco.rest.workflow.api.impl.MapBasedQueryWalker;
 import org.alfresco.rest.workflow.api.impl.MapBasedQueryWalkerOrSupported;
 import org.alfresco.service.cmr.security.AuthorityService;
 import org.alfresco.service.cmr.security.AuthorityType;
@@ -248,25 +249,6 @@ public class GroupsImpl implements Groups
         return CollectionWithPagingInfo.asPaged(paging, groups, pagingResult.hasMoreItems(), totalItems);
     }
 
-//    private boolean groupInZones(Group group, List<String> zonesFilter)
-//    {
-//        Set<String> groupZones = group.getZones();
-//        // The zones may not have been "included" in the request.
-//        if (groupZones == null)
-//        {
-//            groupZones = authorityService.getAuthorityZones(group.getId());
-//        }
-//
-//        for (String zone : zonesFilter)
-//        {
-//            if (groupZones.contains(zone))
-//            {
-//                return true;
-//            }
-//        }
-//        return false;
-//    }
-
     @Override
     public CollectionWithPagingInfo<Group> getGroupsByPersonId(String requestedPersonId, Parameters parameters)
     {
@@ -323,7 +305,7 @@ public class GroupsImpl implements Groups
             Paging paging)
     {
         PagingResults<AuthorityInfo> pagingResult;
-        // TODO: make sure impl works for isRootParam = true,false,null.
+
         if (isRootParam != null)
         {
             List<AuthorityInfo> groupList;
@@ -369,7 +351,18 @@ public class GroupsImpl implements Groups
             }
 
             // Post process paging - this should be moved to service layer.
-            // TODO: filter groupList to remove zones NOT matching the zoneFilter
+
+            if (zoneFilter != null)
+            {
+                // Only AND (e.g. isRoot=true AND zones IN ('MY.APP')) currently supported,
+                // as we can easily post-filter for zones.
+                // If we introduce OR, then a different approach will be needed.
+                groupList = groupList.stream().
+                    filter(authority -> {
+                        Set<String> zones = authorityService.getAuthorityZones(authority.getAuthorityName());
+                        return zones.contains(zoneFilter);
+                    }).collect(Collectors.toList());
+            }
             pagingResult = Util.wrapPagingResults(paging, groupList);
         }
         else
@@ -906,10 +899,16 @@ public class GroupsImpl implements Groups
         return AuthorityType.GROUP.equals(authorityType) || AuthorityType.EVERYONE.equals(authorityType);
     }
 
-    private static class GroupsQueryWalker extends MapBasedQueryWalkerOrSupported
+    private static class GroupsQueryWalker extends MapBasedQueryWalker
     {
         private List<String> zones;
 
+        @Override
+        public void and()
+        {
+            // allow AND, e.g. isRoot=true AND zones in ('BLAH')
+        }
+
         @Override
         public void in(String propertyName, boolean negated, String... propertyValues)
         {

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

@@ -68,6 +68,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
     protected AuthorityService authorityService;
 
     private String rootGroupName = null;
+    private Group rootGroup = null;
     private Group groupA = null;
     private Group groupB = null;
     private GroupMember groupMemberA = null;
@@ -358,9 +359,10 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             List<Group> groups = response.getList();
 
             // We know exactly which groups are in the selected zone.
-            assertEquals(2, groups.size());
+            assertEquals(3, groups.size());
-            assertEquals(groupA, groups.get(0));
+            assertEquals(rootGroup, groups.get(0));
-            assertEquals(groupB, groups.get(1));
+            assertEquals(groupA, groups.get(1));
+            assertEquals(groupB, groups.get(2));
             groups.forEach(group -> assertTrue(group.getZones().contains("APITEST.MYZONE")));
 
             otherParams.put("where", "(zones in ('APITEST.ANOTHER'))");
@@ -388,31 +390,51 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             List<Group> groups = response.getList();
 
             // We know exactly which groups are in the selected zone.
-            assertEquals(2, groups.size());
+            assertEquals(3, groups.size());
-            assertEquals(groupA, groups.get(0));
+            assertEquals(rootGroup, groups.get(0));
-            assertEquals(groupB, groups.get(1));
+            assertEquals(groupA, groups.get(1));
+            assertEquals(groupB, groups.get(2));
             // We haven't included the zones info.
             groups.forEach(group -> assertNull(group.getZones()));
         }
 
         // Filter zones while using where isRoot=true
         // (this causes a different query path to be used)
-        /*{
+        {
             Paging paging = getPaging(0, Integer.MAX_VALUE);
             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);
 
-            // TODO: how will we support all possible boolean operators? AND, OR...?
+            otherParams.put("where", "(isRoot=true AND zones in ('APITEST.MYZONE'))");
-            otherParams.put("where", "(isRoot=true AND zones in ('APITEST.SPECIAL'))");
 
             ListResponse<Group> response = getGroups(paging, otherParams);
             List<Group> groups = response.getList();
 
             assertEquals(1, groups.size());
-            assertEquals(rootGroupName, groups.get(0));
+            assertEquals(rootGroup, groups.get(0));
-            assertTrue(groups.get(0).getZones().contains("APITEST.SPECIAL"));
+            assertTrue(groups.get(0).getZones().contains("APITEST.MYZONE"));
-        }*/
+        }
+
+        // Filter zones while using where isRoot=false
+        {
+            Paging paging = getPaging(0, Integer.MAX_VALUE);
+            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 = getGroups(paging, otherParams);
+            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()));
+        }
 
         // -ve test: invalid zones clause
         {
@@ -439,6 +461,10 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             // 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);
         }
     }
 
@@ -479,7 +505,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser();
 
             rootGroupName = authorityService.createAuthority(AuthorityType.GROUP, groupName);
-            authorityService.addAuthorityToZones(rootGroupName, zoneSet("APITEST.SPECIAL"));
+            authorityService.addAuthorityToZones(rootGroupName, zoneSet("APITEST.MYZONE"));
 
             String groupBAuthorityName = authorityService.createAuthority(AuthorityType.GROUP, "Test_GroupB" + GUID.generate());
             authorityService.addAuthority(rootGroupName, groupBAuthorityName);
@@ -492,6 +518,9 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
             authorityService.addAuthority(groupAAuthorityName, user1);
             authorityService.addAuthority(groupBAuthorityName, user2);
 
+            rootGroup = new Group();
+            rootGroup.setId(rootGroupName);
+
             groupA = new Group();
             groupA.setId(groupAAuthorityName);