From 96e049783b94841582f191856c03e62176d4164f Mon Sep 17 00:00:00 2001
From: Valeriy Ponomaryov <vponomaryov@mirantis.com>
Date: Fri, 3 Apr 2015 19:02:37 +0300
Subject: [PATCH] Fix fields 'deleted' in various DB models for PostgreSQL
 compatibility

Our migrations set field 'deleted' as Boolean for table
'share_type_extra_specs', but model 'ShareTypeExtraSpecs' treats it as Integer.
Same for several other models.
It works using MySQL as far it uses 'tinyint' storing `1` or `0` there. So, in
case of MySQL it is always Integer.
But PostgreSQL fails to write any records there because it distinguishes
Boolean from Integer. So, fix DB migrations, DB methods and models to make
mentioned field compatible for PostreSQL backend too.

Also, reuse func 'model_query' from oslo_db to remove errors with queries that
contain aggregate functions.

Change-Id: I0ca00ce86adad893ac786b10324a934d372e2a04
Closes-Bug: #1440089
---
 ...8e632621e5a_change_volume_type_to_share_type.py | 11 ++-
 .../59eb64046740_add_required_extra_spec.py        |  4 +-
 .../ef0c02b4366_add_share_type_projects.py         |  3 +-
 manila/db/sqlalchemy/api.py                        | 80 ++++++++++------------
 manila/db/sqlalchemy/models.py                     |  6 +-
 5 files changed, 52 insertions(+), 52 deletions(-)

Index: manila-2014.2/manila/db/sqlalchemy/api.py
===================================================================
--- manila-2014.2.orig/manila/db/sqlalchemy/api.py
+++ manila-2014.2/manila/db/sqlalchemy/api.py
@@ -27,6 +27,7 @@ from oslo.config import cfg
 from oslo.db import exception as db_exception
 from oslo.db import options as db_options
 from oslo.db.sqlalchemy import session
+from oslo.db.sqlalchemy import utils as db_utils
 import six
 from sqlalchemy import or_
 from sqlalchemy.orm import joinedload
