From 494a85d968ecb2c77e59c0ef51a66af2d3f0ac2a Mon Sep 17 00:00:00 2001 From: Sven Fuchs Date: Tue, 9 Oct 2012 02:48:19 +0200 Subject: [PATCH] refactor responders --- Gemfile.lock | 4 +-- lib/travis/api/app/endpoint.rb | 3 ++- lib/travis/api/app/endpoint/accounts.rb | 2 +- lib/travis/api/app/endpoint/artifacts.rb | 2 +- lib/travis/api/app/endpoint/branches.rb | 4 +-- lib/travis/api/app/endpoint/builds.rb | 12 ++++----- lib/travis/api/app/endpoint/hooks.rb | 4 +-- lib/travis/api/app/endpoint/jobs.rb | 4 +-- lib/travis/api/app/endpoint/repositories.rb | 2 +- lib/travis/api/app/endpoint/requests.rb | 3 +-- lib/travis/api/app/endpoint/users.rb | 6 ++--- lib/travis/api/app/endpoint/workers.rb | 4 +-- lib/travis/api/app/helpers/flash.rb | 11 ++++++++ lib/travis/api/app/helpers/respond_with.rb | 23 +++++++--------- lib/travis/api/app/helpers/responders.rb | 13 +++++++++ lib/travis/api/app/helpers/responders/base.rb | 27 ++++++++++++++++--- .../api/app/helpers/responders/image.rb | 8 ++++-- lib/travis/api/app/helpers/responders/json.rb | 23 +++++++++------- .../api/app/helpers/responders/service.rb | 15 +++++++++++ lib/travis/api/app/helpers/responders/xml.rb | 8 ++++-- 20 files changed, 121 insertions(+), 57 deletions(-) create mode 100644 lib/travis/api/app/helpers/flash.rb create mode 100644 lib/travis/api/app/helpers/responders.rb create mode 100644 lib/travis/api/app/helpers/responders/service.rb diff --git a/Gemfile.lock b/Gemfile.lock index 37effc92..4f5617d9 100644 --- a/Gemfile.lock +++ b/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) diff --git a/lib/travis/api/app/endpoint.rb b/lib/travis/api/app/endpoint.rb index 4f3aa391..0eb0d1d0 100644 --- a/lib/travis/api/app/endpoint.rb +++ b/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 } diff --git a/lib/travis/api/app/endpoint/accounts.rb b/lib/travis/api/app/endpoint/accounts.rb index 21ca9c80..6f2597e9 100644 --- a/lib/travis/api/app/endpoint/accounts.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/artifacts.rb b/lib/travis/api/app/endpoint/artifacts.rb index eb86fca2..81a52b77 100644 --- a/lib/travis/api/app/endpoint/artifacts.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/branches.rb b/lib/travis/api/app/endpoint/branches.rb index f5931bfd..671de627 100644 --- a/lib/travis/api/app/endpoint/branches.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/builds.rb b/lib/travis/api/app/endpoint/builds.rb index ba534edd..428665db 100644 --- a/lib/travis/api/app/endpoint/builds.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/hooks.rb b/lib/travis/api/app/endpoint/hooks.rb index 2ac0dab7..d72c071b 100644 --- a/lib/travis/api/app/endpoint/hooks.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/jobs.rb b/lib/travis/api/app/endpoint/jobs.rb index edf22132..86e3f1c5 100644 --- a/lib/travis/api/app/endpoint/jobs.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/repositories.rb b/lib/travis/api/app/endpoint/repositories.rb index 5524921c..dac8ac66 100644 --- a/lib/travis/api/app/endpoint/repositories.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/requests.rb b/lib/travis/api/app/endpoint/requests.rb index 8d659820..26618d16 100644 --- a/lib/travis/api/app/endpoint/requests.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/users.rb b/lib/travis/api/app/endpoint/users.rb index d2dc9c59..c2e15339 100644 --- a/lib/travis/api/app/endpoint/users.rb +++ b/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 diff --git a/lib/travis/api/app/endpoint/workers.rb b/lib/travis/api/app/endpoint/workers.rb index 22a4af9d..ca64578f 100644 --- a/lib/travis/api/app/endpoint/workers.rb +++ b/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 diff --git a/lib/travis/api/app/helpers/flash.rb b/lib/travis/api/app/helpers/flash.rb new file mode 100644 index 00000000..9fb0c33a --- /dev/null +++ b/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 diff --git a/lib/travis/api/app/helpers/respond_with.rb b/lib/travis/api/app/helpers/respond_with.rb index 48d3ec6f..ec0526b4 100644 --- a/lib/travis/api/app/helpers/respond_with.rb +++ b/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? diff --git a/lib/travis/api/app/helpers/responders.rb b/lib/travis/api/app/helpers/responders.rb new file mode 100644 index 00000000..0bc1be25 --- /dev/null +++ b/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 diff --git a/lib/travis/api/app/helpers/responders/base.rb b/lib/travis/api/app/helpers/responders/base.rb index fa639fb7..28d51556 100644 --- a/lib/travis/api/app/helpers/responders/base.rb +++ b/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 diff --git a/lib/travis/api/app/helpers/responders/image.rb b/lib/travis/api/app/helpers/responders/image.rb index a338ed71..4e7cd387 100644 --- a/lib/travis/api/app/helpers/responders/image.rb +++ b/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 diff --git a/lib/travis/api/app/helpers/responders/json.rb b/lib/travis/api/app/helpers/responders/json.rb index e05c50ab..06f9d218 100644 --- a/lib/travis/api/app/helpers/responders/json.rb +++ b/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 diff --git a/lib/travis/api/app/helpers/responders/service.rb b/lib/travis/api/app/helpers/responders/service.rb new file mode 100644 index 00000000..bd0aec1e --- /dev/null +++ b/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 + diff --git a/lib/travis/api/app/helpers/responders/xml.rb b/lib/travis/api/app/helpers/responders/xml.rb index 39eca8c4..d87dd563 100644 --- a/lib/travis/api/app/helpers/responders/xml.rb +++ b/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