kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity
gitlab-org--gitlab-foss/spec/services/notification_recipient_service_spec.rb

37 lines
1.1 KiB
Ruby
Raw Normal View History

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.
2018-06-07 07:14:27 -04:00
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)
Remove Gitaly N+1 ignore from NotificationRecipientService spec This was because we were creating projects unnecessarily. These should be set as the `source` for the notification setting, not the `project`!
2018-07-04 11:12:37 -04:00
create(:notification_setting, source: project, user: watcher, level: :watch)
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.
2018-06-07 07:14:27 -04:00
other_projects.each do |other_project|
Remove Gitaly N+1 ignore from NotificationRecipientService spec This was because we were creating projects unnecessarily. These should be set as the `source` for the notification setting, not the `project`!
2018-07-04 11:12:37 -04:00
create(:notification_setting, source: other_project, user: watcher, level: :watch)
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.
2018-06-07 07:14:27 -04:00
end
end
Fix NotificationRecipientService spec for EE EE checks a license, which needs RequestStore enabled to avoid N+1 queries. However, enabling RequestStore causes Gitaly to complain about N+1 invocations, which we really don't care about here.
2018-06-08 06:45:36 -04:00
it 'avoids N+1 queries', :request_store do
Remove Gitaly N+1 ignore from NotificationRecipientService spec This was because we were creating projects unnecessarily. These should be set as the `source` for the notification setting, not the `project`!
2018-07-04 11:12:37 -04:00
create_watcher
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.
2018-06-07 07:14:27 -04:00
service.build_new_note_recipients(note)
control_count = ActiveRecord::QueryRecorder.new do
service.build_new_note_recipients(note)
end
Remove Gitaly N+1 ignore from NotificationRecipientService spec This was because we were creating projects unnecessarily. These should be set as the `source` for the notification setting, not the `project`!
2018-07-04 11:12:37 -04:00
create_watcher
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.
2018-06-07 07:14:27 -04:00
expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count)
end
end
end
Copy permalink