@@ -174,6 +175,7 @@ def model_query(context, model, *args, *
     """Query helper that accounts for context's `read_deleted` field.
 
     :param context: context to query under
+    :param model: model to query. Must be a subclass of ModelBase.
     :param session: if present, the session to use
     :param read_deleted: if present, overrides context's read_deleted field.
     :param project_only: if present and context is user-type, then restrict
@@ -182,24 +184,17 @@ def model_query(context, model, *args, *
     session = kwargs.get('session') or get_session()
     read_deleted = kwargs.get('read_deleted') or context.read_deleted
     project_only = kwargs.get('project_only')
+    kwargs = dict()
 
-    query = session.query(model, *args)
+    if project_only and not context.is_admin:
+        kwargs['project_id'] = context.project_id
+    if read_deleted in ('no', 'n', False):
+        kwargs['deleted'] = False
+    elif read_deleted in ('yes', 'y', True):
+        kwargs['deleted'] = True
 
-    default_deleted_value = str(model.__mapper__.c.deleted.default.arg)
-    if read_deleted == 'no':
-        query = query.filter(model.deleted == default_deleted_value)
-    elif read_deleted == 'yes':
-        pass  # omit the filter to include deleted and active
-    elif read_deleted == 'only':
-        query = query.filter(model.deleted != default_deleted_value)
-    else:
-        raise Exception(_("Unrecognized read_deleted value '%s'")
-                        % read_deleted)
-
-    if project_only and is_user_context(context):
-        query = query.filter_by(project_id=context.project_id)
-
-    return query
+    return db_utils.model_query(
+        model=model, session=session, args=args, **kwargs)
 
 
 def exact_filter(query, model, filters, legal_keys):
@@ -983,7 +978,7 @@ def reservation_commit(context, reservat
             if reservation.delta >= 0:
                 usage.reserved -= reservation.delta
             usage.in_use += reservation.delta
-        reservation_query.update({'deleted': True,
+        reservation_query.update({'deleted': 1,
                                   'deleted_at': timeutils.utcnow(),
                                   'updated_at': literal_column('updated_at')},
                                  synchronize_session=False)
@@ -1000,7 +995,7 @@ def reservation_rollback(context, reserv
             usage = usages[reservation.resource]
             if reservation.delta >= 0:
                 usage.reserved -= reservation.delta
-        reservation_query.update({'deleted': True,
+        reservation_query.update({'deleted': 1,
                                   'deleted_at': timeutils.utcnow(),
                                   'updated_at': literal_column('updated_at')},
                                  synchronize_session=False)
@@ -1014,7 +1009,7 @@ def quota_destroy_all_by_project_and_use
                     read_deleted="no").\
             filter_by(project_id=project_id).\
             filter_by(user_id=user_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1023,7 +1018,7 @@ def quota_destroy_all_by_project_and_use
                     session=session, read_deleted="no").\
             filter_by(project_id=project_id).\
             filter_by(user_id=user_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1032,7 +1027,7 @@ def quota_destroy_all_by_project_and_use
                     session=session, read_deleted="no").\
             filter_by(project_id=project_id).\
             filter_by(user_id=user_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1045,7 +1040,7 @@ def quota_destroy_all_by_project(context
         model_query(context, models.Quota, session=session,
                     read_deleted="no").\
             filter_by(project_id=project_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1053,7 +1048,7 @@ def quota_destroy_all_by_project(context
         model_query(context, models.ProjectUserQuota, session=session,
                     read_deleted="no").\
             filter_by(project_id=project_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1061,7 +1056,7 @@ def quota_destroy_all_by_project(context
         model_query(context, models.QuotaUsage,
                     session=session, read_deleted="no").\
             filter_by(project_id=project_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1069,7 +1064,7 @@ def quota_destroy_all_by_project(context
         model_query(context, models.Reservation,
                     session=session, read_deleted="no").\
             filter_by(project_id=project_id).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')},
                    synchronize_session=False)
@@ -1089,7 +1084,7 @@ def reservation_expire(context):
                 reservation.usage.reserved -= reservation.delta
                 session.add(reservation.usage)
 
-        reservation_query.update({'deleted': True,
+        reservation_query.update({'deleted': 1,
                                   'deleted_at': timeutils.utcnow(),
                                   'updated_at': literal_column('updated_at')},
                                  synchronize_session=False)
@@ -1145,7 +1140,7 @@ def share_data_get_for_project(context,
     else:
         result = query.first()
 
-    return (result[1] or 0, result[2] or 0)
+    return (result[0] or 0, result[1] or 0)
 
 
 @require_context
@@ -1194,13 +1189,13 @@ def share_get_all_by_share_server(contex
 def share_delete(context, share_id):
     session = get_session()
     share_ref = share_get(context, share_id, session)
-    share_ref.update({'deleted': True,
+    share_ref.update({'deleted': 1,
                       'deleted_at': timeutils.utcnow(),
                       'updated_at': literal_column('updated_at'),
                       'status': 'deleted'})
     session.query(models.ShareMetadata).\
         filter_by(share_id=share_id).\
-        update({'deleted': True,
+        update({'deleted': 1,
                 'deleted_at': timeutils.utcnow(),
                 'updated_at': literal_column('updated_at')})
 
@@ -1263,7 +1258,7 @@ def share_access_delete(context, access_
     with session.begin():
         session.query(models.ShareAccessMapping).\
             filter_by(id=access_id).\
-            update({'deleted': True,
+            update({'deleted': access_id,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at'),
                     'state': models.ShareAccessMapping.STATE_DELETED})
@@ -1304,15 +1299,14 @@ def snapshot_data_get_for_project(contex
                         func.sum(models.ShareSnapshot.size),
                         read_deleted="no",
                         session=session).\
-        filter_by(project_id=project_id).\
-        options(joinedload('share'))
+        filter_by(project_id=project_id)
 
     if user_id:
         result = query.filter_by(user_id=user_id).first()
     else:
         result = query.first()
 
-    return (result[1] or 0, result[2] or 0)
+    return (result[0] or 0, result[1] or 0)
 
 
 @require_context
@@ -1322,7 +1316,7 @@ def share_snapshot_destroy(context, snap
         session.query(models.ShareSnapshot).\
             filter_by(id=snapshot_id).\
             update({'status': 'deleted',
-                    'deleted': True,
+                    'deleted': snapshot_id,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')})
 
@@ -1375,7 +1369,6 @@ def share_snapshot_data_get_for_project(
                          read_deleted="no",
                          session=session).\
         filter_by(project_id=project_id).\
-        options(joinedload('share')).\
         first()
 
     # NOTE(vish): convert None to 0
@@ -1407,7 +1400,7 @@ def share_metadata_get(context, share_id
 def share_metadata_delete(context, share_id, key):
     _share_metadata_get_query(context, share_id).\
         filter_by(key=key).\
-        update({'deleted': True,
+        update({'deleted': 1,
                 'deleted_at': timeutils.utcnow(),
                 'updated_at': literal_column('updated_at')})
 
@@ -1453,7 +1446,7 @@ def _share_metadata_update(context, shar
                     meta_ref = _share_metadata_get_item(context, share_id,
                                                         meta_key,
                                                         session=session)
-                    meta_ref.update({'deleted': True})
+                    meta_ref.update({'deleted': 1})
                     meta_ref.save(session=session)
 
         meta_ref = None
@@ -1627,7 +1620,7 @@ def share_network_get_all_by_security_se
         join(models.ShareNetworkSecurityServiceAssociation,
              models.ShareNetwork.id ==
              models.ShareNetworkSecurityServiceAssociation.share_network_id).\
-        filter_by(security_service_id=share_network_id, deleted=False).\
+        filter_by(security_service_id=share_network_id, deleted=0).\
         options(joinedload('share_servers')).all()
 
 
@@ -1984,7 +1977,7 @@ def volume_type_destroy(context, id):
             raise exception.VolumeTypeInUse(volume_type_id=id)
         model_query(context, models.VolumeTypeExtraSpecs, session=session).\
             filter_by(volume_type_id=id).update(
-                {'deleted': True,
+                {'deleted': 1,
                  'deleted_at': timeutils.utcnow(),
                  'updated_at': literal_column('updated_at')})
         model_query(context, models.VolumeTypes, session=session).\
@@ -2040,7 +2033,7 @@ def volume_type_extra_specs_delete(conte
                                           session)
         _volume_type_extra_specs_query(context, volume_type_id, session).\
             filter_by(key=key).\
-            update({'deleted': True,
+            update({'deleted': 1,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')})
 
Index: manila-2014.2/manila/db/sqlalchemy/models.py
===================================================================
--- manila-2014.2.orig/manila/db/sqlalchemy/models.py
+++ manila-2014.2/manila/db/sqlalchemy/models.py
@@ -231,7 +231,7 @@ class VolumeTypeExtraSpecs(BASE, ManilaB
         foreign_keys=volume_type_id,
         primaryjoin='and_('
         'VolumeTypeExtraSpecs.volume_type_id == VolumeTypes.id,'
-        'VolumeTypeExtraSpecs.deleted == False)'
+        'VolumeTypeExtraSpecs.deleted == 0)'
     )
 
 
