Commit 698ba370 authored by Igor Drozdov's avatar Igor Drozdov

Remove N+1 for API commits/:sha/merge_requests

When a commit belongs to multiple MRs, N+1 is performed
parent 6f196179
---
title: Remove N+1 for API commits/:sha/merge_requests
merge_request: 57290
author:
type: performance
...@@ -372,7 +372,7 @@ module API ...@@ -372,7 +372,7 @@ module API
current_user, current_user,
project_id: user_project.id, project_id: user_project.id,
commit_sha: commit.sha commit_sha: commit.sha
).execute ).execute.with_api_entity_associations
present paginate(commit_merge_requests), with: Entities::MergeRequestBasic present paginate(commit_merge_requests), with: Entities::MergeRequestBasic
end end
......
...@@ -1898,8 +1898,12 @@ RSpec.describe API::Commits do ...@@ -1898,8 +1898,12 @@ RSpec.describe API::Commits do
let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') } let(:merged_mr) { create(:merge_request, source_project: project, source_branch: 'master', target_branch: 'feature') }
let(:commit) { merged_mr.merge_request_diff.commits.last } let(:commit) { merged_mr.merge_request_diff.commits.last }
it 'returns the correct merge request' do def perform_request(user)
get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", user) get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", user)
end
it 'returns the correct merge request' do
perform_request(user)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to include_limited_pagination_headers expect(response).to include_limited_pagination_headers
...@@ -1910,7 +1914,7 @@ RSpec.describe API::Commits do ...@@ -1910,7 +1914,7 @@ RSpec.describe API::Commits do
it 'returns 403 for an unauthorized user' do it 'returns 403 for an unauthorized user' do
project.add_guest(user) project.add_guest(user)
get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", user) perform_request(user)
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
end end
...@@ -1926,11 +1930,21 @@ RSpec.describe API::Commits do ...@@ -1926,11 +1930,21 @@ RSpec.describe API::Commits do
let(:non_member) { create(:user) } let(:non_member) { create(:user) }
it 'responds 403 when only members are allowed to read merge requests' do it 'responds 403 when only members are allowed to read merge requests' do
get api("/projects/#{project.id}/repository/commits/#{commit.id}/merge_requests", non_member) perform_request(non_member)
expect(response).to have_gitlab_http_status(:forbidden) expect(response).to have_gitlab_http_status(:forbidden)
end end
end end
it 'returns multiple merge requests without N + 1' do
perform_request(user)
control_count = ActiveRecord::QueryRecorder.new { perform_request(user) }.count
create(:merge_request, :closed, source_project: project, source_branch: 'master', target_branch: 'feature')
expect { perform_request(user) }.not_to exceed_query_limit(control_count)
end
end end
describe 'GET /projects/:id/repository/commits/:sha/signature' do describe 'GET /projects/:id/repository/commits/:sha/signature' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment