Merge "Correct nullable values in models and migrations"

keystone/catalog/backends/sql.py

@@ -30,7 +30,7 @@ class Region(sql.ModelBase, sql.DictBase):
     __tablename__ = 'region'
     attributes = ['id', 'description', 'parent_region_id']
     id = sql.Column(sql.String(64), primary_key=True)
-    description = sql.Column(sql.String(255))
+    description = sql.Column(sql.String(255), nullable=False)
     # NOTE(jaypipes): Right now, using an adjacency list model for
     #                 storing the hierarchy of regions is fine, since
     #                 the API does not support any kind of querying for

keystone/contrib/federation/migrate_repo/versions/003_mapping_id_nullable_false.py

@@ -0,0 +1,35 @@
+# Copyright 2014 Mirantis.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 sqlalchemy as sa
+
+
+def upgrade(migrate_engine):
+    meta = sa.MetaData(bind=migrate_engine)
+    federation_protocol = sa.Table('federation_protocol', meta, autoload=True)
+    # NOTE(i159): The column is changed to non-nullable. To prevent
+    # database errors when the column will be altered, all the existing
+    # null-records should be filled with not null values.
+    stmt = (federation_protocol.update().
+            where(federation_protocol.c.mapping_id.is_(None)).
+            values(mapping_id=''))
+    migrate_engine.execute(stmt)
+    federation_protocol.c.mapping_id.alter(nullable=False)
+
+
+def downgrade(migrate_engine):
+    meta = sa.MetaData(bind=migrate_engine)
+    federation_protocol = sa.Table('federation_protocol', meta, autoload=True)
+    federation_protocol.c.mapping_id.alter(nullable=True)

keystone/tests/test_sql_migrate_extensions.py

@@ -37,6 +37,7 @@ from keystone.contrib import example
 from keystone.contrib import federation
 from keystone.contrib import oauth1
 from keystone.contrib import revoke
+from keystone.openstack.common.db.sqlalchemy import utils
 from keystone.tests import test_sql_upgrade
 
 
@@ -168,8 +169,19 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase):
         self.assertTableColumns(self.mapping,
                                 ['id', 'rules'])
 
+        federation_protocol = utils.get_table(
+            self.engine,
+            'federation_protocol')
+        with self.engine.begin() as conn:
+            conn.execute(federation_protocol.insert(), id=0, idp_id=1)
+            self.upgrade(3, repository=self.repo_path)
+            federation_protocol = utils.get_table(
+                self.engine,
+                'federation_protocol')
+            self.assertFalse(federation_protocol.c.mapping_id.nullable)
+
     def test_downgrade(self):
-        self.upgrade(2, repository=self.repo_path)
+        self.upgrade(3, repository=self.repo_path)
         self.assertTableColumns(self.identity_provider,
                                 ['id', 'enabled', 'description'])
         self.assertTableColumns(self.federation_protocol,
@@ -177,6 +189,12 @@ class FederationExtension(test_sql_upgrade.SqlMigrateBase):
         self.assertTableColumns(self.mapping,
                                 ['id', 'rules'])
 
+        self.downgrade(2, repository=self.repo_path)
+        federation_protocol = utils.get_table(
+            self.engine,
+            'federation_protocol')
+        self.assertTrue(federation_protocol.c.mapping_id.nullable)
+
         self.downgrade(0, repository=self.repo_path)
         self.assertTableDoesNotExist(self.identity_provider)
         self.assertTableDoesNotExist(self.federation_protocol)