diff options
| -rw-r--r-- | CHANGES.txt | 98 | ||||
| -rw-r--r-- | pyramid/config/adapters.py | 78 | ||||
| -rw-r--r-- | pyramid/config/util.py | 50 | ||||
| -rw-r--r-- | pyramid/config/views.py | 46 | ||||
| -rw-r--r-- | pyramid/tests/test_config/test_adapters.py | 9 | ||||
| -rw-r--r-- | pyramid/tests/test_config/test_util.py | 186 | ||||
| -rw-r--r-- | 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): |