diff --git a/source/java/org/alfresco/rest/api/impl/GroupsImpl.java b/source/java/org/alfresco/rest/api/impl/GroupsImpl.java index 50801bf2ec..cc1ea05836 100644 --- a/source/java/org/alfresco/rest/api/impl/GroupsImpl.java +++ b/source/java/org/alfresco/rest/api/impl/GroupsImpl.java @@ -79,6 +79,7 @@ import org.springframework.extensions.surf.util.I18NUtil; */ public class GroupsImpl implements Groups { + private static final int MAX_ZONES = 1; private static final String DISPLAY_NAME = "displayName"; private static final String SHORT_NAME = "shortName"; // private static final String AUTHORITY_NAME = "authorityName"; @@ -180,18 +181,42 @@ public class GroupsImpl implements Groups // Parse where clause properties. Boolean isRootParam = null; Query q = parameters.getQuery(); + + String zoneFilter = null; if (q != null) { - MapBasedQueryWalkerOrSupported propertyWalker = new MapBasedQueryWalkerOrSupported(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null); + GroupsQueryWalker propertyWalker = new GroupsQueryWalker(LIST_GROUPS_EQUALS_QUERY_PROPERTIES, null); QueryHelper.walk(q, propertyWalker); isRootParam = propertyWalker.getProperty(PARAM_IS_ROOT, WhereClauseParser.EQUALS, Boolean.class); + List zonesParam = propertyWalker.getZones(); + if (zonesParam != null) + { + 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. '')"); + } + }); + + zoneFilter = zonesParam.get(0); + } + } final AuthorityType authorityType = AuthorityType.GROUP; final Set rootAuthorities = getAllRootAuthorities(authorityType); - PagingResults pagingResult = getAuthoritiesInfo(authorityType, isRootParam, rootAuthorities, sortProp, paging); + PagingResults pagingResult = getAuthoritiesInfo(authorityType, isRootParam, zoneFilter, rootAuthorities, sortProp, paging); // Create response. final List page = pagingResult.getPage(); @@ -215,6 +240,25 @@ public class GroupsImpl implements Groups return CollectionWithPagingInfo.asPaged(paging, groups, pagingResult.hasMoreItems(), totalItems); } +// private boolean groupInZones(Group group, List zonesFilter) +// { +// Set 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 getGroupsByPersonId(String requestedPersonId, Parameters parameters) { @@ -258,6 +302,8 @@ public class GroupsImpl implements Groups // Transform the page of results into Group objects final Set rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP); + // Zone filter not supported currently. + final String zoneFilter = null; List groups = page.stream(). map(authority -> getGroup(authority, includeParam, rootAuthorities)). collect(Collectors.toList()); @@ -265,10 +311,11 @@ public class GroupsImpl implements Groups return CollectionWithPagingInfo.asPaged(paging, groups, pagingResult.hasMoreItems(), totalItems); } - private PagingResults getAuthoritiesInfo(AuthorityType authorityType, Boolean isRootParam, Set rootAuthorities, Pair sortProp, + private PagingResults getAuthoritiesInfo(AuthorityType authorityType, Boolean isRootParam, String zoneFilter, Set rootAuthorities, Pair sortProp, Paging paging) { PagingResults pagingResult; + // TODO: make sure impl works for isRootParam = true,false,null. if (isRootParam != null) { List groupList; @@ -314,6 +361,7 @@ 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 pagingResult = Util.wrapPagingResults(paging, groupList); } else @@ -321,7 +369,7 @@ public class GroupsImpl implements Groups PagingRequest pagingRequest = Util.getPagingRequest(paging); // Get authorities using canned query. - pagingResult = authorityService.getAuthoritiesInfo(authorityType, null, null, sortProp.getFirst(), sortProp.getSecond(), pagingRequest); + pagingResult = authorityService.getAuthoritiesInfo(authorityType, zoneFilter, null, sortProp.getFirst(), sortProp.getSecond(), pagingRequest); } return pagingResult; } @@ -743,7 +791,7 @@ public class GroupsImpl implements Groups throw new EntityNotFoundException(groupId); } } - + private void validateGroupMemberId(String groupMemberId) { if (groupMemberId == null || groupMemberId.isEmpty()) @@ -849,4 +897,33 @@ public class GroupsImpl implements Groups AuthorityType authorityType = AuthorityType.getAuthorityType(authorityName); return AuthorityType.GROUP.equals(authorityType) || AuthorityType.EVERYONE.equals(authorityType); } + + private static class GroupsQueryWalker extends MapBasedQueryWalkerOrSupported + { + private List zones; + + @Override + public void in(String propertyName, boolean negated, String... propertyValues) + { + if (propertyName.equalsIgnoreCase("zones")) + { + zones = Arrays.asList(propertyValues); + } + } + + public GroupsQueryWalker(Set supportedEqualsParameters, Set supportedMatchesParameters) + { + super(supportedEqualsParameters, supportedMatchesParameters); + } + + /** + * The list of zones specified in the where clause. + * + * @return The zones list if specified, or null if not. + */ + public List getZones() + { + return zones; + } + } } 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 24ab6f7c03..310115667c 100644 --- a/source/test-java/org/alfresco/rest/api/tests/GroupsTest.java +++ b/source/test-java/org/alfresco/rest/api/tests/GroupsTest.java @@ -31,13 +31,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.UUID; +import java.util.*; import javax.servlet.http.HttpServletResponse; @@ -108,6 +102,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest testGetGroupsSkipPaging(); testGetGroupsByIsRoot(true); testGetGroupsByIsRoot(false); + testGetGroupsWithZoneFilter(); } finally { @@ -331,6 +326,118 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest }); } + private void testGetGroupsWithZoneFilter() throws Exception + { + // Filter by zone + { + Paging paging = getPaging(0, Integer.MAX_VALUE); + Map otherParams = new HashMap<>(); + otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES); + + otherParams.put("where", "(zones in ('APP.DEFAULT'))"); + + ListResponse response = getGroups(paging, otherParams); + List 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 - custom zones, "include" them in the response. + { + Paging paging = getPaging(0, Integer.MAX_VALUE); + 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", "(zones in ('APITEST.MYZONE'))"); + + ListResponse response = getGroups(paging, otherParams); + List groups = response.getList(); + + // We know exactly which groups are in the selected zone. + assertEquals(2, groups.size()); + assertEquals(groupA, groups.get(0)); + assertEquals(groupB, groups.get(1)); + groups.forEach(group -> assertTrue(group.getZones().contains("APITEST.MYZONE"))); + + otherParams.put("where", "(zones in ('APITEST.ANOTHER'))"); + response = getGroups(paging, otherParams); + groups = response.getList(); + + // We know exactly which groups are in the selected zone. + assertEquals(1, groups.size()); + assertEquals(groupA, groups.get(0)); + assertTrue(groups.get(0).getZones().contains("APITEST.MYZONE")); + assertTrue(groups.get(0).getZones().contains("APITEST.ANOTHER")); + } + + // Filter without "include"-ing zones + { + Paging paging = getPaging(0, Integer.MAX_VALUE); + Map otherParams = new HashMap<>(); + // Ensure predictable result ordering + addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, true); + + otherParams.put("where", "(zones in ('APITEST.MYZONE'))"); + + assertFalse(otherParams.containsKey("include")); + ListResponse response = getGroups(paging, otherParams); + List groups = response.getList(); + + // We know exactly which groups are in the selected zone. + 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())); + } + + // Filter zones while using where isRoot=true + // (this causes a different query path to be used) + /*{ + Paging paging = getPaging(0, Integer.MAX_VALUE); + Map otherParams = new HashMap<>(); + // Ensure predictable result ordering + addOrderBy(otherParams, org.alfresco.rest.api.Groups.PARAM_DISPLAY_NAME, true); + + // TODO: how will we support all possible boolean operators? AND, OR...? + otherParams.put("where", "(isRoot=true AND zones in ('APITEST.SPECIAL'))"); + + ListResponse response = getGroups(paging, otherParams); + List groups = response.getList(); + + assertEquals(1, groups.size()); + assertEquals(rootGroupName, groups.get(0)); + assertTrue(groups.get(0).getZones().contains("APITEST.SPECIAL")); + }*/ + + // -ve test: invalid zones clause + { + Paging paging = getPaging(0, Integer.MAX_VALUE); + Map otherParams = new HashMap<>(); + otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_ZONES); + + // Empty zone list + otherParams.put("where", "(zones in ())"); + getGroups(paging, otherParams, "Incorrect response", 400); + + // Empty zone name + otherParams.put("where", "(zones in (''))"); + getGroups(paging, otherParams, "Incorrect response", 400); + + // Too many zones + otherParams.put("where", "(zones in ('APP.DEFAULT', 'APITEST.MYZONE'))"); + getGroups(paging, otherParams, "Incorrect response", 400); + + // "A series of unfortunate errors" + otherParams.put("where", "(zones in ('', 'APP.DEFAULT', '', 'APITEST.MYZONE'))"); + getGroups(paging, otherParams, "Incorrect response", 400); + } + } + private ListResponse getGroups(final PublicApiClient.Paging paging, Map otherParams, String errorMessage, int expectedStatus) throws Exception { final Groups groupsProxy = publicApiClient.groups(); @@ -368,12 +475,15 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest AuthenticationUtil.setAdminUserAsFullyAuthenticatedUser(); rootGroupName = authorityService.createAuthority(AuthorityType.GROUP, groupName); + authorityService.addAuthorityToZones(rootGroupName, zoneSet("APITEST.SPECIAL")); String groupBAuthorityName = authorityService.createAuthority(AuthorityType.GROUP, "Test_GroupB" + GUID.generate()); authorityService.addAuthority(rootGroupName, groupBAuthorityName); + authorityService.addAuthorityToZones(groupBAuthorityName, zoneSet("APITEST.MYZONE")); String groupAAuthorityName = authorityService.createAuthority(AuthorityType.GROUP, "Test_GroupA" + GUID.generate()); authorityService.addAuthority(rootGroupName, groupAAuthorityName); + authorityService.addAuthorityToZones(groupAAuthorityName, zoneSet("APITEST.MYZONE", "APITEST.ANOTHER")); authorityService.addAuthority(groupAAuthorityName, user1); authorityService.addAuthority(groupBAuthorityName, user2); @@ -411,6 +521,13 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest } } + private Set zoneSet(String... zones) + { + Set zoneSet = new HashSet<>(zones.length); + zoneSet.addAll(Arrays.asList(zones)); + return zoneSet; + } + /** * Clears authority context: removes root group and all child groups. */