From 28884f9931bd5644167116b5be32af3b947935e2 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 13 May 2016 12:59:18 -0500 Subject: [PATCH 1/5] Enqueue build_restart event in Hub --- lib/travis/api/app/endpoint/builds.rb | 8 +++++++- lib/travis/api/workers/build_restart.rb | 13 +++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 91d375dc..37d4b317 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -54,7 +54,13 @@ class Travis::Api::App status 400 result = false else - Travis::Sidekiq::BuildRestart.perform_async(id: params[:id], user_id: current_user.id) + + ::Sidekiq::Client.push( + 'queue' => 'hub', + 'class' => 'Travis::Hub::Sidekiq::Worker', + 'args' => ["build:restart", {id: params[:id], user_id: current_user.id}] + ) + status 202 result = true end diff --git a/lib/travis/api/workers/build_restart.rb b/lib/travis/api/workers/build_restart.rb index 427bc24b..2bf98c85 100644 --- a/lib/travis/api/workers/build_restart.rb +++ b/lib/travis/api/workers/build_restart.rb @@ -4,14 +4,15 @@ require 'multi_json' module Travis module Sidekiq class BuildRestart - class ProcessingError < StandardError; end - include ::Sidekiq::Worker - sidekiq_options queue: :build_restarts + sidekiq_options queue: :hub - def perform(data) - user = User.find(data['user_id']) - Travis.service(:reset_model, user, build_id: data['id']).run + def perform(payload) + ::Sidekiq::Client.push( + 'queue' => 'hub', + 'class' => 'Travis::Hub::Sidekiq::Worker', + 'args' => ["build:restart", payload] + ) end end From 9bbfb7ec97483c56bec56ea6e02ef0d9dd5706e5 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Wed, 18 May 2016 14:45:42 -0500 Subject: [PATCH 2/5] Add an equeue service --- lib/travis/api/app/endpoint/builds.rb | 10 +++------- .../api/enqueue/services/enqueue_build.rb | 18 ++++++++++++++++++ lib/travis/api/workers/build_restart.rb | 15 +++++++-------- lib/travis/sidekiq.rb | 1 - spec/integration/v2/builds_spec.rb | 4 ++-- 5 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 lib/travis/api/enqueue/services/enqueue_build.rb diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 37d4b317..51197af2 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -1,6 +1,6 @@ require 'travis/api/app' require 'travis/api/workers/build_cancellation' -require 'travis/api/workers/build_restart' +require 'travis/api/enqueue/services/enqueue_build' class Travis::Api::App class Endpoint @@ -54,12 +54,8 @@ class Travis::Api::App status 400 result = false else - - ::Sidekiq::Client.push( - 'queue' => 'hub', - 'class' => 'Travis::Hub::Sidekiq::Worker', - 'args' => ["build:restart", {id: params[:id], user_id: current_user.id}] - ) + payload = {id: params[:id], user_id: current_user.id} + Travis::Enqueue::Services::EnqueueBuild.push("build:restart", payload) status 202 result = true diff --git a/lib/travis/api/enqueue/services/enqueue_build.rb b/lib/travis/api/enqueue/services/enqueue_build.rb new file mode 100644 index 00000000..2601cb1b --- /dev/null +++ b/lib/travis/api/enqueue/services/enqueue_build.rb @@ -0,0 +1,18 @@ +module Travis + module Enqueue + module Services + + class EnqueueBuild + + def self.push(event, payload) + ::Sidekiq::Client.push( + 'queue' => 'hub', + 'class' => 'Travis::Hub::Sidekiq::Worker', + 'args' => [event, payload] + ) + end + end + + end + end +end diff --git a/lib/travis/api/workers/build_restart.rb b/lib/travis/api/workers/build_restart.rb index 2bf98c85..427bc24b 100644 --- a/lib/travis/api/workers/build_restart.rb +++ b/lib/travis/api/workers/build_restart.rb @@ -4,15 +4,14 @@ require 'multi_json' module Travis module Sidekiq class BuildRestart - include ::Sidekiq::Worker - sidekiq_options queue: :hub + class ProcessingError < StandardError; end - def perform(payload) - ::Sidekiq::Client.push( - 'queue' => 'hub', - 'class' => 'Travis::Hub::Sidekiq::Worker', - 'args' => ["build:restart", payload] - ) + include ::Sidekiq::Worker + sidekiq_options queue: :build_restarts + + def perform(data) + user = User.find(data['user_id']) + Travis.service(:reset_model, user, build_id: data['id']).run end end diff --git a/lib/travis/sidekiq.rb b/lib/travis/sidekiq.rb index 077e6055..e1f9b5d2 100644 --- a/lib/travis/sidekiq.rb +++ b/lib/travis/sidekiq.rb @@ -2,7 +2,6 @@ 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' diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index e36aeeb2..517d2623 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -119,13 +119,13 @@ describe 'Builds' do end it 'restarts the build' do - Travis::Sidekiq::BuildRestart.expects(:perform_async).with(id: build.id.to_s, user_id: user.id) + Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {id: build.id.to_s, user_id: user.id}) response = post "/builds/#{build.id}/restart", {}, headers response.status.should == 202 end it 'sends the correct response body' do - Travis::Sidekiq::BuildRestart.expects(:perform_async).with(id: build.id.to_s, user_id: user.id) + Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {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."}]} From 9bd145856b1ac15ee0351906842cb0d20e457bb6 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Thu, 19 May 2016 14:21:49 -0500 Subject: [PATCH 3/5] Add feature flag to enqueue restart build to Hub --- Gemfile.lock | 3 --- lib/travis/api/app/endpoint/builds.rb | 9 +++++-- lib/travis/sidekiq.rb | 1 + spec/integration/v2/builds_spec.rb | 37 ++++++++++++++++++++------- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 1fd8a971..064187b1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -426,6 +426,3 @@ DEPENDENCIES travis-yaml! unicorn yard-sinatra! - -BUNDLED WITH - 1.11.2 diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 51197af2..1efc0f90 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -1,5 +1,6 @@ require 'travis/api/app' require 'travis/api/workers/build_cancellation' +require 'travis/api/workers/build_restart' require 'travis/api/enqueue/services/enqueue_build' class Travis::Api::App @@ -54,8 +55,12 @@ class Travis::Api::App status 400 result = false else - payload = {id: params[:id], user_id: current_user.id} - Travis::Enqueue::Services::EnqueueBuild.push("build:restart", payload) + if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + payload = {id: params[:id], user_id: current_user.id} + Travis::Enqueue::Services::EnqueueBuild.push("build:restart", payload) + else + Travis::Sidekiq::BuildRestart.perform_async(id: params[:id], user_id: current_user.id) + end status 202 result = true diff --git a/lib/travis/sidekiq.rb b/lib/travis/sidekiq.rb index e1f9b5d2..077e6055 100644 --- a/lib/travis/sidekiq.rb +++ b/lib/travis/sidekiq.rb @@ -2,6 +2,7 @@ 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' diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index 517d2623..4585d4ec 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -118,17 +118,36 @@ describe 'Builds' do build.update_attribute(:state, 'passed') end - it 'restarts the build' do - Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {id: build.id.to_s, user_id: user.id}) - response = post "/builds/#{build.id}/restart", {}, headers - response.status.should == 202 + describe 'Enqueues restart event to the Hub' do + before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } + + it 'restarts the build' do + Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {id: build.id.to_s, user_id: user.id}) + response = post "/builds/#{build.id}/restart", {}, headers + response.status.should == 202 + end + + it 'sends the correct response body' do + Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {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 - it 'sends the correct response body' do - Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {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."}]} + describe 'Restart from the Core' do + 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 == 202 + 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 From 2b37d90ffba17642a80dde0bdb8cd08a5ec1f4f1 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Fri, 20 May 2016 12:31:50 -0500 Subject: [PATCH 4/5] Add permission methods to enqueue_build --- lib/travis/api/app/endpoint/builds.rb | 28 +++++++++------ .../api/enqueue/services/enqueue_build.rb | 35 +++++++++++++++++-- spec/integration/v2/builds_spec.rb | 4 +-- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index 1efc0f90..e5766bb5 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -50,21 +50,29 @@ class Travis::Api::App post '/:id/restart' do Metriks.meter("api.request.restart_build").mark - service = self.service(:reset_model, build_id: params[:id]) - if !service.accept? - status 400 - result = false - else - if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + if Travis::Features.owner_active?(:enqueue_to_hub, current_user) + service = Travis::Enqueue::Services::EnqueueBuild.new(current_user, params[:id]) + if !service.accept? + status 400 + result = false + else payload = {id: params[:id], user_id: current_user.id} - Travis::Enqueue::Services::EnqueueBuild.push("build:restart", payload) + service.push("build:restart", payload) + status 202 + result = true + end + else + 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 202 + result = true end - - status 202 - result = true end + respond_with(result: result, flash: service.messages) end end diff --git a/lib/travis/api/enqueue/services/enqueue_build.rb b/lib/travis/api/enqueue/services/enqueue_build.rb index 2601cb1b..8776a5e6 100644 --- a/lib/travis/api/enqueue/services/enqueue_build.rb +++ b/lib/travis/api/enqueue/services/enqueue_build.rb @@ -3,16 +3,47 @@ module Travis module Services class EnqueueBuild + attr_reader :current_user, :build - def self.push(event, payload) + def initialize(current_user, build_id) + @current_user = current_user + @build = Build.find(build_id) + end + + def push(event, payload) ::Sidekiq::Client.push( 'queue' => 'hub', 'class' => 'Travis::Hub::Sidekiq::Worker', 'args' => [event, payload] ) end - end + def accept? + current_user && permission? && resetable? + end + + def messages + messages = [] + messages << { notice: "The build was successfully restarted." } if accept? + messages << { error: 'You do not seem to have sufficient permissions.' } unless permission? + messages << { error: "This build currently can not be restarted." } unless resetable? + messages + end + + private + + def permission? + current_user.permission?(required_role, repository_id: build.repository_id) + end + + def resetable? + build.resetable? + end + + def required_role + Travis.config.roles.reset_model + end + end end end end diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index 4585d4ec..8db3a0b0 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -122,13 +122,13 @@ describe 'Builds' do before { Travis::Features.activate_owner(:enqueue_to_hub, repo.owner) } it 'restarts the build' do - Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {id: build.id.to_s, user_id: user.id}) + ::Sidekiq::Client.expects(:push) response = post "/builds/#{build.id}/restart", {}, headers response.status.should == 202 end it 'sends the correct response body' do - Travis::Enqueue::Services::EnqueueBuild.expects(:push).with("build:restart", {id: build.id.to_s, user_id: user.id}) + ::Sidekiq::Client.expects(:push) response = post "/builds/#{build.id}/restart", {}, headers body = JSON.parse(response.body) body.should == {"result"=>true, "flash"=>[{"notice"=>"The build was successfully restarted."}]} From 3c253bb88c3a0f1b5946ead1b8ee9c4f89d37245 Mon Sep 17 00:00:00 2001 From: Ana Rosas Date: Mon, 23 May 2016 14:38:38 -0500 Subject: [PATCH 5/5] Move stub to before block on restart test --- lib/travis/api/app/endpoint/builds.rb | 1 - spec/integration/v2/builds_spec.rb | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index e5766bb5..ffa6b90a 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/lib/travis/api/app/endpoint/builds.rb @@ -49,7 +49,6 @@ class Travis::Api::App post '/:id/restart' do Metriks.meter("api.request.restart_build").mark - if Travis::Features.owner_active?(:enqueue_to_hub, current_user) service = Travis::Enqueue::Services::EnqueueBuild.new(current_user, params[:id]) if !service.accept? diff --git a/spec/integration/v2/builds_spec.rb b/spec/integration/v2/builds_spec.rb index 8db3a0b0..98a57e26 100644 --- a/spec/integration/v2/builds_spec.rb +++ b/spec/integration/v2/builds_spec.rb @@ -113,7 +113,6 @@ describe 'Builds' do 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 @@ -136,6 +135,8 @@ describe 'Builds' do end describe 'Restart from the Core' do + before { Travis::Sidekiq::BuildRestart.stubs(:perform_async) } + 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