Add count info in 'snapshot list' API

Added support for display count info in share snapshot
list&detail APIs:

1. /v2/snapshots?with_count=True
2. /v2/snapshots/detail?with_count=True

New microversion added 2.79

Closes-bug: #2024556
Change-Id: I37d8ca9022e2ea2c107c6695e20e951d7950043a

api-ref/source/parameters.yaml

@@ -653,11 +653,18 @@ user_id_query:
   type: string
 with_count_query:
   description: |
-    Whether to show ``count`` in API response or not, default is ``False``.
+    Whether to show ``count`` in share list API response or not, default is ``False``.
   in: query
   required: false
   type: boolean
   min_version: 2.42
+with_count_snapshot_query:
+  description: |
+    Whether to show ``count`` in share snapshot list API response or not, default is ``False``.
+  in: query
+  required: false
+  type: boolean
+  min_version: 2.79
 
 # variables in body
 accepted:

api-ref/source/snapshots.inc

@@ -70,6 +70,7 @@ Request
    - all_tenants: all_tenants_query
    - name~: name_inexact_query
    - description~: description_inexact_query
+   - with_count: with_count_snapshot_query
 
 Response parameters
 -------------------
@@ -114,6 +115,7 @@ Request
    - all_tenants: all_tenants_query
    - name~: name_inexact_query
    - description~: description_inexact_query
+   - with_count: with_count_snapshot_query
 
 Response parameters
 -------------------

manila/api/openstack/api_version_request.py

@@ -195,6 +195,8 @@ REST_API_VERSION_HISTORY = """
     * 2.76 - Added 'default_ad_site' field in security service object.
     * 2.77 - Added support for share transfer between different projects.
     * 2.78 - Added Share Network Subnet Metadata to Metadata API.
+    * 2.79 - Added ``with_count`` in share snapshot list API to get total
+             count info.
 
 """
 
