From: Mike Bayer Date: Sun, 3 Sep 2006 02:46:46 +0000 (+0000) Subject: cleanup/unit test fixes X-Git-Tag: rel_0_2_8~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c0d89919ec871c69bbfa32ef83417be61be2291b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git cleanup/unit test fixes --- diff --git a/lib/sqlalchemy/attributes.py b/lib/sqlalchemy/attributes.py index 2a98a5f4da..b29b03aa8f 100644 --- a/lib/sqlalchemy/attributes.py +++ b/lib/sqlalchemy/attributes.py @@ -39,8 +39,7 @@ class InstrumentedAttribute(object): def sethasparent(self, item, value): """sets a boolean flag on the given item corresponding to whether or not it is attached to a parent object via the attribute represented by this InstrumentedAttribute.""" - if item is not None: - item._state[('hasparent', id(self))] = value + item._state[('hasparent', id(self))] = value def get_history(self, obj, passive=False): """return a new AttributeHistory object for the given object/this attribute's key. @@ -55,9 +54,17 @@ class InstrumentedAttribute(object): return AttributeHistory(self, obj, current, passive=passive) def set_callable(self, obj, callable_): - """sets a callable function on the given object which will be executed when this attribute - is next accessed. if the callable is None, then initializes the attribute with an empty value - (which overrides any class-level callables that might be on this attribute.)""" + """set a callable function for this attribute on the given object. + + this callable will be executed when the attribute is next accessed, + and is assumed to construct part of the instances previously stored state. When + its value or values are loaded, they will be established as part of the + instance's "committed state". while "trackparent" information will be assembled + for these instances, attribute-level event handlers will not be fired. + + the callable overrides the class level callable set in the InstrumentedAttribute + constructor. + """ if callable_ is None: self.initialize(obj) else: @@ -105,6 +112,10 @@ class InstrumentedAttribute(object): return data def initialize(self, obj): + """initialize this attribute on the given object instance. + + if this is a list-based attribute, a new, blank list will be created. + if a scalar attribute, the value will be initialized to None.""" if self.uselist: l = InstrumentedList(self, obj, self._blank_list()) obj.__dict__[self.key] = l @@ -242,7 +253,7 @@ class InstrumentedAttribute(object): def append_event(self, event, obj, value): """called by InstrumentedList when an item is appended""" obj._state['modified'] = True - if self.trackparent: + if self.trackparent and value is not None: self.sethasparent(value, True) for ext in self.extensions: ext.append(event or self, obj, value) @@ -250,7 +261,7 @@ class InstrumentedAttribute(object): def remove_event(self, event, obj, value): """called by InstrumentedList when an item is removed""" obj._state['modified'] = True - if self.trackparent: + if self.trackparent and value is not None: self.sethasparent(value, False) for ext in self.extensions: ext.delete(event or self, obj, value) @@ -262,7 +273,10 @@ class InstrumentedList(object): note that this list does a lot less than earlier versions of SA list-based attributes, which used HistoryArraySet. this list wrapper does *not* maintain setlike semantics, meaning you can add as many duplicates as - you want (which can break a lot of SQL), and also does not do anything related to history tracking.""" + you want (which can break a lot of SQL), and also does not do anything related to history tracking. + + Please see ticket #213 for information on the future of this class, where it will be broken out into more + collection-specific subtypes.""" def __init__(self, attr, obj, data, init=True): self.attr = attr # this weakref is to prevent circular references between the parent object @@ -463,24 +477,22 @@ class CommittedState(object): self.commit_attribute(attr, obj) def commit_attribute(self, attr, obj, value=False): - if attr.uselist: - if value is not False: + """establish the value of attribute 'attr' on instance 'obj' as "committed". + + this corresponds to a previously saved state being restored. """ + if value is False: + if obj.__dict__.has_key(attr.key): + value = obj.__dict__[attr.key] + if value is not False: + if attr.uselist: self.data[attr.key] = [x for x in value] if attr.trackparent: - [attr.sethasparent(x, True) for x in self.data[attr.key]] - elif obj.__dict__.has_key(attr.key): - self.data[attr.key] = [x for x in obj.__dict__[attr.key]] - if attr.trackparent: - [attr.sethasparent(x, True) for x in self.data[attr.key]] - else: - if value is not False: + [attr.sethasparent(x, True) for x in self.data[attr.key] if x is not None] + else: self.data[attr.key] = value - if attr.trackparent: - attr.sethasparent(self.data[attr.key], True) - elif obj.__dict__.has_key(attr.key): - self.data[attr.key] = obj.__dict__[attr.key] - if attr.trackparent: - attr.sethasparent(self.data[attr.key], True) + if attr.trackparent and value is not None: + attr.sethasparent(value, True) + def rollback(self, manager, obj): for attr in manager.managed_attributes(obj.__class__): if self.data.has_key(attr.key): diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index e8f3ad7dde..fd9841f336 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -580,7 +580,6 @@ class EagerLoader(LazyLoader): else: towrap = self.localparent.mapped_table - # print "hello, towrap", str(towrap) if self.secondaryjoin is not None: statement._outerjoin = sql.outerjoin(towrap, self.eagersecondary, self.eagerprimary).outerjoin(self.eagertarget, self.eagersecondaryjoin) if self.order_by is False and self.secondary.default_order_by() is not None: @@ -623,8 +622,6 @@ class EagerLoader(LazyLoader): # call _instance on the row, even though the object has been created, # so that we further descend into properties self.mapper._instance(session, decorated_row, imap, None) - - return else: if isnew: # call the SmartProperty's initialize() method to create a new, blank list @@ -636,7 +633,7 @@ class EagerLoader(LazyLoader): # store it in the "scratch" area, which is local to this load operation. imap['_scratch'][(instance, self.key)] = appender result_list = imap['_scratch'][(instance, self.key)] - self.mapper._instance(session, decorated_row, imap, result_list) + self.mapper._instance(session, decorated_row, imap, result_list) def _create_decorator_row(self): class DecoratorDict(object): diff --git a/test/orm/session.py b/test/orm/session.py index 085557021c..263e5c9f72 100644 --- a/test/orm/session.py +++ b/test/orm/session.py @@ -53,7 +53,11 @@ class OrphanDeletionTest(AssertMixin): u.addresses.append(a) u.addresses.remove(a) s.delete(u) - s.flush() # (erroneously) causes "a" to be persisted + try: + s.flush() # (erroneously) causes "a" to be persisted + assert False + except exceptions.FlushError: + assert True assert u.user_id is None, "Error: user should not be persistent" assert a.address_id is None, "Error: address should not be persistent" @@ -109,7 +113,11 @@ class CascadingOrphanDeletionTest(AssertMixin): order.items.append(item) order.items.remove(item) # item is an orphan, but attr is not so flush() tries to save attr - s.flush() + try: + s.flush() + assert False + except exceptions.FlushError: + assert True assert item.id is None assert attr.id is None