Prevent roster_update from firing twice after retrieving the roster.

2 files changed, 28 insertions, 29 deletions

diff --git a/sleekxmpp/clientxmpp.py b/sleekxmpp/clientxmpp.py
index d2f24d16..02f7776a 100644
--- a/sleekxmpp/clientxmpp.py
+++ b/sleekxmpp/clientxmpp.py
@@ -20,7 +20,7 @@ from sleekxmpp.stanza import StreamFeatures
 from sleekxmpp.basexmpp import BaseXMPP
 from sleekxmpp.exceptions import XMPPError
 from sleekxmpp.xmlstream import XMLStream
-from sleekxmpp.xmlstream.matcher import MatchXPath
+from sleekxmpp.xmlstream.matcher import StanzaPath, MatchXPath
 from sleekxmpp.xmlstream.handler import Callback
 
 # Flag indicating if DNS SRV records are available for use.
@@ -103,9 +103,7 @@ class ClientXMPP(BaseXMPP):
                          self._handle_stream_features))
         self.register_handler(
                 Callback('Roster Update',
-                         MatchXPath('{%s}iq/{%s}query' % (
-                             self.default_ns,
-                             'jabber:iq:roster')),
+                         StanzaPath('iq@type=set/roster'),
                          self._handle_roster))
 
         # Setup default stream features
@@ -230,7 +228,8 @@ class ClientXMPP(BaseXMPP):
         response = iq.send(block, timeout, callback)
 
         if block: 
-            self._handle_roster(response, request=True)
+            self.event('roster_received', response)
+            self._handle_roster(response)
             return response
 
     def _handle_connected(self, event=None):
@@ -254,32 +253,28 @@ class ClientXMPP(BaseXMPP):
                     # restarting the XML stream.
                     return True
 
-    def _handle_roster(self, iq, request=False):
+    def _handle_roster(self, iq):
         """Update the roster after receiving a roster stanza.
 
         :param iq: The roster stanza.
-        :param request: Indicates if this stanza is a response
-                        to a request for the roster, and not an
-                        empty acknowledgement from the server.
         """
-        if iq['from'].bare and iq['from'].bare != self.boundjid.bare:
-            raise XMPPError(condition='service-unavailable')
-        if iq['type'] == 'set' or (iq['type'] == 'result' and request):
-            roster = self.client_roster
-            if iq['roster']['ver']:
-                roster.version = iq['roster']['ver']
-            for jid in iq['roster']['items']:
-                item = iq['roster']['items'][jid]
-                roster[jid]['name'] = item['name']
-                roster[jid]['groups'] = item['groups']
-                roster[jid]['from'] = item['subscription'] in ['from', 'both']
-                roster[jid]['to'] = item['subscription'] in ['to', 'both']
-                roster[jid]['pending_out'] = (item['ask'] == 'subscribe')
-
-                roster[jid].save(remove=(item['subscription'] == 'remove'))
-                     
-            self.event('roster_received', iq)
-
+        if iq['type'] == 'set':
+            if iq['from'].bare and iq['from'].bare != self.boundjid.bare:
+                raise XMPPError(condition='service-unavailable')
+
+        roster = self.client_roster
+        if iq['roster']['ver']:
+            roster.version = iq['roster']['ver']
+        for jid in iq['roster']['items']:
+            item = iq['roster']['items'][jid]
+            roster[jid]['name'] = item['name']
+            roster[jid]['groups'] = item['groups']
+            roster[jid]['from'] = item['subscription'] in ['from', 'both']
+            roster[jid]['to'] = item['subscription'] in ['to', 'both']
+            roster[jid]['pending_out'] = (item['ask'] == 'subscribe')
+
+            roster[jid].save(remove=(item['subscription'] == 'remove'))
+                 
         self.event("roster_update", iq)
         if iq['type'] == 'set':
             iq.reply()
diff --git a/tests/test_stream_roster.py b/tests/test_stream_roster.py
index 2d0da173..652ea1ce 100644
--- a/tests/test_stream_roster.py
+++ b/tests/test_stream_roster.py
@@ -24,7 +24,11 @@ class TestStreamRoster(SleekTest):
         def roster_received(iq):
             events.append('roster_received')
 
+        def roster_update(iq):
+            events.append('roster_update')
+
         self.xmpp.add_event_handler('roster_received', roster_received)
+        self.xmpp.add_event_handler('roster_update', roster_update)
 
         # Since get_roster blocks, we need to run it in a thread.
         t = threading.Thread(name='get_roster', target=self.xmpp.get_roster)
@@ -62,8 +66,8 @@ class TestStreamRoster(SleekTest):
         # Give the event queue time to process.
         time.sleep(.1)
 
-        self.failUnless('roster_received' in events,
-                "Roster received event not triggered: %s" % events)
+        self.failUnless(events == ['roster_received', 'roster_update'],
+                "Wrong roster events fired: %s" % events)
 
     def testRosterSet(self):
         """Test handling pushed roster updates."""