Optimize: add build_query_param method to clean code

Currently the client build query params in respective method
this patch intends to use 'utils.build_query_param' to make
the code clean.

Change-Id: I3a3ae90cc6011d1aa0cc39db4329d9bc08801904

cinderclient/tests/unit/test_utils.py

@@ -146,6 +146,46 @@ class CaptureStdout(object):
         self.read = self.stringio.read
 
 
+class BuildQueryParamTestCase(test_utils.TestCase):
+
+    def test_build_param_without_sort_switch(self):
+        dict_param = {
+            'key1': 'val1',
+            'key2': 'val2',
+            'key3': 'val3',
+        }
+        result = utils.build_query_param(dict_param, True)
+
+        self.assertIn('key1=val1', result)
+        self.assertIn('key2=val2', result)
+        self.assertIn('key3=val3', result)
+
+    def test_build_param_with_sort_switch(self):
+        dict_param = {
+            'key1': 'val1',
+            'key2': 'val2',
+            'key3': 'val3',
+        }
+        result = utils.build_query_param(dict_param, True)
+
+        expected = "?key1=val1&key2=val2&key3=val3"
+        self.assertEqual(expected, result)
+
+    def test_build_param_with_none(self):
+        dict_param = {
+            'key1': 'val1',
+            'key2': None,
+            'key3': False,
+            'key4': ''
+        }
+        result_1 = utils.build_query_param(dict_param)
+        result_2 = utils.build_query_param(None)
+
+        expected = "?key1=val1"
+        self.assertEqual(expected, result_1)
+        self.assertFalse(result_2)
+
+
 class PrintListTestCase(test_utils.TestCase):
 
     def test_print_list_with_list(self):

cinderclient/utils.py

@@ -22,6 +22,7 @@ import types
 import uuid
 
 import six
+from six.moves.urllib import parse
 import prettytable
 
 from cinderclient import exceptions
@@ -187,6 +188,24 @@ def unicode_key_value_to_string(dictionary):
                 for k, v in dictionary.items())
 
 
+def build_query_param(params, sort=False):
+    """parse list to url query parameters"""
+
+    if params is None:
+        params = {}
+    if not sort:
+        param_list = list(params.items())
+    else:
+        param_list = list(sorted(params.items()))
+
+    query_string = parse.urlencode(
+        [(k, v) for (k, v) in param_list if v])
+    if query_string:
+        query_string = "?%s" % (query_string,)
+
+    return query_string
+
+
 def print_dict(d, property="Property", formatters=None):
     pt = prettytable.PrettyTable([property, 'Value'], caching=False)
     pt.align = 'l'

cinderclient/v1/volume_snapshots.py

@@ -17,10 +17,8 @@
 Volume snapshot interface (1.1 extension).
 """
 
-import six
-from six.moves.urllib.parse import urlencode
-
 from cinderclient import base
+from cinderclient import utils
 
 
 class Snapshot(base.Resource):
@@ -109,23 +107,7 @@ class SnapshotManager(base.ManagerWithFind):
 
         :rtype: list of :class:`Snapshot`
         """
-
+        query_string = utils.build_query_param(search_opts, True)
-        if search_opts is None:
-            search_opts = {}
-
-        qparams = {}
-
-        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:

cinderclient/v1/volume_transfers.py

@@ -17,10 +17,8 @@
 Volume transfer interface (1.1 extension).
 """
 
-import six
-from six.moves.urllib.parse import urlencode
-
 from cinderclient import base
+from cinderclient import utils
 
 
 class VolumeTransfer(base.Resource):
@@ -73,16 +71,7 @@ class VolumeTransferManager(base.ManagerWithFind):
 
         :rtype: list of :class:`VolumeTransfer`
         """
-        if search_opts is None:
+        query_string = utils.build_query_param(search_opts)
-            search_opts = {}
-
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed:

cinderclient/v1/volumes.py

