Add datastore filter to backup-list

This fix enhances the backup-list command to optionally receive
a datastore name or ID to filter the backup list by. The filter
is sent as a query string.

To attach the query string to the URL and have it still work with
the URL for pagination, i have made some changes in the way url
with query strings are constructed. This includes the pagination
URL.

partially implements: blueprint backup-metadata

Change-Id: I0b9ef3ec7f51ed76517a22f9c0edfdce3694a36f

7 files changed, 65 insertions, 23 deletions

diff --git a/troveclient/base.py b/troveclient/base.py
index cc005d4..6ab817b 100644
--- a/troveclient/base.py
+++ b/troveclient/base.py
@@ -62,8 +62,12 @@ class Manager(utils.HookableMixin):
     def __init__(self, api):
         self.api = api
 
-    def _paginated(self, url, response_key, limit=None, marker=None):
-        resp, body = self.api.client.get(common.limit_url(url, limit, marker))
+    def _paginated(self, url, response_key, limit=None, marker=None,
+                   query_strings=None):
+        query_strings = query_strings or {}
+        url = common.append_query_strings(url, limit=limit, marker=marker,
+                                          **query_strings)
+        resp, body = self.api.client.get(url)
         if not body:
             raise Exception("Call to " + url + " did not return a body.")
         links = body.get('links', [])
diff --git a/troveclient/common.py b/troveclient/common.py
index f413f94..8313351 100644
--- a/troveclient/common.py
+++ b/troveclient/common.py
@@ -23,16 +23,12 @@ def check_for_exceptions(resp, body, url):
         raise exceptions.from_response(resp, body, url)
 
 
-def limit_url(url, limit=None, marker=None):
-    if not limit and not marker:
+def append_query_strings(url, **query_strings):
+    if not query_strings:
         return url
-    query = []
-    if marker:
-        query.append("marker=%s" % marker)
-    if limit:
-        query.append("limit=%s" % limit)
-    query = '?' + '&'.join(query)
-    return url + query
+    query = '&'.join('{0}={1}'.format(key, val)
+                     for key, val in query_strings.items() if val)
+    return url + ('?' + query if query else "")
 
 
 def quote_user_host(user, host):
diff --git a/troveclient/tests/test_backups.py b/troveclient/tests/test_backups.py
index 791c6d4..7efba5f 100644
--- a/troveclient/tests/test_backups.py
+++ b/troveclient/tests/test_backups.py
@@ -80,7 +80,17 @@ class BackupManagerTest(testtools.TestCase):
         limit = "test-limit"
         marker = "test-marker"
         self.backups.list(limit, marker)
-        page_mock.assert_called_with("/backups", "backups", limit, marker)
+        page_mock.assert_called_with("/backups", "backups", limit, marker, {})
+
+    def test_list_by_datastore(self):
+        page_mock = mock.Mock()
+        self.backups._paginated = page_mock
+        limit = "test-limit"
+        marker = "test-marker"
+        datastore = "test-mysql"
+        self.backups.list(limit, marker, datastore)
+        page_mock.assert_called_with("/backups", "backups", limit, marker,
+                                     {'datastore': datastore})
 
     def test_get(self):
         get_mock = mock.Mock()
diff --git a/troveclient/tests/test_base.py b/troveclient/tests/test_base.py
index d56cbbc..223aa3f 100644
--- a/troveclient/tests/test_base.py
+++ b/troveclient/tests/test_base.py
@@ -279,9 +279,14 @@ class MangerPaginationTests(ManagerTest):
 
         def side_effect(url):
             if url == self.url:
-                return (None, self.body)
-            if url == self.next_url:
-                return (None, self.next_body)
+                return None, self.body
+            # In python 3 the order in the dictionary is not constant
+            # between runs. So we cant rely on the URL params to be
+            # in the same order
+            if ('marker=%s' % self.marker in url and
+                    'limit=%s' % self.limit in url):
+                self.next_url = url
+                return None, self.next_body
 
         self.manager.api.client.get = mock.Mock(side_effect=side_effect)
 
