Merge branch 'ci-commits-to-projects' into 'master'

Make Ci::Commits belong to Project instead of Ci::Project



See merge request !1455

app/models/ci/build.rb

@@ -30,7 +30,6 @@ module Ci
     LAZY_ATTRIBUTES = ['trace']
     belongs_to :commit, class_name: 'Ci::Commit'
-    belongs_to :project, class_name: 'Ci::Project'
     belongs_to :runner, class_name: 'Ci::Runner'
     belongs_to :trigger_request, class_name: 'Ci::TriggerRequest'
@@ -80,7 +79,6 @@ module Ci
         new_build.commands = build.commands
         new_build.tag_list = build.tag_list
         new_build.commit_id = build.commit_id
-        new_build.project_id = build.project_id
         new_build.name = build.name
         new_build.allow_failure = build.allow_failure
         new_build.stage = build.stage
@@ -137,7 +135,7 @@ module Ci
       state :canceled, value: 'canceled'
     end
-    delegate :sha, :short_sha, :before_sha, :ref,
+    delegate :sha, :short_sha, :before_sha, :ref, :project,
       to: :commit, prefix: false
     def trace_html
@@ -188,7 +186,7 @@ module Ci
     end
     def project_id
-      commit.project_id
+      commit.project.id
     end
     def project_name

app/models/ci/commit.rb

@@ -18,8 +18,8 @@
 module Ci
   class Commit < ActiveRecord::Base
     extend Ci::Model
-    
-    belongs_to :project, class_name: 'Ci::Project'
+
+    belongs_to :gl_project, class_name: '::Project', foreign_key: :gl_project_id
     has_many :builds, dependent: :destroy, class_name: 'Ci::Build'
     has_many :trigger_requests, dependent: :destroy, class_name: 'Ci::TriggerRequest'
@@ -36,6 +36,14 @@ module Ci
       sha
     end
+    def project
+      @project ||= gl_project.ensure_gitlab_ci_project
+    end
+
+    def project_id
+      project.id
+    end
+
     def last_build
       builds.order(:id).last
     end
@@ -111,15 +119,14 @@ module Ci
       builds_attrs = config_processor.builds_for_stage_and_ref(stage, ref, tag)
       builds_attrs.map do |build_attrs|
         builds.create!({
-          project: project,
-          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,
-        })
+                         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
     end

app/models/ci/project.rb

@@ -33,15 +33,12 @@ module Ci
     belongs_to :gl_project, class_name: '::Project', foreign_key: :gitlab_id
-    has_many :commits, ->() { order('CASE WHEN ci_commits.committed_at IS NULL THEN 0 ELSE 1 END', :committed_at, :id) }, dependent: :destroy, class_name: 'Ci::Commit'
-    has_many :builds, through: :commits, dependent: :destroy, class_name: 'Ci::Build'
     has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject'
     has_many :runners, through: :runner_projects, class_name: 'Ci::Runner'
     has_many :web_hooks, dependent: :destroy, class_name: 'Ci::WebHook'
     has_many :events, dependent: :destroy, class_name: 'Ci::Event'
     has_many :variables, dependent: :destroy, class_name: 'Ci::Variable'
     has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger'
-    has_one :last_commit, -> { order 'ci_commits.created_at DESC' }, class_name: 'Ci::Commit'
     # Project services
     has_many :services, dependent: :destroy, class_name: 'Ci::Service'
@@ -55,13 +52,13 @@ module Ci
     # Validations
     #
     validates_presence_of :name, :timeout, :token, :default_ref,
-      :path, :ssh_url_to_repo, :gitlab_id
+                          :path, :ssh_url_to_repo, :gitlab_id
     validates_uniqueness_of :gitlab_id
     validates :polling_interval,
-      presence: true,
-      if: ->(project) { project.always_build.present? }
+              presence: true,
+              if: ->(project) { project.always_build.present? }
     scope :public_only, ->() { where(public: true) }
@@ -79,12 +76,12 @@ module Ci
       def parse(project)
         params = {
-          name:                     project.name_with_namespace,
-          gitlab_id:                project.id,
-          path:                     project.path_with_namespace,
-          default_ref:              project.default_branch || 'master',
-          ssh_url_to_repo:          project.ssh_url_to_repo,
-          email_add_pusher:         current_application_settings.add_pusher,
+          name: project.name_with_namespace,
+          gitlab_id: project.id,
+          path: project.path_with_namespace,
+          default_ref: project.default_branch || 'master',
+          ssh_url_to_repo: project.ssh_url_to_repo,
+          email_add_pusher: current_application_settings.add_pusher,
           email_only_broken_builds: current_application_settings.all_broken_builds,
         }
