From 9edd3782912ce520a84995a9dee5717d7ffe7aff Mon Sep 17 00:00:00 2001 From: Tom Page Date: Wed, 29 Jul 2015 15:16:25 +0000 Subject: [PATCH] RM-2470 Change security clearance sorting to be by full name. Enable sorting by multiple fields and configure the security clearance page to use first name, last name and then fall back to user name as a tie breaker. The rm-automation test fails for me locally, as the person search is case sensitive. I'm pushing this anyway because Bamboo didn't protest at the old sorting method (which also failed locally due to case sensitivity). Remove "final" from UserQueryParams so it can be mocked. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@109169 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../classification/UserQueryParams.java | 39 +++++++---- .../UserSecurityClearanceGet.java | 38 ++++++++--- .../UserSecurityClearanceGetUnitTest.java | 67 +++++++++++++++++++ 3 files changed, 124 insertions(+), 20 deletions(-) diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/UserQueryParams.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/UserQueryParams.java index 996e632c7e..6f9cd3a069 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/UserQueryParams.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/UserQueryParams.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import com.google.common.collect.ImmutableList; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.alfresco.util.ParameterCheck; @@ -37,7 +38,7 @@ import org.alfresco.util.ParameterCheck; * @author Neil Mc Erlean * @since 3.0.a */ -public final class UserQueryParams +public class UserQueryParams { /** Required parameter. No default value. This is the username fragment. */ private final String searchTerm; @@ -89,6 +90,19 @@ public final class UserQueryParams return this; } + /** + * Sets the sort properties required for the query. + * + * @param sortProps A list of pairs of properties and sort directions. + * @return The updated {@code UserQueryParams} object. + */ + public UserQueryParams withSortProps(List> sortProps) + { + this.sortProps = ImmutableList.copyOf(sortProps); + validateList(sortProps); + return this; + } + public String getSearchTerm() { return this.searchTerm; } public List getFilterProps() { return this.filterProps; } public List> getSortProps() { return this.sortProps; } @@ -99,24 +113,25 @@ public final class UserQueryParams @SuppressWarnings("unchecked") private List toList(T firstElem, T... otherElems) { - // At least one element is required. - ParameterCheck.mandatory("firstElem", firstElem); - List elementList = new ArrayList<>(); elementList.add(firstElem); if (otherElems != null) { - final List tList = asList(otherElems); - final int firstNull = tList.indexOf(null); - if (firstNull != -1) - { - // "+ 2" so that position 1 points to 'firstElem' and so on through otherElems. - throw new IllegalArgumentException("Unexpected null element at position " + firstNull + 2); - } - elementList.addAll(tList); + elementList.addAll(asList(otherElems)); } + validateList(elementList); + return elementList; } + + /** Validate a list of elements to check none of them are null. */ + private void validateList(List elementList) + { + if (elementList.contains(null)) + { + throw new IllegalArgumentException("Unexpected null in parameter list: " + elementList); + } + } } diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGet.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGet.java index 7a2602de15..8fd30bbc6d 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGet.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGet.java @@ -20,10 +20,13 @@ package org.alfresco.module.org_alfresco_module_rm.script.classification; import static java.lang.Boolean.parseBoolean; import static java.lang.Integer.parseInt; +import static org.apache.commons.lang.StringUtils.isBlank; import static org.apache.commons.lang.StringUtils.isNotBlank; import static org.springframework.extensions.webscripts.Status.STATUS_INTERNAL_SERVER_ERROR; +import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -34,6 +37,7 @@ import org.alfresco.module.org_alfresco_module_rm.script.AbstractRmWebScript; import org.alfresco.query.PagingResults; import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; +import org.apache.commons.collections.iterators.ArrayIterator; import org.springframework.extensions.webscripts.Cache; import org.springframework.extensions.webscripts.Status; import org.springframework.extensions.webscripts.WebScriptException; @@ -51,8 +55,9 @@ public class UserSecurityClearanceGet extends AbstractRmWebScript private static final String TOTAL = "total"; private static final String SKIP_COUNT = "startIndex"; private static final String MAX_ITEMS = "pageSize"; - private static final String SORT_FIELD = "sortField"; - private static final String SORT_ASCENDING = "sortAscending"; + private static final String SORT_FIELDS = "sortField"; + private static final String SORT_ASCENDING_FLAGS = "sortAscending"; + private static final String SEPARATOR = ","; private static final String ITEM_COUNT = "itemCount"; private static final String ITEMS = "items"; private static final String DATA = "data"; @@ -204,14 +209,31 @@ public class UserSecurityClearanceGet extends AbstractRmWebScript * @param req {@link WebScriptRequest} The webscript request */ @SuppressWarnings("unchecked") - private void setSortProps(UserQueryParams userQueryParams, WebScriptRequest req) + protected void setSortProps(UserQueryParams userQueryParams, WebScriptRequest req) { - String sortField = req.getParameter(SORT_FIELD); - String sortAscending = req.getParameter(SORT_ASCENDING); - - if (isNotBlank(sortField) && isNotBlank(sortAscending)) + String sortFields = req.getParameter(SORT_FIELDS); + if (isBlank(sortFields)) { - userQueryParams.withSortProps(new Pair<>(QName.createQName(sortField, getNamespaceService()), parseBoolean(sortAscending))); + return; + } + String sortAscendingFlags = req.getParameter(SORT_ASCENDING_FLAGS); + sortAscendingFlags = (isBlank(sortAscendingFlags) ? "True" : sortAscendingFlags); + + List> sortPairs = new ArrayList<>(); + Iterator ascendingFlagIterator = new ArrayIterator((String[]) sortAscendingFlags.split(SEPARATOR)); + for (String sortField : sortFields.split(SEPARATOR)) + { + boolean ascendingFlag = (ascendingFlagIterator.hasNext() ? parseBoolean(ascendingFlagIterator.next()) : true); + + if (isNotBlank(sortField)) + { + Pair sortPair = new Pair<>(QName.createQName(sortField, getNamespaceService()), ascendingFlag); + sortPairs.add(sortPair); + } + } + if (!sortPairs.isEmpty()) + { + userQueryParams.withSortProps(sortPairs); } } } diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGetUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGetUnitTest.java index b8de465dbe..699d534e6c 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGetUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/script/classification/UserSecurityClearanceGetUnitTest.java @@ -23,8 +23,13 @@ import static org.junit.Assert.assertNotNull; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import com.fasterxml.jackson.databind.JsonNode; @@ -38,6 +43,7 @@ import org.alfresco.module.org_alfresco_module_rm.test.util.BaseWebScriptUnitTes import org.alfresco.query.PagingResults; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.security.PersonService.PersonInfo; +import org.alfresco.service.namespace.QName; import org.alfresco.util.Pair; import org.json.JSONArray; import org.json.JSONObject; @@ -46,6 +52,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; import org.springframework.extensions.webscripts.DeclarativeWebScript; +import org.springframework.extensions.webscripts.WebScriptRequest; /** * Test for get user security clearance API @@ -195,6 +202,66 @@ public class UserSecurityClearanceGetUnitTest extends BaseWebScriptUnitTest assertEquals(expected, mapper.readTree(response.toString())); } + /** + * Check that when supplying a single field with no sort direction, the UserQueryParams are populated with the + * specified field and the default direction (true). + */ + @Test + public void testSetSortProps_singleField() + { + UserQueryParams userQueryParams = mock(UserQueryParams.class); + WebScriptRequest req = mock(WebScriptRequest.class); + when(req.getParameter("sortField")).thenReturn("field"); + when(mockedNamespaceService.getNamespaceURI("")).thenReturn("namespace"); + + // Call the method under test. + webscript.setSortProps(userQueryParams, req); + + // Check the userQueryParams contains the field (and the direction has defaulted to ascending (TRUE)). + Pair sortPair = new Pair<>(QName.createQName("field", mockedNamespaceService), Boolean.TRUE); + List> expectedSortProps = Arrays.asList(sortPair); + verify(userQueryParams).withSortProps(expectedSortProps); + } + + /** + * Check that when supplying three fields with different sort directions (ascending, descending, unspecified), the + * UserQueryParams gets populated correctly. + */ + @Test + public void testSetSortProps_multipleFieldsAndDirections() + { + UserQueryParams userQueryParams = mock(UserQueryParams.class); + WebScriptRequest req = mock(WebScriptRequest.class); + when(req.getParameter("sortField")).thenReturn("fieldA,fieldB,fieldC"); + // The sort order for the fields is ascending, descending, unspecified (which should default to ascending). + when(req.getParameter("sortAscending")).thenReturn("true,false"); + when(mockedNamespaceService.getNamespaceURI("")).thenReturn("namespace"); + + // Call the method under test. + webscript.setSortProps(userQueryParams, req); + + Pair sortPairA = new Pair<>(QName.createQName("fieldA", mockedNamespaceService), Boolean.TRUE); + Pair sortPairB = new Pair<>(QName.createQName("fieldB", mockedNamespaceService), Boolean.FALSE); + Pair sortPairC = new Pair<>(QName.createQName("fieldC", mockedNamespaceService), Boolean.TRUE); + List> expectedSortProps = Arrays.asList(sortPairA, sortPairB, sortPairC); + verify(userQueryParams).withSortProps(expectedSortProps); + } + + /** Check that if no sort information is given there are no exceptions. */ + @Test + public void testSetSortProps_noFields() + { + UserQueryParams userQueryParams = mock(UserQueryParams.class); + WebScriptRequest req = mock(WebScriptRequest.class); + when(req.getParameter("sortField")).thenReturn(null); + when(mockedNamespaceService.getNamespaceURI("")).thenReturn("namespace"); + + // Call the method under test. + webscript.setSortProps(userQueryParams, req); + + verifyNoMoreInteractions(userQueryParams); + } + private String getExpectedResult(int total, int startIndex, int pageSize, int fromIndex, int toIndex, int itemCount) { return "{" +