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

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,

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)
+
+
 ###################
 
 

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)
 

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()

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):