REPO-1308: factored out common sorting tests

The sorting/paging tests for this and for getting the actions available
for a particular node are basically the same, so factored out the
common code.

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'.");

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,12 +156,37 @@ 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
         {
@@ -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 sorting by title
+        // Test explicit sorting by title
-        {
+        checkSorting(
-            // Retrieve all the actions directly using the ActionService and sort by title.
+                // Expected
-            List<Pair<String, String>> expectedActions =
+                () -> actionService.getActionDefinitions(validNode).
-                    actionService.getActionDefinitions(validNode).
                         stream().
-                            sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getTitle)).
+                        sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getTitle,
+                                nullsFirst(naturalOrder()))).
                         map(act -> new Pair<>(act.getName(), act.getTitle())).
-                            collect(Collectors.toList());
+                        collect(Collectors.toList()),
-
+                // Actual results
-            // Retrieve all action defs using the REST API - then check that they match
+                (paging, orderBy) ->
-            // the list retrieved directly from the ActionService.
+                        actions.getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200),
-            PublicApiClient.Paging paging = getPaging(0, Integer.MAX_VALUE);
+                "title");
-
-            // Retrieve all the results, sorted, on one page
-            Map<String, String> orderBy = Collections.singletonMap("orderBy", "title");
-            ListResponse<ActionDefinition> actionDefs = actions.
-                    getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200);
-            
-            List<Pair<String, String>> 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
-        {
+        checkSorting(
-            // Retrieve all the actions directly using the ActionService and sort by name.
+                // Expected
-            List<Pair<String, String>> expectedActions =
+                () -> actionService.getActionDefinitions(validNode).
-                    actionService.getActionDefinitions(validNode).
                         stream().
-                            sorted(Comparator.comparing(org.alfresco.service.cmr.action.ActionDefinition::getName)).
+                        sorted(comparing(org.alfresco.service.cmr.action.ActionDefinition::getName,
+                                nullsFirst(naturalOrder()))).
                         map(act -> new Pair<>(act.getName(), act.getTitle())).
-                            collect(Collectors.toList());
+                        collect(Collectors.toList()),
-
+                // Actual results
-            // Retrieve all action defs using the REST API - then check that they match
+                (paging, orderBy) ->
-            // the list retrieved directly from the ActionService.
+                        actions.getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200),
-            PublicApiClient.Paging paging = getPaging(0, Integer.MAX_VALUE);
+                "name");
-
-            // Retrieve all the results, sorted, on one page
-            Map<String, String> orderBy = Collections.singletonMap("orderBy", "name");
-            ListResponse<ActionDefinition> actionDefs = actions.
-                    getActionDefinitionsForNode(validNode.getId(), createParams(paging, orderBy), 200);
-
-            List<Pair<String, String>> 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);
-        }
         
         // Badly formed request -> 400
         {
@@ -442,6 +380,12 @@ public class TestActions extends AbstractBaseApiTest
         U apply(T t) throws V;
     }
 
+    @FunctionalInterface
+    private interface CheckedBiFunction<T, U, V, W extends Exception>
+    {
+        V apply(T t, U u) throws W;
+    }
+    
     private void checkBasicPagingAndSorting(
             Supplier<List<String>> expectedNamesFun,
             CheckedFunction<PublicApiClient.Paging, ListResponse<ActionDefinition>, PublicApiException> actionsFun)
@@ -501,4 +445,63 @@ public class TestActions extends AbstractBaseApiTest
             assertFalse(actionDefs.getPaging().getHasMoreItems());
         }
     }
+    
+    private void checkSorting(
+            Supplier<List<Pair<String, String>>> expectedFun,
+            CheckedBiFunction<PublicApiClient.Paging, Map<String, String>, ListResponse<ActionDefinition>, PublicApiException> actionsFun,
+            String sortField)
+            throws PublicApiException
+    {
+        // Retrieve all the actions directly using the ActionService and sorted appropriately.
+        List<Pair<String, String>> 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<String, String> orderBy = Collections.singletonMap("orderBy", sortField);
+        ListResponse<ActionDefinition> actionDefs = actionsFun.apply(paging, orderBy);
+
+        List<Pair<String, String>> 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);
+    }
 }