Adding pagination to snapshots and backups lists

Snapshot and backups pagination was recently implemented in the Cinder
backend. This patch is implementing a pagination for the snapshots and
backups on the client side in the same way that volume pagination works
using limit, marker and sort parameters.

Partial-Implements: blueprint extend-limit-implementations
Change-Id: Ie3660854407a947f7470b4dc0911704c0a31c1b4

cinderclient/base.py

@@ -24,12 +24,20 @@ import hashlib
 import os
 
 import six
+from six.moves.urllib import parse
 
 from cinderclient import exceptions
 from cinderclient.openstack.common.apiclient import base as common_base
 from cinderclient import utils
 
 
+# Valid sort directions and client sort keys
+SORT_DIR_VALUES = ('asc', 'desc')
+SORT_KEY_VALUES = ('id', 'status', 'size', 'availability_zone', 'name',
+                   'bootable', 'created_at')
+# Mapping of client keys to actual sort keys
+SORT_KEY_MAPPINGS = {'name': 'display_name'}
+
 Resource = common_base.Resource
 
 
@@ -110,6 +118,116 @@ class Manager(utils.HookableMixin):
                                    limit, items)
         return items
 
+    def _build_list_url(self, resource_type, detailed=True, search_opts=None,
+                        marker=None, limit=None, sort_key=None, sort_dir=None,
+                        sort=None):
+
+        if search_opts is None:
+            search_opts = {}
+
+        query_params = {}
+        for key, val in search_opts.items():
+            if val:
+                query_params[key] = val
+
+        if marker:
+            query_params['marker'] = marker
+
+        if limit:
+            query_params['limit'] = limit
+
+        if sort:
+            query_params['sort'] = self._format_sort_param(sort)
+        else:
+            # sort_key and sort_dir deprecated in kilo, prefer sort
+            if sort_key:
+                query_params['sort_key'] = self._format_sort_key_param(
+                    sort_key)
+
+            if sort_dir:
+                query_params['sort_dir'] = self._format_sort_dir_param(
+                    sort_dir)
+
+        # Transform the dict to a sequence of two-element tuples in fixed
+        # order, then the encoded string will be consistent in Python 2&3.
+        query_string = ""
+        if query_params:
+            params = sorted(query_params.items(), key=lambda x: x[0])
+            query_string = "?%s" % parse.urlencode(params)
+
+        detail = ""
+        if detailed:
+            detail = "/detail"
+
+        return ("/%(resource_type)s%(detail)s%(query_string)s" %
+                {"resource_type": resource_type, "detail": detail,
+                 "query_string": query_string})
+
+    def _format_sort_param(self, sort):
+        '''Formats the sort information into the sort query string parameter.
+
+        The input sort information can be any of the following:
+        - Comma-separated string in the form of <key[:dir]>
+        - List of strings in the form of <key[:dir]>
+        - List of either string keys, or tuples of (key, dir)
+
+        For example, the following import sort values are valid:
+        - 'key1:dir1,key2,key3:dir3'
+        - ['key1:dir1', 'key2', 'key3:dir3']
+        - [('key1', 'dir1'), 'key2', ('key3', dir3')]
+
+        :param sort: Input sort information
+        :returns: Formatted query string parameter or None
+        :raise ValueError: If an invalid sort direction or invalid sort key is
+                           given
+        '''
+        if not sort:
+            return None
+
+        if isinstance(sort, six.string_types):
+            # Convert the string into a list for consistent validation
+            sort = [s for s in sort.split(',') if s]
+
+        sort_array = []
+        for sort_item in sort:
+            if isinstance(sort_item, tuple):
+                sort_key = sort_item[0]
+                sort_dir = sort_item[1]
+            else:
+                sort_key, _sep, sort_dir = sort_item.partition(':')
+            sort_key = sort_key.strip()
+            if sort_key in SORT_KEY_VALUES:
+                sort_key = SORT_KEY_MAPPINGS.get(sort_key, sort_key)
+            else:
+                raise ValueError('sort_key must be one of the following: %s.'
+                                 % ', '.join(SORT_KEY_VALUES))
+            if sort_dir:
+                sort_dir = sort_dir.strip()
+                if sort_dir not in SORT_DIR_VALUES:
+                    msg = ('sort_dir must be one of the following: %s.'
+                           % ', '.join(SORT_DIR_VALUES))
+                    raise ValueError(msg)
+                sort_array.append('%s:%s' % (sort_key, sort_dir))
+            else:
+                sort_array.append(sort_key)
+        return ','.join(sort_array)
+
+    def _format_sort_key_param(self, sort_key):
+        if sort_key in SORT_KEY_VALUES:
+            return SORT_KEY_MAPPINGS.get(sort_key, sort_key)
+
+        msg = ('sort_key must be one of the following: %s.' %
+               ', '.join(SORT_KEY_VALUES))
+        raise ValueError(msg)
+
+    def _format_sort_dir_param(self, sort_dir):
+        if sort_dir in SORT_DIR_VALUES:
+            return sort_dir
+
+        msg = ('sort_dir must be one of the following: %s.'
+               % ', '.join(SORT_DIR_VALUES))
+        raise ValueError(msg)
+
     @contextlib.contextmanager
     def completion_cache(self, cache_type, obj_class, mode):
         """

cinderclient/tests/unit/fixture_data/snapshots.py

@@ -52,5 +52,14 @@ class Fixture(base.Fixture):
             else:
                 raise AssertionError("Unexpected action: %s" % action)
             return ''
+
         self.requests.register_uri('POST', self.url('1234', 'action'),
                                    text=action_1234, status_code=202)
+
+        self.requests.register_uri('GET',
+                                   self.url('detail?limit=2&marker=1234'),
+                                   status_code=200, json={'snapshots': []})
+
+        self.requests.register_uri('GET',
+                                   self.url('detail?sort=id'),
+                                   status_code=200, json={'snapshots': []})

cinderclient/tests/unit/v2/test_snapshot_actions.py

@@ -34,3 +34,11 @@ class SnapshotActionsTest(utils.FixturedTestCase):
         stat = {'status': 'available', 'progress': '73%'}
         self.cs.volume_snapshots.update_snapshot_status(s, stat)
         self.assert_called('POST', '/snapshots/1234/action')
+
+    def test_list_snapshots_with_marker_limit(self):
+        self.cs.volume_snapshots.list(marker=1234, limit=2)
+        self.assert_called('GET', '/snapshots/detail?limit=2&marker=1234')
+
+    def test_list_snapshots_with_sort(self):
+        self.cs.volume_snapshots.list(sort="id")
+        self.assert_called('GET', '/snapshots/detail?sort=id')

cinderclient/tests/unit/v2/test_volume_backups.py

@@ -51,6 +51,14 @@ class VolumeBackupsTest(utils.TestCase):
         cs.backups.list()
         cs.assert_called('GET', '/backups/detail')
 
+    def test_list_with_pagination(self):
+        cs.backups.list(limit=2, marker=100)
+        cs.assert_called('GET', '/backups/detail?limit=2&marker=100')
+
+    def test_sorted_list(self):
+        cs.backups.list(sort="id")
+        cs.assert_called('GET', '/backups/detail?sort=id')
+
     def test_delete(self):
         b = cs.backups.list()[0]
         b.delete()

cinderclient/v2/shell.py

@@ -24,11 +24,11 @@ import time
 
 import six
 
+from cinderclient import base
 from cinderclient import exceptions
 from cinderclient import utils
 from cinderclient.openstack.common import strutils
 from cinderclient.v2 import availability_zones
-from cinderclient.v2 import volumes
 
 
 def _poll_for_status(poll_fn, obj_id, action, final_ok_states,
@@ -196,7 +196,7 @@ def _extract_metadata(args):
            help=(('Comma-separated list of sort keys and directions in the '
                   'form of <key>[:<asc|desc>]. '
                   'Valid keys: %s. '
-                  'Default=None.') % ', '.join(volumes.SORT_KEY_VALUES)))
+                  'Default=None.') % ', '.join(base.SORT_KEY_VALUES)))
 @utils.arg('--tenant',
            type=str,
            dest='tenant',
@@ -619,6 +619,23 @@ def do_image_metadata(cs, args):
            help='Filters results by a volume ID. Default=None.')
 @utils.arg('--volume_id',
            help=argparse.SUPPRESS)
+@utils.arg('--marker',
+           metavar='<marker>',
+           default=None,
+           help='Begin returning snapshots that appear later in the snapshot '
+                'list than that represented by this id. '
+                'Default=None.')
+@utils.arg('--limit',
+           metavar='<limit>',
+           default=None,
+           help='Maximum number of snapshots to return. Default=None.')
+@utils.arg('--sort',
+           metavar='<key>[:<direction>]',
+           default=None,
+           help=(('Comma-separated list of sort keys and directions in the '
+                  'form of <key>[:<asc|desc>]. '
+                  'Valid keys: %s. '
+                  'Default=None.') % ', '.join(base.SORT_KEY_VALUES)))
 @utils.service_type('volumev2')
 def do_snapshot_list(cs, args):
     """Lists all snapshots."""
@@ -634,7 +651,10 @@ def do_snapshot_list(cs, args):
         'volume_id': args.volume_id,
     }
 
-    snapshots = cs.volume_snapshots.list(search_opts=search_opts)
+    snapshots = cs.volume_snapshots.list(search_opts=search_opts,
+                                         marker=args.marker,
+                                         limit=args.limit,
+                                         sort=args.sort)
     _translate_volume_snapshot_keys(snapshots)
     utils.print_list(snapshots,
                      ['ID', 'Volume ID', 'Status', 'Name', 'Size'])
@@ -1312,6 +1332,23 @@ def do_backup_show(cs, args):
            help='Filters results by a volume ID. Default=None.')
 @utils.arg('--volume_id',
            help=argparse.SUPPRESS)
+@utils.arg('--marker',
+           metavar='<marker>',
+           default=None,
+           help='Begin returning backups that appear later in the backup '
+                'list than that represented by this id. '
+                'Default=None.')
+@utils.arg('--limit',
+           metavar='<limit>',
+           default=None,
+           help='Maximum number of backups to return. Default=None.')
+@utils.arg('--sort',
+           metavar='<key>[:<direction>]',
+           default=None,
+           help=(('Comma-separated list of sort keys and directions in the '
+                  'form of <key>[:<asc|desc>]. '
+                  'Valid keys: %s. '
+                  'Default=None.') % ', '.join(base.SORT_KEY_VALUES)))
 @utils.service_type('volumev2')
 def do_backup_list(cs, args):
     """Lists all backups."""
@@ -1323,7 +1360,10 @@ def do_backup_list(cs, args):
         'volume_id': args.volume_id,
     }
 
-    backups = cs.backups.list(search_opts=search_opts)
+    backups = cs.backups.list(search_opts=search_opts,
+                              marker=args.marker,
+                              limit=args.limit,
+                              sort=args.sort)
     _translate_volume_snapshot_keys(backups)
     columns = ['ID', 'Volume ID', 'Status', 'Name', 'Size', 'Object Count',
                'Container']

cinderclient/v2/volume_backups.py

@@ -16,7 +16,6 @@
 """
 Volume Backups interface (1.1 extension).
 """
-from six.moves.urllib.parse import urlencode
 from cinderclient import base
 
 
@@ -65,21 +64,17 @@ class VolumeBackupManager(base.ManagerWithFind):
         """
         return self._get("/backups/%s" % backup_id, "backup")
 
-    def list(self, detailed=True, search_opts=None):
+    def list(self, detailed=True, search_opts=None, marker=None, limit=None,
+             sort=None):
         """Get a list of all volume backups.
 
         :rtype: list of :class:`VolumeBackup`
         """
-        search_opts = search_opts or {}
+        resource_type = "backups"
-
+        url = self._build_list_url(resource_type, detailed=detailed,
-        qparams = dict((key, val) for key, val in search_opts.items() if val)
+                                   search_opts=search_opts, marker=marker,
-
+                                   limit=limit, sort=sort)
-        query_string = ("?%s" % urlencode(qparams)) if qparams else ""
+        return self._list(url, resource_type, limit=limit)
-
-        detail = '/detail' if detailed else ''
-
-        return self._list("/backups%s%s" % (detail, query_string),
-                          "backups")
 
     def delete(self, backup):
         """Delete a volume backup.

cinderclient/v2/volume_snapshots.py

@@ -15,12 +15,6 @@
 
 """Volume snapshot interface (1.1 extension)."""
 
-import six
-try:
-    from urllib import urlencode
-except ImportError:
-    from urllib.parse import urlencode
-
 from cinderclient import base
 
 
@@ -102,34 +96,17 @@ class SnapshotManager(base.ManagerWithFind):
         """
         return self._get("/snapshots/%s" % snapshot_id, "snapshot")
 
