Revert "Merge branch 'optimise-pipelines' into 'master' "

This reverts commit 5fbb9e95449fd521447aa802269962c5a972467f, reversing
changes made to 197dad2eedc3722124b570e650432cf58650d07e.

15 files changed, 76 insertions, 101 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index b426c27afbb..70470414bc4 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -103,13 +103,18 @@ module Ci
     end
 
     def playable?
-      action? && manual?
+      project.builds_enabled? && has_commands? &&
+        action? && manual?
     end
 
     def action?
       self.when == 'manual'
     end
 
+    def has_commands?
+      commands.present?
+    end
+
     def play(current_user)
       # Try to queue a current build
       if self.enqueue
@@ -126,7 +131,8 @@ module Ci
     end
 
     def retryable?
-      success? || failed? || canceled?
+      project.builds_enabled? && has_commands? &&
+        (success? || failed? || canceled?)
     end
 
     def retried?
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 445247f1b41..7e8246ba6d6 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -17,12 +17,6 @@ module Ci
     has_many :builds, foreign_key: :commit_id
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id
 
-    has_many :pending_builds, -> { pending }, foreign_key: :commit_id, class_name: 'Ci::Build'
-    has_many :retryable_builds, -> { latest.failed_or_canceled }, foreign_key: :commit_id, class_name: 'Ci::Build'
-    has_many :cancelable_statuses, -> { cancelable }, foreign_key: :commit_id, class_name: 'CommitStatus'
-    has_many :manual_actions, -> { latest.manual_actions }, foreign_key: :commit_id, class_name: 'Ci::Build'
-    has_many :artifacts, -> { latest.with_artifacts_not_expired }, foreign_key: :commit_id, class_name: 'Ci::Build'
-
     delegate :id, to: :project, prefix: true
 
     validates :sha, presence: { unless: :importing? }
@@ -176,6 +170,10 @@ module Ci
       end
     end
 
+    def artifacts
+      builds.latest.with_artifacts_not_expired.includes(project: [:namespace])
+    end
+
     def valid_commit_sha
       if self.sha == Gitlab::Git::BLANK_SHA
         self.errors.add(:sha, " cant be 00000000 (branch removal)")
@@ -212,16 +210,20 @@ module Ci
       !tag?
     end
 
+    def manual_actions
+      builds.latest.manual_actions.includes(project: [:namespace])
+    end
+
     def stuck?
-      pending_builds.any?(&:stuck?)
+      builds.pending.includes(:project).any?(&:stuck?)
     end
 
     def retryable?
-      retryable_builds.any?
+      builds.latest.failed_or_canceled.any?(&:retryable?)
     end
 
     def cancelable?
-      cancelable_statuses.any?
+      statuses.cancelable.any?
     end
 
     def auto_canceled?
@@ -229,8 +231,8 @@ module Ci
     end
 
     def cancel_running
-      Gitlab::OptimisticLocking.retry_lock(cancelable_statuses) do |cancelable|
-        cancelable.find_each do |job|
+      Gitlab::OptimisticLocking.retry_lock(
+        statuses.cancelable) do |job|
           yield(job) if block_given?
           job.cancel
         end
diff --git a/app/models/project.rb b/app/models/project.rb
index a160efba912..e25db4c22d4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -173,8 +173,6 @@ class Project < ActiveRecord::Base
   has_many :environments, dependent: :destroy
   has_many :deployments, dependent: :destroy
 
-  has_many :active_runners, -> { active }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
-
   accepts_nested_attributes_for :variables, allow_destroy: true
   accepts_nested_attributes_for :project_feature
 
@@ -1074,15 +1072,15 @@ class Project < ActiveRecord::Base
   end
 
   def shared_runners
-    @shared_runners ||= shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none
-  end
-
-  def active_shared_runners
-    @active_shared_runners ||= shared_runners.active
+    shared_runners_available? ? Ci::Runner.shared : Ci::Runner.none
   end
 
   def any_runners?(&block)
-    active_runners.any?(&block) || active_shared_runners.any?(&block)
+    if runners.active.any?(&block)
+      return true
+    end
+
+    shared_runners.active.any?(&block)
   end
 
   def valid_runners_token?(token)
