Fix Accept header support for logs request

When log is aggregated and archived we don't have a way to return it in
JSON format, only text format. Till recently we were returning a text
response or a redirect to S3 even if Accept header was set only to JSON.
This commit fixes it.

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::Accept
+
       get '/' do
         prefer_follower do
           respond_with service(:find_jobs, params)
@@ -49,15 +51,21 @@ class Travis::Api::App
 
       get '/:job_id/log' do
         resource = service(:find_log, params).run
-        if !resource || resource.archived?
+        if (!resource || resource.archived?)
-          archived_log_path = archive_url("/jobs/#{params[:job_id]}/log.txt")
+          # the way we use responders makes it hard to validate proper format
+          # automatically here, so we need to check it explicitly
+          if accepts?('text/plain')
+            archived_log_path = archive_url("/jobs/#{params[:job_id]}/log.txt")
 
-          if params[:cors_hax]
+            if params[:cors_hax]
-            status 204
+              status 204
-            headers['Access-Control-Expose-Headers'] = 'Location'
+              headers['Access-Control-Expose-Headers'] = 'Location'
-            headers['Location'] = archived_log_path
+              headers['Location'] = archived_log_path
+            else
+              redirect archived_log_path, 307
+            end
           else
-            redirect archived_log_path, 307
+            status 406
           end
         else
           respond_with resource

lib/travis/api/app/helpers/accept.rb

@@ -64,6 +64,10 @@ class Travis::Api::App
         end
       end
 
+      def accepts?(mime_type)
+        accept_entries.any? { |e| e.accepts?(mime_type) }
+      end
+
       def accept_entries
         entries = env['HTTP_ACCEPT'].to_s.delete(' ').to_s.split(',').map { |e| Entry.new(e) }
         entries.empty? ? [Entry.new('*/*')] : entries.sort

spec/integration/v2/jobs_spec.rb

@@ -28,6 +28,7 @@ describe 'Jobs' do
     context 'when log is archived' do
       it 'redirects to archive' do
         job.log.update_attributes!(content: 'the log', archived_at: Time.now, archive_verified: true)
+        headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' }
         response = get "/jobs/#{job.id}/log.txt", {}, headers
         response.should redirect_to("https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt")
       end
@@ -36,6 +37,7 @@ describe 'Jobs' do
     context 'when log is missing' do
       it 'redirects to archive' do
         job.log.destroy
+        headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' }
         response = get "/jobs/#{job.id}/log.txt", {}, headers
         response.should redirect_to("https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt")
       end
@@ -44,6 +46,7 @@ describe 'Jobs' do
     context 'with cors_hax param' do
       it 'renders No Content response with location of the archived log' do
         job.log.destroy
+        headers = { 'HTTP_ACCEPT' => 'text/plain; version=2' }
         response = get "/jobs/#{job.id}/log.txt?cors_hax=true", {}, headers
         response.status.should == 204
         response.headers['Location'].should == "https://s3.amazonaws.com/archive.travis-ci.org/jobs/#{job.id}/log.txt"
@@ -76,8 +79,10 @@ describe 'Jobs' do
       end
 
       it 'responds with 406 when log is already aggregated' do
-        job.log.update_attributes(aggregated_at: Time.now)
+        job.log.update_attributes(aggregated_at: Time.now, archived_at: Time.now, archive_verified: true)
-        headers = { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json; chunked=true' }
+        job.log.should be_archived
+
+        headers = { 'HTTP_ACCEPT' => 'application/json; version=2; chunked=true' }
         response = get "/jobs/#{job.id}/log", {}, headers
         response.status.should == 406
       end