From 68d1653679bdce47f008d9167a263dbb2259e91d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 23 Dec 2009 22:58:50 -0600 Subject: [PATCH] Mass load of bug fixes Tests * Hand tests now able to test adding and removing of connections * Attempts made at closing of channels Channels * Modified the close calls to allow errors to propogate * Reduced chances of memory leaks, allowing re-opening of text windows * Cleaned up the channel manager's close code Connection * Added capabilities * Worked to reduce memory leaks allowing new connections to be created Connection Manager * Fixed a shutdown bug * Added some logging State Machine * Improved the flexibility of the states Simple Presence * Tried to clean up the presence code and finish implementing it --- hand_tests/generic.py | 105 ++++++++++++++++++++++++++++++++----------- src/capabilities.py | 82 +++++++++++++++++++++++++++++++++ src/channel/call.py | 9 ++++ src/channel/contact_list.py | 12 ++++- src/channel/text.py | 20 +++++---- src/channel_manager.py | 14 +++--- src/connection.py | 24 +++++++--- src/connection_manager.py | 6 +-- src/gtk_toolbox.py | 17 +++++++ src/gvoice/session.py | 6 +++ src/gvoice/state_machine.py | 16 ++++--- src/simple_presence.py | 47 +++++++++++++------ support/theonering.profile | 2 +- 13 files changed, 286 insertions(+), 74 deletions(-) create mode 100644 src/capabilities.py diff --git a/hand_tests/generic.py b/hand_tests/generic.py index a6e4d0b..87aa33a 100755 --- a/hand_tests/generic.py +++ b/hand_tests/generic.py @@ -96,10 +96,10 @@ class DisplayParams(Action): super(DisplayParams, self)._on_done() -class Connect(Action): +class RequestConnection(Action): def __init__(self, cm, username, password, forward): - super(Connect, self).__init__() + super(RequestConnection, self).__init__() self._cm = cm self._conn = None @@ -125,18 +125,28 @@ class Connect(Action): 'password': self._password, 'forward': self._forward, }, - reply_handler = self._on_connection_requested, + reply_handler = self._on_done, error_handler = self._on_error, ) - def _on_connection_requested(self, busName, objectPath): + def _on_done(self, busName, objectPath): self._serviceName = busName self._conn = telepathy.client.Connection(busName, objectPath) - self._conn[telepathy.server.CONNECTION].connect_to_signal( + super(RequestConnection, self)._on_done() + + +class Connect(Action): + + def __init__(self, connAction): + super(Connect, self).__init__() + self._connAction = connAction + + def queue_action(self): + self._connAction.conn[telepathy.server.CONNECTION].connect_to_signal( 'StatusChanged', self._on_change, ) - self._conn[telepathy.server.CONNECTION].Connect( + self._connAction.conn[telepathy.server.CONNECTION].Connect( reply_handler = self._on_generic_message, error_handler = self._on_error, ) @@ -274,6 +284,24 @@ class RequestChannel(Action): super(RequestChannel, self)._on_done() +class CloseChannel(Action): + + def __init__(self, connAction, chanAction): + super(CloseChannel, self).__init__() + self._connAction = connAction + self._chanAction = chanAction + self._handles = [] + + def queue_action(self): + self._chanAction.channel[telepathy.server.CHANNEL].Close( + reply_handler = self._on_done, + error_handler = self._on_error, + ) + + def _on_done(self): + super(CloseChannel, self)._on_done() + + class ContactHandles(Action): def __init__(self, connAction, chanAction): @@ -462,42 +490,51 @@ if __name__ == '__main__': username = sys.argv[1] password = sys.argv[2] forward = sys.argv[3] - con = Connect(cm, username, password, forward) + reqcon = RequestConnection(cm, username, password, forward) + lastAction.append_action(reqcon) + lastAction = reqcon + + if False: + reqcon = RequestConnection(cm, username, password, forward) + lastAction.append_action(reqcon) + lastAction = reqcon + + con = Connect(reqcon) lastAction.append_action(con) lastAction = con if True: - spo = SimplePresenceOptions(con) + spo = SimplePresenceOptions(reqcon) lastAction.append_action(spo) lastAction = spo if True: - uh = UserHandle(con) + uh = UserHandle(reqcon) lastAction.append_action(uh) lastAction = uh - ua = Aliases(con, uh) + ua = Aliases(reqcon, uh) lastAction.append_action(ua) lastAction = ua - sps = SimplePresenceStatus(con, uh) + sps = SimplePresenceStatus(reqcon, uh) lastAction.append_action(sps) lastAction = sps if False: - setdnd = SetSimplePresence(con, "dnd", "") + setdnd = SetSimplePresence(reqcon, "dnd", "") lastAction.append_action(setdnd) lastAction = setdnd - sps = SimplePresenceStatus(con, uh) + sps = SimplePresenceStatus(reqcon, uh) lastAction.append_action(sps) lastAction = sps - setdnd = SetSimplePresence(con, "available", "") + setdnd = SetSimplePresence(reqcon, "available", "") lastAction.append_action(setdnd) lastAction = setdnd - sps = SimplePresenceStatus(con, uh) + sps = SimplePresenceStatus(reqcon, uh) lastAction.append_action(sps) lastAction = sps @@ -507,12 +544,12 @@ if __name__ == '__main__': lastAction = sl if False: - rclh = RequestHandle(con, telepathy.HANDLE_TYPE_LIST, ["subscribe"]) + rclh = RequestHandle(reqcon, telepathy.HANDLE_TYPE_LIST, ["subscribe"]) lastAction.append_action(rclh) lastAction = rclh rclc = RequestChannel( - con, + reqcon, rclh, telepathy.CHANNEL_TYPE_CONTACT_LIST, telepathy.HANDLE_TYPE_LIST, @@ -520,16 +557,16 @@ if __name__ == '__main__': lastAction.append_action(rclc) lastAction = rclc - ch = ContactHandles(con, rclc) + ch = ContactHandles(reqcon, rclc) lastAction.append_action(ch) lastAction = ch - ca = Aliases(con, ch) + ca = Aliases(reqcon, ch) lastAction.append_action(ca) lastAction = ca - if False: - rch = RequestHandle(con, telepathy.HANDLE_TYPE_CONTACT, ["18005558355"]) #(1-800-555-TELL) + if True: + rch = RequestHandle(reqcon, telepathy.HANDLE_TYPE_CONTACT, ["18005558355"]) #(1-800-555-TELL) lastAction.append_action(rch) lastAction = rch @@ -541,7 +578,7 @@ if __name__ == '__main__': smHandle = nullHandle smHandleType = telepathy.HANDLE_TYPE_NONE rsmc = RequestChannel( - con, + reqcon, smHandle, telepathy.CHANNEL_TYPE_STREAMED_MEDIA, smHandleType, @@ -550,13 +587,13 @@ if __name__ == '__main__': lastAction = rsmc if False: - call = Call(con, rsmc, rch) + call = Call(reqcon, rsmc, rch) lastAction.append_action(call) lastAction = call # sending a text rtc = RequestChannel( - con, + reqcon, rch, telepathy.CHANNEL_TYPE_TEXT, smHandleType, @@ -564,8 +601,22 @@ if __name__ == '__main__': lastAction.append_action(rtc) lastAction = rtc + if True: + closechan = CloseChannel(reqcon, rtc) + lastAction.append_action(closechan) + lastAction = closechan + + rtc = RequestChannel( + reqcon, + rch, + telepathy.CHANNEL_TYPE_TEXT, + smHandleType, + ) + lastAction.append_action(rtc) + lastAction = rtc + if False: - sendtext = SendText(con, rtc, rch, telepathy.CHANNEL_TEXT_MESSAGE_TYPE_NORMAL, "Boo!") + sendtext = SendText(reqcon, rtc, rch, telepathy.CHANNEL_TEXT_MESSAGE_TYPE_NORMAL, "Boo!") lastAction.append_action(sendtext) lastAction = sendtext @@ -574,12 +625,12 @@ if __name__ == '__main__': lastAction.append_action(bl) lastAction = bl - if True: + if False: sl = Sleep(30 * 1000) lastAction.append_action(sl) lastAction = sl - dis = Disconnect(con) + dis = Disconnect(reqcon) lastAction.append_action(dis) lastAction = dis diff --git a/src/capabilities.py b/src/capabilities.py new file mode 100644 index 0000000..31227d4 --- /dev/null +++ b/src/capabilities.py @@ -0,0 +1,82 @@ +import logging + +import telepathy + +import gtk_toolbox + + +_moduleLogger = logging.getLogger('capabilities') + + +class CapabilitiesMixin(telepathy.server.ConnectionInterfaceCapabilities): + + def __init__(self): + telepathy.server.ConnectionInterfaceCapabilities.__init__(self) + self._implement_property_get( + telepathy.interfaces.CONN_INTERFACE_CAPABILITIES, + {"caps": self.GetCapabilities}, + ) + + @property + def session(self): + """ + @abstract + """ + raise NotImplementedError("Abstract property called") + + @property + def handle(self): + """ + @abstract + """ + raise NotImplementedError("Abstract property called") + + @gtk_toolbox.log_exception(_moduleLogger) + def GetCapabilities(self, handles): + """ + @todo HACK Remove this once we are building against a fixed version of python-telepathy + """ + ret = [] + for handle in handles: + if handle != 0 and (telepathy.HANDLE_TYPE_CONTACT, handle) not in self._handles: + raise telepathy.errors.InvalidHandle + elif handle in self._caps: + theirs = self._caps[handle] + for type in theirs: + ret.append([handle, type, theirs[0], theirs[1]]) + _moduleLogger.info("GetCaps %r" % ret) + return ret + + @gtk_toolbox.log_exception(_moduleLogger) + def AdvertiseCapabilities(self, add, remove): + """ + @todo HACK Remove this once we are building against a fixed version of python-telepathy + """ + my_caps = self._caps.setdefault(self._self_handle, {}) + + changed = {} + for ctype, spec_caps in add: + changed[ctype] = spec_caps + for ctype in remove: + changed[ctype] = None + + caps = [] + for ctype, spec_caps in changed.iteritems(): + gen_old, spec_old = my_caps.get(ctype, (0, 0)) + if spec_caps is None: + # channel type no longer supported (provider has gone away) + gen_new, spec_new = 0, 0 + else: + # channel type supports new capabilities + gen_new, spec_new = gen_old, spec_old | spec_caps + if spec_old != spec_new or gen_old != gen_new: + caps.append((self._self_handle, ctype, gen_old, gen_new, + spec_old, spec_new)) + + _moduleLogger.info("CapsChanged %r" % caps) + self.CapabilitiesChanged(caps) + + # return all my capabilities + ret = [(ctype, caps[1]) for ctype, caps in my_caps.iteritems()] + _moduleLogger.info("Adv %r" % ret) + return ret diff --git a/src/channel/call.py b/src/channel/call.py index 7535a12..9de629e 100644 --- a/src/channel/call.py +++ b/src/channel/call.py @@ -21,6 +21,15 @@ class CallChannel( self._contactHandle = contactHandle @gtk_toolbox.log_exception(_moduleLogger) + def Close(self): + self.close() + + def close(self): + telepathy.server.ChannelTypeStreamedMedia.Close(self) + self.remove_from_connection() + self._prop_getters = None # HACK to get around python-telepathy memory leaks + + @gtk_toolbox.log_exception(_moduleLogger) def ListStreams(self): """ For org.freedesktop.Telepathy.Channel.Type.StreamedMedia diff --git a/src/channel/contact_list.py b/src/channel/contact_list.py index f82c090..7f9981d 100644 --- a/src/channel/contact_list.py +++ b/src/channel/contact_list.py @@ -31,6 +31,7 @@ class AllContactsListChannel(AbstractListChannel): def __init__(self, connection, h): AbstractListChannel.__init__(self, connection, h) + self._callback = coroutines.func_sink( coroutines.expand_positional( self._on_contacts_refreshed @@ -39,6 +40,7 @@ class AllContactsListChannel(AbstractListChannel): self._session.addressbook.updateSignalHandler.register_sink( self._callback ) + self.GroupFlagsChanged(0, 0) addressbook = connection.session.addressbook @@ -47,11 +49,17 @@ class AllContactsListChannel(AbstractListChannel): @gtk_toolbox.log_exception(_moduleLogger) def Close(self): - telepathy.server.ChannelTypeContactList.Close(self) - self.remove_from_connection() + self.close() + + def close(self): self._session.addressbook.updateSignalHandler.unregister_sink( self._callback ) + self._callback = None + + telepathy.server.ChannelTypeContactList.Close(self) + self.remove_from_connection() + self._prop_getters = None # HACK to get around python-telepathy memory leaks @gobject_utils.async @gtk_toolbox.log_exception(_moduleLogger) diff --git a/src/channel/text.py b/src/channel/text.py index 37d3f6f..3a4d067 100644 --- a/src/channel/text.py +++ b/src/channel/text.py @@ -54,16 +54,18 @@ class TextChannel(telepathy.server.ChannelTypeText): @gtk_toolbox.log_exception(_moduleLogger) def Close(self): - try: - # Clear since the user has seen it all and it should start a new conversation - self._conn.session.conversations.clear_conversation(self._contactKey) + self.close() - self._conn.session.conversations.updateSignalHandler.unregister_sink( - self._callback - ) - finally: - telepathy.server.ChannelTypeText.Close(self) - self.remove_from_connection() + def close(self): + _moduleLogger.info("Closing text channel for %r" % (self._otherHandle, )) + self._conn.session.conversations.updateSignalHandler.unregister_sink( + self._callback + ) + self._callback = None + + telepathy.server.ChannelTypeText.Close(self) + self.remove_from_connection() + self._prop_getters = None # HACK to get around python-telepathy memory leaks @property def _contactKey(self): diff --git a/src/channel_manager.py b/src/channel_manager.py index 906744a..d1e9942 100644 --- a/src/channel_manager.py +++ b/src/channel_manager.py @@ -1,4 +1,5 @@ import weakref +import itertools import logging import telepathy @@ -18,12 +19,13 @@ class ChannelManager(object): self._callChannels = weakref.WeakValueDictionary() def close(self): - for chan in self._listChannels.values(): - chan.Close() - for chan in self._textChannels.values(): - chan.Close() - for chan in self._callChannels.values(): - chan.Close() + for chan in itertools.chain( + self._listChannels.values(), self._textChannels.values(), self._callChannels.values() + ): + try: + chan.close() + except Exception: + _moduleLogger.exception("Shutting down %r" % (chan, )) def channel_for_list(self, handle, suppress_handler=False): try: diff --git a/src/connection.py b/src/connection.py index dc2c0b2..5af02ba 100644 --- a/src/connection.py +++ b/src/connection.py @@ -11,6 +11,7 @@ import gvoice import handle import aliasing import simple_presence +import capabilities import channel_manager @@ -21,6 +22,7 @@ class TheOneRingConnection( telepathy.server.Connection, aliasing.AliasingMixin, simple_presence.SimplePresenceMixin, + capabilities.CapabilitiesMixin, ): # Overriding a base class variable @@ -50,6 +52,7 @@ class TheOneRingConnection( ) aliasing.AliasingMixin.__init__(self) simple_presence.SimplePresenceMixin.__init__(self) + capabilities.CapabilitiesMixin.__init__(self) self._manager = weakref.proxy(manager) self._credentials = ( @@ -59,16 +62,11 @@ class TheOneRingConnection( self._callbackNumber = parameters['forward'].encode('utf-8') self._channelManager = channel_manager.ChannelManager(self) - cookieFilePath = None - self._session = gvoice.session.Session(cookieFilePath) + self._session = gvoice.session.Session(None) self.set_self_handle(handle.create_handle(self, 'connection')) - self._callback = coroutines.func_sink( - coroutines.expand_positional( - self._on_conversations_updated - ) - ) + self._callback = None _moduleLogger.info("Connection to the account %s created" % account) except Exception, e: _moduleLogger.exception("Failed to create Connection") @@ -101,6 +99,14 @@ class TheOneRingConnection( telepathy.CONNECTION_STATUS_REASON_REQUESTED ) try: + cookieFilePath = None + self._session = gvoice.session.Session(cookieFilePath) + + self._callback = coroutines.func_sink( + coroutines.expand_positional( + self._on_conversations_updated + ) + ) self.session.conversations.updateSignalHandler.register_sink( self._callback ) @@ -136,8 +142,11 @@ class TheOneRingConnection( self.session.conversations.updateSignalHandler.unregister_sink( self._callback ) + self._callback = None self._channelManager.close() self.session.logout() + self.session.close() + self._session = None _moduleLogger.info("Disconnected") except Exception: _moduleLogger.exception("Disconnecting Failed") @@ -145,6 +154,7 @@ class TheOneRingConnection( telepathy.CONNECTION_STATUS_DISCONNECTED, telepathy.CONNECTION_STATUS_REASON_REQUESTED ) + self.manager.disconnected(self) @gtk_toolbox.log_exception(_moduleLogger) def RequestChannel(self, type, handleType, handleId, suppressHandler): diff --git a/src/connection_manager.py b/src/connection_manager.py index b392555..c4b5f21 100644 --- a/src/connection_manager.py +++ b/src/connection_manager.py @@ -1,10 +1,9 @@ """ Empathy Experience: - .profile file needs to be updated with proper presence - Can't reopen a conversation for someone when I've already closed it Can't call When first started, reports all read conversations when some might have been read When first started, reports all of an SMS conversation even though some has been reported previously + Still leaking one of two contact lists """ import logging @@ -82,7 +81,7 @@ class TheOneRingConnectionManager(telepathy.server.ConnectionManager): Overrides telepathy.server.ConnectionManager """ result = telepathy.server.ConnectionManager.disconnected(self, conn) - gobject.timeout_add(5000, self.shutdown) + gobject.timeout_add(5000, self._shutdown) def quit(self): """ @@ -92,6 +91,7 @@ class TheOneRingConnectionManager(telepathy.server.ConnectionManager): connection.Disconnect() _moduleLogger.info("Connection manager quitting") + @gtk_toolbox.log_exception(_moduleLogger) def _shutdown(self): if ( self._on_shutdown is not None and diff --git a/src/gtk_toolbox.py b/src/gtk_toolbox.py index 97acd2f..db6c544 100644 --- a/src/gtk_toolbox.py +++ b/src/gtk_toolbox.py @@ -221,6 +221,23 @@ def safecall(f, errorDisplay=None, default=None, exception=Exception): return _safecall +def log_call(logger): + + def log_call_decorator(func): + + @functools.wraps(func) + def wrapper(*args, **kwds): + _moduleLogger.info("-> %s" % (func.__name__, )) + try: + return func(*args, **kwds) + finally: + _moduleLogger.info("<- %s" % (func.__name__, )) + + return wrapper + + return log_call_decorator + + def log_exception(logger): def log_exception_decorator(func): diff --git a/src/gvoice/session.py b/src/gvoice/session.py index d87f7ad..a890ac9 100644 --- a/src/gvoice/session.py +++ b/src/gvoice/session.py @@ -26,6 +26,12 @@ class Session(object): self._stateMachine.request_reset_timers ) + def close(self): + self._conversations.updateSignalHandler.unregister_sink( + self._stateMachine.request_reset_timers + ) + self._stateMachine.close() + def login(self, username, password): self._username = username self._password = password diff --git a/src/gvoice/state_machine.py b/src/gvoice/state_machine.py index 6e7d288..6b0a58d 100644 --- a/src/gvoice/state_machine.py +++ b/src/gvoice/state_machine.py @@ -29,9 +29,9 @@ def _to_milliseconds(**kwd): class StateMachine(object): - STATE_ACTIVE = "active" - STATE_IDLE = "idle" - STATE_DND = "dnd" + STATE_ACTIVE = 0, "active" + STATE_IDLE = 1, "idle" + STATE_DND = 2, "dnd" _ACTION_UPDATE = "update" _ACTION_RESET = "reset" @@ -59,6 +59,9 @@ class StateMachine(object): ) ) + def close(self): + self._callback = None + @gobject_utils.async @gtk_toolbox.log_exception(_moduleLogger) def start(self): @@ -74,8 +77,11 @@ class StateMachine(object): _moduleLogger.info("Stopping an already stopped state machine") self._stop_update() - def set_state(self, state): - self._state = state + def set_state(self, newState): + oldState = self._state + _moduleLogger.info("Transitioning from %s to %s" % (oldState, newState)) + + self._state = newState self.reset_timers() def get_state(self): diff --git a/src/simple_presence.py b/src/simple_presence.py index ff67a4f..dc44399 100644 --- a/src/simple_presence.py +++ b/src/simple_presence.py @@ -4,20 +4,25 @@ import telepathy import gtk_toolbox import handle +import gvoice.state_machine as state_machine _moduleLogger = logging.getLogger("simple_presence") class TheOneRingPresence(object): + + # Note: these strings are also in the theonering.profile file ONLINE = 'available' - IDLE = 'idle' - BUSY = 'dnd' + AWAY = 'away' + HIDDEN = 'hidden' + OFFLINE = 'offline' TO_PRESENCE_TYPE = { ONLINE: telepathy.constants.CONNECTION_PRESENCE_TYPE_AVAILABLE, - IDLE: telepathy.constants.CONNECTION_PRESENCE_TYPE_AWAY, - BUSY: telepathy.constants.CONNECTION_PRESENCE_TYPE_HIDDEN, + AWAY: telepathy.constants.CONNECTION_PRESENCE_TYPE_AWAY, + HIDDEN: telepathy.constants.CONNECTION_PRESENCE_TYPE_HIDDEN, + OFFLINE: telepathy.constants.CONNECTION_PRESENCE_TYPE_OFFLINE, } @@ -26,9 +31,10 @@ class SimplePresenceMixin(telepathy.server.ConnectionInterfaceSimplePresence): def __init__(self): telepathy.server.ConnectionInterfaceSimplePresence.__init__(self) - dbus_interface = 'org.freedesktop.Telepathy.Connection.Interface.SimplePresence' - - self._implement_property_get(dbus_interface, {'Statuses' : self._get_statuses}) + self._implement_property_get( + telepathy.server.CONNECTION_INTERFACE_SIMPLE_PRESENCE, + {'Statuses' : self._get_statuses} + ) @property def session(self): @@ -44,6 +50,12 @@ class SimplePresenceMixin(telepathy.server.ConnectionInterfaceSimplePresence): """ raise NotImplementedError("Abstract property called") + def Disconnect(self): + """ + @abstract + """ + raise NotImplementedError("Abstract function called") + @gtk_toolbox.log_exception(_moduleLogger) def GetPresences(self, contacts): """ @@ -55,10 +67,15 @@ class SimplePresenceMixin(telepathy.server.ConnectionInterfaceSimplePresence): if isinstance(h, handle.ConnectionHandle): isDnd = self.session.backend.is_dnd() if isDnd: - presence = TheOneRingPresence.BUSY + presence = TheOneRingPresence.HIDDEN else: - # @todo switch this over to also supporting idle - presence = TheOneRingPresence.ONLINE + state = self.session.stateMachine.get_state() + if state == state_machine.StateMachine.STATE_ACTIVE: + presence = TheOneRingPresence.ONLINE + elif state == state_machine.StateMachine.STATE_IDLE: + presence = TheOneRingPresence.AWAY + else: + raise telepathy.errors.InvalidArgument("Unsupported state on the state machine: %s" % state) personalMessage = u"" presenceType = TheOneRingPresence.TO_PRESENCE_TYPE[presence] else: @@ -76,11 +93,13 @@ class SimplePresenceMixin(telepathy.server.ConnectionInterfaceSimplePresence): if status == TheOneRingPresence.ONLINE: self.session.backend.set_dnd(False) - elif status == TheOneRingPresence.IDLE: - # @todo Add idle support - raise telepathy.errors.InvalidArgument("Not Supported Yet") - elif status == TheOneRingPresence.BUSY: + self.session.stateMachine.set_state(state_machine.StateMachine.STATE_ACTIVE) + elif status == TheOneRingPresence.AWAY: + self.session.stateMachine.set_state(state_machine.StateMachine.STATE_IDLE) + elif status == TheOneRingPresence.HIDDEN: self.session.backend.set_dnd(True) + elif status == TheOneRingPresence.OFFLINE: + self.Disconnect() else: raise telepathy.errors.InvalidArgument("Unsupported status: %r" % status) _moduleLogger.info("Setting Presence to '%s'" % status) diff --git a/support/theonering.profile b/support/theonering.profile index f1addcf..181bcb9 100644 --- a/support/theonering.profile +++ b/support/theonering.profile @@ -7,5 +7,5 @@ SupportsInvisible = 1 Capabilities = chat-p2p, voice-p2p, supports-alias VCardDefault = 1 VCardField = X-GV -SupportedPresences = offline,available,away,extended-away,hidden,do-not-disturb +SupportedPresences = offline,available,away,hidden -- 1.7.9.5