Refactoring more of resource logic into service. No more search engine on mail(s) resource

6 files changed, 46 insertions, 65 deletions

diff --git a/service/pixelated/adapter/services/mail_service.py b/service/pixelated/adapter/services/mail_service.py
index da74c0ef..6e309ee0 100644
--- a/service/pixelated/adapter/services/mail_service.py
+++ b/service/pixelated/adapter/services/mail_service.py
@@ -35,11 +35,14 @@ class MailService:
         return self.querier.mails(mail_ids), total
 
     def update_tags(self, mail_id, new_tags):
+        new_tags = [x.lower() for x in map(lambda e: e.strip(), new_tags) if x != '']
         reserved_words = self.tag_service.extract_reserved(new_tags)
         if len(reserved_words):
             raise ValueError('None of the following words can be used as tags: ' + ' '.join(reserved_words))
         mail = self.mail(mail_id)
         mail.update_tags(set(new_tags))
+        self.search_engine.index_mail(mail)
+
         return mail
 
     def mail(self, mail_id):
@@ -63,14 +66,24 @@ class MailService:
         return self.mailboxes.sent().add(mail)
 
     def mark_as_read(self, mail_id):
-        return self.mail(mail_id).mark_as_read()
+        mail = self.mail(mail_id)
+        mail.mark_as_read()
+        self.search_engine.index_mail(mail)
 
     def mark_as_unread(self, mail_id):
-        return self.mail(mail_id).mark_as_unread()
+        mail = self.mail(mail_id)
+        mail.mark_as_unread()
+        self.search_engine.index_mail(mail)
 
     def delete_mail(self, mail_id):
-        return self.mailboxes.move_to_trash(mail_id)
+        mail = self.mail(mail_id)
+        if mail.mailbox_name == 'TRASH':
+            self.delete_permanent(mail_id)
+        else:
+            trashed_mail = self.mailboxes.move_to_trash(mail_id)
+            self.search_engine.index_mail(trashed_mail)
 
     def delete_permanent(self, mail_id):
         mail = self.mail(mail_id)
+        self.search_engine.remove_from_index(mail_id)
         self.querier.remove_mail(mail)
diff --git a/service/pixelated/resources/__init__.py b/service/pixelated/resources/__init__.py
index 92a4462f..a2e4c9d4 100644
--- a/service/pixelated/resources/__init__.py
+++ b/service/pixelated/resources/__init__.py
@@ -20,7 +20,6 @@ def respond_json(entity, request, status_code=200):
     request.responseHeaders.addRawHeader(b"content-type", b"application/json")
     request.code = status_code
     return json_response
-    # request.finish()
 
 
 def respond_json_deferred(entity, request, status_code=200):
diff --git a/service/pixelated/resources/mail_resource.py b/service/pixelated/resources/mail_resource.py
index fd1d255f..dff594b0 100644
--- a/service/pixelated/resources/mail_resource.py
+++ b/service/pixelated/resources/mail_resource.py
@@ -7,19 +7,16 @@ class MailTags(Resource):
 
     isLeaf = True
 
-    def __init__(self, mail_id, mail_service, search_engine):
+    def __init__(self, mail_id, mail_service):
         Resource.__init__(self)
-        self._search_engine = search_engine
         self._mail_service = mail_service
         self._mail_id = mail_id
 
     def render_POST(self, request):
-        content_dict = json.loads(request.content.read())
-        new_tags = [x.lower() for x in map(lambda e: e.strip(), content_dict['newtags']) if x != '']
+        new_tags = json.loads(request.content.read()).get('newtags')
+
         try:
-            self._mail_service.update_tags(self._mail_id, new_tags)
-            mail = self._mail_service.mail(self._mail_id)
-            self._search_engine.index_mail(mail)
+            mail = self._mail_service.update_tags(self._mail_id, new_tags)
         except ValueError as ve:
             return respond_json(ve.message, request, 403)
         return respond_json(mail.as_dict(), request)
