Merge pull request #2035 from Alfresco/feature/ACS-5506-be-handling-a-description-of-the-group-and-flag-for-subgroups

ACS-5506 Add description and hasSubgroups to groups API
This commit is contained in:
MichalKinas
2024-02-08 13:35:24 +01:00
committed by GitHub
14 changed files with 509 additions and 135 deletions

View File

@@ -40,8 +40,10 @@ public interface Groups
{
String PARAM_ID = "id";
String PARAM_DISPLAY_NAME = "displayName";
String PARAM_INCLUDE_DESCRIPTION = "description";
String PARAM_INCLUDE_PARENT_IDS = "parentIds";
String PARAM_INCLUDE_ZONES = "zones";
String PARAM_INCLUDE_HAS_SUBGROUPS = "hasSubgroups";
String PARAM_IS_ROOT = "isRoot";
String PARAM_CASCADE = "cascade";
String PARAM_MEMBER_TYPE = "memberType";

View File

@@ -27,6 +27,7 @@ package org.alfresco.rest.api.impl;
import static org.alfresco.repo.security.authentication.AuthenticationUtil.runAsSystem;
import java.io.Serializable;
import java.text.Collator;
import java.util.AbstractList;
import java.util.ArrayList;
@@ -40,6 +41,7 @@ import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import org.alfresco.model.ContentModel;
import org.alfresco.query.CannedQueryPageDetails;
import org.alfresco.query.EmptyPagingResults;
import org.alfresco.query.PagingRequest;
@@ -71,8 +73,10 @@ import org.alfresco.rest.workflow.api.impl.MapBasedQueryWalkerOrSupported;
import org.alfresco.service.cmr.security.AuthorityService;
import org.alfresco.service.cmr.security.AuthorityType;
import org.alfresco.service.cmr.security.PermissionService;
import org.alfresco.service.namespace.QName;
import org.alfresco.util.AlfrescoCollator;
import org.alfresco.util.Pair;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.springframework.extensions.surf.util.I18NUtil;
@@ -101,9 +105,9 @@ public class GroupsImpl implements Groups
}
// List groups filtering (via where clause)
private final static Set<String> LIST_GROUPS_EQUALS_QUERY_PROPERTIES = new HashSet<>(Arrays.asList(new String[] { PARAM_IS_ROOT }));
private final static Set<String> LIST_GROUPS_EQUALS_QUERY_PROPERTIES = new HashSet<>(List.of(PARAM_IS_ROOT));
private final static Set<String> LIST_GROUP_MEMBERS_QUERY_PROPERTIES = new HashSet<>(Arrays.asList(new String[] { PARAM_MEMBER_TYPE }));
private final static Set<String> LIST_GROUP_MEMBERS_QUERY_PROPERTIES = new HashSet<>(List.of(PARAM_MEMBER_TYPE));
protected AuthorityService authorityService;
private AuthorityDAO authorityDAO;
@@ -142,7 +146,12 @@ public class GroupsImpl implements Groups
authorityDisplayName = group.getDisplayName();
}
String authority = authorityService.createAuthority(AuthorityType.GROUP, group.getId(), authorityDisplayName, authorityZones);
Map<QName, Serializable> props = new HashMap<>();
if (StringUtils.isNotEmpty(group.getDescription()))
{
props.put(ContentModel.PROP_DESCRIPTION, group.getDescription());
}
String authority = authorityService.createAuthority(AuthorityType.GROUP, group.getId(), authorityDisplayName, authorityZones, props);
// Set a given child authority to be included by the given parent
// authorities.
@@ -161,7 +170,14 @@ public class GroupsImpl implements Groups
try
{
authorityService.setAuthorityDisplayName(groupId, group.getDisplayName());
if (StringUtils.isNotEmpty(group.getDescription()))
{
authorityService.setAuthorityDisplayNameAndDescription(groupId, group.getDisplayName(), group.getDescription());
}
else
{
authorityService.setAuthorityDisplayName(groupId, group.getDisplayName());
}
}
catch (AuthorityException ae)
{
@@ -173,10 +189,10 @@ public class GroupsImpl implements Groups
public Group getGroup(String groupId, Parameters parameters) throws EntityNotFoundException
{
AuthorityInfo authorityInfo = getAuthorityInfo(groupId);
final List<String> includeParam = parameters.getInclude();
AuthorityInfo authorityInfo = getAuthorityInfo(groupId, includeParam.contains(PARAM_INCLUDE_DESCRIPTION));
final Set<String> rootAuthorities = getAllRootAuthorities(AuthorityType.GROUP);
final List<String> includeParam = parameters.getInclude();
return getGroup(authorityInfo, includeParam, rootAuthorities);
}
@@ -196,7 +212,7 @@ public class GroupsImpl implements Groups
PagingResults<AuthorityInfo> pagingResult;
try
{
pagingResult = getAuthoritiesInfo(authorityType, groupsFilters, rootAuthorities, sortProp, paging);
pagingResult = getAuthoritiesInfo(authorityType, groupsFilters, rootAuthorities, sortProp, paging, parameters.getInclude().contains(PARAM_INCLUDE_DESCRIPTION));
}
catch (UnknownAuthorityException e)
{
@@ -213,7 +229,7 @@ public class GroupsImpl implements Groups
private List<Group> createGroupsResponse(final List<AuthorityInfo> page, final List<String> includeParam, final Set<String> rootAuthorities)
{
List<Group> groups = new AbstractList<Group>()
List<Group> groups = new AbstractList<>()
{
@Override
public Group get(int index)
@@ -336,7 +352,7 @@ public class GroupsImpl implements Groups
filter(a -> a.startsWith(AuthorityType.GROUP.getPrefixString())).
filter(a -> isRootPredicate(finalIsRootParam, rootAuthorities, a)).
filter(a -> zonePredicate(a, finalZoneFilter)).
map(this::getAuthorityInfo).
map(a -> getAuthorityInfo(a, includeParam.contains(PARAM_INCLUDE_DESCRIPTION))).
sorted(new AuthorityInfoComparator(sortProp.getFirst(), sortProp.getSecond())).
collect(Collectors.toList());
@@ -355,23 +371,25 @@ public class GroupsImpl implements Groups
}
private PagingResults<AuthorityInfo> getAuthoritiesInfo(AuthorityType authorityType, GroupsFilter groupsFilter, Set<String> rootAuthorities,
Pair<String, Boolean> sortProp, Paging paging)
Pair<String, Boolean> sortProp, Paging paging, boolean includeDescription)
{
Boolean isRootParam = groupsFilter.getIsRoot();
String zoneFilter = groupsFilter.getZoneFilter();
String displayNameFilter = groupsFilter.getDisplayNameFilter();
PagingResults<AuthorityInfo> pagingResult;
if (isRootParam != null || displayNameFilter != null)
// Don't use canned queries when fetching authorities with description
// if better performance is requested for loading descriptions we can add canned queries in the future
if (isRootParam != null || displayNameFilter != null || includeDescription)
{
List<AuthorityInfo> groupList;
if (isRootParam != null && isRootParam)
if ((isRootParam != null && isRootParam) || includeDescription)
{
// Limit the post processing work by using the already loaded
// list of root authorities.
List<AuthorityInfo> authorities = rootAuthorities.stream().
map(this::getAuthorityInfo).
map(auth -> getAuthorityInfo(auth, includeDescription)).
filter(auth -> zonePredicate(auth.getAuthorityName(), zoneFilter)).
filter(auth -> displayNamePredicate(auth.getAuthorityDisplayName(), displayNameFilter)).
collect(Collectors.toList());
@@ -526,9 +544,9 @@ public class GroupsImpl implements Groups
* The authority name.
* @return The authority info.
*/
private AuthorityInfo getAuthorityInfo(String id)
private AuthorityInfo getAuthorityInfo(String id, boolean includeDescription)
{
return getAuthorityInfo(id, false);
return getAuthorityInfo(id, includeDescription, false);
}
/**
@@ -537,11 +555,13 @@ public class GroupsImpl implements Groups
*
* @param id
* The authority name.
* @param includeDescription
* True if description should be loaded
* @param defaultDisplayNameIfNull
* True if we would like to get a default value (e.g. shortName of the authority) if the authority display name is null.
* @return The authority info.
*/
private AuthorityInfo getAuthorityInfo(String id, boolean defaultDisplayNameIfNull)
private AuthorityInfo getAuthorityInfo(String id, boolean includeDescription, boolean defaultDisplayNameIfNull)
{
if (id == null || id.isEmpty())
{
@@ -554,9 +574,20 @@ public class GroupsImpl implements Groups
throw new EntityNotFoundException(id);
}
String authorityDisplayName = getAuthorityDisplayName(id, defaultDisplayNameIfNull);
String authorityDisplayName;
String description = null;
return new AuthorityInfo(null, authorityDisplayName, id);
if (includeDescription)
{
Pair<String, String> displayNameAndDescription = getAuthorityDisplayNameAndDescription(id, defaultDisplayNameIfNull);
authorityDisplayName = displayNameAndDescription.getFirst();
description = displayNameAndDescription.getSecond();
}
else
{
authorityDisplayName = getAuthorityDisplayName(id, defaultDisplayNameIfNull);
}
return new AuthorityInfo(null, authorityDisplayName, id, description);
}
private String getAuthorityDisplayName(String id, boolean defaultDisplayNameIfNull)
@@ -564,6 +595,11 @@ public class GroupsImpl implements Groups
return defaultDisplayNameIfNull ? authorityService.getAuthorityDisplayName(id) : authorityDAO.getAuthorityDisplayName(id);
}
private Pair<String, String> getAuthorityDisplayNameAndDescription(String id, boolean defaultDisplayNameIfNull)
{
return defaultDisplayNameIfNull ? authorityService.getAuthorityDisplayNameAndDescription(id) : authorityDAO.getAuthorityDisplayNameAndDescription(id);
}
private Group getGroup(AuthorityInfo authorityInfo, List<String> includeParam, Set<String> rootAuthorities)
{
if (authorityInfo == null)
@@ -576,13 +612,23 @@ public class GroupsImpl implements Groups
// REPO-1743
String authorityDisplayName = authorityInfo.getAuthorityDisplayName();
String description = authorityInfo.getDescription();
if (authorityDisplayName == null || authorityDisplayName.isEmpty())
{
authorityDisplayName = authorityService.getAuthorityDisplayName(authorityInfo.getAuthorityName());
if (includeParam != null && includeParam.contains(PARAM_INCLUDE_DESCRIPTION))
{
Pair<String, String> displayNameAndDescription = authorityService.getAuthorityDisplayNameAndDescription(authorityInfo.getAuthorityName());
authorityDisplayName = displayNameAndDescription.getFirst();
description = displayNameAndDescription.getSecond();
}
else
{
authorityDisplayName = authorityService.getAuthorityDisplayName(authorityInfo.getAuthorityName());
}
}
group.setDisplayName(authorityDisplayName);
group.setDescription(description);
group.setIsRoot(isRootAuthority(rootAuthorities, authorityInfo.getAuthorityName()));
// Optionally include
@@ -606,6 +652,19 @@ public class GroupsImpl implements Groups
Set<String> authorityZones = authorityService.getAuthorityZones(authorityInfo.getAuthorityName());
group.setZones(authorityZones);
}
if (includeParam.contains(PARAM_INCLUDE_HAS_SUBGROUPS))
{
Set<String> containedAuthorities;
try
{
containedAuthorities = authorityService.getContainedAuthorities(AuthorityType.GROUP, authorityInfo.getAuthorityName(), true);
} catch (UnknownAuthorityException e)
{
containedAuthorities = Collections.emptySet();
}
group.setHasSubgroups(CollectionUtils.isNotEmpty(containedAuthorities));
}
}
return group;
@@ -621,7 +680,7 @@ public class GroupsImpl implements Groups
Pair<String, Boolean> sortProp;
List<SortColumn> sortCols = parameters.getSorting();
if ((sortCols != null) && (sortCols.size() > 0))
if (sortCols != null && !sortCols.isEmpty())
{
if (sortCols.size() > 1)
{
@@ -636,7 +695,7 @@ public class GroupsImpl implements Groups
throw new InvalidArgumentException("Invalid sort field: " + sortCol.column);
}
sortProp = new Pair<>(sortPropName, (sortCol.asc ? Boolean.TRUE : Boolean.FALSE));
sortProp = new Pair<>(sortPropName, sortCol.asc ? Boolean.TRUE : Boolean.FALSE);
}
else
{
@@ -851,7 +910,6 @@ public class GroupsImpl implements Groups
validateGroupMemberId(groupMemberId);
// Verify if groupMemberId is member of groupId
AuthorityType authorityType = AuthorityType.getAuthorityType(groupMemberId);
Set<String> parents = authorityService.getContainingAuthorities(AuthorityType.GROUP, groupMemberId, true);
if (!parents.contains(groupId))
{
@@ -894,7 +952,7 @@ public class GroupsImpl implements Groups
}
List<AuthorityInfo> authorityInfoList = new ArrayList<>(authorities.size());
authorityInfoList.addAll(authorities.stream().map(this::getAuthorityInfo).collect(Collectors.toList()));
authorityInfoList.addAll(authorities.stream().map(auth -> getAuthorityInfo(auth, false)).collect(Collectors.toList()));
// Post process sorting - this should be moved to service
// layer. It is done here because sorting is not supported at
@@ -943,7 +1001,7 @@ public class GroupsImpl implements Groups
private GroupMember getGroupMember(String authorityId)
{
AuthorityInfo authorityInfo = getAuthorityInfo(authorityId);
AuthorityInfo authorityInfo = getAuthorityInfo(authorityId, false);
return getGroupMember(authorityInfo);
}
@@ -1014,6 +1072,10 @@ public class GroupsImpl implements Groups
{
throw new InvalidArgumentException("Group update does not support field: zones");
}
if (group.wasSet(Group.HAS_SUBGROUPS))
{
throw new InvalidArgumentException("Group update does not support field: hasSubgroups");
}
}
}
@@ -1054,7 +1116,7 @@ public class GroupsImpl implements Groups
{
String name = inferPrefix ? authorityService.getName(authorityType, authorityName) : authorityName;
return (name != null && authorityService.authorityExists(name));
return name != null && authorityService.authorityExists(name);
}
private boolean isGroupAuthority(String authorityName)

View File

@@ -42,7 +42,9 @@ public class Group implements Comparable<Group>
protected String id; // group id (aka authority name)
protected String displayName;
protected String description;
protected Boolean isRoot;
protected Boolean hasSubgroups;
protected Set<String> parentIds;
protected Set<String> zones;
@@ -50,7 +52,9 @@ public class Group implements Comparable<Group>
public static final String ID = "id";
public static final String DISPLAY_NAME = "displayName";
public static final String DESCRIPTION = "description";
public static final String IS_ROOT = "isRoot";
public static final String HAS_SUBGROUPS = "hasSubgroups";
public static final String PARENT_IDS = "parentIds";
public static final String ZONES = "zones";
@@ -81,6 +85,14 @@ public class Group implements Comparable<Group>
setFields.put(DISPLAY_NAME, true);
}
public String getDescription() {
return description;
}
public void setDescription(String description) {
this.description = description;
}
public Boolean getIsRoot()
{
return isRoot;
@@ -92,6 +104,14 @@ public class Group implements Comparable<Group>
setFields.put(IS_ROOT, true);
}
public Boolean getHasSubgroups() {
return hasSubgroups;
}
public void setHasSubgroups(Boolean hasSubgroups) {
this.hasSubgroups = hasSubgroups;
}
public Set<String> getParentIds()
{
return parentIds;
@@ -154,12 +174,13 @@ public class Group implements Comparable<Group>
@Override
public String toString()
{
return "Group [id=" + id + ", displayName=" + displayName + ", isRoot=" + isRoot + "]";
return "Group [id=" + id + ", displayName=" + displayName + ", description=" + description
+ ", isRoot=" + isRoot + ", hasSubgroups=" + hasSubgroups + "]";
}
public boolean wasSet(String fieldName)
{
Boolean b = setFields.get(fieldName);
return (b != null ? b : false);
return b != null && b;
}
}

View File

@@ -43,17 +43,20 @@ import org.alfresco.service.cmr.security.AuthorityService;
import org.alfresco.service.cmr.security.AuthorityType;
import org.alfresco.service.cmr.security.PermissionService;
import org.alfresco.util.GUID;
import org.alfresco.util.testing.category.LuceneTests;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.mockito.Mock;
import jakarta.servlet.http.HttpServletResponse;
import java.util.*;
import static org.junit.Assert.*;
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 static org.mockito.Mockito.when;
/**
@@ -69,13 +72,13 @@ 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;
private GroupMember groupMemberB = null;
private GroupMember personMember = null;
private String rootGroupName;
private Group rootGroup;
private Group groupA;
private Group groupB;
private GroupMember groupMemberA;
private GroupMember groupMemberB;
private GroupMember personMember;
@Mock
private ResultSetRow groupAResultSetRow;
@Mock
@@ -670,6 +673,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
// Optionally included.
assertNull(group.getParentIds());
assertNull(group.getZones());
assertNull(group.getHasSubgroups());
}
}
@@ -956,7 +960,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
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);
assertTrue("List doesn't contain enough items for test to be conclusive.", !expected.isEmpty());
checkList(expected, respPostProcess.getPaging(), respPostProcess);
}
@@ -977,7 +981,7 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
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);
assertTrue("List doesn't contain enough items for test to be conclusive.", !expected.isEmpty());
checkList(expected, respPostProcess.getPaging(), respPostProcess);
}
@@ -1154,7 +1158,6 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
// -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);
@@ -1418,16 +1421,17 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
setRequestContext(networkOne.getId(), networkAdmin, DEFAULT_ADMIN_PWD);
Map<String, String> otherParams = new HashMap<>();
otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_PARENT_IDS);
otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_HAS_SUBGROUPS);
Group group = generateGroup();
Group createdGroup01 = groupsProxy.createGroup(group, null, HttpServletResponse.SC_CREATED);
Group createdGroup01 = groupsProxy.createGroup(group, otherParams, HttpServletResponse.SC_CREATED);
assertNotNull(createdGroup01);
assertNotNull(createdGroup01.getId());
assertTrue(createdGroup01.getIsRoot());
assertNull(createdGroup01.getParentIds());
assertFalse(createdGroup01.getHasSubgroups());
Set<String> subGroup01Parents = new HashSet<>();
subGroup01Parents.add(createdGroup01.getId());
@@ -1435,12 +1439,18 @@ public class GroupsTest extends AbstractSingleNetworkSiteTest
Group subGroup01 = generateGroup();
subGroup01.setParentIds(subGroup01Parents);
otherParams.put("include", org.alfresco.rest.api.Groups.PARAM_INCLUDE_PARENT_IDS + "," + org.alfresco.rest.api.Groups.PARAM_INCLUDE_HAS_SUBGROUPS);
Group createdSubGroup01 = groupsProxy.createGroup(subGroup01, otherParams, HttpServletResponse.SC_CREATED);
assertNotNull(createdSubGroup01);
assertNotNull(createdSubGroup01.getId());
assertFalse(createdSubGroup01.getIsRoot());
assertNotNull(createdSubGroup01.getParentIds());
assertEquals(subGroup01Parents, createdSubGroup01.getParentIds());
assertFalse(createdSubGroup01.getHasSubgroups());
//validate if parent group now has any subgroup
Group group01 = groupsProxy.getGroup(createdGroup01.getId(), otherParams, HttpServletResponse.SC_OK);
assertTrue(group01.getHasSubgroups());
}
// Group id is missing.

View File

@@ -58,7 +58,9 @@ public class Group extends org.alfresco.rest.api.model.Group implements Serializ
AssertUtil.assertEquals("id", getId(), other.getId());
AssertUtil.assertEquals("displayName", getDisplayName(), other.getDisplayName());
AssertUtil.assertEquals("description", getDescription(), other.getDescription());
AssertUtil.assertEquals("isRoot", getIsRoot(), other.getIsRoot());
AssertUtil.assertEquals("hasSubgroups", getHasSubgroups(), other.getHasSubgroups());
AssertUtil.assertEquals("parentIds", getParentIds(), other.getParentIds());
AssertUtil.assertEquals("zones", getZones(), other.getZones());
}
@@ -73,11 +75,21 @@ public class Group extends org.alfresco.rest.api.model.Group implements Serializ
groupJson.put("displayName", getDisplayName());
if (getDescription() != null)
{
groupJson.put("description", getDescription());
}
if (getIsRoot() != null)
{
groupJson.put("isRoot", getIsRoot());
}
if (getHasSubgroups() != null)
{
groupJson.put("hasSubgroups", getHasSubgroups());
}
if (getParentIds() != null)
{
groupJson.put("parentIds", new ArrayList(getParentIds()));
@@ -95,16 +107,19 @@ public class Group extends org.alfresco.rest.api.model.Group implements Serializ
{
String id = (String) jsonObject.get("id");
String displayName = (String) jsonObject.get("displayName");
String description = (String) jsonObject.get("description");
Boolean isRoot = (Boolean) jsonObject.get("isRoot");
Boolean hasSubgroups = (Boolean) jsonObject.get("hasSubgroups");
List<String> parentIds = (List<String>) jsonObject.get("parentIds");
List<String> zones = (List<String>) jsonObject.get("zones");
Group group = new Group();
group.setId(id);
group.setDisplayName(displayName);
group.setDescription(description);
group.setIsRoot(isRoot);
group.setParentIds(parentIds != null ? new HashSet<String>(parentIds) : null);
group.setZones(zones != null ? new HashSet<String>(zones) : null);
group.setHasSubgroups(hasSubgroups);
group.setParentIds(parentIds != null ? new HashSet<>(parentIds) : null);
group.setZones(zones != null ? new HashSet<>(zones) : null);
return group;
}