-    def list(self, detailed=True, search_opts=None):
+    def list(self, detailed=True, search_opts=None, marker=None, limit=None,
+             sort=None):
         """Get a list of all snapshots.
 
         :rtype: list of :class:`Snapshot`
         """
-        if search_opts is None:
+        resource_type = "snapshots"
-            search_opts = {}
+        url = self._build_list_url(resource_type, detailed=detailed,
-
+                                   search_opts=search_opts, marker=marker,
-        qparams = {}
+                                   limit=limit, sort=sort)
-
+        return self._list(url, resource_type, limit=limit)
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        # Transform the dict to a sequence of two-element tuples in fixed
-        # order, then the encoded string will be consistent in Python 2&3.
-        if qparams:
-            new_qparams = sorted(qparams.items(), key=lambda x: x[0])
-            query_string = "?%s" % urlencode(new_qparams)
-        else:
-            query_string = ""
-
-        detail = ""
-        if detailed:
-            detail = "/detail"
-
-        return self._list("/snapshots%s%s" % (detail, query_string),
-                          "snapshots")
 
     def delete(self, snapshot):
         """Delete a snapshot.

cinderclient/v2/volumes.py

@@ -15,23 +15,9 @@
 
 """Volume interface (v2 extension)."""
 
-import six
-try:
-    from urllib import urlencode
-except ImportError:
-    from urllib.parse import urlencode
-
 from cinderclient import base
 
 
-# Valid sort directions and client sort keys
-SORT_DIR_VALUES = ('asc', 'desc')
-SORT_KEY_VALUES = ('id', 'status', 'size', 'availability_zone', 'name',
-                   'bootable', 'created_at')
-# Mapping of client keys to actual sort keys
-SORT_KEY_MAPPINGS = {'name': 'display_name'}
-
-
 class Volume(base.Resource):
     """A volume is an extra block level storage to the OpenStack instances."""
     def __repr__(self):
@@ -260,55 +246,6 @@ class VolumeManager(base.ManagerWithFind):
         """
         return self._get("/volumes/%s" % volume_id, "volume")
 