@@ -27,11 +24,9 @@ class MailTags(Resource):
 
 class Mail(Resource):
 
-    def __init__(self, mail_id, mail_service, search_engine):
+    def __init__(self, mail_id, mail_service):
         Resource.__init__(self)
-        self.putChild('tags', MailTags(mail_id, mail_service, search_engine))
-
-        self._search_engine = search_engine
+        self.putChild('tags', MailTags(mail_id, mail_service))
         self._mail_id = mail_id
         self._mail_service = mail_service
 
@@ -40,25 +35,15 @@ class Mail(Resource):
         return respond_json(mail.as_dict(), request)
 
     def render_DELETE(self, request):
-        self._delete_mail(self._mail_id)
+        self._mail_service.delete_mail(self._mail_id)
         return respond_json(None, request)
 
-    def _delete_mail(self, mail_id):
-        mail = self._mail_service.mail(mail_id)
-        if mail.mailbox_name == 'TRASH':
-            self._mail_service.delete_permanent(mail_id)
-            self._search_engine.remove_from_index(mail_id)
-        else:
-            trashed_mail = self._mail_service.delete_mail(mail_id)
-            self._search_engine.index_mail(trashed_mail)
-
 
 class MailResource(Resource):
 
-    def __init__(self, mail_service, search_engine):
+    def __init__(self, mail_service):
         Resource.__init__(self)
         self._mail_service = mail_service
-        self._search_engine = search_engine
 
     def getChild(self, mail_id, request):
