From cad6c4e68e917d96a48cb01152e1502700d8edd2 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Tue, 10 Jun 2014 14:59:22 -0400 Subject: [PATCH 01/17] Update gemspec --- travis-api.gemspec | 94 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 78 insertions(+), 16 deletions(-) diff --git a/travis-api.gemspec b/travis-api.gemspec index 43570731..9e735082 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -12,39 +12,47 @@ Gem::Specification.new do |s| "Piotr Sarnacki", "Konstantin Haase", "Sven Fuchs", - "Josh Kalderimis", "Mathias Meyer", + "Josh Kalderimis", "Henrik Hodne", "Hiro Asari", "Andre Arko", "Erik Michaels-Ober", - "Steve Richert", "Brian Ford", + "Steve Richert", + "rainsun", + "James Dennes", "Nick Schonning", "Patrick Williams", - "James Dennes", - "Tim Carey-Smith" + "Puneeth Chaganti", + "Thais Camilo and Konstantin Haase", + "Tim Carey-Smith", + "Zachary Scott" ] s.email = [ "drogus@gmail.com", "konstantin.mailinglists@googlemail.com", "me@svenfuchs.com", - "josh.kalderimis@gmail.com", "meyer@paperplanes.de", - "me@henrikhodne.com", + "josh.kalderimis@gmail.com", "asari.ruby@gmail.com", - "konstantin.haase@gmail.com", + "me@henrikhodne.com", "henrik@hodne.io", - "andre@arko.net", + "konstantin.haase@gmail.com", "svenfuchs@artweb-design.de", + "andre@arko.net", "sferik@gmail.com", - "steve.richert@gmail.com", "bford@engineyard.com", - "nschonni@gmail.com", + "steve.richert@gmail.com", + "rainsuner@gmail.com", "jdennes@gmail.com", + "nschonni@gmail.com", + "patrick@bittorrent.com", + "punchagan@muse-amuse.in", + "dev+narwen+rkh@rkh.im", "tim@spork.in", - "patrick@bittorrent.com" + "e@zzak.io" ] s.files = [ @@ -58,8 +66,6 @@ Gem::Specification.new do |s| "config/nginx.conf.erb", "config/puma-config.rb", "config/unicorn.rb", - "docs/00_overview.md", - "docs/01_cross_origin.md", "lib/tasks/build_update_branch.rake", "lib/tasks/build_update_pull_request_data.rake", "lib/tasks/encyrpt_all_data.rake", @@ -74,8 +80,6 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint/broadcasts.rb", "lib/travis/api/app/endpoint/builds.rb", "lib/travis/api/app/endpoint/documentation.rb", - "lib/travis/api/app/endpoint/documentation/css/style.css", - "lib/travis/api/app/endpoint/documentation/resources.rb", "lib/travis/api/app/endpoint/endpoints.rb", "lib/travis/api/app/endpoint/home.rb", "lib/travis/api/app/endpoint/hooks.rb", @@ -83,6 +87,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint/logs.rb", "lib/travis/api/app/endpoint/repos.rb", "lib/travis/api/app/endpoint/requests.rb", + "lib/travis/api/app/endpoint/setting_endpoint.rb", "lib/travis/api/app/endpoint/uptime.rb", "lib/travis/api/app/endpoint/users.rb", "lib/travis/api/app/extensions.rb", @@ -104,24 +109,60 @@ Gem::Specification.new do |s| "lib/travis/api/app/middleware/scope_check.rb", "lib/travis/api/app/responders.rb", "lib/travis/api/app/responders/atom.rb", + "lib/travis/api/app/responders/badge.rb", "lib/travis/api/app/responders/base.rb", "lib/travis/api/app/responders/image.rb", "lib/travis/api/app/responders/json.rb", "lib/travis/api/app/responders/plain.rb", "lib/travis/api/app/responders/service.rb", "lib/travis/api/app/responders/xml.rb", + "lib/travis/api/app/skylight/dalli_probe.rb", + "lib/travis/api/app/skylight/redis_probe.rb", + "lib/travis/api/app/skylight/service_probe.rb", + "lib/travis/api/serializer.rb", + "lib/travis/api/v2.rb", + "lib/travis/api/v2/http.rb", + "lib/travis/api/v2/http/accounts.rb", + "lib/travis/api/v2/http/annotations.rb", + "lib/travis/api/v2/http/branch.rb", + "lib/travis/api/v2/http/branches.rb", + "lib/travis/api/v2/http/broadcasts.rb", + "lib/travis/api/v2/http/build.rb", + "lib/travis/api/v2/http/builds.rb", + "lib/travis/api/v2/http/caches.rb", + "lib/travis/api/v2/http/error.rb", + "lib/travis/api/v2/http/hooks.rb", + "lib/travis/api/v2/http/job.rb", + "lib/travis/api/v2/http/jobs.rb", + "lib/travis/api/v2/http/log.rb", + "lib/travis/api/v2/http/permissions.rb", + "lib/travis/api/v2/http/repositories.rb", + "lib/travis/api/v2/http/repository.rb", + "lib/travis/api/v2/http/request.rb", + "lib/travis/api/v2/http/requests.rb", + "lib/travis/api/v2/http/ssh_key.rb", + "lib/travis/api/v2/http/ssh_keys.rb", + "lib/travis/api/v2/http/ssl_key.rb", + "lib/travis/api/v2/http/user.rb", + "lib/travis/api/v2/http/validation_error.rb", "public/favicon.ico", "public/images/result/error.png", + "public/images/result/error.svg", "public/images/result/failing.png", + "public/images/result/failing.svg", "public/images/result/passing.png", + "public/images/result/passing.svg", "public/images/result/pending.png", + "public/images/result/pending.svg", "public/images/result/unknown.png", + "public/images/result/unknown.svg", "script/console", "script/server", "spec/integration/formats_handling_spec.rb", "spec/integration/responders_spec.rb", "spec/integration/routes.backup.rb", "spec/integration/scopes_spec.rb", + "spec/integration/settings_endpoint_spec.rb", "spec/integration/uptime_spec.rb", "spec/integration/v1/branches_spec.rb", "spec/integration/v1/builds_spec.rb", @@ -134,12 +175,33 @@ Gem::Specification.new do |s| "spec/integration/v2/hooks_spec.rb", "spec/integration/v2/jobs_spec.rb", "spec/integration/v2/repositories_spec.rb", + "spec/integration/v2/requests_spec.rb", "spec/integration/v2/users_spec.rb", "spec/integration/v2_spec.backup.rb", "spec/integration/version_spec.rb", "spec/spec_helper.rb", + "spec/support/formats.rb", "spec/support/matchers.rb", "spec/unit/access_token_spec.rb", + "spec/unit/api/v2/http/accounts_spec.rb", + "spec/unit/api/v2/http/annotations_spec.rb", + "spec/unit/api/v2/http/branch_spec.rb", + "spec/unit/api/v2/http/branches_spec.rb", + "spec/unit/api/v2/http/broadcasts_spec.rb", + "spec/unit/api/v2/http/build_spec.rb", + "spec/unit/api/v2/http/builds_spec.rb", + "spec/unit/api/v2/http/caches_spec.rb", + "spec/unit/api/v2/http/hooks_spec.rb", + "spec/unit/api/v2/http/job_spec.rb", + "spec/unit/api/v2/http/jobs_spec.rb", + "spec/unit/api/v2/http/log_spec.rb", + "spec/unit/api/v2/http/permissions_spec.rb", + "spec/unit/api/v2/http/repositories_spec.rb", + "spec/unit/api/v2/http/repository_spec.rb", + "spec/unit/api/v2/http/request_spec.rb", + "spec/unit/api/v2/http/requests_spec.rb", + "spec/unit/api/v2/http/ssl_key_spec.rb", + "spec/unit/api/v2/http/user_spec.rb", "spec/unit/app_spec.rb", "spec/unit/cors_spec.rb", "spec/unit/default_spec.rb", @@ -148,7 +210,6 @@ Gem::Specification.new do |s| "spec/unit/endpoint/authorization_spec.rb", "spec/unit/endpoint/branches_spec.rb", "spec/unit/endpoint/builds_spec.rb", - "spec/unit/endpoint/documentation_spec.rb", "spec/unit/endpoint/endpoints_spec.rb", "spec/unit/endpoint/hooks_spec.rb", "spec/unit/endpoint/jobs_spec.rb", @@ -165,6 +226,7 @@ Gem::Specification.new do |s| "spec/unit/middleware/scope_check_spec.rb", "spec/unit/responders/json_spec.rb", "spec/unit/responders/service_spec.rb", + "tmp/.gitkeep", "travis-api.gemspec" ] From 5c079f8e66c9ced264c0b52021932ed731660688 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 09:29:12 -0400 Subject: [PATCH 02/17] Add specs for RemoveLog service Status code is debatable; I opted for 422 when the job is still running, and for 500 if unexpected error happened --- lib/travis/api/app/endpoint/logs.rb | 18 ++++++++++++++ spec/unit/endpoint/logs_spec.rb | 38 +++++++++++++++++++++++++++++ travis-api.gemspec | 1 + 3 files changed, 57 insertions(+) create mode 100644 spec/unit/endpoint/logs_spec.rb diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index 5dee65d9..37d38be2 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -8,6 +8,24 @@ class Travis::Api::App get '/:id' do |id| respond_with service(:find_log, params) end + + # Clears up the content of the log by the *job id* + # Optionally takes parameter *reason* + patch '/:id' do |id| + begin + result = self.service(:remove_log, params) + respond_with result + rescue Travis::AuthorizationDenied => ade + status 401 + { error: { message: ade.message } } + rescue Travis::JobUnfinished => jue + status 422 + { error: { message: "Job #{id} is not finished" } } + rescue => e + status 500 + { error: { message: "Unexpected error occurred: #{e.message}" } } + end + end end end end diff --git a/spec/unit/endpoint/logs_spec.rb b/spec/unit/endpoint/logs_spec.rb new file mode 100644 index 00000000..3fa502aa --- /dev/null +++ b/spec/unit/endpoint/logs_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Travis::Api::App::Endpoint::Logs do + let(:user) { Factory(:user) } + let(:job) { Factory(:test, owner: user, log: Factory(:log)) } + let(:provider) { Factory(:annotation_provider) } + + describe "GET /logs/:id/" do + it "finds log successfully" do + get("/logs/#{job.log.id}", {}, "HTTP_ACCEPT" => "application/vnd.travis-ci.2+json, */*; q=0.01").should be_ok + end + end + + describe "PATCH /logs/:id/" do + before do + Travis::Services::RemoveLog.any_instance.stubs(:current_user).returns user + end + + context "user is unauthorized" do + it 'returns status 401' do + response = patch("/logs/#{job.id}") + response.status.should == 401 + JSON.parse(response.body)['error']['message'].should =~ Regexp.new("insufficient permission") + end + end + + context 'job is still running' do + it 'returns status 422' do + job.stubs(:finished?).returns false + user.stubs(:permission?).with(:push, anything).returns true + + response = patch("/logs/#{job.id}") + response.status.should == 422 + JSON.parse(response.body)['error']['message'].should =~ Regexp.new("Job .*is (not |un)finished") + end + end + end +end diff --git a/travis-api.gemspec b/travis-api.gemspec index 9e735082..b517c000 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -213,6 +213,7 @@ Gem::Specification.new do |s| "spec/unit/endpoint/endpoints_spec.rb", "spec/unit/endpoint/hooks_spec.rb", "spec/unit/endpoint/jobs_spec.rb", + "spec/unit/endpoint/logs_spec.rb", "spec/unit/endpoint/repos_spec.rb", "spec/unit/endpoint/users_spec.rb", "spec/unit/endpoint_spec.rb", From 7f830dc41100d17cd1ce68e66f805dac4a2db2d2 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 09:53:19 -0400 Subject: [PATCH 03/17] Update gems --- Gemfile.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 69a7646e..307a8a09 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: 1cb24ef805fb820993b3a052a2b293010eea1dc0 + revision: c5141bb7ffa3ef9c7dac9ce5a1ed17a19b02f7ba specs: travis-core (0.0.1) actionmailer (~> 3.2.12) @@ -204,7 +204,7 @@ GEM net-http-persistent (2.9.4) net-http-pipeline (1.0.1) pg (0.13.2) - polyglot (0.3.4) + polyglot (0.3.5) proxies (0.2.1) pry (0.9.12.4) coderay (~> 1.0) @@ -287,7 +287,7 @@ GEM eventmachine (>= 1.0.0) rack (>= 1.0.0) thor (0.14.6) - thread_safe (0.3.3) + thread_safe (0.3.4) tilt (1.4.1) timers (1.1.0) treetop (1.4.15) From db7828c2feb63d1aa9969248772dafc172c63ae5 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 10:37:11 -0400 Subject: [PATCH 04/17] Bring 'logs' table up to date So that removed_* columns are added --- .travis.yml | 6 ++++++ set_up_travis_logs.sh | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100755 set_up_travis_logs.sh diff --git a/.travis.yml b/.travis.yml index 157e917a..60413522 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,13 @@ rvm: addons: postgresql: 9.3 before_script: + # create 'logs' table matching 'travis-logs' + - ./set_up_travis_logs.sh - 'RAILS_ENV=test bundle exec rake db:create db:structure:load --trace' + # replace 'logs' table in travis_test DB with that in travis_logs_test + - psql -c "DROP TABLE IF EXISTS logs CASCADE" -U postgres travis_test + - pg_dump -t logs travis_logs_test | psql -U postgres travis_test + notifications: irc: "irc.freenode.org#travis" services: diff --git a/set_up_travis_logs.sh b/set_up_travis_logs.sh new file mode 100755 index 00000000..03d10821 --- /dev/null +++ b/set_up_travis_logs.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +# clone travis-logs +pushd $HOME +git clone --depth=1 https://github.com/travis-ci/travis-logs.git +cd travis-logs + +# install ruby runtime which travis-logs wants +RUBY_RUNTIME=$(cat .ruby-version) +rvm install $RUBY_RUNTIME +# using JRuby, migrate the 'logs' table in 'travis_test' database +BUNDLE_GEMFILE=$PWD/Gemfile +rvm $RUBY_RUNTIME do bundle install +psql -c "CREATE DATABASE travis_logs_test;" -U postgres +cp $TRAVIS_BUILD_DIR/config/database.yml config/travis.yml +rvm $RUBY_RUNTIME do bundle exec rake db:migrate +popd From 82a0a5c0a58d55a1ad6cf17ed22ffe415438ce44 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 10:56:14 -0400 Subject: [PATCH 05/17] Retry bundle install --- set_up_travis_logs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/set_up_travis_logs.sh b/set_up_travis_logs.sh index 03d10821..f62bb139 100755 --- a/set_up_travis_logs.sh +++ b/set_up_travis_logs.sh @@ -10,7 +10,7 @@ RUBY_RUNTIME=$(cat .ruby-version) rvm install $RUBY_RUNTIME # using JRuby, migrate the 'logs' table in 'travis_test' database BUNDLE_GEMFILE=$PWD/Gemfile -rvm $RUBY_RUNTIME do bundle install +travis_retry rvm $RUBY_RUNTIME do bundle install psql -c "CREATE DATABASE travis_logs_test;" -U postgres cp $TRAVIS_BUILD_DIR/config/database.yml config/travis.yml rvm $RUBY_RUNTIME do bundle exec rake db:migrate From c822efc94ed695462a95887aa028dc3c7e91739b Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 11:17:42 -0400 Subject: [PATCH 06/17] Duplicate travis_retry() --- set_up_travis_logs.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/set_up_travis_logs.sh b/set_up_travis_logs.sh index f62bb139..4eaaf05e 100755 --- a/set_up_travis_logs.sh +++ b/set_up_travis_logs.sh @@ -1,5 +1,30 @@ #!/bin/bash +function travis_retry() { + $Local:result = 0 + $Local:count = 1 + $Local:cmd_string = $args -join ' ' + + while ( $count -le 3 ) { + if ( $result -ne 0 ) { + Write-Host -foregroundColor Red "`nThe command ""$cmd_string"" failed. Retrying, $count of 3.`n" 2>&1 + } + Invoke-Expression($cmd_string) + $result = $LastExitCode + if ( $result -eq 0 ) { + break + } + $count=$count + 1 + sleep 1 + } + + if ( $count -eq 3 ) { + Write-Host -foregroundColor Red "`nThe command ""$cmd_string"" failed 3 times.`n" 2>&1 + } + + return $result +} + # clone travis-logs pushd $HOME git clone --depth=1 https://github.com/travis-ci/travis-logs.git From 851c2b2db201848b0fcb6f05f77e5e2945f9747d Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 11:28:42 -0400 Subject: [PATCH 07/17] Use bash function, not PowerShell --- set_up_travis_logs.sh | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/set_up_travis_logs.sh b/set_up_travis_logs.sh index 4eaaf05e..54569ecb 100755 --- a/set_up_travis_logs.sh +++ b/set_up_travis_logs.sh @@ -1,25 +1,21 @@ #!/bin/bash -function travis_retry() { - $Local:result = 0 - $Local:count = 1 - $Local:cmd_string = $args -join ' ' - - while ( $count -le 3 ) { - if ( $result -ne 0 ) { - Write-Host -foregroundColor Red "`nThe command ""$cmd_string"" failed. Retrying, $count of 3.`n" 2>&1 +travis_retry() { + local result=0 + local count=1 + while [ $count -le 3 ]; do + [ $result -ne 0 ] && { + echo -e "\n${RED}The command \"$@\" failed. Retrying, $count of 3.${RESET}\n" >&2 } - Invoke-Expression($cmd_string) - $result = $LastExitCode - if ( $result -eq 0 ) { - break - } - $count=$count + 1 + "$@" + result=$? + [ $result -eq 0 ] && break + count=$(($count + 1)) sleep 1 - } + done - if ( $count -eq 3 ) { - Write-Host -foregroundColor Red "`nThe command ""$cmd_string"" failed 3 times.`n" 2>&1 + [ $count -eq 3 ] && { + echo "\n${RED}The command \"$@\" failed 3 times.${RESET}\n" >&2 } return $result From 1cf298464a0383081326e6212240cb9111d73cd3 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 13:28:16 -0400 Subject: [PATCH 08/17] Add RemoveLog service endpoint to jobs/:id/log This replaces https://github.com/travis-ci/travis-api/pull/107. --- Gemfile.lock | 2 +- lib/travis/api/app/endpoint/jobs.rb | 14 ++++++++ spec/integration/v2/jobs_spec.rb | 52 +++++++++++++++++++++++++++++ travis-api.gemspec | 3 +- 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 307a8a09..f1de4e5c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: c5141bb7ffa3ef9c7dac9ce5a1ed17a19b02f7ba + revision: a8d8e4c9c92e436645e80c6fb486fff8a442e6f6 specs: travis-core (0.0.1) actionmailer (~> 3.2.12) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index dec9ce34..26d662f2 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -64,6 +64,20 @@ class Travis::Api::App end end + patch '/:id/log', scope: :private do + # PATCH method for `RemoveLog` service, since we are replacing log content + service = self.service(:remove_log, params) + begin + respond_with service.run + rescue Travis::AuthorizationDenied => e + status 401 + { error: { message: e.message } } + rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e + status 409 + { error: { message: e.message } } + end + end + get "/:job_id/annotations" do respond_with service(:find_annotations, params) end diff --git a/spec/integration/v2/jobs_spec.rb b/spec/integration/v2/jobs_spec.rb index b865ed59..4a30c638 100644 --- a/spec/integration/v2/jobs_spec.rb +++ b/spec/integration/v2/jobs_spec.rb @@ -76,6 +76,58 @@ describe 'Jobs' do end end + describe 'PATCH /jobs/:job_id/log' do + let(:user) { User.where(login: 'svenfuchs').first } + let(:token) { Travis::Api::App::AccessToken.create(user: user, app_id: -1) } + + before :each do + headers.merge! 'HTTP_AUTHORIZATION' => "token #{token}" + end + + context 'when user does not have push permissions' do + before :each do + user.permissions.create!(repository_id: job.repository.id, :push => false) + end + + it 'returns status 401' do + response = patch "/jobs/#{job.id}/log", { reason: 'Because reason!' }, headers + response.status.should == 401 + end + end + + context 'when user has push permission' do + context 'when job is not finished' do + before :each do + job.stubs(:finished?).returns false + user.permissions.create!(repository_id: job.repository.id, :push => true) + end + + it 'returns status 409' do + response = patch "/jobs/#{job.id}/log", { reason: 'Because reason!' }, headers + response.status.should == 409 + end + end + + context 'when job is finished' do + let(:finished_job) { Factory(:test, state: 'passed') } + + before :each do + user.permissions.create!(repository_id: finished_job.repository.id, :push => true) + end + + it 'returns status 200' do + response = patch "/jobs/#{finished_job.id}/log", { reason: 'Because reason!' }, headers + response.status.should == 200 + end + + end + end + + context 'when job is not found' do + # TODO + end + end + it "GET /jobs/:id/annotations" do annotation_provider = Factory(:annotation_provider) annotation = annotation_provider.annotations.create(job_id: job.id, status: "passed", description: "Foobar") diff --git a/travis-api.gemspec b/travis-api.gemspec index b517c000..c0d53e10 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -14,8 +14,8 @@ Gem::Specification.new do |s| "Sven Fuchs", "Mathias Meyer", "Josh Kalderimis", - "Henrik Hodne", "Hiro Asari", + "Henrik Hodne", "Andre Arko", "Erik Michaels-Ober", "Brian Ford", @@ -158,6 +158,7 @@ Gem::Specification.new do |s| "public/images/result/unknown.svg", "script/console", "script/server", + "set_up_travis_logs.sh", "spec/integration/formats_handling_spec.rb", "spec/integration/responders_spec.rb", "spec/integration/routes.backup.rb", From 4a3807b21a0c9dac3b4fd22833066b838649d5fe Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 13:51:47 -0400 Subject: [PATCH 09/17] Standardize error handling for RemoveLog service endpoints Return status 409 for errors --- lib/travis/api/app/endpoint/jobs.rb | 7 +++++-- lib/travis/api/app/endpoint/logs.rb | 5 ++++- spec/unit/endpoint/logs_spec.rb | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 26d662f2..2424e221 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -64,7 +64,7 @@ class Travis::Api::App end end - patch '/:id/log', scope: :private do + patch '/:id/log', scope: :private do |id| # PATCH method for `RemoveLog` service, since we are replacing log content service = self.service(:remove_log, params) begin @@ -72,7 +72,10 @@ class Travis::Api::App rescue Travis::AuthorizationDenied => e status 401 { error: { message: e.message } } - rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e + rescue Travis::JobUnfinished => jue + status 409 + { error: { message: "Job #{id} is not finished" } } + rescue Travis::LogAlreadyRemoved => e status 409 { error: { message: e.message } } end diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index 37d38be2..04b3753a 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -19,8 +19,11 @@ class Travis::Api::App status 401 { error: { message: ade.message } } rescue Travis::JobUnfinished => jue - status 422 + status 409 { error: { message: "Job #{id} is not finished" } } + rescue Travis::LogAlreadyRemoved => e + status 409 + { error: { message: e.message } } rescue => e status 500 { error: { message: "Unexpected error occurred: #{e.message}" } } diff --git a/spec/unit/endpoint/logs_spec.rb b/spec/unit/endpoint/logs_spec.rb index 3fa502aa..20b4e83b 100644 --- a/spec/unit/endpoint/logs_spec.rb +++ b/spec/unit/endpoint/logs_spec.rb @@ -25,12 +25,12 @@ describe Travis::Api::App::Endpoint::Logs do end context 'job is still running' do - it 'returns status 422' do + it 'returns status 409' do job.stubs(:finished?).returns false user.stubs(:permission?).with(:push, anything).returns true response = patch("/logs/#{job.id}") - response.status.should == 422 + response.status.should == 409 JSON.parse(response.body)['error']['message'].should =~ Regexp.new("Job .*is (not |un)finished") end end From 8da49332d64282b16f33e55b3c8cae2bdb52747d Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 13:58:57 -0400 Subject: [PATCH 10/17] Clean up error handling for RemoveLog With the error message change in travis-core, we can handle 2 exceptions in one rescue clause --- Gemfile.lock | 2 +- lib/travis/api/app/endpoint/jobs.rb | 5 +---- lib/travis/api/app/endpoint/logs.rb | 5 +---- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f1de4e5c..459d041a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: a8d8e4c9c92e436645e80c6fb486fff8a442e6f6 + revision: a4f19caa0ae6847a3a10db2b99ba752a656aad98 specs: travis-core (0.0.1) actionmailer (~> 3.2.12) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 2424e221..e7af7d32 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -72,10 +72,7 @@ class Travis::Api::App rescue Travis::AuthorizationDenied => e status 401 { error: { message: e.message } } - rescue Travis::JobUnfinished => jue - status 409 - { error: { message: "Job #{id} is not finished" } } - rescue Travis::LogAlreadyRemoved => e + rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e status 409 { error: { message: e.message } } end diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index 04b3753a..2db13baf 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -18,10 +18,7 @@ class Travis::Api::App rescue Travis::AuthorizationDenied => ade status 401 { error: { message: ade.message } } - rescue Travis::JobUnfinished => jue - status 409 - { error: { message: "Job #{id} is not finished" } } - rescue Travis::LogAlreadyRemoved => e + rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e status 409 { error: { message: e.message } } rescue => e From cc03c5458a2ac0d0392f2ec6ca6e665b8154efb3 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 18:07:12 -0400 Subject: [PATCH 11/17] DRY up log patching logic Move the shared logic into a Helpers method so that there is no code duplication --- lib/travis/api/app/endpoint/jobs.rb | 14 +++----------- lib/travis/api/app/endpoint/logs.rb | 15 ++------------- lib/travis/api/app/helpers.rb | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index e7af7d32..a6d4b8a7 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -3,6 +3,8 @@ require 'travis/api/app' class Travis::Api::App class Endpoint class Jobs < Endpoint + include Helpers + get '/' do prefer_follower do respond_with service(:find_jobs, params) @@ -65,17 +67,7 @@ class Travis::Api::App end patch '/:id/log', scope: :private do |id| - # PATCH method for `RemoveLog` service, since we are replacing log content - service = self.service(:remove_log, params) - begin - respond_with service.run - rescue Travis::AuthorizationDenied => e - status 401 - { error: { message: e.message } } - rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e - status 409 - { error: { message: e.message } } - end + patch_log_for_job(id, params) end get "/:job_id/annotations" do diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index 2db13baf..a7bda278 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -4,6 +4,7 @@ class Travis::Api::App class Endpoint # Logs are generated by builds. class Logs < Endpoint + include Helpers # Fetches a log by its *id*. get '/:id' do |id| respond_with service(:find_log, params) @@ -12,19 +13,7 @@ class Travis::Api::App # Clears up the content of the log by the *job id* # Optionally takes parameter *reason* patch '/:id' do |id| - begin - result = self.service(:remove_log, params) - respond_with result - rescue Travis::AuthorizationDenied => ade - status 401 - { error: { message: ade.message } } - rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e - status 409 - { error: { message: e.message } } - rescue => e - status 500 - { error: { message: "Unexpected error occurred: #{e.message}" } } - end + patch_log_for_job(id, params) end end end diff --git a/lib/travis/api/app/helpers.rb b/lib/travis/api/app/helpers.rb index a009b025..fe1cfd02 100644 --- a/lib/travis/api/app/helpers.rb +++ b/lib/travis/api/app/helpers.rb @@ -4,5 +4,19 @@ class Travis::Api::App # Namespace for helpers. module Helpers Backports.require_relative_dir 'helpers' + + def patch_log_for_job(id, params) + result = self.service(:remove_log, params) + respond_with result + rescue Travis::AuthorizationDenied => ade + status 401 + { error: { message: ade.message } } + rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e + status 409 + { error: { message: e.message } } + rescue => e + status 500 + { error: { message: "Unexpected error occurred: #{e.message}" } } + end end end From 477c8b9212d12509870b0c8736ad58581ac82f51 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 20:19:57 -0400 Subject: [PATCH 12/17] Tweak RemoveLog helper Leave responding up to the endpoint Remove unnecessary parameter from the helper's parameter --- lib/travis/api/app/endpoint/jobs.rb | 2 +- lib/travis/api/app/endpoint/logs.rb | 4 ++-- lib/travis/api/app/helpers.rb | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index a6d4b8a7..29eecdee 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -67,7 +67,7 @@ class Travis::Api::App end patch '/:id/log', scope: :private do |id| - patch_log_for_job(id, params) + respond_with patch_log_for_job(params) end get "/:job_id/annotations" do diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index a7bda278..edf59467 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -12,8 +12,8 @@ class Travis::Api::App # Clears up the content of the log by the *job id* # Optionally takes parameter *reason* - patch '/:id' do |id| - patch_log_for_job(id, params) + patch '/:id' do + respond_with patch_log_for_job(params) end end end diff --git a/lib/travis/api/app/helpers.rb b/lib/travis/api/app/helpers.rb index fe1cfd02..1522f8f0 100644 --- a/lib/travis/api/app/helpers.rb +++ b/lib/travis/api/app/helpers.rb @@ -5,9 +5,8 @@ class Travis::Api::App module Helpers Backports.require_relative_dir 'helpers' - def patch_log_for_job(id, params) - result = self.service(:remove_log, params) - respond_with result + def patch_log_for_job(params) + self.service(:remove_log, params).run rescue Travis::AuthorizationDenied => ade status 401 { error: { message: ade.message } } From d9e5eaaeebe0a7407ee8cdfbb2100202de330c6d Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Wed, 11 Jun 2014 21:10:15 -0400 Subject: [PATCH 13/17] Update README [skip ci] API now requires that `logs` table should be set up properly --- README.md | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 5c82ce7c..d52ce9d1 100644 --- a/README.md +++ b/README.md @@ -2,20 +2,48 @@ This is the app running on https://api.travis-ci.org/ +## Requirements + +1. PostgreSQL 9.3 or higher +1. Redis +1. RabbitMQ + ## Installation -Setup: +### Setup $ bundle install -Run tests: +### Database setup + +1. `rake db:create db:structure:load` +1. Clone `travis-logs` and copy the `logs` database (assume the PostgreSQL user is `postgres`): +```sh-session +cd .. +git clone https://github.com/travis-ci/travis-logs.git +cd travis-logs +rvm jruby do bundle exec rake db:migrate # `travis-logs` requires JRuby +psql -c "DROP TABLE IF EXISTS logs CASCADE" -U postgres travis_development +pg_dump -t logs travis_logs_development | psql -U postgres travis_development +``` + +Repeat the database steps for `RAILS_ENV=test`. +```sh-session +RAILS_ENV=test rake db:create db:structure:load +pushd ../travis-logs +RAILS_ENV=test rvm jruby do bundle exec rake db:migrate +psql -c "DROP TABLE IF EXISTS logs CASCADE" -U postgres travis_test +pg_dump -t logs travis_logs_test | psql -U postgres travis_test +popd +``` + + +### Run tests - $ RAILS_ENV=test rake db:create db:structure:load $ rake spec -Run the server: +### Run the server - $ rake db:create db:structure:load $ script/server ## Contributing From dfffe8e405cb528c2cd1519c5a1c4e97f61b4554 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Thu, 12 Jun 2014 07:20:47 -0400 Subject: [PATCH 14/17] Remove `PATCH logs/:job_id` endpoint To eradicate unnecessary inconsistency. Spec to test the existing `GET logs/:id` remains. --- lib/travis/api/app/endpoint/jobs.rb | 15 ++++++++++++--- lib/travis/api/app/endpoint/logs.rb | 7 ------- lib/travis/api/app/helpers.rb | 13 ------------- spec/unit/endpoint/logs_spec.rb | 29 +---------------------------- travis-api.gemspec | 4 ++-- 5 files changed, 15 insertions(+), 53 deletions(-) diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index 29eecdee..0c497a58 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/lib/travis/api/app/endpoint/jobs.rb @@ -3,8 +3,6 @@ require 'travis/api/app' class Travis::Api::App class Endpoint class Jobs < Endpoint - include Helpers - get '/' do prefer_follower do respond_with service(:find_jobs, params) @@ -67,7 +65,18 @@ class Travis::Api::App end patch '/:id/log', scope: :private do |id| - respond_with patch_log_for_job(params) + begin + self.service(:remove_log, params).run + rescue Travis::AuthorizationDenied => ade + status 401 + { error: { message: ade.message } } + rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e + status 409 + { error: { message: e.message } } + rescue => e + status 500 + { error: { message: "Unexpected error occurred: #{e.message}" } } + end end get "/:job_id/annotations" do diff --git a/lib/travis/api/app/endpoint/logs.rb b/lib/travis/api/app/endpoint/logs.rb index edf59467..5dee65d9 100644 --- a/lib/travis/api/app/endpoint/logs.rb +++ b/lib/travis/api/app/endpoint/logs.rb @@ -4,17 +4,10 @@ class Travis::Api::App class Endpoint # Logs are generated by builds. class Logs < Endpoint - include Helpers # Fetches a log by its *id*. get '/:id' do |id| respond_with service(:find_log, params) end - - # Clears up the content of the log by the *job id* - # Optionally takes parameter *reason* - patch '/:id' do - respond_with patch_log_for_job(params) - end end end end diff --git a/lib/travis/api/app/helpers.rb b/lib/travis/api/app/helpers.rb index 1522f8f0..a009b025 100644 --- a/lib/travis/api/app/helpers.rb +++ b/lib/travis/api/app/helpers.rb @@ -4,18 +4,5 @@ class Travis::Api::App # Namespace for helpers. module Helpers Backports.require_relative_dir 'helpers' - - def patch_log_for_job(params) - self.service(:remove_log, params).run - rescue Travis::AuthorizationDenied => ade - status 401 - { error: { message: ade.message } } - rescue Travis::JobUnfinished, Travis::LogAlreadyRemoved => e - status 409 - { error: { message: e.message } } - rescue => e - status 500 - { error: { message: "Unexpected error occurred: #{e.message}" } } - end end end diff --git a/spec/unit/endpoint/logs_spec.rb b/spec/unit/endpoint/logs_spec.rb index 20b4e83b..7f13049f 100644 --- a/spec/unit/endpoint/logs_spec.rb +++ b/spec/unit/endpoint/logs_spec.rb @@ -1,38 +1,11 @@ require 'spec_helper' describe Travis::Api::App::Endpoint::Logs do - let(:user) { Factory(:user) } - let(:job) { Factory(:test, owner: user, log: Factory(:log)) } - let(:provider) { Factory(:annotation_provider) } + let(:job) { Factory(:test) } describe "GET /logs/:id/" do it "finds log successfully" do get("/logs/#{job.log.id}", {}, "HTTP_ACCEPT" => "application/vnd.travis-ci.2+json, */*; q=0.01").should be_ok end end - - describe "PATCH /logs/:id/" do - before do - Travis::Services::RemoveLog.any_instance.stubs(:current_user).returns user - end - - context "user is unauthorized" do - it 'returns status 401' do - response = patch("/logs/#{job.id}") - response.status.should == 401 - JSON.parse(response.body)['error']['message'].should =~ Regexp.new("insufficient permission") - end - end - - context 'job is still running' do - it 'returns status 409' do - job.stubs(:finished?).returns false - user.stubs(:permission?).with(:push, anything).returns true - - response = patch("/logs/#{job.id}") - response.status.should == 409 - JSON.parse(response.body)['error']['message'].should =~ Regexp.new("Job .*is (not |un)finished") - end - end - end end diff --git a/travis-api.gemspec b/travis-api.gemspec index c0d53e10..244aadfa 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -13,8 +13,8 @@ Gem::Specification.new do |s| "Konstantin Haase", "Sven Fuchs", "Mathias Meyer", - "Josh Kalderimis", "Hiro Asari", + "Josh Kalderimis", "Henrik Hodne", "Andre Arko", "Erik Michaels-Ober", @@ -35,8 +35,8 @@ Gem::Specification.new do |s| "konstantin.mailinglists@googlemail.com", "me@svenfuchs.com", "meyer@paperplanes.de", - "josh.kalderimis@gmail.com", "asari.ruby@gmail.com", + "josh.kalderimis@gmail.com", "me@henrikhodne.com", "henrik@hodne.io", "konstantin.haase@gmail.com", From f21c3a14c58be83254f8c5b566ca1f191f049def Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Tue, 17 Jun 2014 09:57:11 -0400 Subject: [PATCH 15/17] Bring travis-core up to date --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 6a1be31f..21aff496 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -45,7 +45,7 @@ GIT GIT remote: git://github.com/travis-ci/travis-core.git - revision: 111ae389e611157d48177ed732c1822d98fe3059 + revision: ef8fb67bd56569eb55d176f824bb62bc602c8f80 specs: travis-core (0.0.1) actionmailer (~> 3.2.12) From 16aad923d0233fac5ed61ededde65d398c32be5e Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Tue, 17 Jun 2014 10:15:54 -0400 Subject: [PATCH 16/17] Update gemspec --- travis-api.gemspec | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/travis-api.gemspec b/travis-api.gemspec index fc4c5525..6d55d8bd 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -35,8 +35,8 @@ Gem::Specification.new do |s| "konstantin.mailinglists@googlemail.com", "me@svenfuchs.com", "meyer@paperplanes.de", - "josh.kalderimis@gmail.com", "asari.ruby@gmail.com", + "josh.kalderimis@gmail.com", "me@henrikhodne.com", "henrik@hodne.io", "konstantin.haase@gmail.com", @@ -76,7 +76,6 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint.rb", "lib/travis/api/app/endpoint/accounts.rb", "lib/travis/api/app/endpoint/authorization.rb", - "lib/travis/api/app/endpoint/branches.rb", "lib/travis/api/app/endpoint/broadcasts.rb", "lib/travis/api/app/endpoint/builds.rb", "lib/travis/api/app/endpoint/documentation.rb", @@ -84,6 +83,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint/home.rb", "lib/travis/api/app/endpoint/hooks.rb", "lib/travis/api/app/endpoint/jobs.rb", + "lib/travis/api/app/endpoint/lint.rb", "lib/travis/api/app/endpoint/logs.rb", "lib/travis/api/app/endpoint/repos.rb", "lib/travis/api/app/endpoint/requests.rb", @@ -165,13 +165,11 @@ Gem::Specification.new do |s| "spec/integration/scopes_spec.rb", "spec/integration/settings_endpoint_spec.rb", "spec/integration/uptime_spec.rb", - "spec/integration/v1/branches_spec.rb", "spec/integration/v1/builds_spec.rb", "spec/integration/v1/hooks_spec.rb", "spec/integration/v1/jobs_spec.rb", "spec/integration/v1/repositories_spec.rb", "spec/integration/v1_spec.backup.rb", - "spec/integration/v2/branches_spec.rb", "spec/integration/v2/builds_spec.rb", "spec/integration/v2/hooks_spec.rb", "spec/integration/v2/jobs_spec.rb", @@ -209,11 +207,11 @@ Gem::Specification.new do |s| "spec/unit/endpoint/accounts_spec.rb", "spec/unit/endpoint/authorization/user_manager_spec.rb", "spec/unit/endpoint/authorization_spec.rb", - "spec/unit/endpoint/branches_spec.rb", "spec/unit/endpoint/builds_spec.rb", "spec/unit/endpoint/endpoints_spec.rb", "spec/unit/endpoint/hooks_spec.rb", "spec/unit/endpoint/jobs_spec.rb", + "spec/unit/endpoint/lint_spec.rb", "spec/unit/endpoint/logs_spec.rb", "spec/unit/endpoint/repos_spec.rb", "spec/unit/endpoint/users_spec.rb", From 08443b1a3859a75fe08e4184f63902719b1aa798 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Tue, 17 Jun 2014 10:43:52 -0400 Subject: [PATCH 17/17] Update gemspec once again --- travis-api.gemspec | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/travis-api.gemspec b/travis-api.gemspec index 6d55d8bd..b3c53100 100644 --- a/travis-api.gemspec +++ b/travis-api.gemspec @@ -76,6 +76,7 @@ Gem::Specification.new do |s| "lib/travis/api/app/endpoint.rb", "lib/travis/api/app/endpoint/accounts.rb", "lib/travis/api/app/endpoint/authorization.rb", + "lib/travis/api/app/endpoint/branches.rb", "lib/travis/api/app/endpoint/broadcasts.rb", "lib/travis/api/app/endpoint/builds.rb", "lib/travis/api/app/endpoint/documentation.rb", @@ -165,11 +166,13 @@ Gem::Specification.new do |s| "spec/integration/scopes_spec.rb", "spec/integration/settings_endpoint_spec.rb", "spec/integration/uptime_spec.rb", + "spec/integration/v1/branches_spec.rb", "spec/integration/v1/builds_spec.rb", "spec/integration/v1/hooks_spec.rb", "spec/integration/v1/jobs_spec.rb", "spec/integration/v1/repositories_spec.rb", "spec/integration/v1_spec.backup.rb", + "spec/integration/v2/branches_spec.rb", "spec/integration/v2/builds_spec.rb", "spec/integration/v2/hooks_spec.rb", "spec/integration/v2/jobs_spec.rb", @@ -207,6 +210,7 @@ Gem::Specification.new do |s| "spec/unit/endpoint/accounts_spec.rb", "spec/unit/endpoint/authorization/user_manager_spec.rb", "spec/unit/endpoint/authorization_spec.rb", + "spec/unit/endpoint/branches_spec.rb", "spec/unit/endpoint/builds_spec.rb", "spec/unit/endpoint/endpoints_spec.rb", "spec/unit/endpoint/hooks_spec.rb",