Merge branch 'ci-add-runner-minutes-cost-factor-class' into 'master'

Extract Runner minutes cost class to remove duplication

See merge request gitlab-org/gitlab!61711

ee/app/models/ee/ci/runner.rb

@@ -28,17 +28,8 @@ module EE
         ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes { super }
       end
 
-      def minutes_cost_factor(access_level)
-        return 0.0 unless instance_type?
-
-        case access_level
-        when ::Gitlab::VisibilityLevel::PUBLIC
-          public_projects_minutes_cost_factor
-        when ::Gitlab::VisibilityLevel::PRIVATE, ::Gitlab::VisibilityLevel::INTERNAL
-          private_projects_minutes_cost_factor
-        else
-          raise ArgumentError, 'Invalid visibility level'
-        end
+      def minutes_cost_factor(visibility_level)
+        ::Gitlab::Ci::Minutes::CostFactor.new(runner_matcher).for_visibility(visibility_level)
       end
 
       def visibility_levels_without_minutes_quota

ee/lib/ee/gitlab/ci/matching/runner_matcher.rb

@@ -5,24 +5,21 @@ module EE
     module Ci
       module Matching
         module RunnerMatcher
-          def matches_quota?(build_matcher)
-            cost_factor = minutes_cost_factor(build_matcher.project.visibility_level)
+          include ::Gitlab::Utils::StrongMemoize
 
-            cost_factor == 0 || (cost_factor > 0 && !minutes_used_up?(build_matcher))
+          def matches_quota?(build_matcher)
+            cost_factor_disabled?(build_matcher) || !minutes_used_up?(build_matcher)
           end
 
           private
 
-          def minutes_cost_factor(visibility_level)
-            return 0.0 unless instance_type?
+          def cost_factor_disabled?(build_matcher)
+            cost_factor.disabled?(build_matcher.project.visibility_level)
+          end
 
-            case visibility_level
-            when ::Gitlab::VisibilityLevel::PUBLIC
-              public_projects_minutes_cost_factor
-            when ::Gitlab::VisibilityLevel::PRIVATE, ::Gitlab::VisibilityLevel::INTERNAL
-              private_projects_minutes_cost_factor
-            else
-              raise ArgumentError, 'Invalid visibility level'
+          def cost_factor
+            strong_memoize(:cost_factor) do
+              ::Gitlab::Ci::Minutes::CostFactor.new(self)
             end
           end
 

ee/lib/gitlab/ci/minutes/cost_factor.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Minutes
+      class CostFactor
+        def initialize(runner_matcher)
+          ensure_runner_matcher_instance(runner_matcher)
+
+          @runner_matcher = runner_matcher
+        end
+
+        def enabled?(visibility_level)
+          for_visibility(visibility_level) > 0
+        end
+
+        def disabled?(visibility_level)
+          !enabled?(visibility_level)
+        end
+
+        def for_visibility(visibility_level)
+          return 0.0 unless @runner_matcher.instance_type?
+
+          case visibility_level
+          when ::Gitlab::VisibilityLevel::PUBLIC
+            @runner_matcher.public_projects_minutes_cost_factor
+          when ::Gitlab::VisibilityLevel::PRIVATE, ::Gitlab::VisibilityLevel::INTERNAL
+            @runner_matcher.private_projects_minutes_cost_factor
+          else
+            raise ArgumentError, 'Invalid visibility level'
+          end
+        end
+
+        private
+
+        def ensure_runner_matcher_instance(runner_matcher)
+          unless runner_matcher.is_a?(Matching::RunnerMatcher)
+            raise ArgumentError, 'only Matching::RunnerMatcher objects allowed'
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/ci/minutes/build_consumption_spec.rb

@@ -9,7 +9,7 @@ RSpec.describe Gitlab::Ci::Minutes::BuildConsumption do
   let(:build) { build_stubbed(:ci_build, runner: runner, project: project) }
 
   let_it_be(:project) { create(:project) }
-  let_it_be(:runner) { create(:ci_runner, :instance) }
+  let_it_be_with_refind(:runner) { create(:ci_runner, :instance) }
 
   describe '#amount' do
     subject { consumption.amount }

