From 329b00d6dbd624f5554a9c5578b9f6a8070f3312 Mon Sep 17 00:00:00 2001 From: Adam Young Date: Fri, 8 Feb 2013 20:32:35 -0500 Subject: [PATCH] Fix normalize identity sql ugrade for Mysql and postgresql Change-Id: Idf374a748f8ed2add5310b504806ffabfa64bed9 --- .../versions/009_normalize_identity.py | 34 ++++++++++--- .../010_normalize_identity_migration.py | 51 +++++++++---------- tests/backend_sql.conf | 4 +- tests/test_sql_upgrade.py | 37 ++++++++------ 4 files changed, 75 insertions(+), 51 deletions(-) diff --git a/keystone/common/sql/migrate_repo/versions/009_normalize_identity.py b/keystone/common/sql/migrate_repo/versions/009_normalize_identity.py index 3f5ae3b884..7b2b1d6a4f 100644 --- a/keystone/common/sql/migrate_repo/versions/009_normalize_identity.py +++ b/keystone/common/sql/migrate_repo/versions/009_normalize_identity.py @@ -17,10 +17,11 @@ from sqlalchemy import Column, MetaData, String, Table, Text, types from sqlalchemy.orm import sessionmaker +from keystone import config #sqlite doesn't support dropping columns. Copy to a new table instead -def downgrade_user_table(meta, migrate_engine): +def downgrade_user_table_with_copy(meta, migrate_engine): maker = sessionmaker(bind=migrate_engine) session = maker() session.execute("ALTER TABLE user RENAME TO orig_user;") @@ -43,7 +44,7 @@ def downgrade_user_table(meta, migrate_engine): session.execute("drop table orig_user;") -def downgrade_tenant_table(meta, migrate_engine): +def downgrade_tenant_table_with_copy(meta, migrate_engine): maker = sessionmaker(bind=migrate_engine) session = maker() session.execute("ALTER TABLE tenant RENAME TO orig_tenant;") @@ -66,17 +67,30 @@ def downgrade_tenant_table(meta, migrate_engine): session.execute("drop table orig_tenant;") +def downgrade_user_table_with_column_drop(meta, migrate_engine): + user_table = Table('user', meta, autoload=True) + user_table.drop_column(Column('password', String(128))) + user_table.drop_column(Column('enabled', types.Boolean, + default=True)) + + +def downgrade_tenant_table_with_column_drop(meta, migrate_engine): + tenant_table = Table('tenant', meta, autoload=True) + tenant_table.drop_column(Column('description', Text())) + tenant_table.drop_column(Column('enabled', types.Boolean)) + + def upgrade_user_table(meta, migrate_engine): user_table = Table('user', meta, autoload=True) - user_table.create_column(Column("password", String(128))) - user_table.create_column(Column("enabled", types.Boolean, + user_table.create_column(Column('password', String(128))) + user_table.create_column(Column('enabled', types.Boolean, default=True)) def upgrade_tenant_table(meta, migrate_engine): tenant_table = Table('tenant', meta, autoload=True) - tenant_table.create_column(Column("description", Text())) - tenant_table.create_column(Column("enabled", types.Boolean)) + tenant_table.create_column(Column('description', Text())) + tenant_table.create_column(Column('enabled', types.Boolean)) def upgrade(migrate_engine): @@ -89,5 +103,9 @@ def upgrade(migrate_engine): def downgrade(migrate_engine): meta = MetaData() meta.bind = migrate_engine - downgrade_user_table(meta, migrate_engine) - downgrade_tenant_table(meta, migrate_engine) + if migrate_engine.name == 'sqlite': + downgrade_user_table_with_copy(meta, migrate_engine) + downgrade_tenant_table_with_copy(meta, migrate_engine) + else: + downgrade_user_table_with_column_drop(meta, migrate_engine) + downgrade_tenant_table_with_column_drop(meta, migrate_engine) diff --git a/keystone/common/sql/migrate_repo/versions/010_normalize_identity_migration.py b/keystone/common/sql/migrate_repo/versions/010_normalize_identity_migration.py index 4ad4db748b..01c153ce47 100644 --- a/keystone/common/sql/migrate_repo/versions/010_normalize_identity_migration.py +++ b/keystone/common/sql/migrate_repo/versions/010_normalize_identity_migration.py @@ -40,9 +40,11 @@ def downgrade_user_table(meta, migrate_engine): extra = json.loads(user.extra) extra['password'] = user.password extra['enabled'] = '%r' % user.enabled - session.execute( - 'UPDATE `user` SET `extra`=:extra WHERE `id`=:id', - {'id': user.id, 'extra': json.dumps(extra)}) + values = {'extra': json.dumps(extra)} + update = user_table.update().\ + where(user_table.c.id == user.id).\ + values(values) + migrate_engine.execute(update) session.commit() @@ -54,9 +56,11 @@ def downgrade_tenant_table(meta, migrate_engine): extra = json.loads(tenant.extra) extra['description'] = tenant.description extra['enabled'] = '%r' % tenant.enabled - session.execute( - 'UPDATE `tenant` SET `extra`=:extra WHERE `id`=:id', - {'id': tenant.id, 'extra': json.dumps(extra)}) + values = {'extra': json.dumps(extra)} + update = tenant_table.update().\ + where(tenant_table.c.id == tenant.id).\ + values(values) + migrate_engine.execute(update) session.commit() @@ -64,19 +68,15 @@ def upgrade_user_table(meta, migrate_engine): user_table = Table('user', meta, autoload=True) maker = sessionmaker(bind=migrate_engine) session = maker() - for user in session.query(user_table).all(): extra = json.loads(user.extra) - password = extra.pop('password', None) - enabled = extra.pop('enabled', True) - session.execute( - 'UPDATE `user` SET `password`=:password, `enabled`=:enabled, ' - '`extra`=:extra WHERE `id`=:id', - { - 'id': user.id, - 'password': password, - 'enabled': is_enabled(enabled), - 'extra': json.dumps(extra)}) + values = {'password': extra.pop('password', None), + 'enabled': extra.pop('enabled', True), + 'extra': json.dumps(extra)} + update = user_table.update().\ + where(user_table.c.id == user.id).\ + values(values) + migrate_engine.execute(update) session.commit() @@ -87,16 +87,13 @@ def upgrade_tenant_table(meta, migrate_engine): session = maker() for tenant in session.query(tenant_table): extra = json.loads(tenant.extra) - description = extra.pop('description', None) - enabled = extra.pop('enabled', True) - session.execute( - 'UPDATE `tenant` SET `enabled`=:enabled, `extra`=:extra, ' - '`description`=:description WHERE `id`=:id', - { - 'id': tenant.id, - 'description': description, - 'enabled': is_enabled(enabled), - 'extra': json.dumps(extra)}) + values = {'description': extra.pop('description', None), + 'enabled': extra.pop('enabled', True), + 'extra': json.dumps(extra)} + update = tenant_table.update().\ + where(tenant_table.c.id == tenant.id).\ + values(values) + migrate_engine.execute(update) session.commit() diff --git a/tests/backend_sql.conf b/tests/backend_sql.conf index f42eba6515..e1027cc8ff 100644 --- a/tests/backend_sql.conf +++ b/tests/backend_sql.conf @@ -1,7 +1,9 @@ [sql] connection = sqlite:// +#For a file based sqlite use +#connection = sqlite:////tmp/keystone.db #To Test MySQL: -#connection = mysql://root:keystone@localhost/keystone?charset=utf8 +#connection = mysql://keystone:keystone@localhost/keystone?charset=utf8 #To Test PostgreSQL: #connection = postgresql://keystone:keystone@localhost/keystone?client_encoding=utf8 idle_timeout = 200 diff --git a/tests/test_sql_upgrade.py b/tests/test_sql_upgrade.py index e04fc3e5cc..f0b5fa5d45 100644 --- a/tests/test_sql_upgrade.py +++ b/tests/test_sql_upgrade.py @@ -44,6 +44,11 @@ CONF = config.CONF class SqlUpgradeTests(test.TestCase): + + def initialize_sql(self): + self.metadata = sqlalchemy.MetaData() + self.metadata.bind = self.engine + def setUp(self): super(SqlUpgradeTests, self).setUp() self.config([test.etcdir('keystone.conf.sample'), @@ -51,18 +56,16 @@ class SqlUpgradeTests(test.TestCase): test.testsdir('backend_sql.conf')]) # create and share a single sqlalchemy engine for testing - base = sql.Base() - self.engine = base.get_engine(allow_global_engine=False) - self.Session = base.get_sessionmaker( - engine=self.engine, - autocommit=False) - self.metadata = sqlalchemy.MetaData() + self.base = sql.Base() + self.engine = self.base.get_engine(allow_global_engine=False) + self.Session = self.base.get_sessionmaker(engine=self.engine, + autocommit=False) - # populate the engine with tables & fixtures - self.metadata.bind = self.engine + self.initialize_sql() self.repo_path = migration._find_migrate_repo() - self.schema = versioning_api.ControlledSchema.create(self.engine, - self.repo_path, 0) + self.schema = versioning_api.ControlledSchema.create( + self.engine, + self.repo_path, 0) # auto-detect the highest available schema version in the migrate_repo self.max_version = self.schema.repository.version().version @@ -82,7 +85,7 @@ class SqlUpgradeTests(test.TestCase): self.assertEqual(version, 0, "DB is at version 0") def test_two_steps_forward_one_step_back(self): - """You should be able to cleanly undo a re-apply all upgrades. + """You should be able to cleanly undo and re-apply all upgrades. Upgrades are run in the following order:: @@ -97,6 +100,7 @@ class SqlUpgradeTests(test.TestCase): def assertTableColumns(self, table_name, expected_cols): """Asserts that the table contains the expected set of columns.""" + self.initialize_sql() table = self.select_table(table_name) actual_cols = [col.name for col in table.columns] self.assertEqual(expected_cols, actual_cols, '%s table' % table_name) @@ -352,14 +356,17 @@ class SqlUpgradeTests(test.TestCase): ['user_id', 'domain_id', 'data']) def populate_user_table(self): + user_table = sqlalchemy.Table('user', + self.metadata, + autoload=True) + session = self.Session() + insert = user_table.insert() for user in default_fixtures.USERS: extra = copy.deepcopy(user) extra.pop('id') extra.pop('name') - self.engine.execute("insert into user values ('%s', '%s', '%s')" - % (user['id'], - user['name'], - json.dumps(extra))) + user['extra'] = json.dumps(extra) + insert.execute(user) def populate_tenant_table(self): for tenant in default_fixtures.TENANTS: