From a21d1a121264d1ca9bc208a01d7710eda33e00e4 Mon Sep 17 00:00:00 2001 From: Alexander Kislitsky Date: Thu, 18 Aug 2016 17:48:56 +0300 Subject: [PATCH] Hierarchy removed from level values We don't need to have duplicate of levels hierarchy in the level values. Without duplication we can easy change hierarchy levels in the environment. Cascade deletion added on level deletion to level values. Change-Id: I9aded18c93b8c3f0f08e59817d33e7d21ff12d54 --- tuning_box/db.py | 15 ++-- tuning_box/library/levels_hierarchy.py | 3 - ...89a70_remove_hierarchy_for_level_values.py | 78 +++++++++++++++++++ .../tests/library/test_levels_hierarchy.py | 9 +-- .../tests/library/test_resource_overrides.py | 9 +-- .../tests/library/test_resource_values.py | 8 +- 6 files changed, 95 insertions(+), 27 deletions(-) create mode 100644 tuning_box/migrations/versions/a86472389a70_remove_hierarchy_for_level_values.py diff --git a/tuning_box/db.py b/tuning_box/db.py index debf327..8d59e16 100644 --- a/tuning_box/db.py +++ b/tuning_box/db.py @@ -185,20 +185,15 @@ class EnvironmentHierarchyLevel(ModelMixin, db.Model): class EnvironmentHierarchyLevelValue(ModelMixin, db.Model): - level_id = fk(EnvironmentHierarchyLevel) + level_id = fk(EnvironmentHierarchyLevel, ondelete='CASCADE') level = db.relationship(EnvironmentHierarchyLevel) value = db.Column(db.String(128)) - @sa_decl.declared_attr - def parent_id(cls): - return db.Column(pk_type, db.ForeignKey(cls.id)) + __table_args__ = ( + db.UniqueConstraint('level_id', 'value'), + ) - @sa_decl.declared_attr - def parent(cls): - return db.relationship(cls, remote_side=cls.id) - - # TODO(yorik-sar): add UniqueConstraint for all fields - __repr_attrs__ = ('id', 'level', 'parent', 'value') + __repr_attrs__ = ('id', 'level', 'value') class ResourceValues(ModelMixin, db.Model): diff --git a/tuning_box/library/levels_hierarchy.py b/tuning_box/library/levels_hierarchy.py index 2b134fa..d83bea7 100644 --- a/tuning_box/library/levels_hierarchy.py +++ b/tuning_box/library/levels_hierarchy.py @@ -18,7 +18,6 @@ from tuning_box import db def iter_environment_level_values(environment, levels): env_levels = db.EnvironmentHierarchyLevel.get_for_environment(environment) level_pairs = zip(env_levels, levels) - parent_level_value = None for env_level, (level_name, level_value) in level_pairs: if env_level.name != level_name: raise werkzeug.exceptions.BadRequest( @@ -27,11 +26,9 @@ def iter_environment_level_values(environment, levels): level_value_db = db.get_or_create( db.EnvironmentHierarchyLevelValue, level=env_level, - parent=parent_level_value, value=level_value, ) yield level_value_db - parent_level_value = level_value_db def get_environment_level_value(environment, levels): diff --git a/tuning_box/migrations/versions/a86472389a70_remove_hierarchy_for_level_values.py b/tuning_box/migrations/versions/a86472389a70_remove_hierarchy_for_level_values.py new file mode 100644 index 0000000..e339681 --- /dev/null +++ b/tuning_box/migrations/versions/a86472389a70_remove_hierarchy_for_level_values.py @@ -0,0 +1,78 @@ +# 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. + +"""Remove hierarchy for level values + +Revision ID: a86472389a70 +Revises: 0c586adad733 +Create Date: 2016-08-18 14:00:03.197693 + +""" + +# revision identifiers, used by Alembic. +revision = 'a86472389a70' +down_revision = '0c586adad733' +branch_labels = None +depends_on = None + +from alembic import context +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + table_prefix = context.config.get_main_option('table_prefix') + table_name = table_prefix + 'environment_hierarchy_level_value' + with op.batch_alter_table(table_name) as batch: + batch.drop_column('parent_id') + + batch.drop_constraint( + table_name + '_level_id_fkey', + type_='foreignkey' + ) + batch.create_foreign_key( + table_name + '_level_id_fkey', + table_prefix + 'environment_hierarchy_level', + ['level_id'], ['id'], ondelete='CASCADE' + ) + + batch.create_unique_constraint( + table_name + '_level_id_value_unique', + ['level_id', 'value'] + ) + + +def downgrade(): + table_prefix = context.config.get_main_option('table_prefix') + table_name = table_prefix + 'environment_hierarchy_level_value' + with op.batch_alter_table(table_name) as batch: + batch.drop_constraint( + table_name + '_level_id_value_unique', + type_='unique' + ) + + batch.drop_constraint( + table_name + '_level_id_fkey', + type_='foreignkey' + ) + batch.create_foreign_key( + table_name + '_level_id_fkey', + table_prefix + 'environment_hierarchy_level', + ['level_id'], ['id'] + ) + + batch.add_column(sa.Column('parent_id', sa.Integer(), nullable=True)) + batch.create_foreign_key( + table_name + '_parent_id_fkey', + table_name, + ['parent_id'], ['id'] + ) diff --git a/tuning_box/tests/library/test_levels_hierarchy.py b/tuning_box/tests/library/test_levels_hierarchy.py index 11d6421..a160015 100644 --- a/tuning_box/tests/library/test_levels_hierarchy.py +++ b/tuning_box/tests/library/test_levels_hierarchy.py @@ -38,11 +38,10 @@ class TestLevelsHierarchy(BaseTest): self.assertIsNotNone(level_value) self.assertEqual(level_value.level.name, 'lvl2') self.assertEqual(level_value.value, 'val2') - level_value = level_value.parent - self.assertIsNotNone(level_value) - self.assertEqual(level_value.level.name, 'lvl1') - self.assertEqual(level_value.value, 'val1') - self.assertIsNone(level_value.parent) + level = level_value.level.parent + self.assertIsNotNone(level) + self.assertEqual(level.name, 'lvl1') + self.assertIsNone(level.parent) def test_get_environment_level_value_bad_level(self): self._fixture() diff --git a/tuning_box/tests/library/test_resource_overrides.py b/tuning_box/tests/library/test_resource_overrides.py index 40971a7..e0845ec 100644 --- a/tuning_box/tests/library/test_resource_overrides.py +++ b/tuning_box/tests/library/test_resource_overrides.py @@ -51,11 +51,10 @@ class TestResourceOverrides(BaseTest): self.assertIsNotNone(level_value) self.assertEqual(level_value.level.name, 'lvl2') self.assertEqual(level_value.value, 'val2') - level_value = level_value.parent - self.assertIsNotNone(level_value) - self.assertEqual(level_value.level.name, 'lvl1') - self.assertEqual(level_value.value, 'val1') - self.assertIsNone(level_value.parent) + level = level_value.level.parent + self.assertIsNotNone(level) + self.assertEqual(level.name, 'lvl1') + self.assertIsNone(level.parent) def test_get_resource_values_local_override(self): self._fixture() diff --git a/tuning_box/tests/library/test_resource_values.py b/tuning_box/tests/library/test_resource_values.py index 7ec6b86..5f801a7 100644 --- a/tuning_box/tests/library/test_resource_values.py +++ b/tuning_box/tests/library/test_resource_values.py @@ -50,10 +50,10 @@ class TestResourceValues(BaseTest): level_value = resource_values.level_value self.assertEqual(level_value.level.name, 'lvl2') self.assertEqual(level_value.value, 'val2') - level_value = level_value.parent - self.assertEqual(level_value.level.name, 'lvl1') - self.assertEqual(level_value.value, 'val1') - self.assertIsNone(level_value.parent) + level = level_value.level.parent + self.assertIsNotNone(level) + self.assertEqual(level.name, 'lvl1') + self.assertIsNone(level.parent) def test_put_resource_values_bad_level(self): self._fixture()