ee/spec/lib/gitlab/ci/minutes/cost_factor_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Minutes::CostFactor do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:runner) do
+    build_stubbed(:ci_runner,
+      runner_type,
+      public_projects_minutes_cost_factor: public_cost_factor,
+      private_projects_minutes_cost_factor: private_cost_factor
+    )
+  end
+
+  describe '.new' do
+    let(:runner) { build_stubbed(:ci_runner) }
+
+    it 'raises errors when initialized with a runner object' do
+      expect { described_class.new(runner) }.to raise_error(ArgumentError)
+    end
+  end
+
+  describe '#enabled?' do
+    subject { described_class.new(runner.runner_matcher).enabled?(visibility_level) }
+
+    where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do
+      :project  | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | false
+      :project  | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
+      :project  | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | false
+      :group    | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | false
+      :group    | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
+      :group    | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | false
+      :instance | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | true
+      :instance | Gitlab::VisibilityLevel::PRIVATE  | 1 | 0 | false
+      :instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
+      :instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 0 | false
+      :instance | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | true
+      :instance | Gitlab::VisibilityLevel::PUBLIC   | 0 | 1 | false
+    end
+
+    with_them do
+      it { is_expected.to eq(result) }
+    end
+  end
+
+  describe '#disabled?' do
+    subject { described_class.new(runner.runner_matcher).disabled?(visibility_level) }
+
+    where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do
+      :project  | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | true
+      :project  | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
+      :project  | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | true
+      :group    | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | true
+      :group    | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | true
+      :group    | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | true
+      :instance | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | false
+      :instance | Gitlab::VisibilityLevel::PRIVATE  | 1 | 0 | true
+      :instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | false
+      :instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 0 | true
+      :instance | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | false
+      :instance | Gitlab::VisibilityLevel::PUBLIC   | 0 | 1 | true
+    end
+
+    with_them do
+      it { is_expected.to eq(result) }
+    end
+  end
+
+  describe '#for_visibility' do
+    subject { described_class.new(runner.runner_matcher).for_visibility(visibility_level) }
+
+    where(:runner_type, :visibility_level, :public_cost_factor, :private_cost_factor, :result) do
+      :project  | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | 0
+      :project  | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
+      :project  | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | 0
+      :group    | Gitlab::VisibilityLevel::PRIVATE  | 1 | 1 | 0
+      :group    | Gitlab::VisibilityLevel::INTERNAL | 1 | 1 | 0
+      :group    | Gitlab::VisibilityLevel::PUBLIC   | 1 | 1 | 0
+      :instance | Gitlab::VisibilityLevel::PUBLIC   | 1 | 5 | 1
+      :instance | Gitlab::VisibilityLevel::INTERNAL | 1 | 5 | 5
+      :instance | Gitlab::VisibilityLevel::PRIVATE  | 1 | 5 | 5
+    end
+
+    with_them do
+      it { is_expected.to eq(result) }
+    end
+
+    context 'with invalid visibility level' do
+      let(:visibility_level) { 123 }
+      let(:public_cost_factor) { 5 }
+      let(:private_cost_factor) { 5 }
+      let(:runner_type) { :instance }
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(ArgumentError)
+      end
+    end
+  end
+end

ee/spec/models/ee/ci/runner_spec.rb

@@ -82,7 +82,7 @@ RSpec.describe EE::Ci::Runner do
     end
   end
 
-  describe `#visibility_levels_without_minutes_quota` do
+  describe '#visibility_levels_without_minutes_quota' do
     subject { runner.visibility_levels_without_minutes_quota }
 
     context 'with group type runner' do

ee/spec/services/ci/register_job_service_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Ci::RegisterJobService do
-  let_it_be(:shared_runner) { create(:ci_runner, :instance) }
+  let_it_be_with_refind(:shared_runner) { create(:ci_runner, :instance) }
 
   let!(:project) { create :project, shared_runners_enabled: true }
   let!(:pipeline) { create :ci_empty_pipeline, project: project }