Commit 9123aa07 authored by Imre Farkas's avatar Imre Farkas

Merge branch '216452-remove-the-fallback-argument-into-a-constant' into 'master'

Refactor usage data fallback argument

See merge request gitlab-org/gitlab!30983
parents 8939a6b5 a1ce6520
...@@ -19,8 +19,7 @@ module EE ...@@ -19,8 +19,7 @@ module EE
name: :license_management_jobs name: :license_management_jobs
}, },
license_scanning: { license_scanning: {
name: :license_scanning_jobs, name: :license_scanning_jobs
fallback: 0
}, },
sast: { sast: {
name: :sast_jobs name: :sast_jobs
...@@ -111,12 +110,12 @@ module EE ...@@ -111,12 +110,12 @@ module EE
def security_products_usage def security_products_usage
results = SECURE_PRODUCT_TYPES.each_with_object({}) do |(secure_type, attribs), response| results = SECURE_PRODUCT_TYPES.each_with_object({}) do |(secure_type, attribs), response|
response[attribs[:name]] = count(::Ci::Build.where(name: secure_type), fallback: attribs.fetch(:fallback, -1)) # rubocop:disable CodeReuse/ActiveRecord response[attribs[:name]] = count(::Ci::Build.where(name: secure_type)) # rubocop:disable CodeReuse/ActiveRecord
end end
# handle license rename https://gitlab.com/gitlab-org/gitlab/issues/8911 # handle license rename https://gitlab.com/gitlab-org/gitlab/issues/8911
license_scan_count = results.delete(:license_scanning_jobs) license_scan_count = results.delete(:license_scanning_jobs)
results[:license_management_jobs] += license_scan_count results[:license_management_jobs] += license_scan_count > 0 ? license_scan_count : 0
results results
end end
...@@ -345,13 +344,13 @@ module EE ...@@ -345,13 +344,13 @@ module EE
} }
SECURE_PRODUCT_TYPES.each do |secure_type, attribs| SECURE_PRODUCT_TYPES.each do |secure_type, attribs|
results["#{prefix}#{attribs[:name]}".to_sym] = distinct_count(::Ci::Build.where(name: secure_type).where(time_period), :user_id, fallback: attribs.fetch(:fallback, -1)) results["#{prefix}#{attribs[:name]}".to_sym] = distinct_count(::Ci::Build.where(name: secure_type).where(time_period), :user_id)
end end
# handle license rename https://gitlab.com/gitlab-org/gitlab/issues/8911 # handle license rename https://gitlab.com/gitlab-org/gitlab/issues/8911
combined_license_key = "#{prefix}license_management_jobs".to_sym combined_license_key = "#{prefix}license_management_jobs".to_sym
license_scan_count = results.delete("#{prefix}license_scanning_jobs".to_sym) license_scan_count = results.delete("#{prefix}license_scanning_jobs".to_sym)
results[combined_license_key] += license_scan_count results[combined_license_key] += license_scan_count > 0 ? license_scan_count : 0
results results
end end
......
...@@ -19,6 +19,7 @@ ...@@ -19,6 +19,7 @@
module Gitlab module Gitlab
class UsageData class UsageData
BATCH_SIZE = 100 BATCH_SIZE = 100
FALLBACK = -1
class << self class << self
def data(force_refresh: false) def data(force_refresh: false)
...@@ -342,7 +343,7 @@ module Gitlab ...@@ -342,7 +343,7 @@ module Gitlab
results results
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid
{ projects_jira_server_active: -1, projects_jira_cloud_active: -1, projects_jira_active: -1 } { projects_jira_server_active: FALLBACK, projects_jira_cloud_active: FALLBACK, projects_jira_active: FALLBACK }
end end
def successful_deployments_with_cluster(scope) def successful_deployments_with_cluster(scope)
...@@ -367,27 +368,27 @@ module Gitlab ...@@ -367,27 +368,27 @@ module Gitlab
{} # augmented in EE {} # augmented in EE
end end
def count(relation, column = nil, fallback: -1, batch: true, start: nil, finish: nil) def count(relation, column = nil, batch: true, start: nil, finish: nil)
if batch && Feature.enabled?(:usage_ping_batch_counter, default_enabled: true) if batch && Feature.enabled?(:usage_ping_batch_counter, default_enabled: true)
Gitlab::Database::BatchCount.batch_count(relation, column, start: start, finish: finish) Gitlab::Database::BatchCount.batch_count(relation, column, start: start, finish: finish)
else else
relation.count relation.count
end end
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid
fallback FALLBACK
end end
def distinct_count(relation, column = nil, fallback: -1, batch: true, start: nil, finish: nil) def distinct_count(relation, column = nil, batch: true, start: nil, finish: nil)
if batch && Feature.enabled?(:usage_ping_batch_counter, default_enabled: true) if batch && Feature.enabled?(:usage_ping_batch_counter, default_enabled: true)
Gitlab::Database::BatchCount.batch_distinct_count(relation, column, start: start, finish: finish) Gitlab::Database::BatchCount.batch_distinct_count(relation, column, start: start, finish: finish)
else else
relation.distinct_count_by(column) relation.distinct_count_by(column)
end end
rescue ActiveRecord::StatementInvalid rescue ActiveRecord::StatementInvalid
fallback FALLBACK
end end
def alt_usage_data(value = nil, fallback: -1, &block) def alt_usage_data(value = nil, fallback: FALLBACK, &block)
if block_given? if block_given?
yield yield
else else
...@@ -410,7 +411,7 @@ module Gitlab ...@@ -410,7 +411,7 @@ module Gitlab
def redis_usage_counter def redis_usage_counter
yield yield
rescue ::Redis::CommandError, Gitlab::UsageDataCounters::BaseCounter::UnknownEvent rescue ::Redis::CommandError, Gitlab::UsageDataCounters::BaseCounter::UnknownEvent
-1 FALLBACK
end end
def redis_usage_data_totals(counter) def redis_usage_data_totals(counter)
......
...@@ -531,9 +531,10 @@ describe Gitlab::UsageData, :aggregate_failures do ...@@ -531,9 +531,10 @@ describe Gitlab::UsageData, :aggregate_failures do
end end
it 'returns the fallback value when counting fails' do it 'returns the fallback value when counting fails' do
stub_const("Gitlab::UsageData::FALLBACK", 15)
allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new('')) allow(relation).to receive(:count).and_raise(ActiveRecord::StatementInvalid.new(''))
expect(described_class.count(relation, fallback: 15, batch: false)).to eq(15) expect(described_class.count(relation, batch: false)).to eq(15)
end end
end end
...@@ -547,9 +548,10 @@ describe Gitlab::UsageData, :aggregate_failures do ...@@ -547,9 +548,10 @@ describe Gitlab::UsageData, :aggregate_failures do
end end
it 'returns the fallback value when counting fails' do it 'returns the fallback value when counting fails' do
stub_const("Gitlab::UsageData::FALLBACK", 15)
allow(relation).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid.new('')) allow(relation).to receive(:distinct_count_by).and_raise(ActiveRecord::StatementInvalid.new(''))
expect(described_class.distinct_count(relation, fallback: 15, batch: false)).to eq(15) expect(described_class.distinct_count(relation, batch: false)).to eq(15)
end end
end 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