[sugar] review: Guillaume's 'activity' branch of Gadget

[Dafydd Harries]

> diff --git a/gadget/component.py b/gadget/component.py
> index b5fc702..dd85a03 100644
> --- a/gadget/component.py
> +++ b/gadget/component.py
> @@ -92,8 +92,9 @@ class Room(object):
>      def __init__(self, own_nick):
>          self.own_nick = own_nick
>          self.membership = None
> -        self.members = {}
> +        self.members = set()
>          self.properties = {}
> +        self.id = None
>  

Would it be possible to just have a Room.activity property and use that to
store members/id?

>  class GadgetService(component.Service):
>      debug = False
> @@ -333,7 +334,7 @@ class GadgetService(component.Service):
>      def message(self, stanza):
>          type = stanza.getAttribute('type', 'normal')
>  
> -        if type in ('chat', 'groupchat'):
> +        if type in ('chat'):
>              return

This doesn't do quite what you want. "()" doesn't create a tuple, "," does, so
this is the same as "type in 'chat'", which will be true if type is a
substring of "chat". Better to just say "==".

>  
>          if type == 'normal':
> @@ -343,12 +344,17 @@ class GadgetService(component.Service):
>                      return
>                  elif (elem.uri == ns.ACTIVITY_PROP and
>                        elem.name == 'properties'):
> -                    self.message_activity_properties(stanza, elem)
> +                    self.message_pre_invite_activity_properties(stanza, elem)
>                      return
>                  elif elem.uri == ns.PUBSUB_EVENT and elem.name == 'event':
>                      self.message_pubsub_notification(stanza, elem)
>                      return
>  
> +        elif type == 'groupchat':
> +            for elem in stanza.elements():
> +                if elem.uri == ns.ACTIVITY_PROP and elem.name == 'properties':
> +                    self.message_activity_properties(stanza, elem)

You should return here. Perhaps we should dispatch messages more like we
dispatch IQs, i.e. using a dict.

> +
>          # FIXME: handle close query stanza
>          log.msg('got unhandled <message>')
>  
> @@ -357,7 +363,7 @@ class GadgetService(component.Service):
>              if elem.uri == ns.MUC_USER and elem.name == 'invite':
>                  self.message_muc_invite(stanza, elem)
>  
> -    def create_room(self, jid, properties):
> +    def create_room(self, jid, properties, id):

Hmm, can we put the id before the properties? I think it's more aesthetically
pleasing that way.

>          if jid in self.mucs:
>              # We already have a room by this name.
>              return
> @@ -366,9 +372,10 @@ class GadgetService(component.Service):
>          # join it.
>          room = Room('inspector')
>          room.properties = properties
> +        room.id = id
>          self.mucs[jid] = room
>  
> -    def message_activity_properties(self, stanza, properties_elem):
> +    def message_pre_invite_activity_properties(self, stanza, properties_elem):
>          # XXX Restrictions on from address?
>  
>          try:
> @@ -384,7 +391,7 @@ class GadgetService(component.Service):
>          if not (properties and activity and room_jid):
>              return
>  
> -        self.create_room(room_jid, properties)
> +        self.create_room(room_jid, properties, activity)

Perhaps we should rename 'activity' to 'activity_id' for clarity.

>      def message_muc_invite(self, stanza, invite):
>          to = stanza.getAttribute('to')
> @@ -502,17 +509,54 @@ class GadgetService(component.Service):
>                  # Presence for our in-room self; we've finished joining the
>                  # room.
>                  room.membership = 'joined'
> +
> +                # activities are created and added to the model when the
> +                # inspector actually joined the muc.

English nitpicks: Your tenses don't agree here. "are created" ... "joined".
s/joined/join/. If you use a full stop, you should also capitalise the first
letter.

> +                # If we'll do it before, we won't be able to properly track
> +                # activity's properties and members.

I don't understand this comment.

