generates columns to put up onto the base.
- orm
+ - [bug] Fixed backref behavior when "popping" the
+ value off of a many-to-one in response to
+ a removal from a stale one-to-many - the operation
+ is skipped, since the many-to-one has since
+ been updated. [ticket:2315]
+
+ - [bug] After some years of not doing this, added
+ more granularity to the "is X a parent of Y"
+ functionality, which is used when determining
+ if the FK on "Y" needs to be "nulled out" as well
+ as if "Y" should be deleted with delete-orphan
+ cascade. The test now takes into account the
+ Python identity of the parent as well its identity
+ key, to see if the last known parent of Y is
+ definitely X. If a decision
+ can't be made, a StaleDataError is raised. The
+ conditions where this error is raised are fairly
+ rare, requiring that the previous parent was
+ garbage collected, and previously
+ could very well inappropriately update/delete
+ a record that's since moved onto a new parent,
+ though there may be some cases where
+ "silent success" occurred previously that will now
+ raise in the face of ambiguity.
+ Expiring "Y" resets the "parent" tracker, meaning
+ X.remove(Y) could then end up deleting Y even
+ if X is stale, but this is the same behavior
+ as before; it's advised to expire X also in that
+ case. [ticket:2264]
+
- [bug] fixed inappropriate evaluation of user-mapped
object in a boolean context within query.get()
[ticket:2310]. Also in 0.6.9.
from operator import itemgetter
from sqlalchemy import util, event, exc as sa_exc
-from sqlalchemy.orm import interfaces, collections, events
+from sqlalchemy.orm import interfaces, collections, events, exc as orm_exc
mapperutil = util.importlater("sqlalchemy.orm", "util")
return op(other, self.comparator, **kwargs)
def hasparent(self, state, optimistic=False):
- return self.impl.hasparent(state, optimistic=optimistic)
+ return self.impl.hasparent(state, optimistic=optimistic) is not False
def __getattr__(self, key):
try:
will also not have a `hasparent` flag.
"""
- return state.parents.get(id(self.parent_token), optimistic)
+ assert self.trackparent, "This AttributeImpl is not configured to track parents."
- def sethasparent(self, state, value):
+ return state.parents.get(id(self.parent_token), optimistic) \
+ is not False
+
+ def sethasparent(self, state, parent_state, value):
"""Set 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``.
"""
- state.parents[id(self.parent_token)] = value
+ assert self.trackparent, "This AttributeImpl is not configured to track parents."
+
+ id_ = id(self.parent_token)
+ if value:
+ state.parents[id_] = parent_state
+ else:
+ if id_ in state.parents:
+ last_parent = state.parents[id_]
+
+ if last_parent is not False and \
+ last_parent.key != parent_state.key:
+
+ if last_parent.obj() is None:
+ raise orm_exc.StaleDataError(
+ "Removing state %s from parent "
+ "state %s along attribute '%s', "
+ "but the parent record "
+ "has gone stale, can't be sure this "
+ "is the most recent parent." %
+ (mapperutil.state_str(state),
+ mapperutil.state_str(parent_state),
+ self.key))
+
+ return
+
+ state.parents[id_] = False
+
def set_callable(self, state, callable_):
"""Set a callable function for this attribute on the given object.
self.set(state, dict_, value, initiator, passive=passive)
def remove(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
- self.set(state, dict_, None, initiator, passive=passive)
+ self.set(state, dict_, None, initiator,
+ passive=passive, check_old=value)
+
+ def pop(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ self.set(state, dict_, None, initiator,
+ passive=passive, check_old=value, pop=True)
- def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ def set(self, state, dict_, value, initiator,
+ passive=PASSIVE_OFF, check_old=None, pop=False):
raise NotImplementedError()
def get_committed_value(self, state, dict_, passive=PASSIVE_OFF):
return History.from_scalar_attribute(
self, state, dict_.get(self.key, NO_VALUE))
- def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ def set(self, state, dict_, value, initiator,
+ passive=PASSIVE_OFF, check_old=None, pop=False):
if initiator and initiator.parent_token is self.parent_token:
return
ScalarAttributeImpl.delete(self, state, dict_)
state.mutable_dict.pop(self.key)
- def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
- ScalarAttributeImpl.set(self, state, dict_, value, initiator, passive)
+ def set(self, state, dict_, value, initiator,
+ passive=PASSIVE_OFF, check_old=None, pop=False):
+ ScalarAttributeImpl.set(self, state, dict_, value,
+ initiator, passive, check_old=check_old, pop=pop)
state.mutable_dict[self.key] = value
else:
return []
- def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ def set(self, state, dict_, value, initiator,
+ passive=PASSIVE_OFF, check_old=None, pop=False):
"""Set a value on the given InstanceState.
`initiator` is the ``InstrumentedAttribute`` that initiated the
else:
old = self.get(state, dict_, passive=PASSIVE_NO_FETCH)
+ if check_old is not None and \
+ old is not PASSIVE_NO_RESULT and \
+ check_old is not old:
+ if pop:
+ return
+ else:
+ raise ValueError(
+ "Object %s not associated with %s on attribute '%s'" % (
+ mapperutil.instance_str(check_old),
+ mapperutil.state_str(state),
+ self.key
+ ))
value = self.fire_replace_event(state, dict_, value, old, initiator)
dict_[self.key] = value
def fire_remove_event(self, state, dict_, value, initiator):
if self.trackparent and value is not None:
- self.sethasparent(instance_state(value), False)
+ self.sethasparent(instance_state(value), state, False)
for fn in self.dispatch.remove:
fn(state, value, initiator or self)
if (previous is not value and
previous is not None and
previous is not PASSIVE_NO_RESULT):
- self.sethasparent(instance_state(previous), False)
+ self.sethasparent(instance_state(previous), state, False)
for fn in self.dispatch.set:
value = fn(state, value, previous, initiator or self)
if self.trackparent:
if value is not None:
- self.sethasparent(instance_state(value), True)
+ self.sethasparent(instance_state(value), state, True)
return value
state.modified_event(dict_, self, NEVER_SET, True)
if self.trackparent and value is not None:
- self.sethasparent(instance_state(value), True)
+ self.sethasparent(instance_state(value), state, True)
return value
def fire_remove_event(self, state, dict_, value, initiator):
if self.trackparent and value is not None:
- self.sethasparent(instance_state(value), False)
+ self.sethasparent(instance_state(value), state, False)
for fn in self.dispatch.remove:
fn(state, value, initiator or self)
else:
collection.remove_with_event(value, initiator)
- def set(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ def pop(self, state, dict_, value, initiator, passive=PASSIVE_OFF):
+ try:
+ # TODO: better solution here would be to add
+ # a "popper" role to collections.py to complement
+ # "remover".
+ self.remove(state, dict_, value, initiator, passive=passive)
+ except (ValueError, KeyError, IndexError):
+ pass
+
+ def set(self, state, dict_, value, initiator,
+ passive=PASSIVE_OFF, pop=False):
"""Set a value on the given object.
`initiator` is the ``InstrumentedAttribute`` that initiated the
old_state, old_dict = instance_state(oldchild),\
instance_dict(oldchild)
impl = old_state.manager[key].impl
- try:
- impl.remove(old_state,
- old_dict,
- state.obj(),
- initiator, passive=PASSIVE_NO_FETCH)
- except (ValueError, KeyError, IndexError):
- pass
+ impl.pop(old_state,
+ old_dict,
+ state.obj(),
+ initiator, passive=PASSIVE_NO_FETCH)
if child is not None:
child_state, child_dict = instance_state(child),\
if child is not None:
child_state, child_dict = instance_state(child),\
instance_dict(child)
- child_state.manager[key].impl.remove(
+ child_state.manager[key].impl.pop(
child_state,
child_dict,
state.obj(),
value = fn(state, value, initiator or self)
if self.trackparent and value is not None:
- self.sethasparent(attributes.instance_state(value), True)
+ self.sethasparent(attributes.instance_state(value), state, True)
def fire_remove_event(self, state, dict_, value, initiator):
collection_history = self._modified_event(state, dict_)
collection_history.deleted_items.append(value)
if self.trackparent and value is not None:
- self.sethasparent(attributes.instance_state(value), False)
+ self.sethasparent(attributes.instance_state(value), state, False)
for fn in self.dispatch.remove:
fn(state, value, initiator or self)
return state.committed_state[self.key]
def set(self, state, dict_, value, initiator,
- passive=attributes.PASSIVE_OFF):
+ passive=attributes.PASSIVE_OFF,
+ check_old=None, pop=False):
if initiator and initiator.parent_token is self.parent_token:
return
+ if pop and value is None:
+ return
self._set_iterable(state, dict_, value)
+
def _set_iterable(self, state, dict_, iterable, adapter=None):
collection_history = self._modified_event(state, dict_)
new_values = list(iterable)
class StaleDataError(sa.exc.SQLAlchemyError):
"""An operation encountered database state that is unaccounted for.
- Two conditions cause this to happen:
+ Conditions which cause this to happen include:
* A flush may have attempted to update or delete rows
and an unexpected number of rows were matched during
* A mapped object with version_id_col was refreshed,
and the version number coming back from the database does
not match that of the object itself.
+
+ * A object is detached from its parent object, however
+ the object was previously attached to a different parent
+ identity which was garbage collected, and a decision
+ cannot be made if the new parent was really the most
+ recent "parent" (new in 0.7.4).
"""
def __getstate__(self):
d = {'instance':self.obj()}
-
d.update(
(k, self.__dict__[k]) for k in (
- 'committed_state', 'pending', 'parents', 'modified', 'expired',
- 'callables', 'key', 'load_options', 'mutable_dict'
+ 'committed_state', 'pending', 'modified', 'expired',
+ 'callables', 'key', 'parents', 'load_options', 'mutable_dict',
+ 'class_',
) if k in self.__dict__
)
if self.load_path:
def __setstate__(self, state):
from sqlalchemy.orm import instrumentation
- self.obj = weakref.ref(state['instance'], self._cleanup)
- self.class_ = state['instance'].__class__
+ inst = state['instance']
+ if inst is not None:
+ self.obj = weakref.ref(inst, self._cleanup)
+ self.class_ = inst.__class__
+ else:
+ # None being possible here generally new as of 0.7.4
+ # due to storage of state in "parents". "class_"
+ # also new.
+ self.obj = None
+ self.class_ = state['class_']
self.manager = manager = instrumentation.manager_of_class(self.class_)
if manager is None:
raise orm_exc.UnmappedInstanceError(
- state['instance'],
+ inst,
"Cannot deserialize object of type %r - no mapper() has"
" been configured for this class within the current Python process!" %
self.class_)
self.callables = state.get('callables', {})
if self.modified:
- self._strong_obj = state['instance']
+ self._strong_obj = inst
self.__dict__.update([
(k, state[k]) for k in (
self.modified = False
- pending = self.__dict__.get('pending', None)
- mutable_dict = self.mutable_dict
self.committed_state.clear()
- if mutable_dict:
- mutable_dict.clear()
- if pending:
- pending.clear()
+
+ self.__dict__.pop('pending', None)
+ self.__dict__.pop('mutable_dict', None)
+
+ # clear out 'parents' collection. not
+ # entirely clear how we can best determine
+ # which to remove, or not.
+ self.__dict__.pop('parents', None)
for key in self.manager:
impl = self.manager[key].impl
compare_function=compare_function,
useobject=useobject,
extension=attribute_ext,
- trackparent=useobject,
+ trackparent=useobject and (prop.single_parent or prop.direction is interfaces.ONETOMANY),
typecallable=typecallable,
callable_=callable_,
active_history=active_history,
def single_parent_validator(desc, prop):
def _do_check(state, value, oldvalue, initiator):
- if value is not None:
+ if value is not None and initiator.key == prop.key:
hasparent = initiator.hasparent(attributes.instance_state(value))
if hasparent and oldvalue is not value:
raise sa_exc.InvalidRequestError(
MyTest2 = None
+class AttributeImplAPITest(fixtures.MappedTest):
+ def _scalar_obj_fixture(self):
+ class A(object):
+ pass
+ class B(object):
+ pass
+ instrumentation.register_class(A)
+ instrumentation.register_class(B)
+ attributes.register_attribute(A, "b", uselist=False, useobject=True)
+ return A, B
+
+ def _collection_obj_fixture(self):
+ class A(object):
+ pass
+ class B(object):
+ pass
+ instrumentation.register_class(A)
+ instrumentation.register_class(B)
+ attributes.register_attribute(A, "b", uselist=True, useobject=True)
+ return A, B
+
+ def test_scalar_obj_remove_invalid(self):
+ A, B = self._scalar_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+ b2 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b is b1
+
+ assert_raises_message(
+ ValueError,
+ "Object <B at .*?> not "
+ "associated with <A at .*?> on attribute 'b'",
+ A.b.impl.remove,
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b2, None
+ )
+
+ def test_scalar_obj_pop_invalid(self):
+ A, B = self._scalar_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+ b2 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b is b1
+
+ A.b.impl.pop(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b2, None
+ )
+ assert a1.b is b1
+
+ def test_scalar_obj_pop_valid(self):
+ A, B = self._scalar_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b is b1
+
+ A.b.impl.pop(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+ assert a1.b is None
+
+ def test_collection_obj_remove_invalid(self):
+ A, B = self._collection_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+ b2 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b == [b1]
+
+ assert_raises_message(
+ ValueError,
+ r"list.remove\(x\): x not in list",
+ A.b.impl.remove,
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b2, None
+ )
+
+ def test_collection_obj_pop_invalid(self):
+ A, B = self._collection_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+ b2 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b == [b1]
+
+ A.b.impl.pop(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b2, None
+ )
+ assert a1.b == [b1]
+
+ def test_collection_obj_pop_valid(self):
+ A, B = self._collection_obj_fixture()
+
+ a1 = A()
+ b1 = B()
+
+ A.b.impl.append(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+
+ assert a1.b == [b1]
+
+ A.b.impl.pop(
+ attributes.instance_state(a1),
+ attributes.instance_dict(a1), b1, None
+ )
+ assert a1.b == []
+
+
class AttributesTest(fixtures.ORMTest):
def setup(self):
global MyTest, MyTest2
assert attributes.has_parent(Blog, p2, 'posts')
assert attributes.has_parent(Post, b2, 'blog')
+ def test_illegal_trackparent(self):
+ class Post(object):pass
+ class Blog(object):pass
+ instrumentation.register_class(Post)
+ instrumentation.register_class(Blog)
+
+ attributes.register_attribute(Post, 'blog', useobject=True)
+ assert_raises_message(
+ AssertionError,
+ "This AttributeImpl is not configured to track parents.",
+ attributes.has_parent, Post, Blog(), 'blog'
+ )
+ assert_raises_message(
+ AssertionError,
+ "This AttributeImpl is not configured to track parents.",
+ Post.blog.impl.sethasparent, "x", "x", True
+ )
+
+
def test_inheritance(self):
"""tests that attributes are polymorphic"""
sess.commit()
assert i1.keyword is None
assert i2.keyword is k1
+
+class O2MStaleBackrefTest(_fixtures.FixtureTest):
+ run_inserts = None
+
+ @classmethod
+ def setup_mappers(cls):
+ Address, addresses, users, User = (cls.classes.Address,
+ cls.tables.addresses,
+ cls.tables.users,
+ cls.classes.User)
+
+ mapper(Address, addresses)
+ mapper(User, users, properties = dict(
+ addresses = relationship(Address, backref="user"),
+ ))
+
+
+ def test_backref_pop_m2o(self):
+ User, Address = self.classes.User, self.classes.Address
+
+ u1 = User()
+ u2 = User()
+ a1 = Address()
+ u1.addresses.append(a1)
+ u2.addresses.append(a1)
+
+ # events haven't updated
+ # u1.addresses here.
+ u1.addresses.remove(a1)
+
+ assert a1.user is u2
+ assert a1 in u2.addresses
+
+class M2MStaleBackrefTest(_fixtures.FixtureTest):
+ run_inserts = None
+
+ @classmethod
+ def setup_mappers(cls):
+ keywords, items, item_keywords, Keyword, Item = (cls.tables.keywords,
+ cls.tables.items,
+ cls.tables.item_keywords,
+ cls.classes.Keyword,
+ cls.classes.Item)
+
+ mapper(Item, items, properties={
+ 'keywords':relationship(Keyword, secondary=item_keywords,
+ backref='items')
+ })
+ mapper(Keyword, keywords)
+
+ def test_backref_pop_m2m(self):
+ Keyword, Item = self.classes.Keyword, self.classes.Item
+
+ k1 = Keyword()
+ k2 = Keyword()
+ i1 = Item()
+ k1.items.append(i1)
+ k2.items.append(i1)
+ k2.items.append(i1)
+ i1.keywords = []
+ k2.items.remove(i1)
+ assert len(k2.items) == 0
--- /dev/null
+"""test the current state of the hasparent() flag."""
+
+
+from test.lib.testing import assert_raises, assert_raises_message
+from sqlalchemy import Integer, String, ForeignKey, Sequence, \
+ exc as sa_exc
+from test.lib.schema import Table, Column
+from sqlalchemy.orm import mapper, relationship, create_session, \
+ sessionmaker, class_mapper, backref, Session
+from sqlalchemy.orm import attributes, exc as orm_exc
+from test.lib import testing
+from test.lib.testing import eq_
+from test.lib import fixtures
+from test.orm import _fixtures
+from test.lib.util import gc_collect
+
+
+class ParentRemovalTest(fixtures.MappedTest):
+ """Test that the 'hasparent' flag gets flipped to False
+ only if we're sure this object is the real parent.
+
+ In ambiguous cases a stale data exception is
+ raised.
+
+ """
+ run_inserts = None
+
+ @classmethod
+ def define_tables(cls, metadata):
+ Table('users', metadata,
+ Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+ )
+ Table('addresses', metadata,
+ Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+ Column('user_id', Integer, ForeignKey('users.id')),
+ )
+
+ @classmethod
+ def setup_classes(cls):
+ class User(cls.Comparable):
+ pass
+ class Address(cls.Comparable):
+ pass
+
+ @classmethod
+ def setup_mappers(cls):
+ mapper(cls.classes.Address, cls.tables.addresses)
+ mapper(cls.classes.User, cls.tables.users, properties={
+ 'addresses':relationship(cls.classes.Address,
+ cascade='all, delete-orphan'),
+
+ })
+
+ def _assert_hasparent(self, a1):
+ assert attributes.has_parent(
+ self.classes.User, a1, "addresses")
+
+ def _assert_not_hasparent(self, a1):
+ assert not attributes.has_parent(
+ self.classes.User, a1, "addresses")
+
+ def _fixture(self):
+ User, Address = self.classes.User, self.classes.Address
+
+ s = Session()
+
+ u1 = User()
+ a1 = Address()
+ u1.addresses.append(a1)
+ s.add(u1)
+ s.flush()
+ return s, u1, a1
+
+ def test_stale_state_positive(self):
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ s.expunge(u1)
+
+ u1 = s.query(User).first()
+ u1.addresses.remove(a1)
+
+ self._assert_not_hasparent(a1)
+
+ def test_stale_state_positive_gc(self):
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ s.expunge(u1)
+ del u1
+ gc_collect()
+
+ u1 = s.query(User).first()
+ u1.addresses.remove(a1)
+
+ self._assert_not_hasparent(a1)
+
+ def test_stale_state_positive_pk_change(self):
+ """Illustrate that we can't easily link a
+ stale state to a fresh one if the fresh one has
+ a PK change (unless we a. tracked all the previous PKs,
+ wasteful, or b. recycled states - time consuming,
+ breaks lots of edge cases, destabilizes the code)
+
+ """
+
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ s._expunge_state(attributes.instance_state(u1))
+ del u1
+ gc_collect()
+
+ u1 = s.query(User).first()
+
+ # primary key change. now we
+ # can't rely on state.key as the
+ # identifier.
+ u1.id = 5
+ a1.user_id = 5
+ s.flush()
+
+ assert_raises_message(
+ orm_exc.StaleDataError,
+ "can't be sure this is the most recent parent.",
+ u1.addresses.remove, a1
+ )
+
+ # unfortunately, u1.addresses was impacted
+ # here
+ assert u1.addresses == []
+
+ # expire all and we can continue
+ s.expire_all()
+ u1.addresses.remove(a1)
+
+ self._assert_not_hasparent(a1)
+
+ def test_stale_state_negative_child_expired(self):
+ """illustrate the current behavior of
+ expiration on the child.
+
+ there's some uncertainty here in how
+ this use case should work.
+
+ """
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ u2 = User(addresses=[a1])
+
+ s.expire(a1)
+ u1.addresses.remove(a1)
+
+ # controversy here. The action is
+ # to expire one object, not the other, and remove;
+ # this is pretty abusive in any case. for now
+ # we are expiring away the 'parents' collection
+ # so the remove will unset the hasparent flag.
+ # this is what has occurred historically in any case.
+ self._assert_not_hasparent(a1)
+ #self._assert_hasparent(a1)
+
+ def test_stale_state_negative(self):
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ u2 = User(addresses=[a1])
+ s.add(u2)
+ s.flush()
+ s._expunge_state(attributes.instance_state(u2))
+ del u2
+ gc_collect()
+
+ assert_raises_message(
+ orm_exc.StaleDataError,
+ "can't be sure this is the most recent parent.",
+ u1.addresses.remove, a1
+ )
+
+ s.flush()
+ self._assert_hasparent(a1)
+
+ def test_fresh_state_positive(self):
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ self._assert_hasparent(a1)
+
+ def test_fresh_state_negative(self):
+ User = self.classes.User
+ s, u1, a1 = self._fixture()
+
+ u1.addresses.remove(a1)
+
+ self._assert_not_hasparent(a1)
+
+
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
