manual redirects and rewrite rules for supporting v1 style resources

lib/travis/api/app.rb

@@ -17,13 +17,6 @@ require 'newrelic_rpm'
 #
 # Requires TLS in production.
 module Travis::Api
-  ACCEPT_VERSION  = /vnd\.travis-ci\.(\d+)\+/
-  DEFAULT_VERSION = 'v2'
-
-  def version(string)
-    string =~ ACCEPT_VERSION && "v#{$1}" || DEFAULT_VERSION
-  end
-
   class App
     autoload :AccessToken,  'travis/api/app/access_token'
     autoload :Base,         'travis/api/app/base'

lib/travis/api/app/endpoint.rb

@@ -9,7 +9,7 @@ class Travis::Api::App
     set(:prefix) { "/" << name[/[^:]+$/].underscore }
     set disable_root_endpoint: false
     register :scoping
-    helpers :current_user, :services, :flash
+    helpers :current_user, :flash, :services
 
     # TODO hmmm?
     before { flash.clear }

lib/travis/api/app/endpoint/repos.rb

@@ -0,0 +1,31 @@
+require 'travis/api/app'
+
+class Travis::Api::App
+  class Endpoint
+    class Repos < Endpoint
+      get '/' do
+        respond_with service(:repositories, :all, params)
+      end
+
+      get '/:id' do
+        respond_with service(:repositories, :one, params)
+      end
+
+      get '/:id/cc' do
+        respond_with service(:repositories, :one, params.merge(schema: 'cc'))
+      end
+
+      get '/:owner_name/:name' do
+        respond_with service(:repositories, :one, params)
+      end
+
+      get '/:owner_name/:name/builds' do
+        respond_with service(:builds, :all, params)
+      end
+
+      get '/:owner_name/:name/builds/:id' do
+        respond_with service(:builds, :one, params)
+      end
+    end
+  end
+end

lib/travis/api/app/endpoint/repositories.rb

@@ -1,26 +0,0 @@
-require 'travis/api/app'
-
-class Travis::Api::App
-  class Endpoint
-    # TODO v2 should be /repos
-    class Repositories < Endpoint
-      get '/' do
-        respond_with all(params)
-      end
-
-      get '/:id' do
-        respond_with one(params)
-      end
-
-      # TODO the format constraint neither seems to work nor fail?
-      get '/:id/cc.:format', format: 'xml' do # v1
-        respond_with one(params)
-      end
-
-      # get '/:owner_name/:name.?:format?' do       # v1
-      # get '/repos/:owner_name/:name.?:format?' do # v2
-      #   respond_with service(:repositories, :one, params)
-      # end
-    end
-  end
-end

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

@@ -0,0 +1,19 @@
+require 'travis/api/app'
+
+class Travis::Api::App
+  module Helpers
+    module Accept
+      HEADER_FORMAT   = /vnd\.travis-ci\.(\d+)\+(\w+)/
+      DEFAULT_VERSION = 'v2'
+      DEFAULT_FORMAT  = 'json'
+
+      def accept_version
+        @accept_version ||= request.accept.join =~ HEADER_FORMAT && "v#{$1}" || DEFAULT_VERSION
+      end
+
+      def accept_format
+        @accept_format ||= request.accept.join =~ HEADER_FORMAT && $2 || DEFAULT_FORMAT
+      end
+    end
+  end
+end

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

@@ -7,8 +7,10 @@ class Travis::Api::App
     # course). These values will be encoded in JSON.
     module RespondWith
       def respond_with(resource, options = {})
-        options[:format] ||= format_from_content_type || params[:format] || 'json'
+        options[:format] ||= env['format']
-        halt respond(resource, options).to_json
+        result = respond(resource, options)
+        result = result ? result.to_json : 404
+        halt result
       end
 
       def body(value = nil, options = {}, &block)
@@ -31,11 +33,6 @@ class Travis::Api::App
             Responders.const_get(name)
           end
         end
-
-        # TODO is there no support for this kind of mime types?
-        def format_from_content_type
-          request.content_type && request.content_type.split(';').first.split('/').last
-        end
     end
   end
 end

lib/travis/api/app/middleware/rewrite.rb

@@ -3,17 +3,42 @@ require 'travis/api/app'
 class Travis::Api::App
   class Middleware
     class Rewrite < Middleware
