diff --git a/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java b/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java index 3ffb720bd1..cdde9fa8a0 100644 --- a/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java +++ b/src/main/java/org/alfresco/rest/api/impl/ActionsImpl.java @@ -41,6 +41,10 @@ import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; +import static java.util.Comparator.nullsFirst; + public class ActionsImpl implements Actions { private ActionService actionService; @@ -89,10 +93,10 @@ public class ActionsImpl implements Actions switch (sortKey) { case TITLE: - comparator = Comparator.comparing(ActionDefinition::getTitle); + comparator = comparing(ActionDefinition::getTitle, nullsFirst(naturalOrder())); break; case NAME: - comparator = Comparator.comparing(ActionDefinition::getName); + comparator = comparing(ActionDefinition::getName, nullsFirst(naturalOrder())); break; default: throw new IllegalArgumentException("Invalid sort key, must be either 'title' or 'name'."); diff --git a/src/test/java/org/alfresco/rest/api/tests/TestActions.java b/src/test/java/org/alfresco/rest/api/tests/TestActions.java index 25c94526bb..d27cff48d0 100644 --- a/src/test/java/org/alfresco/rest/api/tests/TestActions.java +++ b/src/test/java/org/alfresco/rest/api/tests/TestActions.java @@ -50,7 +50,6 @@ import org.junit.Before; import org.junit.Test; import java.util.Collections; -import java.util.Comparator; import java.util.EmptyStackException; import java.util.Iterator; import java.util.List; @@ -58,6 +57,9 @@ import java.util.Map; import java.util.function.Supplier; import java.util.stream.Collectors; +import static java.util.Comparator.comparing; +import static java.util.Comparator.naturalOrder; +import static java.util.Comparator.nullsFirst; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -111,7 +113,7 @@ public class TestActions extends AbstractBaseApiTest } @Test - public void canGetActionsDefinitions() throws PublicApiException + public void canGetActionDefinitions() throws PublicApiException { final String person1 = account1PersonIt.next(); publicApiClient.setRequestContext(new RequestContext(account1.getId(), person1)); @@ -154,13 +156,38 @@ public class TestActions extends AbstractBaseApiTest // Expected () -> actionService.getActionDefinitions(). stream(). - sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)). map(ParameterizedItemDefinition::getName). collect(Collectors.toList()), // Actual results paging -> actions.getActionDefinitions(createParams(paging, null), 200)); + // Explicit sorting by title + checkSorting( + // Expected + () -> actionService.getActionDefinitions(). + stream(). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getTitle, + nullsFirst(naturalOrder()))). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()), + // Actual results + (paging, orderBy) -> actions.getActionDefinitions(createParams(paging, orderBy), 200), + "title"); + // Explicit sorting by name + checkSorting( + // Expected + () -> actionService.getActionDefinitions(). + stream(). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getName, + nullsFirst(naturalOrder()))). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()), + // Actual results + (paging, orderBy) -> actions.getActionDefinitions(createParams(paging, orderBy), 200), + "name"); + // Badly formed request -> 400 { PublicApiClient.Paging paging = getPaging(0, -1); // -1 is not acceptable @@ -173,7 +200,7 @@ public class TestActions extends AbstractBaseApiTest actions.getActionDefinitions(emptyParams, 401); } } - + @Test public void canGetActionDefinitionsForNode() throws Exception { @@ -281,133 +308,44 @@ public class TestActions extends AbstractBaseApiTest assertFalse("Action definition list should not be empty", actionDefs.getList().isEmpty()); } + // Basic/default paging and sorting checkBasicPagingAndSorting( // Expected () -> actionService.getActionDefinitions(validNode). stream(). - sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)). map(ParameterizedItemDefinition::getName). collect(Collectors.toList()), // Actual results paging -> actions.getActionDefinitionsForNode(validNode.getId(), createParams(paging, null), 200)); + + // Test explicit sorting by title + checkSorting( + // Expected + () -> actionService.getActionDefinitions(validNode). + stream(). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getTitle, + nullsFirst(naturalOrder()))). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()), + // Actual results + (paging, orderBy) -> + actions.getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200), + "title"); - // Test sorting by title - { - // Retrieve all the actions directly using the ActionService and sort by title. - List> expectedActions = - actionService.getActionDefinitions(validNode). - stream(). - sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getTitle)). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Retrieve all action defs using the REST API - then check that they match - // the list retrieved directly from the ActionService. - PublicApiClient.Paging paging = getPaging(0, Integer.MAX_VALUE); - - // Retrieve all the results, sorted, on one page - Map orderBy = Collections.singletonMap("orderBy", "title"); - ListResponse actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - List> retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - assertEquals(expectedActions, retrievedActions); - - // Again, by title, but with explicit ascending sort order - orderBy = Collections.singletonMap("orderBy", "title asc"); - actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - assertEquals(expectedActions, retrievedActions); - - - // Descending sort order - orderBy = Collections.singletonMap("orderBy", "title desc"); - actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - Collections.reverse(expectedActions); - assertEquals(expectedActions, retrievedActions); - - // Combine paging with sorting by title, descending. - final int pageSize = 2; - paging = getPaging(pageSize, pageSize); - actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - assertEquals(expectedActions.subList(pageSize, pageSize*2), retrievedActions); - } - // Test explicit sorting by name - { - // Retrieve all the actions directly using the ActionService and sort by name. - List> expectedActions = - actionService.getActionDefinitions(validNode). - stream(). - sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Retrieve all action defs using the REST API - then check that they match - // the list retrieved directly from the ActionService. - PublicApiClient.Paging paging = getPaging(0, Integer.MAX_VALUE); - - // Retrieve all the results, sorted, on one page - Map orderBy = Collections.singletonMap("orderBy", "name"); - ListResponse actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - List> retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - assertEquals(expectedActions, retrievedActions); - - // Again, by name, but with explicit ascending sort order - orderBy = Collections.singletonMap("orderBy", "name asc"); - actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - assertEquals(expectedActions, retrievedActions); - - - // Descending sort order - orderBy = Collections.singletonMap("orderBy", "name desc"); - actionDefs = actions. - getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200); - - retrievedActions = actionDefs.getList().stream(). - map(act -> new Pair<>(act.getName(), act.getTitle())). - collect(Collectors.toList()); - - // Check the whole lists match - Collections.reverse(expectedActions); - assertEquals(expectedActions, retrievedActions); - } + checkSorting( + // Expected + () -> actionService.getActionDefinitions(validNode). + stream(). + sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getName, + nullsFirst(naturalOrder()))). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()), + // Actual results + (paging, orderBy) -> + actions.getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200), + "name"); // Badly formed request -> 400 { @@ -441,6 +379,12 @@ public class TestActions extends AbstractBaseApiTest { U apply(T t) throws V; } + + @FunctionalInterface + private interface CheckedBiFunction + { + V apply(T t, U u) throws W; + } private void checkBasicPagingAndSorting( Supplier> expectedNamesFun, @@ -501,4 +445,63 @@ public class TestActions extends AbstractBaseApiTest assertFalse(actionDefs.getPaging().getHasMoreItems()); } } + + private void checkSorting( + Supplier>> expectedFun, + CheckedBiFunction, ListResponse, PublicApiException> actionsFun, + String sortField) + throws PublicApiException + { + // Retrieve all the actions directly using the ActionService and sorted appropriately. + List> expectedActions = expectedFun.get(); + + // Retrieve all action defs using the REST API - then check that they match + // the list retrieved directly from the ActionService. + PublicApiClient.Paging paging = getPaging(0, Integer.MAX_VALUE); + + // Retrieve all the results, sorted, on one page + Map orderBy = Collections.singletonMap("orderBy", sortField); + ListResponse actionDefs = actionsFun.apply(paging, orderBy); + + List> retrievedActions = actionDefs.getList().stream(). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()); + + // Check the whole lists match + assertEquals(expectedActions, retrievedActions); + + // Again, by sortField, but with explicit ascending sort order + orderBy = Collections.singletonMap("orderBy", sortField + " asc"); + actionDefs = actionsFun.apply(paging, orderBy); + + retrievedActions = actionDefs.getList().stream(). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()); + + // Check the whole lists match + assertEquals(expectedActions, retrievedActions); + + // Descending sort order + orderBy = Collections.singletonMap("orderBy", sortField + " desc"); + actionDefs = actionsFun.apply(paging, orderBy); + + retrievedActions = actionDefs.getList().stream(). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()); + + // Check the whole lists match + Collections.reverse(expectedActions); + assertEquals(expectedActions, retrievedActions); + + // Combine paging with sorting by sortField, descending. + final int pageSize = 2; + paging = getPaging(pageSize, pageSize); + actionDefs = actionsFun.apply(paging, orderBy); + + retrievedActions = actionDefs.getList().stream(). + map(act -> new Pair<>(act.getName(), act.getTitle())). + collect(Collectors.toList()); + + assertEquals(expectedActions.subList(pageSize, pageSize*2), retrievedActions); + } }