Add pagination support for actions query API

Add query params for action list REST API:

* limit: return a maximun number of items at a time, default is None, the
  query result will include all the resource items, which is backward
  compatible.

* marker: the ID of the last item in the previous list.

* sort_keys: columns to sort results by. Default: name, which is backward
  compatible.

* sort_dirs: directions to sort corresponding to sort_keys, "asc" or
  "desc" can be choosed. Default: asc. The length of sort_dirs can
  be equal or less than that of sort_keys.

Change-Id: Ied5b48244cc94a3039ebafce40f3c2ae25f9a48e
Partially-Implements: blueprint mistral-pagination-support

mistral/api/controllers/resource.py

@@ -72,6 +72,20 @@ class ResourceList(Resource):
     def collection(self):
         return getattr(self, self._type)
 
+    @classmethod
+    def convert_with_links(cls, resources, limit, url=None, **kwargs):
+        resource_collection = cls()
+
+        setattr(resource_collection, resource_collection._type, resources)
+
+        resource_collection.next = resource_collection.get_next(
+            limit,
+            url=url,
+            **kwargs
+        )
+
+        return resource_collection
+
     def has_next(self, limit):
         """Return whether resources has more items."""
         return len(self.collection) and len(self.collection) == limit

mistral/api/controllers/v2/action.py

@@ -1,6 +1,5 @@
-# -*- coding: utf-8 -*-
-#
 # Copyright 2014 - Mirantis, Inc.
+# Copyright 2015 Huawei Technologies Co., Ltd.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License");
 #    you may not use this file except in compliance with the License.
@@ -22,6 +21,7 @@ from wsme import types as wtypes
 import wsmeext.pecan as wsme_pecan
 
 from mistral.api.controllers import resource
+from mistral.api.controllers.v2 import types
 from mistral.api.hooks import content_type as ct_hook
 from mistral.db.v2 import api as db_api
 from mistral import exceptions as exc
@@ -70,9 +70,21 @@ class Actions(resource.ResourceList):
 
     actions = [Action]
 
+    def __init__(self, **kwargs):
+        self._type = 'actions'
+
+        super(Actions, self).__init__(**kwargs)
+
     @classmethod
     def sample(cls):
-        return cls(actions=[Action.sample()])
+        actions_sample = cls()
+        actions_sample.actions = [Action.sample()]
+        actions_sample.next = "http://localhost:8989/v2/actions?" \
+                              "sort_keys=id,name&" \
+                              "sort_dirs=asc,desc&limit=10&" \
+                              "marker=123e4567-e89b-12d3-a456-426655440000"
+
+        return actions_sample
 
 
 class ActionsController(rest.RestController, hooks.HookController):
@@ -144,16 +156,49 @@ class ActionsController(rest.RestController, hooks.HookController):
 
             db_api.delete_action_definition(name)
 
-    @wsme_pecan.wsexpose(Actions)
+    @wsme_pecan.wsexpose(Actions, types.uuid, int, types.uniquelist,
-    def get_all(self):
+                         types.list)
+    def get_all(self, marker=None, limit=None, sort_keys='name',
+                sort_dirs='asc'):
         """Return all actions.
 
+        :param marker: Optional. Pagination marker for large data sets.
+        :param limit: Optional. Maximum number of resources to return in a
+                      single result. Default value is None for backward
+                      compatability.
+        :param sort_keys: Optional. Columns to sort results by.
+                          Default: name.
+        :param sort_dirs: Optional. Directions to sort corresponding to
+                          sort_keys, "asc" or "desc" can be choosed.
+                          Default: asc.
+
         Where project_id is the same as the requester or
         project_id is different but the scope is public.
         """
-        LOG.info("Fetch actions.")
+        LOG.info("Fetch actions. marker=%s, limit=%s, sort_keys=%s, "
+                 "sort_dirs=%s", marker, limit, sort_keys, sort_dirs)
 
-        action_list = [Action.from_dict(db_model.to_dict())
+        rest_utils.validate_query_params(limit, sort_keys, sort_dirs)
-                       for db_model in db_api.get_action_definitions()]
 
-        return Actions(actions=action_list)
+        marker_obj = None
+
+        if marker:
+            marker_obj = db_api.get_action_definition_by_id(marker)
+
+        db_action_defs = db_api.get_action_definitions(
+            limit=limit,
+            marker=marker_obj,
+            sort_keys=sort_keys,
+            sort_dirs=sort_dirs
+        )
+
+        actions_list = [Action.from_dict(db_model.to_dict())
+                        for db_model in db_action_defs]
+
+        return Actions.convert_with_links(
+            actions_list,
+            limit,
+            pecan.request.host_url,
+            sort_keys=','.join(sort_keys),
+            sort_dirs=','.join(sort_dirs)
+        )