-    def _format_sort_param(self, sort):
-        '''Formats the sort information into the sort query string parameter.
-
-        The input sort information can be any of the following:
-        - Comma-separated string in the form of <key[:dir]>
-        - List of strings in the form of <key[:dir]>
-        - List of either string keys, or tuples of (key, dir)
-
-        For example, the following import sort values are valid:
-        - 'key1:dir1,key2,key3:dir3'
-        - ['key1:dir1', 'key2', 'key3:dir3']
-        - [('key1', 'dir1'), 'key2', ('key3', dir3')]
-
-        :param sort: Input sort information
-        :returns: Formatted query string parameter or None
-        :raise ValueError: If an invalid sort direction or invalid sort key is
-                           given
-        '''
-        if not sort:
-            return None
-
-        if isinstance(sort, six.string_types):
-            # Convert the string into a list for consistent validation
-            sort = [s for s in sort.split(',') if s]
-
-        sort_array = []
-        for sort_item in sort:
-            if isinstance(sort_item, tuple):
-                sort_key = sort_item[0]
-                sort_dir = sort_item[1]
-            else:
-                sort_key, _sep, sort_dir = sort_item.partition(':')
-            sort_key = sort_key.strip()
-            if sort_key in SORT_KEY_VALUES:
-                sort_key = SORT_KEY_MAPPINGS.get(sort_key, sort_key)
-            else:
-                raise ValueError('sort_key must be one of the following: %s.'
-                                 % ', '.join(SORT_KEY_VALUES))
-            if sort_dir:
-                sort_dir = sort_dir.strip()
-                if sort_dir not in SORT_DIR_VALUES:
-                    msg = ('sort_dir must be one of the following: %s.'
-                           % ', '.join(SORT_DIR_VALUES))
-                    raise ValueError(msg)
-                sort_array.append('%s:%s' % (sort_key, sort_dir))
-            else:
-                sort_array.append(sort_key)
-        return ','.join(sort_array)
-
     def list(self, detailed=True, search_opts=None, marker=None, limit=None,
              sort_key=None, sort_dir=None, sort=None):
         """Lists all volumes.
@@ -324,55 +261,13 @@ class VolumeManager(base.ManagerWithFind):
         :param sort: Sort information
         :rtype: list of :class:`Volume`
         """
