Refactor internas of ConnectionCaps

* Make it testable and write a very basic test for the presenceCallback
 * Use Extract Method to make the code more readable and to increase reusability
 * Start to decouple ConnectionCaps from the other Connection classes/handlers

src/common/caps.py

@@ -36,9 +36,11 @@ import helpers
 import base64
 import hashlib
 
-from common.xmpp import NS_XHTML_IM, NS_RECEIPTS, NS_ESESSION, NS_CHATSTATES
+import logging
-from common.xmpp import NS_JINGLE_ICE_UDP, NS_JINGLE_RTP_AUDIO
+log = logging.getLogger('gajim.c.caps')
-from common.xmpp import NS_JINGLE_RTP_VIDEO
+
+from common.xmpp import (NS_XHTML_IM, NS_RECEIPTS, NS_ESESSION, NS_CHATSTATES,
+	NS_JINGLE_ICE_UDP, NS_JINGLE_RTP_AUDIO, NS_JINGLE_RTP_VIDEO, NS_CAPS)
 # Features where we cannot safely assume that the other side supports them
 FEATURE_BLACKLIST = [NS_CHATSTATES, NS_XHTML_IM, NS_RECEIPTS, NS_ESESSION,
 	NS_JINGLE_ICE_UDP, NS_JINGLE_RTP_AUDIO, NS_JINGLE_RTP_VIDEO]
@@ -149,6 +151,22 @@ def compute_caps_hash(identities, features, dataforms=[], hash_method='sha-1'):
 ### Internal classes of this module
 ################################################################################
 
+
+def create_suitable_client_caps(node, caps_hash, hash_method):
+	"""
+	Create and return a suitable ClientCaps object for the given node,
+	caps_hash, hash_method combination. 
+	"""
+	if not node or not caps_hash:
+		# improper caps, ignore client capabilities.
+		client_caps = NullClientCaps()
+	elif not hash_method:
+		client_caps = OldClientCaps(caps_hash, node)
+	else:
+		client_caps = ClientCaps(caps_hash, node, hash_method)
+	return client_caps
+
+
 class AbstractClientCaps(object):
 	"""
 	Base class representing a client and its capabilities as advertised by a
@@ -378,63 +396,58 @@ class CapsCache(object):
 ################################################################################
 
 class ConnectionCaps(object):
-	"""
+
-	This class highly depends on that it is a part of Connection class
+	def __init__(self, account, dispatch_event):
-	"""
+		self._account = account
+		self._dispatch_event = dispatch_event
 
 	def _capsPresenceCB(self, con, presence):
 		"""
-		Handle incoming presence stanzas... This is a callback for xmpp
+		XMMPPY callback method to handle retrieved caps info
-		registered in connection_handlers.py
 		"""
-		# we will put these into proper Contact object and ask
-		# for disco... so that disco will learn how to interpret
-		# these caps
-		pm_ctrl = None
 		try:
 			jid = helpers.get_full_jid_from_iq(presence)
 		except:
-			# Bad jid
+			log.info("Ignoring invalid JID in caps presenceCB")
 			return
-		contact = gajim.contacts.get_contact_from_full_jid(self.name, jid)
+
+		client_caps = self._extract_client_caps_from_presence(presence)
+		capscache.query_client_of_jid_if_unknown(self, jid, client_caps)
+		self._update_client_caps_of_contact(jid, client_caps)
+
+		self._dispatch_event('CAPS_RECEIVED', (jid,))
+
+	def _extract_client_caps_from_presence(self, presence):
+		caps_tag = presence.getTag('c', namespace=NS_CAPS)
+		if caps_tag:
+			hash_method, node, caps_hash = caps_tag['hash'], caps_tag['node'], caps_tag['ver']
+		else:
+			hash_method = node = caps_hash = None
+		return create_suitable_client_caps(node, caps_hash, hash_method)
+
+	def _update_client_caps_of_contact(self, jid, client_caps):
+		contact = self._get_contact_or_gc_contact_for_jid(jid)
+		if contact:
+			contact.client_caps = client_caps
+		else:
+			log.info("Received Caps from unknown contact %s" % jid)
+
+	def _get_contact_or_gc_contact_for_jid(self, jid):
+		contact = gajim.contacts.get_contact_from_full_jid(self._account, jid)
 		if contact is None:
 			room_jid, nick = gajim.get_room_and_nick_from_fjid(jid)
-			contact = gajim.contacts.get_gc_contact(
+			contact = gajim.contacts.get_gc_contact(self._account, room_jid, nick)
-				self.name, room_jid, nick)
+		return contact
-			pm_ctrl = gajim.interface.msg_win_mgr.get_control(jid, self.name)
-			if contact is None:
-				# TODO: a way to put contact not-in-roster
-				# into Contacts
-				return
-
-		caps_tag = presence.getTag('c')
-		if not caps_tag:
-			# presence did not contain caps_tag
-			client_caps = NullClientCaps()
-		else:
-			hash_method, node, caps_hash = caps_tag['hash'], caps_tag['node'], caps_tag['ver']
-
-			if not node or not caps_hash:
-				# improper caps in stanza, ignore client capabilities.
-				client_caps = NullClientCaps()
-			elif not hash_method:
-				client_caps = OldClientCaps(caps_hash, node)
-			else:
-				client_caps = ClientCaps(caps_hash, node, hash_method)
-
-		capscache.query_client_of_jid_if_unknown(self, jid, client_caps)
-		contact.client_caps = client_caps
-
-		if pm_ctrl:
-			pm_ctrl.update_contact()
 
 	def _capsDiscoCB(self, jid, node, identities, features, dataforms):
-		contact = gajim.contacts.get_contact_from_full_jid(self.name, jid)
+		"""
+		XMMPPY callback to update our caps cache with queried information after
+		we have retrieved an unknown caps hash and issued a disco
+		"""
+		contact = self._get_contact_or_gc_contact_for_jid(jid)
 		if not contact:
-			room_jid, nick = gajim.get_room_and_nick_from_fjid(jid)
+			log.info("Received Disco from unknown contact %s" % jid)
-			contact = gajim.contacts.get_gc_contact(self.name, room_jid, nick)
+			return
-			if contact is None:
-				return
 
 		lookup = contact.client_caps.get_cache_lookup_strategy()
 		cache_item = lookup(capscache)
@@ -449,5 +462,9 @@ class ConnectionCaps(object):
 				cache_item.set_and_store(identities, features)
 			else:
 				contact.client_caps = NullClientCaps()
+				log.warn("Computed and retrieved caps hash differ." +
+					"Ignoring caps of contact %s" % contact.get_full_jid())
+
+			self._dispatch_event('CAPS_RECEIVED', (jid,))
 
 # vim: se ts=3:

src/common/connection_handlers.py

@@ -1519,6 +1519,8 @@ class ConnectionHandlers(ConnectionVcard, ConnectionBytestream,
 		ConnectionPubSub.__init__(self)
 		ConnectionPEP.__init__(self, account=self.name, dispatcher=self,
 			pubsub_connection=self)
+		ConnectionCaps.__init__(self, account=self.name,
+			dispatch_event=self.dispatch)
 		ConnectionJingle.__init__(self)
 		ConnectionHandlersBase.__init__(self)
 		self.gmail_url = None

src/gui_interface.py

@@ -1997,6 +1997,14 @@ class Interface:
 			self.roster.draw_pep(jid, account, pep_type)
 			if ctrl:
 				ctrl.update_pep(pep_type)
+				
+	def handle_event_caps_received(self, account, data):
+		# ('CAPS_RECEIVED', account, (full_jid))
+		full_jid = data[0]
+		pm_ctrl = gajim.interface.msg_win_mgr.get_control(full_jid, account)
+		if pm_ctrl:
+			print "pm updated"
+			pm_ctrl.update_contact()
 			
 	def register_handler(self, event, handler):																																									
 		if event not in self.handlers:
@@ -2097,7 +2105,8 @@ class Interface:
 			'JINGLE_CONNECTED': [self.handle_event_jingle_connected],
 			'JINGLE_DISCONNECTED': [self.handle_event_jingle_disconnected],
 			'JINGLE_ERROR': [self.handle_event_jingle_error],
-			'PEP_RECEIVED': [self.handle_event_pep_received]
+			'PEP_RECEIVED': [self.handle_event_pep_received],
+			'CAPS_RECEIVED': [self.handle_event_caps_received]
 		}
 	
 	def dispatch(self, event, account, data):

test/unit/test_caps.py

@@ -1,6 +1,7 @@
 '''
 Tests for capabilities and the capabilities cache
 '''
+from common.caps import NullClientCaps
 import unittest
 
 import lib
@@ -12,32 +13,32 @@ from common.contacts import Contact
 
 from mock import Mock
 
-		
+
-class CommonCapsTest(unittest.TestCase):	
+class CommonCapsTest(unittest.TestCase):
-	
+
 	def setUp(self):
 		self.caps_method = 'sha-1'
 		self.caps_hash = 'm3P2WeXPMGVH2tZPe7yITnfY0Dw='
 		self.client_caps = (self.caps_method, self.caps_hash)
-		
+
 		self.node = "http://gajim.org"
 		self.identity = {'category': 'client', 'type': 'pc', 'name':'Gajim'}
 
 		self.identities = [self.identity]
 		self.features = [NS_MUC, NS_XHTML_IM] # NS_MUC not supported!
-		
+
 		# Simulate a filled db
 		db_caps_cache = [
 				(self.caps_method, self.caps_hash, self.identities, self.features),
 				('old', self.node + '#' + self.caps_hash, self.identities, self.features)]
 		self.logger = Mock(returnValues={"iter_caps_data":db_caps_cache})
-		
+
 		self.cc = caps.CapsCache(self.logger)
 		caps.capscache = self.cc
-			
+
-			
+
 class TestCapsCache(CommonCapsTest):
-	
+
 	def test_set_retrieve(self):
 		''' Test basic set / retrieve cycle '''
 
@@ -54,18 +55,18 @@ class TestCapsCache(CommonCapsTest):
 		identity = identities[0]
 		self.assertEqual('client', identity['category'])
 		self.assertEqual('pc', identity['type'])
-		
+
 	def test_set_and_store(self):
 		''' Test client_caps update gets logged into db '''
-		
+
 		item = self.cc[self.client_caps]
 		item.set_and_store(self.identities, self.features)
-		
+
 		self.logger.mockCheckCall(0, "add_caps_entry", self.caps_method,
 				self.caps_hash, self.identities, self.features)
-		
+
 	def test_initialize_from_db(self):
-		''' Read cashed dummy data from db ''' 
+		''' Read cashed dummy data from db '''
 		self.assertEqual(self.cc[self.client_caps].status, caps.NEW)
 		self.cc.initialize_from_db()
 		self.assertEqual(self.cc[self.client_caps].status, caps.CACHED)
@@ -74,12 +75,12 @@ class TestCapsCache(CommonCapsTest):
 		''' Make sure that preload issues a disco '''
 		connection = Mock()
 		client_caps = caps.ClientCaps(self.caps_hash, self.node, self.caps_method)
-				
+
 		self.cc.query_client_of_jid_if_unknown(connection, "test@gajim.org",
 				client_caps)
-		
+
 		self.assertEqual(1, len(connection.mockGetAllCalls()))
-		
+
 	def test_no_preload_query_if_cashed(self):
 		''' Preload must not send a query if the data is already cached '''
 		connection = Mock()
@@ -88,9 +89,9 @@ class TestCapsCache(CommonCapsTest):
 		self.cc.initialize_from_db()
 		self.cc.query_client_of_jid_if_unknown(connection, "test@gajim.org",
 				client_caps)
-	
+
 		self.assertEqual(0, len(connection.mockGetAllCalls()))
-		
+
 	def test_hash(self):
 		'''tests the hash computation'''
 		computed_hash = caps.compute_caps_hash(self.identities, self.features)
@@ -98,54 +99,112 @@ class TestCapsCache(CommonCapsTest):
 
 
 class TestClientCaps(CommonCapsTest):
-	
+
 	def setUp(self):
 		CommonCapsTest.setUp(self)
-		self.client_caps = caps.ClientCaps(self.caps_hash, self.node, self.caps_method) 
+		self.client_caps = caps.ClientCaps(self.caps_hash, self.node, self.caps_method)
-		
+
 	def test_query_by_get_discover_strategy(self):
-		''' Client must be queried if the data is unkown '''	
+		''' Client must be queried if the data is unkown '''
 		connection = Mock()
 		discover = self.client_caps.get_discover_strategy()
-		discover(connection, "test@gajim.org")	
+		discover(connection, "test@gajim.org")
-		
+
-		connection.mockCheckCall(0, "discoverInfo", "test@gajim.org", 
+		connection.mockCheckCall(0, "discoverInfo", "test@gajim.org",
 				"http://gajim.org#m3P2WeXPMGVH2tZPe7yITnfY0Dw=")
-		
+
 	def test_client_supports(self):
 		self.assertTrue(caps.client_supports(self.client_caps, NS_PING),
 				msg="Assume supported, if we don't have caps")
-		
+
 		self.assertFalse(caps.client_supports(self.client_caps, NS_XHTML_IM),
 			msg="Must not assume blacklisted feature is supported on default")
-		
+
 		self.cc.initialize_from_db()
-		
+
 		self.assertFalse(caps.client_supports(self.client_caps, NS_PING),
 				msg="Must return false on unsupported feature")
-		
+
 		self.assertTrue(caps.client_supports(self.client_caps, NS_XHTML_IM),
 				msg="Must return True on supported feature")
-		
+
 		self.assertTrue(caps.client_supports(self.client_caps, NS_MUC),
 				msg="Must return True on supported feature")
-			
 
-class TestOldClientCaps(TestClientCaps):	
+
+class TestOldClientCaps(TestClientCaps):
 
 	def setUp(self):
 		TestClientCaps.setUp(self)
-		self.client_caps = caps.OldClientCaps(self.caps_hash, self.node) 
+		self.client_caps = caps.OldClientCaps(self.caps_hash, self.node)
-	
+
 	def test_query_by_get_discover_strategy(self):
-		''' Client must be queried if the data is unknown '''	
+		''' Client must be queried if the data is unknown '''
 		connection = Mock()
 		discover = self.client_caps.get_discover_strategy()
-		discover(connection, "test@gajim.org")	
+		discover(connection, "test@gajim.org")
-		
+
 		connection.mockCheckCall(0, "discoverInfo", "test@gajim.org")
 
 
+from common.xmpp import simplexml
+from common.xmpp import protocol
+
+class TestableConnectionCaps(caps.ConnectionCaps):
+
+	def __init__(self, *args, **kwargs):
+		self._mocked_contacts = {}
+		caps.ConnectionCaps.__init__(self, *args, **kwargs)
+
+	def _get_contact_or_gc_contact_for_jid(self, jid):
+		"""
+		Overwrite to decouple form contact handling
+		"""
+		if jid not in self._mocked_contacts:
+			self._mocked_contacts[jid] = Mock(realClass=Contact)
+			self._mocked_contacts[jid].jid = jid
+		return self._mocked_contacts[jid]
+
+	def discoverInfo(self, *args, **kwargs):
+		pass
+
+	def get_mocked_contact_for_jid(self, jid):
+		return self._mocked_contacts[jid]
+
+
+class TestConnectionCaps(CommonCapsTest):
+
+	def test_capsPresenceCB(self):
+		jid = "user@server.com/a"
+		connection_caps = TestableConnectionCaps("account",
+			self._build_assertering_dispatcher_function("CAPS_RECEIVED", jid))
+
+		xml = """<presence from='user@server.com/a'
+								to='%s' id='123'>
+						<c node='http://gajim.org' ver='pRCD6cgQ4SDqNMCjdhRV6TECx5o='
+							hash='sha-1' xmlns='http://jabber.org/protocol/caps'/>
+					</presence>
+				""" % (jid)
+		iq = protocol.Iq(node=simplexml.XML2Node(xml))
+		connection_caps._capsPresenceCB(None, iq)
+
+		self.assertTrue(self._dispatcher_called, msg="Must have received caps")
+
+		client_caps = connection_caps.get_mocked_contact_for_jid(jid).client_caps
+		self.assertTrue(client_caps, msg="Client caps must be set")
+		self.assertFalse(isinstance(client_caps, NullClientCaps),
+			msg="On receive of proper caps, we must not use the fallback")
+
+	def _build_assertering_dispatcher_function(self, expected_event, jid):
+		self._dispatcher_called = False
+		def dispatch(event, data):
+			self.assertFalse(self._dispatcher_called, msg="Must only be called once")
+			self._dispatcher_called = True
+			self.assertEqual(expected_event, event)
+			self.assertEqual(jid, data[0])
+		return dispatch
+
+
 if __name__ == '__main__':
 	unittest.main()