-      V1_REPO_URL = %r(^(/[^/]+/[^/]+(?:/builds(?:/[\d]+)?|/cc\.xml)?)$)
+      FORMAT      = %r(\.(json|xml|png)$)
+      V1_REPO_URL = %r(^(/[^/]+/[^/]+(?:/builds(?:/[\d]+)?|/cc)?)$)
+
+      helpers :accept
 
       set(:setup) { ActiveRecord::Base.logger = Travis.logger }
 
+      before do
+        extract_format
+        rewrite_v1_repo_segment if v1? || xml?
+        rewrite_v1_named_repo_image_path if png?
+      end
+
       after do
-p not_found?
+        redirect_v1_named_repo_path if (v1? || xml?) && not_found?
-        force_redirect("/repositories#{$1}") if response.status == 404 && version == 'v1' && request.path =~ V1_REPO_URL
       end
 
       private
 
+        def extract_format
+          env['PATH_INFO'].sub!(FORMAT, '')
+          env['format'] = $1 || accept_format
+        end
+
+        def rewrite_v1_repo_segment
+          env['PATH_INFO'].sub!(%r(^/repositories), '/repos')
+        end
+
+        def rewrite_v1_named_repo_image_path
+          env['PATH_INFO'].sub!(V1_REPO_URL) { "/repos#{$1}" }
+        end
+
+        def redirect_v1_named_repo_path
+          force_redirect("/repositories#{$1}.#{env['format']}") if request.path =~ V1_REPO_URL
+        end
+
         def force_redirect(path)
           response.body = ''
           response['Content-Length'] = '0'
@@ -21,8 +46,16 @@ p not_found?
           redirect(path)
         end
 
-        def version
+        def png?
-          API.version(request.accept.join)
+          env['format'] == 'png'
+        end
+
+        def xml?
+          env['format'] == 'xml'
+        end
+
+        def v1?
+          accept_version == 'v1'
         end
     end
   end

lib/travis/api/app/responders/base.rb

@@ -12,6 +12,10 @@ module Travis::Api::App::Responders
       endpoint.halt(*args)
     end
 
+    def send_file(*args)
+      endpoint.send_file(*args)
+    end
+
     def flash
       endpoint.flash
     end

lib/travis/api/app/responders/json.rb

@@ -1,7 +1,10 @@
-module Travis::Api::App::Responders
+class Travis::Api::App
+  module Responders
     class Json < Base
+      include Helpers::Accept
+
       def apply?
-      options[:format] == 'json' && !resource.is_a?(String)
+        options[:format] == 'json' && !resource.is_a?(String) && !resource.nil?
       end
 
       def apply
@@ -15,11 +18,8 @@ module Travis::Api::App::Responders
         end
 
         def builder
-        @builder ||= Travis::Api.builder(resource, { :version => version }.merge(options))
+          @builder ||= Travis::Api.builder(resource, { :version => accept_version }.merge(options))
-      end
+        end
-
-      def version
-        API.version(request.accept.join)
     end
   end
 end

lib/travis/api/app/responders/service.rb

@@ -7,8 +7,7 @@ module Travis::Api::App::Responders
     def apply
       # TODO add caching headers depending on the resource
       data = result
-      halt 404 if data.nil?
+      flash.concat(data.messages) if data && resource.respond_to?(:messages)
-      flash.concat(data.messages) if resource.respond_to?(:messages)
       data
     end
 
@@ -16,9 +15,8 @@ module Travis::Api::App::Responders
 
       # Services potentially return all sorts of things
       # If it's a string, true or false we'll wrap it into a hash.
-      # If it's an active record instance or scope we just pass it on
+      # If it's an active record or scope we just pass so it can be processed by the json responder.
-      # so it can be processed by the json responder.
+      # If it's nil we also pass it but yield not_found.
-      # If it's nil we also pass it but immediately yield not_found.
       def result
         case result = resource.run
         when String, true, false

spec/integration/v1/builds_spec.rb

@@ -7,28 +7,28 @@ describe 'Builds' do
   let(:build)   { repo.builds.first }
   let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.1+json' } }
 
-  it 'GET /builds?repository_id=1' do
+  it 'GET /builds.json?repository_id=1' do
-    response = get '/builds', { repository_id: repo.id }, headers
+    response = get '/builds.json', { repository_id: repo.id }, headers
     response.should deliver_json_for(repo.builds.order('id DESC'), version: 'v1')
   end
 
-  it 'GET /builds/1' do
+  it 'GET /builds/1.json' do
-    response = get "/builds/#{build.id}", {}, headers
+    response = get "/builds/#{build.id}.json", {}, headers
     response.should deliver_json_for(build, version: 'v1')
   end
 