-        if search_opts is None:
-            search_opts = {}
 
-        qparams = {}
+        resource_type = "volumes"
-
+        url = self._build_list_url(resource_type, detailed=detailed,
-        for opt, val in six.iteritems(search_opts):
+                                   search_opts=search_opts, marker=marker,
-            if val:
+                                   limit=limit, sort_key=sort_key,
-                qparams[opt] = val
+                                   sort_dir=sort_dir, sort=sort)
-
+        return self._list(url, resource_type, limit=limit)
-        if marker:
-            qparams['marker'] = marker
-
-        if limit:
-            qparams['limit'] = limit
-
-        # sort_key and sort_dir deprecated in kilo, prefer sort
-        if sort:
-            qparams['sort'] = self._format_sort_param(sort)
-        else:
-            if sort_key is not None:
-                if sort_key in SORT_KEY_VALUES:
-                    qparams['sort_key'] = SORT_KEY_MAPPINGS.get(sort_key,
-                                                                sort_key)
-                else:
-                    msg = ('sort_key must be one of the following: %s.'
-                           % ', '.join(SORT_KEY_VALUES))
-                    raise ValueError(msg)
-            if sort_dir is not None:
-                if sort_dir in SORT_DIR_VALUES:
-                    qparams['sort_dir'] = sort_dir
-                else:
-                    msg = ('sort_dir must be one of the following: %s.'
-                           % ', '.join(SORT_DIR_VALUES))
-                    raise ValueError(msg)
-
-        # Transform the dict to a sequence of two-element tuples in fixed
-        # order, then the encoded string will be consistent in Python 2&3.
-        if qparams:
-            new_qparams = sorted(qparams.items(), key=lambda x: x[0])
-            query_string = "?%s" % urlencode(new_qparams)
-        else:
-            query_string = ""
-
-        detail = ""
-        if detailed:
-            detail = "/detail"
-
-        return self._list("/volumes%s%s" % (detail, query_string),
-                          "volumes", limit=limit)
 
     def delete(self, volume):
         """Delete a volume.