From: Mike Bayer Date: Mon, 21 Jan 2013 23:17:10 +0000 (-0500) Subject: Fixed the (most likely never used) "@collection.link" collection X-Git-Tag: rel_0_8_0~28^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b1df6fab53a0d740fe60f04e5c9ad01027ba59af;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fixed the (most likely never used) "@collection.link" collection method, which fires off each time the collection is associated or de-associated with a mapped object - the decorator was not tested or functional. The decorator method is now named :meth:`.collection.linker` though the name "link" remains for backwards compatibility. Courtesy Luca Wehrstedt. [ticket:2653] --- b1df6fab53a0d740fe60f04e5c9ad01027ba59af diff --cc doc/build/changelog/changelog_08.rst index bd175b5300,c9b66be7ec..052dfaaf1f --- a/doc/build/changelog/changelog_08.rst +++ b/doc/build/changelog/changelog_08.rst @@@ -6,28 -6,6 +6,39 @@@ .. changelog:: :version: 0.8.0 ++ .. change:: ++ :tags: orm, bug ++ :tickets: 2653 ++ ++ Fixed the (most likely never used) "@collection.link" collection ++ method, which fires off each time the collection is associated ++ or de-associated with a mapped object - the decorator ++ was not tested or functional. The decorator method ++ is now named :meth:`.collection.linker` though the name "link" ++ remains for backwards compatibility. Courtesy Luca Wehrstedt. ++ + .. change:: + :tags: orm, bug + :tickets: 2654 + + Made some fixes to the system of producing custom instrumented + collections, mainly that the usage of the @collection decorators + will now honor the __mro__ of the given class, applying the + logic of the sub-most classes' version of a particular collection + method. Previously, it wasn't predictable when subclassing + an existing instrumented class such as :class:`.MappedCollection` + whether or not custom methods would resolve correctly. + + .. change:: + :tags: orm, removed + + The undocumented (and hopefully unused) system of producing + custom collections using an ``__instrumentation__`` datastructure + associated with the collection has been removed, as this was a complex + and untested feature which was also essentially redundant versus the + decorator approach. Other internal simplifcations to the + orm.collections module have been made as well. + .. change:: :tags: mssql, feature :pullreq: 35 diff --cc lib/sqlalchemy/orm/collections.py index c37f9b135a,3047a5d268..5691acfffb --- a/lib/sqlalchemy/orm/collections.py +++ b/lib/sqlalchemy/orm/collections.py @@@ -418,8 -419,8 +418,8 @@@ class collection(object) return fn @staticmethod - def link(fn): - """Tag the method as a the "linked to attribute" event handler. + def linker(fn): - """Tag the method as a the "linked to attribute" event handler. ++ """Tag the method as a "linked to attribute" event handler. This optional event handler will be called when the collection class is linked to or unlinked from the InstrumentedAttribute. It is @@@ -428,9 -429,9 +428,12 @@@ that has been linked, or None if unlinking. """ - setattr(fn, '_sa_instrument_role', 'link') + setattr(fn, '_sa_instrument_role', 'linker') return fn ++ link = linker ++ """deprecated; synonym for :meth:`.collection.linker`.""" ++ @staticmethod def converter(fn): """Tag the method as the collection converter. @@@ -874,55 -900,53 +877,55 @@@ def _instrument_class(cls) "Can not instrument a built-in type. Use a " "subclass, even a trivial one.") + roles = {} + methods = {} + + # search for _sa_instrument_role-decorated methods in + # method resolution order, assign to roles + for supercls in cls.__mro__: + for name, method in vars(supercls).items(): + if not util.callable(method): + continue + + # note role declarations + if hasattr(method, '_sa_instrument_role'): + role = method._sa_instrument_role + assert role in ('appender', 'remover', 'iterator', - 'link', 'converter') ++ 'linker', 'converter') + roles.setdefault(role, name) + + # transfer instrumentation requests from decorated function + # to the combined queue + before, after = None, None + if hasattr(method, '_sa_instrument_before'): + op, argument = method._sa_instrument_before + assert op in ('fire_append_event', 'fire_remove_event') + before = op, argument + if hasattr(method, '_sa_instrument_after'): + op = method._sa_instrument_after + assert op in ('fire_append_event', 'fire_remove_event') + after = op + if before: + methods[name] = before[0], before[1], after + elif after: + methods[name] = None, None, after + + # see if this class has "canned" roles based on a known + # collection type (dict, set, list). Apply those roles + # as needed to the "roles" dictionary, and also + # prepare "decorator" methods collection_type = util.duck_type_collection(cls) if collection_type in __interfaces: - roles = __interfaces[collection_type].copy() - decorators = roles.pop('_decorators', {}) - else: - roles, decorators = {}, {} - - if hasattr(cls, '__instrumentation__'): - roles.update(copy.deepcopy(getattr(cls, '__instrumentation__'))) - - methods = roles.pop('methods', {}) - - for name in dir(cls): - method = getattr(cls, name, None) - if not util.callable(method): - continue - - # note role declarations - if hasattr(method, '_sa_instrument_role'): - role = method._sa_instrument_role - assert role in ('appender', 'remover', 'iterator', - 'linker', 'converter') - roles[role] = name - - # transfer instrumentation requests from decorated function - # to the combined queue - before, after = None, None - if hasattr(method, '_sa_instrument_before'): - op, argument = method._sa_instrument_before - assert op in ('fire_append_event', 'fire_remove_event') - before = op, argument - if hasattr(method, '_sa_instrument_after'): - op = method._sa_instrument_after - assert op in ('fire_append_event', 'fire_remove_event') - after = op - if before: - methods[name] = before[0], before[1], after - elif after: - methods[name] = None, None, after - - # apply ABC auto-decoration to methods that need it + canned_roles, decorators = __interfaces[collection_type] + for role, name in canned_roles.items(): + roles.setdefault(role, name) - for method, decorator in decorators.items(): - fn = getattr(cls, method, None) - if (fn and method not in methods and - not hasattr(fn, '_sa_instrumented')): - setattr(cls, method, decorator(fn)) + # apply ABC auto-decoration to methods that need it + for method, decorator in decorators.items(): + fn = getattr(cls, method, None) + if (fn and method not in methods and + not hasattr(fn, '_sa_instrumented')): + setattr(cls, method, decorator(fn)) # ensure all roles are present, and apply implicit instrumentation if # needed diff --cc test/orm/test_collection.py index 7859af37c9,dc8aa2bc46..f6cf510371 --- a/test/orm/test_collection.py +++ b/test/orm/test_collection.py @@@ -2099,50 -2039,6 +2099,77 @@@ class InstrumentationTest(fixtures.ORMT assert not hasattr(Touchy, 'no_touch') assert 'no_touch' in dir(Touchy) - instrumented = collections._instrument_class(Touchy) - assert True + collections._instrument_class(Touchy) + + def test_name_setup(self): + + class Base(object): + @collection.iterator + def base_iterate(self, x): + return "base_iterate" + + @collection.appender + def base_append(self, x): + return "base_append" + + @collection.converter + def base_convert(self, x): + return "base_convert" + + @collection.remover + def base_remove(self, x): + return "base_remove" + + + from sqlalchemy.orm.collections import _instrument_class + _instrument_class(Base) + + eq_(Base._sa_remover(Base(), 5), "base_remove") + eq_(Base._sa_appender(Base(), 5), "base_append") + eq_(Base._sa_iterator(Base(), 5), "base_iterate") + eq_(Base._sa_converter(Base(), 5), "base_convert") + + class Sub(Base): + @collection.converter + def base_convert(self, x): + return "sub_convert" + + @collection.remover + def sub_remove(self, x): + return "sub_remove" + _instrument_class(Sub) + + eq_(Sub._sa_appender(Sub(), 5), "base_append") + eq_(Sub._sa_remover(Sub(), 5), "sub_remove") + eq_(Sub._sa_iterator(Sub(), 5), "base_iterate") + eq_(Sub._sa_converter(Sub(), 5), "sub_convert") + ++ def test_link_event(self): ++ canary = [] ++ class Collection(list): ++ @collection.linker ++ def _on_link(self, obj): ++ canary.append(obj) ++ ++ class Foo(object): ++ pass ++ ++ instrumentation.register_class(Foo) ++ attributes.register_attribute(Foo, 'attr', uselist=True, ++ typecallable=Collection, useobject=True) ++ ++ f1 = Foo() ++ f1.attr.append(3) ++ ++ eq_(canary, [f1.attr._sa_adapter]) ++ adapter_1 = f1.attr._sa_adapter ++ ++ l2 = Collection() ++ f1.attr = l2 ++ eq_(canary, [adapter_1, f1.attr._sa_adapter, None]) ++ ++ ++ ++ +