> +                activity = self.model.activity_add(room.id, room_jid,
> +                        room.properties)
> +
> +                # Activity and Room now share the same set() object
> +                activity.members = room.members
>                  return
>  
> +        item = xpath_query('/presence/x[@xmlns="%s"]/item' % ns.MUC_USER,
> +                stanza)
> +        if not item or item[0].getAttribute('jid') is None:
> +            log.msg("full jid of %s missing. Can't update activity members"
> +                    % jid.resource)

English nitpicks: you should capitalise "full" and have a full stop after
the second sentence.

> +            return
> +
> +        full_jid = JID(item[0]['jid']).userhost()

Hmm, "full_jid" seems like a bad name, since it implies a JID with a resource.
Perhaps it should be "real_jid" or just "jid".

> +
> +        activities = self.model.activity_by_room(room_jid)
> +
>          if type is None:
>              log.msg('room %s has member %s' % (room_jid, jid.resource))
> -            room.members[jid.resource] = True
> +            room.members.add(full_jid)
> +
>          elif type == 'unavailable':
>              if room_jid in self.mucs:
>                  room = self.mucs[room_jid]
>  
> -                if jid.resource in room.members:
> -                    del room.members[jid.resource]
> +                if full_jid in room.members:
> +                    room.members.remove(full_jid)
> +
> +                    if len(room.members) == 0:
> +                        self.leave_activity(room_jid, room.id)
> +
> +    def leave_activity(self, room_jid, activity_id):

If we have Room.activity you won't need to pass around the ID here.

> +        log.msg('leave room %s' % room_jid)
> +

Should be "leaving room".

> +        presence = domish.Element((None, 'presence'))
> +        presence['to'] = '%s/inspector' % room_jid

You should use the JID class to generate the JID, and use Room.own_nick for
the resource.

> +        presence['type'] = 'unavailable'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        self.send(presence)
> +
> +        self.model.activity_remove(activity_id)
> +        del self.mucs[room_jid]
>  
>      def message_pubsub_notification(self, stanza, event):
>          items = event.firstChildElement()
> @@ -533,3 +577,33 @@ class GadgetService(component.Service):
>                  self.model.buddy_add(jid, props)
>  
>              # FIXME: send notification messages to interested buddies
> +
> +    def message_activity_properties(self, stanza, properties_elem):
> +        id = properties_elem.getAttribute('activity')
> +        room_jid = properties_elem.getAttribute('room')
> +        jid = JID(stanza['from'])
> +
> +        if room_jid != jid.userhost():
> +            log.msg("Ignored activity properties message from %s trying to change properties for another room"
> +                    % stanza['from'])

Can you wrap this to 79 characters? You can just split the string literal in
two.

> +            return
> +
> +        try:
> +            properties = parse_properties(properties_elem)
> +        except PropertyTypeError:
> +            log.msg("Ignored activity properties message from %s containing an invalid property"
> +                    % stanza['from'])

Same here.

> +            return
> +
> +        if not self.model.has_activity(id):
> +            log.msg("Ignored activity properties message from %s about an invalid activity: %s "
> +                    % stanza['from'], id)

And here.

> +            return
> +
> +        # is activity private?
> +        if properties.get('private') in (('bool', '1'), ('bool', 'true')):

Perhaps we should do some kind of normalisation in parse_properties, and maybe
ignore malformed values.

> +            log.msg('Activity %s is not private. Leave it' % id)

s/Leave/Leaving/.

