From a903b93124bcc5301d4cd2bcaedda50345308c86 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Thu, 26 Jan 2017 12:51:17 +0100
Subject: [PATCH] Refactor volumes summary

Current volumes summary code has unnecessary code duplication as well as
a variable name that doesn't follow the project guidelines.

The variable in question is `allTenants` that should have been named
`all_tenants`.

This patch removes the code duplication by merging the two DB and OVO
methods into one.

TrivialFix

Change-Id: Ia80568147462450a707bc316100e5289c4b02eed
---
 cinder/db/api.py            | 11 +++--------
 cinder/db/sqlalchemy/api.py | 31 ++++++++-----------------------
 cinder/objects/volume.py    |  9 ++-------
 cinder/volume/api.py        | 12 ++++--------
 4 files changed, 17 insertions(+), 46 deletions(-)

diff --git a/cinder/db/api.py b/cinder/db/api.py
index e136363e96d..0a1c85fd91c 100644
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -294,14 +294,9 @@ def volume_get_all_by_project(context, project_id, marker, limit,
                                           offset=offset)
 
 
-def get_volume_summary_all(context):
-    """Get all volume summary."""
-    return IMPL.get_volume_summary_all(context)
-
-
-def get_volume_summary_by_project(context, project_id):
-    """Get all volume summary belonging to a project."""
-    return IMPL.get_volume_summary_by_project(context, project_id)
+def get_volume_summary(context, project_only):
+    """Get volume summary."""
+    return IMPL.get_volume_summary(context, project_only)
 
 
 def volume_update(context, volume_id, values):
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 9cd5b0fa2e4..901115800f2 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2029,15 +2029,19 @@ def volume_get_all(context, marker=None, limit=None, sort_keys=None,
         return query.all()
 
 
-@require_admin_context
-def get_volume_summary_all(context):
+@require_context
+def get_volume_summary(context, project_only):
     """Retrieves all volumes summary.
 
     :param context: context to query under
-    :returns: volume summary of all projects
+    :param project_only: limit summary to project volumes
+    :returns: volume summary
     """
+    if not (project_only or is_admin_context(context)):
+        raise exception.AdminRequired()
     query = model_query(context, func.count(models.Volume.id),
-                        func.sum(models.Volume.size), read_deleted="no")
+                        func.sum(models.Volume.size), read_deleted="no",
+                        project_only=project_only)
 
     if query is None:
         return []
@@ -2378,25 +2382,6 @@ def process_sort_params(sort_keys, sort_dirs, default_keys=None,
     return result_keys, result_dirs
 
 
-@require_context
-def get_volume_summary_by_project(context, project_id):
-    """Retrieves all volumes summary in a project.
-
-    :param context: context to query under
-    :param project_id: project for all volumes being retrieved
-    :returns: volume summary of a project
-    """
-    query = model_query(context, func.count(models.Volume.id),
-                        func.sum(models.Volume.size), read_deleted="no").\
-        filter_by(project_id=project_id)
-
-    if query is None:
-        return []
-
-    result = query.first()
-    return (result[0] or 0, result[1] or 0)
-
-
 @handle_db_data_error
 @require_context
 def volume_update(context, volume_id, values):
diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py
index c8b4e2f379c..a956ef0890a 100644
--- a/cinder/objects/volume.py
+++ b/cinder/objects/volume.py
@@ -625,13 +625,8 @@ class VolumeList(base.ObjectListBase, base.CinderObject):
                                   volumes, expected_attrs=expected_attrs)
 
     @classmethod
-    def get_volume_summary_all(cls, context):
-        volumes = db.get_volume_summary_all(context)
-        return volumes
-
-    @classmethod
-    def get_volume_summary_by_project(cls, context, project_id):
-        volumes = db.get_volume_summary_by_project(context, project_id)
+    def get_volume_summary(cls, context, project_only):
+        volumes = db.get_volume_summary(context, project_only)
         return volumes
 
     @classmethod
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 75931ac2631..cb3b26884f0 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -574,14 +574,10 @@ class API(base.Base):
         if filters is None:
             filters = {}
 
-        allTenants = utils.get_bool_param('all_tenants', filters)
-
-        if context.is_admin and allTenants:
-            del filters['all_tenants']
-            volumes = objects.VolumeList.get_volume_summary_all(context)
-        else:
-            volumes = objects.VolumeList.get_volume_summary_by_project(
-                context, context.project_id)
+        all_tenants = utils.get_bool_param('all_tenants', filters)
+        filters.pop('all_tenants', None)
+        project_only = not (all_tenants and context.is_admin)
+        volumes = objects.VolumeList.get_volume_summary(context, project_only)
 
         LOG.info(_LI("Get summary completed successfully."))
         return volumes