Commit 16b62948 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Simplify contructor for size limiter validator

These are unused now that we read from application settings all the
time. The application settings are also validated already so we remove
the validations in this class.
parent 5a6e90e7
...@@ -49,18 +49,15 @@ module Gitlab ...@@ -49,18 +49,15 @@ module Gitlab
attr_reader :mode, :size_limit, :compression_threshold attr_reader :mode, :size_limit, :compression_threshold
def initialize( def initialize(worker_class, job)
worker_class, job,
mode: Gitlab::CurrentSettings.sidekiq_job_limiter_mode,
compression_threshold: Gitlab::CurrentSettings.sidekiq_job_limiter_compression_threshold_bytes,
size_limit: Gitlab::CurrentSettings.sidekiq_job_limiter_limit_bytes
)
@worker_class = worker_class @worker_class = worker_class
@job = job @job = job
set_mode(mode) current_settings = Gitlab::CurrentSettings.current_application_settings
set_compression_threshold(compression_threshold)
set_size_limit(size_limit) @mode = current_settings.sidekiq_job_limiter_mode
@compression_threshold = current_settings.sidekiq_job_limiter_compression_threshold_bytes
@size_limit = current_settings.sidekiq_job_limiter_limit_bytes
end end
def validate! def validate!
...@@ -80,30 +77,6 @@ module Gitlab ...@@ -80,30 +77,6 @@ module Gitlab
private private
def set_mode(mode)
@mode = (mode || TRACK_MODE).to_s.strip
unless MODES.include?(@mode)
::Sidekiq.logger.warn "Invalid Sidekiq size limiter mode: #{@mode}. Fallback to #{TRACK_MODE} mode."
@mode = TRACK_MODE
end
end
def set_compression_threshold(compression_threshold)
@compression_threshold = (compression_threshold || DEFAULT_COMPRESSION_THRESHOLD_BYTES).to_i
if @compression_threshold <= 0
::Sidekiq.logger.warn "Invalid Sidekiq size limiter compression threshold: #{@compression_threshold}"
@compression_threshold = DEFAULT_COMPRESSION_THRESHOLD_BYTES
end
end
def set_size_limit(size_limit)
@size_limit = (size_limit || DEFAULT_SIZE_LIMIT).to_i
if @size_limit < 0
::Sidekiq.logger.warn "Invalid Sidekiq size limiter limit: #{@size_limit}"
@size_limit = DEFAULT_SIZE_LIMIT
end
end
def exceed_limit_error(job_args) def exceed_limit_error(job_args)
ExceedLimitError.new(@worker_class, job_args.bytesize, @size_limit).tap do |exception| ExceedLimitError.new(@worker_class, job_args.bytesize, @size_limit).tap do |exception|
# This should belong to Gitlab::ErrorTracking. We'll remove this # This should belong to Gitlab::ErrorTracking. We'll remove this
......
...@@ -59,111 +59,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai ...@@ -59,111 +59,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
expect(validator.size_limit).to eq(2) expect(validator.size_limit).to eq(2)
end end
end end
context 'when the input mode is valid' do
it 'does not log a warning message' do
expect(::Sidekiq.logger).not_to receive(:warn)
described_class.new(TestSizeLimiterWorker, job_payload, mode: 'track')
described_class.new(TestSizeLimiterWorker, job_payload, mode: 'compress')
end
end
context 'when the input mode is invalid' do
it 'defaults to track mode and logs a warning message' do
expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter mode: invalid. Fallback to track mode.')
validator = described_class.new(TestSizeLimiterWorker, job_payload, mode: 'invalid')
expect(validator.mode).to eql('track')
end
end
context 'when the input mode is empty' do
it 'defaults to track mode' do
expect(::Sidekiq.logger).not_to receive(:warn)
validator = described_class.new(TestSizeLimiterWorker, job_payload, mode: nil)
expect(validator.mode).to eql('track')
end
end
context 'when the size input is valid' do
it 'does not log a warning message' do
expect(::Sidekiq.logger).not_to receive(:warn)
described_class.new(TestSizeLimiterWorker, job_payload, size_limit: 300)
described_class.new(TestSizeLimiterWorker, job_payload, size_limit: 0)
end
end
context 'when the size input is invalid' do
it 'logs a warning message' do
expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter limit: -1')
validator = described_class.new(TestSizeLimiterWorker, job_payload, size_limit: -1)
expect(validator.size_limit).to be(0)
end
end
context 'when the size input is empty' do
it 'defaults to 0' do
expect(::Sidekiq.logger).not_to receive(:warn)
validator = described_class.new(TestSizeLimiterWorker, job_payload, size_limit: nil)
expect(validator.size_limit).to be(described_class::DEFAULT_SIZE_LIMIT)
end
end
context 'when the compression threshold is valid' do
it 'does not log a warning message' do
expect(::Sidekiq.logger).not_to receive(:warn)
described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 300)
described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 1)
end
end
context 'when the compression threshold is negative' do
it 'logs a warning message' do
expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter compression threshold: -1')
described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: -1)
end
it 'falls back to the default' do
validator = described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: -1)
expect(validator.compression_threshold).to be(100_000)
end
end
context 'when the compression threshold is zero' do
it 'logs a warning message' do
expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter compression threshold: 0')
described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 0)
end
it 'falls back to the default' do
validator = described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 0)
expect(validator.compression_threshold).to be(100_000)
end
end
context 'when the compression threshold is empty' do
it 'defaults to 100_000' do
expect(::Sidekiq.logger).not_to receive(:warn)
validator = described_class.new(TestSizeLimiterWorker, job_payload)
expect(validator.compression_threshold).to be(100_000)
end
end
end end
shared_examples 'validate limit job payload size' do shared_examples 'validate limit job payload size' do
...@@ -171,20 +66,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai ...@@ -171,20 +66,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
let(:compression_threshold) { nil } let(:compression_threshold) { nil }
let(:mode) { 'track' } let(:mode) { 'track' }
context 'when size limit negative' do
let(:size_limit) { -1 }
it 'does not track jobs' do
expect(Gitlab::ErrorTracking).not_to receive(:track_exception)
validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300))
end
it 'does not raise exception' do
expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) }.not_to raise_error
end
end
context 'when size limit is 0' do context 'when size limit is 0' do
let(:size_limit) { 0 } let(:size_limit) { 0 }
...@@ -366,9 +247,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai ...@@ -366,9 +247,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
end end
describe '#validate!' do describe '#validate!' do
context 'when creating an instance with the related configuration variables' do
let(:validate) do let(:validate) do
->(worker_clas, job) do ->(worker_class, job) do
described_class.new(worker_class, job).validate! described_class.new(worker_class, job).validate!
end end
end end
...@@ -383,19 +263,4 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai ...@@ -383,19 +263,4 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
it_behaves_like 'validate limit job payload size' it_behaves_like 'validate limit job payload size'
end end
context 'when creating an instance with mode and size limit' do
let(:validate) do
->(worker_clas, job) do
validator = described_class.new(
worker_class, job,
mode: mode, size_limit: size_limit, compression_threshold: compression_threshold
)
validator.validate!
end
end
it_behaves_like 'validate limit job payload size'
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