diff --git a/app/serializers/pipeline_entity.rb b/app/serializers/pipeline_entity.rb
index ad8b4d43e8f..3f16dd66d54 100644
--- a/app/serializers/pipeline_entity.rb
+++ b/app/serializers/pipeline_entity.rb
@@ -69,13 +69,13 @@ class PipelineEntity < Grape::Entity
   alias_method :pipeline, :object
 
   def can_retry?
-    can?(request.user, :update_pipeline, pipeline) &&
-      pipeline.retryable?
+    pipeline.retryable? &&
+      can?(request.user, :update_pipeline, pipeline)
   end
 
   def can_cancel?
-    can?(request.user, :update_pipeline, pipeline) &&
-      pipeline.cancelable?
+    pipeline.cancelable? &&
+      can?(request.user, :update_pipeline, pipeline)
   end
 
   def detailed_status
diff --git a/app/serializers/pipeline_serializer.rb b/app/serializers/pipeline_serializer.rb
index e7a9df8ac4e..7829df9fada 100644
--- a/app/serializers/pipeline_serializer.rb
+++ b/app/serializers/pipeline_serializer.rb
@@ -13,15 +13,7 @@ class PipelineSerializer < BaseSerializer
 
   def represent(resource, opts = {})
     if resource.is_a?(ActiveRecord::Relation)
-      resource = resource.preload([
-        :retryable_builds,
-        :cancelable_statuses,
-        :trigger_requests,
-        :project,
-        { pending_builds: :project },
-        { manual_actions: :project },
-        { artifacts: :project }
-      ])
+      resource = resource.includes(project: :namespace)
     end
 
     if paginated?
diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb
index ecc6173a96a..f72ddbf690c 100644
--- a/app/services/ci/retry_pipeline_service.rb
+++ b/app/services/ci/retry_pipeline_service.rb
@@ -7,7 +7,9 @@ module Ci
         raise Gitlab::Access::AccessDeniedError
       end
 
-      pipeline.retryable_builds.find_each do |build|
+      pipeline.builds.latest.failed_or_canceled.find_each do |build|
+        next unless build.retryable?
+
         Ci::RetryBuildService.new(project, current_user)
           .reprocess(build)
       end
diff --git a/changelogs/unreleased/optimise-pipelines-json.yml b/changelogs/unreleased/optimise-pipelines-json.yml
deleted file mode 100644
index 948679dcbeb..00000000000
--- a/changelogs/unreleased/optimise-pipelines-json.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Optimise pipelines.json endpoint
-merge_request:
-author:
diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb
index c3a29d8bf04..2295455575d 100644
--- a/spec/factories/ci/triggers.rb
+++ b/spec/factories/ci/triggers.rb
@@ -1,7 +1,7 @@
 FactoryGirl.define do
   factory :ci_trigger_without_token, class: Ci::Trigger do
     factory :ci_trigger do
-      sequence(:token) { |n| "token#{n}" }
+      token 'token'
 
       factory :ci_trigger_for_trigger_schedule do
         token { SecureRandom.hex(15) }
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 0abf89d060c..96031797bc0 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -92,11 +92,6 @@ pipelines:
 - auto_canceled_by
 - auto_canceled_pipelines
 - auto_canceled_jobs
-- pending_builds
-- retryable_builds
-- cancelable_statuses
-- manual_actions
-- artifacts
 statuses:
 - project
 - pipeline
@@ -214,7 +209,6 @@ project:
 - builds
 - runner_projects
 - runners
-- active_runners
 - variables
 - triggers
 - trigger_schedules
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 6e8845cdcf4..8601160561f 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -764,6 +764,40 @@ describe Ci::Build, :models do
     end
   end
 
+  describe '#has_commands?' do
+    context 'when build has commands' do
+      let(:build) do
+        create(:ci_build, commands: 'rspec')
+      end
+
+      it 'has commands' do
+        expect(build).to have_commands
+      end
+    end
+
+    context 'when does not have commands' do
+      context 'when commands are an empty string' do
+        let(:build) do
+          create(:ci_build, commands: '')
+        end
+
+        it 'has no commands' do
+          expect(build).not_to have_commands
+        end
+      end
+
+      context 'when commands are not set at all' do
+        let(:build) do
+          create(:ci_build, commands: nil)
+        end
+
+        it 'has no commands' do
+          expect(build).not_to have_commands
+        end
+      end
+    end
+  end
+
   describe '#has_tags?' do
     context 'when build has tags' do
       subject { create(:ci_build, tag_list: ['tag']) }
diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb
index d26121018ce..42170de0180 100644
--- a/spec/models/ci/trigger_spec.rb
+++ b/spec/models/ci/trigger_spec.rb
@@ -17,8 +17,8 @@ describe Ci::Trigger, models: true do
       expect(trigger.token).not_to be_nil
     end
 
-    it 'does not set a random token if one provided' do
-      trigger = create(:ci_trigger, project: project, token: 'token')
+    it 'does not set an random token if one provided' do
+      trigger = create(:ci_trigger, project: project)
 
       expect(trigger.token).to eq('token')
     end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 92d420337f9..0441f2790fc 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -58,7 +58,6 @@ describe Project, models: true do
     it { is_expected.to have_many(:builds) }
     it { is_expected.to have_many(:runner_projects) }
     it { is_expected.to have_many(:runners) }
-    it { is_expected.to have_many(:active_runners) }
     it { is_expected.to have_many(:variables) }
     it { is_expected.to have_many(:triggers) }
     it { is_expected.to have_many(:pages_domains) }
diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb
index f6249ab4664..8642b803844 100644
--- a/spec/serializers/pipeline_serializer_spec.rb
+++ b/spec/serializers/pipeline_serializer_spec.rb
@@ -93,44 +93,6 @@ describe PipelineSerializer do
         end
       end
     end
-
-    context 'number of queries' do
-      let(:resource) { Ci::Pipeline.all }
-      let(:project) { create(:empty_project) }
-
-      before do
-        Ci::Pipeline::AVAILABLE_STATUSES.each do |status|
-          create_pipeline(status)
-        end
-
-        RequestStore.begin!
-      end
-
-      after do
-        RequestStore.end!
-        RequestStore.clear!
-      end
-
-      it "verifies number of queries" do
-        recorded = ActiveRecord::QueryRecorder.new { subject }
-        expect(recorded.count).to be_within(1).of(50)
-        expect(recorded.cached_count).to eq(0)
-      end
-
-      def create_pipeline(status)
-        create(:ci_empty_pipeline, project: project, status: status).tap do |pipeline|
-          Ci::Build::AVAILABLE_STATUSES.each do |status|
-            create_build(pipeline, status, status)
-          end
-        end
-      end
-
-      def create_build(pipeline, stage, status)
-        create(:ci_build, :tags, :triggered, :artifacts,
-          pipeline: pipeline, stage: stage,
-          name: stage, status: status)
-      end
-    end
   end
 
   describe '#represent_status' do
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index 245e19822f3..bb98fb37a90 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -462,9 +462,7 @@ describe Ci::ProcessPipelineService, '#execute', :services do
     builds.find_by(name: name).play(user)
   end
 
-  def manual_actions
-    pipeline.manual_actions(true)
-  end
+  delegate :manual_actions, to: :pipeline
 
   def create_build(name, **opts)
     create(:ci_build, :created, pipeline: pipeline, name: name, **opts)
diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb
index 55b531b4cf7..e40d5ebd9a8 100644
--- a/spec/support/query_recorder.rb
+++ b/spec/support/query_recorder.rb
@@ -1,29 +1,21 @@
 module ActiveRecord
   class QueryRecorder
-    attr_reader :log, :cached
+    attr_reader :log
 
     def initialize(&block)
       @log = []
-      @cached = []
       ActiveSupport::Notifications.subscribed(method(:callback), 'sql.active_record', &block)
     end
 
     def callback(name, start, finish, message_id, values)
-      if values[:name]&.include?("CACHE")
-        @cached << values[:sql]
-      elsif !values[:name]&.include?("SCHEMA")
-        @log << values[:sql]
-      end
+      return if %w(CACHE SCHEMA).include?(values[:name])
+      @log << values[:sql]
     end
 
     def count
       @log.count
     end
 
-    def cached_count
-      @cached.count
-    end
-
     def log_message
       @log.join("\n\n")
     end