moar refactoring on responders, fix specs

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) || not_found
+        respond_with one(params)
       end
     end
   end

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

@@ -8,7 +8,7 @@ class Travis::Api::App
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found # TODO hrmmmmmm
+        respond_with one(params)
       end
 
       # get '/repositories/:repository_id/builds' do   # v1

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

@@ -8,8 +8,7 @@ class Travis::Api::App
       end
 
       put '/:id?', scope: :private do
-        update(id: params[:id] || params[:hook][:id], active: params[:hook][:active])
+        respond_with update(id: params[:id] || params[:hook][:id], active: params[:hook][:active])
-        204
       end
     end
   end

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

@@ -8,7 +8,7 @@ class Travis::Api::App
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found # TODO hrmmmmmm
+        respond_with one(params).run
       end
     end
   end

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

@@ -9,17 +9,17 @@ class Travis::Api::App
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found
+        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).run || not_found
+        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).run(:raise => params[:format] != 'png')
+      #   respond_with service(:repositories, :one, params)
       # end
     end
   end

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

@@ -28,13 +28,11 @@ class Travis::Api::App
       end
 
       put '/:id?', scope: :private do
-        update(params[:user])
+        respond_with update(params[:user])
-        204
       end
 
       post '/sync', scope: :private do
-        service(:users, :sync)
+        respond_with service(:users, :sync)
-        204
       end
     end
   end

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

@@ -8,7 +8,7 @@ class Travis::Api::App
       end
 
       get '/:id' do
-        respond_with one(params).run || not_found # TODO hrmmmmm
+        respond_with one(params)
       end
     end
   end

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

@@ -7,13 +7,8 @@ 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] ||= format_from_content_type || params[:format] || 'json'
-        responders.each do |responder|
+        halt respond(resource, options).to_json
-          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)
@@ -23,8 +18,18 @@ class Travis::Api::App
 
       private
 
-        def responders
+        def respond(resource, options)
-          [Responders::Service, Responders::Json, Responders::Image, Responders::Xml]
+          responders(resource, options).each do |const|
+            responder = const.new(self, resource, options)
+            resource = responder.apply if responder.apply?
+          end
+          resource
+        end
+
+        def responders(resource, options)
+          [:Service, :Json, :Image, :Xml].map do |name|
+            Responders.const_get(name)
+          end
         end
 
         # TODO is there no support for this kind of mime types?

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

@@ -3,7 +3,7 @@ module Travis::Api::App::Helpers::Responders
     attr_reader :endpoint, :resource, :options
 
     def initialize(endpoint, resource, options = {})
-      @endpoint  = endpoint
+      @endpoint = endpoint
       @resource = resource
       @options  = options
     end

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

@@ -8,16 +8,16 @@ module Travis::Api::App::Helpers::Responders
 
     def apply
       headers['Expires'] = Time.now.utc.httpdate
-      halt send_file(filename(resource), type: :png, disposition: :inline)
+      halt send_file(filename, type: :png, disposition: :inline)
     end
 
     private
 
-      def filename(resource)
+      def filename
-        "#{root}/public/images/result/#{result(resource)}.png"
+        "#{root}/public/images/result/#{result}.png"
       end
 
-      def result(resource)
+      def result
         NAMES[resource.try(:last_build_result_on, branch: params[:branch])]
       end
 

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

@@ -4,18 +4,19 @@ module Travis::Api::App::Helpers::Responders
     DEFAULT_VERSION = 'v2'
 
     def apply?
-      !resource.is_a?(String) && options[:format] == 'json'
+      options[:format] == 'json' && !resource.is_a?(String)
     end
 
     def apply
-      resource = builder.new(self.resource, request.params).data if builder
+      halt result.to_json
-      resource ||= self.resource || {}
-      resource.merge!(flash: flash) unless flash.empty?
-      halt resource.to_json
     end
 
     private
 
+      def result
+        builder ? builder.new(resource, request.params).data : resource
+      end
+
       def builder
         @builder ||= Travis::Api.builder(resource, { :version => version }.merge(options))
       end

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

@@ -6,10 +6,27 @@ module Travis::Api::App::Helpers::Responders
 
     def apply
       # TODO add caching headers depending on the resource
-      result = resource.run || {}
+      data = result
-      flash.concat(resource.messages) if resource.respond_to?(:messages)
+      halt 404 if data.nil?
-      result
+      flash.concat(data.messages) if resource.respond_to?(:messages)
+      data
     end
+
+    private
+
+      # 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
+      # so it can be processed by the json responder.
+      # If it's nil we also pass it but immediately yield not_found.
+      def result
+        case result = resource.run
+        when String, true, false
+          { result: result }
+        else
+          result
+        end
+      end
   end
 end
 

spec/integration/v1/builds_spec.rb

@@ -9,7 +9,7 @@ describe 'Builds' do
 
   it 'GET /builds?repository_id=1' do
     response = get '/builds', { repository_id: repo.id }, headers
-    response.should deliver_json_for(repo.builds.was_started.order('id DESC'), version: 'v1')
+    response.should deliver_json_for(repo.builds.order('id DESC'), version: 'v1')
   end
 
   it 'GET /builds/1' do

spec/integration/v1/jobs_spec.rb

@@ -10,7 +10,7 @@ describe 'Jobs' do
 
   it '/jobs?queue=builds.common' do
     response = get '/jobs', { queue: 'builds.common' }, headers
-    response.should deliver_json_for(Job.queued('builds.common'), versin: 'v1')
+    response.should deliver_json_for(Job.queued('builds.common'), version: 'v1')
   end
 
   it '/jobs/:job_id' do

spec/integration/v2/builds_spec.rb

@@ -9,7 +9,7 @@ describe 'Builds' do
 
   it 'GET /builds?repository_id=1' do
     response = get '/builds', { repository_id: repo.id }, headers
-    response.should deliver_json_for(repo.builds.was_started.order('id DESC'), version: 'v2')
+    response.should deliver_json_for(repo.builds.order('id DESC'), version: 'v2')
   end
 
   it 'GET /builds/1' do