From 561b2490c42376a48fc4be6588f6957b912ee43a Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 1 Aug 2016 19:40:28 +0530 Subject: [PATCH 1/9] [minor] email queue system more optimized --- frappe/email/doctype/newsletter/newsletter.py | 2 +- frappe/email/queue.py | 34 ++++++++++++------- frappe/utils/redis_wrapper.py | 6 ++++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/frappe/email/doctype/newsletter/newsletter.py b/frappe/email/doctype/newsletter/newsletter.py index 9dcc5f88d2..f8973aeb48 100755 --- a/frappe/email/doctype/newsletter/newsletter.py +++ b/frappe/email/doctype/newsletter/newsletter.py @@ -37,7 +37,7 @@ class Newsletter(Document): self.validate_send() # using default queue with a longer timeout as this isn't a scheduled task - enqueue(send_newsletter, queue='default', timeout=1500, event='send_newsletter', newsletter=self.name) + enqueue(send_newsletter, queue='default', timeout=3000, event='send_newsletter', newsletter=self.name) else: self.queue_all() diff --git a/frappe/email/queue.py b/frappe/email/queue.py index e4ca335098..96f4416424 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -135,7 +135,7 @@ def add(email, sender, subject, formatted, text_content=None, e.reference_name = reference_name e.communication = communication e.send_after = send_after - e.insert(ignore_permissions=True) + e.db_insert() def check_email_limit(recipients): # if using settings from site_config.json, check email limit @@ -151,6 +151,9 @@ def check_email_limit(recipients): monthly_email_limit = frappe.conf.get('limits', {}).get('emails') or 500 + if frappe.flags.in_test: + monthly_email_limit = 500 + if (this_month + len(recipients)) > monthly_email_limit: throw(_("Cannot send this email. You have crossed the sending limit of {0} emails for this month.").format(monthly_email_limit), EmailLimitCrossedError) @@ -256,16 +259,12 @@ def flush(from_test=False): smtpserver = SMTPServer() + make_cache_queue() + for i in xrange(500): - # don't use for update here, as it leads deadlocks - email = frappe.db.sql('''select * from `tabEmail Queue` - where status='Not Sent' and (send_after is null or send_after < %(now)s) - order by priority desc, creation asc - limit 1''', { 'now': now_datetime() }, as_dict=True) - - if email: - email = email[0] - else: + email = frappe.cache().lpop('cache_email_queue') + + if not email: break send_one(email, smtpserver, auto_commit) @@ -273,12 +272,23 @@ def flush(from_test=False): # NOTE: removing commit here because we pass auto_commit # finally: # frappe.db.commit() +def make_cache_queue(): + '''cache values in queue before sendign''' + cache = frappe.cache() + cache.delete_value('cache_email_queue') + + for l in frappe.db.sql('''select name from `tabEmail Queue` + where status='Not Sent' and (send_after is null or send_after < %(now)s) + order by priority desc, creation asc + limit 500''', { 'now': now_datetime() }): + cache.lpush('cache_email_queue', l[0]) def send_one(email, smtpserver=None, auto_commit=True, now=False): '''Send Email Queue with given smtpserver''' - status = frappe.db.sql('''select status from `tabEmail Queue` where name=%s for update''', email.name)[0][0] - if status != 'Not Sent': + email = frappe.db.sql('''select name, status, communication, message, sender, recipient + from `tabEmail Queue` where name=%s for update''', email, as_dict=True)[0] + if email.status != 'Not Sent': # rollback to release lock and return frappe.db.rollback() return diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index fadf3f70d5..1a7e54c43f 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -117,6 +117,12 @@ class RedisWrapper(redis.Redis): if key in frappe.local.cache: del frappe.local.cache[key] + def lpush(self, key, value): + super(redis.Redis, self).lpush(self.make_key(key), value) + + def lpop(self, key): + return super(redis.Redis, self).lpop(self.make_key(key)) + def hset(self, name, key, value): if not name in frappe.local.cache: frappe.local.cache[name] = {} From fd92c72cbe6f9be0b04236e8788014e47282c106 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 1 Aug 2016 20:05:36 +0530 Subject: [PATCH 2/9] [fix] [email] tests --- .../doctype/email_account/test_email_account.py | 1 + frappe/email/queue.py | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/frappe/email/doctype/email_account/test_email_account.py b/frappe/email/doctype/email_account/test_email_account.py index 53e405a27b..57645a2b29 100644 --- a/frappe/email/doctype/email_account/test_email_account.py +++ b/frappe/email/doctype/email_account/test_email_account.py @@ -16,6 +16,7 @@ class TestEmailAccount(unittest.TestCase): def setUp(self): email_account = frappe.get_doc("Email Account", "_Test Email Account 1") email_account.db_set("enable_incoming", 1) + frappe.db.sql('delete from `tabEmail Queue`') def tearDown(self): email_account = frappe.get_doc("Email Account", "_Test Email Account 1") diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 96f4416424..bf708d954b 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -275,18 +275,21 @@ def flush(from_test=False): def make_cache_queue(): '''cache values in queue before sendign''' cache = frappe.cache() - cache.delete_value('cache_email_queue') - for l in frappe.db.sql('''select name from `tabEmail Queue` + emails = frappe.db.sql('''select name from `tabEmail Queue` where status='Not Sent' and (send_after is null or send_after < %(now)s) order by priority desc, creation asc - limit 500''', { 'now': now_datetime() }): - cache.lpush('cache_email_queue', l[0]) + limit 500''', { 'now': now_datetime() }) + + cache.delete_value('cache_email_queue') + for e in emails: + cache.lpush('cache_email_queue', e[0]) def send_one(email, smtpserver=None, auto_commit=True, now=False): '''Send Email Queue with given smtpserver''' - email = frappe.db.sql('''select name, status, communication, message, sender, recipient + email = frappe.db.sql('''select name, status, communication, + message, sender, recipient, reference_doctype, from `tabEmail Queue` where name=%s for update''', email, as_dict=True)[0] if email.status != 'Not Sent': # rollback to release lock and return From 6429d7f067e12becd99f86dc187c942fafa242d1 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 1 Aug 2016 20:07:53 +0530 Subject: [PATCH 3/9] [fix] [email] tests --- frappe/email/queue.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index bf708d954b..84bb8ec959 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -289,7 +289,7 @@ def send_one(email, smtpserver=None, auto_commit=True, now=False): '''Send Email Queue with given smtpserver''' email = frappe.db.sql('''select name, status, communication, - message, sender, recipient, reference_doctype, + message, sender, recipient, reference_doctype from `tabEmail Queue` where name=%s for update''', email, as_dict=True)[0] if email.status != 'Not Sent': # rollback to release lock and return From 2bf6be0629583a8a3a2b516bf5f57531260e5c60 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Mon, 1 Aug 2016 22:04:23 +0530 Subject: [PATCH 4/9] [fix] email queue must be fifo with priority --- frappe/email/queue.py | 9 +++++---- frappe/utils/redis_wrapper.py | 4 ++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 84bb8ec959..64f1aaaf7d 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -254,9 +254,6 @@ def flush(from_test=False): msgprint(_("Emails are muted")) from_test = True - frappe.db.sql("""update `tabEmail Queue` set status='Expired' - where datediff(curdate(), creation) > 7 and status='Not Sent'""", auto_commit=auto_commit) - smtpserver = SMTPServer() make_cache_queue() @@ -281,9 +278,10 @@ def make_cache_queue(): order by priority desc, creation asc limit 500''', { 'now': now_datetime() }) + # reset value cache.delete_value('cache_email_queue') for e in emails: - cache.lpush('cache_email_queue', e[0]) + cache.rpush('cache_email_queue', e[0]) def send_one(email, smtpserver=None, auto_commit=True, now=False): '''Send Email Queue with given smtpserver''' @@ -350,3 +348,6 @@ def clear_outbox(): """Remove mails older than 31 days in Outbox. Called daily via scheduler.""" frappe.db.sql("""delete from `tabEmail Queue` where datediff(now(), creation) > 31""") + + frappe.db.sql("""update `tabEmail Queue` set status='Expired' + where datediff(curdate(), creation) > 7 and status='Not Sent'""") diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 1a7e54c43f..ed2f3d7497 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -120,6 +120,9 @@ class RedisWrapper(redis.Redis): def lpush(self, key, value): super(redis.Redis, self).lpush(self.make_key(key), value) + def rpush(self, key, value): + super(redis.Redis, self).rpush(self.make_key(key), value) + def lpop(self, key): return super(redis.Redis, self).lpop(self.make_key(key)) @@ -180,3 +183,4 @@ class RedisWrapper(redis.Redis): except redis.exceptions.ConnectionError: return [] + From ab1d9478714f423d9c9d17eb4e411a3fb5c825ed Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 2 Aug 2016 10:25:31 +0530 Subject: [PATCH 5/9] [fix] email queue --- frappe/email/queue.py | 11 +++++------ frappe/utils/redis_wrapper.py | 3 +++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/frappe/email/queue.py b/frappe/email/queue.py index 64f1aaaf7d..e07abd288a 100755 --- a/frappe/email/queue.py +++ b/frappe/email/queue.py @@ -247,6 +247,7 @@ def return_unsubscribed_page(email, doctype, name): def flush(from_test=False): """flush email queue, every time: called from scheduler""" # additional check + cache = frappe.cache() check_email_limit([]) auto_commit = not from_test @@ -258,13 +259,11 @@ def flush(from_test=False): make_cache_queue() - for i in xrange(500): - email = frappe.cache().lpop('cache_email_queue') - - if not email: - break + for i in xrange(cache.llen('cache_email_queue')): + email = cache.lpop('cache_email_queue') - send_one(email, smtpserver, auto_commit) + if email: + send_one(email, smtpserver, auto_commit) # NOTE: removing commit here because we pass auto_commit # finally: diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index ed2f3d7497..55ec0ae4e6 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -126,6 +126,9 @@ class RedisWrapper(redis.Redis): def lpop(self, key): return super(redis.Redis, self).lpop(self.make_key(key)) + def llen(self, key): + super(redis.Redis, self).llen(self.make_key(key)) + def hset(self, name, key, value): if not name in frappe.local.cache: frappe.local.cache[name] = {} From 61ea2522d9cce0f3613005b10eed7704b0180626 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 2 Aug 2016 10:35:38 +0530 Subject: [PATCH 6/9] [fix] email queue --- frappe/utils/redis_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 55ec0ae4e6..06d124447d 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -127,7 +127,7 @@ class RedisWrapper(redis.Redis): return super(redis.Redis, self).lpop(self.make_key(key)) def llen(self, key): - super(redis.Redis, self).llen(self.make_key(key)) + return super(redis.Redis, self).llen(self.make_key(key)) def hset(self, name, key, value): if not name in frappe.local.cache: From c61bae36599e22a2177ea496bcf7a87c12bae1e7 Mon Sep 17 00:00:00 2001 From: Rushabh Mehta Date: Tue, 2 Aug 2016 11:57:06 +0530 Subject: [PATCH 7/9] [fix] test for unsubscribe --- frappe/utils/verified_command.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/frappe/utils/verified_command.py b/frappe/utils/verified_command.py index 8cc58a73c2..57170295f8 100644 --- a/frappe/utils/verified_command.py +++ b/frappe/utils/verified_command.py @@ -25,19 +25,23 @@ def get_secret(): def verify_request(): """Verify if the incoming signed request if it is correct.""" - query_string = frappe.request.query_string if hasattr(frappe.request, "query_string") \ - else frappe.local.flags.signed_query_string + query_string = frappe.local.flags.signed_query_string or \ + getattr(frappe.request, 'query_string', None) \ - params, signature = query_string.split("&_signature=") + valid = False - given_signature = hmac.new(params.encode("utf-8")) + if '&_signature=' in query_string: + params, signature = query_string.split("&_signature=") - given_signature.update(get_secret()) - valid = signature == given_signature.hexdigest() + given_signature = hmac.new(params.encode("utf-8")) + + given_signature.update(get_secret()) + valid = signature == given_signature.hexdigest() if not valid: frappe.respond_as_web_page(_("Invalid Link"), _("This link is invalid or expired. Please make sure you have pasted correctly.")) + return valid def get_url(cmd, params, nonce=None, secret=None): From 18b2837b4de457851b1784ca0eee7570e8ab4897 Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Tue, 2 Aug 2016 15:37:46 +0530 Subject: [PATCH 8/9] [ui] blog comments area fix --- frappe/public/css/website.css | 11 +---------- frappe/public/less/website.less | 12 +----------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/frappe/public/css/website.css b/frappe/public/css/website.css index be733bb491..3ef2ccfc6b 100644 --- a/frappe/public/css/website.css +++ b/frappe/public/css/website.css @@ -686,17 +686,8 @@ fieldset { padding-bottom: 30px; } .blog-comments { - background-color: #fafbfc; position: relative; -} -.blog-comments:before { - content: ""; - background-color: #fafbfc; - position: absolute; - height: 100%; - width: 100vw; - left: calc((100vw - 100%)/ -2); - z-index: -1; + border-top: 1px solid #d1d8dd; } .blog-comment-row { margin: 0px -15px; diff --git a/frappe/public/less/website.less b/frappe/public/less/website.less index 1175be249b..70e98eb732 100644 --- a/frappe/public/less/website.less +++ b/frappe/public/less/website.less @@ -396,18 +396,8 @@ fieldset { .help-article-comments { } .blog-comments { - background-color: @light-bg; position: relative; -} - -.blog-comments:before { - content:""; - background-color: @light-bg; - position: absolute; - height: 100%; - width: 100vw; - left: ~"calc((100vw - 100%)/ -2)"; - z-index: -1; + border-top: 1px solid @border-color; } .blog-comment-row { From ec625b9988f9ee191c19a36bdd2a11ce22c1d783 Mon Sep 17 00:00:00 2001 From: Nabin Hait Date: Tue, 2 Aug 2016 17:47:49 +0600 Subject: [PATCH 9/9] bumped to version 7.0.15 --- frappe/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 311fc5b993..43e6edf02b 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -13,7 +13,7 @@ import os, sys, importlib, inspect, json from .exceptions import * from .utils.jinja import get_jenv, get_template, render_template -__version__ = "7.0.14" +__version__ = "7.0.15" local = Local()