From c48177f0fed3a43b3b8b02c18243cb1664ce0abb Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 18 Aug 2006 17:21:01 +0000 Subject: [PATCH] - unit-of-work does a better check for "orphaned" objects that are part of a "delete-orphan" cascade, for certain conditions where the parent isnt available to cascade from. - it is now invalid to declare a self-referential relationship with "delete-orphan" (as the abovementioned check would make them impossible to save) - improved the check for objects being part of a session when the unit of work seeks to flush() them as part of a relationship.. --- CHANGES | 8 ++++ examples/adjacencytree/basic_tree.py | 2 +- examples/adjacencytree/byroot_tree.py | 2 +- lib/sqlalchemy/orm/mapper.py | 8 ++++ lib/sqlalchemy/orm/properties.py | 5 +++ lib/sqlalchemy/orm/session.py | 6 +++ lib/sqlalchemy/orm/unitofwork.py | 12 ++++-- test/orm/alltests.py | 1 + test/orm/cycles.py | 4 +- test/orm/session.py | 62 +++++++++++++++++++++++++++ 10 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 test/orm/session.py diff --git a/CHANGES b/CHANGES index 0639cdd7a6..6ba3a90d5d 100644 --- a/CHANGES +++ b/CHANGES @@ -8,6 +8,14 @@ step in case the given property references a non-compiled mapper (as it is using the sessioncontext plugin, etc), a lazy load operation will use that session by default if the parent object is not persistent with a session already. +- unit-of-work does a better check for "orphaned" objects that are +part of a "delete-orphan" cascade, for certain conditions where the +parent isnt available to cascade from. +- it is now invalid to declare a self-referential relationship with +"delete-orphan" (as the abovementioned check would make them impossible +to save) +- improved the check for objects being part of a session when the +unit of work seeks to flush() them as part of a relationship.. 0.2.7 - quoting facilities set up so that database-specific quoting can be diff --git a/examples/adjacencytree/basic_tree.py b/examples/adjacencytree/basic_tree.py index 4111337459..6a3470fa05 100644 --- a/examples/adjacencytree/basic_tree.py +++ b/examples/adjacencytree/basic_tree.py @@ -49,7 +49,7 @@ mapper(TreeNode, trees, properties=dict( id=trees.c.node_id, name=trees.c.node_name, parent_id=trees.c.parent_node_id, - children=relation(TreeNode, private=True, backref=backref("parent", foreignkey=trees.c.node_id)), + children=relation(TreeNode, cascade="all", backref=backref("parent", foreignkey=trees.c.node_id)), )) print "\n\n\n----------------------------" diff --git a/examples/adjacencytree/byroot_tree.py b/examples/adjacencytree/byroot_tree.py index 6342b5f273..d191710514 100644 --- a/examples/adjacencytree/byroot_tree.py +++ b/examples/adjacencytree/byroot_tree.py @@ -127,7 +127,7 @@ mapper(TreeNode, trees, properties=dict( parent_id=trees.c.parent_node_id, root_id=trees.c.root_node_id, root=relation(TreeNode, primaryjoin=trees.c.root_node_id==trees.c.node_id, foreignkey=trees.c.node_id, lazy=None, uselist=False), - children=relation(TreeNode, primaryjoin=trees.c.parent_node_id==trees.c.node_id, lazy=None, uselist=True, cascade="delete,delete-orphan,save-update"), + children=relation(TreeNode, primaryjoin=trees.c.parent_node_id==trees.c.node_id, lazy=None, uselist=True, cascade="delete,save-update"), data=relation(mapper(TreeData, treedata, properties=dict(id=treedata.c.data_id)), cascade="delete,delete-orphan,save-update", lazy=False) ), extension = TreeLoader()) diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index ab0226a41d..2347da2ba7 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -86,6 +86,7 @@ class Mapper(object): self.properties = properties or {} self.allow_column_override = allow_column_override self.allow_null_pks = allow_null_pks + self.delete_orphans = [] # a Column which is used during a select operation to retrieve the # "polymorphic identity" of the row, which indicates which Mapper should be used @@ -139,6 +140,13 @@ class Mapper(object): # of dependency #self.compile() + def _is_orphan(self, obj): + for (key,klass) in self.delete_orphans: + if not getattr(klass, key).hasparent(obj): + return True + else: + return False + def _get_props(self): self.compile() return self.__props diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index b5d66c160e..16760dc060 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -216,6 +216,11 @@ class PropertyLoader(mapper.MapperProperty): self.target = self.mapper.mapped_table + if self.cascade.delete_orphan: + if self.parent.class_ is self.mapper.class_: + raise exceptions.ArgumentError("Cant establish 'delete-orphan' cascade rule on a self-referential relationship. You probably want cascade='all', which includes delete cascading but not orphan detection.") + self.mapper.primary_mapper().delete_orphans.append((self.key, self.parent.class_)) + if self.secondaryjoin is not None and self.secondary is None: raise exceptions.ArgumentError("Property '" + self.key + "' specified with secondary join condition but no secondary argument") # if join conditions were not specified, figure them out based on foreign keys diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 5c962aa398..e5c95cbe4b 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -350,6 +350,12 @@ class Session(object): raise exceptions.InvalidRequestError("Instance '%s' is a detached instance or is already persistent in a different Session" % repr(object)) else: m = class_mapper(object.__class__, entity_name=kwargs.get('entity_name', None)) + + # this would be a nice exception to raise...however this is incompatible with a contextual + # session which puts all objects into the session upon construction. + #if m._is_orphan(object): + # raise exceptions.InvalidRequestError("Instance '%s' is an orphan, and must be attached to a parent object to be saved" % (repr(object))) + m._assign_entity_name(object) self._register_new(object) diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py index 33e0149e45..00d34a1048 100644 --- a/lib/sqlalchemy/orm/unitofwork.py +++ b/lib/sqlalchemy/orm/unitofwork.py @@ -106,9 +106,10 @@ class UnitOfWork(object): pass def _validate_obj(self, obj): - if hasattr(obj, '_instance_key') and not self.identity_map.has_key(obj._instance_key): - raise InvalidRequestError("Instance '%s' is not attached or pending within this session" % repr(obj._instance_key)) - + if (hasattr(obj, '_instance_key') and not self.identity_map.has_key(obj._instance_key)) or \ + (not hasattr(obj, '_instance_key') and obj not in self.new): + raise InvalidRequestError("Instance '%s' is not attached or pending within this session" % repr(obj)) + def update(self, obj): """called to add an object to this UnitOfWork as though it were loaded from the DB, but is actually coming from somewhere else, like a web session or similar.""" @@ -184,7 +185,10 @@ class UnitOfWork(object): continue if obj in self.deleted: continue - flush_context.register_object(obj) + if object_mapper(obj)._is_orphan(obj): + flush_context.register_object(obj, isdelete=True) + else: + flush_context.register_object(obj) for obj in self.deleted: if objset is not None and not obj in objset: diff --git a/test/orm/alltests.py b/test/orm/alltests.py index 6bcad2c151..f99df43b04 100644 --- a/test/orm/alltests.py +++ b/test/orm/alltests.py @@ -11,6 +11,7 @@ def suite(): 'orm.sessioncontext', 'orm.objectstore', + 'orm.session', 'orm.cascade', 'orm.relationships', 'orm.association', diff --git a/test/orm/cycles.py b/test/orm/cycles.py index 8a41613649..9d494020bf 100644 --- a/test/orm/cycles.py +++ b/test/orm/cycles.py @@ -44,7 +44,7 @@ class SelfReferentialTest(AssertMixin): class C1(Tester): pass m1 = mapper(C1, t1, properties = { - 'c1s':relation(C1, private=True), + 'c1s':relation(C1, cascade="all"), 'parent':relation(C1, primaryjoin=t1.c.parent_c1==t1.c.c1, foreignkey=t1.c.c1, lazy=True, uselist=False) }) a = C1('head c1') @@ -62,7 +62,7 @@ class SelfReferentialTest(AssertMixin): pass m1 = mapper(C1, t1, properties = { - 'c1s' : relation(C1, private=True), + 'c1s' : relation(C1, cascade="all"), 'c2s' : relation(mapper(C2, t2), private=True) }) diff --git a/test/orm/session.py b/test/orm/session.py new file mode 100644 index 0000000000..4bc5264d15 --- /dev/null +++ b/test/orm/session.py @@ -0,0 +1,62 @@ +from testbase import AssertMixin +import testbase +import unittest, sys, datetime + +import tables +from tables import * + +db = testbase.db +from sqlalchemy import * + + +class SessionTest(AssertMixin): + + def setUpAll(self): + db.echo = False + tables.create() + tables.data() + db.echo = testbase.echo + def tearDownAll(self): + db.echo = False + tables.drop() + db.echo = testbase.echo + def tearDown(self): + clear_mappers() + def setUp(self): + pass + + def test_orphan(self): + mapper(Address, addresses) + mapper(User, users, properties=dict( + addresses=relation(Address, cascade="all,delete-orphan", backref="user") + )) + s = create_session() + a = Address() + try: + s.save(a) + except exceptions.InvalidRequestError, e: + pass + s.flush() + assert a.address_id is None, "Error: address should not be persistent" + + def test_delete_new_object(self): + mapper(Address, addresses) + mapper(User, users, properties=dict( + addresses=relation(Address, cascade="all,delete-orphan", backref="user") + )) + s = create_session() + + u = User() + s.save(u) + a = Address() + assert a not in s.new + u.addresses.append(a) + u.addresses.remove(a) + s.delete(u) + s.flush() # (erroneously) causes "a" to be persisted + assert u.user_id is None, "Error: user should not be persistent" + assert a.address_id is None, "Error: address should not be persistent" + + +if __name__ == "__main__": + testbase.main() -- 2.47.2