Commit 1164707e authored by Stan Hu's avatar Stan Hu

Merge branch 'mo-remove-artifacts-from-pipeline-entity' into 'master'

Remove artifacts from pipeline details entity

See merge request gitlab-org/gitlab!61253
parents a2d53b3f a903d6c7
...@@ -223,7 +223,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -223,7 +223,7 @@ class Projects::PipelinesController < Projects::ApplicationController
PipelineSerializer PipelineSerializer
.new(project: @project, current_user: @current_user) .new(project: @project, current_user: @current_user)
.with_pagination(request, response) .with_pagination(request, response)
.represent(@pipelines, disable_coverage: true, preload: true, disable_artifacts: true) .represent(@pipelines, disable_coverage: true, preload: true)
end end
def render_show def render_show
......
...@@ -8,15 +8,6 @@ class PipelineDetailsEntity < Ci::PipelineEntity ...@@ -8,15 +8,6 @@ class PipelineDetailsEntity < Ci::PipelineEntity
end end
expose :details do expose :details do
expose :artifacts, unless: proc { options[:disable_artifacts] } do |pipeline, options|
rel = pipeline.downloadable_artifacts
if Feature.enabled?(:non_public_artifacts, type: :development)
rel = rel.select { |artifact| can?(request.current_user, :read_job_artifacts, artifact.job) }
end
BuildArtifactEntity.represent(rel, options.merge(project: pipeline.project))
end
expose :manual_actions, using: BuildActionEntity expose :manual_actions, using: BuildActionEntity
expose :scheduled_actions, using: BuildActionEntity expose :scheduled_actions, using: BuildActionEntity
end end
......
...@@ -49,10 +49,6 @@ class PipelineSerializer < BaseSerializer ...@@ -49,10 +49,6 @@ class PipelineSerializer < BaseSerializer
{ {
manual_actions: :metadata, manual_actions: :metadata,
scheduled_actions: :metadata, scheduled_actions: :metadata,
downloadable_artifacts: {
project: [:route, { namespace: :route }],
job: []
},
failed_builds: %i(project metadata), failed_builds: %i(project metadata),
merge_request: { merge_request: {
source_project: [:route, { namespace: :route }], source_project: [:route, { namespace: :route }],
......
---
title: Stop exposing artifacts in pipelines.json
merge_request: 61253
author:
type: performance
...@@ -42,6 +42,4 @@ RSpec.describe MergeRequests::PipelineEntity do ...@@ -42,6 +42,4 @@ RSpec.describe MergeRequests::PipelineEntity do
expect(entity.as_json).not_to include(:coverage) expect(entity.as_json).not_to include(:coverage)
end end
end end
it_behaves_like 'public artifacts'
end end
...@@ -32,7 +32,7 @@ RSpec.describe PipelineDetailsEntity do ...@@ -32,7 +32,7 @@ RSpec.describe PipelineDetailsEntity do
expect(subject[:details]) expect(subject[:details])
.to include :duration, :finished_at .to include :duration, :finished_at
expect(subject[:details]) expect(subject[:details])
.to include :stages, :artifacts, :manual_actions, :scheduled_actions .to include :stages, :manual_actions, :scheduled_actions
expect(subject[:details][:status]).to include :icon, :favicon, :text, :label expect(subject[:details][:status]).to include :icon, :favicon, :text, :label
end end
...@@ -184,7 +184,5 @@ RSpec.describe PipelineDetailsEntity do ...@@ -184,7 +184,5 @@ RSpec.describe PipelineDetailsEntity do
expect(source_jobs[child_pipeline.id][:name]).to eq('child') expect(source_jobs[child_pipeline.id][:name]).to eq('child')
end end
end end
it_behaves_like 'public artifacts'
end end
end end
...@@ -155,7 +155,7 @@ RSpec.describe PipelineSerializer do ...@@ -155,7 +155,7 @@ RSpec.describe PipelineSerializer do
it 'verifies number of queries', :request_store do it 'verifies number of queries', :request_store do
recorded = ActiveRecord::QueryRecorder.new { subject } recorded = ActiveRecord::QueryRecorder.new { subject }
expected_queries = Gitlab.ee? ? 39 : 36 expected_queries = Gitlab.ee? ? 33 : 30
expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
...@@ -176,7 +176,7 @@ RSpec.describe PipelineSerializer do ...@@ -176,7 +176,7 @@ RSpec.describe PipelineSerializer do
# pipeline. With the same ref this check is cached but if refs are # pipeline. With the same ref this check is cached but if refs are
# different then there is an extra query per ref # different then there is an extra query per ref
# https://gitlab.com/gitlab-org/gitlab-foss/issues/46368 # https://gitlab.com/gitlab-org/gitlab-foss/issues/46368
expected_queries = Gitlab.ee? ? 42 : 39 expected_queries = Gitlab.ee? ? 36 : 33
expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
...@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do ...@@ -202,7 +202,7 @@ RSpec.describe PipelineSerializer do
# Existing numbers are high and require performance optimization # Existing numbers are high and require performance optimization
# Ongoing issue: # Ongoing issue:
# https://gitlab.com/gitlab-org/gitlab/-/issues/225156 # https://gitlab.com/gitlab-org/gitlab/-/issues/225156
expected_queries = Gitlab.ee? ? 82 : 76 expected_queries = Gitlab.ee? ? 77 : 70
expect(recorded.count).to be_within(2).of(expected_queries) expect(recorded.count).to be_within(2).of(expected_queries)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
...@@ -221,8 +221,7 @@ RSpec.describe PipelineSerializer do ...@@ -221,8 +221,7 @@ RSpec.describe PipelineSerializer do
create(:ci_build, :scheduled, project: project, environment: env.name) create(:ci_build, :scheduled, project: project, environment: env.name)
recorded = ActiveRecord::QueryRecorder.new { subject } recorded = ActiveRecord::QueryRecorder.new { subject }
expected_queries = Gitlab.ee? ? 61 : 57 expected_queries = Gitlab.ee? ? 56 : 52
expect(recorded.count).to be_within(1).of(expected_queries) expect(recorded.count).to be_within(1).of(expected_queries)
expect(recorded.cached_count).to eq(0) expect(recorded.cached_count).to eq(0)
end end
......
# frozen_string_literal: true
RSpec.shared_examples 'public artifacts' do
let_it_be(:project) { create(:project, :public) }
let(:pipeline) { create(:ci_empty_pipeline, status: :success, project: project) }
context 'that has artifacts' do
let!(:build) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
it 'contains information about artifacts' do
expect(subject[:details][:artifacts].length).to eq(1)
end
end
context 'that has non public artifacts' do
let!(:build) { create(:ci_build, :success, :artifacts, :non_public_artifacts, pipeline: pipeline) }
it 'does not contain information about artifacts' do
expect(subject[:details][:artifacts].length).to eq(0)
end
end
end
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