From 0b2585de45d7ac4e785c895ca9de479ba6894269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steffen=20Ko=CC=88tte?= Date: Tue, 12 Apr 2016 11:33:30 +0200 Subject: [PATCH] remove cronjobs and disallow creating if branch does no longer exist on GitHub --- lib/travis/api/v3/queries/crons.rb | 15 ++++++++----- lib/travis/api/v3/services/cron/create.rb | 1 + spec/v3/queries/cron_spec.rb | 27 +++++++++++++++++++++++ spec/v3/services/cron/create_spec.rb | 11 +++++++++ 4 files changed, 48 insertions(+), 6 deletions(-) create mode 100644 spec/v3/queries/cron_spec.rb diff --git a/lib/travis/api/v3/queries/crons.rb b/lib/travis/api/v3/queries/crons.rb index 9a9249fc..e702ca1f 100644 --- a/lib/travis/api/v3/queries/crons.rb +++ b/lib/travis/api/v3/queries/crons.rb @@ -6,22 +6,25 @@ module Travis::API::V3 end def start_all() - started = [] + started_crons = [] Models::Cron.all.each do |cron| if cron.next_enqueuing <= Time.now - start(cron) - started.push cron + started = start(cron) + started_crons.push cron if started end end - started + started_crons end def start(cron) branch = cron.branch raise ServerError, 'repository does not have a github_id'.freeze unless branch.repository.github_id - access_control.permissions(cron).start! + unless branch.exists_on_github + cron.destroy + return false + end user_id = branch.repository.users.detect { |u| u.github_oauth_token }.id @@ -33,7 +36,7 @@ module Travis::API::V3 class_name, queue = Query.sidekiq_queue(:build_request) ::Sidekiq::Client.push('queue'.freeze => queue, 'class'.freeze => class_name, 'args'.freeze => [{type: 'cron'.freeze, payload: JSON.dump(payload), credentials: {}}]) - payload + true end end end diff --git a/lib/travis/api/v3/services/cron/create.rb b/lib/travis/api/v3/services/cron/create.rb index 3f3c24ec..dcbfffb8 100644 --- a/lib/travis/api/v3/services/cron/create.rb +++ b/lib/travis/api/v3/services/cron/create.rb @@ -7,6 +7,7 @@ module Travis::API::V3 raise LoginRequired unless access_control.logged_in? or access_control.full_access? raise NotFound unless repository = find(:repository) raise NotFound unless branch = find(:branch, repository) + raise Error.new('Crons can only be set up for branches existing on GitHub!', status: 422) unless branch.exists_on_github raise Error.new('Invalid value for interval. Interval must be "daily", "weekly" or "monthly"!', status: 422) unless ["daily", "weekly", "monthly"].include?(params["interval"]) access_control.permissions(repository).create_cron! access_control.permissions(branch.cron).delete! if branch.cron diff --git a/spec/v3/queries/cron_spec.rb b/spec/v3/queries/cron_spec.rb new file mode 100644 index 00000000..bb4c559a --- /dev/null +++ b/spec/v3/queries/cron_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Travis::API::V3::Queries::Crons do + let(:user) { Travis::API::V3::Models::User.find_by_login('svenfuchs') } + let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first } + let(:existing_branch) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test-existing', exists_on_github: true) } + let(:non_existing_branch) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test-non-existing', exists_on_github: false) } + let(:query) { Travis::API::V3::Queries::Crons.new({}, 'Overview') +} + + describe "start all" do + before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true) } + + it "starts crons on existing branches" do + cron = Travis::API::V3::Models::Cron.create(branch_id: existing_branch.id, interval: 'daily', disable_by_build: false) + expect(query.start_all).to include(cron) + + end + + it "delete crons on branches not existing on GitHub" do + cron = Travis::API::V3::Models::Cron.create(branch_id: non_existing_branch.id, interval: 'daily', disable_by_build: false) + expect(query.start_all).to_not include(cron) + expect(Travis::API::V3::Models::Cron.where(id: cron.id).length).to equal(0) + end + end + +end diff --git a/spec/v3/services/cron/create_spec.rb b/spec/v3/services/cron/create_spec.rb index 467f6bf1..dc578e80 100644 --- a/spec/v3/services/cron/create_spec.rb +++ b/spec/v3/services/cron/create_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe Travis::API::V3::Services::Cron::Create do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first } let(:branch) { Travis::API::V3::Models::Branch.where(repository_id: repo).first } + let(:non_existing_branch) { Travis::API::V3::Models::Branch.create(repository: repo, name: 'cron-test', exists_on_github: false) } let(:last_cron) {Travis::API::V3::Models::Cron.where(branch_id: branch.id).last} let(:current_cron) {Travis::API::V3::Models::Cron.where(branch_id: branch.id).last} let(:token) { Travis::Api::App::AccessToken.create(user: repo.owner, app_id: 1) } @@ -87,6 +88,16 @@ describe Travis::API::V3::Services::Cron::Create do }} end + describe "creating a cron job on a branch not existing on GitHub" do + before { Travis::API::V3::Models::Permission.create(repository: repo, user: repo.owner, push: true) } + before { post("/v3/repo/#{repo.id}/branch/#{non_existing_branch.name}/cron", options, headers) } + example { expect(parsed_body).to be == { + "@type" => "error", + "error_type" => "error", + "error_message" => "Crons can only be set up for branches existing on GitHub!" + }} + end + describe "try creating a cron job without login" do before { post("/v3/repo/#{repo.id}/branch/#{branch.name}/cron", options) } example { expect(parsed_body).to be == {