Check schema when dropping constraints.

MySQL constraints are not named like others.

Previous to this patch, we were looking up the names in MySQL's
INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS table, but were
neglecting to also check the Schema. The same table
could be in multiple schemas; as an example a unit test
run on the same machine as devstack in two different schemas
but the same RDBMS.

This patch uses a better approach. Sqlalchemy knows about the
name of the constraint, it just doesn't use it in the
drop statement by default. The Constraint objects from
the main sqlalchemy package don't do drops, only those out
of the migrate package. This patch finds the name of the
constraint in the constraint bound to the table and passes it
to the migrate ForeignKeyConstraint to use in the drop statement

Bug 1186353

Change-Id: Ida2184021de9dd220a36507a8a625cf4210d17f7

keystone/common/sql/migrate_repo/versions/023_drop_credential_constraints.py

@@ -14,64 +14,36 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-from migrate import ForeignKeyConstraint
 import sqlalchemy
-from sqlalchemy.orm import sessionmaker
 
-MYSQL_FKEY_QUERY = ("select CONSTRAINT_NAME from "
-                    "INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS "
-                    "where table_name = 'credential'")
+from keystone.common.sql import migration_helpers
 
 
-def drop_constraint_mysql(migrate_engine):
-    session = sessionmaker(bind=migrate_engine)()
-    #http://bugs.mysql.com/bug.php?id=10333
-    #MySQL varies from the SQL norm in naming
-    #Foreign Keys.  The mapping from the column name
-    #to the actual foreign key is stored in
-    #INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS
-    #SQLAlchemy expects the constraint name to be
-    # the column name.
-    for constraint in session.execute(MYSQL_FKEY_QUERY):
-        session.execute('ALTER TABLE credential DROP FOREIGN KEY %s;'
-                        % constraint[0])
-    session.commit()
-
-
-def remove_constraints(migrate_engine):
-    if migrate_engine.name == 'sqlite':
-        return
-    if migrate_engine.name == 'mysql':
-        drop_constraint_mysql(migrate_engine)
-        return
+def list_constraints(migrate_engine):
     meta = sqlalchemy.MetaData()
     meta.bind = migrate_engine
     user_table = sqlalchemy.Table('user', meta, autoload=True)
     proj_table = sqlalchemy.Table('project', meta, autoload=True)
     cred_table = sqlalchemy.Table('credential', meta, autoload=True)
-    ForeignKeyConstraint(columns=[cred_table.c.user_id],
-                         refcolumns=[user_table.c.id]).drop()
-    ForeignKeyConstraint(columns=[cred_table.c.project_id],
-                         refcolumns=[proj_table.c.id]).drop()
 
-
-def add_constraints(migrate_engine):
-    if migrate_engine.name == 'sqlite':
-        return
-    meta = sqlalchemy.MetaData()
-    meta.bind = migrate_engine
-    user_table = sqlalchemy.Table('user', meta, autoload=True)
-    proj_table = sqlalchemy.Table('project', meta, autoload=True)
-    cred_table = sqlalchemy.Table('credential', meta, autoload=True)
-    ForeignKeyConstraint(columns=[cred_table.c.user_id],
-                         refcolumns=[user_table.c.id]).create()
-    ForeignKeyConstraint(columns=[cred_table.c.project_id],
-                         refcolumns=[proj_table.c.id]).create()
+    constraints = [{'table': cred_table,
+                    'fk_column': 'user_id',
+                    'ref_column': user_table.c.id},
+                   {'table': cred_table,
+                    'fk_column': 'project_id',
+                    'ref_column': proj_table.c.id}]
+    return constraints
 
 
 def upgrade(migrate_engine):
-    remove_constraints(migrate_engine)
+    # SQLite does not support constraints, and querying the constraints
+    # raises an exception
+    if migrate_engine.name == 'sqlite':
+        return
+    migration_helpers.remove_constraints(list_constraints(migrate_engine))
 
 
 def downgrade(migrate_engine):
-    add_constraints(migrate_engine)
+    if migrate_engine.name == 'sqlite':
+        return
+    migration_helpers.add_constraints(list_constraints(migrate_engine))

keystone/common/sql/migration_helpers.py

@@ -0,0 +1,58 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2013 OpenStack LLC
+# Copyright 2013 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+import migrate
+import sqlalchemy
+
+
+#  Different RDBMSs use different schemes for naming the Foreign Key
+#  Constraints.  SQLAlchemy does not yet attempt to determine the name
+#  for the constraint, and instead attempts to deduce it from the column.
+#  This fails on MySQL.
+def get_fkey_constraint_name(table, column_name):
+    fkeys = [fk for fk in table.constraints
+             if (column_name in fk.columns and
+                 isinstance(fk, sqlalchemy.ForeignKeyConstraint))]
+    constraint_name = fkeys[0].name
+    return constraint_name
+
+
+#  remove_constraints and add_constraints both accept a list of dictionaries
+#  that contain:
+#  {'table': a sqlalchemy table. The constraint is added to to dropped from
+#           this table.
+#  'fk_column': the name of a column on the above table,  The constraint
+#               is added to or dropped from this column
+#  'ref_column':a sqlalchemy column object.  This is the reference column
+#               for the constraint.
+def remove_constraints(constraints):
+    for constraint_def in constraints:
+        migrate.ForeignKeyConstraint(
+            columns=[getattr(constraint_def['table'].c,
+                             constraint_def['fk_column'])],
+            refcolumns=[constraint_def['ref_column']],
+            name=(get_fkey_constraint_name
+                  (constraint_def['table'],
+                   constraint_def['fk_column']))).drop()
+
+
+def add_constraints(constraints):
+    for constraint_def in constraints:
+        migrate.ForeignKeyConstraint(
+            columns=[getattr(constraint_def['table'].c,
+                             constraint_def['fk_column'])],
+            refcolumns=[constraint_def['ref_column']]).create()