Skip to content

Commit

Permalink
Adds after filters
Browse files Browse the repository at this point in the history
Originally by jschementi (http://bit.ly/1RTt2H)
Updated for Sinatra 1.0 by rtomayko
  • Loading branch information
jschementi authored and rtomayko committed Nov 11, 2009
1 parent f375530 commit 4e50ddb
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 23 deletions.
10 changes: 9 additions & 1 deletion README.rdoc
Expand Up @@ -297,9 +297,17 @@ filters are accessible by routes and templates:
params[:splat] #=> 'bar/baz'
end

After filter are evaluated after each request within the context of the
require and can also modify the request and response. Instance variables
set in before filters and routes are accessible by after filters:

after do
puts response.status
end

== Halting

To immediately stop a request during a before filter or route use:
To immediately stop a request within a filter or route use:

halt

Expand Down
54 changes: 35 additions & 19 deletions lib/sinatra/base.rb
Expand Up @@ -65,7 +65,7 @@ class NotFound < NameError #:nodoc:
def code ; 404 ; end
end

# Methods available to routes, before filters, and views.
# Methods available to routes, before/after filters, and views.
module Helpers
# Set or retrieve the response status code.
def status(value=nil)
Expand Down Expand Up @@ -432,9 +432,15 @@ def forward

private
# Run before filters defined on the class and all superclasses.
def filter!(base=self.class)
filter!(base.superclass) if base.superclass.respond_to?(:filters)
base.filters.each { |block| instance_eval(&block) }
def before_filter!(base=self.class)
before_filter!(base.superclass) if base.superclass.respond_to?(:before_filters)
base.before_filters.each { |block| instance_eval(&block) }
end

# Run after filters defined on the class and all superclasses.
def after_filter!(base=self.class)
after_filter!(base.superclass) if base.superclass.respond_to?(:after_filters)
base.after_filters.each { |block| instance_eval(&block) }
end

# Run routes defined on the class and all superclasses.
Expand Down Expand Up @@ -564,12 +570,14 @@ def invoke(&block)
# Dispatch a request with error handling.
def dispatch!
static! if settings.static? && (request.get? || request.head?)
filter!
before_filter!
route!
rescue NotFound => boom
handle_not_found!(boom)
rescue ::Exception => boom
handle_exception!(boom)
ensure
after_filter!
end

def handle_not_found!(boom)
Expand Down Expand Up @@ -623,16 +631,17 @@ def clean_backtrace(trace)
end

class << self
attr_reader :routes, :filters, :templates, :errors
attr_reader :routes, :before_filters, :after_filters, :templates, :errors

def reset!
@conditions = []
@routes = {}
@filters = []
@errors = {}
@middleware = []
@prototype = nil
@extensions = []
@conditions = []
@routes = {}
@before_filters = []
@after_filters = []
@errors = {}
@middleware = []
@prototype = nil
@extensions = []

if superclass.respond_to?(:templates)
@templates = Hash.new { |hash,key| superclass.templates[key] }
Expand Down Expand Up @@ -748,11 +757,18 @@ def mime_type(type, value=nil)
Rack::Mime::MIME_TYPES[type] = value
end

# Define a before filter. Filters are run before all requests
# within the same context as route handlers and may access/modify the
# request and response.
# Define a before filter; runs before all requests within the same
# context as route handlers and may access/modify the request and
# response.
def before(&block)
@filters << block
@before_filters << block
end

# Define an after filter; runs after all requests within the same
# context as route handlers and may access/modify the request and
# response.
def after(&block)
@after_filters << block
end

# Add a route condition. The route is considered non-matching when the
Expand Down Expand Up @@ -1098,8 +1114,8 @@ def #{method_name}(*args, &b)
end
end

delegate :get, :put, :post, :delete, :head, :template, :layout, :before,
:error, :not_found, :configure, :set, :mime_type,
delegate :get, :put, :post, :delete, :head, :template, :layout,
:before, :after, :error, :not_found, :configure, :set, :mime_type,
:enable, :disable, :use, :development?, :test?,
:production?, :use_in_file_templates!, :helpers
end
Expand Down
90 changes: 87 additions & 3 deletions test/filter_test.rb
@@ -1,6 +1,6 @@
require File.dirname(__FILE__) + '/helper'

class FilterTest < Test::Unit::TestCase
class BeforeFilterTest < Test::Unit::TestCase
it "executes filters in the order defined" do
count = 0
mock_app do
Expand All @@ -21,7 +21,7 @@ class FilterTest < Test::Unit::TestCase
assert_equal 'Hello World', body
end

it "allows filters to modify the request" do
it "can modify the request" do
mock_app {
get('/foo') { 'foo' }
get('/bar') { 'bar' }
Expand All @@ -44,7 +44,7 @@ class FilterTest < Test::Unit::TestCase
assert_equal 'bar', body
end

it "allows redirects in filters" do
it "allows redirects" do
mock_app {
before { redirect '/bar' }
get('/foo') do
Expand Down Expand Up @@ -109,3 +109,87 @@ class FilterTest < Test::Unit::TestCase
assert_equal 'hello from superclass', body
end
end

class AfterFilterTest < Test::Unit::TestCase
it "executes filters in the order defined" do
invoked = 0
mock_app do
before { invoked = 2 }
get('/') { invoked += 2 }
after { invoked *= 2 }
end

get '/'
assert ok?

assert_equal 8, invoked
end

it "executes filters in the order defined" do
count = 0
mock_app do
get('/') { 'Hello World' }
after {
assert_equal 0, count
count = 1
}
after {
assert_equal 1, count
count = 2
}
end

get '/'
assert ok?
assert_equal 2, count
assert_equal 'Hello World', body
end

it "allows redirects" do
mock_app {
get('/foo') { 'ORLY' }
after { redirect '/bar' }
}

get '/foo'
assert redirect?
assert_equal '/bar', response['Location']
assert_equal '', body
end

it "does not modify the response with its return value" do
mock_app {
get('/foo') { 'cool' }
after { 'Hello World!' }
}

get '/foo'
assert ok?
assert_equal 'cool', body
end

it "does modify the response with halt" do
mock_app {
get '/foo' do
"should not be returned"
end
after { halt 302, 'Hi' }
}

get '/foo'
assert_equal 302, response.status
assert_equal 'Hi', body
end

it "runs filters defined in superclasses" do
count = 2
base = Class.new(Sinatra::Base)
base.after { count *= 2 }
mock_app(base) {
get('/foo') { count += 2 }
}

get '/foo'
assert_equal 8, count
end
end

20 comments on commit 4e50ddb

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 11, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! i was just going to start this today, but this will save me that trouble :)

@carlosbrando
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@hallison
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Greate feature! Thanks! I wanted for this feature ...

@seban
Copy link

@seban seban commented on 4e50ddb Nov 13, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! Just a few modified code lines but feature is great.

@rkh
Copy link
Member

@rkh rkh commented on 4e50ddb Nov 13, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jinzhu
Copy link

@jinzhu jinzhu commented on 4e50ddb Nov 13, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, finally, good job!

@joeyrobert
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

@benschwarz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have never needed this feature once.
What are you guys doing wrong?

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its slightly useless in some ways. since you can consider "before" the next request, "after" the last, which i use frequently

@rtomayko
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

benschwarz: I wanted to persist an object to a file on disk after each request without having to explicitly call something in each handler.

visionmedia: that's interesting but you lose the context, so any instance variables set in the previous request are gone.

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 17, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true. thats how i was dealing with it previously tho, totally welcome this feature :)

@alexch
Copy link

@alexch alexch commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Now how about an around filter? I want to wrap each request in profiling, logging, and activating/deactivating the ActiveRecord cache. These are things best done surrounding a yield (and unfortunately the API for the ActiveRecord cache can only be called via yield as far as I can tell).

@hassox
Copy link

@hassox hassox commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is an around filter something that may be more suited to a custom middleware?

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could just do all that with #before / #after no?

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plus middleware is only downstream no? unless you can process stuff after the lowest level app does its thing, in which case thats good to know :D

@hassox
Copy link

@hassox hassox commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

er... when you're talking about sinatra, it is either the lowest level app, or it contains the lowest level app, in either case, the middleware would be "around" the request....

Just a thought...

@alexch
Copy link

@alexch alexch commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visionmedia: no, unfortunately the AR cache has the following API:

get '/foo' do
ActiveRecord::Base.cache do
...
end
end

You can't call half of a yielding function in 'before' and the other half in 'after'.

hassox: yeah, a rack middleware component (I refuse to use middleware as a singular noun) would likely do it but I haven't seen a Rack Around Filter and I haven't managed to wrap my brain around the Rack dispatch mechanism yet in order to write one. The occasional Google search for Rack::Around and the like has been fruitless... any pointers?

@hassox
Copy link

@hassox hassox commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a really really simple custom around filter. http://gist.github.com/238624 The important parts of a bit of middleware kit is an initialize that at least takes an app argument (the next app in the chain), and a call instance method that takes one argument, that environment hash.

All you have to do then is make sure that you're returning either the downstream response (as per the gist) or a valid rack response. You can check it out in the spec at http://rack.rubyforge.org/doc/SPEC.html

in sinatra then, according to that middleware you could do:

use MyCustomAroundMiddleware

easy around actions :)

@alexch
Copy link

@alexch alexch commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So simple! Thanks very much.

Here's a Query Caching gist in return: http://gist.github.com/238847

@tj
Copy link
Contributor

@tj tj commented on 4e50ddb Nov 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah cool yeah that makes sense, i have not needed to implement any middleware like that yet but makes perfect sense.

Please sign in to comment.