launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05044
[Merge] lp:~allenap/launchpad/longpoll-storm-events-subscribe into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/longpoll-storm-events-subscribe into lp:launchpad with lp:~allenap/launchpad/longpoll-storm-events as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/launchpad/longpoll-storm-events-subscribe/+merge/76363
There are a few changes in here:
- For ILongPollEvent, switch from multi-adapters to single-adapters,
optionally named. This means that, for most objects, there can be a
default event registered, and more esoteric things can be registered
by name.
The Storm -> ILongPollEvent adapter takes advantage of this;
interested code can subscribe to any Storm object in a single step
and receive all lifecycle events about that object (that the object
chooses to publish via zope.event.notify).
- Change ILongPollEvent.emit() to accept keyword arguments instead of
a blob. I was concerned that there was just too much nesting of data
structures going on, it was getting confusing. Now the keyword
arguments are merged with an event_key item to form the message
payload.
- Rename LongPollSubscriber to LongPollApplicationRequestSubscriber
because that's what it is; it's not intended as a reusable
base-class.
--
https://code.launchpad.net/~allenap/launchpad/longpoll-storm-events-subscribe/+merge/76363
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/longpoll-storm-events-subscribe into lp:launchpad.
=== modified file 'lib/lp/app/longpoll/__init__.py'
--- lib/lp/app/longpoll/__init__.py 2011-09-16 15:01:47 +0000
+++ lib/lp/app/longpoll/__init__.py 2011-09-21 09:20:47 +0000
@@ -10,7 +10,7 @@
]
from lazr.restful.utils import get_current_browser_request
-from zope.component import getMultiAdapter
+from zope.component import getAdapter
from lp.services.longpoll.interfaces import (
ILongPollEvent,
@@ -18,16 +18,17 @@
)
-def subscribe(target, event, request=None):
+def subscribe(target, event_name=u"", request=None):
"""Convenience method to subscribe the current request.
:param target: Something that can be adapted to `ILongPollEvent`.
- :param event: The name of the event to subscribe to.
+ :param event_name: The name of the event to subscribe to. This is used to
+ look up a named adapter from `target` to `ILongPollEvent`.
:param request: The request for which to get an `ILongPollSubscriber`. It
a request is not specified the currently active request is used.
:return: The `ILongPollEvent` that has been subscribed to.
"""
- event = getMultiAdapter((target, event), ILongPollEvent)
+ event = getAdapter(target, ILongPollEvent, name=event_name)
if request is None:
request = get_current_browser_request()
subscriber = ILongPollSubscriber(request)
@@ -35,14 +36,15 @@
return event
-def emit(source, event, data):
+def emit(source, event_name=u"", **data):
"""Convenience method to emit a message for an event.
- :param source: Something, along with `event`, that can be adapted to
- `ILongPollEvent`.
- :param event: A name/key of the event that is emitted.
+ :param source: Something that can be be adapted to `ILongPollEvent`.
+ :param event_name: The name of the event to subscribe to. This is used to
+ look up a named adapter from `target` to `ILongPollEvent`.
+ :param data: See `ILongPollEvent.emit`.
:return: The `ILongPollEvent` that has been emitted.
"""
- event = getMultiAdapter((source, event), ILongPollEvent)
- event.emit(data)
+ event = getAdapter(source, ILongPollEvent, name=event_name)
+ event.emit(**data)
return event
=== modified file 'lib/lp/app/longpoll/tests/test_longpoll.py'
--- lib/lp/app/longpoll/tests/test_longpoll.py 2011-09-16 15:01:47 +0000
+++ lib/lp/app/longpoll/tests/test_longpoll.py 2011-09-21 09:20:47 +0000
@@ -45,19 +45,17 @@
class FakeEvent:
- adapts(IFakeObject, Interface)
+ adapts(IFakeObject)
implements(ILongPollEvent)
- def __init__(self, source, event):
+ def __init__(self, source):
self.source = source
- self.event = event
@property
def event_key(self):
- return "event-key-%s-%s" % (
- self.source.ident, self.event)
+ return "event-key-%s" % self.source.ident
- def emit(self, data):
+ def emit(self, **data):
# Don't cargo-cult this; see .adapters.event.LongPollEvent instead.
RabbitRoutingKey(self.event_key).send_now(data)
@@ -67,38 +65,54 @@
layer = LaunchpadFunctionalLayer
def test_subscribe(self):
- # subscribe() gets the ILongPollEvent for the given (target, event)
- # and the ILongPollSubscriber for the given request (or the current
- # request is discovered). It subscribes the latter to the event, then
- # returns the event.
+ # subscribe() gets the ILongPollEvent for the given target and the
+ # ILongPollSubscriber for the given request (or the current request is
+ # discovered). It subscribes the latter to the event, then returns the
+ # event.
request = LaunchpadTestRequest()
an_object = FakeObject(12345)
with ZopeAdapterFixture(FakeEvent):
- event = subscribe(an_object, "foo", request=request)
+ event = subscribe(an_object, request=request)
self.assertIsInstance(event, FakeEvent)
- self.assertEqual("event-key-12345-foo", event.event_key)
+ self.assertEqual("event-key-12345", event.event_key)
# Emitting an event-key-12345-foo event will put something on the
# subscriber's queue.
event_data = {"1234": 5678}
- event.emit(event_data)
+ event.emit(**event_data)
subscriber = ILongPollSubscriber(request)
subscribe_queue = RabbitQueue(subscriber.subscribe_key)
message = subscribe_queue.receive(timeout=5)
self.assertEqual(event_data, message)
+ def test_subscribe_to_named_event(self):
+ # When an event_name is given to subscribe(), a named adapter is used
+ # to get the ILongPollEvent for the given target.
+ request = LaunchpadTestRequest()
+ an_object = FakeObject(12345)
+ with ZopeAdapterFixture(FakeEvent, name="foo"):
+ event = subscribe(an_object, event_name="foo", request=request)
+ self.assertIsInstance(event, FakeEvent)
+
def test_emit(self):
- # subscribe() gets the ILongPollEvent for the given (target, event)
- # and passes the given data to its emit() method. It then returns the
- # event.
+ # emit() gets the ILongPollEvent for the given target and passes the
+ # given data to its emit() method. It then returns the event.
an_object = FakeObject(12345)
with ZopeAdapterFixture(FakeEvent):
- event = emit(an_object, "bar", {})
+ event = emit(an_object)
routing_key = RabbitRoutingKey(event.event_key)
subscribe_queue = RabbitQueue("whatever")
routing_key.associateConsumer(subscribe_queue)
# Emit the event again; the subscribe queue was not associated
# with the event before now.
event_data = {"8765": 4321}
- event = emit(an_object, "bar", event_data)
+ event = emit(an_object, **event_data)
message = subscribe_queue.receive(timeout=5)
self.assertEqual(event_data, message)
+
+ def test_emit_named_event(self):
+ # When an event_name is given to emit(), a named adapter is used to
+ # get the ILongPollEvent for the given target.
+ an_object = FakeObject(12345)
+ with ZopeAdapterFixture(FakeEvent, name="foo"):
+ event = emit(an_object, "foo")
+ self.assertIsInstance(event, FakeEvent)
=== modified file 'lib/lp/services/longpoll/adapters/event.py'
--- lib/lp/services/longpoll/adapters/event.py 2011-09-16 14:41:21 +0000
+++ lib/lp/services/longpoll/adapters/event.py 2011-09-21 09:20:47 +0000
@@ -27,24 +27,40 @@
class LongPollEvent:
"""Base-class for event adapters.
- Sub-classes need to declare something along the lines of:
-
- adapts(Interface, Interface)
- implements(ILongPollEvent)
+ Sub-classes need to define the `event_key` property and declare something
+ along the lines of::
+
+ class LongPollAwesomeThingEvent(LongPollEvent):
+ adapts(IAwesomeThing)
+ implements(ILongPollEvent)
+
+ Alternatively, use the `long_poll_event` class decorator::
+
+ @long_poll_event(IAwesomeThing)
+ class LongPollAwesomeThingEvent(LongPollEvent):
+ ...
+
+ In both cases the adapter should be registered in a `configure.zcml`
+ somewhere sensible::
+
+ <adapter factory=".adapters.LongPollAwesomeThingEvent" />
"""
- def __init__(self, source, event):
+ def __init__(self, source):
self.source = source
- self.event = event
@property
def event_key(self):
"""See `ILongPollEvent`."""
raise NotImplementedError(self.__class__.event_key)
- def emit(self, data):
- """See `ILongPollEvent`."""
- payload = {"event_key": self.event_key, "event_data": data}
- router = router_factory(self.event_key)
- router.send(payload)
+ def emit(self, **data):
+ """See `ILongPollEvent`.
+
+ The data will be updated with `event_key`, a copy of `self.event_key`.
+ """
+ event_key = self.event_key
+ data.update(event_key=event_key)
+ router = router_factory(event_key)
+ router.send(data)
=== modified file 'lib/lp/services/longpoll/adapters/storm.py'
--- lib/lp/services/longpoll/adapters/storm.py 2011-09-21 09:20:47 +0000
+++ lib/lp/services/longpoll/adapters/storm.py 2011-09-21 09:20:47 +0000
@@ -16,23 +16,24 @@
from storm.base import Storm
from storm.info import get_obj_info
from zope.component import adapter
-from zope.interface import implements
from lp.services.longpoll.adapters.event import (
generate_event_key,
LongPollEvent,
)
-from lp.services.longpoll.interfaces import ILongPollEvent
-
-
+from lp.services.longpoll.interfaces import (
+ ILongPollEvent,
+ long_poll_event,
+ )
+
+
+@long_poll_event(Storm)
class LongPollStormEvent(LongPollEvent):
"""A `ILongPollEvent` for events of `Storm` objects.
This class knows how to construct a stable event key given a Storm object.
"""
- implements(ILongPollEvent)
-
@property
def event_key(self):
"""See `ILongPollEvent`.
@@ -41,30 +42,29 @@
Storm model object.
"""
cls_info = get_obj_info(self.source).cls_info
- key_parts = [cls_info.table.name.lower()]
- key_parts.extend(
- primary_key_column.__get__(self.source)
- for primary_key_column in cls_info.primary_key)
- key_parts.append(self.event)
- return generate_event_key(*key_parts)
+ return generate_event_key(
+ cls_info.table.name.lower(), *(
+ primary_key_column.__get__(self.source)
+ for primary_key_column in cls_info.primary_key))
@adapter(Storm, IObjectCreatedEvent)
def object_created(model_instance, object_event):
"""Subscription handler for `Storm` creation events."""
- event = LongPollStormEvent(model_instance, "created")
- event.emit({})
+ event = ILongPollEvent(model_instance)
+ event.emit(what="created")
@adapter(Storm, IObjectDeletedEvent)
def object_deleted(model_instance, object_event):
"""Subscription handler for `Storm` deletion events."""
- event = LongPollStormEvent(model_instance, "deleted")
- event.emit({})
+ event = ILongPollEvent(model_instance)
+ event.emit(what="deleted")
@adapter(Storm, IObjectModifiedEvent)
def object_modified(model_instance, object_event):
"""Subscription handler for `Storm` modification events."""
- event = LongPollStormEvent(model_instance, "modified")
- event.emit({"edited_fields": sorted(object_event.edited_fields)})
+ edited_fields = sorted(object_event.edited_fields)
+ event = ILongPollEvent(model_instance)
+ event.emit(what="modified", edited_fields=edited_fields)
=== modified file 'lib/lp/services/longpoll/adapters/subscriber.py'
--- lib/lp/services/longpoll/adapters/subscriber.py 2011-09-16 14:59:47 +0000
+++ lib/lp/services/longpoll/adapters/subscriber.py 2011-09-21 09:20:47 +0000
@@ -6,7 +6,7 @@
__metaclass__ = type
__all__ = [
"generate_subscribe_key",
- "LongPollSubscriber",
+ "LongPollApplicationRequestSubscriber",
]
from uuid import uuid4
@@ -28,7 +28,7 @@
return "longpoll.subscribe.%s" % uuid4()
-class LongPollSubscriber:
+class LongPollApplicationRequestSubscriber:
adapts(IApplicationRequest)
implements(ILongPollSubscriber)
=== modified file 'lib/lp/services/longpoll/adapters/tests/test_event.py'
--- lib/lp/services/longpoll/adapters/tests/test_event.py 2011-09-16 14:59:47 +0000
+++ lib/lp/services/longpoll/adapters/tests/test_event.py 2011-09-21 09:20:47 +0000
@@ -7,10 +7,7 @@
from zope.interface import implements
-from canonical.testing.layers import (
- BaseLayer,
- LaunchpadFunctionalLayer,
- )
+from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.services.longpoll.adapters.event import (
generate_event_key,
LongPollEvent,
@@ -27,7 +24,7 @@
@property
def event_key(self):
- return "event-key-%s-%s" % (self.source, self.event)
+ return "event-key-%s" % self.source
class TestLongPollEvent(TestCase):
@@ -35,24 +32,21 @@
layer = LaunchpadFunctionalLayer
def test_interface(self):
- event = FakeEvent("source", "event")
+ event = FakeEvent("source")
self.assertProvides(event, ILongPollEvent)
def test_event_key(self):
# event_key is not implemented in LongPollEvent; subclasses must
# provide it.
- event = LongPollEvent("source", "event")
+ event = LongPollEvent("source")
self.assertRaises(NotImplementedError, getattr, event, "event_key")
def test_emit(self):
# LongPollEvent.emit() sends the given data to `event_key`.
- event = FakeEvent("source", "event")
+ event = FakeEvent("source")
event_data = {"hello": 1234}
- event.emit(event_data)
- expected_message = {
- "event_key": event.event_key,
- "event_data": event_data,
- }
+ event.emit(**event_data)
+ expected_message = dict(event_data, event_key=event.event_key)
pending_messages = [
message for (call, message) in
RabbitMessageBase.class_locals.messages]
@@ -61,8 +55,6 @@
class TestFunctions(TestCase):
- layer = BaseLayer
-
def test_generate_event_key_no_components(self):
self.assertRaises(
AssertionError, generate_event_key)
=== modified file 'lib/lp/services/longpoll/adapters/tests/test_storm.py'
--- lib/lp/services/longpoll/adapters/tests/test_storm.py 2011-09-21 09:20:47 +0000
+++ lib/lp/services/longpoll/adapters/tests/test_storm.py 2011-09-21 09:20:47 +0000
@@ -15,11 +15,13 @@
from zope.event import notify
from canonical.testing.layers import LaunchpadFunctionalLayer
+from lp.services.longpoll.interfaces import ILongPollEvent
from lp.services.longpoll.testing import (
capture_longpoll_emissions,
LongPollEventRecord,
)
from lp.testing import TestCase
+from lp.testing.matchers import Provides
class FakeStormClass(Storm):
@@ -33,42 +35,47 @@
layer = LaunchpadFunctionalLayer
+ def test_storm_event_adapter(self):
+ storm_object = FakeStormClass()
+ storm_object.id = 1234
+ event = ILongPollEvent(storm_object)
+ self.assertThat(event, Provides(ILongPollEvent))
+ self.assertEqual(
+ "longpoll.event.faketable.1234",
+ event.event_key)
+
def test_storm_object_created(self):
storm_object = FakeStormClass()
storm_object.id = 1234
with capture_longpoll_emissions() as log:
notify(ObjectCreatedEvent(storm_object))
- expected = [
- LongPollEventRecord(
- "longpoll.event.faketable.1234.created",
- {"event_key": "longpoll.event.faketable.1234.created",
- "event_data": {}}),
- ]
- self.assertEqual(expected, log)
+ expected = LongPollEventRecord(
+ "longpoll.event.faketable.1234",
+ {"event_key": "longpoll.event.faketable.1234",
+ "what": "created"})
+ self.assertEqual([expected], log)
def test_storm_object_deleted(self):
storm_object = FakeStormClass()
storm_object.id = 1234
with capture_longpoll_emissions() as log:
notify(ObjectDeletedEvent(storm_object))
- expected = [
- LongPollEventRecord(
- "longpoll.event.faketable.1234.deleted",
- {"event_key": "longpoll.event.faketable.1234.deleted",
- "event_data": {}}),
- ]
- self.assertEqual(expected, log)
+ expected = LongPollEventRecord(
+ "longpoll.event.faketable.1234",
+ {"event_key": "longpoll.event.faketable.1234",
+ "what": "deleted"})
+ self.assertEqual([expected], log)
def test_storm_object_modified(self):
storm_object = FakeStormClass()
storm_object.id = 1234
with capture_longpoll_emissions() as log:
- notify(ObjectModifiedEvent(
- storm_object, storm_object, ("itchy", "scratchy")))
- expected = [
- LongPollEventRecord(
- "longpoll.event.faketable.1234.modified",
- {"event_key": "longpoll.event.faketable.1234.modified",
- "event_data": {"edited_fields": ["itchy", "scratchy"]}}),
- ]
- self.assertEqual(expected, log)
+ object_event = ObjectModifiedEvent(
+ storm_object, storm_object, ("itchy", "scratchy"))
+ notify(object_event)
+ expected = LongPollEventRecord(
+ "longpoll.event.faketable.1234",
+ {"event_key": "longpoll.event.faketable.1234",
+ "what": "modified",
+ "edited_fields": ["itchy", "scratchy"]})
+ self.assertEqual([expected], log)
=== modified file 'lib/lp/services/longpoll/adapters/tests/test_subscriber.py'
--- lib/lp/services/longpoll/adapters/tests/test_subscriber.py 2011-09-16 14:59:47 +0000
+++ lib/lp/services/longpoll/adapters/tests/test_subscriber.py 2011-09-21 09:20:47 +0000
@@ -15,13 +15,10 @@
from zope.interface import implements
from canonical.launchpad.webapp.servers import LaunchpadTestRequest
-from canonical.testing.layers import (
- BaseLayer,
- LaunchpadFunctionalLayer,
- )
+from canonical.testing.layers import LaunchpadFunctionalLayer
from lp.services.longpoll.adapters.subscriber import (
generate_subscribe_key,
- LongPollSubscriber,
+ LongPollApplicationRequestSubscriber,
)
from lp.services.longpoll.interfaces import (
ILongPollEvent,
@@ -51,12 +48,12 @@
def test_interface(self):
request = LaunchpadTestRequest()
- subscriber = LongPollSubscriber(request)
+ subscriber = LongPollApplicationRequestSubscriber(request)
self.assertProvides(subscriber, ILongPollSubscriber)
def test_subscribe_key(self):
request = LaunchpadTestRequest()
- subscriber = LongPollSubscriber(request)
+ subscriber = LongPollApplicationRequestSubscriber(request)
# A subscribe key is not generated yet.
self.assertIs(subscriber.subscribe_key, None)
# It it only generated on the first subscription.
@@ -71,7 +68,8 @@
def test_adapter(self):
request = LaunchpadTestRequest()
subscriber = ILongPollSubscriber(request)
- self.assertIsInstance(subscriber, LongPollSubscriber)
+ self.assertIsInstance(
+ subscriber, LongPollApplicationRequestSubscriber)
# A difference subscriber is returned on subsequent adaptions, but it
# has the same subscribe_key.
subscriber2 = ILongPollSubscriber(request)
@@ -79,8 +77,8 @@
self.assertEqual(subscriber.subscribe_key, subscriber2.subscribe_key)
def test_subscribe_queue(self):
- # LongPollSubscriber.subscribe() creates a new queue with a new unique
- # name that is bound to the event's event_key.
+ # LongPollApplicationRequestSubscriber.subscribe() creates a new queue
+ # with a new unique name that is bound to the event's event_key.
request = LaunchpadTestRequest()
event = FakeEvent()
subscriber = ILongPollSubscriber(request)
@@ -93,8 +91,8 @@
message, subscribe_queue.receive(timeout=5))
def test_json_cache_not_populated_on_init(self):
- # LongPollSubscriber does not put the name of the new queue into the
- # JSON cache.
+ # LongPollApplicationRequestSubscriber does not put the name of the
+ # new queue into the JSON cache.
request = LaunchpadTestRequest()
cache = IJSONRequestCache(request)
self.assertThat(cache.objects, Not(Contains("longpoll")))
@@ -124,8 +122,6 @@
class TestFunctions(TestCase):
- layer = BaseLayer
-
def test_generate_subscribe_key(self):
subscribe_key = generate_subscribe_key()
expected_prefix = "longpoll.subscribe."
=== modified file 'lib/lp/services/longpoll/configure.zcml'
--- lib/lp/services/longpoll/configure.zcml 2011-09-21 09:20:47 +0000
+++ lib/lp/services/longpoll/configure.zcml 2011-09-21 09:20:47 +0000
@@ -6,7 +6,8 @@
xmlns:browser="http://namespaces.zope.org/browser";
xmlns:i18n="http://namespaces.zope.org/i18n";
i18n_domain="launchpad">
- <adapter factory=".adapters.subscriber.LongPollSubscriber" />
+ <adapter factory=".adapters.storm.LongPollStormEvent" />
+ <adapter factory=".adapters.subscriber.LongPollApplicationRequestSubscriber" />
<subscriber handler=".adapters.storm.object_created" />
<subscriber handler=".adapters.storm.object_deleted" />
<subscriber handler=".adapters.storm.object_modified" />
=== modified file 'lib/lp/services/longpoll/interfaces.py'
--- lib/lp/services/longpoll/interfaces.py 2011-09-20 10:14:41 +0000
+++ lib/lp/services/longpoll/interfaces.py 2011-09-21 09:20:47 +0000
@@ -22,20 +22,14 @@
source = Attribute("The event source.")
- event = Attribute("An object indicating the type of event.")
-
event_key = Attribute(
"The key with which events will be emitted. Should be predictable "
"and stable.")
- def emit(data):
+ def emit(**data):
"""Emit the given data to `event_key`.
- The data will be wrapped up into a `dict` with the keys `event_key`
- and `event_data`, where `event_key` is a copy of `self.event_key` and
- `event_data` is the `data` argument.
-
- :param data: Any data structure that can be dumped as JSON.
+ :param data: Any data structures that can be dumped as JSON.
"""
@@ -53,19 +47,14 @@
"""
-def long_poll_event(source_spec, event_spec=basestring):
+def long_poll_event(source_spec):
"""Class decorator to declare an `ILongPollEvent`.
:param source_spec: An interface or other specification understood by
`zope.component` (a plain class can be passed too) that defines the
source of an event. `IJob` or `storm.base.Storm` for example.
- :param source_event: An interface or other specification understood by
- `zope.component`. The exact use here is left to implementers. By
- default it is `basestring` so that terms like "modified" or
- "lifecycle" can be used when looking up the event, but it could also
- be `IObjectModifiedEvent`. The dominant use case is evolving.
"""
- declare_adapter = adapter(source_spec, event_spec)
+ declare_adapter = adapter(source_spec)
def declare_event(cls):
classImplements(cls, ILongPollEvent)
=== modified file 'lib/lp/services/longpoll/tests/test_interfaces.py'
--- lib/lp/services/longpoll/tests/test_interfaces.py 2011-09-16 16:07:59 +0000
+++ lib/lp/services/longpoll/tests/test_interfaces.py 2011-09-21 09:20:47 +0000
@@ -19,30 +19,15 @@
"""Test interface for an event source."""
-class IEventSpecifierInterface(Interface):
- """Test interface for an event specifier."""
-
-
class TestLongPollInterfaces(TestCase):
def test_long_poll_event(self):
# long_poll_event is a class decorator that declares a class as an
# ILongPollEvent.
- @long_poll_event(IEventSourceInterface, IEventSpecifierInterface)
- class Something:
- """An example event source."""
- self.assertTrue(ILongPollEvent.implementedBy(Something))
- self.assertEqual(
- (IEventSourceInterface, IEventSpecifierInterface),
- adaptedBy(Something))
-
- def test_long_poll_event_default(self):
- # By default, long_poll_event assumes that the event spec is
- # basestring.
@long_poll_event(IEventSourceInterface)
class Something:
"""An example event source."""
self.assertTrue(ILongPollEvent.implementedBy(Something))
self.assertEqual(
- (IEventSourceInterface, basestring),
+ (IEventSourceInterface,),
adaptedBy(Something))