-  it 'GET /builds/1?repository_id=1' do
+  it 'GET /builds/1?repository_id=1.json' do
-    response = get "/builds/#{build.id}", { repository_id: repo.id }, headers
+    response = get "/builds/#{build.id}.json", { repository_id: repo.id }, headers
     response.should deliver_json_for(build, version: 'v1')
   end
 
-  xit 'GET /svenfuchs/minimal/builds' do
+  it 'GET /svenfuchs/minimal/builds.json' do
-    response = get '/svenfuchs/minimal/builds', {}, headers
+    response = get '/svenfuchs/minimal/builds.json', {}, headers
-    response.should deliver_json_for(repo.builds, version: 'v1')
+    response.should redirect_to('/repositories/svenfuchs/minimal/builds.json')
   end
 
-  xit 'GET /svenfuchs/minimal/builds/1' do
+  it 'GET /svenfuchs/minimal/builds/1.json' do
-    response = get "/svenfuchs/minimal/builds/#{build.id}", {}, headers
+    response = get "/svenfuchs/minimal/builds/#{build.id}.json", {}, headers
-    response.should deliver_json_for(build, version: 'v1')
+    response.should redirect_to("/repositories/svenfuchs/minimal/builds/#{build.id}.json")
   end
 end

spec/integration/v1/repositories_spec.rb

@@ -1,39 +1,39 @@
 require 'spec_helper'
 
-describe 'Repos' do
+describe 'v1 repos' do
   before(:each) { Scenario.default }
 
   let(:repo)    { Repository.by_slug('svenfuchs/minimal').first }
   let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.1+json' } }
 
-  it 'GET /repositories' do
+  it 'GET /repositories.json' do
-    response = get '/repositories', {}, headers
+    response = get '/repositories.json', {}, headers
     response.should deliver_json_for(Repository.timeline, version: 'v1')
   end
 
-  it 'GET /repositories?owner_name=svenfuchs' do
+  it 'GET /repositories.json?owner_name=svenfuchs' do
-    response = get '/repositories', { owner_name: 'svenfuchs' }, headers
+    response = get '/repositories.json', { owner_name: 'svenfuchs' }, headers
     response.should deliver_json_for(Repository.by_owner_name('svenfuchs'), version: 'v1')
   end
 
-  it 'GET /repositories?member=svenfuchs' do
+  it 'GET /repositories.json?member=svenfuchs' do
-    response = get '/repositories', { member: 'svenfuchs' }, headers
+    response = get '/repositories.json', { member: 'svenfuchs' }, headers
     response.should deliver_json_for(Repository.by_member('svenfuchs'), version: 'v1')
   end
 
-  it 'GET /repositories?slug=svenfuchs/name=minimal' do
+  it 'GET /repositories.json?slug=svenfuchs/name=minimal' do
-    response = get '/repositories', { slug: 'svenfuchs/minimal' }, headers
+    response = get '/repositories.json', { slug: 'svenfuchs/minimal' }, headers
     response.should deliver_json_for(Repository.by_slug('svenfuchs/minimal'), version: 'v1')
   end
 
-  it 'GET /repositories/1' do
+  it 'GET /repositories/1.json' do
-    response = get "repositories/#{repo.id}", {}, headers
+    response = get "repositories/#{repo.id}.json", {}, headers
     response.should deliver_json_for(Repository.by_slug('svenfuchs/minimal').first, version: 'v1')
   end
 
-  it 'GET /svenfuchs/minimal' do
+  it 'GET /svenfuchs/minimal.json' do
-    response = get '/svenfuchs/minimal', {}, headers
+    response = get '/svenfuchs/minimal.json', {}, headers
-    response.should redirect_to('/repositories/svenfuchs/minimal')
+    response.should redirect_to('/repositories/svenfuchs/minimal.json')
   end
 
   it 'GET /svenfuchs/minimal/cc.xml' do
@@ -42,55 +42,58 @@ describe 'Repos' do
   end
 
   describe 'GET /svenfuchs/minimal.png' do
-    xit '"unknown" when the repository does not exist' do
+    it '"unknown" when the repository does not exist' do
       get('/svenfuchs/does-not-exist.png').should deliver_result_image_for('unknown')
     end
 
-    xit '"unknown" when it only has one build that is not finished' do
+    it '"unknown" when it only has one build that is not finished' do
+      Build.delete_all
+      Factory(:build, repository: repo, state: :created, result: nil)
       repo.update_attributes!(last_build_result: nil)
       get('/svenfuchs/minimal.png').should deliver_result_image_for('unknown')
     end
 
