refactor responders

Gemfile.lock

@@ -40,7 +40,7 @@ GIT
 
 GIT
   remote: git://github.com/travis-ci/travis-core.git
-  revision: 7c84635b5c180a716150c4300bff3ea8f381248c
+  revision: d30228a62f87698d20d1373e12d7e4fdda654c26
   branch: sf-travis-api
   specs:
     travis-core (0.0.1)
@@ -129,7 +129,7 @@ GEM
       activesupport
     faraday (0.8.4)
       multipart-post (~> 1.1)
-    foreman (0.60.0)
+    foreman (0.60.2)
       thor (>= 0.13.6)
     hashr (0.0.22)
     hike (1.2.1)

lib/travis/api/app/endpoint.rb

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

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

@@ -4,7 +4,7 @@ class Travis::Api::App
   class Endpoint
     class Accounts < Endpoint
       get '/', scope: :private do
-        respond_with all(params).run, type: :accounts
+        respond_with all(params), type: :accounts
       end
     end
   end

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

@@ -7,7 +7,7 @@ class Travis::Api::App
     class Artifacts < Endpoint
       # Fetches an artifact by it's *id*.
       get '/:id' do |id|
-        respond_with one(params).run || not_found
+        respond_with one(params) || not_found
       end
     end
   end

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

@@ -4,12 +4,12 @@ class Travis::Api::App
   class Endpoint
     class Branches < Endpoint
       get '/' do
-        respond_with all(params).run, type: :branches
+        respond_with all(params), type: :branches
       end
 
       # get '/:owner_name/:name/branches' do       # v1
       # get '/repos/:owner_name/:name/branches' do # v2
-      #   respond_with all(params).run, type: :branches
+      #   respond_with all(params), type: :branches
       # end
     end
   end

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

@@ -4,29 +4,29 @@ class Travis::Api::App
   class Endpoint
     class Builds < Endpoint
       get '/' do
-        respond_with all(params).run
+        respond_with all(params)
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found
+        respond_with one(params).run || not_found # TODO hrmmmmmm
       end
 
       # get '/repositories/:repository_id/builds' do   # v1
       # get '/repos/:repository_id/builds' do          # v2
-      #   respond_with all(params).run
+      #   respond_with all(params)
       # end
 
       # get '/repositories/:repository_id/builds/1' do # v1
-      #   respond_with all(params).run
+      #   respond_with all(params)
       # end
 
       # get '/:owner_name/:name/builds' do             # v1
       # get '/repos/:owner_name/:name/builds' do       # v2
-      #   respond_with all(params).run
+      #   respond_with all(params)
       # end
 
       # get '/:owner_name/:name/builds/:id' do         # v1
-      #   respond_with all(params).run
+      #   respond_with all(params)
       # end
     end
   end

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

@@ -4,11 +4,11 @@ class Travis::Api::App
   class Endpoint
     class Hooks < Endpoint
       get '/', scope: :private do
-        respond_with all(params).run, type: :hooks
+        respond_with all(params), type: :hooks
       end
 
       put '/:id?', scope: :private do
-        update(id: params[:id] || params[:hook][:id], active: params[:hook][:active]).run
+        update(id: params[:id] || params[:hook][:id], active: params[:hook][:active])
         204
       end
     end

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

@@ -4,11 +4,11 @@ class Travis::Api::App
   class Endpoint
     class Jobs < Endpoint
       get '/' do
-        respond_with all(params).run
+        respond_with all(params)
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found
+        respond_with one(params).run || not_found # TODO hrmmmmmm
       end
     end
   end

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

@@ -5,7 +5,7 @@ class Travis::Api::App
     # TODO v2 should be /repos
     class Repositories < Endpoint
       get '/' do
-        respond_with all(params).run
+        respond_with all(params)
       end
 
       get '/:id' do

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

@@ -4,8 +4,7 @@ class Travis::Api::App
   class Endpoint
     class Requests < Endpoint
       post '/' do
-        service(:requests, :requeue, params).run
-        204
+        respond_with service(:requests, :requeue, params)
       end
     end
   end

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

@@ -24,16 +24,16 @@ class Travis::Api::App
       end
 
       get '/permissions', scope: :private do
-        respond_with service(:users, :permissions).run, type: :permissions
+        respond_with service(:users, :permissions), type: :permissions
       end
 
       put '/:id?', scope: :private do
-        update(params[:user]).run
+        update(params[:user])
         204
       end
 
       post '/sync', scope: :private do
-        service(:users, :sync).run
+        service(:users, :sync)
         204
       end
     end

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

@@ -4,11 +4,11 @@ class Travis::Api::App
   class Endpoint
     class Workers < Endpoint
       get '/' do
-        respond_with all(params).run
+        respond_with all(params)
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found
+        respond_with one(params).run || not_found # TODO hrmmmmm
       end
     end
   end

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

@@ -0,0 +1,11 @@
+require 'travis/api/app'
+
+class Travis::Api::App
+  module Helpers
+    module Flash
+      def flash
+        @flash ||= []
+      end
+    end
+  end
+end

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

@@ -2,19 +2,18 @@ require 'travis/api/app'
 
 class Travis::Api::App
   module Helpers
-    module Responders
-      autoload :Base,  'travis/api/app/helpers/responders/base'
-      autoload :Image, 'travis/api/app/helpers/responders/image'
-      autoload :Json,  'travis/api/app/helpers/responders/json'
-      autoload :Xml,   'travis/api/app/helpers/responders/xml'
-    end
-
     # Allows routes to return either hashes or anything Travis::API.data can
     # convert (in addition to the return values supported by Sinatra, of
     # course). These values will be encoded in JSON.
     module RespondWith
       def respond_with(resource, options = {})
-        halt responder.new(request, headers, resource, options).render
+        options[:format] ||= format_from_content_type || params[:format] || :json
+        responders.each do |responder|
+          responder = responder.new(self, resource, options)
+          resource = responder.apply if responder.apply?
+        end
+        resource = resource.to_json unless resource.is_a?(String) # TODO when does this happen?
+        halt resource
       end
 
       def body(value = nil, options = {}, &block)
@@ -24,12 +23,8 @@ class Travis::Api::App
 
       private
 
-        def responder
-          Responders.const_get(responder_type.to_s.camelize) # or raise shit
-        end
-
-        def responder_type
-          format_from_content_type || params[:format] || 'json'
+        def responders
+          [Responders::Service, Responders::Json, Responders::Image, Responders::Xml]
         end
 
         # TODO is there no support for this kind of mime types?

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

@@ -0,0 +1,13 @@
+require 'travis/api/app'
+
+class Travis::Api::App
+  module Helpers
+    module Responders
+      autoload :Base,    'travis/api/app/helpers/responders/base'
+      autoload :Image,   'travis/api/app/helpers/responders/image'
+      autoload :Json,    'travis/api/app/helpers/responders/json'
+      autoload :Service, 'travis/api/app/helpers/responders/service'
+      autoload :Xml,     'travis/api/app/helpers/responders/xml'
+    end
+  end
+end

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

@@ -1,12 +1,31 @@
 module Travis::Api::App::Helpers::Responders
   class Base
-    attr_reader :request, :headers, :resource, :options
+    attr_reader :endpoint, :resource, :options
 
-    def initialize(request, headers, resource, options = {})
-      @request  = request
-      @headers  = headers
+    def initialize(endpoint, resource, options = {})
+      @endpoint  = endpoint
       @resource = resource
       @options  = options
     end
+
+    def halt(*args)
+      endpoint.halt(*args)
+    end
+
+    def flash
+      endpoint.flash
+    end
+
+    def request
+      endpoint.request
+    end
+
+    def params
+      endpoint.params
+    end
+
+    def headers
+      endpoint.headers
+    end
   end
 end

lib/travis/api/app/helpers/responders/image.rb

@@ -2,9 +2,13 @@ module Travis::Api::App::Helpers::Responders
   class Image < Base
     NAMES = { nil => 'unknown', 0 => 'passing', 1 => 'failing' }
 
-    def render
+    def apply?
+      options[:format] == 'png'
+    end
+
+    def apply
       headers['Expires'] = Time.now.utc.httpdate
-      send_file filename(resource), type: :png, disposition: :inline
+      halt send_file(filename(resource), type: :png, disposition: :inline)
     end
 
     private

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

@@ -3,22 +3,25 @@ module Travis::Api::App::Helpers::Responders
     ACCEPT_VERSION  = /vnd\.travis-ci\.(\d+)\+/
     DEFAULT_VERSION = 'v2'
 
-    def render
-      options[:version] ||= version
-      builder  = Travis::Api.builder(resource, options) || raise_undefined_builder
-      resource = builder.new(self.resource, request.params).data
-      resource = resource.to_json unless resource.is_a?(String)
-      resource
+    def apply?
+      !resource.is_a?(String) && options[:format] == 'json'
+    end
+
+    def apply
+      resource = builder.new(self.resource, request.params).data if builder
+      resource ||= self.resource || {}
+      resource.merge!(flash: flash) unless flash.empty?
+      halt resource.to_json
     end
 
     private
 
+      def builder
+        @builder ||= Travis::Api.builder(resource, { :version => version }.merge(options))
+      end
+
       def version
         request.accept.join =~ ACCEPT_VERSION && "v#{$1}" || DEFAULT_VERSION
       end
-
-      def raise_undefined_builder
-        raise("could not determine a builder for #{resource}, #{options}")
-      end
   end
 end

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

@@ -0,0 +1,15 @@
+module Travis::Api::App::Helpers::Responders
+  class Service < Base
+    def apply?
+      resource.respond_to?(:run)
+    end
+
+    def apply
+      # TODO add caching headers depending on the resource
+      result = resource.run || {}
+      flash.concat(resource.messages) if resource.respond_to?(:messages)
+      result
+    end
+  end
+end
+

lib/travis/api/app/helpers/responders/xml.rb

@@ -14,8 +14,12 @@ module Travis::Api::App::Helpers::Responders
       'finished' => 'Sleeping'
     }
 
-    def render
-      TEMPLATE % data
+    def apply?
+      options[:format] == 'xml'
+    end
+
+    def apply
+      halt TEMPLATE % data
     end
 
     private