@@ -202,7 +204,7 @@ REST_API_VERSION_HISTORY = """
 # The default api version request is defined to be the
 # minimum version of the API supported.
 _MIN_API_VERSION = "2.0"
-_MAX_API_VERSION = "2.78"
+_MAX_API_VERSION = "2.79"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

manila/api/openstack/rest_api_version_history.rst

@@ -427,3 +427,7 @@ ____
 ---------------------------------
   Added Metadata API methods (GET, PUT, POST, DELETE)
   to Share Network Subnets.
+
+2.79
+------------------------
+  Added ``with_count`` in share snapshot list API to get total count info.

manila/api/v1/share_snapshots.py

@@ -30,6 +30,7 @@ from manila import db
 from manila import exception
 from manila.i18n import _
 from manila import share
+from manila import utils
 
 LOG = log.getLogger(__name__)
 
@@ -100,6 +101,13 @@ class ShareSnapshotMixin(object):
         # Remove keys that are not related to share attrs
         search_opts.pop('limit', None)
         search_opts.pop('offset', None)
+
+        show_count = False
+        if 'with_count' in search_opts:
+            show_count = utils.get_bool_from_api_params(
+                'with_count', search_opts)
+            search_opts.pop('with_count')
+
         sort_key, sort_dir = common.get_sort_params(search_opts)
         key_dict = {"name": "display_name",
                     "description": "display_description"}
@@ -137,19 +145,23 @@ class ShareSnapshotMixin(object):
         common.remove_invalid_options(context, search_opts,
                                       self._get_snapshots_search_options())
 
-        snapshots = self.share_api.get_all_snapshots(
+        total_count = None
-            context,
+        if show_count:
-            search_opts=search_opts,
+            count, snapshots = self.share_api.get_all_snapshots_with_count(
-            limit=limit,
+                context, search_opts=search_opts, limit=limit, offset=offset,
-            offset=offset,
+                sort_key=sort_key, sort_dir=sort_dir)
-            sort_key=sort_key,
+            total_count = count
-            sort_dir=sort_dir,
+        else:
-        )
+            snapshots = self.share_api.get_all_snapshots(
+                context, search_opts=search_opts, limit=limit, offset=offset,
+                sort_key=sort_key, sort_dir=sort_dir)
 
         if is_detail:
-            snapshots = self._view_builder.detail_list(req, snapshots)
+            snapshots = self._view_builder.detail_list(
+                req, snapshots, total_count)
         else:
-            snapshots = self._view_builder.summary_list(req, snapshots)
+            snapshots = self._view_builder.summary_list(
+                req, snapshots, total_count)
         return snapshots
 
     def _get_snapshots_search_options(self):

manila/api/v2/share_snapshots.py

@@ -336,6 +336,10 @@ class ShareSnapshotsController(share_snapshots.ShareSnapshotMixin,
             req.GET.pop('name~', None)
             req.GET.pop('description~', None)
             req.GET.pop('description', None)
+
+        if req.api_version_request < api_version.APIVersionRequest("2.79"):
+            req.GET.pop('with_count', None)
+
         return self._get_snapshots(req, is_detail=False)
 
     @wsgi.Controller.api_version("2.0")

manila/api/views/share_snapshots.py

@@ -26,13 +26,13 @@ class ViewBuilder(common.ViewBuilder):
         "add_metadata"
     ]
 
-    def summary_list(self, request, snapshots):
+    def summary_list(self, request, snapshots, count=None):
         """Show a list of share snapshots without many details."""
-        return self._list_view(self.summary, request, snapshots)
+        return self._list_view(self.summary, request, snapshots, count)
 
-    def detail_list(self, request, snapshots):
+    def detail_list(self, request, snapshots, count=None):
         """Detailed view of a list of share snapshots."""
-        return self._list_view(self.detail, request, snapshots)
+        return self._list_view(self.detail, request, snapshots, count)
 
     def summary(self, request, snapshot):
         """Generic, non-detailed view of an share snapshot."""
@@ -84,7 +84,7 @@ class ViewBuilder(common.ViewBuilder):
             metadata = {}
         snapshot_dict['metadata'] = metadata
 
-    def _list_view(self, func, request, snapshots):
+    def _list_view(self, func, request, snapshots, count=None):
         """Provide a view for a list of share snapshots."""
         snapshots_list = [func(request, snapshot)['snapshot']
                           for snapshot in snapshots]
@@ -93,6 +93,8 @@ class ViewBuilder(common.ViewBuilder):
                                                      self._collection_name)
         snapshots_dict = {self._collection_name: snapshots_list}
 
+        if count is not None:
+            snapshots_dict['count'] = count
         if snapshots_links:
             snapshots_dict['share_snapshots_links'] = snapshots_links
 

manila/db/api.py

@@ -692,6 +692,15 @@ def share_snapshot_get_all(context, filters=None, limit=None, offset=None,
         sort_key=sort_key, sort_dir=sort_dir)
 
 
+def share_snapshot_get_all_with_count(context, filters=None, limit=None,
+                                      offset=None, sort_key=None,
+                                      sort_dir=None):
+    """Get all snapshots."""
+    return IMPL.share_snapshot_get_all_with_count(
+        context, filters=filters, limit=limit, offset=offset,
+        sort_key=sort_key, sort_dir=sort_dir)
+
+
 def share_snapshot_get_all_by_project(context, project_id, filters=None,
                                       limit=None, offset=None, sort_key=None,
                                       sort_dir=None):
@@ -701,6 +710,16 @@ def share_snapshot_get_all_by_project(context, project_id, filters=None,
         sort_key=sort_key, sort_dir=sort_dir)
 
 
+def share_snapshot_get_all_by_project_with_count(context, project_id,
+                                                 filters=None, limit=None,
+                                                 offset=None, sort_key=None,
+                                                 sort_dir=None):
+    """Get all snapshots belonging to a project."""
+    return IMPL.share_snapshot_get_all_by_project_with_count(
+        context, project_id, filters=filters, limit=limit, offset=offset,
+        sort_key=sort_key, sort_dir=sort_dir)
+
+
 def share_snapshot_get_all_for_share(context, share_id, filters=None,
                                      sort_key=None, sort_dir=None):
     """Get all snapshots for a share."""

manila/db/sqlalchemy/api.py

@@ -3319,7 +3319,8 @@ def share_snapshot_get(context, snapshot_id, project_only=True, session=None):
 def _share_snapshot_get_all_with_filters(context, project_id=None,
                                          share_id=None, filters=None,
                                          limit=None, offset=None,
-                                         sort_key=None, sort_dir=None):
+                                         sort_key=None, sort_dir=None,
+                                         show_count=False):
     """Retrieves all snapshots.
 
     If no sorting parameters are specified then returned snapshots are sorted