mistral/api/controllers/v2/workflow.py

@@ -95,14 +95,6 @@ class Workflows(resource.ResourceList):
 
         super(Workflows, self).__init__(**kwargs)
 
-    @staticmethod
-    def convert_with_links(workflows, limit, url=None, **kwargs):
-        wf_collection = Workflows()
-        wf_collection.workflows = workflows
-        wf_collection.next = wf_collection.get_next(limit, url=url, **kwargs)
-
-        return wf_collection
-
     @classmethod
     def sample(cls):
         workflows_sample = cls()

mistral/db/v2/api.py

@@ -151,6 +151,10 @@ def delete_workflow_definitions(**kwargs):
 
 # Action definitions.
 
+def get_action_definition_by_id(id):
+    return IMPL.get_action_definition_by_id(id)
+
+
 def get_action_definition(name):
     return IMPL.get_action_definition(name)
 
@@ -160,8 +164,15 @@ def load_action_definition(name):
     return IMPL.load_action_definition(name)
 
 
-def get_action_definitions(**kwargs):
+def get_action_definitions(limit=None, marker=None, sort_keys=['name'],
-    return IMPL.get_action_definitions(**kwargs)
+                           sort_dirs=None, **kwargs):
+    return IMPL.get_action_definitions(
+        limit=limit,
+        marker=marker,
+        sort_keys=sort_keys,
+        sort_dirs=sort_dirs,
+        **kwargs
+    )
 
 
 def create_action_definition(values):

mistral/db/v2/sqlalchemy/api.py

@@ -346,6 +346,17 @@ def _get_workflow_definition_by_id(id):
 
 # Action definitions.
 
+def get_action_definition_by_id(id):
+    action_def = _get_db_object_by_id(models.ActionDefinition, id)
+
+    if not action_def:
+        raise exc.NotFoundException(
+            "Action not found [action_id=%s]" % id
+        )
+
+    return action_def
+
+
 def get_action_definition(name):
     a_def = _get_action_definition(name)
 
@@ -361,8 +372,24 @@ def load_action_definition(name):
     return _get_action_definition(name)
 
 
-def get_action_definitions(**kwargs):
+def get_action_definitions(limit=None, marker=None, sort_keys=['name'],
-    return _get_collection_sorted_by_name(models.ActionDefinition, **kwargs)
+                           sort_dirs=None, **kwargs):
+    query = _secure_query(models.ActionDefinition).filter_by(**kwargs)
+
+    try:
+        return _paginate_query(
+            models.ActionDefinition,
+            limit,
+            marker,
+            sort_keys,
+            sort_dirs,
+            query
+        )
+    except Exception as e:
+        raise exc.DBQueryEntryException(
+            "Failed when quering database, error type: %s, "
+            "error message: %s" % (e.__class__.__name__, e.message)
+        )
 
 
 @b.session_aware()

mistral/tests/functional/api/v2/test_mistral_basic_v2.py

@@ -673,6 +673,83 @@ class ActionTestsV2(base.TestCase):
 
         self.assertEqual(200, resp.status)
         self.assertNotEqual([], body['actions'])
