Permalink
Browse files

started what's probably a misguided effort to refactor request handli…

…ng into a Request class. not going so well...
  • Loading branch information...
1 parent 0a5afd7 commit fd09e3fd933b41d1aae049850665a5910b7b3f36 @nakajima committed Feb 1, 2009
@@ -19,6 +19,10 @@ class MountedApp < Sinatra::Base
finder { |model, params| model.all }
record { |model, params| model.first(:id => params[:id]) }
+ caches :show do
+ set :etag,
+ end
+
# Mount children as a nested resource
mount(Comment) do
finder { |model, params| model.all }
View
@@ -18,5 +18,6 @@
require 'sinatras-hat/responder'
require 'sinatras-hat/model'
require 'sinatras-hat/router'
+require 'sinatras-hat/request'
require 'sinatras-hat/actions'
require 'sinatras-hat/maker'
@@ -10,49 +10,45 @@ module Hat
module Actions
def self.included(map)
map.action :destroy, '/:id', :verb => :delete do |request|
- record = model.find(request.params) || responder.not_found(request)
- record.destroy
- responder.success(:destroy, request, record)
+ set :result, model.find(request.params)
+ result.destroy rescue nil
+ set :success, true
end
map.action :new, '/new' do |request|
- new_record = model.new(request.params)
- responder.success(:new, request, new_record)
+ set :result, model.new(request.params)
+ set :success, true
end
map.action :update, '/:id', :verb => :put do |request|
- record = model.update(request.params) || responder.not_found(request)
- result = record.save ? :success : :failure
- responder.send(result, :update, request, record)
+ set :result, model.update(request.params)
+ set :success, result.save rescue false
end
map.action :edit, '/:id/edit' do |request|
- record = model.find(request.params) || responder.not_found(request)
- responder.success(:edit, request, record)
+ set :result, model.find(request.params)
+ set :success, true
end
map.action :show, '/:id' do |request|
- record = model.find(request.params) || responder.not_found(request)
- set_cache_headers(request, record)
- responder.success(:show, request, record)
+ set :result, model.find(request.params)
+ set :success, result
end
map.action :create, '/', :verb => :post do |request|
record = model.new(request.params)
- result = record.save ? :success : :failure
- responder.send(result, :create, request, record)
+ set :success, record.save
+ set :result, record
end
map.action :index, '/' do |request|
- records = model.all(request.params)
- set_cache_headers(request, records)
- responder.success(:index, request, records)
+ set :result, model.all(request.params)
+ set :success, true
end
private
def set_cache_headers(request, data)
-
set_etag(request, data)
set_last_modified(request, data)
end
@@ -36,7 +36,12 @@ def handle(action, request)
protect!(request) if protect.include?(action)
log_with_benchmark(request, action) do
- instance_exec(request, &self.class.actions[action][:fn])
+ r = Request.new(request, action)
+ r.set_last_modified(model)
+ r.perform!
+ r.success? ?
+ responder.success(action, request, r.result) :
+ responder.failure(action, request, r.result)
end
end
@@ -0,0 +1,58 @@
+module Sinatra
+ module Hat
+ class Request
+ attr_accessor :result
+ attr_writer :success
+ attr_reader :request, :action, :model
+
+ def self.cache_header_definitions
+ @cache_headers ||= {
+ :index => {
+ :last_modified => proc { |model, params|
+ if record = model.klass.first(:order => 'updated_at DESC')
+ record.updated_at
+ end
+ }
+ },
+
+ :show => {
+ :last_modified => proc { |model, params|
+ if record = model.find(params)
+ record.updated_at
+ end
+ }
+ }
+ }
+ end
+
+ def initialize(request, action)
+ @request, @action = request, action
+ end
+
+ def perform!
+ instance_exec(request, &Maker.actions[action][:fn])
+ request.not_found if result.nil?
+ end
+
+ def success
+ @success ||= true
+ end
+
+ alias_method :success?, :success
+
+ def set_last_modified(model)
+ @model = model
+ return unless cache_headers
+ request.last_modified cache_headers[:last_modified][model, request.params]
+ end
+
+ def cache_headers
+ Request.cache_header_definitions[action]
+ end
+
+ def set(key, value)
+ send("#{key}=", value)
+ end
+ end
+ end
+end
@@ -62,10 +62,6 @@ def serialize(request, data)
formatter = to_format(name)
formatter[data] || request.error(406)
end
-
- def not_found(request)
- request.not_found
- end
private
View
@@ -66,12 +66,15 @@ class Comment; end
before(:each) do
@request = fake_request
@maker = new_maker
- stub(Sinatra::Hat::Maker.actions[:index])[:fn].returns proc { |passed_request| [self, passed_request] }
+ stub(Sinatra::Hat::Maker.actions[:index])[:fn].returns proc { |passed_request|
+ set :result, [self, passed_request]
+ }
end
- it "takes an action and instance_exec's its event handler" do
- mock(Sinatra::Hat::Maker.actions[:index])[:fn].returns proc { |passed_request| [self, passed_request] }
- maker.handle(:index, request).should == [maker, request]
+ it "instantiates a new Request and calls perform!" do
+ mock(stub_request = Sinatra::Hat::Request.new(@request, :index)).perform!
+ mock(Sinatra::Hat::Request).new(@request, :index) { stub_request }
+ maker.handle(:index, request)
end
context "when the action is protected" do
View
@@ -0,0 +1,81 @@
+require 'spec/spec_helper'
+
+describe Sinatra::Hat::Request do
+ def new_request(request=fake_request, action=:show)
+ Sinatra::Hat::Request.new(request, action)
+ end
+
+ describe "perform!" do
+ it "instance_exec's the handler block, passing the request" do
+ Sinatra::Hat::Maker.actions[:show] = { :fn => proc { |r| called_with(r); set :result, :ok } }
+
+ sinatra_request = fake_request
+
+ request = new_request(sinatra_request)
+
+ mock(request).called_with(sinatra_request)
+ mock.proxy(request).instance_exec(sinatra_request)
+
+ request.perform!
+ end
+
+ it "calls not_found when result is nil" do
+ Sinatra::Hat::Maker.actions[:show] = { :fn => proc { set :result, nil } }
+ mock(sinatra_request = fake_request).not_found
+ new_request(sinatra_request).perform!
+ end
+ end
+
+ describe "#set" do
+ it "sets the attr value" do
+ request = new_request
+ request.set(:result, :the_result)
+ request.result.should == :the_result
+ end
+ end
+
+ describe "setting cache headers" do
+ # This feels a bit awkard still...
+ describe "#set_last_modified" do
+ describe "for show" do
+ before(:each) do
+ @sinatra_request = fake_request
+ @request = new_request(@sinatra_request, :show)
+ @request.set :result, :article
+ end
+
+ it "sets last_modified to results of cache header definition" do
+ Sinatra::Hat::Request.cache_header_definitions[:show][:last_modified] = proc { :the_result }
+ mock(@sinatra_request).last_modified(:the_result)
+ @request.set_last_modified(:some_model)
+ end
+
+ it "passes the model, params, and result" do
+ Sinatra::Hat::Request.cache_header_definitions[:show][:last_modified] = proc { |*a| a }
+ mock(@sinatra_request).last_modified([:some_model, { }])
+ @request.set_last_modified(:some_model)
+ end
+ end
+
+ describe "for index" do
+ before(:each) do
+ @sinatra_request = fake_request
+ @request = new_request(@sinatra_request, :index)
+ @request.set :result, [:article]
+ end
+
+ it "sets last_modified to results of cache header definition" do
+ Sinatra::Hat::Request.cache_header_definitions[:index][:last_modified] = proc { :the_result }
+ mock(@sinatra_request).last_modified(:the_result)
+ @request.set_last_modified(:some_model)
+ end
+
+ it "passes the model, params, and result" do
+ Sinatra::Hat::Request.cache_header_definitions[:index][:last_modified] = proc { |*a| a }
+ mock(@sinatra_request).last_modified([:some_model, { }])
+ @request.set_last_modified(:some_model)
+ end
+ end
+ end
+ end
+end
View
@@ -66,13 +66,6 @@ def new_responder(maker=@maker)
end
end
- describe "not_found" do
- it "calls not_found on the request" do
- mock(request = fake_request).not_found
- new_responder.not_found(request)
- end
- end
-
describe "failure" do
# context "when there's a format" do
# it "serializes the response" do
View
@@ -75,6 +75,7 @@ def fake_request(options={})
stub(request).response.returns(Sinatra::Response.new)
stub(request).last_modified(anything)
stub(request).etag(anything)
+ stub(request).not_found
request
end

0 comments on commit fd09e3f

Please sign in to comment.