diff --git a/source/java/org/alfresco/rest/api/impl/GroupsImpl.java b/source/java/org/alfresco/rest/api/impl/GroupsImpl.java index b293f57eea..aab90a5e63 100644 --- a/source/java/org/alfresco/rest/api/impl/GroupsImpl.java +++ b/source/java/org/alfresco/rest/api/impl/GroupsImpl.java @@ -181,23 +181,21 @@ public class GroupsImpl implements Groups Pair 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 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 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 zonesParam = propertyWalker.getZones(); if (zonesParam != null) { validateZonesParam(zonesParam); + zoneFilter = zonesParam.get(0); } } - final String zoneFilter = zonesParam != null ? zonesParam.get(0) : null; final List includeParam = parameters.getInclude(); Paging paging = parameters.getPaging(); @@ -296,11 +297,16 @@ public class GroupsImpl implements Groups Set userAuthorities = runAsSystem( () -> authorityService.getAuthoritiesForUser(personId)); + final Set 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 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 rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP); - List 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. + *

+ * 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 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 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 supportedEqualsParameters, Set 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); + } } } diff --git a/source/test-java/org/alfresco/rest/api/tests/GroupsTest.java b/source/test-java/org/alfresco/rest/api/tests/GroupsTest.java index 7a9a269521..fa9d8c564b 100644 --- a/source/test-java/org/alfresco/rest/api/tests/GroupsTest.java +++ b/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 params = new HashMap<>(); + + params.put("where", "(isRoot=true)"); + ListResponse response = groupsProxy.getGroupsByPersonId("-me-", params, "Couldn't get user's groups", 200); + List 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 otherParams = new HashMap<>(); + addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, null); + + // Get and sort groups using canned query. + ListResponse respCannedQuery = getGroupsByPersonId(personAlice.getId(), paging, otherParams); + + // Get and sort groups using postprocessing. + otherParams.put("where", "(isRoot=true)"); + ListResponse respPostProcess = getGroupsByPersonId(personAlice.getId(), paging, otherParams); + + List 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 otherParams = new HashMap<>(); + addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_ID, null); + + // Get and sort groups using canned query. + ListResponse respCannedQuery = getGroupsByPersonId(personAlice.getId(), paging, otherParams); + + // Get and sort groups using postprocessing. + otherParams.put("where", "(isRoot=true)"); + ListResponse respPostProcess = getGroupsByPersonId(personAlice.getId(), paging, otherParams); + + List 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 params = new HashMap<>(); - params.put("where", "(isRoot=false AND zones in ('I.DO.NOT.EXIST'))"); + Map 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 response = groupsProxy. - getGroupsByPersonId(personAlice.getId(), params, "Incorrect response", 200); + getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200); List 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 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 response = groupsProxy. + getGroupsByPersonId("-me-", otherParams, "Unexpected response", 200); + List 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()); }