@@ -3389,13 +3390,25 @@ def _share_snapshot_get_all_with_filters(context, project_id=None,
     query = exact_filter(query, models.ShareSnapshot,
                          filters, legal_filter_keys)
 
-    query = utils.paginate_query(query, models.ShareSnapshot, limit,
+    query = apply_sorting(models.ShareSnapshot, query, sort_key, sort_dir)
-                                 sort_key=sort_key,
-                                 sort_dir=sort_dir,
-                                 offset=offset)
 
-    # Returns list of shares that satisfy filters
+    count = None
-    return query.all()
+    if show_count:
+        count = query.count()
+
+    if limit is not None:
+        query = query.limit(limit)
+
+    if offset:
+        query = query.offset(offset)
+
+    # Returns list of share snapshots that satisfy filters
+    query = query.all()
+
+    if show_count:
+        return count, query
+
+    return query
 
 
 @require_admin_context
@@ -3406,6 +3419,17 @@ def share_snapshot_get_all(context, filters=None, limit=None, offset=None,
         offset=offset, sort_key=sort_key, sort_dir=sort_dir)
 
 
+@require_admin_context
+def share_snapshot_get_all_with_count(context, filters=None, limit=None,
+                                      offset=None, sort_key=None,
+                                      sort_dir=None):
+    count, query = _share_snapshot_get_all_with_filters(
+        context, filters=filters, limit=limit,
+        offset=offset, sort_key=sort_key, sort_dir=sort_dir,
+        show_count=True)
+    return count, query
+
+
 @require_context
 def share_snapshot_get_all_by_project(context, project_id, filters=None,
                                       limit=None, offset=None,
@@ -3416,6 +3440,19 @@ def share_snapshot_get_all_by_project(context, project_id, filters=None,
         offset=offset, sort_key=sort_key, sort_dir=sort_dir)
 
 
+@require_context
+def share_snapshot_get_all_by_project_with_count(context, project_id,
+                                                 filters=None, limit=None,
+                                                 offset=None, sort_key=None,
+                                                 sort_dir=None):
+    authorize_project_context(context, project_id)
+    count, query = _share_snapshot_get_all_with_filters(
+        context, project_id=project_id, filters=filters, limit=limit,
+        offset=offset, sort_key=sort_key, sort_dir=sort_dir,
+        show_count=True)
+    return count, query
+
+
 @require_context
 def share_snapshot_get_all_for_share(context, share_id, filters=None,
                                      sort_key=None, sort_dir=None):

manila/share/api.py

@@ -2177,6 +2177,21 @@ class API(base.Base):
 
     def get_all_snapshots(self, context, search_opts=None, limit=None,
                           offset=None, sort_key='share_id', sort_dir='desc'):
+        return self._get_all_snapshots(context, search_opts=search_opts,
+                                       limit=limit, offset=offset,
+                                       sort_key=sort_key, sort_dir=sort_dir)
+
+    def get_all_snapshots_with_count(self, context, search_opts=None,
+                                     limit=None, offset=None,
+                                     sort_key='share_id', sort_dir='desc'):
+        return self._get_all_snapshots(context, search_opts=search_opts,
+                                       limit=limit, offset=offset,
+                                       sort_key=sort_key, sort_dir=sort_dir,
+                                       show_count=True)
+
+    def _get_all_snapshots(self, context, search_opts=None, limit=None,
+                           offset=None, sort_key='share_id', sort_dir='desc',
+                           show_count=False):
         policy.check_policy(context, 'share_snapshot', 'get_all_snapshots')
 
         search_opts = search_opts or {}
@@ -2193,17 +2208,32 @@ class API(base.Base):
                         "'%(v)s'.") % {'k': k, 'v': string_args[k]}
                 raise exception.InvalidInput(reason=msg)
 
+        get_methods = {
+            'get_all': (
+                self.db.share_snapshot_get_all_with_count
+                if show_count else self.db.share_snapshot_get_all),
+            'get_all_by_project': (
+                self.db.share_snapshot_get_all_by_project_with_count
+                if show_count else self.db.share_snapshot_get_all_by_project)}
+
         if context.is_admin and all_tenants:
-            snapshots = self.db.share_snapshot_get_all(
+            result = get_methods['get_all'](
                 context, filters=search_opts, limit=limit, offset=offset,
                 sort_key=sort_key, sort_dir=sort_dir)
         else:
-            snapshots = self.db.share_snapshot_get_all_by_project(
+            result = get_methods['get_all_by_project'](
                 context, context.project_id, filters=search_opts,
                 limit=limit, offset=offset, sort_key=sort_key,
                 sort_dir=sort_dir)
 
-        return snapshots
+        if show_count:
+            count = result[0]
+            snapshots = result[1]
+        else:
+            snapshots = result
+
+        result = (count, snapshots) if show_count else snapshots
+        return result
 
     def get_latest_snapshot_for_share(self, context, share_id):
         """Get the newest snapshot of a share."""

manila/tests/api/v2/test_share_snapshots.py

@@ -196,6 +196,10 @@ class ShareSnapshotAPITest(test.TestCase):
                 api_version.APIVersionRequest('2.36')):
             search_opts.pop('name')
             search_opts['display_name~'] = 'fake_name'
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            search_opts.update({'with_count': 'true'})
+
         # fake_key should be filtered for non-admin
         url = '/v2/fake/snapshots?fake_key=fake_value'
         for k, v in search_opts.items():
@@ -203,14 +207,21 @@ class ShareSnapshotAPITest(test.TestCase):
         req = fakes.HTTPRequest.blank(
             url, use_admin_context=use_admin_context, version=version)
 
+        method = 'get_all_snapshots'
         db_snapshots = [
             {'id': 'id1', 'display_name': 'n1', 'status': 'fake_status', },
             {'id': 'id2', 'display_name': 'n2', 'status': 'fake_status', },
             {'id': 'id3', 'display_name': 'n3', 'status': 'fake_status', },
         ]
-        snapshots = [db_snapshots[1]]
+
-        self.mock_object(share_api.API, 'get_all_snapshots',
+        mock_action = {'return_value': [db_snapshots[1]]}
-                         mock.Mock(return_value=snapshots))
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            method = 'get_all_snapshots_with_count'
+            mock_action = {'side_effect': [(1, [db_snapshots[1]])]}
+
+        mock_get_all_snapshots = (
+            self.mock_object(share_api.API, method, mock.Mock(**mock_action)))
 
         result = self.controller.index(req)
 
@@ -225,7 +236,8 @@ class ShareSnapshotAPITest(test.TestCase):
             search_opts_expected['display_name'] = search_opts['name']
         if use_admin_context:
             search_opts_expected.update({'fake_key': 'fake_value'})
-        share_api.API.get_all_snapshots.assert_called_once_with(
+
+        mock_get_all_snapshots.assert_called_once_with(
             req.environ['manila.context'],
             limit=int(search_opts['limit']),
             offset=int(search_opts['offset']),
@@ -234,14 +246,19 @@ class ShareSnapshotAPITest(test.TestCase):
             search_opts=search_opts_expected,
         )
         self.assertEqual(1, len(result['snapshots']))
-        self.assertEqual(snapshots[0]['id'], result['snapshots'][0]['id'])
+        self.assertEqual(db_snapshots[1]['id'], result['snapshots'][0]['id'])
         self.assertEqual(
-            snapshots[0]['display_name'], result['snapshots'][0]['name'])
+            db_snapshots[1]['display_name'], result['snapshots'][0]['name'])
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            self.assertEqual(1, result['count'])
 
     @ddt.data({'version': '2.35', 'use_admin_context': True},
               {'version': '2.36', 'use_admin_context': True},
+              {'version': '2.79', 'use_admin_context': True},
               {'version': '2.35', 'use_admin_context': False},
-              {'version': '2.36', 'use_admin_context': False})
+              {'version': '2.36', 'use_admin_context': False},
+              {'version': '2.79', 'use_admin_context': False})
     @ddt.unpack
     def test_snapshot_list_summary_with_search_opts(self, version,
                                                     use_admin_context):
@@ -288,14 +305,20 @@ class ShareSnapshotAPITest(test.TestCase):
         self.assertEqual(1, len(result['snapshots']))
         self.assertEqual(snapshots[0]['id'], result['snapshots'][0]['id'])
 
-    def _snapshot_list_detail_with_search_opts(self, use_admin_context):
+    def _snapshot_list_detail_with_search_opts(self, version,
+                                               use_admin_context):
         search_opts = fake_share.search_opts()
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            search_opts.update({'with_count': 'true'})
+
         # fake_key should be filtered for non-admin
         url = '/v2/fake/shares/detail?fake_key=fake_value'
         for k, v in search_opts.items():
             url = url + '&' + k + '=' + v
         req = fakes.HTTPRequest.blank(url, use_admin_context=use_admin_context)
 
+        method = 'get_all_snapshots'
         db_snapshots = [
             {
                 'id': 'id1',
@@ -317,10 +340,14 @@ class ShareSnapshotAPITest(test.TestCase):
                 'aggregate_status': 'fake_status',
             },
         ]
-        snapshots = [db_snapshots[1]]
+        mock_action = {'return_value': [db_snapshots[1]]}
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            method = 'get_all_snapshots_with_count'
+            mock_action = {'side_effect': [(1, [db_snapshots[1]])]}
 
-        self.mock_object(share_api.API, 'get_all_snapshots',
+        mock_get_all_snapshots = (
-                         mock.Mock(return_value=snapshots))
+            self.mock_object(share_api.API, method, mock.Mock(**mock_action)))
 
         result = self.controller.detail(req)
 
@@ -331,7 +358,7 @@ class ShareSnapshotAPITest(test.TestCase):
         }
         if use_admin_context:
             search_opts_expected.update({'fake_key': 'fake_value'})
