Better route and filter inheritance [#180]

lib/sinatra/base.rb

@@ -357,7 +357,7 @@ module Sinatra
       @env      = env
       @request  = Request.new(env)
       @response = Response.new
-      @params   = nil
+      @params   = indifferent_params(@request.params)
 
       invoke { dispatch! }
       invoke { error_block!(response.status) }
@@ -405,15 +405,15 @@ module Sinatra
     end
 
   private
-    # Run before filters and then locate and run a matching route.
+    # Run before filters defined on the class and all superclasses.
-    def route!
+    def filter!(base=self.class)
-      @params = indifferent_params(@request.params)
+      filter!(base.superclass) if base.superclass.respond_to?(:filters)
+      base.filters.each { |block| instance_eval(&block) }
+    end
 
-      # before filters
+    # Run routes defined on the class and all superclasses.
-      self.class.filters.each { |block| instance_eval(&block) }
+    def route!(base=self.class)
-
+      if routes = base.routes[@request.request_method]
-      # routes
-      if routes = self.class.routes[@request.request_method]
         original_params = @params
         path            = unescape(@request.path_info)
 
@@ -445,6 +445,14 @@ module Sinatra
             end
           end
         end
+
+        @params = original_params
+      end
+
+      # Run routes defined in superclass.
+      if base.superclass.respond_to?(:routes)
+        route! base.superclass
+        return
       end
 
       route_missing
@@ -468,6 +476,19 @@ module Sinatra
       end
     end
 
+    # Attempt to serve static files from public directory. Throws :halt when
+    # a matching file is found, returns nil otherwise.
+    def static!
+      return if (public_dir = options.public).nil?
+      public_dir = File.expand_path(public_dir)
+
+      path = File.expand_path(public_dir + unescape(request.path_info))
+      return if path[0, public_dir.length] != public_dir
+      return unless File.file?(path)
+
+      send_file path, :disposition => nil
+    end
+
     # Enable string or symbol key access to the nested params hash.
     def indifferent_params(params)
       params = indifferent_hash.merge(params)
@@ -516,6 +537,8 @@ module Sinatra
 
     # Dispatch a request with error handling.
     def dispatch!
+      filter!
+      static! if options.static? && (request.get? || request.head?)
       route!
     rescue NotFound => boom
       handle_not_found!(boom)
@@ -863,24 +886,16 @@ module Sinatra
       end
 
       def reset!(base=superclass)
-        @routes     = base.dupe_routes
+        @routes     = {}
         @templates  = base.templates.dup
         @conditions = []
-        @filters    = base.filters.dup
+        @filters    = []
         @errors     = base.errors.dup
         @middleware = base.middleware.dup
         @prototype  = nil
         @extensions = []
       end
 
-    protected
-      def dupe_routes
-        routes.inject({}) do |hash,(request_method,routes)|
-          hash[request_method] = routes.dup
-          hash
-        end
-      end
-
     private
       def detect_rack_handler
         servers = Array(self.server)
@@ -959,16 +974,6 @@ module Sinatra
     set :public, Proc.new { root && File.join(root, 'public') }
     set :lock, false
 
-    # static files route
-    get(/.*[^\/]$/) do
-      pass unless options.static? && options.public?
-      public_dir = File.expand_path(options.public)
-      path = File.expand_path(public_dir + unescape(request.path_info))
-      pass if path[0, public_dir.length] != public_dir
-      pass unless File.file?(path)
-      send_file path, :disposition => nil
-    end
-
     error ::Exception do
       response.status = 500
       content_type 'text/html'

test/filter_test.rb

@@ -96,4 +96,16 @@ class FilterTest < Test::Unit::TestCase
     assert ok?
     assert_equal 'cool', body
   end
+
+  it "runs filters defined in superclasses" do
+    base = Class.new(Sinatra::Base)
+    base.before { @foo = 'hello from superclass' }
+
+    mock_app(base) {
+      get('/foo') { @foo }
+    }
+
+    get '/foo'
+    assert_equal 'hello from superclass', body
+  end
 end

test/routing_test.rb

@@ -781,4 +781,39 @@ class RoutingTest < Test::Unit::TestCase
     end
 
   end
+
+  it "matches routes defined in superclasses" do
+    base = Class.new(Sinatra::Base)
+    base.get('/foo') { 'foo in baseclass' }
+
+    mock_app(base) {
+      get('/bar') { 'bar in subclass' }
+    }
+
+    get '/foo'
+    assert ok?
+    assert_equal 'foo in baseclass', body
+
+    get '/bar'
+    assert ok?
+    assert_equal 'bar in subclass', body
+  end
+
+  it "matches routes in subclasses before superclasses" do
+    base = Class.new(Sinatra::Base)
+    base.get('/foo') { 'foo in baseclass' }
+    base.get('/bar') { 'bar in baseclass' }
+
+    mock_app(base) {
+      get('/foo') { 'foo in subclass' }
+    }
+
+    get '/foo'
+    assert ok?
+    assert_equal 'foo in subclass', body
+
+    get '/bar'
+    assert ok?
+    assert_equal 'bar in baseclass', body
+  end
 end

test/static_test.rb

@@ -34,6 +34,13 @@ class StaticTest < Test::Unit::TestCase
     assert response.headers.include?('Last-Modified')
   end
 
+  %w[POST PUT DELETE].each do |verb|
+    it "does not serve #{verb} requests" do
+      send verb.downcase, "/#{File.basename(__FILE__)}"
+      assert_equal 404, status
+    end
+  end
+
   it 'serves files in preference to custom routes' do
     @app.get("/#{File.basename(__FILE__)}") { 'Hello World' }
     get "/#{File.basename(__FILE__)}"