From 947ebe9a6d516271092853f8164c414f126cff6e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Aug 2015 15:42:46 -0700 Subject: remove Strexp This was a useless object. We can just directly construct a Path::Pattern object without a Strexp object. --- .../lib/action_dispatch/journey/path/pattern.rb | 18 +++--- actionpack/lib/action_dispatch/journey/router.rb | 1 - .../lib/action_dispatch/journey/router/strexp.rb | 26 -------- .../lib/action_dispatch/routing/route_set.rb | 8 +-- actionpack/test/journey/path/pattern_test.rb | 71 +++++++++++----------- actionpack/test/journey/route_test.rb | 3 +- actionpack/test/journey/router_test.rb | 53 +++++++--------- actionpack/test/journey/routes_test.rb | 8 +-- 8 files changed, 73 insertions(+), 115 deletions(-) delete mode 100644 actionpack/lib/action_dispatch/journey/router/strexp.rb diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 5d1680f3a7..24b90e214d 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -1,5 +1,3 @@ -require 'action_dispatch/journey/router/strexp' - module ActionDispatch module Journey # :nodoc: module Path # :nodoc: @@ -7,13 +5,19 @@ module ActionDispatch attr_reader :spec, :requirements, :anchored def self.from_string string - new Journey::Router::Strexp.build(string, {}, ["/.?"]), true + build(string, {}, ["/.?"], true) + end + + def self.build(path, requirements, separators, anchored) + parser = Journey::Parser.new + ast = parser.parse path + new ast, requirements, separators, anchored end - def initialize(strexp, anchored) - @spec = strexp.ast - @requirements = strexp.requirements - @separators = strexp.separators.join + def initialize(ast, requirements, separators, anchored) + @spec = ast + @requirements = requirements + @separators = separators.join @anchored = anchored @names = nil diff --git a/actionpack/lib/action_dispatch/journey/router.rb b/actionpack/lib/action_dispatch/journey/router.rb index b84aad8eb6..31b6731a67 100644 --- a/actionpack/lib/action_dispatch/journey/router.rb +++ b/actionpack/lib/action_dispatch/journey/router.rb @@ -1,5 +1,4 @@ require 'action_dispatch/journey/router/utils' -require 'action_dispatch/journey/router/strexp' require 'action_dispatch/journey/routes' require 'action_dispatch/journey/formatter' diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb deleted file mode 100644 index 53630ea87b..0000000000 --- a/actionpack/lib/action_dispatch/journey/router/strexp.rb +++ /dev/null @@ -1,26 +0,0 @@ -module ActionDispatch - module Journey # :nodoc: - class Router # :nodoc: - class Strexp # :nodoc: - class << self - alias :compile :new - end - - attr_reader :path, :requirements, :separators, :ast - - def self.build(path, requirements, separators) - parser = Journey::Parser.new - ast = parser.parse path - new ast, path, requirements, separators - end - - def initialize(ast, path, requirements, separators) - @ast = ast - @path = path - @requirements = requirements - @separators = separators - end - end - end - end -end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 0455686ec0..e549316f24 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -537,13 +537,7 @@ module ActionDispatch end def build_path(path, ast, requirements, anchor) - strexp = Journey::Router::Strexp.new( - ast, - path, - requirements, - SEPARATORS) - - pattern = Journey::Path::Pattern.new(strexp, anchor) + pattern = Journey::Path::Pattern.new(ast, requirements, SEPARATORS, anchor) builder = Journey::GTG::Builder.new ast diff --git a/actionpack/test/journey/path/pattern_test.rb b/actionpack/test/journey/path/pattern_test.rb index 2efa89b298..7b97379bd5 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -19,12 +19,12 @@ module ActionDispatch '/:foo|*bar' => %r{\A/(?:([^/.?]+)|(.+))\Z}, }.each do |path, expected| define_method(:"test_to_regexp_#{path}") do - strexp = Router::Strexp.build( + path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_equal(expected, path.to_regexp) end end @@ -43,12 +43,12 @@ module ActionDispatch '/:foo|*bar' => %r{\A/(?:([^/.?]+)|(.+))}, }.each do |path, expected| define_method(:"test_to_non_anchored_regexp_#{path}") do - strexp = Router::Strexp.build( + path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"] + ["/", ".", "?"], + false ) - path = Pattern.new strexp, false assert_equal(expected, path.to_regexp) end end @@ -66,27 +66,27 @@ module ActionDispatch '/:controller/*foo/bar' => %w{ controller foo }, }.each do |path, expected| define_method(:"test_names_#{path}") do - strexp = Router::Strexp.build( + path = Pattern.build( path, { :controller => /.+/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_equal(expected, path.names) end end def test_to_regexp_with_extended_group - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name', { :name => / #ROFL (tender|love #MAO )/x }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_match(path, '/page/tender') assert_match(path, '/page/love') assert_no_match(path, '/page/loving') @@ -104,23 +104,23 @@ module ActionDispatch end def test_to_regexp_match_non_optional - strexp = Router::Strexp.build( + path = Pattern.build( '/:name', { :name => /\d+/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_match(path, '/123') assert_no_match(path, '/') end def test_to_regexp_with_group - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name', { :name => /(tender|love)/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_match(path, '/page/tender') assert_match(path, '/page/love') assert_no_match(path, '/page/loving') @@ -128,15 +128,13 @@ module ActionDispatch def test_ast_sets_regular_expressions requirements = { :name => /(tender|love)/, :value => /./ } - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name/:value', requirements, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - assert_equal requirements, strexp.requirements - - path = Pattern.new strexp, true nodes = path.ast.grep(Nodes::Symbol) assert_equal 2, nodes.length nodes.each do |node| @@ -145,24 +143,24 @@ module ActionDispatch end def test_match_data_with_group - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name', { :name => /(tender|love)/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true match = path.match '/page/tender' assert_equal 'tender', match[1] assert_equal 2, match.length end def test_match_data_with_multi_group - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name/:id', { :name => /t(((ender|love)))()/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true match = path.match '/page/tender/10' assert_equal 'tender', match[1] assert_equal '10', match[2] @@ -172,30 +170,29 @@ module ActionDispatch def test_star_with_custom_re z = /\d+/ - strexp = Router::Strexp.build( + path = Pattern.build( '/page/*foo', { :foo => z }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_equal(%r{\A/page/(#{z})\Z}, path.to_regexp) end def test_insensitive_regexp_with_group - strexp = Router::Strexp.build( + path = Pattern.build( '/page/:name/aaron', { :name => /(tender|love)/i }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Pattern.new strexp, true assert_match(path, '/page/TENDER/aaron') assert_match(path, '/page/loVE/aaron') assert_no_match(path, '/page/loVE/AAron') end def test_to_regexp_with_strexp - strexp = Router::Strexp.build('/:controller', { }, ["/", ".", "?"]) - path = Pattern.new strexp, true + path = Pattern.build('/:controller', { }, ["/", ".", "?"], true) x = %r{\A/([^/.?]+)\Z} assert_equal(x.source, path.source) diff --git a/actionpack/test/journey/route_test.rb b/actionpack/test/journey/route_test.rb index 14cb3a259c..cdd59a3316 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -26,8 +26,7 @@ module ActionDispatch end def test_path_requirements_override_defaults - strexp = Router::Strexp.build(':name', { name: /love/ }, ['/']) - path = Path::Pattern.new strexp, true + path = Path::Pattern.build(':name', { name: /love/ }, ['/'], true) defaults = { name: 'tender' } route = Route.new('name', nil, path, nil, [], defaults) assert_equal(/love/, route.requirements[:name]) diff --git a/actionpack/test/journey/router_test.rb b/actionpack/test/journey/router_test.rb index cd8c77f1c1..5c6d9645fb 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -32,8 +32,7 @@ module ActionDispatch def test_dashes router = Router.new(routes) - exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/foo-bar-baz', {}, ['/.?'], true routes.add_route nil, path, {}, [], {:id => nil}, {} @@ -49,8 +48,7 @@ module ActionDispatch router = Router.new(routes) #match the escaped version of /ほげ - exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/%E3%81%BB%E3%81%92', {}, ['/.?'], true routes.add_route nil, path, {}, [], {:id => nil}, {} @@ -68,8 +66,7 @@ module ActionDispatch requirements = { :hello => /world/ } - exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -88,8 +85,7 @@ module ActionDispatch requirements = { :hello => /mom/ } - exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true router.routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -115,8 +111,7 @@ module ActionDispatch def test_request_class_overrides_path_info router = Router.new(routes) - exp = Router::Strexp.build '/bar', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/bar', {}, ['/.?'], true routes.add_route nil, path, {}, [], {}, {} @@ -133,8 +128,8 @@ module ActionDispatch def test_regexp_first_precedence add_routes @router, [ - Router::Strexp.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?']), - Router::Strexp.build("/whois/:id(.:format)", {}, ['/', '.', '?']) + Path::Pattern.build("/whois/:domain", {:domain => /\w+\.[\w\.]+/}, ['/', '.', '?'], true), + Path::Pattern.build("/whois/:id(.:format)", {}, ['/', '.', '?'], true) ] env = rails_env 'PATH_INFO' => '/whois/example.com' @@ -152,8 +147,8 @@ module ActionDispatch def test_required_parts_verified_are_anchored add_routes @router, [ - Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?']) - ], false + Path::Pattern.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) + ] assert_raises(ActionController::UrlGenerationError) do @formatter.generate(nil, { :id => '10' }, { }) @@ -162,8 +157,8 @@ module ActionDispatch def test_required_parts_are_verified_when_building add_routes @router, [ - Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?']) - ], false + Path::Pattern.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) + ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) assert_equal '/foo/10', path @@ -175,8 +170,8 @@ module ActionDispatch def test_only_required_parts_are_verified add_routes @router, [ - Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?']) - ], false + Path::Pattern.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) + ] path, _ = @formatter.generate(nil, { :id => '10' }, { }) assert_equal '/foo/10', path @@ -190,8 +185,7 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" - pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?']) - path = Path::Pattern.new pattern, false + path = Path::Pattern.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) @router.routes.add_route nil, path, {}, [], {}, route_name error = assert_raises(ActionController::UrlGenerationError) do @@ -249,8 +243,8 @@ module ActionDispatch def test_recognize_with_unbound_regexp add_routes @router, [ - Router::Strexp.build("/foo", { }, ['/', '.', '?']) - ], false + Path::Pattern.build("/foo", { }, ['/', '.', '?'], false) + ] env = rails_env 'PATH_INFO' => '/foo/bar' @@ -262,8 +256,8 @@ module ActionDispatch def test_bound_regexp_keeps_path_info add_routes @router, [ - Router::Strexp.build("/foo", { }, ['/', '.', '?']) - ], true + Path::Pattern.build("/foo", { }, ['/', '.', '?'], true) + ] env = rails_env 'PATH_INFO' => '/foo' @@ -329,8 +323,7 @@ module ActionDispatch def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - str = Router::Strexp.build("/", Hash[params], ['/', '.', '?']) - path = Path::Pattern.new str, true + path = Path::Pattern.build("/", Hash[params], ['/', '.', '?'], true) @router.routes.add_route @app, path, {}, [], {}, {} @@ -498,12 +491,12 @@ module ActionDispatch end def test_namespaced_controller - strexp = Router::Strexp.build( + path = Path::Pattern.build( "/:controller(/:action(/:id))", { :controller => /.+?/ }, - ["/", ".", "?"] + ["/", ".", "?"], + true ) - path = Path::Pattern.new strexp, true app = Object.new route = @router.routes.add_route(app, path, {}, [], {}, {}) @@ -615,7 +608,7 @@ module ActionDispatch if String === path path = Path::Pattern.from_string path else - path = Path::Pattern.new path, anchor + path end router.routes.add_route @app, path, {}, [], {}, {} end diff --git a/actionpack/test/journey/routes_test.rb b/actionpack/test/journey/routes_test.rb index c7555e68ee..6853cefc01 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -9,8 +9,7 @@ module ActionDispatch def test_clear routes = Routes.new - exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp, true + path = Path::Pattern.build '/foo(/:id)', {}, ['/.?'], true requirements = { :hello => /world/ } routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -49,10 +48,9 @@ module ActionDispatch assert_equal [anchored_route], @routes.anchored_routes assert_equal [], @routes.custom_routes - strexp = Router::Strexp.build( - "/hello/:who", { who: /\d/ }, ['/', '.', '?'] + path = Path::Pattern.build( + "/hello/:who", { who: /\d/ }, ['/', '.', '?'], false ) - path = Path::Pattern.new strexp, true custom_route = @routes.add_route nil, path, {}, [], {}, {} assert_equal [custom_route], @routes.custom_routes -- cgit v1.2.3