[tor-commits] [gettor/master] Refactor to dry out sendmail

cohosh at torproject.org cohosh at torproject.org
Wed Apr 29 15:23:31 UTC 2020


commit 390120a0f926449e833705ec5323d80bb70f3327
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Mon Apr 27 18:26:03 2020 -0400

    Refactor to dry out sendmail
    
    Instead of processing link and help requests separately, we can process
    them together and use the command to determine the different subject and
    body messages.
    
    Removed unused code and simplified database command.
---
 gettor/services/email/sendmail.py | 135 +++++++++++++-------------------------
 gettor/utils/db.py                |   7 +-
 tests/test_db.py                  |  16 ++---
 3 files changed, 57 insertions(+), 101 deletions(-)

diff --git a/gettor/services/email/sendmail.py b/gettor/services/email/sendmail.py
index c4fdc95..cb2ab9a 100644
--- a/gettor/services/email/sendmail.py
+++ b/gettor/services/email/sendmail.py
@@ -168,77 +168,36 @@ class Sendmail(object):
         """
 
         # Manage help and links messages separately
-        help_requests = yield self.conn.get_requests(
-            status="ONHOLD", command="help", service="email"
+        requests = yield self.conn.get_requests(
+            status="ONHOLD", service="email"
         )
 
-        link_requests = yield self.conn.get_requests(
-            status="ONHOLD", command="links", service="email"
-        )
+        strings.load_strings("en")
+        try:
 
-        if help_requests:
-            strings.load_strings("en")
-            try:
-                log.info("Got new help request.")
+            for request in requests:
+                id = request[0]
+                command = request[1]
+                platform = request[2]
+                language = request[3]
+                date = request[5]
 
-                for request in help_requests:
-                    id = request[0]
-                    date = request[5]
+                if not language:
+                    language = 'en'
+
+                body_msg =""
+                subject_msg =""
+
+                if command == "help":
 
-                    hid = hashlib.sha256(id.encode('utf-8'))
-                    log.info(
-                        "Sending help message to {}.".format(
-                            hid.hexdigest()
-                        )
-                    )
                     locales = yield self.conn.get_locales()
                     locale_string = self.build_locale_string(locales)
 
+                    # build message
                     body_msg = self.build_help_body_message(locale_string)
+                    subject_msg = strings._("help_subject")
 
-                    yield self.sendmail(
-                        email_addr=id,
-                        subject=strings._("help_subject"),
-                        body=body_msg
-                    )
-
-                    yield self.conn.update_stats(
-                        command="help", platform='', language='en',
-                        service="email"
-                    )
-
-                    yield self.conn.remove_request(
-                        id=id, service="email", date=date
-                    )
-
-            except smtp.SMTPClientError as e:
-                if e.code == 501: # Bad recipient address syntax
-                    yield self.conn.remove_request(
-                        id=id, service="email", date=date
-                    )
-                log.info("Error sending email: {}.".format(e))
-
-            except Exception as e:
-                log.info("Error sending email: {}.".format(e))
-
-        elif link_requests:
-            try:
-                log.info("Got new links request.")
-
-                for request in link_requests:
-                    id = request[0]
-                    date = request[5]
-                    platform = request[2]
-                    language = request[3]
-
-                    if not language:
-                        language = 'en'
-
-                    locales = strings.get_locales()
-
-                    strings.load_strings(language)
-                    locale = locales['en']['locale']
-
+                elif command == "links":
                     log.info("Getting links for {} {}.".format(platform, language))
                     links = yield self.conn.get_links(
                         platform=platform, language=language, status="ACTIVE"
@@ -248,37 +207,35 @@ class Sendmail(object):
                     link_msg, file = self.build_link_strings(links, platform, language)
                     body_msg = self.build_body_message(link_msg, platform, file)
                     subject_msg = strings._("links_subject")
-
-                    hid = hashlib.sha256(id.encode('utf-8'))
-                    log.info(
-                        "Sending links to {}.".format(
-                            hid.hexdigest()
-                        )
+                else:
+                    log.info("Invalid gettor command {}.".format(command))
+                    yield self.conn.remove_request(
+                        id=id, service="email", date=date
                     )
 
-                    yield self.sendmail(
-                        email_addr=id,
-                        subject=subject_msg,
-                        body=body_msg
-                    )
+                log.info("Sending {} message.".format(request[1]))
 
-                    yield self.conn.update_stats(
-                        command="links", platform=platform, language=language,
-                        service="email"
-                    )
+                yield self.sendmail(
+                    email_addr=id,
+                    subject=subject_msg,
+                    body=body_msg
+                )
 
-                    yield self.conn.remove_request(
-                        id=id, service="email", date=date
-                    )
+                yield self.conn.update_stats(
+                    command=command, platform=platform, language=language,
+                    service="email"
+                )
 
-            except smtp.SMTPClientError as e:
-                if e.code == 501: # Bad recipient address syntax
-                    yield self.conn.remove_request(
-                        id=id, service="email", date=date
-                    )
-                log.info("Error sending email: {}.".format(e))
+                yield self.conn.remove_request(
+                    id=id, service="email", date=date
+                )
+
+        except smtp.SMTPClientError as e:
+            if e.code == 501: # Bad recipient address syntax
+                yield self.conn.remove_request(
+                    id=id, service="email", date=date
+                )
+            log.info("Error sending email: {}.".format(e))
 
-            except Exception as e:
-                log.info("Error sending email: {}.".format(e))
-        else:
-            log.debug("No pending email requests. Keep waiting.")
+        except Exception as e:
+            log.error("Error sending email: {}.".format(e))
diff --git a/gettor/utils/db.py b/gettor/utils/db.py
index 064049e..d6bf153 100644
--- a/gettor/utils/db.py
+++ b/gettor/utils/db.py
@@ -54,15 +54,14 @@ class SQLite3(object):
 			query, (id, command, platform, language, service, date, status)
 		).addCallback(self.query_callback).addErrback(self.query_errback)
 
-	def get_requests(self, status, command, service):
+	def get_requests(self, status, service):
 		"""
 		Perform a SELECT request to the database
 		"""
-		query = "SELECT * FROM requests WHERE service=? AND command=? AND "\
-		"status = ?"
+		query = "SELECT * FROM requests WHERE service=? AND status = ?"
 
 		return self.dbpool.runQuery(
-			query, (service, command, status)
+			query, (service, status)
 		).addCallback(self.query_callback).addErrback(self.query_errback)
 
 	def get_num_requests(self, id, service):
diff --git a/tests/test_db.py b/tests/test_db.py
index 7d5634b..909abb6 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -21,12 +21,12 @@ class DatabaseTests(unittest.TestCase):
         del self.conn
 
     @pytest_twisted.inlineCallbacks
-    def add_dummy_requests(self, num):
+    def add_dummy_requests(self, command, num):
         now_str = datetime.now().strftime("%Y%m%d")
-        for i in (0, num):
+        for i in range(0, num):
             yield self.conn.new_request(
                 id='testid',
-                command='links',
+                command=command,
                 platform='linux',
                 language='en',
                 service='email',
@@ -45,17 +45,17 @@ class DatabaseTests(unittest.TestCase):
     @pytest_twisted.inlineCallbacks
     def test_requests(self):
         now_str = datetime.now().strftime("%Y%m%d")
-        yield self.add_dummy_requests(2)
+        yield self.add_dummy_requests("links", 2)
+        yield self.add_dummy_requests("help", 1)
         num = yield self.conn.get_num_requests("testid", "email")
-        self.assertEqual(num[0][0], 2)
+        self.assertEqual(num[0][0], 3)
 
-        requests = yield self.conn.get_requests("ONHOLD", "links", "email")
+        requests = yield self.conn.get_requests("ONHOLD", "email")
         for request in requests:
-            self.assertEqual(request[1], "links")
             self.assertEqual(request[4], "email")
             self.assertEqual(request[5], now_str)
             self.assertEqual(request[6], "ONHOLD")
-        self.assertEqual(len(requests), 2)
+        self.assertEqual(len(requests), 3)
 
         yield self.conn.remove_request("testid", "email", now_str)
         num = yield self.conn.get_num_requests("testid", "email")



More information about the tor-commits mailing list