From d6167a9214b3a3c13850cdac9895c9d7577ddf25 Mon Sep 17 00:00:00 2001
From: Alexis Reigel <alexis.reigel.ext@siemens.com>
Date: Wed, 4 Oct 2017 09:27:27 +0200
Subject: [PATCH] split up Ci::Runner.owned_or_shared scope

---
 app/models/ci/runner.rb       | 32 +++++++++++++-------------
 spec/models/ci/runner_spec.rb | 42 ++++++++++++++++++++---------------
 2 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 2f4342b79aa..b7af33c0480 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -21,17 +21,17 @@ module Ci
 
     before_validation :set_default_values
 
-    scope :specific, ->() { where(is_shared: false) }
-    scope :shared, ->() { where(is_shared: true) }
-    scope :active, ->() { where(active: true) }
-    scope :paused, ->() { where(active: false) }
-    scope :online, ->() { where('contacted_at > ?', contact_time_deadline) }
-    scope :ordered, ->() { order(id: :desc) }
-
-    scope :owned_or_shared, ->(project_id) do
-      project_runners = joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
-
-      group_runners = joins(
+    scope :specific, -> { where(is_shared: false) }
+    scope :shared, -> { where(is_shared: true) }
+    scope :active, -> { where(active: true) }
+    scope :paused, -> { where(active: false) }
+    scope :online, -> { where('contacted_at > ?', contact_time_deadline) }
+    scope :ordered, -> { order(id: :desc) }
+    scope :belonging_to_project, -> (project_id) {
+      joins(:runner_projects).where(ci_runner_projects: { project_id: project_id })
+    }
+    scope :belonging_to_group, -> (project_id) {
+      joins(
         %{
           INNER JOIN ci_runner_groups ON ci_runner_groups.runner_id = ci_runners.id
           INNER JOIN namespaces ON namespaces.id = ci_runner_groups.group_id
@@ -43,13 +43,13 @@ module Ci
             AND
           projects.group_runners_enabled = :true
         },
-        project_id: project_id,
-        true: true
+          project_id: project_id,
+          true: true
       )
+    }
 
-      shared_runners = where(is_shared: true)
-
-      union = Gitlab::SQL::Union.new([project_runners, group_runners, shared_runners])
+    scope :owned_or_shared, -> (project_id) do
+      union = Gitlab::SQL::Union.new([belonging_to_project(project_id), belonging_to_group(project_id), shared])
       from("(#{union.to_sql}) ci_runners")
     end
 
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 933bd6e5f23..a073af7c4b2 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -49,7 +49,23 @@ describe Ci::Runner do
     end
   end
 
-  describe '.owned_or_shared' do
+  describe '.shared' do
+    it 'returns the shared group runner' do
+      group = create :group
+      runner = create :ci_runner, :shared, groups: [group]
+
+      expect(described_class.shared).to eq [runner]
+    end
+
+    it 'returns the shared project runner' do
+      project = create :project
+      runner = create :ci_runner, :shared, projects: [project]
+
+      expect(described_class.shared).to eq [runner]
+    end
+  end
+
+  describe '.belonging_to_project' do
     it 'returns the specific project runner' do
       # own
       specific_project = create :project
@@ -59,16 +75,11 @@ describe Ci::Runner do
       other_project = create :project
       create :ci_runner, :specific, projects: [other_project]
 
-      expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner]
-    end
-
-    it 'returns the shared project runner' do
-      project = create :project
-      runner = create :ci_runner, :shared, projects: [project]
-
-      expect(described_class.owned_or_shared(0)).to eq [runner]
+      expect(described_class.belonging_to_project(specific_project.id)).to eq [specific_runner]
     end
+  end
 
+  describe '.belonging_to_group' do
     it 'returns the specific group runner' do
       # own
       specific_group = create :group
@@ -80,7 +91,7 @@ describe Ci::Runner do
       create :project, group: other_group
       create :ci_runner, :specific, groups: [other_group]
 
-      expect(described_class.owned_or_shared(specific_project.id)).to eq [specific_runner]
+      expect(described_class.belonging_to_group(specific_project.id)).to eq [specific_runner]
     end
 
     it 'does not return the group runner if the project has group runners disabled' do
@@ -88,16 +99,11 @@ describe Ci::Runner do
       specific_project = create :project, group: specific_group, group_runners_enabled: false
       create :ci_runner, :specific, groups: [specific_group]
 
-      expect(described_class.owned_or_shared(specific_project.id)).to be_empty
-    end
-
-    it 'returns the shared group runner' do
-      group = create :group
-      runner = create :ci_runner, :shared, groups: [group]
-
-      expect(described_class.owned_or_shared(0)).to eq [runner]
+      expect(described_class.belonging_to_group(specific_project.id)).to be_empty
     end
+  end
 
+  describe '.owned_or_shared' do
     it 'returns a globally shared, a project specific and a group specific runner' do
       # group specific
       group = create :group