From 1863f5bd0c9dc34547f0ebe0fc63d865334cb8f8 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Thu, 26 May 2016 17:08:22 +0200 Subject: [PATCH 01/13] Enqueue restarting jobs for the Hub --- lib/travis/api/app/endpoint/jobs.rb | 29 +++++++++++----- spec/integration/v2/builds_spec.rb | 2 +- spec/integration/v2/jobs_spec.rb | 53 ++++++++++++++++++++++------- 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 9bb2fbc9..e8a0e1e9 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -1,6 +1,7 @@ require 'travis/api/app' require 'travis/api/workers/job_cancellation' require 'travis/api/workers/job_restart' +require 'travis/api/enqueue/services/restart_model' class Travis::Api::App class Endpoint @@ -55,15 +56,27 @@ class Travis::Api::App post '/:id/restart' do Metriks.meter("api.request.restart_job").mark - - service = self.service(:reset_model, job_id: params[:id]) - if !service.accept? - status 400 - result = false + if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + service = Travis::Enqueue::Services::RestartModel.new(current_user, { job_id: params[:id] }) + if !service.accept? + status 400 + result = false + else + payload = {id: params[:id], user_id: current_user.id} + service.push("job:restart", payload) + status 202 + result = true + end else - Travis::Sidekiq::JobRestart.perform_async(id: params[:id], user_id: current_user.id) - status 202 - result = true + 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 202 + result = true + end end respond_with(result: result, flash: service.messages) end diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index 98a57e26..c4e78d44 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -117,7 +117,7 @@ describe 'Builds' do build.update_attribute(:state, 'passed') end - describe 'Enqueues restart event to the Hub' do + describe 'Enqueues restart event for the Hub' do before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } it 'restarts the build' do diff --git a/spec/integration/v2/jobs_spec.rb b/spec/integration/v2/jobs_spec.rb index 81486845..72dac3d0 100644 --- a/spec/integration/v2/jobs_spec.rb +++ b/spec/integration/v2/jobs_spec.rb @@ -274,24 +274,51 @@ describe 'Jobs' do response = post "/jobs/#{job.id}/restart", {}, headers response.status.should == 400 end + + context 'when enqueueing for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, job.repository.owner) } + + it 'responds with 400' do + response = post "/jobs/#{job.id}/restart", {}, headers + response.status.should == 400 + end + end end context 'when job passed' do - before do - Travis::Sidekiq::JobCancellation.stubs(:perform_async) - job.update_attribute(:state, 'passed') + before { job.update_attribute(:state, 'passed') } + + context 'Restart from travis-core' do + before { Travis::Sidekiq::JobCancellation.stubs(:perform_async) } + + 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 == 202 + 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 - 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 == 202 - 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."}]} + context 'Enqueues restart event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, job.repository.owner) } + + it 'restarts the job' do + ::Sidekiq::Client.expects(:push) + response = post "/jobs/#{job.id}/restart", {}, headers + response.status.should == 202 + end + it 'sends the correct response body' do + ::Sidekiq::Client.expects(:push) + 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 end From 428422d569d86b9eff5cf81d430fe60ae0319954 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 12:44:48 +0200 Subject: [PATCH 02/13] Add enqueue service for canceling jobs and builds --- .../api/enqueue/services/cancel_model.rb | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 lib/travis/api/enqueue/services/cancel_model.rb diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb new file mode 100644 index 00000000..47c5f006 --- /dev/null +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -0,0 +1,84 @@ +module Travis + module Enqueue + module Services + + class CancelModel + + attr_reader :current_user, :source, :target + + def initialize(current_user, params) + @current_user = current_user + @params = params + @source = params.delete(:source) || 'unknown' + target + end + + def run + cancel + end + instrument :run + + def messages + messages = [] + messages << { :notice => "The #{type} was successfully cancelled." } if can_cancel? + messages << { :error => "You are not authorized to cancel this #{type}." } unless authorized? + messages << { :error => "The #{type} could not be cancelled." } unless build.cancelable? + messages + end + + def cancel(payload) + # target may have been retrieved with a :join query, so we need to reset the readonly status + if can_cancel? + target.send(:instance_variable_set, :@readonly, false) + target.cancel! + + if type == :build + push_matrix(payload) + else + push(payload, target) + end + end + end + + def push_matrix(payload) + + target.matrix.each do |job| + push(payload, job) + end + + end + + def push(payload, job) + Travis.logger.info("Publishing cancel_job message to worker.commands queue for Job##{job.id}") + ::Sidekiq::Client.push( + 'queue' => 'hub', + 'class' => 'Travis::Hub::Sidekiq::Worker', + 'args' => ["#{type}:cancel", payload] + ) + end + + def type + @type ||= @params[:build_id] ? :build : :job + end + + def target + if type == :build + @target = Build.find(@params[:build_id]) + else + @target = Job.find(@params[:job_id]) + end + end + + def can_cancel? + authorized? && target.cancelable? + end + + def authorized? + current_user.permission?(:pull, :repository_id => target.repository_id) + end + + end + + end + end +end From c3e0d6d6bb7e4da2ebfeb17f375f7806c1612fdc Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 15:57:19 +0200 Subject: [PATCH 03/13] Use CancelModel in Builds endpoint --- lib/travis/api/app/endpoint/builds.rb | 15 ++++- .../api/enqueue/services/cancel_model.rb | 9 +-- spec/integration/v2/builds_spec.rb | 56 ++++++++++++++++--- 3 files changed, 63 insertions(+), 17 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index e04dc25f..477731ed 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -2,6 +2,7 @@ require 'travis/api/app' require 'travis/api/workers/build_cancellation' require 'travis/api/workers/build_restart' require 'travis/api/enqueue/services/restart_model' +require 'travis/api/enqueue/services/cancel_model' class Travis::Api::App class Endpoint @@ -21,7 +22,12 @@ class Travis::Api::App post '/:id/cancel' do Metriks.meter("api.request.cancel_build").mark - service = self.service(:cancel_build, params.merge(source: 'api')) + if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { build_id: params[:id], source: 'api' }) + else + service = self.service(:cancel_build, params.merge(source: 'api')) + end + if !service.authorized? json = { error: { message: "You don't have access to cancel build(#{params[:id]})" @@ -40,7 +46,12 @@ class Travis::Api::App status 422 respond_with json else - Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + if service.respond_to?(:enqueue_to_hub) + payload = { build_id: params[:id], source: 'api'} + service.enqueue_to_hub(payload) + else + Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + end Metriks.meter("api.request.cancel_build.success").mark status 204 diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb index 47c5f006..d8868a50 100644 --- a/lib/travis/api/enqueue/services/cancel_model.rb +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -13,11 +13,6 @@ module Travis target end - def run - cancel - end - instrument :run - def messages messages = [] messages << { :notice => "The #{type} was successfully cancelled." } if can_cancel? @@ -26,7 +21,7 @@ module Travis messages end - def cancel(payload) + def enqueue_to_hub(payload) # target may have been retrieved with a :join query, so we need to reset the readonly status if can_cancel? target.send(:instance_variable_set, :@readonly, false) @@ -41,11 +36,9 @@ module Travis end def push_matrix(payload) - target.matrix.each do |job| push(payload, job) end - end def push(payload, job) diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index c4e78d44..f3fb77a1 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -63,6 +63,16 @@ describe 'Builds' do response = post "/builds/#{build.id}/cancel", {}, headers response.status.should == 403 end + + context 'and enqueues cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } + + it 'responds with 403' do + response = post "/builds/#{build.id}/cancel", {}, headers + response.status.should == 403 + end + + end end context 'when build is not cancelable' do @@ -72,23 +82,55 @@ describe 'Builds' do response = post "/builds/#{build.id}/cancel", {}, headers response.status.should == 422 end + + context 'and enqueues cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } + + it 'responds with 422' do + response = post "/builds/#{build.id}/cancel", {}, headers + response.status.should == 422 + end + end end context 'when build can be canceled' do before do - Travis::Sidekiq::BuildCancellation.stubs(:perform_async) build.matrix.each { |j| j.update_attribute(:state, 'created') } build.update_attribute(:state, 'created') end - it 'cancels the build' do - Travis::Sidekiq::BuildCancellation.expects(:perform_async).with( id: build.id.to_s, user_id: user.id, source: 'api') - post "/builds/#{build.id}/cancel", {}, headers + context 'from the Core' do + before { Travis::Sidekiq::BuildCancellation.stubs(:perform_async) } + + it 'cancels the build' do + Travis::Sidekiq::BuildCancellation.expects(:perform_async).with( id: build.id.to_s, user_id: user.id, source: 'api') + post "/builds/#{build.id}/cancel", {}, headers + end + + it 'responds with 204' do + response = post "/builds/#{build.id}/cancel", {}, headers + response.status.should == 204 + end end - it 'responds with 204' do - response = post "/builds/#{build.id}/cancel", {}, headers - response.status.should == 204 + context 'and enqueues cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } + + before do + build.matrix.each { |j| j.update_attribute(:state, 'created') } + build.update_attribute(:state, 'created') + end + + it 'cancels the build' do + ::Sidekiq::Client.expects(:push).times(4) + post "/builds/#{build.id}/cancel", {}, headers + end + + it 'responds with 204' do + ::Sidekiq::Client.expects(:push).times(4) + response = post "/builds/#{build.id}/cancel", {}, headers + response.status.should == 204 + end end end end From ef7811ead9e898f460df6830bc8e7d30ef4cfbfe Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 16:10:16 +0200 Subject: [PATCH 04/13] Small refacto --- lib/travis/api/app/endpoint/builds.rb | 5 ++--- lib/travis/api/enqueue/services/cancel_model.rb | 9 ++++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 477731ed..57ae056a 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -23,7 +23,7 @@ class Travis::Api::App Metriks.meter("api.request.cancel_build").mark if Travis::Features.owner_active?(:enqueue_to_hub, current_user) - service = Travis::Enqueue::Services::CancelModel.new(current_user, { build_id: params[:id], source: 'api' }) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { build_id: params[:id] }) else service = self.service(:cancel_build, params.merge(source: 'api')) end @@ -47,8 +47,7 @@ class Travis::Api::App respond_with json else if service.respond_to?(:enqueue_to_hub) - payload = { build_id: params[:id], source: 'api'} - service.enqueue_to_hub(payload) + service.enqueue_to_hub else Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') end diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb index d8868a50..fb434c9d 100644 --- a/lib/travis/api/enqueue/services/cancel_model.rb +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -4,12 +4,11 @@ module Travis class CancelModel - attr_reader :current_user, :source, :target + attr_reader :current_user, :target def initialize(current_user, params) @current_user = current_user @params = params - @source = params.delete(:source) || 'unknown' target end @@ -21,16 +20,16 @@ module Travis messages end - def enqueue_to_hub(payload) + def enqueue_to_hub # target may have been retrieved with a :join query, so we need to reset the readonly status if can_cancel? target.send(:instance_variable_set, :@readonly, false) target.cancel! if type == :build - push_matrix(payload) + push_matrix(@params) else - push(payload, target) + push(@params, target) end end end From 29f80ab31ba1d36d242a19d96dce45563c9c7d61 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 16:15:41 +0200 Subject: [PATCH 05/13] Use CancelModel on Jobs endpoint --- lib/travis/api/app/endpoint/jobs.rb | 14 ++++++++++-- spec/integration/v2/jobs_spec.rb | 33 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index e8a0e1e9..02122a61 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -2,6 +2,7 @@ require 'travis/api/app' require 'travis/api/workers/job_cancellation' require 'travis/api/workers/job_restart' require 'travis/api/enqueue/services/restart_model' +require 'travis/api/enqueue/services/cancel_model' class Travis::Api::App class Endpoint @@ -28,7 +29,12 @@ class Travis::Api::App post '/:id/cancel' do Metriks.meter("api.request.cancel_job").mark - service = self.service(:cancel_job, params.merge(source: 'api')) + if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { job_id: params[:id] }) + else + service = self.service(:cancel_job, params.merge(source: 'api')) + end + if !service.authorized? json = { error: { message: "You don't have access to cancel job(#{params[:id]})" @@ -47,7 +53,11 @@ class Travis::Api::App status 422 respond_with json else - Travis::Sidekiq::JobCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + if service.respond_to?(:enqueue_to_hub) + service.enqueue_to_hub + else + Travis::Sidekiq::JobCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + end Metriks.meter("api.request.cancel_job.success").mark status 204 diff --git a/spec/integration/v2/jobs_spec.rb b/spec/integration/v2/jobs_spec.rb index 72dac3d0..6b82a1a7 100644 --- a/spec/integration/v2/jobs_spec.rb +++ b/spec/integration/v2/jobs_spec.rb @@ -230,6 +230,15 @@ describe 'Jobs' do response = post "/jobs/#{job.id}/cancel", {}, headers response.status.should == 403 end + + context 'and tries to enqueue cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, job.repository.owner) } + + it 'responds with 403' do + response = post "/jobs/#{job.id}/cancel", {}, headers + response.status.should == 403 + end + end end context 'when job is not cancelable' do @@ -239,6 +248,15 @@ describe 'Jobs' do response = post "/jobs/#{job.id}/cancel", {}, headers response.status.should == 422 end + + context 'and tries to enqueue cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, job.repository.owner) } + + it 'responds with 422' do + response = post "/jobs/#{job.id}/cancel", {}, headers + response.status.should == 422 + end + end end context 'when job can be canceled' do @@ -255,6 +273,21 @@ describe 'Jobs' do response = post "/jobs/#{job.id}/cancel", {}, headers response.status.should == 204 end + + context 'and enqueues cancel event for the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, job.repository.owner) } + + it 'cancels the job' do + ::Sidekiq::Client.expects(:push) + post "/jobs/#{job.id}/cancel", {}, headers + end + + it 'responds with 204' do + ::Sidekiq::Client.expects(:push) + response = post "/jobs/#{job.id}/cancel", {}, headers + response.status.should == 204 + end + end end end From 00bae7a9452b33f0ec72007babb8937495227478 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 16:41:07 +0200 Subject: [PATCH 06/13] Remove push_matrix method, no need of it --- lib/travis/api/app/endpoint/builds.rb | 4 +-- lib/travis/api/app/endpoint/jobs.rb | 4 +-- .../api/enqueue/services/cancel_model.rb | 31 ++++--------------- spec/integration/v2/builds_spec.rb | 4 +-- 4 files changed, 12 insertions(+), 31 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 57ae056a..8ee7ee71 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -46,8 +46,8 @@ class Travis::Api::App status 422 respond_with json else - if service.respond_to?(:enqueue_to_hub) - service.enqueue_to_hub + if service.respond_to?(:push) + service.push else Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') end diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 02122a61..aa8b9f76 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -53,8 +53,8 @@ class Travis::Api::App status 422 respond_with json else - if service.respond_to?(:enqueue_to_hub) - service.enqueue_to_hub + if service.respond_to?(:push) + service.push else Travis::Sidekiq::JobCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') end diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb index fb434c9d..152187d1 100644 --- a/lib/travis/api/enqueue/services/cancel_model.rb +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -20,35 +20,17 @@ module Travis messages end - def enqueue_to_hub + def push # target may have been retrieved with a :join query, so we need to reset the readonly status if can_cancel? - target.send(:instance_variable_set, :@readonly, false) - target.cancel! - - if type == :build - push_matrix(@params) - else - push(@params, target) - end + ::Sidekiq::Client.push( + 'queue' => 'hub', + 'class' => 'Travis::Hub::Sidekiq::Worker', + 'args' => ["#{type}:cancel", @params] + ) end end - def push_matrix(payload) - target.matrix.each do |job| - push(payload, job) - end - end - - def push(payload, job) - Travis.logger.info("Publishing cancel_job message to worker.commands queue for Job##{job.id}") - ::Sidekiq::Client.push( - 'queue' => 'hub', - 'class' => 'Travis::Hub::Sidekiq::Worker', - 'args' => ["#{type}:cancel", payload] - ) - end - def type @type ||= @params[:build_id] ? :build : :job end @@ -70,7 +52,6 @@ module Travis end end - end end end diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index f3fb77a1..e3f77141 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -122,12 +122,12 @@ describe 'Builds' do end it 'cancels the build' do - ::Sidekiq::Client.expects(:push).times(4) + ::Sidekiq::Client.expects(:push) post "/builds/#{build.id}/cancel", {}, headers end it 'responds with 204' do - ::Sidekiq::Client.expects(:push).times(4) + ::Sidekiq::Client.expects(:push) response = post "/builds/#{build.id}/cancel", {}, headers response.status.should == 204 end From 68daabafa06af32f2758362bf3d03c0b8aaf5d88 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 27 May 2016 17:52:55 +0200 Subject: [PATCH 07/13] Use only params[:id] --- lib/travis/api/app/endpoint/builds.rb | 2 +- lib/travis/api/app/endpoint/jobs.rb | 2 +- lib/travis/api/enqueue/services/cancel_model.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 8ee7ee71..68df723f 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -23,7 +23,7 @@ class Travis::Api::App Metriks.meter("api.request.cancel_build").mark if Travis::Features.owner_active?(:enqueue_to_hub, current_user) - service = Travis::Enqueue::Services::CancelModel.new(current_user, { build_id: params[:id] }) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { id: params[:id], type: :build }) else service = self.service(:cancel_build, params.merge(source: 'api')) end diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index aa8b9f76..cd5cc300 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -30,7 +30,7 @@ class Travis::Api::App Metriks.meter("api.request.cancel_job").mark if Travis::Features.owner_active?(:enqueue_to_hub, current_user) - service = Travis::Enqueue::Services::CancelModel.new(current_user, { job_id: params[:id] }) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { id: params[:id], type: :job }) else service = self.service(:cancel_job, params.merge(source: 'api')) end diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb index 152187d1..1b2e722c 100644 --- a/lib/travis/api/enqueue/services/cancel_model.rb +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -32,14 +32,14 @@ module Travis end def type - @type ||= @params[:build_id] ? :build : :job + @params[:type] end def target if type == :build - @target = Build.find(@params[:build_id]) + @target = Build.find(@params[:id]) else - @target = Job.find(@params[:job_id]) + @target = Job.find(@params[:id]) end end From 75460b242768febd0801900346419400399d9a61 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Mon, 13 Jun 2016 18:13:21 +0200 Subject: [PATCH 08/13] Use Travis::Enqueue::Services::CancelModel on v3 build cancel endpoint --- lib/travis/api/v3/queries/build.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/travis/api/v3/queries/build.rb b/lib/travis/api/v3/queries/build.rb index 5314a7c9..efa90e6f 100644 --- a/lib/travis/api/v3/queries/build.rb +++ b/lib/travis/api/v3/queries/build.rb @@ -1,3 +1,6 @@ +require 'travis/api/enqueue/services/restart_model' +require 'travis/api/enqueue/services/cancel_model' + module Travis::API::V3 class Queries::Build < Query params :id @@ -10,7 +13,13 @@ module Travis::API::V3 def cancel(user) raise BuildNotCancelable if %w(passed failed canceled errored).include? find.state payload = { id: id, user_id: user.id, source: 'api' } - perform_async(:build_cancellation, payload) + if Travis::Features.owner_active?(:enqueue_to_hub, user) + payload[:type] = :build + service = Travis::Enqueue::Services::CancelModel.new(user, payload) + service.push + else + perform_async(:build_cancellation, payload) + end payload end From e097c8889e1484098ec9b475471dc5574835ab2a Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Tue, 14 Jun 2016 15:15:59 +0200 Subject: [PATCH 09/13] Enqueue cancel message on build cancel v3 endpoint --- lib/travis/api/v3/models/permission.rb | 19 +++++ lib/travis/api/v3/models/user.rb | 8 ++ spec/v3/services/build/cancel_spec.rb | 106 +++++++++++++++++++++++++ 3 files changed, 133 insertions(+) diff --git a/lib/travis/api/v3/models/permission.rb b/lib/travis/api/v3/models/permission.rb index 7c592903..77683c9c 100644 --- a/lib/travis/api/v3/models/permission.rb +++ b/lib/travis/api/v3/models/permission.rb @@ -1,6 +1,25 @@ module Travis::API::V3 class Models::Permission < Model + ROLES = %w(admin push pull) + belongs_to :user belongs_to :repository + + class << self + def by_roles(roles) + roles = Array(roles).select { |role| ROLES.include?(role.to_s) } + roles.empty? ? none : where(has_roles(roles)) + end + + def has_roles(roles) + roles.inject(has_role(roles.shift)) do |sql, role| + sql.or(has_role(role)) + end + end + + def has_role(role) + arel_table[role].eq(true) + end + end end end diff --git a/lib/travis/api/v3/models/user.rb b/lib/travis/api/v3/models/user.rb index d41fee33..510542c8 100644 --- a/lib/travis/api/v3/models/user.rb +++ b/lib/travis/api/v3/models/user.rb @@ -22,5 +22,13 @@ module Travis::API::V3 def starred_repository_ids @starred_repository_ids ||= stars.map(&:repository_id) end + + def permission?(roles, options = {}) + roles, options = nil, roles if roles.is_a?(Hash) + scope = permissions.where(options) + scope = scope.by_roles(roles) if roles + scope.any? + end + end end diff --git a/spec/v3/services/build/cancel_spec.rb b/spec/v3/services/build/cancel_spec.rb index 90e9a107..836d98d5 100644 --- a/spec/v3/services/build/cancel_spec.rb +++ b/spec/v3/services/build/cancel_spec.rb @@ -8,6 +8,7 @@ describe Travis::API::V3::Services::Build::Cancel do before do Travis::Features.stubs(:owner_active?).returns(true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(false) @original_sidekiq = Sidekiq::Client Sidekiq.send(:remove_const, :Client) # to avoid a warning Sidekiq::Client = [] @@ -227,6 +228,111 @@ describe Travis::API::V3::Services::Build::Cancel do end end + describe "existing repository, push & pull access, cancelable, enqueues message for Hub" do + let(:params) {{}} + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:headers) {{ 'HTTP_AUTHORIZATION' => "token #{token}" }} + before do + Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true, pull: true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(true) + end + + describe "started state" do + before { build.update_attribute(:state, "started") } + before { post("/v3/build/#{build.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "build", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id, + "source" => "api", + "type" => "build"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "queued state" do + before { build.update_attribute(:state, "queued") } + before { post("/v3/build/#{build.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "build", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id, + "source" => "api", + "type" => "build"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "received state" do + before { build.update_attribute(:state, "received") } + before { post("/v3/build/#{build.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "build", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id, + "source" => "api", + "type" => "build"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "setting id has no effect" do + let(:params) {{ id: 42 }} + before { post("/v3/build/#{build.id}/cancel", params, headers) } + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id, + "source" => "api", + "type" => "build"} + } + end + end + + + + # TODO decided to discuss further with rkh as this use case doesn't really exist at the moment # and 'fixing' the query requires modifying workers that v2 uses, thereby running the risk of breaking v2, # and also because in 6 months or so travis-hub will be able to cancel builds without using travis-core at all. From bb7260bc9ee7a5712daea06d4af8a21f006cf3e3 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Tue, 14 Jun 2016 16:46:29 +0200 Subject: [PATCH 10/13] Enqueue restart message on build restart v3 endpoint --- lib/travis/api/v3/queries/build.rb | 10 +- spec/v3/services/build/restart_spec.rb | 128 +++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/lib/travis/api/v3/queries/build.rb b/lib/travis/api/v3/queries/build.rb index efa90e6f..f9926f75 100644 --- a/lib/travis/api/v3/queries/build.rb +++ b/lib/travis/api/v3/queries/build.rb @@ -25,8 +25,14 @@ module Travis::API::V3 def restart(user) raise BuildAlreadyRunning if %w(received queued started).include? find.state - payload = { id: id, user_id: user.id, source: 'api' } - perform_async(:build_restart, payload) + if Travis::Features.owner_active?(:enqueue_to_hub, user) + service = Travis::Enqueue::Services::RestartModel.new(user, { build_id: id }) + payload = { id: id, user_id: user.id } + service.push("build:restart", payload) + else + payload = { id: id, user_id: user.id, source: 'api' } + perform_async(:build_restart, payload) + end payload end end diff --git a/spec/v3/services/build/restart_spec.rb b/spec/v3/services/build/restart_spec.rb index 121c9f9b..048f2307 100644 --- a/spec/v3/services/build/restart_spec.rb +++ b/spec/v3/services/build/restart_spec.rb @@ -8,6 +8,7 @@ describe Travis::API::V3::Services::Build::Restart do before do Travis::Features.stubs(:owner_active?).returns(true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(false) @original_sidekiq = Sidekiq::Client Sidekiq.send(:remove_const, :Client) # to avoid a warning Sidekiq::Client = [] @@ -245,6 +246,133 @@ describe Travis::API::V3::Services::Build::Restart do end end + describe "existing repository, push access, build not already running, enqueues message for Hub" do + let(:params) {{}} + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:headers) {{ 'HTTP_AUTHORIZATION' => "token #{token}" }} + before do + Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(true) + end + + describe "errored state" do + before do + build.update_attribute(:state, "errored") + post("/v3/build/#{build.id}/restart", params, headers) + end + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "build", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "passed state" do + before { build.update_attribute(:state, "passed") } + before { post("/v3/build/#{build.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "build", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "failed state" do + before { build.update_attribute(:state, "failed") } + before { post("/v3/build/#{build.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "build", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "canceled state" do + before { build.update_attribute(:state, "canceled") } + before { post("/v3/build/#{build.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "build", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "setting id has no effect" do + before { build.update_attribute(:state, "canceled") } + before { post("/v3/build/#{build.id}/restart", params, headers) } + let(:params) {{ id: 42 }} + + example { expect(sidekiq_payload).to be == { + "id" => "#{build.id}", + "user_id"=> repo.owner_id} + } + end + end + # TODO decided to discuss further with rkh as this use case doesn't really exist at the moment # and 'fixing' the query requires modifying workers that v2 uses, thereby running the risk of breaking v2, # and also because in 6 months or so travis-hub will be able to cancel builds without using travis-core at all. From a2d1322b6003266b298b93d66632f51bf9b25aa2 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Tue, 14 Jun 2016 17:24:05 +0200 Subject: [PATCH 11/13] Enqueue cancel message on job/cancel v3 endpoint --- lib/travis/api/v3/queries/job.rb | 10 ++- spec/v3/services/job/cancel_spec.rb | 102 ++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/lib/travis/api/v3/queries/job.rb b/lib/travis/api/v3/queries/job.rb index ba824d4d..e6b0b7d2 100644 --- a/lib/travis/api/v3/queries/job.rb +++ b/lib/travis/api/v3/queries/job.rb @@ -1,3 +1,6 @@ +require 'travis/api/enqueue/services/restart_model' +require 'travis/api/enqueue/services/cancel_model' + module Travis::API::V3 class Queries::Job < Query params :id @@ -10,7 +13,12 @@ module Travis::API::V3 def cancel(user) raise JobNotCancelable if %w(passed failed canceled errored).include? find.state payload = { id: id, user_id: user.id, source: 'api' } - perform_async(:job_cancellation, payload) + if Travis::Features.owner_active?(:enqueue_to_hub, user) + service = Travis::Enqueue::Services::CancelModel.new(user, payload) + service.push + else + perform_async(:job_cancellation, payload) + end payload end diff --git a/spec/v3/services/job/cancel_spec.rb b/spec/v3/services/job/cancel_spec.rb index 48faac52..b429cfa6 100644 --- a/spec/v3/services/job/cancel_spec.rb +++ b/spec/v3/services/job/cancel_spec.rb @@ -9,6 +9,7 @@ describe Travis::API::V3::Services::Job::Cancel do before do Travis::Features.stubs(:owner_active?).returns(true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(false) @original_sidekiq = Sidekiq::Client Sidekiq.send(:remove_const, :Client) # to avoid a warning Sidekiq::Client = [] @@ -174,6 +175,107 @@ describe Travis::API::V3::Services::Job::Cancel do } end end + + describe "existing repository, push access, job cancelable, enqueues message for Hub" do + let(:params) {{}} + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:headers) {{ 'HTTP_AUTHORIZATION' => "token #{token}" }} + before do + Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true, pull: true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(true) + end + + describe "started state" do + before { job.update_attribute(:state, "started") } + before { post("/v3/job/#{job.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id, + "source" => "api"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + describe "queued state" do + before { job.update_attribute(:state, "queued") } + before { post("/v3/job/#{job.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id, + "source" => "api"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "received state" do + before { job.update_attribute(:state, "received") } + before { post("/v3/job/#{job.id}/cancel", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "cancel", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id, + "source" => "api"} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "setting id has no effect" do + before { post("/v3/job/#{job.id}/cancel", params, headers) } + let(:params) {{ id: 42 }} + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id, + "source" => "api"} + } + end + end + describe "existing repository, push access, not cancelable" do let(:params) {{}} let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } From 5949429f75abd88c51cca00fb76ac7d5a455f06b Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Tue, 14 Jun 2016 17:39:29 +0200 Subject: [PATCH 12/13] Enqueue restart message on job/restart v3 endpoint --- lib/travis/api/v3/queries/job.rb | 11 ++- spec/v3/services/job/restart_spec.rb | 125 +++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/lib/travis/api/v3/queries/job.rb b/lib/travis/api/v3/queries/job.rb index e6b0b7d2..606cb6dc 100644 --- a/lib/travis/api/v3/queries/job.rb +++ b/lib/travis/api/v3/queries/job.rb @@ -24,8 +24,15 @@ module Travis::API::V3 def restart(user) raise JobAlreadyRunning if %w(received queued started).include? find.state - payload = { id: id, user_id: user.id, source: 'api' } - perform_async(:job_restart, payload) + + if Travis::Features.owner_active?(:enqueue_to_hub, user) + service = Travis::Enqueue::Services::RestartModel.new(user, { job_id: id }) + payload = { id: id, user_id: user.id } + service.push("job:restart", payload) + else + payload = { id: id, user_id: user.id, source: 'api' } + perform_async(:job_restart, payload) + end payload end end diff --git a/spec/v3/services/job/restart_spec.rb b/spec/v3/services/job/restart_spec.rb index a07f0b82..fde730f6 100644 --- a/spec/v3/services/job/restart_spec.rb +++ b/spec/v3/services/job/restart_spec.rb @@ -9,6 +9,7 @@ describe Travis::API::V3::Services::Job::Restart do before do Travis::Features.stubs(:owner_active?).returns(true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(false) @original_sidekiq = Sidekiq::Client Sidekiq.send(:remove_const, :Client) # to avoid a warning Sidekiq::Client = [] @@ -200,6 +201,130 @@ describe Travis::API::V3::Services::Job::Restart do end end + describe "existing repository, push access, job not already running, enqueues message for Hub" do + let(:params) {{}} + let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } + let(:headers) {{ 'HTTP_AUTHORIZATION' => "token #{token}" }} + before do + Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true) + Travis::Features.stubs(:owner_active?).with(:enqueue_to_hub, repo.owner).returns(true) + end + + describe "canceled state" do + before { job.update_attribute(:state, "canceled") } + before { post("/v3/job/#{job.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + describe "errored state" do + before { job.update_attribute(:state, "errored") } + before { post("/v3/job/#{job.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + describe "failed state" do + before { job.update_attribute(:state, "failed") } + before { post("/v3/job/#{job.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + describe "passed state" do + before { job.update_attribute(:state, "passed") } + before { post("/v3/job/#{job.id}/restart", params, headers) } + + example { expect(last_response.status).to be == 202 } + example { expect(JSON.load(body).to_s).to include( + "@type", + "pending", + "job", + "@href", + "@representation", + "minimal", + "restart", + "id", + "state_change") + } + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id} + } + + example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } + example { expect(Sidekiq::Client.last['class']).to be == 'Travis::Hub::Sidekiq::Worker' } + end + + describe "setting id has no effect" do + before do + job.update_attribute(:state, "passed") + post("/v3/job/#{job.id}/restart", params, headers) + end + let(:params) {{ id: 42 }} + + example { expect(sidekiq_payload).to be == { + "id" => "#{job.id}", + "user_id"=> repo.owner_id} + } + end + end + describe "existing repository, push access, job already running" do let(:params) {{}} let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } From 41c274b222c3f44df8ae6d9eb82cd11824f117d5 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Wed, 15 Jun 2016 14:36:59 +0200 Subject: [PATCH 13/13] Remove 'type' from payload --- lib/travis/api/app/endpoint/builds.rb | 7 ++++--- lib/travis/api/app/endpoint/jobs.rb | 7 ++++--- lib/travis/api/enqueue/services/cancel_model.rb | 12 +++++++----- lib/travis/api/v3/queries/build.rb | 5 ++--- lib/travis/api/v3/queries/job.rb | 4 ++-- spec/v3/services/build/cancel_spec.rb | 12 ++++-------- 6 files changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 36d95293..b21a7cd6 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -23,7 +23,7 @@ class Travis::Api::App Metriks.meter("api.request.cancel_build").mark if Travis::Features.owner_active?(:enqueue_to_hub, current_user) - service = Travis::Enqueue::Services::CancelModel.new(current_user, { id: params[:id], type: :build }) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { build_id: params[:id] }) else service = self.service(:cancel_build, params.merge(source: 'api')) end @@ -46,10 +46,11 @@ class Travis::Api::App status 422 respond_with json else + payload = { id: params[:id], user_id: current_user.id, source: 'api' } if service.respond_to?(:push) - service.push + service.push("build:cancel", payload) else - Travis::Sidekiq::BuildCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + Travis::Sidekiq::BuildCancellation.perform_async(payload) end Metriks.meter("api.request.cancel_build.success").mark diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 74b053e3..2f1fd0ed 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -30,7 +30,7 @@ class Travis::Api::App Metriks.meter("api.request.cancel_job").mark if Travis::Features.owner_active?(:enqueue_to_hub, current_user) - service = Travis::Enqueue::Services::CancelModel.new(current_user, { id: params[:id], type: :job }) + service = Travis::Enqueue::Services::CancelModel.new(current_user, { job_id: params[:id] }) else service = self.service(:cancel_job, params.merge(source: 'api')) end @@ -53,10 +53,11 @@ class Travis::Api::App status 422 respond_with json else + payload = { id: params[:id], user_id: current_user.id, source: 'api' } if service.respond_to?(:push) - service.push + service.push("job:cancel", payload) else - Travis::Sidekiq::JobCancellation.perform_async(id: params[:id], user_id: current_user.id, source: 'api') + Travis::Sidekiq::JobCancellation.perform_async(payload) end Metriks.meter("api.request.cancel_job.success").mark diff --git a/lib/travis/api/enqueue/services/cancel_model.rb b/lib/travis/api/enqueue/services/cancel_model.rb index 1b2e722c..bcc3f741 100644 --- a/lib/travis/api/enqueue/services/cancel_model.rb +++ b/lib/travis/api/enqueue/services/cancel_model.rb @@ -20,26 +20,27 @@ module Travis messages end - def push + def push(event, payload) # target may have been retrieved with a :join query, so we need to reset the readonly status if can_cancel? ::Sidekiq::Client.push( 'queue' => 'hub', 'class' => 'Travis::Hub::Sidekiq::Worker', - 'args' => ["#{type}:cancel", @params] + #'args' => ["#{type}:cancel", @params] + 'args' => [event, payload] ) end end def type - @params[:type] + @type ||= @params[:build_id] ? :build : :job end def target if type == :build - @target = Build.find(@params[:id]) + @target = Build.find(@params[:build_id]) else - @target = Job.find(@params[:id]) + @target = Job.find(@params[:job_id]) end end @@ -47,6 +48,7 @@ module Travis authorized? && target.cancelable? end + # check on web def authorized? current_user.permission?(:pull, :repository_id => target.repository_id) end diff --git a/lib/travis/api/v3/queries/build.rb b/lib/travis/api/v3/queries/build.rb index f9926f75..ca391e76 100644 --- a/lib/travis/api/v3/queries/build.rb +++ b/lib/travis/api/v3/queries/build.rb @@ -14,9 +14,8 @@ module Travis::API::V3 raise BuildNotCancelable if %w(passed failed canceled errored).include? find.state payload = { id: id, user_id: user.id, source: 'api' } if Travis::Features.owner_active?(:enqueue_to_hub, user) - payload[:type] = :build - service = Travis::Enqueue::Services::CancelModel.new(user, payload) - service.push + service = Travis::Enqueue::Services::CancelModel.new(user, { build_id: id }) + service.push("build:cancel", payload) else perform_async(:build_cancellation, payload) end diff --git a/lib/travis/api/v3/queries/job.rb b/lib/travis/api/v3/queries/job.rb index 606cb6dc..52ee29bd 100644 --- a/lib/travis/api/v3/queries/job.rb +++ b/lib/travis/api/v3/queries/job.rb @@ -14,8 +14,8 @@ module Travis::API::V3 raise JobNotCancelable if %w(passed failed canceled errored).include? find.state payload = { id: id, user_id: user.id, source: 'api' } if Travis::Features.owner_active?(:enqueue_to_hub, user) - service = Travis::Enqueue::Services::CancelModel.new(user, payload) - service.push + service = Travis::Enqueue::Services::CancelModel.new(user, { job_id: id }) + service.push("job:cancel", payload) else perform_async(:job_cancellation, payload) end diff --git a/spec/v3/services/build/cancel_spec.rb b/spec/v3/services/build/cancel_spec.rb index 836d98d5..00c63c54 100644 --- a/spec/v3/services/build/cancel_spec.rb +++ b/spec/v3/services/build/cancel_spec.rb @@ -256,8 +256,7 @@ describe Travis::API::V3::Services::Build::Cancel do example { expect(sidekiq_payload).to be == { "id" => "#{build.id}", "user_id"=> repo.owner_id, - "source" => "api", - "type" => "build"} + "source" => "api"} } example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } @@ -283,8 +282,7 @@ describe Travis::API::V3::Services::Build::Cancel do example { expect(sidekiq_payload).to be == { "id" => "#{build.id}", "user_id"=> repo.owner_id, - "source" => "api", - "type" => "build"} + "source" => "api"} } example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } @@ -310,8 +308,7 @@ describe Travis::API::V3::Services::Build::Cancel do example { expect(sidekiq_payload).to be == { "id" => "#{build.id}", "user_id"=> repo.owner_id, - "source" => "api", - "type" => "build"} + "source" => "api"} } example { expect(Sidekiq::Client.last['queue']).to be == 'hub' } @@ -324,8 +321,7 @@ describe Travis::API::V3::Services::Build::Cancel do example { expect(sidekiq_payload).to be == { "id" => "#{build.id}", "user_id"=> repo.owner_id, - "source" => "api", - "type" => "build"} + "source" => "api"} } end end