-    xit '"failing" when the last build has failed' do
+    it '"failing" when the last build has failed' do
       repo.update_attributes!(last_build_result: 1)
       get('/svenfuchs/minimal.png').should deliver_result_image_for('failing')
     end
 
-    xit '"passing" when the last build has passed' do
+    it '"passing" when the last build has passed' do
       repo.update_attributes!(last_build_result: 0)
       get('/svenfuchs/minimal.png').should deliver_result_image_for('passing')
     end
 
-    xit '"passing" when there is a running build but the previous one has passed' do
+    it '"passing" when there is a running build but the previous one has passed' do
       Factory(:build, repository: repo, state: :finished, result: nil, previous_result: 0)
       repo.update_attributes!(last_build_result: nil)
       get('/svenfuchs/minimal.png').should deliver_result_image_for('passing')
     end
   end
 
-  describe 'GET /svenfuchs/minimal.png' do
+  describe 'GET /svenfuchs/minimal.png?branch=dev' do
     let(:commit) { Factory(:commit, branch: 'dev') }
 
-    xit '"unknown" when the repository does not exist' do
+    it '"unknown" when the repository does not exist' do
       get('/svenfuchs/does-not-exist.png?branch=dev').should deliver_result_image_for('unknown')
     end
 
-    xit '"unknown" when it only has a build that is not finished' do
+    it '"unknown" when it only has a build that is not finished' do
+      Build.delete_all
       Factory(:build, repository: repo, state: :started, result: nil, commit: commit)
       get('/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('unknown')
     end
 
-    xit '"failing" when the last build has failed' do
+    it '"failing" when the last build has failed' do
       Factory(:build, repository: repo, state: :finished, result: 1, commit: commit)
       get('/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('failing')
     end
 
-    xit '"passing" when the last build has passed' do
+    it '"passing" when the last build has passed' do
       Factory(:build, repository: repo, state: :finished, result: 0, commit: commit)
       get('/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('passing')
     end
 
-    xit '"passing" when there is a running build but the previous one has passed' do
+    it '"passing" when there is a running build but the previous one has passed' do
       Factory(:build, repository: repo, state: :finished, result: 0, commit: commit)
       Factory(:build, repository: repo, state: :started, result: nil, commit: commit)
       repo.update_attributes!(last_build_result: nil)

spec/integration/v2/builds_spec.rb

@@ -22,13 +22,13 @@ describe 'Builds' do
     response.should deliver_json_for(build, version: 'v2')
   end
 
-  xit 'GET /svenfuchs/minimal/builds' do
+  it 'GET /repos/svenfuchs/minimal/builds' do
-    response = get '/svenfuchs/minimal/builds', {}, headers
+    response = get '/repos/svenfuchs/minimal/builds', {}, headers
-    response.should deliver_json_for(repo.builds, version: 'v2')
+    response.should deliver_json_for(repo.builds, version: 'v2', type: :builds)
   end
 
-  xit 'GET /svenfuchs/minimal/builds/1' do
+  it 'GET /repos/svenfuchs/minimal/builds/1' do
-    response = get "/svenfuchs/minimal/builds/#{build.id}", {}, headers
+    response = get "/repos/svenfuchs/minimal/builds/#{build.id}", {}, headers
     response.should deliver_json_for(build, version: 'v2')
   end
 end

spec/integration/v2/repositories_spec.rb

@@ -6,38 +6,98 @@ describe 'Repos' do
   let(:repo)    { Repository.by_slug('svenfuchs/minimal').first }
   let(:headers) { { 'HTTP_ACCEPT' => 'application/vnd.travis-ci.2+json' } }
 
-  it 'GET /repositories' do
+  it 'GET /repos' do
-    response = get '/repositories', {}, headers
+    response = get '/repos', {}, headers
     response.should deliver_json_for(Repository.timeline, version: 'v2')
   end
 
-  it 'GET /repositories?owner_name=svenfuchs' do
+  it 'GET /repos?owner_name=svenfuchs' do
-    response = get '/repositories', { owner_name: 'svenfuchs' }, headers
+    response = get '/repos', { owner_name: 'svenfuchs' }, headers
     response.should deliver_json_for(Repository.by_owner_name('svenfuchs'), version: 'v2')
   end
 
-  it 'GET /repositories?member=svenfuchs' do
+  it 'GET /repos?member=svenfuchs' do
-    response = get '/repositories', { member: 'svenfuchs' }, headers
+    response = get '/repos', { member: 'svenfuchs' }, headers
     response.should deliver_json_for(Repository.by_member('svenfuchs'), version: 'v2')
   end
 
-  it 'GET /repositories?slug=svenfuchs/name=minimal' do
+  it 'GET /repos?slug=svenfuchs/name=minimal' do
-    response = get '/repositories', { slug: 'svenfuchs/minimal' }, headers
+    response = get '/repos', { slug: 'svenfuchs/minimal' }, headers
     response.should deliver_json_for(Repository.by_slug('svenfuchs/minimal'), version: 'v2')
   end
 
-  it 'GET /repositories/1' do
+  it 'GET /repos/1' do
-    response = get "repositories/#{repo.id}", {}, headers
+    response = get "repos/#{repo.id}", {}, headers
     response.should deliver_json_for(Repository.by_slug('svenfuchs/minimal').first, version: 'v2')
   end
 
-  xit 'GET /svenfuchs/minimal' do
+  it 'GET /repos/svenfuchs/minimal' do
-    response = get '/svenfuchs/minimal', {}, headers
+    response = get '/repos/svenfuchs/minimal', {}, headers
     response.should deliver_json_for(Repository.by_slug('svenfuchs/minimal').first, version: 'v2')
   end
 
-  xit 'GET /svenfuchs/minimal/cc.xml' do # TODO wat.
+  xit 'GET /repos/svenfuchs/minimal/cc.xml' do
-    response = get '/svenfuchs/minimal/cc.xml'
+    response = get '/repos/svenfuchs/minimal/cc.xml', {}, headers
-    response.should deliver_xml_for()
+    response.should deliver_xml_for(Repository.by_slug('svenfuchs/minimal').first, version: 'v2')
+  end
+
+  describe 'GET /repos/svenfuchs/minimal.png' do
+    it '"unknown" when the repository does not exist' do
+      get('/svenfuchs/does-not-exist.png').should deliver_result_image_for('unknown')
+    end
+
+    it '"unknown" when it only has one build that is not finished' do
+      Build.delete_all
+      Factory(:build, repository: repo, state: :created, result: nil)
+      repo.update_attributes!(last_build_result: nil)
+      get('/repos/svenfuchs/minimal.png').should deliver_result_image_for('unknown')
+    end
+
+    it '"failing" when the last build has failed' do
+      repo.update_attributes!(last_build_result: 1)
+      get('/repos/svenfuchs/minimal.png').should deliver_result_image_for('failing')
+    end
+
+    it '"passing" when the last build has passed' do
+      repo.update_attributes!(last_build_result: 0)
+      get('/repos/svenfuchs/minimal.png').should deliver_result_image_for('passing')
+    end
+
+    it '"passing" when there is a running build but the previous one has passed' do
+      Factory(:build, repository: repo, state: :finished, result: nil, previous_result: 0)
+      repo.update_attributes!(last_build_result: nil)
+      get('/repos/svenfuchs/minimal.png').should deliver_result_image_for('passing')
+    end
+  end
+
+  describe 'GET /repos/svenfuchs/minimal.png?branch=dev' do
+    let(:commit) { Factory(:commit, branch: 'dev') }
+
+    it '"unknown" when the repository does not exist' do
+      get('/repos/svenfuchs/does-not-exist.png?branch=dev').should deliver_result_image_for('unknown')
+    end
+
+    it '"unknown" when it only has a build that is not finished' do
+      Build.delete_all
+      Factory(:build, repository: repo, state: :started, result: nil, commit: commit)
+      get('/repos/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('unknown')
+    end
+
+    it '"failing" when the last build has failed' do
+      Factory(:build, repository: repo, state: :finished, result: 1, commit: commit)
+      get('/repos/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('failing')
+    end
+
+    it '"passing" when the last build has passed' do
+      Factory(:build, repository: repo, state: :finished, result: 0, commit: commit)
+      get('/repos/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('passing')
+    end
+
+    it '"passing" when there is a running build but the previous one has passed' do
+      Factory(:build, repository: repo, state: :finished, result: 0, commit: commit)
+      Factory(:build, repository: repo, state: :started, result: nil, commit: commit)
+      repo.update_attributes!(last_build_result: nil)
+      get('/repos/svenfuchs/minimal.png?branch=dev').should deliver_result_image_for('passing')
+    end
   end
 end