From 8a19e11bee032a75affe98c17cbfbd7f1cffce2f Mon Sep 17 00:00:00 2001
From: Stephan Erb <steve-e@h3c.de>
Date: Sun, 11 Jan 2009 13:42:39 +0000
Subject: [PATCH] Add some FIXME statements due to open questions. Improve a
 two default parameters.

---
 src/common/xmpp/client_nb.py     | 14 ++++++++------
 src/common/xmpp/dispatcher_nb.py | 15 +++++++++++++--
 src/common/xmpp/transports_nb.py |  4 +++-
 3 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/common/xmpp/client_nb.py b/src/common/xmpp/client_nb.py
index 91e30838f..0e372dd2e 100644
--- a/src/common/xmpp/client_nb.py
+++ b/src/common/xmpp/client_nb.py
@@ -92,6 +92,8 @@ class NonBlockingClient:
 			self.NonBlockingHTTP.PlugOut()
 		if 'NonBlockingBOSH' in self.__dict__:
 			self.NonBlockingBOSH.PlugOut()
+		# FIXME: we never unplug dispatcher, only on next connect
+		# See _xmpp_connect_machine and SASLHandler
 
 		connected = self.connected
 		stream_started = self.stream_started
@@ -129,7 +131,7 @@ class NonBlockingClient:
 		self.disconnecting = False
 
 	def connect(self, on_connect, on_connect_failure, hostname=None, port=5222,
-	on_proxy_failure=None, proxy=None, secure_tuple=(None, None, None)):
+	on_proxy_failure=None, proxy=None, secure_tuple=('plain', None, None)):
 		''' 
 		Open XMPP connection (open XML streams in both directions).
 
@@ -353,22 +355,22 @@ class NonBlockingClient:
 					# if stream version is less than 1.0, we can't do more
 					log.warn('While connecting with type = "tls": stream version is less than 1.0')
 					self._on_connect()
-					return
 				if self.Dispatcher.Stream.features.getTag('starttls'):
 					# Server advertises TLS support, start negotiation
 					self.stream_started = False
 					log.info('TLS supported by remote server. Requesting TLS start.')
 					self._tls_negotiation_handler()
-					return
 				else:
 					log.warn('While connecting with type = "tls": TLS unsupported by remote server')
 					self._on_connect()
 					return
-
+	
 		elif self.connected in ['ssl', 'tls']:
 			self._on_connect()
 			return
-		assert False # should never be reached
+		else:
+			log.error('Stream opened for unsupported connection: %s' % 
+				(self.connected or 'Disconnected'))
 
 	def _tls_negotiation_handler(self, con=None, tag=None):
 		''' takes care of TLS negotioation with <starttls> '''
@@ -477,7 +479,7 @@ class NonBlockingClient:
 			# wrong user/pass, stop auth
 			if 'SASL' in self.__dict__:
 				self.SASL.PlugOut()
-			self.connected = None # FIXME: is this intended?
+			self.connected = None # FIXME: is this intended? We use ''elsewhere
 			self._on_sasl_auth(None)
 		elif self.SASL.startsasl == 'success':
 			auth_nb.NonBlockingBind.get_instance().PlugIn(self)
diff --git a/src/common/xmpp/dispatcher_nb.py b/src/common/xmpp/dispatcher_nb.py
index 63f03fc11..b683d3888 100644
--- a/src/common/xmpp/dispatcher_nb.py
+++ b/src/common/xmpp/dispatcher_nb.py
@@ -52,6 +52,8 @@ class Dispatcher():
 			XMPPDispatcher().PlugIn(client_obj)
 		elif client_obj.protocol_type == 'BOSH':
 			BOSHDispatcher().PlugIn(client_obj, after_SASL, old_features)
+		else:
+			assert False # should never be reached
 	
 	@classmethod
 	def get_instance(cls, *args, **kwargs):
@@ -111,6 +113,8 @@ class XMPPDispatcher(PlugIn):
 		'''
 			Registers default namespaces/protocols/handlers. Used internally.
 		'''
+		# FIXME: inject dependencies, do not rely that they are defined by our
+		# owner
 		self.RegisterNamespace('unknown')
 		self.RegisterNamespace(NS_STREAMS)
 		self.RegisterNamespace(self._owner.defaultNamespace)
@@ -162,7 +166,7 @@ class XMPPDispatcher(PlugIn):
 			raise ValueError('Incorrect stream start: (%s,%s). Terminating.'
 				% (tag, ns))
 
-	def ProcessNonBlocking(self, data=None):
+	def ProcessNonBlocking(self, data):
 		'''
 		Check incoming stream for data waiting.
 
@@ -172,6 +176,11 @@ class XMPPDispatcher(PlugIn):
 			2) '0' string if no data were processed but link is alive;
 			3) 0 (zero) if underlying connection is closed.
 		'''
+		# FIXME:
+		# When an error occurs we disconnect the transport directly. Client's
+		# disconnect method will never be called.
+		# Is this intended?
+		# also look at transports start_disconnect()
 		for handler in self._cycleHandlers:
 			handler(self)
 		if len(self._pendingExceptions) > 0:
@@ -317,7 +326,7 @@ class XMPPDispatcher(PlugIn):
 		'''
 		self._eventHandler = handler
 
-	def returnStanzaHandler(self,conn,stanza):
+	def returnStanzaHandler(self, conn, stanza):
 		'''
 		Return stanza back to the sender with <feature-not-implemented/> error set
 		'''
@@ -348,6 +357,8 @@ class XMPPDispatcher(PlugIn):
 		'''
 		if self._eventHandler:
 			self._eventHandler(realm, event, data)
+		else:
+			log.warning('Received unhandled event: %s' % event)
 
 	def dispatch(self, stanza, session=None, direct=0):
 		'''
diff --git a/src/common/xmpp/transports_nb.py b/src/common/xmpp/transports_nb.py
index 99754bfb2..b9736919c 100644
--- a/src/common/xmpp/transports_nb.py
+++ b/src/common/xmpp/transports_nb.py
@@ -239,6 +239,7 @@ class NonBlockingTransport(PlugIn):
 		else:
 			self.on_timeout = None
 
+	# FIXME: where and why does this need to be called
 	def start_disconnect(self):
 		self.set_state(DISCONNECTING)
 
@@ -267,7 +268,8 @@ class NonBlockingTCP(NonBlockingTransport, IdleObject):
 
 		self.proxy_dict = proxy_dict
 		self.on_remote_disconnect = self.disconnect
-
+	
+	# FIXME: transport should not be aware xmpp
 	def start_disconnect(self):
 		NonBlockingTransport.start_disconnect(self)
 		self.send('</stream:stream>', now=True)