Refactor commit and build

8 files changed, 69 insertions, 112 deletions

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 645ad68e1b3..bf3e8915205 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -34,10 +34,12 @@ module Ci
     belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
 
     serialize :options
+    serialize :push_data
 
     validates :commit, presence: true
     validates :status, presence: true
     validates :coverage, numericality: true, allow_blank: true
+    validates_presence_of :ref
 
     scope :running, ->() { where(status: "running") }
     scope :pending, ->() { where(status: "pending") }
@@ -45,6 +47,9 @@ module Ci
     scope :failed, ->() { where(status: "failed")  }
     scope :unstarted, ->() { where(runner_id: nil) }
     scope :running_or_pending, ->() { where(status:[:running, :pending]) }
+    scope :latest, ->() { group(:name).order(stage_idx: :asc, created_at: :desc) }
+    scope :ignore_failures, ->() { where(allow_failure: false) }
+    scope :for_ref, ->(ref) { where(ref: ref) }
 
     acts_as_taggable
 
@@ -82,6 +87,7 @@ module Ci
         new_build.name = build.name
         new_build.allow_failure = build.allow_failure
         new_build.stage = build.stage
+        new_build.stage_idx = build.stage_idx
         new_build.trigger_request = build.trigger_request
         new_build.save
         new_build
@@ -187,6 +193,16 @@ module Ci
       project.name
     end
 
+    def project_recipients
+      recipients = project.email_recipients.split(' ')
+
+      if project.email_add_pusher? && push_data[:user_email].present?
+        recipients << push_data[:user_email]
+      end
+
+      recipients.uniq
+    end
+
     def repo_url
       project.repo_url_with_auth
     end
diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb
index 6d048779cde..35134b6628e 100644
--- a/app/models/ci/commit.rb
+++ b/app/models/ci/commit.rb
@@ -23,9 +23,7 @@ module Ci
     has_many :builds, dependent: :destroy, class_name: 'Ci::Build'
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest'
 
-    serialize :push_data
-
-    validates_presence_of :ref, :sha, :before_sha, :push_data
+    validates_presence_of :sha
     validate :valid_commit_sha
 
     def self.truncate_sha(sha)
@@ -69,15 +67,15 @@ module Ci
     end
 
     def git_author_name
-      commit_data[:author][:name] if commit_data && commit_data[:author]
+      commit_data.author_name if commit_data
     end
 
     def git_author_email
-      commit_data[:author][:email] if commit_data && commit_data[:author]
+      commit_data.author_email if commit_data
     end
 
     def git_commit_message
-      commit_data[:message] if commit_data && commit_data[:message]
+      commit_data.message if commit_data
     end
 
     def short_before_sha
@@ -89,84 +87,31 @@ module Ci
     end
 
     def commit_data
-      push_data[:commits].find do |commit|
-        commit[:id] == sha
-      end
+      @commit ||= gl_project.commit(sha)
     rescue
       nil
     end
 
-    def project_recipients
-      recipients = project.email_recipients.split(' ')
-
-      if project.email_add_pusher? && push_data[:user_email].present?
-        recipients << push_data[:user_email]
-      end
-
-      recipients.uniq
-    end
-
     def stage
-      return unless config_processor
-      stages = builds_without_retry.select(&:active?).map(&:stage)
-      config_processor.stages.find { |stage| stages.include? stage }
+      builds_without_retry.group(:stage_idx).select(:stage).last
     end
 
-    def create_builds_for_stage(stage, trigger_request)
+    def create_builds(ref, tag, push_data, trigger_request = nil)
       return if skip_ci? && trigger_request.blank?
       return unless config_processor
-
-      builds_attrs = config_processor.builds_for_stage_and_ref(stage, ref, tag)
-      builds_attrs.map do |build_attrs|
-        builds.create!({
-                         name: build_attrs[:name],
-                         commands: build_attrs[:script],
-                         tag_list: build_attrs[:tags],
-                         options: build_attrs[:options],
-                         allow_failure: build_attrs[:allow_failure],
-                         stage: build_attrs[:stage],
-                         trigger_request: trigger_request,
-                       })
-      end
+      CreateBuildsService.new.execute(self, config_processor, ref, tag, push_data, trigger_request)
     end
 