diff --git a/troveclient/tests/test_common.py b/troveclient/tests/test_common.py
index 55e8645..7739eff 100644
--- a/troveclient/tests/test_common.py
+++ b/troveclient/tests/test_common.py
@@ -31,15 +31,34 @@ class CommonTest(testtools.TestCase):
             self.assertRaises(Exception,
                               common.check_for_exceptions, resp, "body")
 
-    def test_limit_url(self):
+    def test_append_query_strings(self):
         url = "test-url"
-        self.assertEqual(url, common.limit_url(url, limit=None, marker=None))
+        self.assertEqual(url, common.append_query_strings(url))
+
+        self.assertEqual(url, common.append_query_strings(
+            url, limit=None, marker=None))
 
         limit = "test-limit"
         marker = "test-marker"
-        expected = "test-url?marker=test-marker&limit=test-limit"
-        self.assertEqual(expected,
-                         common.limit_url(url, limit=limit, marker=marker))
+        result = common.append_query_strings(url, limit=limit, marker=marker)
+        self.assertTrue(result.startswith(url + '?'))
+        self.assertIn("limit=%s" % limit, result)
+        self.assertIn("marker=%s" % marker, result)
+        self.assertEqual(result.count('&'), 1)
+
+        opts = {}
+        self.assertEqual(url, common.append_query_strings(
+            url, limit=None, marker=None, **opts))
+
+        opts = {'key1': 'value1', 'key2': None}
+        result = common.append_query_strings(url, limit=None, marker=marker,
+                                             **opts)
+        self.assertTrue(result.startswith(url + '?'))
+        self.assertEqual(result.count('&'), 1)
+        self.assertNotIn("limit=%s" % limit, result)
+        self.assertIn("marker=%s" % marker, result)
+        self.assertIn("key1=%s" % opts['key1'], result)
+        self.assertNotIn("key2=%s" % opts['key2'], result)
 
 
 class PaginatedTest(testtools.TestCase):
diff --git a/troveclient/v1/backups.py b/troveclient/v1/backups.py
index bee134e..2128bbb 100644
--- a/troveclient/v1/backups.py
+++ b/troveclient/v1/backups.py
@@ -38,12 +38,17 @@ class Backups(base.ManagerWithFind):
         return self._get("/backups/%s" % base.getid(backup),
                          "backup")
 
-    def list(self, limit=None, marker=None):
+    def list(self, limit=None, marker=None, datastore=None):
         """Get a list of all backups.
 
         :rtype: list of :class:`Backups`.
         """
-        return self._paginated("/backups", "backups", limit, marker)
+        query_strings = {}
+        if datastore:
+            query_strings = {'datastore': datastore}
+
+        return self._paginated("/backups", "backups", limit, marker,
+                               query_strings)
 
     def create(self, name, instance, description=None, parent_id=None):
         """Create a new backup from the given instance."""
diff --git a/troveclient/v1/shell.py b/troveclient/v1/shell.py
index 75f9127..0c13fb8 100644
--- a/troveclient/v1/shell.py
+++ b/troveclient/v1/shell.py
@@ -332,10 +332,13 @@ def do_backup_list_instance(cs, args):
 @utils.arg('--limit', metavar='<limit>',
            default=None,
            help='Return up to N number of the most recent backups.')
+@utils.arg('--datastore', metavar='<datastore>',
+           default=None,
+           help='Name or ID of the datastore to list backups for.')
 @utils.service_type('database')
 def do_backup_list(cs, args):
     """Lists available backups."""
-    wrapper = cs.backups.list(limit=args.limit)
+    wrapper = cs.backups.list(limit=args.limit, datastore=args.datastore)
     backups = wrapper.items
     while wrapper.next and not args.limit:
         wrapper = cs.backups.list(marker=wrapper.next)