From 28fc3d575107a95a977a049eb38f55c6c422813a Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 21 Nov 2012 06:26:40 -0500 Subject: - In order to normalize the relationship between event subscribers and subscriber predicates, we now allow both subscribers and subscriber predicates to accept only a single ``event`` argument even if they've been subscribed for notifications that involve multiple interfaces. Subscribers and subscriber predicates that accept only one argument will receive the first object passed to ``notify``; this is typically (but not always) the event object. The other objects involved in the subscription lookup will be discarded. For instance, if an event is sent by code like this:: registry.notify(event, context) In the past, in order to catch such an event, you were obligated to write and register an event subscriber that mentioned both the event and the context in its argument list:: @subscriber([SomeEvent, SomeContextType]) def subscriber(event, context): pass With the event-only feature you can now write an event subscriber that accepts only ``event`` even if it subscribes to multiple interfaces:: @subscriber([SomeEvent, SomeContextType]) def subscriber(event): # this will work! Note, however, that if the event object is not the first object in the call to ``notify``, you'll run into trouble. For example, if notify is called with the context argument first:: registry.notify(context, event) You won't be able to take advantage of the feature. It will "work", but the object received by your event handler won't be the event object, it will be the context object, which won't be very useful:: @subscriber([SomeContextType, SomeEvent]) def subscriber(event): # bzzt! you'll be getting the context here as ``event``, and it'll # be useless Existing multiple-argument subscribers continue to work without issue, so you should continue use those if your system notifies using multiple interfaces and the first interface is not the event interface. For example:: @subscriber([SomeContextType, SomeEvent]) def subscriber(context, event): # this will still work! The event-only feature makes it possible to use a subscriber predicate that accepts only a request argument within both multiple-interface subscriber registrations and single-interface subscriber registrations. In the past, if you had a subscriber predicate like this:: class RequestPathStartsWith(object): def __init__(self, val, config): self.val = val def text(self): return 'path_startswith = %s' % (self.val,) phash = text def __call__(self, event): return event.request.path.startswith(self.val) If you attempted to use the above predicate to condition a subscription that involved multiple interfaces, it would not work. You had to change it to accept the same arguments as the subscription itself. For example, you might have had to change its ``__call__`` method like so, adding a ``context`` argument:: def __call__(self, event, context): return event.request.path.startswith(self.val) With the event-only feature, you needn't make the change. Instead, you can write all predicates so they only accept ``event`` in their ``__call__`` and they'll be useful across all registrations for subscriptions that use an event as their first argument, even ones which accept more than just ``event``. However, the same caveat applies to predicates as to subscriptions: if you're subscribing to a multi-interface event, and the first interface is not the event interface, the predicate won't work properly. In such a case, you'll need to match the predicate ``__call__`` argument ordering and composition to the ordering of the interfaces. For example:: def __call__(self, context, event): return event.request.path.startswith(self.val) tl;dr: 1) Always use the event as the first argument to a multi-interface subscription and 2) Use only ``event`` in your subscriber and subscriber predicate parameter lists, no matter how many interfaces the subscriber is notified with, as long as the event object is the first argument passed to ``registry.notify``. This will result in the maximum amount of reusability of subscriber predicates. --- CHANGES.txt | 98 +++++++++++++++ pyramid/config/adapters.py | 78 +++++++++--- pyramid/config/util.py | 50 ++++++++ pyramid/config/views.py | 46 +------ pyramid/tests/test_config/test_adapters.py | 9 ++ pyramid/tests/test_config/test_util.py | 186 ++++++++++++++++++++++++++++ pyramid/tests/test_config/test_views.py | 190 ++--------------------------- 7 files changed, 416 insertions(+), 241 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 5344cb7d1..a625af3f9 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -15,6 +15,104 @@ Features values in parameterized ``.ini`` file, e.g. ``pshell etc/development.ini http_port=8080``. See https://github.com/Pylons/pyramid/pull/714 +- In order to normalize the relationship between event subscribers and + subscriber predicates, we now allow both subscribers and subscriber + predicates to accept only a single ``event`` argument even if they've been + subscribed for notifications that involve multiple interfaces. Subscribers + and subscriber predicates that accept only one argument will receive the + first object passed to ``notify``; this is typically (but not always) the + event object. The other objects involved in the subscription lookup will be + discarded. + + For instance, if an event is sent by code like this:: + + registry.notify(event, context) + + In the past, in order to catch such an event, you were obligated to write and + register an event subscriber that mentioned both the event and the context in + its argument list:: + + @subscriber([SomeEvent, SomeContextType]) + def subscriber(event, context): + pass + + With the event-only feature you can now write an event subscriber that + accepts only ``event`` even if it subscribes to multiple interfaces:: + + @subscriber([SomeEvent, SomeContextType]) + def subscriber(event): + # this will work! + + Note, however, that if the event object is not the first object in the call + to ``notify``, you'll run into trouble. For example, if notify is called + with the context argument first:: + + registry.notify(context, event) + + You won't be able to take advantage of the feature. It will "work", but the + object received by your event handler won't be the event object, it will be + the context object, which won't be very useful:: + + @subscriber([SomeContextType, SomeEvent]) + def subscriber(event): + # bzzt! you'll be getting the context here as ``event``, and it'll + # be useless + + Existing multiple-argument subscribers continue to work without issue, so you + should continue use those if your system notifies using multiple interfaces + and the first interface is not the event interface. For example:: + + @subscriber([SomeContextType, SomeEvent]) + def subscriber(context, event): + # this will still work! + + The event-only feature makes it possible to use a subscriber predicate that + accepts only a request argument within both multiple-interface subscriber + registrations and single-interface subscriber registrations. In the past, if + you had a subscriber predicate like this:: + + class RequestPathStartsWith(object): + def __init__(self, val, config): + self.val = val + + def text(self): + return 'path_startswith = %s' % (self.val,) + + phash = text + + def __call__(self, event): + return event.request.path.startswith(self.val) + + If you attempted to use the above predicate to condition a subscription that + involved multiple interfaces, it would not work. You had to change it to + accept the same arguments as the subscription itself. For example, you might + have had to change its ``__call__`` method like so, adding a ``context`` + argument:: + + def __call__(self, event, context): + return event.request.path.startswith(self.val) + + With the event-only feature, you needn't make the change. Instead, you can + write all predicates so they only accept ``event`` in their ``__call__`` and + they'll be useful across all registrations for subscriptions that use an + event as their first argument, even ones which accept more than just + ``event``. However, the same caveat applies to predicates as to + subscriptions: if you're subscribing to a multi-interface event, and the + first interface is not the event interface, the predicate won't work + properly. In such a case, you'll need to match the predicate ``__call__`` + argument ordering and composition to the ordering of the interfaces. For + example:: + + def __call__(self, context, event): + return event.request.path.startswith(self.val) + + tl;dr: 1) Always use the event as the first argument to a multi-interface + subscription and 2) Use only ``event`` in your subscriber and subscriber + predicate parameter lists, no matter how many interfaces the subscriber is + notified with, as long as the event object is the first argument passed to + ``registry.notify``. This will result in the maximum amount of reusability + of subscriber predicates. + Bug Fixes --------- diff --git a/pyramid/config/adapters.py b/pyramid/config/adapters.py index 12c4de660..dad60660a 100644 --- a/pyramid/config/adapters.py +++ b/pyramid/config/adapters.py @@ -10,6 +10,7 @@ from pyramid.interfaces import ( from pyramid.config.util import ( action_method, + takes_one_arg, ) @@ -51,8 +52,22 @@ class AdaptersConfiguratorMixin(object): def register(): predlist = self.get_predlist('subscriber') order, preds, phash = predlist.make(self, **predicates) - intr.update({'phash':phash, 'order':order, 'predicates':preds}) - derived_subscriber = self._derive_subscriber(subscriber, preds) + + derived_predicates = [ self._derive_predicate(p) for p in preds ] + derived_subscriber = self._derive_subscriber( + subscriber, + derived_predicates, + ) + + intr.update( + {'phash':phash, + 'order':order, + 'predicates':preds, + 'derived_predicates':derived_predicates, + 'derived_subscriber':derived_subscriber, + } + ) + self.registry.registerHandler(derived_subscriber, iface) intr = self.introspectable( @@ -68,25 +83,54 @@ class AdaptersConfiguratorMixin(object): self.action(None, register, introspectables=(intr,)) return subscriber + def _derive_predicate(self, predicate): + derived_predicate = predicate + + if eventonly(predicate): + def derived_predicate(*arg): + return predicate(arg[0]) + # seems pointless to try to fix __doc__, __module__, etc as + # predicate will invariably be an instance + + return derived_predicate + def _derive_subscriber(self, subscriber, predicates): + derived_subscriber = subscriber + + if eventonly(subscriber): + def derived_subscriber(*arg): + return subscriber(arg[0]) + if hasattr(subscriber, '__name__'): + update_wrapper(derived_subscriber, subscriber) + if not predicates: - return subscriber + return derived_subscriber + def subscriber_wrapper(*arg): - # We need to accept *arg and pass it along because zope - # subscribers are designed poorly. Notification will always call - # an associated subscriber with all of the objects involved in - # the subscription lookup, despite the fact that the event sender - # always has the option to attach those objects to the event - # object itself (and usually does). It would be much saner if the - # registry just used extra args passed to notify to do the lookup - # but only called event subscribers with the actual event object, - # or if we had been smart enough early on to always wrap - # subscribers in something that threw away the extra args, but - # c'est la vie. + # We need to accept *arg and pass it along because zope subscribers + # are designed awkwardly. Notification via + # registry.adapter.subscribers will always call an associated + # subscriber with all of the objects involved in the subscription + # lookup, despite the fact that the event sender always has the + # option to attach those objects to the event object itself, and + # almost always does. + # + # The "eventonly" jazz sprinkled in this function and related + # functions allows users to define subscribers and predicates which + # accept only an event argument without needing to accept the rest + # of the adaptation arguments. Had I been smart enough early on to + # use .subscriptions to find the subscriber functions in order to + # call them manually with a single "event" argument instead of + # relying on .subscribers to both find and call them implicitly + # with all args, the eventonly hack would not have been required. + # At this point, though, using .subscriptions and manual execution + # is not possible without badly breaking backwards compatibility. if all((predicate(*arg) for predicate in predicates)): - return subscriber(*arg) + return derived_subscriber(*arg) + if hasattr(subscriber, '__name__'): update_wrapper(subscriber_wrapper, subscriber) + return subscriber_wrapper @action_method @@ -266,7 +310,7 @@ class AdaptersConfiguratorMixin(object): if resource_iface is None: resource_iface = Interface self.registry.registerAdapter( - adapter, + adapter, (resource_iface, Interface), IResourceURL, ) @@ -281,3 +325,5 @@ class AdaptersConfiguratorMixin(object): intr['resource_iface'] = resource_iface self.action(discriminator, register, introspectables=(intr,)) +def eventonly(callee): + return takes_one_arg(callee, argname='event') diff --git a/pyramid/config/util.py b/pyramid/config/util.py index a83e23798..af0dd1641 100644 --- a/pyramid/config/util.py +++ b/pyramid/config/util.py @@ -1,10 +1,12 @@ from hashlib import md5 +import inspect from pyramid.compat import ( bytes_, is_nonstr_iter, ) +from pyramid.compat import im_func from pyramid.exceptions import ConfigurationError from pyramid.registry import predvalseq @@ -110,3 +112,51 @@ class PredicateList(object): order = (MAX_ORDER - score) / (len(preds) + 1) return order, preds, phash.hexdigest() +def takes_one_arg(callee, attr=None, argname=None): + ismethod = False + if attr is None: + attr = '__call__' + if inspect.isroutine(callee): + fn = callee + elif inspect.isclass(callee): + try: + fn = callee.__init__ + except AttributeError: + return False + ismethod = hasattr(fn, '__call__') + else: + try: + fn = getattr(callee, attr) + except AttributeError: + return False + + try: + argspec = inspect.getargspec(fn) + except TypeError: + return False + + args = argspec[0] + + if hasattr(fn, im_func) or ismethod: + # it's an instance method (or unbound method on py2) + if not args: + return False + args = args[1:] + + if not args: + return False + + if len(args) == 1: + return True + + if argname: + + defaults = argspec[3] + if defaults is None: + defaults = () + + if args[0] == argname: + if len(args) - len(defaults) == 1: + return True + + return False diff --git a/pyramid/config/views.py b/pyramid/config/views.py index 4e5af480d..d1b69566b 100644 --- a/pyramid/config/views.py +++ b/pyramid/config/views.py @@ -81,6 +81,7 @@ import pyramid.config.predicates from pyramid.config.util import ( DEFAULT_PHASH, MAX_ORDER, + takes_one_arg, ) urljoin = urlparse.urljoin @@ -503,50 +504,7 @@ class DefaultViewMapper(object): return _attr_view def requestonly(view, attr=None): - ismethod = False - if attr is None: - attr = '__call__' - if inspect.isroutine(view): - fn = view - elif inspect.isclass(view): - try: - fn = view.__init__ - except AttributeError: - return False - ismethod = hasattr(fn, '__call__') - else: - try: - fn = getattr(view, attr) - except AttributeError: - return False - - try: - argspec = inspect.getargspec(fn) - except TypeError: - return False - - args = argspec[0] - - if hasattr(fn, im_func) or ismethod: - # it's an instance method (or unbound method on py2) - if not args: - return False - args = args[1:] - if not args: - return False - - if len(args) == 1: - return True - - defaults = argspec[3] - if defaults is None: - defaults = () - - if args[0] == 'request': - if len(args) - len(defaults) == 1: - return True - - return False + return takes_one_arg(view, attr=attr, argname='request') @implementer(IMultiView) class MultiView(object): diff --git a/pyramid/tests/test_config/test_adapters.py b/pyramid/tests/test_config/test_adapters.py index d47e012dc..4cbb1bf80 100644 --- a/pyramid/tests/test_config/test_adapters.py +++ b/pyramid/tests/test_config/test_adapters.py @@ -331,6 +331,15 @@ class AdaptersConfiguratorMixinTests(unittest.TestCase): self.assertEqual(intr['adapter'], DummyResourceURL) self.assertEqual(intr['resource_iface'], DummyIface) +class Test_eventonly(unittest.TestCase): + def _callFUT(self, callee): + from pyramid.config.adapters import eventonly + return eventonly(callee) + + def test_defaults(self): + def acallable(event, a=1, b=2): pass + self.assertTrue(self._callFUT(acallable)) + class DummyTraverser(object): def __init__(self, root): self.root = root diff --git a/pyramid/tests/test_config/test_util.py b/pyramid/tests/test_config/test_util.py index b32f9c6ef..a984acfd0 100644 --- a/pyramid/tests/test_config/test_util.py +++ b/pyramid/tests/test_config/test_util.py @@ -365,6 +365,192 @@ class TestPredicateList(unittest.TestCase): from pyramid.exceptions import ConfigurationError self.assertRaises(ConfigurationError, self._callFUT, unknown=1) +class Test_takes_one_arg(unittest.TestCase): + def _callFUT(self, view, attr=None, argname=None): + from pyramid.config.util import takes_one_arg + return takes_one_arg(view, attr=attr, argname=argname) + + def test_requestonly_newstyle_class_no_init(self): + class foo(object): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_requestonly_newstyle_class_init_toomanyargs(self): + class foo(object): + def __init__(self, context, request): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_requestonly_newstyle_class_init_onearg_named_request(self): + class foo(object): + def __init__(self, request): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_newstyle_class_init_onearg_named_somethingelse(self): + class foo(object): + def __init__(self, req): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_newstyle_class_init_defaultargs_firstname_not_request(self): + class foo(object): + def __init__(self, context, request=None): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_newstyle_class_init_defaultargs_firstname_request(self): + class foo(object): + def __init__(self, request, foo=1, bar=2): + """ """ + self.assertTrue(self._callFUT(foo, argname='request')) + + def test_newstyle_class_init_firstname_request_with_secondname(self): + class foo(object): + def __init__(self, request, two): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_newstyle_class_init_noargs(self): + class foo(object): + def __init__(): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_oldstyle_class_no_init(self): + class foo: + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_oldstyle_class_init_toomanyargs(self): + class foo: + def __init__(self, context, request): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_oldstyle_class_init_onearg_named_request(self): + class foo: + def __init__(self, request): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_oldstyle_class_init_onearg_named_somethingelse(self): + class foo: + def __init__(self, req): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_oldstyle_class_init_defaultargs_firstname_not_request(self): + class foo: + def __init__(self, context, request=None): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_oldstyle_class_init_defaultargs_firstname_request(self): + class foo: + def __init__(self, request, foo=1, bar=2): + """ """ + self.assertTrue(self._callFUT(foo, argname='request'), True) + + def test_oldstyle_class_init_noargs(self): + class foo: + def __init__(): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_function_toomanyargs(self): + def foo(context, request): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_function_with_attr_false(self): + def bar(context, request): + """ """ + def foo(context, request): + """ """ + foo.bar = bar + self.assertFalse(self._callFUT(foo, 'bar')) + + def test_function_with_attr_true(self): + def bar(context, request): + """ """ + def foo(request): + """ """ + foo.bar = bar + self.assertTrue(self._callFUT(foo, 'bar')) + + def test_function_onearg_named_request(self): + def foo(request): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_function_onearg_named_somethingelse(self): + def foo(req): + """ """ + self.assertTrue(self._callFUT(foo)) + + def test_function_defaultargs_firstname_not_request(self): + def foo(context, request=None): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_function_defaultargs_firstname_request(self): + def foo(request, foo=1, bar=2): + """ """ + self.assertTrue(self._callFUT(foo, argname='request')) + + def test_function_noargs(self): + def foo(): + """ """ + self.assertFalse(self._callFUT(foo)) + + def test_instance_toomanyargs(self): + class Foo: + def __call__(self, context, request): + """ """ + foo = Foo() + self.assertFalse(self._callFUT(foo)) + + def test_instance_defaultargs_onearg_named_request(self): + class Foo: + def __call__(self, request): + """ """ + foo = Foo() + self.assertTrue(self._callFUT(foo)) + + def test_instance_defaultargs_onearg_named_somethingelse(self): + class Foo: + def __call__(self, req): + """ """ + foo = Foo() + self.assertTrue(self._callFUT(foo)) + + def test_instance_defaultargs_firstname_not_request(self): + class Foo: + def __call__(self, context, request=None): + """ """ + foo = Foo() + self.assertFalse(self._callFUT(foo)) + + def test_instance_defaultargs_firstname_request(self): + class Foo: + def __call__(self, request, foo=1, bar=2): + """ """ + foo = Foo() + self.assertTrue(self._callFUT(foo, argname='request'), True) + + def test_instance_nocall(self): + class Foo: pass + foo = Foo() + self.assertFalse(self._callFUT(foo)) + + def test_method_onearg_named_request(self): + class Foo: + def method(self, request): + """ """ + foo = Foo() + self.assertTrue(self._callFUT(foo.method)) + class DummyCustomPredicate(object): def __init__(self): diff --git a/pyramid/tests/test_config/test_views.py b/pyramid/tests/test_config/test_views.py index bb4c5d519..4cebdce8a 100644 --- a/pyramid/tests/test_config/test_views.py +++ b/pyramid/tests/test_config/test_views.py @@ -1896,192 +1896,20 @@ class TestViewsConfigurationMixin(unittest.TestCase): from pyramid.tests import test_config self.assertEqual(result, test_config) - class Test_requestonly(unittest.TestCase): def _callFUT(self, view, attr=None): from pyramid.config.views import requestonly - return requestonly(view, attr) - - def test_requestonly_newstyle_class_no_init(self): - class foo(object): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_requestonly_newstyle_class_init_toomanyargs(self): - class foo(object): - def __init__(self, context, request): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_requestonly_newstyle_class_init_onearg_named_request(self): - class foo(object): - def __init__(self, request): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_newstyle_class_init_onearg_named_somethingelse(self): - class foo(object): - def __init__(self, req): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_newstyle_class_init_defaultargs_firstname_not_request(self): - class foo(object): - def __init__(self, context, request=None): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_newstyle_class_init_defaultargs_firstname_request(self): - class foo(object): - def __init__(self, request, foo=1, bar=2): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_newstyle_class_init_firstname_request_with_secondname(self): - class foo(object): - def __init__(self, request, two): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_newstyle_class_init_noargs(self): - class foo(object): - def __init__(): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_oldstyle_class_no_init(self): - class foo: - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_oldstyle_class_init_toomanyargs(self): - class foo: - def __init__(self, context, request): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_oldstyle_class_init_onearg_named_request(self): - class foo: - def __init__(self, request): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_oldstyle_class_init_onearg_named_somethingelse(self): - class foo: - def __init__(self, req): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_oldstyle_class_init_defaultargs_firstname_not_request(self): - class foo: - def __init__(self, context, request=None): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_oldstyle_class_init_defaultargs_firstname_request(self): - class foo: - def __init__(self, request, foo=1, bar=2): - """ """ - self.assertTrue(self._callFUT(foo), True) - - def test_oldstyle_class_init_noargs(self): - class foo: - def __init__(): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_function_toomanyargs(self): - def foo(context, request): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_function_with_attr_false(self): - def bar(context, request): - """ """ - def foo(context, request): - """ """ - foo.bar = bar - self.assertFalse(self._callFUT(foo, 'bar')) - - def test_function_with_attr_true(self): - def bar(context, request): - """ """ - def foo(request): - """ """ - foo.bar = bar - self.assertTrue(self._callFUT(foo, 'bar')) - - def test_function_onearg_named_request(self): - def foo(request): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_function_onearg_named_somethingelse(self): - def foo(req): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_function_defaultargs_firstname_not_request(self): - def foo(context, request=None): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_function_defaultargs_firstname_request(self): - def foo(request, foo=1, bar=2): - """ """ - self.assertTrue(self._callFUT(foo)) - - def test_function_noargs(self): - def foo(): - """ """ - self.assertFalse(self._callFUT(foo)) - - def test_instance_toomanyargs(self): - class Foo: - def __call__(self, context, request): - """ """ - foo = Foo() - self.assertFalse(self._callFUT(foo)) + return requestonly(view, attr=attr) - def test_instance_defaultargs_onearg_named_request(self): - class Foo: - def __call__(self, request): - """ """ - foo = Foo() - self.assertTrue(self._callFUT(foo)) - - def test_instance_defaultargs_onearg_named_somethingelse(self): - class Foo: - def __call__(self, req): - """ """ - foo = Foo() - self.assertTrue(self._callFUT(foo)) - - def test_instance_defaultargs_firstname_not_request(self): - class Foo: - def __call__(self, context, request=None): - """ """ - foo = Foo() - self.assertFalse(self._callFUT(foo)) - - def test_instance_defaultargs_firstname_request(self): - class Foo: - def __call__(self, request, foo=1, bar=2): - """ """ - foo = Foo() - self.assertTrue(self._callFUT(foo), True) + def test_defaults(self): + def aview(request, a=1, b=2): pass + self.assertTrue(self._callFUT(aview)) - def test_instance_nocall(self): - class Foo: pass - foo = Foo() - self.assertFalse(self._callFUT(foo)) - - def test_method_onearg_named_request(self): - class Foo: - def method(self, request): - """ """ - foo = Foo() - self.assertTrue(self._callFUT(foo.method)) + def test_otherattr(self): + class AView(object): + def __init__(self, request, a=1, b=2): pass + def bleh(self): pass + self.assertTrue(self._callFUT(AView, 'bleh')) class Test_isexception(unittest.TestCase): def _callFUT(self, ob): -- cgit v1.2.3 From f700d374e903a501bc32a4b10774e87c74edc4d8 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 21 Nov 2012 07:01:00 -0500 Subject: garden --- CHANGES.txt | 172 ++++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 110 insertions(+), 62 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index a625af3f9..4e3e45a79 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -15,16 +15,10 @@ Features values in parameterized ``.ini`` file, e.g. ``pshell etc/development.ini http_port=8080``. See https://github.com/Pylons/pyramid/pull/714 -- In order to normalize the relationship between event subscribers and - subscriber predicates, we now allow both subscribers and subscriber - predicates to accept only a single ``event`` argument even if they've been - subscribed for notifications that involve multiple interfaces. Subscribers - and subscriber predicates that accept only one argument will receive the - first object passed to ``notify``; this is typically (but not always) the - event object. The other objects involved in the subscription lookup will be - discarded. - - For instance, if an event is sent by code like this:: +- A somewhat advanced and obscure feature of Pyramid event handlers is their + ability to handle "multi-interface" notifications. These notifications have + traditionally presented multiple objects to the subscriber callable. For + instance, if an event was sent by code like this:: registry.notify(event, context) @@ -33,28 +27,101 @@ Features its argument list:: @subscriber([SomeEvent, SomeContextType]) - def subscriber(event, context): + def asubscriber(event, context): + pass + + In many subscriber callables registered this way, it was common for the logic + in the subscriber callable to completely ignore the second and following + arguments (e.g. ``context`` in the above example might be ignored), because + they usually existed as attributes of the event anyway. You could usually + get the same value by doing ``event.context`` or similar in most cases. + + The fact that you needed to put an extra argument which you usually ignored + in the subscriber callable body was only a minor annoyance until we added + "subscriber predicates" in a prior 1.4 alpha release. Subscriber predicates + are used to narrow the set of circumstances under which a subscriber will be + executed. Once those were added, the annoyance was escalated, because + subscriber predicates needed to accept the same argument list and arity as + the subscriber callables that they were configured against. So, for example, + if you had these two subscriber registrations in your code:: + + @subscriber([SomeEvent, SomeContextType]) + def asubscriber(event, context): + pass + + @subscriber(SomeOtherEvent) + def asubscriber(event): + pass + + And you wanted to use a subscriber predicate:: + + @subscriber([SomeEvent, SomeContextType], mypredicate=True) + def asubscriber(event, context): pass - With the event-only feature you can now write an event subscriber that - accepts only ``event`` even if it subscribes to multiple interfaces:: + @subscriber(SomeOtherEvent, mypredicate=True) + def asubscriber(event): + pass + + If you had previously written your ``mypredicate`` subscriber predicate that + accepted in such a way that it accepted only one argument in its + ``__call__``, you could not use it against a subscription which named more + than one interface in its subscriber interface list. Similarly, if you had + written a subscriber predicate that accepted two arguments, you couldn't use + it against a registration that named only a single interface type. For + example, if you created this predicate:: + + class MyPredicate(object): + # portions elided... + def __call__(self, event): + return self.val == event.context.foo + + It would not work against a multi-interface-registered subscription. + + To hack around this limitation, you needed to design a ``mypredicate`` + predicate to expect to receive in its ``__call__`` either a single ``event`` + argument (a SomeOtherEvent object) *or* a pair of arguments (a SomeEvent + object and a SomeContextType object), presumably by doing something like + this:: + + class MyPredicate(object): + # portions elided... + def __call__(self, event, context=None): + return self.val == event.context.foo + + This was confusing and bad. + + In order to allow people to ignore unused arguments to subscriber callables + and to normalize the relationship between event subscribers and subscriber + predicates, we now allow both subscribers and subscriber predicates to accept + only a single ``event`` argument even if they've been subscribed for + notifications that involve multiple interfaces. Subscribers and subscriber + predicates that accept only one argument will receive the first object passed + to ``notify``; this is typically (but not always) the event object. The + other objects involved in the subscription lookup will be discarded. You can + now write an event subscriber that accepts only ``event`` even if it + subscribes to multiple interfaces:: @subscriber([SomeEvent, SomeContextType]) - def subscriber(event): + def asubscriber(event): # this will work! - Note, however, that if the event object is not the first object in the call - to ``notify``, you'll run into trouble. For example, if notify is called - with the context argument first:: + This prevents you from needing to match the subscriber callable parameters to + the subscription type unnecessarily, especially when you don't make use of + any argument in your subscribers except for the event object itself. + + Note, however, that if the event object is not the first + object in the call to ``notify``, you'll run into trouble. For example, if + notify is called with the context argument first:: registry.notify(context, event) - You won't be able to take advantage of the feature. It will "work", but the - object received by your event handler won't be the event object, it will be - the context object, which won't be very useful:: + You won't be able to take advantage of the event-only feature. It will + "work", but the object received by your event handler won't be the event + object, it will be the context object, which won't be very useful:: @subscriber([SomeContextType, SomeEvent]) - def subscriber(event): + def asubscriber(event): # bzzt! you'll be getting the context here as ``event``, and it'll # be useless @@ -63,55 +130,36 @@ Features and the first interface is not the event interface. For example:: @subscriber([SomeContextType, SomeEvent]) - def subscriber(context, event): + def asubscriber(context, event): # this will still work! The event-only feature makes it possible to use a subscriber predicate that accepts only a request argument within both multiple-interface subscriber - registrations and single-interface subscriber registrations. In the past, if - you had a subscriber predicate like this:: - - class RequestPathStartsWith(object): - def __init__(self, val, config): - self.val = val - - def text(self): - return 'path_startswith = %s' % (self.val,) - - phash = text - - def __call__(self, event): - return event.request.path.startswith(self.val) - - If you attempted to use the above predicate to condition a subscription that - involved multiple interfaces, it would not work. You had to change it to - accept the same arguments as the subscription itself. For example, you might - have had to change its ``__call__`` method like so, adding a ``context`` - argument:: - - def __call__(self, event, context): - return event.request.path.startswith(self.val) - - With the event-only feature, you needn't make the change. Instead, you can - write all predicates so they only accept ``event`` in their ``__call__`` and - they'll be useful across all registrations for subscriptions that use an - event as their first argument, even ones which accept more than just - ``event``. However, the same caveat applies to predicates as to - subscriptions: if you're subscribing to a multi-interface event, and the - first interface is not the event interface, the predicate won't work - properly. In such a case, you'll need to match the predicate ``__call__`` - argument ordering and composition to the ordering of the interfaces. For - example:: + registrations and single-interface subscriber registrations. You needn't + make slightly different variations of predicates depending on the + subscription type arguments. Instead, just write all your subscriber + predicates so they only accept ``event`` in their ``__call__`` and they'll be + useful across all registrations for subscriptions that use an event as their + first argument, even ones which accept more than just ``event``. + + However, the same caveat applies to predicates as to subscriber callables: if + you're subscribing to a multi-interface event, and the first interface is not + the event interface, the predicate won't work properly. In such a case, + you'll need to match the predicate ``__call__`` argument ordering and + composition to the ordering of the interfaces. For example, if the + registration for the subscription uses ``[SomeContext, SomeEvent]``, you'll + need to reflect that in the ordering of the parameters of the predicate's + ``__call__`` method:: def __call__(self, context, event): return event.request.path.startswith(self.val) - tl;dr: 1) Always use the event as the first argument to a multi-interface - subscription and 2) Use only ``event`` in your subscriber and subscriber - predicate parameter lists, no matter how many interfaces the subscriber is - notified with, as long as the event object is the first argument passed to - ``registry.notify``. This will result in the maximum amount of reusability - of subscriber predicates. + tl;dr: 1) When using multi-interface subscriptions, always use the event type + as the first subscription registration argument and 2) When 1 is true, use + only ``event`` in your subscriber and subscriber predicate parameter lists, + no matter how many interfaces the subscriber is notified with. This + combination will result in the maximum amount of reusability of subscriber + predicates and the least amount of thought on your part. Drink responsibly. Bug Fixes --------- -- cgit v1.2.3 From a3810e706bce12c0195737c2713362795613e2ae Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 21 Nov 2012 07:05:04 -0500 Subject: garden --- CHANGES.txt | 48 ++++++++++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 4e3e45a79..01fd60161 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -34,16 +34,16 @@ Features in the subscriber callable to completely ignore the second and following arguments (e.g. ``context`` in the above example might be ignored), because they usually existed as attributes of the event anyway. You could usually - get the same value by doing ``event.context`` or similar in most cases. + get the same value by doing ``event.context`` or similar. The fact that you needed to put an extra argument which you usually ignored in the subscriber callable body was only a minor annoyance until we added - "subscriber predicates" in a prior 1.4 alpha release. Subscriber predicates - are used to narrow the set of circumstances under which a subscriber will be - executed. Once those were added, the annoyance was escalated, because - subscriber predicates needed to accept the same argument list and arity as - the subscriber callables that they were configured against. So, for example, - if you had these two subscriber registrations in your code:: + "subscriber predicates", used to narrow the set of circumstances under which + a subscriber will be executed, in a prior 1.4 alpha release. Once those were + added, the annoyance was escalated, because subscriber predicates needed to + accept the same argument list and arity as the subscriber callables that they + were configured against. So, for example, if you had these two subscriber + registrations in your code:: @subscriber([SomeEvent, SomeContextType]) def asubscriber(event, context): @@ -56,33 +56,37 @@ Features And you wanted to use a subscriber predicate:: @subscriber([SomeEvent, SomeContextType], mypredicate=True) - def asubscriber(event, context): + def asubscriber1(event, context): pass @subscriber(SomeOtherEvent, mypredicate=True) - def asubscriber(event): + def asubscriber2(event): pass - If you had previously written your ``mypredicate`` subscriber predicate that - accepted in such a way that it accepted only one argument in its - ``__call__``, you could not use it against a subscription which named more - than one interface in its subscriber interface list. Similarly, if you had - written a subscriber predicate that accepted two arguments, you couldn't use - it against a registration that named only a single interface type. For - example, if you created this predicate:: + If an existing ``mypredicate`` subscriber predicate had been written in such + a way that it accepted only one argument in its ``__call__``, you could not + use it against a subscription which named more than one interface in its + subscriber interface list. Similarly, if you had written a subscriber + predicate that accepted two arguments, you couldn't use it against a + registration that named only a single interface type. + + For example, if you created this predicate:: class MyPredicate(object): # portions elided... def __call__(self, event): return self.val == event.context.foo - It would not work against a multi-interface-registered subscription. + It would not work against a multi-interface-registered subscription, so in + the above example, when you attempted to use it against ``asubscriber1``, it + would fail at runtime with a TypeError, claiming something was attempting to + call it with too many arguments. - To hack around this limitation, you needed to design a ``mypredicate`` - predicate to expect to receive in its ``__call__`` either a single ``event`` - argument (a SomeOtherEvent object) *or* a pair of arguments (a SomeEvent - object and a SomeContextType object), presumably by doing something like - this:: + To hack around this limitation, you were obligated to design the + ``mypredicate`` predicate to expect to receive in its ``__call__`` either a + single ``event`` argument (a SomeOtherEvent object) *or* a pair of arguments + (a SomeEvent object and a SomeContextType object), presumably by doing + something like this:: class MyPredicate(object): # portions elided... -- cgit v1.2.3 From d91dc7bd96bdc8638c52e4b0685612237cf13578 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 21 Nov 2012 07:27:40 -0500 Subject: add an integration test for the eventonly behavior --- pyramid/tests/test_integration.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index 9a8f842aa..bb33c5c27 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -174,6 +174,18 @@ class TestStaticAppBase(IntegrationBase): def test_oob_slash(self): self.testapp.get('/%2F/test_integration.py', status=404) +class TestEventOnlySubscribers(IntegrationBase, unittest.TestCase): + package = 'pyramid.tests.pkgs.eventonly' + + def test_sendfoo(self): + res = self.testapp.get('/sendfoo', status=200) + self.assertEqual(sorted(res.body.split()), ['foo', 'fooyup']) + + def test_sendfoobar(self): + res = self.testapp.get('/sendfoobar', status=200) + self.assertEqual(sorted(res.body.split()), + ['foobar', 'foobar2', 'foobaryup', 'foobaryup2']) + class TestStaticAppUsingAbsPath(TestStaticAppBase, unittest.TestCase): package = 'pyramid.tests.pkgs.static_abspath' -- cgit v1.2.3 From f55f9b704fb493327151e8641ae3cd5377eed0c6 Mon Sep 17 00:00:00 2001 From: Chris McDonough Date: Wed, 21 Nov 2012 07:35:03 -0500 Subject: fix for py3 --- pyramid/tests/test_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyramid/tests/test_integration.py b/pyramid/tests/test_integration.py index bb33c5c27..bf3960b2d 100644 --- a/pyramid/tests/test_integration.py +++ b/pyramid/tests/test_integration.py @@ -179,12 +179,12 @@ class TestEventOnlySubscribers(IntegrationBase, unittest.TestCase): def test_sendfoo(self): res = self.testapp.get('/sendfoo', status=200) - self.assertEqual(sorted(res.body.split()), ['foo', 'fooyup']) + self.assertEqual(sorted(res.body.split()), [b'foo', b'fooyup']) def test_sendfoobar(self): res = self.testapp.get('/sendfoobar', status=200) self.assertEqual(sorted(res.body.split()), - ['foobar', 'foobar2', 'foobaryup', 'foobaryup2']) + [b'foobar', b'foobar2', b'foobaryup', b'foobaryup2']) class TestStaticAppUsingAbsPath(TestStaticAppBase, unittest.TestCase): package = 'pyramid.tests.pkgs.static_abspath' -- cgit v1.2.3