From 3b24ccaf28fd85a6e54aa813fde8b3677253f67f Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 30 Jun 2013 11:09:37 -0400 Subject: [PATCH] A warning is emitted when trying to flush an object of an inherited mapped class where the polymorphic discriminator has been assigned to a value that is invalid for the class. [ticket:2750] --- doc/build/changelog/changelog_08.rst | 8 +++++++ doc/build/changelog/changelog_09.rst | 8 +++++++ lib/sqlalchemy/orm/mapper.py | 33 +++++++++++++++++++++++++++- lib/sqlalchemy/orm/persistence.py | 3 +++ test/orm/inheritance/test_basic.py | 26 +++++++++++++++++++--- 5 files changed, 74 insertions(+), 4 deletions(-) diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst index 442b7a2d9c..c0e430ad66 100644 --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@ -6,6 +6,14 @@ .. changelog:: :version: 0.8.2 + .. change:: + :tags: bug, orm + :tickets: 2750 + + A warning is emitted when trying to flush an object of an inherited + class where the polymorphic discriminator has been assigned + to a value that is invalid for the class. + .. change:: :tags: bug, postgresql :tickets: 2740 diff --git a/doc/build/changelog/changelog_09.rst b/doc/build/changelog/changelog_09.rst index f0f0bd0911..af3081687e 100644 --- a/doc/build/changelog/changelog_09.rst +++ b/doc/build/changelog/changelog_09.rst @@ -6,6 +6,14 @@ .. changelog:: :version: 0.9.0 + .. change:: + :tags: bug, orm + :tickets: 2750 + + A warning is emitted when trying to flush an object of an inherited + mapped class where the polymorphic discriminator has been assigned + to a value that is invalid for the class. Also in 0.8.2. + .. change:: :tags: bug, postgresql :tickets: 2740 diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 7f14d83cb4..12d2234d2d 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -28,7 +28,7 @@ from .interfaces import MapperProperty, _InspectionAttr, _MappedAttribute from .util import _INSTRUMENTOR, _class_to_mapper, \ _state_mapper, class_mapper, \ - PathRegistry + PathRegistry, state_str import sys properties = util.importlater("sqlalchemy.orm", "properties") descriptor_props = util.importlater("sqlalchemy.orm", "descriptor_props") @@ -1040,6 +1040,8 @@ class Mapper(_InspectionAttr): if self.polymorphic_on is not None: self._set_polymorphic_identity = \ mapper._set_polymorphic_identity + self._validate_polymorphic_identity = \ + mapper._validate_polymorphic_identity else: self._set_polymorphic_identity = None return @@ -1050,10 +1052,39 @@ class Mapper(_InspectionAttr): state.get_impl(polymorphic_key).set(state, dict_, state.manager.mapper.polymorphic_identity, None) + def _validate_polymorphic_identity(mapper, state, dict_): + if dict_[polymorphic_key] not in \ + mapper._acceptable_polymorphic_identities: + util.warn( + "Flushing object %s with " + "incompatible polymorphic identity %r; the " + "object may not refresh and/or load correctly" % ( + state_str(state), + dict_[polymorphic_key] + ) + ) + self._set_polymorphic_identity = _set_polymorphic_identity + self._validate_polymorphic_identity = _validate_polymorphic_identity else: self._set_polymorphic_identity = None + + _validate_polymorphic_identity = None + + @_memoized_configured_property + def _acceptable_polymorphic_identities(self): + identities = set() + + stack = deque([self]) + while stack: + item = stack.popleft() + if item.mapped_table is self.mapped_table: + identities.add(item.polymorphic_identity) + stack.extend(item._inheriting_mappers) + + return identities + def _adapt_inherited_property(self, key, prop, init): if not self.concrete: self._configure_property(key, prop, init=False, setparent=False) diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py index a773786c4a..944623b079 100644 --- a/lib/sqlalchemy/orm/persistence.py +++ b/lib/sqlalchemy/orm/persistence.py @@ -150,6 +150,9 @@ def _organize_states_for_save(base_mapper, states, uowtransaction): else: mapper.dispatch.before_update(mapper, connection, state) + if mapper._validate_polymorphic_identity: + mapper._validate_polymorphic_identity(mapper, state, dict_) + # detect if we have a "pending" instance (i.e. has # no instance_key attached to it), and another instance # with the same identity key already exists as persistent. diff --git a/test/orm/inheritance/test_basic.py b/test/orm/inheritance/test_basic.py index 612d6e8ca3..3165237b2e 100644 --- a/test/orm/inheritance/test_basic.py +++ b/test/orm/inheritance/test_basic.py @@ -3,6 +3,7 @@ from sqlalchemy.testing import eq_, assert_raises, assert_raises_message from sqlalchemy import * from sqlalchemy import exc as sa_exc, util, event from sqlalchemy.orm import * +from sqlalchemy.orm.util import instance_str from sqlalchemy.orm import exc as orm_exc, attributes from sqlalchemy.testing.assertsql import AllOf, CompiledSQL from sqlalchemy.sql import table, column @@ -573,6 +574,8 @@ class PolymorphicAttributeManagementTest(fixtures.MappedTest): pass class C(B): pass + class D(B): + pass mapper(A, table_a, polymorphic_on=table_a.c.class_name, @@ -582,6 +585,8 @@ class PolymorphicAttributeManagementTest(fixtures.MappedTest): polymorphic_identity='b') mapper(C, table_c, inherits=B, polymorphic_identity='c') + mapper(D, inherits=B, + polymorphic_identity='d') def test_poly_configured_immediate(self): A, C, B = (self.classes.A, @@ -612,15 +617,30 @@ class PolymorphicAttributeManagementTest(fixtures.MappedTest): assert isinstance(sess.query(A).first(), C) def test_assignment(self): - C, B = self.classes.C, self.classes.B + D, B = self.classes.D, self.classes.B sess = Session() b1 = B() - b1.class_name = 'c' + b1.class_name = 'd' sess.add(b1) sess.commit() sess.close() - assert isinstance(sess.query(B).first(), C) + assert isinstance(sess.query(B).first(), D) + + def test_invalid_assignment(self): + C, B = self.classes.C, self.classes.B + + sess = Session() + c1 = C() + c1.class_name = 'b' + sess.add(c1) + assert_raises_message( + sa_exc.SAWarning, + "Flushing object %s with incompatible " + "polymorphic identity 'b'; the object may not " + "refresh and/or load correctly" % instance_str(c1), + sess.flush + ) class CascadeTest(fixtures.MappedTest): """that cascades on polymorphic relationships continue -- 2.47.3