diff --git a/Gemfile.lock b/Gemfile.lock index 75d64679..8194f765 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -50,7 +50,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: e71a198112cbb6abefbbf0c2a096414ee9853a24 + revision: 9ef018a267e94251dbaac54503a410baca71d1c6 specs: travis-core (0.0.1) actionmailer (~> 3.2.19) diff --git a/lib/travis/api/v3.rb b/lib/travis/api/v3.rb index 6ca5eabc..1a38e2bd 100644 --- a/lib/travis/api/v3.rb +++ b/lib/travis/api/v3.rb @@ -24,16 +24,17 @@ module Travis load_dir("#{__dir__}/v3/extensions") load_dir("#{__dir__}/v3") - ClientError = Error .create(status: 400) - NotFound = ClientError .create(:resource, status: 404, template: '%s not found (or insufficient access)') - EntityMissing = NotFound .create(type: 'not_found') - WrongCredentials = ClientError .create('access denied', status: 403) - LoginRequired = ClientError .create('login required', status: 403) - InsufficientAccess = ClientError .create(status: 403) - PushAccessRequired = InsufficientAccess .create('push access required') - WrongParams = ClientError .create('wrong parameters') - ServerError = Error .create(status: 500) - NotImplemented = ServerError .create('request not (yet) implemented', status: 501) + ClientError = Error .create(status: 400) + NotFound = ClientError .create(:resource, status: 404, template: '%s not found (or insufficient access)') + EntityMissing = NotFound .create(type: 'not_found') + WrongCredentials = ClientError .create('access denied', status: 403) + LoginRequired = ClientError .create('login required', status: 403) + InsufficientAccess = ClientError .create(status: 403) + PushAccessRequired = InsufficientAccess .create('push access required') + WrongParams = ClientError .create('wrong parameters') + ServerError = Error .create(status: 500) + NotImplemented = ServerError .create('request not (yet) implemented', status: 501) + RequestLimitReached = ClientError .create('request limit reached for resource', status: 429) end end end diff --git a/lib/travis/api/v3/queries/request.rb b/lib/travis/api/v3/queries/request.rb index 60c551b9..8208b43e 100644 --- a/lib/travis/api/v3/queries/request.rb +++ b/lib/travis/api/v3/queries/request.rb @@ -6,13 +6,16 @@ module Travis::API::V3 raise ServerError, 'repository does not have a github_id'.freeze unless repository.github_id raise WrongParams, 'missing user'.freeze unless user and user.id - perform_async(:build_request, type: 'api'.freeze, credentials: {}, payload: { + payload = { repository: { id: repository.github_id }, user: { id: user.id }, message: message, branch: branch || repository.default_branch_name, config: config || {} - }) + } + + perform_async(:build_request, type: 'api'.freeze, credentials: {}, payload: payload) + payload end end end diff --git a/lib/travis/api/v3/queries/requests.rb b/lib/travis/api/v3/queries/requests.rb index 05d01869..9c536a1c 100644 --- a/lib/travis/api/v3/queries/requests.rb +++ b/lib/travis/api/v3/queries/requests.rb @@ -1,6 +1,13 @@ module Travis::API::V3 class Queries::Requests < Query def find(repository) + repository.requests + end + + def count(repository, time_frame) + find(repository). + where(event_type: 'api'.freeze, result: 'accepted'.freeze). + where('created_at > ?'.freeze, time_frame.ago).count end end end diff --git a/lib/travis/api/v3/renderer/accepted.rb b/lib/travis/api/v3/renderer/accepted.rb index ad38d60f..145a80d3 100644 --- a/lib/travis/api/v3/renderer/accepted.rb +++ b/lib/travis/api/v3/renderer/accepted.rb @@ -2,11 +2,8 @@ module Travis::API::V3 module Renderer::Accepted extend self - def render(type, **) - { - :@type => 'pending'.freeze, - :resource_type => type - } + def render(payload, **options) + { :@type => 'pending'.freeze, **Renderer.render_value(payload) } end end end diff --git a/lib/travis/api/v3/service.rb b/lib/travis/api/v3/service.rb index affd37ff..b19e662c 100644 --- a/lib/travis/api/v3/service.rb +++ b/lib/travis/api/v3/service.rb @@ -52,8 +52,9 @@ module Travis::API::V3 params.keys.any? { |key| key.start_with? "#{prefix}." } end - def accepted(type = self.class.result_type) - Result.new(:accepted, type, status: 202) + def accepted(**payload) + payload[:resource_type] ||= self.class.result_type + Result.new(:accepted, payload, status: 202) end def not_implemented diff --git a/lib/travis/api/v3/services/requests/create.rb b/lib/travis/api/v3/services/requests/create.rb index 8c539b69..c651a03e 100644 --- a/lib/travis/api/v3/services/requests/create.rb +++ b/lib/travis/api/v3/services/requests/create.rb @@ -1,5 +1,9 @@ module Travis::API::V3 class Services::Requests::Create < Service + TIME_FRAME = 1.hour + LIMIT = 10 + private_constant :TIME_FRAME, :LIMIT + result_type :request def run @@ -7,13 +11,20 @@ module Travis::API::V3 raise NotFound unless repository = find(:repository) raise PushAccessRequired, repository: repository unless access_control.writable?(repository) - user = find(:user) if access_control.full_access? and params_for? 'user'.freeze - user ||= access_control.user + user = find(:user) if access_control.full_access? and params_for? 'user'.freeze + user ||= access_control.user + remaining = remaining_requests(repository) - not_implemented unless Travis::Features.owner_active?(:request_create, repository.owner) + raise RequestLimitReached, repository: repository if remaining == 0 - query.schedule(repository, user) - accepted(:request) + payload = query.schedule(repository, user) + accepted(remaining_requests: remaining, repository: repository, request: payload) + end + + def remaining_requests(repository) + return LIMIT if access_control.full_access? + count = query(:requests).count(repository, TIME_FRAME) + count > LIMIT ? 0 : LIMIT - count end end end diff --git a/spec/v3/services/requests/create_spec.rb b/spec/v3/services/requests/create_spec.rb index c0c3d93d..94c8a515 100644 --- a/spec/v3/services/requests/create_spec.rb +++ b/spec/v3/services/requests/create_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe Travis::API::V3::Services::Requests::Create do let(:repo) { Travis::API::V3::Models::Repository.where(owner_name: 'svenfuchs', name: 'minimal').first } let(:sidekiq_payload) { Sidekiq::Client.last['args'].last[:payload] } + before { repo.requests.each(&:delete) } before do Travis::Features.stubs(:owner_active?).returns(true) @@ -83,8 +84,16 @@ describe Travis::API::V3::Services::Requests::Create do example { expect(last_response.status).to be == 202 } example { expect(JSON.load(body)).to be == { - "@type" => "pending", - "resource_type" => "request" + "@type" => "pending", + "remaining_requests" => 10, + "repository" => {"@type"=>"repository", "@href"=>"/repo/#{repo.id}", "id"=>repo.id, "slug"=>"svenfuchs/minimal"}, + "request" => { + "repository" => {"id"=>repo.id}, + "user" => {"id"=>repo.owner.id}, + "message" => nil, + "branch" => "master", + "config" => {}}, + "resource_type" => "request" }} example { expect(sidekiq_payload).to be == { @@ -207,6 +216,19 @@ describe Travis::API::V3::Services::Requests::Create do config: {} }} end + + describe "when request limit is reached" do + before { 10.times { repo.requests.create(event_type: 'api', result: 'accepted') } } + before { post("/v3/repo/#{repo.id}/requests", params, headers) } + + example { expect(last_response.status).to be == 429 } + example { expect(JSON.load(body)).to be == { + "@type" => "error", + "error_type" => "request_limit_reached", + "error_message" => "request limit reached for resource", + "repository" => {"@type"=>"repository", "@href"=>"/repo/#{repo.id}", "id"=>repo.id, "slug"=>"svenfuchs/minimal" } + }} + end end