Permalink
Browse files

Added custom regexps to ASTs that have literal nodes on either side of

symbol nodes.  Fixes #4585
  • Loading branch information...
1 parent b840461 commit 3913b510b7136a90113fecf2699ea218e849aaa7 @tenderlove tenderlove committed Jan 24, 2012
Showing with 82 additions and 1 deletion.
  1. +20 −1 actionpack/lib/action_dispatch/routing/route_set.rb
  2. +62 −0 actionpack/test/controller/routing_test.rb
@@ -367,7 +367,26 @@ def build_path(path, requirements, separators, anchor)
SEPARATORS,
anchor)
- Journey::Path::Pattern.new(strexp)
+ pattern = Journey::Path::Pattern.new(strexp)
+
+ builder = Journey::GTG::Builder.new pattern.spec
+
+ # Get all the symbol nodes followed by literals that are not the
+ # dummy node.
+ symbols = pattern.spec.grep(Journey::Nodes::Symbol).find_all { |n|
+ builder.followpos(n).first.literal?
+ }
+
+ # Get all the symbol nodes preceded by literals.
+ symbols.concat pattern.spec.find_all(&:literal?).map { |n|
+ builder.followpos(n).first
+ }.find_all(&:symbol?)
+
+ symbols.each { |x|
+ x.regexp = /(?:#{Regexp.union(x.regexp, '-')})+/
+ }
+
+ pattern
end
private :build_path
@@ -93,6 +93,68 @@ def get(uri_or_host, path = nil, port = nil)
@rs.call(params)[2].join
end
+ def test_symbols_with_dashes
+ rs.draw do
+ match '/:artist/:song-omg', :to => lambda { |env|
+ resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+ [200, {}, [resp]]
+ }
+ end
+
+ hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
+ assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
+ end
+
+ def test_id_with_dash
+ rs.draw do
+ match '/journey/:id', :to => lambda { |env|
+ resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+ [200, {}, [resp]]
+ }
+ end
+
+ hash = JSON.load get(URI('http://example.org/journey/faithfully-omg'))
+ assert_equal({"id"=>"faithfully-omg"}, hash)
+ end
+
+ def test_dash_with_custom_regexp
+ rs.draw do
+ match '/:artist/:song-omg', :constraints => { :song => /\d+/ }, :to => lambda { |env|
+ resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+ [200, {}, [resp]]
+ }
+ end
+
+ hash = JSON.load get(URI('http://example.org/journey/123-omg'))
+ assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
+ assert_equal 'Not Found', get(URI('http://example.org/journey/faithfully-omg'))
+ end
+
+ def test_pre_dash
+ rs.draw do
+ match '/:artist/omg-:song', :to => lambda { |env|
+ resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+ [200, {}, [resp]]
+ }
+ end
+
+ hash = JSON.load get(URI('http://example.org/journey/omg-faithfully'))
+ assert_equal({"artist"=>"journey", "song"=>"faithfully"}, hash)
+ end
+
+ def test_pre_dash_with_custom_regexp
+ rs.draw do
+ match '/:artist/omg-:song', :constraints => { :song => /\d+/ }, :to => lambda { |env|
+ resp = JSON.dump env[ActionDispatch::Routing::RouteSet::PARAMETERS_KEY]
+ [200, {}, [resp]]
+ }
+ end
+
+ hash = JSON.load get(URI('http://example.org/journey/omg-123'))
+ assert_equal({"artist"=>"journey", "song"=>"123"}, hash)
+ assert_equal 'Not Found', get(URI('http://example.org/journey/omg-faithfully'))
+ end
+
def test_regexp_precidence
@rs.draw do
match '/whois/:domain', :constraints => {

4 comments on commit 3913b51

Contributor

cgriego replied Jan 24, 2012

Not sure if this is a work in progress but just in case, fyi, my app won't boot after this commit.

rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:268:in `to_proc': undefined method `literal?' for #<Journey::Nodes::Slash:0x10be0f400 @memo=nil, @left="/"> (NoMethodError)
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:268:in `find_all'
    from journey-1.0.0/lib/journey/visitors.rb:48:in `call'
    from journey-1.0.0/lib/journey/visitors.rb:48:in `visit'
    from journey-1.0.0/lib/journey/visitors.rb:15:in `binary'
    from journey-1.0.0/lib/journey/visitors.rb:18:in `visit_CAT'
    from journey-1.0.0/lib/journey/visitors.rb:11:in `send'
    from journey-1.0.0/lib/journey/visitors.rb:11:in `visit'
    from journey-1.0.0/lib/journey/visitors.rb:47:in `visit'
    from journey-1.0.0/lib/journey/visitors.rb:6:in `accept'
    from journey-1.0.0/lib/journey/nodes/node.rb:16:in `each'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:381:in `find_all'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:381:in `build_path'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:355:in `add_route'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:1304:in `add_route'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:1282:in `decomposed_match'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:1268:in `match'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:1268:in `each'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:1268:in `match'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/mapper.rb:411:in `mount'
    from rails-1a56a761530e/actionpack/lib/sprockets/bootstrap.rb:28:in `run'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:272:in `instance_exec'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:272:in `eval_block'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:287:in `clear!'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:287:in `each'
    from rails-1a56a761530e/actionpack/lib/action_dispatch/routing/route_set.rb:287:in `clear!'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:35:in `clear!'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:33:in `each'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:33:in `clear!'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:15:in `reload!'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:26:in `updater'
    from rails-1a56a761530e/activesupport/lib/active_support/file_update_checker.rb:78:in `call'
    from rails-1a56a761530e/activesupport/lib/active_support/file_update_checker.rb:78:in `execute'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:27:in `updater'
    from rails-1a56a761530e/railties/lib/rails/application/routes_reloader.rb:9:in `execute_if_updated'
    from rails-1a56a761530e/railties/lib/rails/application/finisher.rb:66
    from rails-1a56a761530e/railties/lib/rails/initializable.rb:30:in `instance_exec'
    from rails-1a56a761530e/railties/lib/rails/initializable.rb:30:in `run'
    from rails-1a56a761530e/railties/lib/rails/initializable.rb:55:in `run_initializers'
    from rails-1a56a761530e/railties/lib/rails/initializable.rb:54:in `each'
    from rails-1a56a761530e/railties/lib/rails/initializable.rb:54:in `run_initializers'
    from rails-1a56a761530e/railties/lib/rails/application.rb:136:in `initialize!'
Owner

rafaelfranca replied Jan 24, 2012

@cgriego you have to use journey 1-0-stable branch or wait for a new release

Contributor

cgriego replied Jan 24, 2012

Thanks @rafaelfranca, maybe I'm wrong but shouldn't the Gemfile generator reflect that if people generate with the --edge option?

Owner

rafaelfranca replied Jan 24, 2012

I don't know. Maybe @tenderlove have an answer

Please sign in to comment.