@@ -104,8 +101,8 @@ module Ci
       end
       def ordered_by_last_commit_date
-        last_commit_subquery = "(SELECT project_id, MAX(committed_at) committed_at FROM #{Ci::Commit.table_name} GROUP BY project_id)"
-        joins("LEFT JOIN #{last_commit_subquery} AS last_commit ON #{Ci::Project.table_name}.id = last_commit.project_id").
+        last_commit_subquery = "(SELECT gl_project_id, MAX(committed_at) committed_at FROM #{Ci::Commit.table_name} GROUP BY gl_project_id)"
+        joins("LEFT JOIN #{last_commit_subquery} AS last_commit ON #{Ci::Project.table_name}.gitlab_id = last_commit.gl_project_id").
           order("CASE WHEN last_commit.committed_at IS NULL THEN 1 ELSE 0 END, last_commit.committed_at DESC")
       end
@@ -125,10 +122,14 @@ module Ci
     def set_default_values
       self.token = SecureRandom.hex(15) if self.token.blank?
+      self.default_ref ||= 'master'
+      self.name ||= gl_project.name_with_namespace
+      self.path ||= gl_project.path_with_namespace
+      self.ssh_url_to_repo ||= gl_project.ssh_url_to_repo
     end
     def tracked_refs
-      @tracked_refs ||= default_ref.split(",").map{|ref| ref.strip}
+      @tracked_refs ||= default_ref.split(",").map { |ref| ref.strip }
     end
     def valid_token? token
@@ -207,5 +208,13 @@ module Ci
     def setup_finished?
       commits.any?
     end
+
+    def commits
+      gl_project.ci_commits
+    end
+
+    def builds
+      gl_project.ci_builds
+    end
   end
 end

app/models/ci/runner.rb

@@ -41,6 +41,10 @@ module Ci
             query: "%#{query.try(:downcase)}%")
     end
+    def gl_projects_ids
+      projects.select(:gitlab_id)
+    end
+
     def set_default_values
       self.token = SecureRandom.hex(15) if self.token.blank?
     end

app/models/project.rb

@@ -118,6 +118,8 @@ class Project < ActiveRecord::Base
   has_many :deploy_keys, through: :deploy_keys_projects
   has_many :users_star_projects, dependent: :destroy
   has_many :starrers, through: :users_star_projects, source: :user
+  has_many :ci_commits, ->() { order('CASE WHEN ci_commits.committed_at IS NULL THEN 0 ELSE 1 END', :committed_at, :id) }, dependent: :destroy, class_name: 'Ci::Commit', foreign_key: :gl_project_id
+  has_many :ci_builds, through: :ci_commits, source: :builds, dependent: :destroy, class_name: 'Ci::Build'
   has_one :import_data, dependent: :destroy, class_name: "ProjectImportData"
   has_one :gitlab_ci_project, dependent: :destroy, class_name: "Ci::Project", foreign_key: :gitlab_id
@@ -745,6 +747,10 @@ class Project < ActiveRecord::Base
     gitlab_ci_project.commits.find_by(sha: sha) if gitlab_ci?
   end
+  def ensure_gitlab_ci_project
+    gitlab_ci_project || create_gitlab_ci_project
+  end
+
   def enable_ci(user)
     # Enable service
     service = gitlab_ci_service || create_gitlab_ci_service

app/services/ci/register_build_service.rb

@@ -8,10 +8,10 @@ module Ci
       builds =
         if current_runner.shared?
           # don't run projects which have not enables shared runners
-          builds.includes(:project).where(ci_projects: { shared_runners_enabled: true })
+          builds.joins(commit: { gl_project: :gitlab_ci_project }).where(ci_projects: { shared_runners_enabled: true })
         else
           # do run projects which are only assigned to this runner
-          builds.where(project_id: current_runner.projects)
+          builds.joins(:commit).where(ci_commits: { gl_project_id: current_runner.gl_projects_ids })
         end
       builds = builds.order('created_at ASC')
