From a395ba865db80db9583d6ffc763734bbad5174c0 Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Mon, 9 Feb 2015 17:47:06 +0100 Subject: [PATCH] throttle build request receive api --- lib/travis/api/app/endpoint/requests.rb | 8 +-- .../api/app/services/schedule_request.rb | 53 ++++++++++++++++++- spec/integration/v2/requests_spec.rb | 44 +++++++++++---- spec/unit/endpoint/requests/throttle_spec.rb | 20 +++++++ 4 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 spec/unit/endpoint/requests/throttle_spec.rb diff --git a/lib/travis/api/app/endpoint/requests.rb b/lib/travis/api/app/endpoint/requests.rb index 7cefe2ab..6a1aff30 100644 --- a/lib/travis/api/app/endpoint/requests.rb +++ b/lib/travis/api/app/endpoint/requests.rb @@ -4,10 +4,6 @@ require 'travis/api/app/services/schedule_request' class Travis::Api::App class Endpoint class Requests < Endpoint - post '/', scope: :private do - respond_with service(:schedule_request, params[:request]) - end - get '/' do begin respond_with(service(:find_requests, params).run) @@ -20,6 +16,10 @@ class Travis::Api::App get '/:id' do respond_with service(:find_request, params) end + + post '/', scope: :private do + respond_with service(:schedule_request, params[:request]) + end end end end diff --git a/lib/travis/api/app/services/schedule_request.rb b/lib/travis/api/app/services/schedule_request.rb index e66bc7f2..7186bf7e 100644 --- a/lib/travis/api/app/services/schedule_request.rb +++ b/lib/travis/api/app/services/schedule_request.rb @@ -8,7 +8,15 @@ class Travis::Api::App register :schedule_request def run - repo && active? ? schedule_request : not_found + if repo.nil? + not_found + elsif !active? + not_active + elsif throttle.throttled? + throttled + else + schedule_request + end end def messages @@ -29,6 +37,16 @@ class Travis::Api::App :not_found end + def not_active + messages << { error: "Repository #{slug} not active." } + :not_active + end + + def throttled + messages << { error: "Repository #{slug} throttled." } + :throttled + end + def active? Travis::Features.owner_active?(:request_create, repo.owner) end @@ -40,13 +58,44 @@ class Travis::Api::App end def repo - @repo ||= Repository.by_slug(slug).first + instance_variable_defined?(:@repo) ? @repo : @repo = Repository.by_slug(slug).first end def slug repo = params[:repository] || {} repo.values_at(:owner_name, :name).join('/') end + + def throttle + @throttle ||= Throttle.new(slug) + end + + class Throttle < Struct.new(:slug) + def throttled? + current_requests >= max_requests + end + + def message + 'API throttled' + end + + private + + def current_requests + @current_requests ||= begin + sql = "repository_id = ? AND event_type = ? AND result = ? AND created_at > ?" + Request.where(sql, repository.id, 'api', 'accepted', 1.hour.ago).count + end + end + + def max_requests + Travis.config.max_api_requests || 10 + end + + def repository + @repository ||= Repository.by_slug(slug).first || raise(Travis::RepositoryNotFoundError.new(slug: slug)) + end + end end end end diff --git a/spec/integration/v2/requests_spec.rb b/spec/integration/v2/requests_spec.rb index 4b671ad6..02207446 100644 --- a/spec/integration/v2/requests_spec.rb +++ b/spec/integration/v2/requests_spec.rb @@ -1,13 +1,15 @@ require 'spec_helper' +require 'json' describe 'Requests' do - let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } } + let(:repo) { Factory.create(:repository) } + let(:request) { Factory.create(:request, repository: repo) } + let(:user) { Factory.create(:user) } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json', 'HTTP_AUTHORIZATION' => "token #{token}" } } - describe '/requests' do + describe 'GET /requests' do it 'fetches requests' do - repo = Factory.create(:repository) - request = Factory.create(:request, repository: repo) - response = get '/requests', { repository_id: repo.id }, headers response.should deliver_json_for(repo.requests, version: 'v2', type: 'requests') end @@ -18,13 +20,37 @@ describe 'Requests' do end end - describe '/requests/:id' do + describe 'GET /requests/:id' do it 'fetches a request' do - repo = Factory.create(:repository) - request = Factory.create(:request, repository: repo) - response = get "/requests/#{request.id}", {}, headers response.should deliver_json_for(request, version: 'v2', type: 'request') end end + + describe 'POST /requests' do + let(:payload) { { request: { repository: { owner_name: repo.owner_name, name: repo.name } } } } + + before do + Travis::Features.stubs(:owner_active?).returns(true) + end + + it 'schedules a request' do + response = post '/requests', payload, headers + expect(response.status).to eq 200 + end + + it 'requires activation' do + Travis::Features.stubs(:owner_active?).returns(false) + response = post '/requests', payload, headers + json = JSON.parse(response.body) + expect(json['result']).to eq 'not_active' + end + + it 'throttles requests' do + Travis::Api::App::Services::ScheduleRequest::Throttle.any_instance.stubs(:throttled?).returns(true) + response = post '/requests', payload, headers + json = JSON.parse(response.body) + expect(json['result']).to eq 'throttled' + end + end end diff --git a/spec/unit/endpoint/requests/throttle_spec.rb b/spec/unit/endpoint/requests/throttle_spec.rb new file mode 100644 index 00000000..b0d0470a --- /dev/null +++ b/spec/unit/endpoint/requests/throttle_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +describe Travis::Api::App::Services::ScheduleRequest::Throttle do + let(:repo) { Factory(:repository) } + subject { described_class.new(repo.slug) } + + it 'does not throttle by default' do + expect(subject.throttled?).to eq false + end + + it 'throttles with more then N requests for the same repo in the last hour' do + 10.times { Request.create!(repository: repo, event_type: 'api', result: 'accepted') } + expect(subject.throttled?).to eq true + end + + it 'does not throttle with more then N requests for other repos in the last hour' do + 10.times { Request.create!(repository: Factory(:repository), event_type: 'api', result: 'accepted') } + expect(subject.throttled?).to eq false + end +end