Fix optimistic locking

7 files changed, 27 insertions, 16 deletions

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 4cb3a69416e..d159fc6c5c7 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -73,16 +73,16 @@ class CommitStatus < ActiveRecord::Base
       transition [:created, :pending, :running] => :canceled
     end
 
-    after_transition created: [:pending, :running] do |commit_status|
-      commit_status.update_attributes queued_at: Time.now
+    before_transition created: [:pending, :running] do |commit_status|
+      commit_status.queued_at = Time.now
     end
 
-    after_transition [:created, :pending] => :running do |commit_status|
-      commit_status.update_attributes started_at: Time.now
+    before_transition [:created, :pending] => :running do |commit_status|
+      commit_status.started_at = Time.now
     end
 
-    after_transition any => [:success, :failed, :canceled] do |commit_status|
-      commit_status.update_attributes finished_at: Time.now
+    before_transition any => [:success, :failed, :canceled] do |commit_status|
+      commit_status.finished_at = Time.now
     end
 
     after_transition do |commit_status, transition|
diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb
index 3b010269e0c..76e36395110 100644
--- a/app/services/ci/process_pipeline_service.rb
+++ b/app/services/ci/process_pipeline_service.rb
@@ -40,10 +40,12 @@ module Ci
 
     def process_build(build, current_status)
       if valid_statuses_for_when(build.when).include?(current_status)
-        build.enqueue
+        r = build.enqueue
+        puts "process_build: #{build.id}: enqueue: #{build.status} => #{r}"
         true
       else
-        build.skip
+        r = build.skip
+        puts "process_build: #{build.id}: skip: #{build.status} => #{r}"
         false
       end
     end
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index feee0f025d8..07a2c316899 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -178,6 +178,7 @@ Ci::Pipeline:
 - finished_at
 - duration
 - user_id
+- lock_version
 CommitStatus:
 - id
 - project_id
@@ -217,6 +218,7 @@ CommitStatus:
 - yaml_variables
 - queued_at
 - token
+- lock_version
 Ci::Variable:
 - id
 - project_id
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 43397c5ae39..e13cd8c1dd0 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -138,9 +138,9 @@ describe Ci::Pipeline, models: true do
 
   describe 'state machine' do
     let(:current) { Time.now.change(usec: 0) }
-    let(:build) { create_build('build1', current, 10) }
-    let(:build_b) { create_build('build2', current, 20) }
-    let(:build_c) { create_build('build3', current + 50, 10) }
+    let(:build) { create_build('build1', 0) }
+    let(:build_b) { create_build('build2', 0) }
+    let(:build_c) { create_build('build3', 0) }
 
     describe '#duration' do
       before do
@@ -163,11 +163,12 @@ describe Ci::Pipeline, models: true do
           build_c.success
         end
 
-        pipeline.drop
+        # We have to reload pipeline, because its status is updated by processing builds
+        pipeline.reload.drop
       end
 
       it 'matches sum of builds duration' do
-        pipeline.reload
+        binding.pry
 
         expect(pipeline.duration).to eq(40)
       end
@@ -455,7 +456,9 @@ describe Ci::Pipeline, models: true do
         context 'when all builds succeed' do
           before do
             build_a.success
-            build_b.success
+
+            # We have to reload build_b as this is in next stage and it gets triggered by PipelineProcessWorker
+            build_b.reload.success
           end
 
           it 'receives a success event once' do
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index 95c7bbf99c9..fc72a44d663 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -277,6 +277,7 @@ describe API::API, api: true do
 
       context 'with regular branch' do
         before do
+          pipeline.reload
           pipeline.update(ref: 'master',
                           sha: project.commit('master').sha)
 
@@ -288,6 +289,7 @@ describe API::API, api: true do
 
       context 'with branch name containing slash' do
         before do
+          pipeline.reload
           pipeline.update(ref: 'improve/awesome',
                           sha: project.commit('improve/awesome').sha)
         end
diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb
index 1e21a32a062..a3fc23ba177 100644
--- a/spec/services/ci/register_build_service_spec.rb
+++ b/spec/services/ci/register_build_service_spec.rb
@@ -101,11 +101,11 @@ module Ci
           it 'equalises number of running builds' do
             # after finishing the first build for project 1, get a second build from the same project
             expect(service.execute(shared_runner)).to eq(build1_project1)
-            build1_project1.success
+            build1_project1.reload.success
             expect(service.execute(shared_runner)).to eq(build2_project1)
 
             expect(service.execute(shared_runner)).to eq(build1_project2)
-            build1_project2.success
+            build1_project2.reload.success
             expect(service.execute(shared_runner)).to eq(build2_project2)
             expect(service.execute(shared_runner)).to eq(build1_project3)
             expect(service.execute(shared_runner)).to eq(build3_project1)
diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
index b80cfd8f450..1f90efdbd6a 100644
--- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb
@@ -147,6 +147,7 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
         expect(MergeWorker).not_to receive(:perform_async)
 
         build.success
+        test.reload
         test.drop
       end
 
@@ -154,6 +155,7 @@ describe MergeRequests::MergeWhenBuildSucceedsService do
         expect(MergeWorker).to receive(:perform_async)
 
         build.success
+        test.reload
         test.success
       end
     end