- 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]

diff --git a/CHANGES b/CHANGES
index e554003e526c014369ae407e7ddc0ed2e5d4df60..e830654e40f28d2412620d0c01a84c458971c830 100644 (file)
--- 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 5a756f5d27a517f4dc5c131065091f667bf34136..0b3b70d219f86bd44ed63fc65cb6cc32a18dc128 100644 (file)
--- 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 10fab3ba3cf41bd6689be0d13f8a67569cd39133..e1fa56c650e0917102fa546fca8dc1b9a463c179 100644 (file)
--- 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 8cd682da21da7e002a53722c62b61cca718deeaa..ab29fdf07d2d5e81b334c658feb5d677d3e943cf 100644 (file)
--- 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()