Commit 0c8e707b authored by Sean McGivern's avatar Sean McGivern

Reduce queries needed for CI artifacts in MR widget

While the individual queries here are reasonable, these methods are only called
together: so we always call `sast_artifact`, `dependency_scanning_artifact`,
etc. in the same request, and always for the merge request widget.

That means that we can save a few queries by just getting the artifacts once,
and filtering in Ruby.
parent fdaeeec0
......@@ -20,23 +20,9 @@ module EE
DAST_FILE = 'gl-dast-report.json'.freeze
included do
scope :code_quality, -> { where(name: %w[codeclimate codequality code_quality]) }
scope :performance, -> { where(name: %w[performance deploy]) }
scope :sast, -> { where(name: 'sast') }
scope :dependency_scanning, -> { where(name: 'dependency_scanning') }
scope :license_management, -> { where(name: 'license_management') }
scope :sast_container, -> { where(name: %w[sast:container container_scanning]) }
scope :dast, -> { where(name: 'dast') }
after_save :stick_build_if_status_changed
end
class_methods do
def find_dast
dast.find(&:has_dast_json?)
end
end
def shared_runners_minutes_limit_enabled?
runner && runner.instance_type? && project.shared_runners_minutes_limit_enabled?
end
......@@ -50,39 +36,48 @@ module EE
# has_codeclimate_json? is deprecated and replaced with has_code_quality_json? (#5779)
def has_codeclimate_json?
name_in?(%w[codeclimate codequality code_quality]) &&
has_artifact?(CODECLIMATE_FILE)
end
def has_code_quality_json?
name_in?(%w[codeclimate codequality code_quality]) &&
has_artifact?(CODE_QUALITY_FILE)
end
def has_performance_json?
name_in?(%w[performance deploy]) &&
has_artifact?(PERFORMANCE_FILE)
end
def has_sast_json?
name_in?('sast') &&
has_artifact?(SAST_FILE)
end
def has_dependency_scanning_json?
name_in?('dependency_scanning') &&
has_artifact?(DEPENDENCY_SCANNING_FILE)
end
def has_license_management_json?
name_in?('license_management') &&
has_artifact?(LICENSE_MANAGEMENT_FILE)
end
# has_sast_container_json? is deprecated and replaced with has_container_scanning_json? (#5778)
def has_sast_container_json?
name_in?(%w[sast:container container_scanning]) &&
has_artifact?(SAST_CONTAINER_FILE)
end
def has_container_scanning_json?
name_in?(%w[sast:container container_scanning]) &&
has_artifact?(CONTAINER_SCANNING_FILE)
end
def has_dast_json?
name_in?('dast') &&
has_artifact?(DAST_FILE)
end
......@@ -92,6 +87,10 @@ module EE
options.dig(:artifacts, :paths)&.include?(name) &&
artifacts_metadata?
end
def name_in?(names)
name.in?(Array(names))
end
end
end
end
......@@ -18,40 +18,40 @@ module EE
# codeclimate_artifact is deprecated and replaced with code_quality_artifact (#5779)
def codeclimate_artifact
@codeclimate_artifact ||= artifacts.code_quality.find(&:has_codeclimate_json?)
@codeclimate_artifact ||= artifacts.find(&:has_codeclimate_json?)
end
def code_quality_artifact
@code_quality_artifact ||= artifacts.code_quality.find(&:has_code_quality_json?)
@code_quality_artifact ||= artifacts.find(&:has_code_quality_json?)
end
def performance_artifact
@performance_artifact ||= artifacts.performance.find(&:has_performance_json?)
@performance_artifact ||= artifacts.find(&:has_performance_json?)
end
def sast_artifact
@sast_artifact ||= artifacts.sast.find(&:has_sast_json?)
@sast_artifact ||= artifacts.find(&:has_sast_json?)
end
def dependency_scanning_artifact
@dependency_scanning_artifact ||= artifacts.dependency_scanning.find(&:has_dependency_scanning_json?)
@dependency_scanning_artifact ||= artifacts.find(&:has_dependency_scanning_json?)
end
def license_management_artifact
@license_management_artifact ||= artifacts.license_management.find(&:has_license_management_json?)
@license_management_artifact ||= artifacts.find(&:has_license_management_json?)
end
# sast_container_artifact is deprecated and replaced with container_scanning_artifact (#5778)
def sast_container_artifact
@sast_container_artifact ||= artifacts.sast_container.find(&:has_sast_container_json?)
@sast_container_artifact ||= artifacts.find(&:has_sast_container_json?)
end
def container_scanning_artifact
@container_scanning_artifact ||= artifacts.sast_container.find(&:has_container_scanning_json?)
@container_scanning_artifact ||= artifacts.find(&:has_container_scanning_json?)
end
def dast_artifact
@dast_artifact ||= artifacts.dast.find(&:has_dast_json?)
@dast_artifact ||= artifacts.find(&:has_dast_json?)
end
def initialize_yaml_processor
......
---
title: Reduce queries needed for CI artifacts on merge request widget
merge_request: 6978
author:
type: performance
......@@ -13,34 +13,6 @@ describe Ci::Build do
let(:job) { create(:ci_build, pipeline: pipeline) }
describe '.code_quality' do
subject { described_class.code_quality }
context 'when a job name is codeclimate' do
let!(:job) { create(:ci_build, pipeline: pipeline, name: 'codeclimate') }
it { is_expected.to include(job) }
end
context 'when a job name is codequality' do
let!(:job) { create(:ci_build, pipeline: pipeline, name: 'codequality') }
it { is_expected.to include(job) }
end
context 'when a job name is code_quality' do
let!(:job) { create(:ci_build, pipeline: pipeline, name: 'code_quality') }
it { is_expected.to include(job) }
end
context 'when a job name is irrelevant' do
let!(:job) { create(:ci_build, pipeline: pipeline, name: 'codechecker') }
it { is_expected.not_to include(job) }
end
end
describe '#shared_runners_minutes_limit_enabled?' do
subject { job.shared_runners_minutes_limit_enabled? }
......@@ -143,27 +115,59 @@ describe Ci::Build do
end
end
BUILD_ARTIFACTS_METHODS = {
build_artifacts_methods = {
# has_codeclimate_json? is deprecated and replaced with code_quality_artifact (#5779)
has_codeclimate_json?: Ci::Build::CODECLIMATE_FILE,
has_code_quality_json?: Ci::Build::CODE_QUALITY_FILE,
has_performance_json?: Ci::Build::PERFORMANCE_FILE,
has_sast_json?: Ci::Build::SAST_FILE,
has_dependency_scanning_json?: Ci::Build::DEPENDENCY_SCANNING_FILE,
has_license_management_json?: Ci::Build::LICENSE_MANAGEMENT_FILE,
has_codeclimate_json?: {
filename: Ci::Build::CODECLIMATE_FILE,
job_names: %w[codeclimate codequality code_quality]
},
has_code_quality_json?: {
filename: Ci::Build::CODE_QUALITY_FILE,
job_names: %w[codeclimate codequality code_quality]
},
has_performance_json?: {
filename: Ci::Build::PERFORMANCE_FILE,
job_names: %w[performance deploy]
},
has_sast_json?: {
filename: Ci::Build::SAST_FILE,
job_names: %w[sast]
},
has_dependency_scanning_json?: {
filename: Ci::Build::DEPENDENCY_SCANNING_FILE,
job_names: %w[dependency_scanning]
},
has_license_management_json?: {
filename: Ci::Build::LICENSE_MANAGEMENT_FILE,
job_names: %w[license_management]
},
# has_sast_container_json? is deprecated and replaced with has_container_scanning_json (#5778)
has_sast_container_json?: Ci::Build::SAST_CONTAINER_FILE,
has_container_scanning_json?: Ci::Build::CONTAINER_SCANNING_FILE,
has_dast_json?: Ci::Build::DAST_FILE
}.freeze
has_sast_container_json?: {
filename: Ci::Build::SAST_CONTAINER_FILE,
job_names: %w[sast:container container_scanning]
},
has_container_scanning_json?: {
filename: Ci::Build::CONTAINER_SCANNING_FILE,
job_names: %w[sast:container container_scanning]
},
has_dast_json?: {
filename: Ci::Build::DAST_FILE,
job_names: %w[dast]
}
}
build_artifacts_methods.each do |method, requirements|
filename = requirements[:filename]
job_names = requirements[:job_names]
BUILD_ARTIFACTS_METHODS.each do |method, filename|
describe "##{method}" do
context 'valid build' do
let!(:build) do
job_names.each do |job_name|
context "with a job named #{job_name} and a file named #{filename}" do
let(:build) do
create(
:ci_build,
:artifacts,
name: job_name,
pipeline: pipeline,
options: {
artifacts: {
......@@ -175,12 +179,14 @@ describe Ci::Build do
it { expect(build.send(method)).to be_truthy }
end
end
context 'invalid build' do
let!(:build) do
context 'with an invalid filename' do
let(:build) do
create(
:ci_build,
:artifacts,
name: job_names.first,
pipeline: pipeline,
options: {}
)
......@@ -188,6 +194,23 @@ describe Ci::Build do
it { expect(build.send(method)).to be_falsey }
end
context 'with an invalid job name' do
let(:build) do
create(
:ci_build,
:artifacts,
pipeline: pipeline,
options: {
artifacts: {
paths: [filename, 'some-other-artifact.txt']
}
}
)
end
it { expect(build.send(method)).to be_falsey }
end
end
end
end
......@@ -171,4 +171,24 @@ describe Ci::Pipeline do
expect(described_class.with_security_reports).to eq([pipeline_1, pipeline_2, pipeline_3, pipeline_4])
end
end
context 'performance' do
def create_build(job_name)
create(
:ci_build,
:artifacts,
name: job_name,
pipeline: pipeline
)
end
it 'does not perform extra queries when calling pipeline artifacts methods after the first' do
create_build('codeclimate')
create_build('dependency_scanning')
pipeline.code_quality_artifact
expect { pipeline.dependency_scanning_artifact }.not_to exceed_query_limit(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