@@ -17,10 +17,8 @@
 Volume interface (1.1 extension).
 """
 
-import six
-from six.moves.urllib.parse import urlencode
-
 from cinderclient import base
+from cinderclient import utils
 
 
 class Volume(base.Resource):
@@ -202,22 +200,10 @@ class VolumeManager(base.ManagerWithFind):
         if search_opts is None:
             search_opts = {}
 
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
         if limit:
-            qparams['limit'] = limit
+            search_opts['limit'] = limit
 
-        # Transform the dict to a sequence of two-element tuples in fixed
+        query_string = utils.build_query_param(search_opts, True)
-        # 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:

cinderclient/v3/cgsnapshots.py

@@ -15,11 +15,10 @@
 
 """cgsnapshot interface (v3 extension)."""
 
-import six
-from six.moves.urllib.parse import urlencode
 
 from cinderclient.apiclient import base as common_base
 from cinderclient import base
+from cinderclient import utils
 
 
 class Cgsnapshot(base.Resource):
@@ -76,16 +75,7 @@ class CgsnapshotManager(base.ManagerWithFind):
 
         :rtype: list of :class:`Cgsnapshot`
         """
-        if search_opts is None:
+        query_string = utils.build_query_param(search_opts)
-            search_opts = {}
-
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed:

cinderclient/v3/consistencygroups.py

@@ -15,11 +15,9 @@
 
 """Consistencygroup interface (v3 extension)."""
 
-import six
-from six.moves.urllib.parse import urlencode
-
 from cinderclient.apiclient import base as common_base
 from cinderclient import base
+from cinderclient import utils
 
 
 class Consistencygroup(base.Resource):
@@ -107,16 +105,8 @@ class ConsistencygroupManager(base.ManagerWithFind):
 
         :rtype: list of :class:`Consistencygroup`
         """
-        if search_opts is None:
-            search_opts = {}
 
-        qparams = {}
+        query_string = utils.build_query_param(search_opts)
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed:

cinderclient/v3/group_snapshots.py

@@ -15,11 +15,10 @@
 
 """group snapshot interface (v3)."""
 
-import six
-from six.moves.urllib.parse import urlencode
 
 from cinderclient.apiclient import base as common_base
 from cinderclient import base
+from cinderclient import utils
 
 
 class GroupSnapshot(base.Resource):
@@ -82,16 +81,7 @@ class GroupSnapshotManager(base.ManagerWithFind):
         :param search_opts: search options
         :rtype: list of :class:`GroupSnapshot`
         """
-        if search_opts is None:
+        query_string = utils.build_query_param(search_opts)
-            search_opts = {}
-
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed:

cinderclient/v3/groups.py

@@ -15,11 +15,9 @@
 
 """Group interface (v3 extension)."""
 
-import six
-from six.moves.urllib.parse import urlencode
-
-from cinderclient.apiclient import base as common_base
 from cinderclient import base
+from cinderclient.apiclient import base as common_base
+from cinderclient import utils
 
 
 class Group(base.Resource):
@@ -107,16 +105,7 @@ class GroupManager(base.ManagerWithFind):
 
         :rtype: list of :class:`Group`
         """
-        if search_opts is None:
+        query_string = utils.build_query_param(search_opts)
-            search_opts = {}
-
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed:

cinderclient/v3/volume_transfers.py

@@ -17,10 +17,8 @@
 Volume transfer interface (v3 extension).
 """
 
-import six
-from six.moves.urllib.parse import urlencode
-
 from cinderclient import base
+from cinderclient import utils
 
 
 class VolumeTransfer(base.Resource):
@@ -73,16 +71,7 @@ class VolumeTransferManager(base.ManagerWithFind):
 
         :rtype: list of :class:`VolumeTransfer`
         """
-        if search_opts is None:
+        query_string = utils.build_query_param(search_opts)
-            search_opts = {}
-
-        qparams = {}
-
-        for opt, val in six.iteritems(search_opts):
-            if val:
-                qparams[opt] = val
-
-        query_string = "?%s" % urlencode(qparams) if qparams else ""
 
         detail = ""
         if detailed: