[sugar] review: gadget presence branch

[Dafydd Harries]

A review of the 'presence' branch in Guillaume's Gadget tree, as taken from:

  https://dev.laptop.org/git?p=users/guillaume/gadget;a=shortlog;h=presence

> diff --git a/gadget/component.py b/gadget/component.py
> index 6fcb682..f52e780 100644
> --- a/gadget/component.py
> +++ b/gadget/component.py
> @@ -83,6 +83,7 @@ class GadgetService(component.Service):
>          xmlstream.addObserver('//message', self.message)
>          xmlstream.addObserver('//presence', self.presence)
>  
> +        # FIXME: we should send presence to buddies subscribed to gadget. How?
>          if self.debug:
>              self.xmlstream.setDispatchFn(self.onElement)
>  

I'm guessing your roster branch answers this question.

> @@ -235,6 +236,9 @@ class GadgetService(component.Service):
>      def presence(self, stanza):
>          type = stanza.getAttribute('type')
>          from_ = stanza.getAttribute('from')
> +        # remove the ressource
> +        jid = from_.split('/')[0]
> +        to = stanza.getAttribute('to')
>  
>          if from_ is None:
>              return

Twisted has code for handling JIDs; let's use that.

  >>> from twisted.words.protocols.jabber import jid
  >>> jid.JID('foo at bar/baz').userhost()
  'foo at bar'

> @@ -249,20 +253,47 @@ class GadgetService(component.Service):
>              result = domish.Element((None, 'presence'))
>              result['type'] = 'subscribed'
>              result['to'] = from_
> +            result['from'] = to
>              self.send(result)
>  
> -            log.msg('%s subscribed to gadget' % from_)
> +            log.msg('%s subscribed to gadget' % jid)
>              # request subscription to the buddy
>              result = domish.Element((None, 'presence'))
>              result['type'] = 'subscribe'
>              result['to'] = from_
> +            result['from'] = to
>              self.send(result)
> +
>          elif type == 'subscribed':
> -            log.msg('subscribed to %s' % from_)
> -            # FIXME: add buddy to the model ?
> +            log.msg('subscribed to %s' % jid)
> +            # send gadget's presence
> +            presence = domish.Element((None, 'presence'))
> +            presence['to'] = from_
> +            presence['from'] = to
> +            self.send(presence)
> +            # FIXME: add buddy to model
> +
>          elif type == 'unsubscribed':
> -            log.msg('unsubscribed from %s' % from_)
> -            # FIXME: remove buddy from the model ?
> +            log.msg('unsubscribed from %s' % jid)
> +            # the buddy will be removed from the model when we'll receive
> +            # the 'unavailable' presence
> +
> +        elif type == 'probe':
> +            log.msg('%s is online' % jid)
> +            self.model.buddy_add(jid, {})
> +
> +            # send gadget's presence
> +            presence = domish.Element((None, 'presence'))
> +            presence['to'] = from_
> +            presence['from'] = to
> +            self.send(presence)
> +
> +        elif type == 'unavailable':
> +            log.msg('%s is offline' % jid)
> +            try:
> +                self.model.buddy_remove(jid)
> +            except KeyError:
> +                pass
>  
>      def muc_presence(self, stanza):
>          type = stanza.getAttribute('type')
> diff --git a/gadget/test_component.py b/gadget/test_component.py
> index 4d4a808..5382edb 100644
> --- a/gadget/test_component.py
> +++ b/gadget/test_component.py
> @@ -238,6 +238,7 @@ class SubscriptionTest(TestCase):
>  
>          presence = domish.Element((ns.CLIENT, 'presence'))
>          presence['from'] = 'bob at foo.org'
> +        presence['to'] = 'gadget.foo.org'
>          presence['type'] = 'subscribe'
>          self.server.send(presence)
>          return self.done
> @@ -250,6 +251,50 @@ class SubscriptionTest(TestCase):
>          assert stanza['type'] == 'subscribe'
>          self.done.callback(None)
>  
> +class ProbePresenceTest(TestCase):
> +    """test if gadget adds buddies to the model when it receives their
> +    probe presence"""
> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.server.addOnetimeObserver('//presence', self.presence)
> +
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'bob at foo.org/Ressource'

s/Ressource/Resource/.

> +        presence['to'] = 'gadget.foo.org'
> +        presence['type'] = 'probe'
> +        self.server.send(presence)
> +        return self.done
> +
> +    def presence(self, stanza):
> +        assert stanza.name == 'presence'
> +
> +        buddy = self.service.model.buddy_by_id('bob at foo.org')
> +        assert buddy.jid == 'bob at foo.org'
> +
> +        self.done.callback(None)
> +
> +class UnavailablePresenceTest(TestCase):
> +    """test if gadget removes buddies from the model when it receives their
> +    unavailable presence"""
> +    def runTest(self):
> +        self.done = defer.Deferred()
> +        self.service.model.buddy_add('bob at foo.org', {})
> +
> +        presence = domish.Element((ns.CLIENT, 'presence'))
> +        presence['from'] = 'bob at foo.org'
> +        presence['to'] = 'gadget.foo.org'
> +        presence['type'] = 'unavailable'
> +        self.server.send(presence)
> +        reactor.callLater(0, self.later)
> +
> +        return self.done
> +
> +    def later(self):
> +
> +        self.assertRaises(KeyError, self.service.model.buddy_by_id,
> +                'bob at foo.org')
> +        self.done.callback(None)
> +
>  class InviteTest(TestCase):
>      def runTest(self):
>          self.done = defer.Deferred()

Looks great otherwise. Please merge!

-- 
Dafydd