commit 390120a0f926449e833705ec5323d80bb70f3327 Author: Cecylia Bocovich <cohosh@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")