PERF: Restrict number of skipped email log for `Jobs::UserEmail`.

This commit is contained in:
Guo Xiang Tan 2018-08-21 11:14:43 +08:00
parent 7c69fa8cfd
commit 8bdf14834b
5 changed files with 62 additions and 23 deletions

View File

@ -0,0 +1,28 @@
module Skippable
extend ActiveSupport::Concern
def create_skipped_email_log(email_type:,
to_address:,
user_id:,
post_id:,
reason_type:)
attributes = {
email_type: email_type,
to_address: to_address,
user_id: user_id,
post_id: post_id,
reason_type: reason_type
}
if reason_type == SkippedEmailLog.reason_types[:exceeded_emails_limit]
exists = SkippedEmailLog.exists?({
created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)
}.merge!(attributes.except(:post_id)))
return if exists
end
SkippedEmailLog.create!(attributes)
end
end

View File

@ -3,6 +3,7 @@ require_dependency 'post'
module Jobs module Jobs
class NotifyMailingListSubscribers < Jobs::Base class NotifyMailingListSubscribers < Jobs::Base
include Skippable
sidekiq_options queue: 'low' sidekiq_options queue: 'low'
@ -80,23 +81,13 @@ module Jobs
end end
def skip(to_address, user_id, post_id, reason_type) def skip(to_address, user_id, post_id, reason_type)
attributes = { create_skipped_email_log(
email_type: 'mailing_list', email_type: 'mailing_list',
to_address: to_address, to_address: to_address,
user_id: user_id, user_id: user_id,
post_id: post_id, post_id: post_id,
reason_type: reason_type reason_type: reason_type
} )
if reason_type == SkippedEmailLog.reason_types[:exceeded_emails_limit]
exists = SkippedEmailLog.exists?({
created_at: (Time.zone.now.beginning_of_day..Time.zone.now.end_of_day)
}.merge!(attributes.except(:post_id)))
return if exists
end
SkippedEmailLog.create!(attributes)
end end
end end
end end

View File

@ -5,6 +5,7 @@ module Jobs
# Asynchronously send an email to a user # Asynchronously send an email to a user
class UserEmail < Jobs::Base class UserEmail < Jobs::Base
include Skippable
sidekiq_options queue: 'low' sidekiq_options queue: 'low'
@ -202,7 +203,7 @@ module Jobs
end end
def skip(reason_type) def skip(reason_type)
SkippedEmailLog.create!( create_skipped_email_log(
email_type: @skip_context[:type], email_type: @skip_context[:type],
to_address: @skip_context[:to_address], to_address: @skip_context[:to_address],
user_id: @skip_context[:user_id], user_id: @skip_context[:user_id],

View File

@ -136,7 +136,7 @@ describe Jobs::NotifyMailingListSubscribers do
end end
Jobs::NotifyMailingListSubscribers.new.execute( Jobs::NotifyMailingListSubscribers.new.execute(
post_id: Fabricate(:post, user: user) post_id: Fabricate(:post, user: user).id
) )
end.to change { SkippedEmailLog.count }.by(1) end.to change { SkippedEmailLog.count }.by(1)
@ -148,13 +148,19 @@ describe Jobs::NotifyMailingListSubscribers do
reason_type: SkippedEmailLog.reason_types[:exceeded_emails_limit] reason_type: SkippedEmailLog.reason_types[:exceeded_emails_limit]
)).to eq(true) )).to eq(true)
freeze_time(Time.zone.now + 1.day) freeze_time(Time.zone.now.tomorrow + 1.second)
expect do expect do
post = Fabricate(:post, user: user)
UserNotifications.expects(:mailing_list_notify)
.with(mailing_list_user, post)
.once
Jobs::NotifyMailingListSubscribers.new.execute( Jobs::NotifyMailingListSubscribers.new.execute(
post_id: Fabricate(:post, user: user) post_id: post.id
) )
end.to change { SkippedEmailLog.count }.by(1) end.to change { SkippedEmailLog.count }.by(0)
end end
end end

View File

@ -245,12 +245,14 @@ describe Jobs::UserEmail do
it "does not send notification if limit is reached" do it "does not send notification if limit is reached" do
expect do expect do
2.times do
Jobs::UserEmail.new.execute( Jobs::UserEmail.new.execute(
type: :user_mentioned, type: :user_mentioned,
user_id: user.id, user_id: user.id,
notification_id: notification.id, notification_id: notification.id,
post_id: post.id post_id: post.id
) )
end
end.to change { SkippedEmailLog.count }.by(1) end.to change { SkippedEmailLog.count }.by(1)
expect(SkippedEmailLog.exists?( expect(SkippedEmailLog.exists?(
@ -260,6 +262,17 @@ describe Jobs::UserEmail do
to_address: user.email, to_address: user.email,
reason_type: SkippedEmailLog.reason_types[:exceeded_emails_limit] reason_type: SkippedEmailLog.reason_types[:exceeded_emails_limit]
)).to eq(true) )).to eq(true)
freeze_time(Time.zone.now.tomorrow + 1.second)
expect do
Jobs::UserEmail.new.execute(
type: :user_mentioned,
user_id: user.id,
notification_id: notification.id,
post_id: post.id
)
end.to change { SkippedEmailLog.count }.by(0)
end end
it "sends critical email" do it "sends critical email" do