-    def create_next_builds(trigger_request)
-      return if skip_ci? && trigger_request.blank?
-      return unless config_processor
-
-      stages = builds.where(trigger_request: trigger_request).group_by(&:stage)
-
-      config_processor.stages.any? do |stage|
-        !stages.include?(stage) && create_builds_for_stage(stage, trigger_request).present?
-      end
+    def refs
+      builds.group(:ref).pluck(:ref)
     end
 
-    def create_builds(trigger_request = nil)
-      return if skip_ci? && trigger_request.blank?
-      return unless config_processor
-
-      config_processor.stages.any? do |stage|
-        create_builds_for_stage(stage, trigger_request).present?
-      end
+    def last_ref
+      builds.latest.first.try(:ref)
     end
 
     def builds_without_retry
-      @builds_without_retry ||=
-        begin
-          grouped_builds = builds.group_by(&:name)
-          grouped_builds.map do |name, builds|
-            builds.sort_by(&:id).last
-          end
-        end
-    end
-
-    def builds_without_retry_sorted
-      return builds_without_retry unless config_processor
-
-      stages = config_processor.stages
-      builds_without_retry.sort_by do |build|
-        [stages.index(build.stage) || -1, build.name || ""]
-      end
+      builds.latest
     end
 
     def retried_builds
@@ -180,35 +125,32 @@ module Ci
         return 'failed'
       elsif builds.none?
         return 'skipped'
-      elsif success?
-        'success'
-      elsif pending?
-        'pending'
-      elsif running?
-        'running'
-      elsif canceled?
-        'canceled'
+      end
+
+      statuses = builds_without_retry.ignore_failures.pluck(:status)
+      if statuses.all? { |status| status == 'success' }
+        return 'success'
+      elsif statuses.all? { |status| status == 'pending' }
+        return 'pending'
+      elsif statuses.include?('running') || statuses.include?('pending')
+        return 'running'
+      elsif statuses.all? { |status| status == 'canceled' }
+        return 'canceled'
       else
-        'failed'
+        return 'failed'
       end
     end
 
     def pending?
-      builds_without_retry.all? do |build|
-        build.pending?
-      end
+      status == 'pending'
     end
 
     def running?
-      builds_without_retry.any? do |build|
-        build.running? || build.pending?
-      end
+      status == 'running'
     end
 
     def success?
-      builds_without_retry.all? do |build|
-        build.success? || build.ignored?
-      end
+      status == 'success'
     end
 
     def failed?
@@ -216,15 +158,17 @@ module Ci
     end
 
     def canceled?
-      builds_without_retry.all? do |build|
-        build.canceled?
-      end
+      status == 'canceled'
     end
 
     def duration
       @duration ||= builds_without_retry.select(&:duration).sum(&:duration).to_i
     end
 
+    def duration_for_ref(ref)
+      builds_without_retry.for_ref(ref).select(&:duration).sum(&:duration).to_i
+    end
+
     def finished_at
       @finished_at ||= builds.order('finished_at DESC').first.try(:finished_at)
     end
@@ -238,12 +182,12 @@ module Ci
       end
     end
 
-    def matrix?
-      builds_without_retry.size > 1
+    def matrix_for_ref?(ref)
+      builds_without_retry.for_ref(ref).pluck(:id).size > 1
     end
 
     def config_processor
-      @config_processor ||= Ci::GitlabCiYamlProcessor.new(push_data[:ci_yaml_file])
+      @config_processor ||= Ci::GitlabCiYamlProcessor.new(ci_yaml_file)
     rescue Ci::GitlabCiYamlProcessor::ValidationError => e
       save_yaml_error(e.message)
       nil
@@ -253,10 +197,15 @@ module Ci
       nil
     end
 
+    def ci_yaml_file
+      gl_project.repository.blob_at(sha, '.gitlab-ci.yml')
+    rescue
+      nil
+    end
+
     def skip_ci?
       return false if builds.any?
-      commits = push_data[:commits]
-      commits.present? && commits.last[:message] =~ /(\[ci skip\])/
+      git_commit_message =~ /(\[ci skip\])/ if git_commit_message
     end
 
     def update_committed!
diff --git a/app/models/ci/project_status.rb b/app/models/ci/project_status.rb
index 6d5cafe81a2..b66f1212f23 100644
--- a/app/models/ci/project_status.rb
+++ b/app/models/ci/project_status.rb
@@ -28,18 +28,6 @@ module Ci
       status
     end
 
