diff options
| -rw-r--r-- | oslo_db/sqlalchemy/exc_filters.py | 4 | ||||
| -rw-r--r-- | oslo_db/sqlalchemy/utils.py | 69 | ||||
| -rw-r--r-- | oslo_db/tests/sqlalchemy/test_exc_filters.py | 53 | ||||
| -rw-r--r-- | oslo_db/tests/sqlalchemy/test_utils.py | 119 | ||||
| -rw-r--r-- | setup.cfg | 2 |
5 files changed, 201 insertions, 46 deletions
diff --git a/oslo_db/sqlalchemy/exc_filters.py b/oslo_db/sqlalchemy/exc_filters.py index 051d1f8..9e5a2e5 100644 --- a/oslo_db/sqlalchemy/exc_filters.py +++ b/oslo_db/sqlalchemy/exc_filters.py @@ -197,7 +197,7 @@ def _sqlite_dupe_key_error(integrity_error, match, engine_name, is_disconnect): "is (not present in|still referenced from) table " "\"(?P<key_table>[^\"]+)\".") @filters("mysql", sqla_exc.IntegrityError, - r".* u?'Cannot (add|delete) or update a (child|parent) row: " + r".*Cannot (add|delete) or update a (child|parent) row: " 'a foreign key constraint fails \([`"].+[`"]\.[`"](?P<table>.+)[`"], ' 'CONSTRAINT [`"](?P<constraint>.+)[`"] FOREIGN KEY ' '\([`"](?P<key>.+)[`"]\) REFERENCES [`"](?P<key_table>.+)[`"] ') @@ -276,7 +276,7 @@ def _check_constraint_non_existing( @filters("sqlite", sqla_exc.OperationalError, r".* no such table: (?P<table>.+)") @filters("mysql", sqla_exc.InternalError, - r".*1051,.*\"Unknown table '(.+\.)?(?P<table>.+)'\"") + r".*1051,.*Unknown table '(.+\.)?(?P<table>.+)'\"") @filters("postgresql", sqla_exc.ProgrammingError, r".* table \"(?P<table>.+)\" does not exist") def _check_table_non_existing( diff --git a/oslo_db/sqlalchemy/utils.py b/oslo_db/sqlalchemy/utils.py index 594a7f2..f5dda31 100644 --- a/oslo_db/sqlalchemy/utils.py +++ b/oslo_db/sqlalchemy/utils.py @@ -31,6 +31,7 @@ from sqlalchemy import Column from sqlalchemy.engine import Connectable from sqlalchemy.engine import reflection from sqlalchemy.engine import url as sa_url +from sqlalchemy import exc from sqlalchemy import func from sqlalchemy import Index from sqlalchemy import inspect @@ -66,6 +67,66 @@ def sanitize_db_url(url): return url +def get_unique_keys(model): + """Get a list of sets of unique model keys. + + :param model: the ORM model class + :rtype: list of sets of strings + :return: unique model keys or None if unable to find them + """ + + try: + mapper = inspect(model) + except exc.NoInspectionAvailable: + return None + else: + table = mapper.mapped_table + if table is None: + return None + + # extract result from cache if present + info = table.info + if 'oslodb_unique_keys' in info: + return info['oslodb_unique_keys'] + + res = [] + try: + constraints = table.constraints + except AttributeError: + constraints = [] + for constraint in constraints: + # filter out any CheckConstraints + if isinstance(constraint, (sqlalchemy.UniqueConstraint, + sqlalchemy.PrimaryKeyConstraint)): + res.append({c.name for c in constraint.columns}) + try: + indexes = table.indexes + except AttributeError: + indexes = [] + for index in indexes: + if index.unique: + res.append({c.name for c in index.columns}) + # cache result for next calls with the same model + info['oslodb_unique_keys'] = res + return res + + +def _stable_sorting_order(model, sort_keys): + """Check whetever the sorting order is stable. + + :return: True if it is stable, False if it's not, None if it's impossible + to determine. + """ + keys = get_unique_keys(model) + if keys is None: + return None + sort_keys_set = set(sort_keys) + for unique_keys in keys: + if unique_keys.issubset(sort_keys_set): + return True + return False + + # copy from glance/db/sqlalchemy/api.py def paginate_query(query, model, limit, sort_keys, marker=None, sort_dir=None, sort_dirs=None): @@ -100,11 +161,9 @@ def paginate_query(query, model, limit, sort_keys, marker=None, :rtype: sqlalchemy.orm.query.Query :return: The query with sorting/pagination added. """ - - if 'id' not in sort_keys: - # TODO(justinsb): If this ever gives a false-positive, check - # the actual primary key, rather than assuming its id - LOG.warning(_LW('Id not in sort_keys; is sort_keys unique?')) + if _stable_sorting_order(model, sort_keys) is False: + LOG.warning(_LW('Unique keys not in sort_keys. ' + 'The sorting order may be unstable.')) assert(not (sort_dir and sort_dirs)) diff --git a/oslo_db/tests/sqlalchemy/test_exc_filters.py b/oslo_db/tests/sqlalchemy/test_exc_filters.py index 17153a7..e31f8dd 100644 --- a/oslo_db/tests/sqlalchemy/test_exc_filters.py +++ b/oslo_db/tests/sqlalchemy/test_exc_filters.py @@ -512,16 +512,11 @@ class TestReferenceErrorMySQL(TestReferenceErrorSQLite, self.table_2.insert({'id': 1, 'foo_id': 2}) ) - self.assertInnerException( - matched, - "IntegrityError", - (1452, "Cannot add or update a child row: a " - "foreign key constraint fails (`{0}`.`resource_entity`, " - "CONSTRAINT `foo_fkey` FOREIGN KEY (`foo_id`) REFERENCES " - "`resource_foo` (`id`))".format(self.engine.url.database)), - "INSERT INTO resource_entity (id, foo_id) VALUES (%s, %s)", - (1, 2) - ) + # NOTE(jd) Cannot check precisely with assertInnerException since MySQL + # error are not the same depending on its version… + self.assertIsInstance(matched.inner_exception, + sqlalchemy.exc.IntegrityError) + self.assertEqual(matched.inner_exception.orig.args[0], 1452) self.assertEqual("resource_entity", matched.table) self.assertEqual("foo_fkey", matched.constraint) self.assertEqual("foo_id", matched.key) @@ -540,19 +535,11 @@ class TestReferenceErrorMySQL(TestReferenceErrorSQLite, self.table_2.insert({'id': 1, 'foo_id': 2}) ) - self.assertInnerException( - matched, - "IntegrityError", - ( - 1452, - 'Cannot add or update a child row: a ' - 'foreign key constraint fails ("{0}"."resource_entity", ' - 'CONSTRAINT "foo_fkey" FOREIGN KEY ("foo_id") REFERENCES ' - '"resource_foo" ("id"))'.format(self.engine.url.database) - ), - "INSERT INTO resource_entity (id, foo_id) VALUES (%s, %s)", - (1, 2) - ) + # NOTE(jd) Cannot check precisely with assertInnerException since MySQL + # error are not the same depending on its version… + self.assertIsInstance(matched.inner_exception, + sqlalchemy.exc.IntegrityError) + self.assertEqual(matched.inner_exception.orig.args[0], 1452) self.assertEqual("resource_entity", matched.table) self.assertEqual("foo_fkey", matched.constraint) self.assertEqual("foo_id", matched.key) @@ -567,21 +554,11 @@ class TestReferenceErrorMySQL(TestReferenceErrorSQLite, self.engine.execute, self.table_1.delete() ) - self.assertInnerException( - matched, - "IntegrityError", - ( - 1451, - "Cannot delete or update a parent row: a foreign key " - "constraint fails (`{0}`.`resource_entity`, " - "constraint `foo_fkey` " - "foreign key (`foo_id`) references " - "`resource_foo` (`id`))".format(self.engine.url.database) - ), - "DELETE FROM resource_foo", - (), - ) - + # NOTE(jd) Cannot check precisely with assertInnerException since MySQL + # error are not the same depending on its version… + self.assertIsInstance(matched.inner_exception, + sqlalchemy.exc.IntegrityError) + self.assertEqual(1451, matched.inner_exception.orig.args[0]) self.assertEqual("resource_entity", matched.table) self.assertEqual("foo_fkey", matched.constraint) self.assertEqual("foo_id", matched.key) diff --git a/oslo_db/tests/sqlalchemy/test_utils.py b/oslo_db/tests/sqlalchemy/test_utils.py index 19f78ce..e5f5a6c 100644 --- a/oslo_db/tests/sqlalchemy/test_utils.py +++ b/oslo_db/tests/sqlalchemy/test_utils.py @@ -31,6 +31,7 @@ from sqlalchemy.engine import url as sa_url from sqlalchemy.exc import OperationalError from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.ext.hybrid import hybrid_property +from sqlalchemy.orm import mapper from sqlalchemy.orm import Session from sqlalchemy.sql import select from sqlalchemy.types import UserDefinedType, NullType @@ -93,6 +94,42 @@ class FakeTable(Base): pass +class FakeTableWithMultipleKeys(Base): + __tablename__ = 'fake_table_multiple_keys' + + key1 = Column(String(50), primary_key=True) + key2 = Column(String(50), primary_key=True) + key3 = Column(String(50)) + + +class FakeTableWithIndexes(Base): + __tablename__ = 'fake_table_unique_index' + + id = Column(String(50), primary_key=True) + key1 = Column(String(50)) + key2 = Column(String(50)) + key3 = Column(String(50)) + + __table_args__ = ( + Index('idx_unique', 'key1', 'key2', unique=True), + Index('idx_unique', 'key1', 'key3', unique=False), + ) + + +class FakeTableClassicalyMapped(object): + pass + + +fake_table = Table( + 'fake_table_classically_mapped', + Base.metadata, + Column('id', Integer, primary_key=True), + Column('key', String(50)) +) + +mapper(FakeTableClassicalyMapped, fake_table) + + class FakeModel(object): def __init__(self, values): self.values = values @@ -276,6 +313,88 @@ class TestPaginateQuery(test_base.BaseTestCase): sort_dirs=['asc', 'desc']) +class Test_UnstableSortingOrder(test_base.BaseTestCase): + def test_multiple_primary_keys_stable(self): + self.assertTrue( + utils._stable_sorting_order( + FakeTableWithMultipleKeys, ['key1', 'key2'])) + + def test_classically_mapped_primary_keys_stable(self): + self.assertTrue( + utils._stable_sorting_order(FakeTableClassicalyMapped, ['id'])) + + def test_multiple_primary_keys_unstable(self): + self.assertFalse( + utils._stable_sorting_order( + FakeTableWithMultipleKeys, ['key1', 'key3'])) + + def test_unknown_primary_keys_stable(self): + self.assertIsNone( + utils._stable_sorting_order(object, ['key1', 'key2'])) + + def test_unique_index_stable(self): + self.assertTrue( + utils._stable_sorting_order( + FakeTableWithIndexes, ['key1', 'key2'])) + + def test_unique_index_unstable(self): + self.assertFalse( + utils._stable_sorting_order( + FakeTableWithIndexes, ['key1', 'key3'])) + + +class TestGetUniqueKeys(test_base.BaseTestCase): + def test_multiple_primary_keys(self): + self.assertEqual( + [{'key1', 'key2'}], + utils.get_unique_keys(FakeTableWithMultipleKeys)) + + def test_unique_index(self): + self.assertEqual( + [{'id'}, {'key1', 'key2'}], + utils.get_unique_keys(FakeTableWithIndexes)) + + def test_unknown_primary_keys(self): + self.assertIsNone(utils.get_unique_keys(object)) + + def test_cache(self): + + class CacheTable(object): + info = {} + constraints_called = 0 + indexes_called = 0 + + @property + def constraints(self): + self.constraints_called += 1 + return [] + + @property + def indexes(self): + self.indexes_called += 1 + return [] + + class CacheModel(object): + pass + + table = CacheTable() + mock_inspect = mock.Mock(return_value=mock.Mock(mapped_table=table)) + model = CacheModel() + self.assertNotIn('oslodb_unique_keys', CacheTable.info) + with mock.patch("oslo_db.sqlalchemy.utils.inspect", mock_inspect): + utils.get_unique_keys(model) + + self.assertIn('oslodb_unique_keys', CacheTable.info) + self.assertEqual(1, table.constraints_called) + self.assertEqual(1, table.indexes_called) + + for i in range(10): + utils.get_unique_keys(model) + + self.assertEqual(1, table.constraints_called) + self.assertEqual(1, table.indexes_called) + + class TestPaginateQueryActualSQL(test_base.BaseTestCase): def test_paginate_on_hybrid_assert_stmt(self): @@ -22,7 +22,7 @@ classifier = [extras] # So e.g. nova can test-depend on oslo.db[mysql] mysql = - PyMySQL>=0.6.2 # MIT License + PyMySQL!=0.7.7,>=0.6.2 # MIT License # or oslo.db[mysql-c] mysql-c = MySQL-python:python_version=='2.7' # GPL with FOSS exception |