@@ -19,7 +19,7 @@ module Ci
       build = builds.find do |build|
         (build.tag_list - current_runner.tag_list).empty?
       end
-        
+
       if build
         # In case when 2 runners try to assign the same build, second runner will be declined

app/views/ci/admin/projects/_project.html.haml

-- last_commit = project.last_commit
+- last_commit = project.commits.last
 %tr
   %td
     = project.id

db/migrate/20150924125150_add_project_id_to_ci_commit.rb

+class AddProjectIdToCiCommit < ActiveRecord::Migration
+  def up
+    add_column :ci_commits, :gl_project_id, :integer
+  end
+end

db/migrate/20150924125436_migrate_project_id_for_ci_commits.rb

+class MigrateProjectIdForCiCommits < ActiveRecord::Migration
+  def up
+    subquery = 'SELECT gitlab_id FROM ci_projects WHERE ci_projects.id = ci_commits.project_id'
+    execute("UPDATE ci_commits SET gl_project_id=(#{subquery}) WHERE gl_project_id IS NULL")
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20150920161119) do
+ActiveRecord::Schema.define(version: 20150924125436) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -115,9 +115,10 @@ ActiveRecord::Schema.define(version: 20150920161119) do
     t.text     "push_data"
     t.datetime "created_at"
     t.datetime "updated_at"
-    t.boolean  "tag",          default: false
+    t.boolean  "tag",           default: false
     t.text     "yaml_errors"
     t.datetime "committed_at"
+    t.integer  "gl_project_id"
   end
   add_index "ci_commits", ["project_id", "committed_at", "id"], name: "index_ci_commits_on_project_id_and_committed_at_and_id", using: :btree

features/steps/project/commits/commits.rb

@@ -104,7 +104,7 @@ class Spinach::Features::ProjectCommits < Spinach::FeatureSteps
   step 'commit has ci status' do
     @project.enable_ci(@user)
-    create :ci_commit, project: @project.gitlab_ci_project, sha: sample_commit.id
+    create :ci_commit, gl_project: @project, sha: sample_commit.id
   end
   step 'I see commit ci info' do

features/steps/shared/project.rb

@@ -204,6 +204,6 @@ module SharedProject
   step 'project "Shop" has CI build' do
     project = Project.find_by(name: "Shop")
-    create :ci_commit, project: project.gitlab_ci_project, sha: project.commit.sha
+    create :ci_commit, gl_project: project, sha: project.commit.sha
   end
 end

spec/controllers/ci/commits_controller_spec.rb

 require "spec_helper"
 describe Ci::CommitsController do
-  before do
-    @project = FactoryGirl.create :ci_project
-  end
-
   describe "GET /status" do
     it "returns status of commit" do
-      commit = FactoryGirl.create :ci_commit, project: @project
-      get :status, id: commit.sha, ref_id: commit.ref, project_id: @project.id
+      commit = FactoryGirl.create :ci_commit
+      get :status, id: commit.sha, ref_id: commit.ref, project_id: commit.project.id
       expect(response).to be_success
       expect(response.code).to eq('200')
@@ -16,8 +12,8 @@ describe Ci::CommitsController do
     end
     it "returns not_found status" do
-      commit = FactoryGirl.create :ci_commit, project: @project
-      get :status, id: commit.sha, ref_id: "deploy", project_id: @project.id
+      commit = FactoryGirl.create :ci_commit
+      get :status, id: commit.sha, ref_id: "deploy", project_id: commit.project.id
       expect(response).to be_success
       expect(response.code).to eq('200')

spec/factories/ci/commits.rb

@@ -51,6 +51,8 @@ FactoryGirl.define do
       }
     end
+    gl_project factory: :empty_project
+
     factory :ci_commit_without_jobs do
       after(:create) do |commit, evaluator|
         commit.push_data[:ci_yaml_file] = YAML.dump({})

spec/factories/ci/projects.rb

@@ -43,7 +43,7 @@ FactoryGirl.define do
       "git@demo.gitlab.com:gitlab/gitlab-shell#{n}.git"
     end
-    gl_project factory: :project
+    gl_project factory: :empty_project
     factory :ci_project do
       token 'iPWx6WM4lhHNedGfBpPJNP'

spec/features/ci/admin/builds_spec.rb

 require 'spec_helper'
 describe "Admin Builds" do
