diff options
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/nodes/node.rb | 6 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/route.rb | 67 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/journey/visitors.rb | 127 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 17 | ||||
| -rw-r--r-- | actionpack/test/controller/resources_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/dispatch/mapper_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/journey/route_test.rb | 24 |
7 files changed, 174 insertions, 75 deletions
diff --git a/actionpack/lib/action_dispatch/journey/nodes/node.rb b/actionpack/lib/action_dispatch/journey/nodes/node.rb index 2b57c368dc..542e8c6577 100644 --- a/actionpack/lib/action_dispatch/journey/nodes/node.rb +++ b/actionpack/lib/action_dispatch/journey/nodes/node.rb @@ -14,15 +14,15 @@ module ActionDispatch end def each(&block) - Visitors::Each.new(block).accept(self) + Visitors::Each::INSTANCE.accept(self, block) end def to_s - Visitors::String.new.accept(self) + Visitors::String::INSTANCE.accept(self, '') end def to_dot - Visitors::Dot.new.accept(self) + Visitors::Dot::INSTANCE.accept(self) end def to_sym diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index f9e0e360c3..5d5fe4698d 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -8,14 +8,61 @@ module ActionDispatch attr_accessor :precedence + module VerbMatchers + VERBS = %w{ DELETE GET HEAD OPTIONS LINK PATCH POST PUT TRACE UNLINK } + VERBS.each do |v| + class_eval <<-eoc + class #{v} + def self.verb; name.split("::").last; end + def self.call(req); req.#{v.downcase}?; end + end + eoc + end + + class Unknown + attr_reader :verb + + def initialize(verb) + @verb = verb + end + + def call(request); @verb === request.request_method; end + end + + class All + def self.call(_); true; end + def self.verb; ''; end + end + + VERB_TO_CLASS = VERBS.each_with_object({ :all => All }) do |verb, hash| + klass = const_get verb + hash[verb] = klass + hash[verb.downcase] = klass + hash[verb.downcase.to_sym] = klass + end + + end + + def self.verb_matcher(verb) + VerbMatchers::VERB_TO_CLASS.fetch(verb) do + VerbMatchers::Unknown.new verb.to_s.dasherize.upcase + end + end + + def self.build(name, app, path, constraints, required_defaults, defaults) + request_method_match = verb_matcher(constraints.delete(:request_method)) || [] + new name, app, path, constraints, required_defaults, defaults, request_method_match + end + ## # +path+ is a path constraint. # +constraints+ is a hash of constraints to be applied to this route. - def initialize(name, app, path, constraints, required_defaults, defaults) + def initialize(name, app, path, constraints, required_defaults, defaults, request_method_match) @name = name @app = app @path = path + @request_method_match = request_method_match @constraints = constraints @defaults = defaults @required_defaults = nil @@ -92,7 +139,8 @@ module ActionDispatch end def matches?(request) - constraints.all? do |method, value| + match_verb(request) && + constraints.all? { |method, value| case value when Regexp, String value === request.send(method).to_s @@ -105,7 +153,7 @@ module ActionDispatch else value === request.send(method) end - end + } end def ip @@ -113,11 +161,20 @@ module ActionDispatch end def requires_matching_verb? - constraints[:request_method] + !@request_method_match.all? { |x| x == VerbMatchers::All } end def verb - constraints[:request_method] || // + %r[^#{verbs.join('|')}$] + end + + private + def verbs + @request_method_match.map(&:verb) + end + + def match_verb(request) + @request_method_match.any? { |m| m.call request } end end end diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb index 52b4c8b489..537c9b2f5c 100644 --- a/actionpack/lib/action_dispatch/journey/visitors.rb +++ b/actionpack/lib/action_dispatch/journey/visitors.rb @@ -92,6 +92,45 @@ module ActionDispatch end end + class FunctionalVisitor # :nodoc: + DISPATCH_CACHE = {} + + def accept(node, seed) + visit(node, seed) + end + + def visit node, seed + send(DISPATCH_CACHE[node.type], node, seed) + end + + def binary(node, seed) + visit(node.right, visit(node.left, seed)) + end + def visit_CAT(n, seed); binary(n, seed); end + + def nary(node, seed) + node.children.inject(seed) { |s, c| visit(c, s) } + end + def visit_OR(n, seed); nary(n, seed); end + + def unary(node, seed) + visit(node.left, seed) + end + def visit_GROUP(n, seed); unary(n, seed); end + def visit_STAR(n, seed); unary(n, seed); end + + def terminal(node, seed); seed; end + def visit_LITERAL(n, seed); terminal(n, seed); end + def visit_SYMBOL(n, seed); terminal(n, seed); end + def visit_SLASH(n, seed); terminal(n, seed); end + def visit_DOT(n, seed); terminal(n, seed); end + + instance_methods(false).each do |pim| + next unless pim =~ /^visit_(.*)$/ + DISPATCH_CACHE[$1.to_sym] = pim + end + end + class FormatBuilder < Visitor # :nodoc: def accept(node); Journey::Format.new(super); end def terminal(node); [node.left]; end @@ -117,104 +156,110 @@ module ActionDispatch end # Loop through the requirements AST - class Each < Visitor # :nodoc: - attr_reader :block - - def initialize(block) - @block = block - end - - def visit(node) + class Each < FunctionalVisitor # :nodoc: + def visit(node, block) block.call(node) super end + + INSTANCE = new end - class String < Visitor # :nodoc: + class String < FunctionalVisitor # :nodoc: private - def binary(node) - [visit(node.left), visit(node.right)].join + def binary(node, seed) + visit(node.right, visit(node.left, seed)) end - def nary(node) - node.children.map { |c| visit(c) }.join '|' + def nary(node, seed) + last_child = node.children.last + node.children.inject(seed) { |s, c| + string = visit(c, s) + string << "|".freeze unless last_child == c + string + } end - def terminal(node) - node.left + def terminal(node, seed) + seed + node.left end - def visit_GROUP(node) - "(#{visit(node.left)})" + def visit_GROUP(node, seed) + visit(node.left, seed << "(".freeze) << ")".freeze end + + INSTANCE = new end - class Dot < Visitor # :nodoc: + class Dot < FunctionalVisitor # :nodoc: def initialize @nodes = [] @edges = [] end - def accept(node) + def accept(node, seed = [[], []]) super + nodes, edges = seed <<-eodot digraph parse_tree { size="8,5" node [shape = none]; edge [dir = none]; - #{@nodes.join "\n"} - #{@edges.join("\n")} + #{nodes.join "\n"} + #{edges.join("\n")} } eodot end private - def binary(node) - node.children.each do |c| - @edges << "#{node.object_id} -> #{c.object_id};" - end + def binary(node, seed) + seed.last.concat node.children.map { |c| + "#{node.object_id} -> #{c.object_id};" + } super end - def nary(node) - node.children.each do |c| - @edges << "#{node.object_id} -> #{c.object_id};" - end + def nary(node, seed) + seed.last.concat node.children.map { |c| + "#{node.object_id} -> #{c.object_id};" + } super end - def unary(node) - @edges << "#{node.object_id} -> #{node.left.object_id};" + def unary(node, seed) + seed.last << "#{node.object_id} -> #{node.left.object_id};" super end - def visit_GROUP(node) - @nodes << "#{node.object_id} [label=\"()\"];" + def visit_GROUP(node, seed) + seed.first << "#{node.object_id} [label=\"()\"];" super end - def visit_CAT(node) - @nodes << "#{node.object_id} [label=\"○\"];" + def visit_CAT(node, seed) + seed.first << "#{node.object_id} [label=\"○\"];" super end - def visit_STAR(node) - @nodes << "#{node.object_id} [label=\"*\"];" + def visit_STAR(node, seed) + seed.first << "#{node.object_id} [label=\"*\"];" super end - def visit_OR(node) - @nodes << "#{node.object_id} [label=\"|\"];" + def visit_OR(node, seed) + seed.first << "#{node.object_id} [label=\"|\"];" super end - def terminal(node) + def terminal(node, seed) value = node.left - @nodes << "#{node.object_id} [label=\"#{value}\"];" + seed.first << "#{node.object_id} [label=\"#{value}\"];" + seed end + INSTANCE = new end end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 2c9cab605f..0afe1852f2 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -149,7 +149,8 @@ module ActionDispatch path, conditions, required_defaults, - defaults) + defaults, + request_method) route.precedence = precedence route @@ -170,21 +171,17 @@ module ActionDispatch def build_conditions(current_conditions, request_class) conditions = current_conditions.dup - # Rack-Mount requires that :request_method be a regular expression. - # :request_method represents the HTTP verb that matches this route. - # - # Here we munge values before they get sent on to rack-mount. - unless @via == [:all] - verbs = @via.map { |m| m.to_s.dasherize.upcase } - conditions[:request_method] = %r[^#{verbs.join('|')}$] - end - conditions.keep_if do |k, _| request_class.public_method_defined?(k) end end private :build_conditions + def request_method + @via.map { |x| Journey::Route.verb_matcher(x) } + end + private :request_method + JOINED_SEPARATORS = SEPARATORS.join # :nodoc: def build_path(ast, requirements, anchor) diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 04d6cc1792..dd7c128566 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -505,8 +505,8 @@ class ResourcesTest < ActionController::TestCase routes = @routes.routes routes.each do |route| routes.each do |r| - next if route === r # skip the comparison instance - assert_not_equal [route.conditions, route.path.spec.to_s], [r.conditions, r.path.spec.to_s] + next if route == r # skip the comparison instance + assert_not_equal [route.conditions, route.path.spec.to_s, route.verb], [r.conditions, r.path.spec.to_s, r.verb] end end end diff --git a/actionpack/test/dispatch/mapper_test.rb b/actionpack/test/dispatch/mapper_test.rb index eca4ca8e0e..f35ffd8845 100644 --- a/actionpack/test/dispatch/mapper_test.rb +++ b/actionpack/test/dispatch/mapper_test.rb @@ -82,7 +82,7 @@ module ActionDispatch end assert_equal({:omg=>:awesome, :controller=>"posts", :action=>"index"}, fakeset.defaults.first) - assert_equal(/^GET$/, fakeset.conditions.first[:request_method]) + assert_equal(/^GET$/, fakeset.routes.first.verb) end def test_mapping_requirements @@ -99,7 +99,7 @@ module ActionDispatch mapper.scope(via: :put) do mapper.match '/', :to => 'posts#index', :as => :main end - assert_equal(/^PUT$/, fakeset.conditions.first[:request_method]) + assert_equal(/^PUT$/, fakeset.routes.first.verb) end def test_map_slash diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index 75e41193b4..22c3b8113d 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -7,7 +7,7 @@ module ActionDispatch app = Object.new path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} - route = Route.new("name", app, path, {}, [], defaults) + route = Route.build("name", app, path, {}, [], defaults) assert_equal app, route.app assert_equal path, route.path @@ -18,7 +18,7 @@ module ActionDispatch app = Object.new path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' defaults = {} - route = Route.new("name", app, path, {}, [], defaults) + route = Route.build("name", app, path, {}, [], defaults) route.ast.grep(Nodes::Terminal).each do |node| assert_equal route, node.memo @@ -28,27 +28,27 @@ module ActionDispatch def test_path_requirements_override_defaults path = Path::Pattern.build(':name', { name: /love/ }, '/', true) defaults = { name: 'tender' } - route = Route.new('name', nil, path, nil, [], defaults) + route = Route.build('name', nil, path, {}, [], defaults) assert_equal(/love/, route.requirements[:name]) end def test_ip_address path = Path::Pattern.from_string '/messages/:id(.:format)' - route = Route.new("name", nil, path, {:ip => '192.168.1.1'}, [], + route = Route.build("name", nil, path, {:ip => '192.168.1.1'}, [], { :controller => 'foo', :action => 'bar' }) assert_equal '192.168.1.1', route.ip end def test_default_ip path = Path::Pattern.from_string '/messages/:id(.:format)' - route = Route.new("name", nil, path, {}, [], + route = Route.build("name", nil, path, {}, [], { :controller => 'foo', :action => 'bar' }) assert_equal(//, route.ip) end def test_format_with_star path = Path::Pattern.from_string '/:controller/*extra' - route = Route.new("name", nil, path, {}, [], + route = Route.build("name", nil, path, {}, [], { :controller => 'foo', :action => 'bar' }) assert_equal '/foo/himom', route.format({ :controller => 'foo', @@ -58,7 +58,7 @@ module ActionDispatch def test_connects_all_match path = Path::Pattern.from_string '/:controller(/:action(/:id(.:format)))' - route = Route.new("name", nil, path, {:action => 'bar'}, [], { :controller => 'foo' }) + route = Route.build("name", nil, path, {:action => 'bar'}, [], { :controller => 'foo' }) assert_equal '/foo/bar/10', route.format({ :controller => 'foo', @@ -69,21 +69,21 @@ module ActionDispatch def test_extras_are_not_included_if_optional path = Path::Pattern.from_string '/page/:id(/:action)' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/page/10', route.format({ :id => 10 }) end def test_extras_are_not_included_if_optional_with_parameter path = Path::Pattern.from_string '(/sections/:section)/pages/:id' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/pages/10', route.format({:id => 10}) end def test_extras_are_not_included_if_optional_parameter_is_nil path = Path::Pattern.from_string '(/sections/:section)/pages/:id' - route = Route.new("name", nil, path, { }, [], { :action => 'show' }) + route = Route.build("name", nil, path, { }, [], { :action => 'show' }) assert_equal '/pages/10', route.format({:id => 10, :section => nil}) end @@ -93,10 +93,10 @@ module ActionDispatch defaults = {:controller=>"pages", :action=>"show"} path = Path::Pattern.from_string "/page/:id(/:action)(.:format)" - specific = Route.new "name", nil, path, constraints, [:controller, :action], defaults + specific = Route.build "name", nil, path, constraints, [:controller, :action], defaults path = Path::Pattern.from_string "/:controller(/:action(/:id))(.:format)" - generic = Route.new "name", nil, path, constraints, [], {} + generic = Route.build "name", nil, path, constraints, [], {} knowledge = {:id=>20, :controller=>"pages", :action=>"show"} |