Merge "Fix nullable constraints in service provider table"2015.1.0b3

2 files changed, 131 insertions, 0 deletions

diff --git a/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py b/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py
new file mode 100644
index 000000000..8a42ce3a4
--- /dev/null
+++ b/keystone/contrib/federation/migrate_repo/versions/006_fixup_service_provider_attributes.py
@@ -0,0 +1,48 @@
+# 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 sqlalchemy as sql
+
+_SP_TABLE_NAME = 'service_provider'
+
+
+def _update_null_columns(migrate_engine, sp_table):
+    stmt = (sp_table.update().
+            where(sp_table.c.auth_url.is_(None)).
+            values(auth_url=''))
+    migrate_engine.execute(stmt)
+
+    stmt = (sp_table.update().
+            where(sp_table.c.sp_url.is_(None)).
+            values(sp_url=''))
+    migrate_engine.execute(stmt)
+
+
+def upgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    sp_table = sql.Table(_SP_TABLE_NAME, meta, autoload=True)
+    # The columns are being changed to non-nullable. To prevent
+    # database errors when both are altered, all the existing
+    # null-records should be filled with not null values.
+    _update_null_columns(migrate_engine, sp_table)
+
+    sp_table.c.auth_url.alter(nullable=False)
+    sp_table.c.sp_url.alter(nullable=False)
+
+
+def downgrade(migrate_engine):
+    meta = sql.MetaData()
+    meta.bind = migrate_engine
+    sp_table = sql.Table(_SP_TABLE_NAME, meta, autoload=True)
+    sp_table.c.auth_url.alter(nullable=True)
+    sp_table.c.sp_url.alter(nullable=True)
diff --git a/keystone/tests/unit/test_sql_migrate_extensions.py b/keystone/tests/unit/test_sql_migrate_extensions.py
index 4b23ba517..edfb91d71 100644
--- a/keystone/tests/unit/test_sql_migrate_extensions.py
+++ b/keystone/tests/unit/test_sql_migrate_extensions.py
@@ -29,6 +29,10 @@ WARNING::
    all data will be lost.
 """
 
+import sqlalchemy
+import uuid
+
+from oslo_db import exception as db_exception
 from oslo_db.sqlalchemy import utils
 
 from keystone.contrib import endpoint_filter
@@ -216,11 +220,19 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase):
         super(FederationExtension, self).setUp()
         self.identity_provider = 'identity_provider'
         self.federation_protocol = 'federation_protocol'
+        self.service_provider = 'service_provider'
         self.mapping = 'mapping'
 
     def repo_package(self):
         return federation
 
+    def insert_dict(self, session, table_name, d):
+        """Naively inserts key-value pairs into a table, given a dictionary."""
+        table = sqlalchemy.Table(table_name, self.metadata, autoload=True)
+        insert = table.insert().values(**d)
+        session.execute(insert)
+        session.commit()
+
     def test_upgrade(self):
         self.assertTableDoesNotExist(self.identity_provider)
         self.assertTableDoesNotExist(self.federation_protocol)
@@ -272,6 +284,77 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase):
         self.assertTableDoesNotExist(self.federation_protocol)
         self.assertTableDoesNotExist(self.mapping)
 
+    def test_fixup_service_provider_attributes(self):
+        self.upgrade(6, repository=self.repo_path)
+        self.assertTableColumns(self.service_provider,
+                                ['id', 'description', 'enabled', 'auth_url',
+                                 'sp_url'])
+
+        session = self.Session()
+        sp1 = {'id': uuid.uuid4().hex,
+               'auth_url': None,
+               'sp_url': uuid.uuid4().hex,
+               'description': uuid.uuid4().hex,
+               'enabled': True}
+        sp2 = {'id': uuid.uuid4().hex,
+               'auth_url': uuid.uuid4().hex,
+               'sp_url': None,
+               'description': uuid.uuid4().hex,
+               'enabled': True}
+        sp3 = {'id': uuid.uuid4().hex,
+               'auth_url': None,
+               'sp_url': None,
+               'description': uuid.uuid4().hex,
+               'enabled': True}
+
+        # Insert with 'auth_url' or 'sp_url' set to null must fail
+        self.assertRaises(db_exception.DBError,
+                          self.insert_dict,
+                          session,
+                          self.service_provider,
+                          sp1)
+        self.assertRaises(db_exception.DBError,
+                          self.insert_dict,
+                          session,
+                          self.service_provider,
+                          sp2)
+        self.assertRaises(db_exception.DBError,
+                          self.insert_dict,
+                          session,
+                          self.service_provider,
+                          sp3)
+
+        session.close()
+        self.downgrade(5, repository=self.repo_path)
+        self.assertTableColumns(self.service_provider,
+                                ['id', 'description', 'enabled', 'auth_url',
+                                 'sp_url'])
+        session = self.Session()
+        self.metadata.clear()
+
+        # Before the migration, the table should accept null values
+        self.insert_dict(session, self.service_provider, sp1)
+        self.insert_dict(session, self.service_provider, sp2)
+        self.insert_dict(session, self.service_provider, sp3)
+
+        # Check if null values are updated to empty string when migrating
+        session.close()
+        self.upgrade(6, repository=self.repo_path)
+        sp_table = sqlalchemy.Table(self.service_provider,
+                                    self.metadata,
+                                    autoload=True)
+        session = self.Session()
+        self.metadata.clear()
+
+        sp = session.query(sp_table).filter(sp_table.c.id == sp1['id'])[0]
+        self.assertEqual('', sp.auth_url)
+
+        sp = session.query(sp_table).filter(sp_table.c.id == sp2['id'])[0]
+        self.assertEqual('', sp.sp_url)
+
+        sp = session.query(sp_table).filter(sp_table.c.id == sp3['id'])[0]
+        self.assertEqual('', sp.auth_url)
+        self.assertEqual('', sp.sp_url)
 
 _REVOKE_COLUMN_NAMES = ['id', 'domain_id', 'project_id', 'user_id', 'role_id',
                         'trust_id', 'consumer_id', 'access_token_id',