-  let(:project) { FactoryGirl.create :ci_project }
-  let(:commit) { FactoryGirl.create :ci_commit, project: project }
+  let(:commit) { FactoryGirl.create :ci_commit }
   let(:build) { FactoryGirl.create :ci_build, commit: commit }
   before do

spec/features/ci/builds_spec.rb

@@ -3,16 +3,15 @@ require 'spec_helper'
 describe "Builds" do
   context :private_project do
     before do
-      @project = FactoryGirl.create :ci_project
-      @commit = FactoryGirl.create :ci_commit, project: @project
+      @commit = FactoryGirl.create :ci_commit
       @build = FactoryGirl.create :ci_build, commit: @commit
       login_as :user
-      @project.gl_project.team << [@user, :master]
+      @commit.project.gl_project.team << [@user, :master]
     end
     describe "GET /:project/builds/:id" do
       before do
-        visit ci_project_build_path(@project, @build)
+        visit ci_project_build_path(@commit.project, @build)
       end
       it { expect(page).to have_content @commit.sha[0..7] }
@@ -23,7 +22,7 @@ describe "Builds" do
     describe "GET /:project/builds/:id/cancel" do
       before do
         @build.run!
-        visit cancel_ci_project_build_path(@project, @build)
+        visit cancel_ci_project_build_path(@commit.project, @build)
       end
       it { expect(page).to have_content 'canceled' }
@@ -33,7 +32,7 @@ describe "Builds" do
     describe "POST /:project/builds/:id/retry" do
       before do
         @build.cancel!
-        visit ci_project_build_path(@project, @build)
+        visit ci_project_build_path(@commit.project, @build)
         click_link 'Retry'
       end
@@ -45,13 +44,15 @@ describe "Builds" do
   context :public_project do
     describe "Show page public accessible" do
       before do
-        @project = FactoryGirl.create :ci_public_project
-        @commit = FactoryGirl.create :ci_commit, project: @project
+        @commit = FactoryGirl.create :ci_commit
+        @commit.project.public = true
+        @commit.project.save
+        
         @runner = FactoryGirl.create :ci_specific_runner
         @build = FactoryGirl.create :ci_build, commit: @commit, runner: @runner
         stub_gitlab_calls
-        visit ci_project_build_path(@project, @build)
+        visit ci_project_build_path(@commit.project, @build)
       end
       it { expect(page).to have_content @commit.sha[0..7] }

spec/features/ci/commits_spec.rb

@@ -5,11 +5,10 @@ describe "Commits" do
   context "Authenticated user" do
     before do
-      @project = FactoryGirl.create :ci_project
-      @commit = FactoryGirl.create :ci_commit, project: @project
+      @commit = FactoryGirl.create :ci_commit
       @build = FactoryGirl.create :ci_build, commit: @commit
       login_as :user
-      @project.gl_project.team << [@user, :master]
+      @commit.project.gl_project.team << [@user, :master]
     end
     describe "GET /:project/commits/:sha" do
@@ -51,8 +50,10 @@ describe "Commits" do
   context "Public pages" do
     before do
-      @project = FactoryGirl.create :ci_public_project
-      @commit = FactoryGirl.create :ci_commit, project: @project
+      @commit = FactoryGirl.create :ci_commit
+      @commit.project.public = true
+      @commit.project.save
+
       @build = FactoryGirl.create :ci_build, commit: @commit
     end

spec/lib/ci/charts_spec.rb

@@ -4,13 +4,12 @@ describe "Charts" do
   context "build_times" do
     before do
-      @project = FactoryGirl.create(:ci_project)
-      @commit = FactoryGirl.create(:ci_commit, project: @project)
+      @commit = FactoryGirl.create(:ci_commit)
       FactoryGirl.create(:ci_build, commit: @commit)
     end
     it 'should return build times in minutes' do
-      chart = Ci::Charts::BuildTime.new(@project)
+      chart = Ci::Charts::BuildTime.new(@commit.project)
       expect(chart.build_times).to eq([2])
     end
   end

spec/mailers/ci/notify_spec.rb

@@ -5,8 +5,7 @@ describe Ci::Notify do
   include EmailSpec::Matchers
   before do
-    @project = FactoryGirl.create :ci_project
-    @commit = FactoryGirl.create :ci_commit, project: @project
+    @commit = FactoryGirl.create :ci_commit
     @build = FactoryGirl.create :ci_build, commit: @commit
   end