From 15ffbedf3bb5907c3774fcc38466c799e92f9a04 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 29 May 2014 15:23:30 -0700 Subject: [PATCH] add an alternate constructor to Strexp that takes a string --- .../action_dispatch/journey/router/strexp.rb | 16 ++++++---- .../lib/action_dispatch/routing/route_set.rb | 2 +- actionpack/test/journey/path/pattern_test.rb | 24 +++++++------- actionpack/test/journey/router/strexp_test.rb | 4 +-- actionpack/test/journey/router_test.rb | 32 +++++++++---------- actionpack/test/journey/routes_test.rb | 2 +- 6 files changed, 41 insertions(+), 39 deletions(-) diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb index 161b41d6b2..29196d20bb 100644 --- a/actionpack/lib/action_dispatch/journey/router/strexp.rb +++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb @@ -6,20 +6,22 @@ module ActionDispatch alias :compile :new end - attr_reader :path, :requirements, :separators, :anchor + attr_reader :path, :requirements, :separators, :anchor, :ast - def initialize(path, requirements, separators, anchor = true) + def self.build(path, requirements, separators, anchor = true) + parser = Journey::Parser.new + ast = parser.parse path + new ast, path, requirements, separators, anchor + end + + def initialize(ast, path, requirements, separators, anchor = true) + @ast = ast @path = path @requirements = requirements @separators = separators @anchor = anchor end - def ast - parser = Journey::Parser.new - parser.parse path - end - def names @path.scan(/:\w+/).map { |s| s.tr(':', '') } end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 3ca5abf0fd..66d1805702 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -427,7 +427,7 @@ module ActionDispatch end def build_path(path, requirements, separators, anchor) - strexp = Journey::Router::Strexp.new( + strexp = Journey::Router::Strexp.build( path, requirements, SEPARATORS, diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index ce02104181..dfb20d2977 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -18,7 +18,7 @@ module ActionDispatch '/:controller/*foo/bar' => %r{\A/(#{x})/(.+)/bar\Z}, }.each do |path, expected| define_method(:"test_to_regexp_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"] @@ -41,7 +41,7 @@ module ActionDispatch '/:controller/*foo/bar' => %r{\A/(#{x})/(.+)/bar}, }.each do |path, expected| define_method(:"test_to_non_anchored_regexp_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"], @@ -65,7 +65,7 @@ module ActionDispatch '/:controller/*foo/bar' => %w{ controller foo }, }.each do |path, expected| define_method(:"test_names_#{path}") do - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( path, { :controller => /.+/ }, ["/", ".", "?"] @@ -80,7 +80,7 @@ module ActionDispatch end def test_to_regexp_with_extended_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => / #ROFL @@ -107,7 +107,7 @@ module ActionDispatch end def test_to_regexp_match_non_optional - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/:name', { :name => /\d+/ }, ["/", ".", "?"] @@ -118,7 +118,7 @@ module ActionDispatch end def test_to_regexp_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => /(tender|love)/ }, ["/", ".", "?"] @@ -131,7 +131,7 @@ module ActionDispatch def test_ast_sets_regular_expressions requirements = { :name => /(tender|love)/, :value => /./ } - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/:value', requirements, ["/", ".", "?"] @@ -148,7 +148,7 @@ module ActionDispatch end def test_match_data_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name', { :name => /(tender|love)/ }, ["/", ".", "?"] @@ -160,7 +160,7 @@ module ActionDispatch end def test_match_data_with_multi_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/:id', { :name => /t(((ender|love)))()/ }, ["/", ".", "?"] @@ -175,7 +175,7 @@ module ActionDispatch def test_star_with_custom_re z = /\d+/ - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/*foo', { :foo => z }, ["/", ".", "?"] @@ -185,7 +185,7 @@ module ActionDispatch end def test_insensitive_regexp_with_group - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( '/page/:name/aaron', { :name => /(tender|love)/i }, ["/", ".", "?"] @@ -197,7 +197,7 @@ module ActionDispatch end def test_to_regexp_with_strexp - strexp = Router::Strexp.new('/:controller', { }, ["/", ".", "?"]) + strexp = Router::Strexp.build('/:controller', { }, ["/", ".", "?"]) path = Pattern.new strexp x = %r{\A/([^/.?]+)\Z} diff --git a/actionpack/test/journey/router/strexp_test.rb b/actionpack/test/journey/router/strexp_test.rb index 7ccdfb7b4d..9cb333757c 100644 --- a/actionpack/test/journey/router/strexp_test.rb +++ b/actionpack/test/journey/router/strexp_test.rb @@ -5,7 +5,7 @@ module ActionDispatch class Router class TestStrexp < ActiveSupport::TestCase def test_many_names - exp = Strexp.new( + exp = Strexp.build( "/:controller(/:action(/:id(.:format)))", {:controller=>/.+?/}, ["/", ".", "?"], @@ -22,7 +22,7 @@ module ActionDispatch ":format0" => %w{ format0 }, ":format1,:format2" => %w{ format1 format2 }, }.each do |string, expected| - exp = Strexp.new(string, {}, ["/", ".", "?"]) + exp = Strexp.build(string, {}, ["/", ".", "?"]) assert_equal expected, exp.names end end diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index f298808be8..48d1e78b96 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -32,7 +32,7 @@ module ActionDispatch def test_dashes router = Router.new(routes) - exp = Router::Strexp.new '/foo-bar-baz', {}, ['/.?'] + exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {:id => nil}, {} @@ -49,7 +49,7 @@ module ActionDispatch router = Router.new(routes) #match the escaped version of /ほげ - exp = Router::Strexp.new '/%E3%81%BB%E3%81%92', {}, ['/.?'] + exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {:id => nil}, {} @@ -68,7 +68,7 @@ module ActionDispatch requirements = { :hello => /world/ } - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, requirements, {:id => nil}, {} @@ -88,7 +88,7 @@ module ActionDispatch requirements = { :hello => /mom/ } - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp router.routes.add_route nil, path, requirements, {:id => nil}, {} @@ -115,7 +115,7 @@ module ActionDispatch def test_request_class_overrides_path_info router = Router.new(routes) - exp = Router::Strexp.new '/bar', {}, ['/.?'] + exp = Router::Strexp.build '/bar', {}, ['/.?'] path = Path::Pattern.new exp routes.add_route nil, path, {}, {}, {} @@ -133,8 +133,8 @@ module ActionDispatch def test_regexp_first_precedence add_routes @router, [ - Router::Strexp.new("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']), - Router::Strexp.new("/whois/:id(.:format)", {}, ['/', '.', '?']) + Router::Strexp.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']), + Router::Strexp.build("/whois/:id(.:format)", {}, ['/', '.', '?']) ] env = rails_env 'PATH_INFO' => '/whois/example.com' @@ -152,7 +152,7 @@ module ActionDispatch def test_required_parts_verified_are_anchored add_routes @router, [ - Router::Strexp.new("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) + Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) ] assert_raises(ActionController::UrlGenerationError) do @@ -162,7 +162,7 @@ module ActionDispatch def test_required_parts_are_verified_when_building add_routes @router, [ - Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) + Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) @@ -175,7 +175,7 @@ module ActionDispatch def test_only_required_parts_are_verified add_routes @router, [ - Router::Strexp.new("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) + Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) @@ -190,7 +190,7 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" - pattern = Router::Strexp.new("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) + pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) path = Path::Pattern.new pattern @router.routes.add_route nil, path, {}, {}, route_name @@ -213,7 +213,7 @@ module ActionDispatch route_set = Routing::RouteSet.new mapper = Routing::Mapper.new route_set - strexp = Router::Strexp.new("/", {}, ['/', '.', '?'], false) + strexp = Router::Strexp.build("/", {}, ['/', '.', '?'], false) path = Path::Pattern.new strexp app = lambda { |env| [200, {}, ['success!']] } mapper.get '/weblog', :to => app @@ -241,7 +241,7 @@ module ActionDispatch def test_recognize_with_unbound_regexp add_routes @router, [ - Router::Strexp.new("/foo", { }, ['/', '.', '?'], false) + Router::Strexp.build("/foo", { }, ['/', '.', '?'], false) ] env = rails_env 'PATH_INFO' => '/foo/bar' @@ -254,7 +254,7 @@ module ActionDispatch def test_bound_regexp_keeps_path_info add_routes @router, [ - Router::Strexp.new("/foo", { }, ['/', '.', '?'], true) + Router::Strexp.build("/foo", { }, ['/', '.', '?'], true) ] env = rails_env 'PATH_INFO' => '/foo' @@ -321,7 +321,7 @@ module ActionDispatch def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - str = Router::Strexp.new("/", Hash[params], ['/', '.', '?'], true) + str = Router::Strexp.build("/", Hash[params], ['/', '.', '?'], true) path = Path::Pattern.new str @router.routes.add_route @app, path, {}, {}, {} @@ -463,7 +463,7 @@ module ActionDispatch end def test_namespaced_controller - strexp = Router::Strexp.new( + strexp = Router::Strexp.build( "/:controller(/:action(/:id))", { :controller => /.+?/ }, ["/", ".", "?"] diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb index 25e0321d31..2103cbb9b8 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -5,7 +5,7 @@ module ActionDispatch class TestRoutes < ActiveSupport::TestCase def test_clear routes = Routes.new - exp = Router::Strexp.new '/foo(/:id)', {}, ['/.?'] + exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] path = Path::Pattern.new exp requirements = { :hello => /world/ }