From 8835866fdda46220f1361716ef6a4fb4afa6c918 Mon Sep 17 00:00:00 2001 From: Boris Pavlovic Date: Fri, 16 Nov 2012 12:36:28 +0400 Subject: Add DBDuplicateEntry exception for unique constraint violations Unique constraint violation is wrapped by IntegrityError in SqlAlchemy. SqlAlchemy doesn't unify errors from different DBs so we must do this in nova. Also in some tables (for example instance_types) there are more than one unique constraint. This means we should get names of columns, which values violate unique constraint, from error message. Unique constraint violations messages are different for different DB backedns: 1) sqlite: 1 column - (IntegrityError) column c1 is not unique N columns - (IntegrityError) column c1, c2, ..., N are not unique 2) postgres: 1 column - (IntegrityError) duplicate key value violates unique constraint "users_c1_key" N columns - (IntegrityError) duplicate key value violates unique constraint "name_of_our_constraint" 3) mysql: 1 column - (IntegrityError) (1062, "Duplicate entry 'value_of_c1' for key 'c1'") N columns - (IntegrityError) (1062, "Duplicate entry 'values joined with -' for key 'name_of_our_constraint'") There is no information about table and columns in `N columns` messages for mysql and postgres. So we should make name convention for UniqueConstraints name: "uniq_c1_x_c2_x_c3" means that columns c1, c2, c3 are in UniqueConstraint. Also there is another way to get columns name from unique constraint: 1) Get table name. 2) Load table = Table(table_name, meta, autoload=True). 3) Find unique constraint that is in table.constraints by name. 4) Get columns names from unique constraint object. But I think that it is a bad approach because: 1) We must do table autoload in exception handling 2) There is no easy way to get table name in wrap_db_error() method. Remove Duplicate exception handling from models.save() method. Add new DBDuplicateEntry exception to exceptions module. Add new wrapper in sqlalchemy get_session() for handling Integrity Error. blueprint db-unique-keys Change-Id: Ic8fd8e0613f10d06c1d7d90f76a436099e8bfef2 --- nova/db/sqlalchemy/models.py | 9 +---- nova/db/sqlalchemy/session.py | 87 +++++++++++++++++++++++++++++++++++++++++-- nova/exception.py | 20 ++++------ 3 files changed, 91 insertions(+), 25 deletions(-) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index a038b6745..94d6df0e1 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -22,7 +22,6 @@ SQLAlchemy models for nova data. """ from sqlalchemy import Column, Integer, BigInteger, String, schema -from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.declarative import declarative_base from sqlalchemy import ForeignKey, DateTime, Boolean, Text, Float from sqlalchemy.orm import relationship, backref, object_mapper @@ -51,13 +50,7 @@ class NovaBase(object): if not session: session = get_session() session.add(self) - try: - session.flush() - except IntegrityError, e: - if str(e).endswith('is not unique'): - raise exception.Duplicate(str(e)) - else: - raise + session.flush() def delete(self, session=None): """Delete this object.""" diff --git a/nova/db/sqlalchemy/session.py b/nova/db/sqlalchemy/session.py index cb05cc444..8a8414662 100644 --- a/nova/db/sqlalchemy/session.py +++ b/nova/db/sqlalchemy/session.py @@ -169,12 +169,13 @@ try: import MySQLdb except ImportError: MySQLdb = None -from sqlalchemy.exc import DisconnectionError, OperationalError +from sqlalchemy.exc import DisconnectionError, OperationalError, IntegrityError import sqlalchemy.interfaces import sqlalchemy.orm from sqlalchemy.pool import NullPool, StaticPool -import nova.exception +from nova.exception import DBDuplicateEntry +from nova.exception import DBError from nova.openstack.common import cfg import nova.openstack.common.log as logging @@ -245,10 +246,88 @@ def get_session(autocommit=True, expire_on_commit=False): return session +# note(boris-42): In current versions of DB backends unique constraint +# violation messages follow the structure: +# +# sqlite: +# 1 column - (IntegrityError) column c1 is not unique +# N columns - (IntegrityError) column c1, c2, ..., N are not unique +# +# postgres: +# 1 column - (IntegrityError) duplicate key value violates unique +# constraint "users_c1_key" +# N columns - (IntegrityError) duplicate key value violates unique +# constraint "name_of_our_constraint" +# +# mysql: +# 1 column - (IntegrityError) (1062, "Duplicate entry 'value_of_c1' for key +# 'c1'") +# N columns - (IntegrityError) (1062, "Duplicate entry 'values joined +# with -' for key 'name_of_our_constraint'") +_RE_DB = { + "sqlite": re.compile(r"^.*columns?([^)]+)(is|are)\s+not\s+unique$"), + "postgresql": re.compile(r"^.*duplicate\s+key.*\"([^\"]+)\"\s*\n.*$"), + "mysql": re.compile(r"^.*\(1062,.*'([^\']+)'\"\)$") +} + + +def raise_if_duplicate_entry_error(integrity_error, engine_name): + """ In this function will be raised DBDuplicateEntry exception if integrity + error wrap unique constraint violation. """ + + def get_columns_from_uniq_cons_or_name(columns): + # note(boris-42): UniqueConstraint name convention: "uniq_c1_x_c2_x_c3" + # means that columns c1, c2, c3 are in UniqueConstraint. + uniqbase = "uniq_" + if not columns.startswith(uniqbase): + if engine_name == "postgresql": + return [columns[columns.index("_") + 1:columns.rindex("_")]] + return [columns] + return columns[len(uniqbase):].split("_x_") + + if engine_name not in ["mysql", "sqlite", "postgresql"]: + return + + m = _RE_DB[engine_name].match(integrity_error.message) + if not m: + return + columns = m.group(1) + + if engine_name == "sqlite": + columns = columns.strip().split(", ") + else: + columns = get_columns_from_uniq_cons_or_name(columns) + raise DBDuplicateEntry(columns, integrity_error) + + +def wrap_db_error(f): + def _wrap(*args, **kwargs): + try: + return f(*args, **kwargs) + except UnicodeEncodeError: + raise InvalidUnicodeParameter() + # note(boris-42): We should catch unique constraint violation and + # wrap it by our own DBDuplicateEntry exception. Unique constraint + # violation is wrapped by IntegrityError. + except IntegrityError, e: + # note(boris-42): SqlAlchemy doesn't unify errors from different + # DBs so we must do this. Also in some tables (for example + # instance_types) there are more than one unique constraint. This + # means we should get names of columns, which values violate + # unique constraint, from error message. + raise_if_duplicate_entry_error(e, get_engine().name) + raise DBError(e) + except Exception, e: + LOG.exception(_('DB exception wrapped.')) + raise DBError(e) + _wrap.func_name = f.func_name + return _wrap + + def wrap_session(session): """Return a session whose exceptions are wrapped.""" - session.query = nova.exception.wrap_db_error(session.query) - session.flush = nova.exception.wrap_db_error(session.flush) + session.query = wrap_db_error(session.query) + session.flush = wrap_db_error(session.flush) return session diff --git a/nova/exception.py b/nova/exception.py index 0d08491cd..10349f386 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -62,19 +62,6 @@ class ProcessExecutionError(IOError): IOError.__init__(self, message) -def wrap_db_error(f): - def _wrap(*args, **kwargs): - try: - return f(*args, **kwargs) - except UnicodeEncodeError: - raise InvalidUnicodeParameter() - except Exception, e: - LOG.exception(_('DB exception wrapped.')) - raise DBError(e) - _wrap.func_name = f.func_name - return _wrap - - def wrap_exception(notifier=None, publisher_id=None, event_type=None, level=None): """This decorator wraps a method to catch any exceptions that may @@ -173,6 +160,13 @@ class DBError(NovaException): super(DBError, self).__init__(str(inner_exception)) +class DBDuplicateEntry(DBError): + """Wraps an implementation specific exception.""" + def __init__(self, columns=[], inner_exception=None): + self.columns = columns + super(DBDuplicateEntry, self).__init__(inner_exception) + + class DecryptionFailure(NovaException): message = _("Failed to decrypt text") -- cgit