summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorYorick Peterse <yorickpeterse@gmail.com>2018-05-11 16:17:03 +0200
committerYorick Peterse <yorickpeterse@gmail.com>2018-05-17 13:53:00 +0200
commit878ca2e69b371e6c12acee6bddd32b4406c651d7 (patch)
tree7f829cdf6d9020cf106b32b8ae63d7d9e96a629e
parent19428e800895ba20eacb3357285acef8d69f6d8c (diff)
downloadgitlab-ce-878ca2e69b371e6c12acee6bddd32b4406c651d7.tar.gz
Exclude coverage data from the pipelines page
When displaying a project's pipelines (Projects::PipelinesController#index) we now exclude the coverage data. This data was not used by the frontend, yet getting it would require one SQL query per pipeline. These queries in turn could be quite expensive on GitLab.com.
-rw-r--r--app/controllers/projects/pipelines_controller.rb2
-rw-r--r--app/serializers/pipeline_entity.rb6
-rw-r--r--spec/controllers/projects/pipelines_controller_spec.rb6
-rw-r--r--spec/serializers/pipeline_entity_spec.rb7
4 files changed, 19 insertions, 2 deletions
diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb
index 91afe24b707..6b40fc2fe68 100644
--- a/app/controllers/projects/pipelines_controller.rb
+++ b/app/controllers/projects/pipelines_controller.rb
@@ -34,7 +34,7 @@ class Projects::PipelinesController < Projects::ApplicationController
pipelines: PipelineSerializer
.new(project: @project, current_user: @current_user)
.with_pagination(request, response)
- .represent(@pipelines),
+ .represent(@pipelines, disable_coverage: true),
count: {
all: @pipelines_count,
running: @running_count,
diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb
index 6457294b285..f782b411b84 100644
--- a/app/serializers/pipeline_entity.rb
+++ b/app/serializers/pipeline_entity.rb
@@ -4,7 +4,11 @@ class PipelineEntity < Grape::Entity
expose :id
expose :user, using: UserEntity
expose :active?, as: :active
- expose :coverage
+
+ # Coverage isn't always necessary (e.g. when displaying project pipelines in
+ # the UI). Instead of creating an entirely different entity we just allow the
+ # disabling of this specific field whenever necessary.
+ expose :coverage, unless: proc { options[:disable_coverage] }
expose :source
expose :created_at, :updated_at
diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb
index c22e455ea2d..9e7bc20a6d1 100644
--- a/spec/controllers/projects/pipelines_controller_spec.rb
+++ b/spec/controllers/projects/pipelines_controller_spec.rb
@@ -41,6 +41,12 @@ describe Projects::PipelinesController do
expect(json_response['count']['finished']).to eq '1'
end
+ it 'does not include coverage data for the pipelines' do
+ subject
+
+ expect(json_response['pipelines'][0]).not_to include('coverage')
+ end
+
context 'when performing gitaly calls', :request_store do
it 'limits the Gitaly requests' do
expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3)
diff --git a/spec/serializers/pipeline_entity_spec.rb b/spec/serializers/pipeline_entity_spec.rb
index 2473c561f4b..e67d12b7a89 100644
--- a/spec/serializers/pipeline_entity_spec.rb
+++ b/spec/serializers/pipeline_entity_spec.rb
@@ -26,6 +26,13 @@ describe PipelineEntity do
expect(subject).to include :updated_at, :created_at
end
+ it 'excludes coverage data when disabled' do
+ entity = described_class
+ .represent(pipeline, request: request, disable_coverage: true)
+
+ expect(entity.as_json).not_to include(:coverage)
+ end
+
it 'contains details' do
expect(subject).to include :details
expect(subject[:details])