> +            self.leave_activity(room_jid, id)
> +            return
> +
> +        self.model.activity_update(id, properties)
> diff --git a/gadget/model.py b/gadget/model.py
> index faf4967..1b33734 100644
> --- a/gadget/model.py
> +++ b/gadget/model.py
> @@ -3,7 +3,7 @@ import random
>  class Activity(object):
>      __slots__ = ('id', 'room', 'properties', 'members')
>  
> -    def __init__(self, id, room, properties=None):
> +    def __init__(self, id, room, properties=None, members=None):
>          self.id = id
>          self.room = room
>  
> @@ -12,7 +12,10 @@ class Activity(object):
>          else:
>              self.properties = {}
>  
> -        self.members = []
> +        if members:
> +            self.members = members
> +        else:
> +            self.members = set()
>  
>      def __repr__(self):
>          return '<Activity %r %r>' % (self.id, self.room)
> @@ -85,7 +88,7 @@ class GadgetModel(object):
>          # FIXME: this is not very efficient
>          results = []
>          for activity in self.activities.itervalues():
> -            if set(buddies).issubset(set(activity.members)):
> +            if set(buddies).issubset(activity.members):
>                      results.append(activity)
>  
>          return results
> @@ -197,3 +200,6 @@ class GadgetModel(object):
>  
>      def has_buddy(self, jid):
>          return self.buddies.has_key(jid)
> +
> +    def has_activity(self, id):
> +        return self.activities.has_key(id)
> diff --git a/gadget/test_activity_query.py b/gadget/test_activity_query.py
> index dc8dc58..707c332 100644
> --- a/gadget/test_activity_query.py
> +++ b/gadget/test_activity_query.py
> @@ -25,7 +25,7 @@ def parse_activities_result(stanza):
>                  if child.name == 'member':
>                      jid = child.getAttribute('jid')
>                      assert jid
> -                    activities[-1].members.append(jid)
> +                    activities[-1].members.add(jid)
>  
>      return sorted(activities)
>  
> @@ -79,8 +79,8 @@ class ActivityIqTest3(TestCase):
>  
>          model = self.service.model
>          activity = model.activity_add(1, 'foo', {})
> -        activity.members.append('bob at foo.org')
> -        activity.members.append('tom at foo.org')
> +        activity.members.add('bob at foo.org')
> +        activity.members.add('tom at foo.org')
>  
>          iq = IQ(self.server, "get")
>          iq['from'] = 'bob at foo.org'
> @@ -93,7 +93,7 @@ class ActivityIqTest3(TestCase):
>          activities = parse_activities_result(stanza)
>          assert len(activities) == 1
>          assert activities[0].id == '1'
> -        assert activities[0].members == ['bob at foo.org', 'tom at foo.org']
> +        assert activities[0].members == set(['bob at foo.org', 'tom at foo.org'])
>          self.done.callback(None)
>  
>  class ActivityIqTest4(TestCase):
> @@ -161,12 +161,12 @@ class ActivityIqTest6(TestCase):
>  
>          model = self.service.model
>          activity = model.activity_add(1, 'foo', {'type': ('str', 'Paint')})
> -        activity.members.append('alice at foo.org')
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('alice at foo.org')
> +        activity.members.add('bob at foo.org')
>          activity = model.activity_add(2, 'bar', {'type': ('str', 'Connect')})
> -        activity.members.append('alice at foo.org')
> +        activity.members.add('alice at foo.org')
>          activity = model.activity_add(3, 'test', {'type': ('str', 'Paint')})
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('bob at foo.org')
>  
>          iq = IQ(self.server, "get")
>          iq['from'] = 'bob at foo.org'
> @@ -194,12 +194,12 @@ class ActivityIqTest7(TestCase):
>  
>          model = self.service.model
>          activity = model.activity_add(1, 'foo', {'type': ('str', 'Paint')})
> -        activity.members.append('alice at foo.org')
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('alice at foo.org')
> +        activity.members.add('bob at foo.org')
>          activity = model.activity_add(2, 'bar', {'type': ('str', 'Connect')})
> -        activity.members.append('alice at foo.org')
> +        activity.members.add('alice at foo.org')
>          activity = model.activity_add(3, 'test', {'type': ('str', 'Paint')})
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('bob at foo.org')
>  
>          iq = IQ(self.server, "get")
>          iq['from'] = 'bob at foo.org'
> @@ -231,12 +231,12 @@ class ActivityIqTest8(TestCase):
>  
>          model = self.service.model
>          activity = model.activity_add(1, 'foo', {'type': ('str', 'Paint')})
> -        activity.members.append('alice at foo.org')
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('alice at foo.org')
> +        activity.members.add('bob at foo.org')
>          activity = model.activity_add(2, 'bar', {'type': ('str', 'Connect')})
> -        activity.members.append('alice at foo.org')
> +        activity.members.add('alice at foo.org')
>          activity = model.activity_add(3, 'test', {'type': ('str', 'Paint')})
> -        activity.members.append('bob at foo.org')
> +        activity.members.add('bob at foo.org')
>  
>          iq = IQ(self.server, "get")
>          iq['from'] = 'bob at foo.org'
> diff --git a/gadget/test_component.py b/gadget/test_component.py
> index 4325593..00d5270 100644
> --- a/gadget/test_component.py
> +++ b/gadget/test_component.py
> @@ -9,7 +9,7 @@ from twisted.words.protocols.jabber.client import IQ
>  
>  from component import GadgetService, parse_properties, properties_to_xml,\
>          PropertyTypeError
> -from model import GadgetModel
> +from model import GadgetModel, Activity
>  
>  import ns
>  
> @@ -214,7 +214,7 @@ class RoomMembershipTest(TestCase):
>          self.done = defer.Deferred()
>          self.server.addOnetimeObserver('//presence', self.presence)
>          # Simulate invitation process.
> -        self.service.create_room('chat at conf.foo.org', {})
> +        self.service.create_room('chat at conf.foo.org', {}, 'activity1')
>          self.service.invited('inspector at gadget.bar.org', 'chat at conf.foo.org')
>          return self.done
>  
> @@ -222,11 +222,15 @@ class RoomMembershipTest(TestCase):
>          presence = domish.Element((ns.CLIENT, 'presence'))
>          presence['from'] = 'chat at conf.foo.org/amy'
>          x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'amy at foo.org/resource'
>          self.server.send(presence)
>  
>          presence = domish.Element((ns.CLIENT, 'presence'))
>          presence['from'] = 'chat at conf.foo.org/bob'
>          x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'bob at foo.org/resource'
>          self.server.send(presence)

