From 7693a680edf9776d3037283f3d63e3aeeb06649e Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 5 Mar 2007 05:12:09 +0000 Subject: [PATCH] - put an aggressive check for "flushing object A with a collection of B's, but you put a C in the collection" error condition - **even if C is a subclass of B**, unless B's mapper loads polymorphically. Otherwise, the collection will later load a "B" which should be a "C" (since its not polymorphic) which breaks in bi-directional relationships (i.e. C has its A, but A's backref will lazyload it as a different instance of type "B") [ticket:500] --- CHANGES | 8 ++- lib/sqlalchemy/orm/dependency.py | 9 ++- lib/sqlalchemy/orm/mapper.py | 7 ++ test/orm/relationships.py | 118 +++++++++++++++++++++++++++++++ 4 files changed, 140 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index e554003e52..e830654e40 100644 --- a/CHANGES +++ b/CHANGES @@ -32,7 +32,13 @@ - more fixes to polymorphic relations, involving proper lazy-clause generation on many-to-one relationships to polymorphic mappers [ticket:493] - + - put an aggressive check for "flushing object A with a collection + of B's, but you put a C in the collection" error condition - + **even if C is a subclass of B**, unless B's mapper loads polymorphically. + Otherwise, the collection will later load a "B" which should be a "C" + (since its not polymorphic) which breaks in bi-directional relationships + (i.e. C has its A, but A's backref will lazyload it as a different + instance of type "B") [ticket:500] 0.3.5 - sql: - the value of "case_sensitive" defaults to True now, regardless of the diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py index 5a756f5d27..0b3b70d219 100644 --- a/lib/sqlalchemy/orm/dependency.py +++ b/lib/sqlalchemy/orm/dependency.py @@ -12,7 +12,7 @@ list-based dependencies at flush time. from sqlalchemy.orm import sync from sqlalchemy.orm.sync import ONETOMANY,MANYTOONE,MANYTOMANY -from sqlalchemy import sql, util +from sqlalchemy import sql, util, exceptions from sqlalchemy.orm import session as sessionlib def create_dependency_processor(prop): @@ -100,6 +100,10 @@ class DependencyProcessor(object): raise NotImplementedError() + def _verify_canload(self, child): + if child is not None and not self.mapper.canload(child): + raise exceptions.FlushError("Attempting to flush an item of type %s on collection '%s', which is handled by mapper '%s' and does not load items of that type. Did you mean to use a polymorphic mapper for this relationship ?" % (child.__class__, self.prop, self.mapper)) + def _synchronize(self, obj, child, associationrow, clearkeys, uowcommit): """Called during a flush to synchronize primary key identifier values between a parent/child object, as well as to an @@ -245,6 +249,7 @@ class OneToManyDP(DependencyProcessor): dest = child if dest is None or (not self.post_update and uowcommit.is_deleted(dest)): return + self._verify_canload(child) self.syncrules.execute(source, dest, obj, child, clearkeys) class ManyToOneDP(DependencyProcessor): @@ -309,6 +314,7 @@ class ManyToOneDP(DependencyProcessor): dest = obj if dest is None or (not self.post_update and uowcommit.is_deleted(dest)): return + self._verify_canload(child) self.syncrules.execute(source, dest, obj, child, clearkeys) class ManyToManyDP(DependencyProcessor): @@ -379,6 +385,7 @@ class ManyToManyDP(DependencyProcessor): source = None if dest is None: return + self._verify_canload(child) self.syncrules.execute(source, dest, obj, child, clearkeys) class AssociationDP(OneToManyDP): diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 10fab3ba3c..e1fa56c650 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -956,6 +956,13 @@ class Mapper(object): return [self.get_attr_by_column(instance, column) for column in self.pks_by_table[self.mapped_table]] + def canload(self, instance): + """return true if this mapper is capable of loading the given instance""" + if self.polymorphic_on is not None: + return isinstance(instance, self.class_) + else: + return instance.__class__ is self.class_ + def instance_key(self, instance): """Deprecated. A synonym for `identity_key_from_instance`.""" diff --git a/test/orm/relationships.py b/test/orm/relationships.py index 8cd682da21..ab29fdf07d 100644 --- a/test/orm/relationships.py +++ b/test/orm/relationships.py @@ -498,6 +498,124 @@ class RelationTest4(testbase.ORMTest): sess.flush() assert a1 not in sess assert b1 not in sess + +class TypeMatchTest(testbase.ORMTest): + """test errors raised when trying to add items whose type is not handled by a relation""" + def define_tables(self, metadata): + global a, b, c, d + a = Table("a", metadata, + Column('id', Integer, primary_key=True), + Column('data', String(30))) + b = Table("b", metadata, + Column('id', Integer, primary_key=True), + Column("a_id", Integer, ForeignKey("a.id")), + Column('data', String(30))) + c = Table("c", metadata, + Column('id', Integer, primary_key=True), + Column("b_id", Integer, ForeignKey("b.id")), + Column('data', String(30))) + d = Table("d", metadata, + Column('id', Integer, primary_key=True), + Column("a_id", Integer, ForeignKey("a.id")), + Column('data', String(30))) + def test_o2m_oncascade(self): + class A(object):pass + class B(object):pass + class C(object):pass + mapper(A, a, properties={'bs':relation(B)}) + mapper(B, b) + mapper(C, c) + + a1 = A() + b1 = B() + c1 = C() + a1.bs.append(b1) + a1.bs.append(c1) + sess = create_session() + try: + sess.save(a1) + assert False + except exceptions.AssertionError, err: + assert str(err) == "Attribute 'bs' on class '%s' doesn't handle objects of type '%s'" % (A, C) + def test_o2m_onflush(self): + class A(object):pass + class B(object):pass + class C(object):pass + mapper(A, a, properties={'bs':relation(B, cascade="none")}) + mapper(B, b) + mapper(C, c) + + a1 = A() + b1 = B() + c1 = C() + a1.bs.append(b1) + a1.bs.append(c1) + sess = create_session() + sess.save(a1) + sess.save(b1) + sess.save(c1) + try: + sess.flush() + assert False + except exceptions.FlushError, err: + assert str(err) == "Attempting to flush an item of type %s on collection 'A.bs (B)', which is handled by mapper 'Mapper|B|b' and does not load items of that type. Did you mean to use a polymorphic mapper for this relationship ?" % C + def test_o2m_nopoly_onflush(self): + class A(object):pass + class B(object):pass + class C(B):pass + mapper(A, a, properties={'bs':relation(B, cascade="none")}) + mapper(B, b) + mapper(C, c, inherits=B) + + a1 = A() + b1 = B() + c1 = C() + a1.bs.append(b1) + a1.bs.append(c1) + sess = create_session() + sess.save(a1) + sess.save(b1) + sess.save(c1) + try: + sess.flush() + assert False + except exceptions.FlushError, err: + assert str(err) == "Attempting to flush an item of type %s on collection 'A.bs (B)', which is handled by mapper 'Mapper|B|b' and does not load items of that type. Did you mean to use a polymorphic mapper for this relationship ?" % C + def test_m2o_nopoly_onflush(self): + class A(object):pass + class B(A):pass + class D(object):pass + mapper(A, a) + mapper(B, b, inherits=A) + mapper(D, d, properties={"a":relation(A, cascade="none")}) + b1 = B() + d1 = D() + d1.a = b1 + sess = create_session() + sess.save(b1) + sess.save(d1) + try: + sess.flush() + assert False + except exceptions.FlushError, err: + assert str(err) == "Attempting to flush an item of type %s on collection 'D.a (A)', which is handled by mapper 'Mapper|A|a' and does not load items of that type. Did you mean to use a polymorphic mapper for this relationship ?" % B + def test_m2o_oncascade(self): + class A(object):pass + class B(object):pass + class D(object):pass + mapper(A, a) + mapper(B, b) + mapper(D, d, properties={"a":relation(A)}) + b1 = B() + d1 = D() + d1.a = b1 + sess = create_session() + try: + sess.save(d1) + assert False + except exceptions.AssertionError, err: + assert str(err) == "Attribute 'a' on class '%s' doesn't handle objects of type '%s'" % (D, B) + if __name__ == "__main__": testbase.main() -- 2.47.2