-        return Mail(mail_id, self._mail_service, self._search_engine)
+        return Mail(mail_id, self._mail_service)
diff --git a/service/pixelated/resources/mails_resource.py b/service/pixelated/resources/mails_resource.py
index 68d0246a..77a47cda 100644
--- a/service/pixelated/resources/mails_resource.py
+++ b/service/pixelated/resources/mails_resource.py
@@ -11,60 +11,45 @@ from leap.common.events import (
 class MailsUnreadResource(Resource):
     isLeaf = True
 
-    def __init__(self, mail_service, search_engine):
+    def __init__(self, mail_service):
         Resource.__init__(self)
-        self._search_engine = search_engine
         self._mail_service = mail_service
 
     def render_POST(self, request):
-        content_dict = json.load(request.content)
-        idents = content_dict.get('idents')
+        idents = json.load(request.content).get('idents')
         for ident in idents:
-            mail = self._mail_service.mark_as_unread(ident)
-            self._search_engine.index_mail(mail)
+            self._mail_service.mark_as_unread(ident)
         return respond_json(None, request)
 
 
 class MailsReadResource(Resource):
     isLeaf = True
 
-    def __init__(self, mail_service, search_engine):
+    def __init__(self, mail_service):
         Resource.__init__(self)
-        self._search_engine = search_engine
         self._mail_service = mail_service
 
     def render_POST(self, request):
-        content_dict = json.load(request.content)
-        idents = content_dict.get('idents')
+        idents = json.load(request.content).get('idents')
         for ident in idents:
-            mail = self._mail_service.mark_as_read(ident)
-            self._search_engine.index_mail(mail)
+            self._mail_service.mark_as_read(ident)
+
         return respond_json(None, request)
 
 
 class MailsDeleteResource(Resource):
     isLeaf = True
 
-    def __init__(self, mail_service, search_engine):
+    def __init__(self, mail_service):
         Resource.__init__(self)
         self._mail_service = mail_service
-        self._search_engine = search_engine
 
     def render_POST(self, request):
         idents = json.loads(request.content.read())['idents']
         for ident in idents:
-            self._delete_mail(ident)
+            self._mail_service.delete_mail(ident)
         return respond_json(None, request)
 
-    def _delete_mail(self, mail_id):
-        mail = self._mail_service.mail(mail_id)
-        if mail.mailbox_name == 'TRASH':
-            self._mail_service.delete_permanent(mail_id)
-            self._search_engine.remove_from_index(mail_id)
-        else:
-            trashed_mail = self._mail_service.delete_mail(mail_id)
-            self._search_engine.index_mail(trashed_mail)
-
 
 class MailsResource(Resource):
 
@@ -72,20 +57,18 @@ class MailsResource(Resource):
 
         def on_error(event):
             delivery_error_mail = InputMail.delivery_error_template(delivery_address=event.content)
-            delivery_error_mail = self._mail_service.mailboxes.inbox().add(delivery_error_mail)
-            self._search_engine.index_mail(delivery_error_mail)
+            self._mail_service.mailboxes.inbox().add(delivery_error_mail)
 
         register(signal=proto.SMTP_SEND_MESSAGE_ERROR, callback=on_error)
 
-    def __init__(self, search_engine, mail_service, draft_service):
+    def __init__(self, mail_service, draft_service):
         Resource.__init__(self)
-        self.putChild('delete', MailsDeleteResource(mail_service, search_engine))
-        self.putChild('read', MailsReadResource(mail_service, search_engine))
-        self.putChild('unread', MailsUnreadResource(mail_service, search_engine))
+        self.putChild('delete', MailsDeleteResource(mail_service))
+        self.putChild('read', MailsReadResource(mail_service))
+        self.putChild('unread', MailsUnreadResource(mail_service))
 
         self._draft_service = draft_service
         self._mail_service = mail_service
-        self._search_engine = search_engine
         self._register_smtp_error_handler()
 
     def render_GET(self, request):
@@ -116,9 +99,7 @@ class MailsResource(Resource):
             if not self._mail_service.mail_exists(draft_id):
                 return respond_json("", request, status_code=422)
             pixelated_mail = self._draft_service.update_draft(draft_id, _mail)
-            self._search_engine.remove_from_index(draft_id)
         else:
             pixelated_mail = self._draft_service.create_draft(_mail)
-        self._search_engine.index_mail(pixelated_mail)
 
         return respond_json({'ident': pixelated_mail.ident}, request)
diff --git a/service/pixelated/resources/root_resource.py b/service/pixelated/resources/root_resource.py
index 7768472c..71fe1fce 100644
--- a/service/pixelated/resources/root_resource.py
+++ b/service/pixelated/resources/root_resource.py
@@ -28,8 +28,8 @@ class RootResource(Resource):
         self.putChild('features', FeaturesResource())
         self.putChild('sync_info', SyncInfoResource())
         self.putChild('tags', TagsResource(search_engine))
-        self.putChild('mails', MailsResource(search_engine, mail_service, draft_service))
-        self.putChild('mail', MailResource(mail_service, search_engine))
+        self.putChild('mails', MailsResource(mail_service, draft_service))
+        self.putChild('mail', MailResource(mail_service))
 
     def _get_static_folder(self):
 
diff --git a/service/test/unit/adapter/test_mail_service.py b/service/test/unit/adapter/test_mail_service.py
index 4b1c60d6..137c17ee 100644
--- a/service/test/unit/adapter/test_mail_service.py
+++ b/service/test/unit/adapter/test_mail_service.py
@@ -14,10 +14,10 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with Pixelated. If not, see <http://www.gnu.org/licenses/>.
 import unittest
-from pixelated.adapter.model.mail import InputMail
+from pixelated.adapter.model.mail import InputMail, PixelatedMail
 
 from pixelated.adapter.services.mail_service import MailService
-from test.support.test_helper import mail_dict
+from test.support.test_helper import mail_dict, leap_mail
 from mockito import *
 
 
@@ -49,6 +49,9 @@ class TestMailService(unittest.TestCase):
         verify(mail).mark_as_read()
 
     def test_delete_mail(self):
+        mail_to_delete = PixelatedMail.from_soledad(*leap_mail(), soledad_querier=None)
+        when(self.mail_service).mail(1).thenReturn(mail_to_delete)
+
         self.mail_service.delete_mail(1)
 
         verify(self.mailboxes).move_to_trash(1)