We should probably have a test that includes a MUC presence message without a
jid.

Perhaps we should factor out the code for creating MUC presence messages here.

>          presence = domish.Element((ns.CLIENT, 'presence'))
> @@ -239,22 +243,30 @@ class RoomMembershipTest(TestCase):
>      def blah(self):
>          assert 'chat at conf.foo.org' in self.service.mucs
>          room = self.service.mucs['chat at conf.foo.org']
> -        assert sorted(room.members.keys()) == ['amy', 'bob']
> +        assert room.members == set(['amy at foo.org', 'bob at foo.org'])
> +        activity = self.model.activity_by_id('activity1')
> +        assert activity.members == set(['amy at foo.org', 'bob at foo.org'])
>  
>          # Amy leaves the room.
>          presence = domish.Element((ns.CLIENT, 'presence'))
>          presence['from'] = 'chat at conf.foo.org/amy'
>          presence['type'] = 'unavailable'
>          x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'amy at foo.org/resource'
>          self.server.send(presence)
>  
>          # Cat joins the room.
>          presence = domish.Element((ns.CLIENT, 'presence'))
>          presence['from'] = 'chat at conf.foo.org/cat'
>          x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'cat at foo.org/resource'
>          self.server.send(presence)
>  
> -        assert sorted(room.members.keys()) == ['bob', 'cat']
> +        assert room.members == set(['bob at foo.org', 'cat at foo.org'])
> +        activity = self.model.activity_by_id('activity1')
> +        assert activity.members == set(['bob at foo.org', 'cat at foo.org'])
>          self.done.callback(None)
>  
>  class PropertiesTest(TestCase):
> @@ -528,3 +540,224 @@ class BuddyPropertiesChangeTest(TestCase):
>          assert buddy.properties == {'color': ('str', 'blue'),
>              'key': ('str', 'my key')}
>          self.done.callback(None)
> +
> +class ActivityPropertiesTest(TestCase):
> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.server.addOnetimeObserver('//presence', self.presence)
> +        # Simulate invitation process.
> +        self.service.create_room('chat at conf.localhost', {}, 'activity1')
> +        self.service.invited('inspector at gadget.localhost', 'chat at conf.localhost')
> +        return self.done
> +
> +    def presence(self, _):
> +        # bob muc presence

s/bob/Bob's/

> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/bob'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'bob at foo.org/resource'
> +        self.server.send(presence)
> +
> +        # gadget muc presence

s/gadget/Gadget's/

> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/inspector'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        self.server.send(presence)
> +
> +        reactor.callLater(0, self.later)
> +
> +    def later(self):
> +        assert 'chat at conf.localhost' in self.service.mucs
> +        room = self.service.mucs['chat at conf.localhost']
> +        assert room.members == set(['bob at foo.org'])
> +
> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost', {},
> +                set(['bob at foo.org']))
> +
> +        # bob change activity properties

s/change/changes/

> +        message = domish.Element((None, 'message'))
> +        message['type'] = 'groupchat'
> +        message['to'] = 'chat at conf.localhost'
> +        message['from'] = 'chat at conf.localhost/bob'
> +        properties = message.addElement((ns.ACTIVITY_PROP, 'properties'))
> +        properties['activity'] = 'activity1'
> +        properties['room'] = 'chat at conf.localhost'
> +        property = properties.addElement((None, 'property'))
> +        property['type'] = 'str'
> +        property['name'] = 'title'
> +        property.addContent('Test Activity')
> +        self.server.send(message)
> +
> +        reactor.callLater(0, self.later2)
> +
> +    def later2(self):
> +

Delete this blank line.

> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost',
> +                {'title': ('str', 'Test Activity')}, set(['bob at foo.org']))
> +
> +        self.done.callback(None)
> +
> +class ActivityLeaveTest(TestCase):
> +    """Gaget leave and remove activity when the latest buddy left"""

"Gadget leaves an activity and forgets about it when the last buddy leaves."

> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.server.addOnetimeObserver('//presence', self.presence)
> +        # Simulate invitation process.
> +        self.service.create_room('chat at conf.localhost', {}, 'activity1')
> +        self.service.invited('inspector at gadget.localhost', 'chat at conf.localhost')

Needs wrapping.

> +        return self.done
> +
> +    def presence(self, _):
> +        # georges muc presence

George's.

> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/georges'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'georges at foo.org/resource'
> +        self.server.send(presence)
> +
> +        # gadget muc presence
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/inspector'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        self.server.send(presence)
> +
> +        reactor.callLater(0, self.later)
> +
> +    def later(self):
> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost', {},
> +                set(['georges at foo.org']))
> +
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/georges'
> +        presence['type'] = 'unavailable'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'georges at foo.org/resource'
> +        self.server.send(presence)
> +
> +        reactor.callLater(0, self.later2)
> +
> +    def later2(self):
> +
> +        assert not self.model.has_activity('activity1')
> +        assert len(self.service.mucs) == 0
> +        self.done.callback(None)
> +
> +class ActivityPrivate(TestCase):

Maybe "ActivityBecomesPrivate"?