-        share_api.API.get_all_snapshots.assert_called_once_with(
+        mock_get_all_snapshots.assert_called_once_with(
             req.environ['manila.context'],
             limit=int(search_opts['limit']),
             offset=int(search_opts['offset']),
@@ -340,19 +367,27 @@ class ShareSnapshotAPITest(test.TestCase):
             search_opts=search_opts_expected,
         )
         self.assertEqual(1, len(result['snapshots']))
-        self.assertEqual(snapshots[0]['id'], result['snapshots'][0]['id'])
+        self.assertEqual(db_snapshots[1]['id'], result['snapshots'][0]['id'])
         self.assertEqual(
-            snapshots[0]['display_name'], result['snapshots'][0]['name'])
+            db_snapshots[1]['display_name'], result['snapshots'][0]['name'])
         self.assertEqual(
-            snapshots[0]['aggregate_status'], result['snapshots'][0]['status'])
+            db_snapshots[1]['aggregate_status'],
+            result['snapshots'][0]['status'])
         self.assertEqual(
-            snapshots[0]['share_id'], result['snapshots'][0]['share_id'])
+            db_snapshots[1]['share_id'], result['snapshots'][0]['share_id'])
+        if (api_version.APIVersionRequest(version) >=
+                api_version.APIVersionRequest('2.79')):
+            self.assertEqual(1, result['count'])
 
