diff options
author | Sean McGivern <sean@gitlab.com> | 2018-06-07 14:14:27 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-06-07 14:37:57 +0300 |
commit | 0206476ae2a2d659fd0fb42338050253a9a91439 (patch) | |
tree | b3fa79ce632f03185652b2a5801e08cef9e24893 /spec/services/notification_recipient_service_spec.rb | |
parent | 95b3e51f117167d48ba04f2dcef6922aff885617 (diff) |
Fix some N+1s when calculating notification recipients
First N+1: we may have loaded a user's notification settings already, but not
have loaded their sources. Because we're iterating through, we'd potentially
load sources that are completely unrelated, just because they belong to this
user.
Second N+1: we do a separate query for each user who could be subscribed to or
unsubcribed from the target. It's actually more efficient in this case to get
all subscriptions at once, as we will need to check most of them.
We can fix both by the slightly unpleasant means of checking IDs manually,
rather than object equality.
Diffstat (limited to 'spec/services/notification_recipient_service_spec.rb')
-rw-r--r-- | spec/services/notification_recipient_service_spec.rb | 36 |
1 files changed, 36 insertions, 0 deletions
diff --git a/spec/services/notification_recipient_service_spec.rb b/spec/services/notification_recipient_service_spec.rb new file mode 100644 index 00000000000..340d4585e0c --- /dev/null +++ b/spec/services/notification_recipient_service_spec.rb @@ -0,0 +1,36 @@ +require 'spec_helper' + +describe NotificationRecipientService do + let(:service) { described_class } + let(:assignee) { create(:user) } + let(:project) { create(:project, :public) } + let(:other_projects) { create_list(:project, 5, :public) } + + describe '#build_new_note_recipients' do + let(:issue) { create(:issue, project: project, assignees: [assignee]) } + let(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id) } + + def create_watcher + watcher = create(:user) + create(:notification_setting, project: project, user: watcher, level: :watch) + + other_projects.each do |other_project| + create(:notification_setting, project: other_project, user: watcher, level: :watch) + end + end + + it 'avoids N+1 queries' do + create_watcher + + service.build_new_note_recipients(note) + + control_count = ActiveRecord::QueryRecorder.new do + service.build_new_note_recipients(note) + end + + create_watcher + + expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count) + end + end +end |