> +    """Gaget leave and remove activity when it becomes private"""
> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.server.addOnetimeObserver('//presence', self.presence)
> +        # Simulate invitation process.
> +        self.service.create_room('chat at conf.localhost', {}, 'activity1')
> +        self.service.invited('inspector at gadget.localhost', 'chat at conf.localhost')
> +        return self.done
> +
> +    def presence(self, _):
> +        # georges muc presence
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/georges'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'georges at foo.org/resource'
> +        self.server.send(presence)
> +
> +        # gadget muc presence
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/inspector'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        self.server.send(presence)
> +
> +        reactor.callLater(0, self.later)
> +
> +    def later(self):
> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost', {},
> +                set(['georges at foo.org']))
> +
> +        # Georges makes activity private
> +        message = domish.Element((None, 'message'))
> +        message['type'] = 'groupchat'
> +        message['to'] = 'chat at conf.localhost'
> +        message['from'] = 'chat at conf.localhost/georges'
> +        properties = message.addElement((ns.ACTIVITY_PROP, 'properties'))
> +        properties['activity'] = 'activity1'
> +        properties['room'] = 'chat at conf.localhost'
> +        property = properties.addElement((None, 'property'))
> +        property['type'] = 'bool'
> +        property['name'] = 'private'
> +        property.addContent('1')
> +        self.server.send(message)
> +
> +        reactor.callLater(0, self.later2)
> +
> +    def later2(self):
> +
> +        assert not self.model.has_activity('activity1')
> +        assert len(self.service.mucs) == 0
> +        self.done.callback(None)
> +
> +class ActivityPropertiesSpoofTest(TestCase):
> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.server.addOnetimeObserver('//presence', self.presence)
> +        # Simulate invitation process.
> +        self.service.create_room('chat at conf.localhost', {}, 'activity1')
> +        self.service.invited('inspector at gadget.localhost', 'chat at conf.localhost')
> +        return self.done
> +
> +    def presence(self, _):
> +        # bob muc presence
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/bob'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        item = x.addElement('item')
> +        item['jid'] = 'bob at foo.org/resource'
> +        self.server.send(presence)
> +
> +        # gadget muc presence
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'chat at conf.localhost/inspector'
> +        x = presence.addElement((ns.MUC_USER, 'x'))
> +        self.server.send(presence)
> +
> +        reactor.callLater(0, self.later)
> +
> +    def later(self):
> +        assert 'chat at conf.localhost' in self.service.mucs
> +        room = self.service.mucs['chat at conf.localhost']
> +        assert room.members == set(['bob at foo.org'])
> +
> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost', {},
> +                set(['bob at foo.org']))
> +
> +        # bad oscar tries to change properties but is not in the muc
> +        message = domish.Element((None, 'message'))
> +        message['type'] = 'groupchat'
> +        message['to'] = 'anotherchat at conf.localhost'
> +        message['from'] = 'anotherchat at conf.localhost/oscar'
> +        properties = message.addElement((ns.ACTIVITY_PROP, 'properties'))
> +        properties['activity'] = 'activity1'
> +        properties['room'] = 'chat at conf.localhost'
> +        property = properties.addElement((None, 'property'))
> +        property['type'] = 'str'
> +        property['name'] = 'title'
> +        property.addContent('Test Activity')
> +        self.server.send(message)
> +
> +        reactor.callLater(0, self.later2)
> +
> +    def later2(self):
> +
> +        activity = self.service.model.activity_by_id('activity1')
> +        assert activity == Activity('activity1', 'chat at conf.localhost', {},
> +                set(['bob at foo.org']))
> +
> +        self.done.callback(None)
> +
> diff --git a/gadget/test_model.py b/gadget/test_model.py
> index 08e2791..3d52c34 100644
> --- a/gadget/test_model.py
> +++ b/gadget/test_model.py
> @@ -6,8 +6,10 @@ from gadget.model import GadgetModel
>  class ModelTest(unittest.TestCase):
>      def test_activity_search(self):
>          model = GadgetModel()
> +        assert not model.has_activity('123')
>          activity = model.activity_add('123', 'room at conf.foo.org',
>                  {'type': ('str', 'Paint')})
> +        assert model.has_activity('123')
>  
>          self.assertEquals(activity, model.activity_by_id('123'))
>          self.assertRaises(KeyError, model.activity_by_id, '124')
> @@ -24,15 +26,15 @@ class ModelTest(unittest.TestCase):
>          results = model.activity_by_room('room at conf.bar.org')
>          self.assertEquals([], results)
>  
> -        activity.members.append('alice at foo.org')
> +        activity.members.add('alice at foo.org')
>          results = model.activity_by_participants(['alice at foo.org'])
>          self.assertEquals([activity], results)
>          results = model.activity_by_participants(['boo at foo.org'])
>          self.assertEquals([], results)
>  
>          activity2 = model.activity_add('456', 'room2 at conf.foo.org', {})
> -        activity2.members.append('alice at foo.org')
> -        activity2.members.append('bob at foo.org')
> +        activity2.members.add('alice at foo.org')
> +        activity2.members.add('bob at foo.org')
>          results = model.activity_by_participants(['alice at foo.org'])
>          self.assertEquals([activity, activity2], sorted(results))
>          results = model.activity_by_participants(

-- 
Dafydd