-    # only check for toggling build status within same ref.
-    def last_commit_changed_status?
-      ref = last_commit.ref
-      last_commits = commits.where(ref: ref).last(2)
-
-      if last_commits.size < 2
-        false
-      else
-        last_commits[0].status != last_commits[1].status
-      end
-    end
-
     def last_commit_for_ref(ref)
       commits.where(ref: ref).last
     end
diff --git a/app/models/project.rb b/app/models/project.rb
index b90a82da9f2..bb47b9abb03 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -744,7 +744,11 @@ class Project < ActiveRecord::Base
   end
 
   def ci_commit(sha)
-    gitlab_ci_project.commits.find_by(sha: sha) if gitlab_ci?
+    ci_commits.find_by(sha: sha)
+  end
+
+  def ensure_ci_commit(sha)
+    ci_commit(sha) || ci_commits.create(sha: sha)
   end
 
   def ensure_gitlab_ci_project
diff --git a/app/models/project_services/ci/hip_chat_message.rb b/app/models/project_services/ci/hip_chat_message.rb
index 25c72033eac..73fdbe801c0 100644
--- a/app/models/project_services/ci/hip_chat_message.rb
+++ b/app/models/project_services/ci/hip_chat_message.rb
@@ -13,7 +13,7 @@ module Ci
       lines.push("<a href=\"#{ci_project_url(project)}\">#{project.name}</a> - ")
       
       if commit.matrix?
-        lines.push("<a href=\"#{ci_project_ref_commits_url(project, commit.ref, commit.sha)}\">Commit ##{commit.id}</a></br>")
+        lines.push("<a href=\"#{ci_project_commits_url(project, commit.sha)}\">Commit ##{commit.id}</a></br>")
       else
         first_build = commit.builds_without_retry.first
         lines.push("<a href=\"#{ci_project_build_url(project, first_build)}\">Build '#{first_build.name}' ##{first_build.id}</a></br>")
diff --git a/app/models/project_services/ci/mail_service.rb b/app/models/project_services/ci/mail_service.rb
index 1bd2f33612b..11a2743f969 100644
--- a/app/models/project_services/ci/mail_service.rb
+++ b/app/models/project_services/ci/mail_service.rb
@@ -61,7 +61,7 @@ module Ci
     end
 
     def execute(build)
-      build.commit.project_recipients.each do |recipient|
+      build.project_recipients.each do |recipient|
         case build.status.to_sym
         when :success
           mailer.build_success_email(build.id, recipient)
diff --git a/app/models/project_services/ci/slack_message.rb b/app/models/project_services/ci/slack_message.rb
index 757b1961143..7d254836fb5 100644
--- a/app/models/project_services/ci/slack_message.rb
+++ b/app/models/project_services/ci/slack_message.rb
@@ -48,7 +48,7 @@ module Ci
     def attachment_message
       out = "<#{ci_project_url(project)}|#{project_name}>: "
       if commit.matrix?
-        out << "Commit <#{ci_project_ref_commits_url(project, commit.ref, commit.sha)}|\##{commit.id}> "
+        out << "Commit <#{ci_project_commits_url(project, commit.sha)}|\##{commit.id}> "
       else
         build = commit.builds_without_retry.first
         out << "Build <#{ci_project_build_url(project, build)}|\##{build.id}> "
diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb
index 6d2cf79b691..fd108516530 100644
--- a/app/models/project_services/gitlab_ci_service.rb
+++ b/app/models/project_services/gitlab_ci_service.rb
@@ -63,7 +63,7 @@ class GitlabCiService < CiService
   end
 
   def get_ci_commit(sha, ref)
-    Ci::Project.find(project.gitlab_ci_project).commits.find_by_sha_and_ref!(sha, ref)
+    Ci::Project.find(project.gitlab_ci_project).commits.find_by_sha!(sha)
   end
 
   def commit_status(sha, ref)
@@ -80,7 +80,7 @@ class GitlabCiService < CiService
 
   def build_page(sha, ref)
     if project.gitlab_ci_project.present?
-      ci_project_ref_commits_url(project.gitlab_ci_project, ref, sha)
+      ci_project_commits_url(project.gitlab_ci_project, sha)
     end
   end