+        self.assertNotIn('next', body)
+
+    @test.attr(type='smoke')
+    def test_get_list_actions_with_pagination(self):
+        resp, body = self.client.get_list_obj(
+            'actions?limit=1&sort_keys=name&sort_dirs=desc'
+        )
+
+        self.assertEqual(200, resp.status)
+        self.assertEqual(1, len(body['actions']))
+        self.assertIn('next', body)
+
+        name_1 = body['actions'][0].get('name')
+        next = body.get('next')
+
+        param_dict = utils.get_dict_from_string(
+            next.split('?')[1],
+            delimiter='&'
+        )
+
+        expected_sub_dict = {
+            'limit': 1,
+            'sort_keys': 'name',
+            'sort_dirs': 'desc'
+        }
+
+        self.assertDictContainsSubset(expected_sub_dict, param_dict)
+
+        # Query again using 'next' hint
+        url_param = next.split('/')[-1]
+        resp, body = self.client.get_list_obj(url_param)
+
+        self.assertEqual(200, resp.status)
+        self.assertEqual(1, len(body['actions']))
+
+        name_2 = body['actions'][0].get('name')
+
+        self.assertGreater(name_1, name_2)
+
+    @test.attr(type='negative')
+    def test_get_list_actions_nonexist_sort_dirs(self):
+        context = self.assertRaises(
+            exceptions.BadRequest,
+            self.client.get_list_obj,
+            'actions?limit=1&sort_keys=id&sort_dirs=nonexist'
+        )
+
+        self.assertIn(
+            'Unknown sort direction',
+            context.resp_body.get('faultstring')
+        )
+
+    @test.attr(type='negative')
+    def test_get_list_actions_invalid_limit(self):
+        context = self.assertRaises(
+            exceptions.BadRequest,
+            self.client.get_list_obj,
+            'actions?limit=-1&sort_keys=id&sort_dirs=asc'
+        )
+
+        self.assertIn(
+            'Limit must be positive',
+            context.resp_body.get('faultstring')
+        )
+
+    @test.attr(type='negative')
+    def test_get_list_actions_duplicate_sort_keys(self):
+        context = self.assertRaises(
+            exceptions.BadRequest,
+            self.client.get_list_obj,
+            'actions?limit=1&sort_keys=id,id&sort_dirs=asc,asc'
+        )
+
+        self.assertIn(
+            'Length of sort_keys must be equal or greater than sort_dirs',
+            context.resp_body.get('faultstring')
+        )
 
     @test.attr(type='sanity')
     def test_create_and_delete_few_actions(self):

mistral/tests/unit/api/v2/test_actions.py

@@ -21,6 +21,7 @@ from mistral.db.v2 import api as db_api
 from mistral.db.v2.sqlalchemy import models
 from mistral import exceptions as exc
 from mistral.tests.unit.api import base
+from mistral import utils
 
 
 ACTION_DEFINITION = """
@@ -46,7 +47,7 @@ std.echo:
 """
 
 ACTION = {
-    'id': '123',
+    'id': '123e4567-e89b-12d3-a456-426655440000',
     'name': 'my_action',
     'is_system': False,
     'description': 'My super cool action.',
@@ -220,3 +221,70 @@ class TestActionsController(base.FunctionalTest):
         self.assertEqual(resp.status_int, 200)
 
         self.assertEqual(len(resp.json['actions']), 0)
+
+    @mock.patch.object(db_api, "get_action_definitions", MOCK_ACTIONS)
+    def test_get_all_pagination(self):
+        resp = self.app.get(
+            '/v2/actions?limit=1&sort_keys=id,name')
+
+        self.assertEqual(resp.status_int, 200)
+        self.assertIn('next', resp.json)
+        self.assertEqual(len(resp.json['actions']), 1)
+        self.assertDictEqual(ACTION, resp.json['actions'][0])
+
+        param_dict = utils.get_dict_from_string(
+            resp.json['next'].split('?')[1],
+            delimiter='&'
+        )
+
+        expected_dict = {
+            'marker': '123e4567-e89b-12d3-a456-426655440000',
+            'limit': 1,
+            'sort_keys': 'id,name',
+            'sort_dirs': 'asc,asc'
+        }
+
+        self.assertDictEqual(expected_dict, param_dict)
+
+    def test_get_all_pagination_limit_negative(self):
+        resp = self.app.get(
+            '/v2/actions?limit=-1&sort_keys=id,name&sort_dirs=asc,asc',
+            expect_errors=True
+        )
+
+        self.assertEqual(resp.status_int, 400)
+
+        self.assertIn("Limit must be positive", resp.body)
+
+    def test_get_all_pagination_limit_not_integer(self):
+        resp = self.app.get(
+            '/v2/actions?limit=1.1&sort_keys=id,name&sort_dirs=asc,asc',
+            expect_errors=True
+        )
+
+        self.assertEqual(resp.status_int, 400)
+
+        self.assertIn("unable to convert to int", resp.body)
+
+    def test_get_all_pagination_invalid_sort_dirs_length(self):
+        resp = self.app.get(
+            '/v2/actions?limit=1&sort_keys=id,name&sort_dirs=asc,asc,asc',
+            expect_errors=True
+        )
+
+        self.assertEqual(resp.status_int, 400)
+
+        self.assertIn(
+            "Length of sort_keys must be equal or greater than sort_dirs",
+            resp.body
+        )
+
+    def test_get_all_pagination_unknown_direction(self):
+        resp = self.app.get(
+            '/v2/actions?limit=1&sort_keys=id&sort_dirs=nonexist',
+            expect_errors=True
+        )
+
+        self.assertEqual(resp.status_int, 400)
+
+        self.assertIn("Unknown sort direction", resp.body)