Merge "Fixup region description uniqueness"

keystone/common/sql/migrate_repo/versions/037_add_region_table.py

@@ -23,8 +23,7 @@ def upgrade(migrate_engine):
         'region',
         meta,
         sql.Column('id', sql.String(64), primary_key=True),
-        sql.Column('description', sql.String(255), unique=True,
-                   nullable=False),
+        sql.Column('description', sql.String(255), nullable=False),
         sql.Column('parent_region_id', sql.String(64), nullable=True),
         sql.Column('extra', sql.Text()))
     region_table.create(migrate_engine, checkfirst=True)

keystone/common/sql/migrate_repo/versions/043_fixup_region_description.py

@@ -0,0 +1,90 @@
+# Copyright 2014 IBM Corp.
+#
+# 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.
+
+
+"""Relax the uniqueness of `description` column in region table.
+
+The region table has a dedicated column for the region `description`. This
+column originally was not nullable and had to be unique. So if a user wanted
+to create a region without sending a `description` in the request, they would
+experience an SQL error because the `description` column can't be null for a
+region. This means that every region had to have a unique description.
+
+To upgrade, we are going to transfer all the data from the existing region
+table to a temporary table, drop the original region table, and then finally
+rename the temporary table to the correct name.
+
+There is no downgrade path as the original migration has been fixed to not
+include the unique constraint on description column.
+
+"""
+
+import migrate
+import sqlalchemy as sql
+from sqlalchemy.orm import sessionmaker
+
+
+_TEMP_REGION_TABLE_NAME = 'temp_region'
+_REGION_TABLE_NAME = 'region'
+
+
+def _migrate_to_new_region_table(meta, migrate_engine, region_table):
+    # Create a temporary region table to hold data while we recreate the
+    # new region table without a unique constraint on the description column
+
+    session = sessionmaker(bind=migrate_engine)()
+
+    temp_region_table = sql.Table(
+        _TEMP_REGION_TABLE_NAME,
+        meta,
+        sql.Column('id', sql.String(64), primary_key=True),
+        sql.Column('description', sql.String(255), nullable=False),
+        sql.Column('parent_region_id', sql.String(64), nullable=True),
+        sql.Column('extra', sql.Text()))
+    temp_region_table.create(migrate_engine, checkfirst=True)
+
+    # Migrate the data
+    for region in list(session.query(region_table)):
+        session.execute(temp_region_table.insert().values(
+            id=region.id,
+            description=region.description,
+            parent_region_id=region.parent_region_id,
+            extra=region.extra))
+
+    session.commit()
+    session.close()
+
+    # Drop the old region table
+    region_table.drop(checkfirst=True)
+    migrate.rename_table(temp_region_table, _REGION_TABLE_NAME, meta.bind)
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+
+    region_table = sql.Table(_REGION_TABLE_NAME, meta, autoload=True)
+    for idx in region_table.indexes:
+        if ((idx.columns.get('description') == region_table.c.description) and
+                len(idx.columns.values()) is 1):
+            # Constraint was found, do the migration.
+            _migrate_to_new_region_table(meta, migrate_engine, region_table)
+            break
+
+
+def downgrade(migrate_engine):
+    # There is no downgrade option. The unique constraint should not have
+    # existed and therefore does not need to be re-added. The previous
+    # migration has been modified to not contain the unique constraint.
+    pass

keystone/tests/test_sql_upgrade.py

@@ -35,6 +35,7 @@ import uuid
 
 from migrate.versioning import api as versioning_api
 import sqlalchemy
+import sqlalchemy.exc
 
 from keystone.assignment.backends import sql as assignment_sql
 from keystone.common import sql
@@ -595,9 +596,13 @@ class SqlUpgradeTests(SqlMigrateBase):
         session.commit()
         session.close()
 
-    def insert_dict(self, session, table_name, d):
+    def insert_dict(self, session, table_name, d, table=None):
         """Naively inserts key-value pairs into a table, given a dictionary."""
-        this_table = sqlalchemy.Table(table_name, self.metadata, autoload=True)
+        if table is None:
+            this_table = sqlalchemy.Table(table_name, self.metadata,
+                                          autoload=True)
+        else:
+            this_table = table
         insert = this_table.insert()
         insert.execute(d)
         session.commit()
@@ -2289,6 +2294,95 @@ class SqlUpgradeTests(SqlMigrateBase):
             extra = fetch_endpoint(endpoint_id)
             self.assertEqual(exp_extra, extra, msg)
 
+    def test_upgrade_region_non_unique_description(self):
+        """Test upgrade to migration 43.
+
+        This migration should occur with no unique constraint on the region
+        description column.
+
+        Create two regions with the same description.
+
+        """
+        session = self.Session()
+
+        def add_region():
+            region_uuid = uuid.uuid4().hex
+
+            region = {
+                'id': region_uuid,
+                'description': ''
+            }
+
+            self.insert_dict(session, 'region', region)
+            return region_uuid
+
+        self.upgrade(43)
+        # Write one region to the database
+        add_region()
+        # Write another region to the database with the same description
+        add_region()
+
+    def test_upgrade_region_unique_description(self):
+        """Test upgrade to migration 43.
+
+        This test models a migration where there is a unique constraint on the
+        description column.
+
+        Create two regions with the same description.
+
+        """
+        session = self.Session()
+
+        def add_region(table):
+            region_uuid = uuid.uuid4().hex
+
+            region = {
+                'id': region_uuid,
+                'description': ''
+            }
+
+            self.insert_dict(session, 'region', region, table=table)
+            return region_uuid
+
+        def get_metadata():
+            meta = sqlalchemy.MetaData()
+            meta.bind = self.engine
+            return meta
+
+        # Migrate to version 42
+        self.upgrade(42)
+        region_table = sqlalchemy.Table('region',
+                                        get_metadata(),
+                                        autoload=True)
+        # create the unique constraint and load the new version of the
+        # reflection cache
+        idx = sqlalchemy.Index('description', region_table.c.description,
+                               unique=True)
+        idx.create(self.engine)
+
+        region_unique_table = sqlalchemy.Table('region',
+                                               get_metadata(),
+                                               autoload=True)
+        add_region(region_unique_table)
+        self.assertEqual(1, session.query(region_unique_table).count())
+        # verify the unique constraint is enforced
+        self.assertRaises(sqlalchemy.exc.IntegrityError,
+                          add_region,
+                          table=region_unique_table)
+
+        # migrate to 43, unique constraint should be dropped
+        self.upgrade(43)
+
+        # reload the region table from the schema
+        region_nonunique = sqlalchemy.Table('region',
+                                            get_metadata(),
+                                            autoload=True)
+        self.assertEqual(1, session.query(region_nonunique).count())
+
+        # Write a second region to the database with the same description
+        add_region(region_nonunique)
+        self.assertEqual(2, session.query(region_nonunique).count())
+
     def populate_user_table(self, with_pass_enab=False,
                             with_pass_enab_domain=False):
         # Populate the appropriate fields in the user