From a9c31fa5e989681760396cb2ca026dc92353d96d Mon Sep 17 00:00:00 2001 From: Tyranja Date: Fri, 23 Jan 2015 16:02:29 +0100 Subject: [PATCH] Extract cancel and restart builds and jobs into sidekiq worker See below for all the commit messages squashed into this one... add a test delete empty space add job_cancellation worker change id change job_id param add job restart worker add require to sidekiq.rb change params add test for job restart and cancellation add test for job restart update build.rb improve build spec update job restart with correct response, add test to chack for correct response when restarting job --- Procfile | 2 +- lib/travis/api/app/endpoint/builds.rb | 14 ++++-- lib/travis/api/app/endpoint/jobs.rb | 16 ++++++- lib/travis/api/workers/job_cancellation.rb | 19 ++++++++ lib/travis/api/workers/job_restart.rb | 19 ++++++++ lib/travis/sidekiq.rb | 2 + spec/integration/v2/builds_spec.rb | 40 +++++++++++++++++ spec/integration/v2/jobs_spec.rb | 52 +++++++++++++++++++--- 8 files changed, 151 insertions(+), 13 deletions(-) create mode 100644 lib/travis/api/workers/job_cancellation.rb create mode 100644 lib/travis/api/workers/job_restart.rb diff --git a/Procfile b/Procfile index 3c9c0456..6d6d6353 100644 --- a/Procfile +++ b/Procfile @@ -1,3 +1,3 @@ web: bundle exec ./script/server console: bundle exec ./script/console -sidekiq: bundle exec sidekiq -c 5 -r ./lib/travis/sidekiq.rb -q build_cancellations, -q build_restarts +sidekiq: bundle exec sidekiq -c 5 -r ./lib/travis/sidekiq.rb -q build_cancellations, -q build_restarts, -q job_cancellations, -q job_restarts diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index f6a983e1..ca0d56b3 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -39,8 +39,6 @@ class Travis::Api::App status 422 respond_with json else - #service.run - #check syntax of line below Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') Metriks.meter("api.request.cancel_build.success").mark @@ -51,8 +49,16 @@ class Travis::Api::App post '/:id/restart' do Metriks.meter("api.request.restart_build").mark - Travis::Sidekiq::BuildRestart.perform_async(id: params[:id], user_id: current_user.id) - #respond_with service(:reset_model, build_id: params[:id]) + service = self.service(:reset_model, build_id: params[:id]) + if !service.accept? + status 400 + result = false + else + Travis::Sidekiq::BuildRestart.perform_async(id: params[:id], user_id: current_user.id) + status 200 + result = true + end + respond_with(result: result, flash: service.messages) end end end diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 0d65e78e..d856093c 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -1,4 +1,6 @@ require 'travis/api/app' +require 'travis/api/workers/job_cancellation' +require 'travis/api/workers/job_restart' class Travis::Api::App class Endpoint @@ -44,7 +46,7 @@ class Travis::Api::App status 422 respond_with json else - service.run + Travis::Sidekiq::JobCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') Metriks.meter("api.request.cancel_job.success").mark status 204 @@ -53,7 +55,17 @@ class Travis::Api::App post '/:id/restart' do Metriks.meter("api.request.restart_job").mark - respond_with service(:reset_model, job_id: params[:id]) + + service = self.service(:reset_model, job_id: params[:id]) + if !service.accept? + status 400 + result = false + else + Travis::Sidekiq::JobRestart.perform_async(id: params[:id], user_id: current_user.id) + status 200 + result = true + end + respond_with(result: result, flash: service.messages) end get '/:job_id/log' do diff --git a/lib/travis/api/workers/job_cancellation.rb b/lib/travis/api/workers/job_cancellation.rb new file mode 100644 index 00000000..dadf60d0 --- /dev/null +++ b/lib/travis/api/workers/job_cancellation.rb @@ -0,0 +1,19 @@ +require 'sidekiq/worker' +require 'multi_json' + +module Travis + module Sidekiq + class JobCancellation + class ProcessingError < StandardError; end + + include ::Sidekiq::Worker + sidekiq_options queue: :job_cancellations + + def perform(data) + user = User.find(data['user_id']) + Travis.service(:cancel_job, user, { id: data['id'], source: data['source'] }).run + end + + end + end +end diff --git a/lib/travis/api/workers/job_restart.rb b/lib/travis/api/workers/job_restart.rb new file mode 100644 index 00000000..12ab6b33 --- /dev/null +++ b/lib/travis/api/workers/job_restart.rb @@ -0,0 +1,19 @@ +require 'sidekiq/worker' +require 'multi_json' + +module Travis + module Sidekiq + class JobRestart + class ProcessingError < StandardError; end + + include ::Sidekiq::Worker + sidekiq_options queue: :job_restarts + + def perform(data) + user = User.find(data['user_id']) + Travis.service(:reset_model, user, job_id: data['id']).run + end + + end + end +end diff --git a/lib/travis/sidekiq.rb b/lib/travis/sidekiq.rb index 68cf3ead..14b62d63 100644 --- a/lib/travis/sidekiq.rb +++ b/lib/travis/sidekiq.rb @@ -3,6 +3,8 @@ require 'sidekiq' require 'travis' require 'travis/api/workers/build_cancellation' require 'travis/api/workers/build_restart' +require 'travis/api/workers/job_cancellation' +require 'travis/api/workers/job_restart' require 'travis/support/amqp' Travis::Database.connect diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index d254229d..b1c20506 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -92,4 +92,44 @@ describe 'Builds' do end end end + + describe 'POST /builds/:id/restart' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + + before { + headers.merge! 'HTTP_AUTHORIZATION' => "token #{token}" + user.permissions.create!(repository_id: build.repository.id, :pull => true, :push => true) + } + + context 'when restart is not acceptable' do + before { user.permissions.destroy_all } + + it 'responds with 400' do + response = post "/builds/#{build.id}/restart", {}, headers + response.status.should == 400 + end + end + + context 'when build passed' do + before do + Travis::Sidekiq::BuildCancellation.stubs(:perform_async) + build.matrix.each { |j| j.update_attribute(:state, 'passed') } + build.update_attribute(:state, 'passed') + end + + it 'restarts the build' do + Travis::Sidekiq::BuildRestart.expects(:perform_async).with(id: build.id.to_s, user_id: user.id) + response = post "/builds/#{build.id}/restart", {}, headers + response.status.should == 200 + end + + it 'sends the correct response body' do + Travis::Sidekiq::BuildRestart.expects(:perform_async).with(id: build.id.to_s, user_id: user.id) + response = post "/builds/#{build.id}/restart", {}, headers + body = JSON.parse(response.body) + body.should == {"result"=>true, "flash"=>[{"notice"=>"The build was successfully restarted."}]} + end + end + end end diff --git a/spec/integration/v2/jobs_spec.rb b/spec/integration/v2/jobs_spec.rb index a17a4b17..372a5245 100644 --- a/spec/integration/v2/jobs_spec.rb +++ b/spec/integration/v2/jobs_spec.rb @@ -242,16 +242,56 @@ describe 'Jobs' do end context 'when job can be canceled' do - it 'cancels the job and responds with 204' do + before do job.update_attribute(:state, 'created') + end - response = nil - expect { - response = post "/jobs/#{job.id}/cancel", {}, headers - }.to change { job.reload.state } + it 'cancels the job' do + Travis::Sidekiq::JobCancellation.expects(:perform_async).with( id: job.id.to_s, user_id: user.id, source: 'api') + post "/jobs/#{job.id}/cancel", {}, headers + end + + it 'responds with 204' do + response = post "/jobs/#{job.id}/cancel", {}, headers response.status.should == 204 + end + end + end - job.state.should == 'canceled' + describe 'POST /jobs/:id/restart' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + + before { + headers.merge! 'HTTP_AUTHORIZATION' => "token #{token}" + user.permissions.create!(repository_id: job.repository.id, :pull => true, :push => true) + } + + context 'when restart is not acceptable' do + before { user.permissions.destroy_all } + + it 'responds with 400' do + response = post "/jobs/#{job.id}/restart", {}, headers + response.status.should == 400 + end + end + + context 'when job passed' do + before do + Travis::Sidekiq::JobCancellation.stubs(:perform_async) + job.update_attribute(:state, 'passed') + end + + it 'restarts the job' do + Travis::Sidekiq::JobRestart.expects(:perform_async).with(id: job.id.to_s, user_id: user.id) + response = post "/jobs/#{job.id}/restart", {}, headers + response.status.should == 200 + end + it 'sends the correct response body' do + Travis::Sidekiq::JobRestart.expects(:perform_async).with(id: job.id.to_s, user_id: user.id) + response = post "/jobs/#{job.id}/restart", {}, headers + body = JSON.parse(response.body) + body.should == {"result"=>true, "flash"=>[{"notice"=>"The job was successfully restarted."}]} end end end