Merge branch '321827-fix-n+1-in-runner-validations' into 'master'

Fix N+1 when validating groups

See merge request gitlab-org/gitlab!54774

app/models/ci/runner_namespace.rb

@@ -4,10 +4,17 @@ module Ci
   class RunnerNamespace < ApplicationRecord
     extend Gitlab::Ci::Model
 
-    belongs_to :runner, inverse_of: :runner_namespaces, validate: true
+    belongs_to :runner, inverse_of: :runner_namespaces
     belongs_to :namespace, inverse_of: :runner_namespaces, class_name: '::Namespace'
     belongs_to :group, class_name: '::Group', foreign_key: :namespace_id
 
     validates :runner_id, uniqueness: { scope: :namespace_id }
+    validate :group_runner_type
+
+    private
+
+    def group_runner_type
+      errors.add(:runner, 'is not a group runner') unless runner&.group_type?
+    end
   end
 end

app/models/namespace.rb

@@ -67,8 +67,6 @@ class Namespace < ApplicationRecord
   validate :changing_shared_runners_enabled_is_allowed
   validate :changing_allow_descendants_override_disabled_shared_runners_is_allowed
 
-  validates_associated :runners
-
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :avatar_url, to: :owner, allow_nil: true
 

changelogs/unreleased/321827-fix-n-1-in-runner-validations.yml

+---
+title: Improve performance of validations when a group has a lot of runners
+merge_request: 54774
+author:
+type: performance

spec/models/ci/runner_spec.rb

@@ -40,41 +40,39 @@ RSpec.describe Ci::Runner do
     context 'runner_type validations' do
       let_it_be(:group) { create(:group) }
       let_it_be(:project) { create(:project) }
-      let(:group_runner) { create(:ci_runner, :group, groups: [group]) }
-      let(:project_runner) { create(:ci_runner, :project, projects: [project]) }
-      let(:instance_runner) { create(:ci_runner, :instance) }
 
       it 'disallows assigning group to project_type runner' do
-        project_runner.groups << build(:group)
+        project_runner = build(:ci_runner, :project, groups: [group])
 
         expect(project_runner).not_to be_valid
         expect(project_runner.errors.full_messages).to include('Runner cannot have groups assigned')
       end
 
       it 'disallows assigning group to instance_type runner' do
-        instance_runner.groups << build(:group)
+        instance_runner = build(:ci_runner, :instance, groups: [group])
 
         expect(instance_runner).not_to be_valid
         expect(instance_runner.errors.full_messages).to include('Runner cannot have groups assigned')
       end
 
       it 'disallows assigning project to group_type runner' do
-        group_runner.projects << build(:project)
+        group_runner = build(:ci_runner, :instance, projects: [project])
 
         expect(group_runner).not_to be_valid
         expect(group_runner.errors.full_messages).to include('Runner cannot have projects assigned')
       end
 
       it 'disallows assigning project to instance_type runner' do
-        instance_runner.projects << build(:project)
+        instance_runner = build(:ci_runner, :instance, projects: [project])
 
         expect(instance_runner).not_to be_valid
         expect(instance_runner.errors.full_messages).to include('Runner cannot have projects assigned')
       end
 
       it 'fails to save a group assigned to a project runner even if the runner is already saved' do
-        group.runners << project_runner
-        expect { group.save! }
+        project_runner = create(:ci_runner, :project, projects: [project])
+
+        expect { create(:group, runners: [project_runner]) }
           .to raise_error(ActiveRecord::RecordInvalid)
       end
     end