-    def test_snapshot_list_detail_with_search_opts_by_non_admin(self):
+    @ddt.data({'version': '2.78', 'use_admin_context': True},
-        self._snapshot_list_detail_with_search_opts(use_admin_context=False)
+              {'version': '2.78', 'use_admin_context': False},
-
+              {'version': '2.79', 'use_admin_context': True},
-    def test_snapshot_list_detail_with_search_opts_by_admin(self):
+              {'version': '2.79', 'use_admin_context': False})
-        self._snapshot_list_detail_with_search_opts(use_admin_context=True)
+    @ddt.unpack
+    def test_snapshot_list_detail_with_search_opts(self, version,
+                                                   use_admin_context):
+        self._snapshot_list_detail_with_search_opts(
+            version=version, use_admin_context=use_admin_context)
 
     @ddt.data('2.0', '2.16', '2.17')
     def test_snapshot_list_detail(self, version):

manila/tests/db/sqlalchemy/test_api.py

@@ -1732,6 +1732,42 @@ class ShareSnapshotDatabaseAPITestCase(test.TestCase):
         self.assertEqual(1, len(actual_result.instances))
         self.assertSubDictMatch(values, actual_result.to_dict())
 
+    @ddt.data(
+        ({'with_count': True}, 3, 3),
+        ({'with_count': True, 'limit': 2}, 3, 2)
+    )
+    @ddt.unpack
+    def test_share_snapshot_get_all_with_count(self, filters,
+                                               amount_of_share_snapshots,
+                                               expected_share_snapshots_len):
+        share = db_utils.create_share(size=1)
+        values = {
+            'share_id': share['id'],
+            'size': share['size'],
+            'user_id': share['user_id'],
+            'project_id': share['project_id'],
+            'status': constants.STATUS_CREATING,
+            'progress': '0%',
+            'share_size': share['size'],
+            'display_description': 'fake_count_test',
+            'share_proto': share['share_proto'],
+        }
+
+        # consider only shares created in this function
+        filters.update({'share_id': share['id']})
+
+        for i in range(amount_of_share_snapshots):
+            tmp_values = copy.deepcopy(values)
+            tmp_values['display_name'] = 'fake_name_%s' % str(i)
+            db_api.share_snapshot_create(self.ctxt, tmp_values)
+
+        limit = filters.get('limit')
+        count, share_snapshots = db_api.share_snapshot_get_all_with_count(
+            self.ctxt, filters=filters, limit=limit)
+
+        self.assertEqual(count, amount_of_share_snapshots)
+        self.assertEqual(expected_share_snapshots_len, len(share_snapshots))
+
     def test_share_snapshot_get_all_with_filters_some(self):
         expected_status = constants.STATUS_AVAILABLE
         filters = {

releasenotes/notes/add-count-info-in-share-snapshot-eee90f1471f7a5c4.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    Added total count info in Manila's /snapshots and /snapshots/detail APIs.