Merge branch 'simplify-size-limiter-constructor' into 'master'

Simplify contructor for size limiter validator

See merge request gitlab-org/gitlab!72510

lib/gitlab/sidekiq_middleware/size_limiter/validator.rb

@@ -55,18 +55,15 @@ module Gitlab
 
         attr_reader :mode, :size_limit, :compression_threshold
 
-        def initialize(
-          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
-        )
+        def initialize(worker_class, job)
           @worker_class = worker_class
           @job = job
 
-          set_mode(mode)
-          set_compression_threshold(compression_threshold)
-          set_size_limit(size_limit)
+          current_settings = Gitlab::CurrentSettings.current_application_settings
+
+          @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
 
         def validate!
@@ -90,30 +87,6 @@ module Gitlab
 
         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)
           ExceedLimitError.new(@worker_class, job_args.bytesize, @size_limit).tap do |exception|
             # This should belong to Gitlab::ErrorTracking. We'll remove this

spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb

@@ -59,111 +59,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
         expect(validator.size_limit).to eq(2)
       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
 
   shared_examples 'validate limit job payload size' do
@@ -171,20 +66,6 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
       let(:compression_threshold) { nil }
       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
         let(:size_limit) { 0 }
         let(:job) { job_payload(a: 'a' * 300) }
@@ -438,36 +319,20 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator, :aggregate_fai
   end
 
   describe '#validate!' do
-    context 'when creating an instance with the related configuration variables' do
-      let(:validate) do
-        ->(worker_clas, job) do
-          described_class.new(worker_class, job).validate!
-        end
+    let(:validate) do
+      ->(worker_class, job) do
+        described_class.new(worker_class, job).validate!
       end
-
-      before do
-        stub_application_setting(
-          sidekiq_job_limiter_mode: mode,
-          sidekiq_job_limiter_compression_threshold_bytes: compression_threshold,
-          sidekiq_job_limiter_limit_bytes: size_limit
-        )
-      end
-
-      it_behaves_like 'validate limit job payload size'
     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'
+    before do
+      stub_application_setting(
+        sidekiq_job_limiter_mode: mode,
+        sidekiq_job_limiter_compression_threshold_bytes: compression_threshold,
+        sidekiq_job_limiter_limit_bytes: size_limit
+      )
     end
+
+    it_behaves_like 'validate limit job payload size'
   end
 end