Commit 191b5153 authored by Stan Hu's avatar Stan Hu

Merge branch 'builds-api-nplusone' into 'master'

Resolve "N+1 queries with /projects/:project_id/builds API endpoint"

Closes #41957

See merge request gitlab-org/gitlab-ce!16445
parents 79b05cdd feb34497
...@@ -38,6 +38,7 @@ module API ...@@ -38,6 +38,7 @@ module API
builds = user_project.builds.order('id DESC') builds = user_project.builds.order('id DESC')
builds = filter_builds(builds, params[:scope]) builds = filter_builds(builds, params[:scope])
builds = builds.preload(:user, :job_artifacts_archive, :runner, pipeline: :project)
present paginate(builds), with: Entities::Job present paginate(builds), with: Entities::Job
end end
......
...@@ -36,6 +36,7 @@ module API ...@@ -36,6 +36,7 @@ module API
builds = user_project.builds.order('id DESC') builds = user_project.builds.order('id DESC')
builds = filter_builds(builds, params[:scope]) builds = filter_builds(builds, params[:scope])
builds = builds.preload(:user, :job_artifacts_archive, :runner, pipeline: :project)
present paginate(builds), with: ::API::V3::Entities::Build present paginate(builds), with: ::API::V3::Entities::Build
end end
......
...@@ -25,9 +25,11 @@ describe API::Jobs do ...@@ -25,9 +25,11 @@ describe API::Jobs do
describe 'GET /projects/:id/jobs' do describe 'GET /projects/:id/jobs' do
let(:query) { Hash.new } let(:query) { Hash.new }
before do before do |example|
unless example.metadata[:skip_before_request]
get api("/projects/#{project.id}/jobs", api_user), query get api("/projects/#{project.id}/jobs", api_user), query
end end
end
context 'authorized user' do context 'authorized user' do
it 'returns project jobs' do it 'returns project jobs' do
...@@ -51,6 +53,23 @@ describe API::Jobs do ...@@ -51,6 +53,23 @@ describe API::Jobs do
expect(json_job['pipeline']['status']).to eq job.pipeline.status expect(json_job['pipeline']['status']).to eq job.pipeline.status
end end
it 'avoids N+1 queries', skip_before_request: true do
first_build = create(:ci_build, :artifacts, pipeline: pipeline)
first_build.runner = create(:ci_runner)
first_build.user = create(:user)
first_build.save
control_count = ActiveRecord::QueryRecorder.new { go }.count
second_pipeline = create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch)
second_build = create(:ci_build, :artifacts, pipeline: second_pipeline)
second_build.runner = create(:ci_runner)
second_build.user = create(:user)
second_build.save
expect { go }.not_to exceed_query_limit(control_count)
end
context 'filter project with one scope element' do context 'filter project with one scope element' do
let(:query) { { 'scope' => 'pending' } } let(:query) { { 'scope' => 'pending' } }
...@@ -83,6 +102,10 @@ describe API::Jobs do ...@@ -83,6 +102,10 @@ describe API::Jobs do
expect(response).to have_gitlab_http_status(401) expect(response).to have_gitlab_http_status(401)
end end
end end
def go
get api("/projects/#{project.id}/jobs", api_user), query
end
end end
describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do describe 'GET /projects/:id/pipelines/:pipeline_id/jobs' do
......
...@@ -13,11 +13,13 @@ describe API::V3::Builds do ...@@ -13,11 +13,13 @@ describe API::V3::Builds do
describe 'GET /projects/:id/builds ' do describe 'GET /projects/:id/builds ' do
let(:query) { '' } let(:query) { '' }
before do before do |example|
create(:ci_build, :skipped, pipeline: pipeline) create(:ci_build, :skipped, pipeline: pipeline)
unless example.metadata[:skip_before_request]
get v3_api("/projects/#{project.id}/builds?#{query}", api_user) get v3_api("/projects/#{project.id}/builds?#{query}", api_user)
end end
end
context 'authorized user' do context 'authorized user' do
it 'returns project builds' do it 'returns project builds' do
...@@ -40,6 +42,23 @@ describe API::V3::Builds do ...@@ -40,6 +42,23 @@ describe API::V3::Builds do
expect(json_build['pipeline']['status']).to eq build.pipeline.status expect(json_build['pipeline']['status']).to eq build.pipeline.status
end end
it 'avoids N+1 queries', skip_before_request: true do
first_build = create(:ci_build, :artifacts, pipeline: pipeline)
first_build.runner = create(:ci_runner)
first_build.user = create(:user)
first_build.save
control_count = ActiveRecord::QueryRecorder.new { go }.count
second_pipeline = create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch)
second_build = create(:ci_build, :artifacts, pipeline: second_pipeline)
second_build.runner = create(:ci_runner)
second_build.user = create(:user)
second_build.save
expect { go }.not_to exceed_query_limit(control_count)
end
context 'filter project with one scope element' do context 'filter project with one scope element' do
let(:query) { 'scope=pending' } let(:query) { 'scope=pending' }
...@@ -84,6 +103,10 @@ describe API::V3::Builds do ...@@ -84,6 +103,10 @@ describe API::V3::Builds do
expect(response).to have_gitlab_http_status(401) expect(response).to have_gitlab_http_status(401)
end end
end end
def go
get v3_api("/projects/#{project.id}/builds?#{query}", api_user)
end
end end
describe 'GET /projects/:id/repository/commits/:sha/builds' do describe 'GET /projects/:id/repository/commits/:sha/builds' 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