Use of ast for integers doesn't changes type

ast.literal_eval doesn't change types for Integer
Booleans resulting in DBAPIError exceptions with
at least postgresql. Boolean columns need to be
explicitly cast to boolean types. This changes
the way filters are processed in the volume API.

APIImpact
Change-Id: Ice9c57fa99c17e4c87de6362bf30ed30503a1bed
Closes-Bug: #1494475

diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py
index a73b65a0e9155bc4b04ad82075dddc83a2c11d95..eec160bfff10896324731b25d8a5aa3d557ca49a 100644 (file)
--- a/cinder/api/v2/volumes.py
+++ b/cinder/api/v2/volumes.py
@@ -16,8 +16,6 @@
 """The volumes api."""
 
 
-import ast
-
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import uuidutils
@@ -243,12 +241,7 @@ class VolumeController(wsgi.Controller):
             filters['display_name'] = filters['name']
             del filters['name']
 
-        for k, v in filters.items():
-            try:
-                filters[k] = ast.literal_eval(v)
-            except (ValueError, SyntaxError):
-                LOG.debug('Could not evaluate value %s, assuming string', v)
-
+        self.volume_api.check_volume_filters(filters)
         volumes = self.volume_api.get_all(context, marker, limit,
                                           sort_keys=sort_keys,
                                           sort_dirs=sort_dirs,

diff --git a/cinder/db/api.py b/cinder/db/api.py
index 5fe56e4435942fbf0be7f28bc669a7f1c106c42f..9472c59dc21f91b92c908efcd4670f26b19238ec 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -1020,6 +1020,10 @@ def purge_deleted_rows(context, age_in_days):
     return IMPL.purge_deleted_rows(context, age_in_days=age_in_days)
 
 
+def get_booleans_for_table(table_name):
+    return IMPL.get_booleans_for_table(table_name)
+
+
 ###################
 
 

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 622b7eb0d320f22109dd98131a0338f394c01c7d..148c993f705c4101a8e963a6bdd0bd8b1e39e357 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -46,6 +46,7 @@ from sqlalchemy.sql.expression import desc
 from sqlalchemy.sql.expression import literal_column
 from sqlalchemy.sql.expression import true
 from sqlalchemy.sql import func
+from sqlalchemy.sql import sqltypes
 
 from cinder.api import common
 from cinder.common import sqlalchemyutils
@@ -1104,6 +1105,18 @@ def volume_create(context, values):
     return _volume_get(context, values['id'], session=session)
 
 
+def get_booleans_for_table(table_name):
+    booleans = set()
+    table = getattr(models, table_name.capitalize())
+    if hasattr(table, '__table__'):
+        columns = table.__table__.columns
+        for column in columns:
+            if isinstance(column.type, sqltypes.Boolean):
+                booleans.add(column.name)
+
+    return booleans
+
+
 @require_admin_context
 def volume_data_get_for_host(context, host, count_only=False):
     host_attr = models.Volume.host
@@ -3180,8 +3193,8 @@ def volume_type_encryption_update(context, volume_type_id, values):
                                                 session)
 
         if not encryption:
-            raise exception.VolumeTypeEncryptionNotFound(type_id=
-                                                         volume_type_id)
+            raise exception.VolumeTypeEncryptionNotFound(
+                type_id=volume_type_id)
 
         encryption.update(values)
 

diff --git a/cinder/tests/unit/api/v2/test_volumes.py b/cinder/tests/unit/api/v2/test_volumes.py
index 73697464c53b2b6f8ea0548f89357f4230bfd1f3..546602afde1ea09c0df907589fae3a7818de62a9 100644 (file)
--- a/cinder/tests/unit/api/v2/test_volumes.py
+++ b/cinder/tests/unit/api/v2/test_volumes.py
@@ -1306,6 +1306,34 @@ class VolumeApiTest(test.TestCase):
             filters={'display_name': 'Volume-573108026'},
             viewable_admin_meta=True, offset=0)
 
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_true(self, get_all):
+        req = mock.MagicMock()
+        context = mock.Mock()
+        req.environ = {'cinder.context': context}
+        req.params = {'display_name': 'Volume-573108026', 'bootable': 1}
+        self.controller._view_builder.detail_list = mock.Mock()
+        self.controller._get_volumes(req, True)
+        get_all.assert_called_once_with(
+            context, None, CONF.osapi_max_limit,
+            sort_keys=['created_at'], sort_dirs=['desc'],
+            filters={'display_name': 'Volume-573108026', 'bootable': True},
+            viewable_admin_meta=True, offset=0)
+
+    @mock.patch('cinder.volume.api.API.get_all')
+    def test_get_volumes_filter_with_false(self, get_all):
+        req = mock.MagicMock()
+        context = mock.Mock()
+        req.environ = {'cinder.context': context}
+        req.params = {'display_name': 'Volume-573108026', 'bootable': 0}
+        self.controller._view_builder.detail_list = mock.Mock()
+        self.controller._get_volumes(req, True)
+        get_all.assert_called_once_with(
+            context, None, CONF.osapi_max_limit,
+            sort_keys=['created_at'], sort_dirs=['desc'],
+            filters={'display_name': 'Volume-573108026', 'bootable': False},
+            viewable_admin_meta=True, offset=0)
+
     @mock.patch('cinder.volume.api.API.get_all')
     def test_get_volumes_filter_with_list(self, get_all):
         req = mock.MagicMock()

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 03f1d74fae2d75295fc02e209fb01173c542e322..2fd8db425647078944566479617b587d0eef8979 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -16,7 +16,7 @@
 
 """Handles all requests relating to volumes."""
 
-
+import ast
 import collections
 import datetime
 import functools
@@ -1691,6 +1691,17 @@ class API(base.Base):
         # level to show an admin how a backend is configured.
         return self.volume_rpcapi.list_replication_targets(ctxt, volume)
 
+    def check_volume_filters(self, filters):
+        booleans = self.db.get_booleans_for_table('volume')
+        for k, v in filters.iteritems():
+            try:
+                if k in booleans:
+                    filters[k] = bool(v)
+                else:
+                    filters[k] = ast.literal_eval(v)
+            except (ValueError, SyntaxError):
+                LOG.debug('Could not evaluate value %s, assuming string', v)
+
 
 class HostAPI(base.Base):
     def __init__(self):