From 99a955c64266dc9dc596170beb024f84cd322254 Mon Sep 17 00:00:00 2001 From: Ruben Pollan Date: Thu, 15 Jan 2015 09:40:55 -0600 Subject: Use the incoming mail IService From kali: add some notes about the improper handling of the mailbox required to initialize the account, and draft some notes about how to improve this in next iterations. --- src/leap/bitmask/services/mail/conductor.py | 2 +- src/leap/bitmask/services/mail/imap.py | 40 ++++++++++++++++++--- src/leap/bitmask/services/mail/imapcontroller.py | 44 ++++++++++++++++-------- 3 files changed, 67 insertions(+), 19 deletions(-) (limited to 'src/leap') diff --git a/src/leap/bitmask/services/mail/conductor.py b/src/leap/bitmask/services/mail/conductor.py index 0fb9f4fa..42bdd032 100644 --- a/src/leap/bitmask/services/mail/conductor.py +++ b/src/leap/bitmask/services/mail/conductor.py @@ -262,7 +262,7 @@ class MailConductor(IMAPControl, SMTPControl): if self._firewall is not None: self._firewall.start() if not offline: - logger.debug("not starting smtp in offline mode") + logger.debug("Starting smtp service...") self.start_smtp_service(download_if_needed=download_if_needed) self.start_imap_service() diff --git a/src/leap/bitmask/services/mail/imap.py b/src/leap/bitmask/services/mail/imap.py index 5db18cb9..d044a600 100644 --- a/src/leap/bitmask/services/mail/imap.py +++ b/src/leap/bitmask/services/mail/imap.py @@ -21,7 +21,9 @@ import logging import os import sys +from leap.mail.constants import INBOX_NAME from leap.mail.imap.service import imap +from leap.mail.incoming.service import IncomingMail, INCOMING_CHECK_PERIOD from twisted.python import log logger = logging.getLogger(__name__) @@ -49,6 +51,9 @@ def get_mail_check_period(): logger.warning("Unhandled error while getting %s: %r" % ( INCOMING_CHECK_PERIOD_ENV, exc)) + + if period is None: + period = INCOMING_CHECK_PERIOD return period @@ -61,12 +66,39 @@ def start_imap_service(*args, **kwargs): from leap.bitmask.config import flags logger.debug('Launching imap service') - override_period = get_mail_check_period() - if override_period: - kwargs['check_period'] = override_period - if flags.MAIL_LOGFILE: log.startLogging(open(flags.MAIL_LOGFILE, 'w')) log.startLogging(sys.stdout) return imap.run_service(*args, **kwargs) + + +def start_incoming_mail_service(keymanager, soledad, imap_factory, userid): + """ + Initalizes and starts the incomming mail service. + + :returns: a Deferred that will be fired with the IncomingMail instance + """ + def setUpIncomingMail(inbox): + incoming_mail = IncomingMail( + keymanager, + soledad, + inbox, + userid, + check_period=get_mail_check_period()) + return incoming_mail + + # XXX: do I really need to know here how to get a mailbox?? + # XXX: ideally, the parent service in mail would take care of initializing + # the account, and passing the mailbox to the incoming service. + # In an even better world, we just would subscribe to a channel that would + # pass us the serialized object to be inserted. + # XXX: I think we might be at risk here because the account top object + # currently just returns as many mailbox objects as it's asked for, so + # we should be careful about concurrent operations (ie, maybe just use + # this one to do inserts, or let account have an in-memory map of + # mailboxes, and just return references to them). + acc = imap_factory.theAccount + d = acc.callWhenReady(lambda _: acc.getMailbox(INBOX_NAME)) + d.addCallback(setUpIncomingMail) + return d diff --git a/src/leap/bitmask/services/mail/imapcontroller.py b/src/leap/bitmask/services/mail/imapcontroller.py index d0bf4c34..bb9eb9dc 100644 --- a/src/leap/bitmask/services/mail/imapcontroller.py +++ b/src/leap/bitmask/services/mail/imapcontroller.py @@ -43,9 +43,12 @@ class IMAPController(object): self._soledad = soledad self._keymanager = keymanager - self.imap_service = None + # XXX: this should live in its own controller + # or, better, just be managed by a composite Mail Service in + # leap.mail. self.imap_port = None self.imap_factory = None + self.incoming_mail_service = None def start_imap_service(self, userid, offline=False): """ @@ -58,16 +61,28 @@ class IMAPController(object): """ logger.debug('Starting imap service') - self.imap_service, self.imap_port, \ - self.imap_factory = imap.start_imap_service( - self._soledad, - self._keymanager, - userid=userid, - offline=offline) + self.imap_port, self.imap_factory = imap.start_imap_service( + self._soledad, + userid=userid) + + def start_incoming_service(incoming_mail): + d = incoming_mail.startService() + d.addCallback(lambda started: incoming_mail) + return d + + def assign_incoming_service(incoming_mail): + self.incoming_mail_service = incoming_mail + return incoming_mail if offline is False: - logger.debug("Starting loop") - self.imap_service.start_loop() + d = imap.start_incoming_mail_service( + self._keymanager, + self._soledad, + self.imap_factory, + userid) + d.addCallback(start_incoming_service) + d.addCallback(assign_incoming_service) + d.addErrback(lambda f: logger.error(f.printTraceback())) def stop_imap_service(self, cv): """ @@ -77,11 +92,12 @@ class IMAPController(object): indeed stops. :type cv: threading.Condition """ - if self.imap_service is not None: + if self.incoming_mail_service is not None: # Stop the loop call in the fetcher - self.imap_service.stop() - self.imap_service = None + self.incoming_mail_service.stopService() + self.incoming_mail_service = None + if self.imap_port is not None: # Stop listening on the IMAP port self.imap_port.stopListening() @@ -98,6 +114,6 @@ class IMAPController(object): """ Fetch incoming mail. """ - if self.imap_service: + if self.incoming_mail_service is not None: logger.debug('Client connected, fetching mail...') - self.imap_service.fetch() + self.incoming_mail_service.fetch() -- cgit v1.2.3