From 0b8b9f2de7270a74fb54d40122a9ed397b6360c7 Mon Sep 17 00:00:00 2001
From: Anton Arefiev <aarefiev@mirantis.com>
Date: Wed, 6 May 2015 14:54:15 +0300
Subject: [PATCH] Add findall server side filtering

Findall method in cinderclient/base.py isn't very efficient: it loads
the entire list then filters on the Python side. When calling "cinder
show 'volName'" on a tenant holding a lot of volumes (> 1000) the run
time is too long.

On my env show command with 10000 volumes takes ~2 min, after apply
this patch few seconds.

Closes-Bug: #1449444
Change-Id: I23070d94d5bb100b2dd8677f67d7c8b1e7d34e52
---
 cinderclient/base.py                     | 27 ++++++++++++++++++------
 cinderclient/tests/unit/v1/test_shell.py |  3 ++-
 cinderclient/tests/unit/v2/test_shell.py |  3 ++-
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/cinderclient/base.py b/cinderclient/base.py
index f2ed7b85f..c6a22b294 100644
--- a/cinderclient/base.py
+++ b/cinderclient/base.py
@@ -210,8 +210,8 @@ class ManagerWithFind(six.with_metaclass(abc.ABCMeta, Manager)):
         """
         Find a single item with attributes matching ``**kwargs``.
 
-        This isn't very efficient: it loads the entire list then filters on
-        the Python side.
+        This isn't very efficient for search options which require the
+        Python side filtering(e.g. 'human_id')
         """
         matches = self.findall(**kwargs)
         num_matches = len(matches)
@@ -227,16 +227,29 @@ class ManagerWithFind(six.with_metaclass(abc.ABCMeta, Manager)):
         """
         Find all items with attributes matching ``**kwargs``.
 
-        This isn't very efficient: it loads the entire list then filters on
-        the Python side.
+        This isn't very efficient for search options which require the
+        Python side filtering(e.g. 'human_id')
         """
-        found = []
-        searches = list(kwargs.items())
 
         # Want to search for all tenants here so that when attempting to delete
         # that a user like admin doesn't get a failure when trying to delete
         # another tenant's volume by name.
-        for obj in self.list(search_opts={'all_tenants': 1}):
+        search_opts = {'all_tenants': 1}
+
+        # Pass 'name' or 'display_name' search_opts to server filtering to
+        # increase search performance.
+        if 'name' in kwargs:
+            search_opts['name'] = kwargs['name']
+        elif 'display_name' in kwargs:
+            search_opts['display_name'] = kwargs['display_name']
+
+        found = []
+        searches = kwargs.items()
+
+        # Not all resources attributes support filters on server side
+        # (e.g. 'human_id' doesn't), so when doing findall some client
+        # side filtering is still needed.
+        for obj in self.list(search_opts=search_opts):
             try:
                 if all(getattr(obj, attr) == value
                        for (attr, value) in searches):
diff --git a/cinderclient/tests/unit/v1/test_shell.py b/cinderclient/tests/unit/v1/test_shell.py
index 44e7b09b9..d0dbc5c15 100644
--- a/cinderclient/tests/unit/v1/test_shell.py
+++ b/cinderclient/tests/unit/v1/test_shell.py
@@ -234,7 +234,8 @@ class ShellTest(utils.TestCase):
 
     def test_delete_by_name(self):
         self.run_command('delete sample-volume')
-        self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1')
+        self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1&'
+                                          'display_name=sample-volume')
         self.assert_called('DELETE', '/volumes/1234')
 
     def test_delete_multiple(self):
diff --git a/cinderclient/tests/unit/v2/test_shell.py b/cinderclient/tests/unit/v2/test_shell.py
index f9ee58ec0..07f44ebbc 100644
--- a/cinderclient/tests/unit/v2/test_shell.py
+++ b/cinderclient/tests/unit/v2/test_shell.py
@@ -344,7 +344,8 @@ class ShellTest(utils.TestCase):
 
     def test_delete_by_name(self):
         self.run_command('delete sample-volume')
-        self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1')
+        self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1&'
+                                          'name=sample-volume')
         self.assert_called('DELETE', '/volumes/1234')
 
     def test_delete_multiple(self):