From 4868692687f2904d2a02c1d6cd09882b6916cc5f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Aug 2015 15:16:25 -0700 Subject: pass anchor directly to `Pattern` the caller already has it, there is no reason to pack it in to an object and just throw that object away. --- .../lib/action_dispatch/journey/path/pattern.rb | 6 +-- .../lib/action_dispatch/journey/router/strexp.rb | 9 ++--- .../lib/action_dispatch/routing/route_set.rb | 5 +-- actionpack/test/journey/path/pattern_test.rb | 27 +++++++------ actionpack/test/journey/route_test.rb | 2 +- actionpack/test/journey/router_test.rb | 44 +++++++++++----------- actionpack/test/journey/routes_test.rb | 4 +- 7 files changed, 47 insertions(+), 50 deletions(-) diff --git a/actionpack/lib/action_dispatch/journey/path/pattern.rb b/actionpack/lib/action_dispatch/journey/path/pattern.rb index 64b48ca45f..5d1680f3a7 100644 --- a/actionpack/lib/action_dispatch/journey/path/pattern.rb +++ b/actionpack/lib/action_dispatch/journey/path/pattern.rb @@ -7,14 +7,14 @@ module ActionDispatch attr_reader :spec, :requirements, :anchored def self.from_string string - new Journey::Router::Strexp.build(string, {}, ["/.?"], true) + new Journey::Router::Strexp.build(string, {}, ["/.?"]), true end - def initialize(strexp) + def initialize(strexp, anchored) @spec = strexp.ast @requirements = strexp.requirements @separators = strexp.separators.join - @anchored = strexp.anchor + @anchored = anchored @names = nil @optional_names = nil diff --git a/actionpack/lib/action_dispatch/journey/router/strexp.rb b/actionpack/lib/action_dispatch/journey/router/strexp.rb index 4b7738f335..53630ea87b 100644 --- a/actionpack/lib/action_dispatch/journey/router/strexp.rb +++ b/actionpack/lib/action_dispatch/journey/router/strexp.rb @@ -6,20 +6,19 @@ module ActionDispatch alias :compile :new end - attr_reader :path, :requirements, :separators, :anchor, :ast + attr_reader :path, :requirements, :separators, :ast - def self.build(path, requirements, separators, anchor = true) + def self.build(path, requirements, separators) parser = Journey::Parser.new ast = parser.parse path - new ast, path, requirements, separators, anchor + new ast, path, requirements, separators end - def initialize(ast, path, requirements, separators, anchor = true) + def initialize(ast, path, requirements, separators) @ast = ast @path = path @requirements = requirements @separators = separators - @anchor = anchor end end end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index b72031633b..0455686ec0 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -541,10 +541,9 @@ module ActionDispatch ast, path, requirements, - SEPARATORS, - anchor) + SEPARATORS) - pattern = Journey::Path::Pattern.new(strexp) + pattern = Journey::Path::Pattern.new(strexp, 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 6939b426f6..2efa89b298 100644 --- a/actionpack/test/journey/path/pattern_test.rb +++ b/actionpack/test/journey/path/pattern_test.rb @@ -24,7 +24,7 @@ module ActionDispatch { :controller => /.+/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_equal(expected, path.to_regexp) end end @@ -46,10 +46,9 @@ module ActionDispatch strexp = Router::Strexp.build( path, { :controller => /.+/ }, - ["/", ".", "?"], - false + ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, false assert_equal(expected, path.to_regexp) end end @@ -72,7 +71,7 @@ module ActionDispatch { :controller => /.+/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_equal(expected, path.names) end end @@ -87,7 +86,7 @@ module ActionDispatch )/x }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_match(path, '/page/tender') assert_match(path, '/page/love') assert_no_match(path, '/page/loving') @@ -110,7 +109,7 @@ module ActionDispatch { :name => /\d+/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_match(path, '/123') assert_no_match(path, '/') end @@ -121,7 +120,7 @@ module ActionDispatch { :name => /(tender|love)/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_match(path, '/page/tender') assert_match(path, '/page/love') assert_no_match(path, '/page/loving') @@ -137,7 +136,7 @@ module ActionDispatch assert_equal requirements, strexp.requirements - path = Pattern.new strexp + path = Pattern.new strexp, true nodes = path.ast.grep(Nodes::Symbol) assert_equal 2, nodes.length nodes.each do |node| @@ -151,7 +150,7 @@ module ActionDispatch { :name => /(tender|love)/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true match = path.match '/page/tender' assert_equal 'tender', match[1] assert_equal 2, match.length @@ -163,7 +162,7 @@ module ActionDispatch { :name => /t(((ender|love)))()/ }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true match = path.match '/page/tender/10' assert_equal 'tender', match[1] assert_equal '10', match[2] @@ -178,7 +177,7 @@ module ActionDispatch { :foo => z }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_equal(%r{\A/page/(#{z})\Z}, path.to_regexp) end @@ -188,7 +187,7 @@ module ActionDispatch { :name => /(tender|love)/i }, ["/", ".", "?"] ) - path = Pattern.new strexp + path = Pattern.new strexp, true assert_match(path, '/page/TENDER/aaron') assert_match(path, '/page/loVE/aaron') assert_no_match(path, '/page/loVE/AAron') @@ -196,7 +195,7 @@ module ActionDispatch def test_to_regexp_with_strexp strexp = Router::Strexp.build('/:controller', { }, ["/", ".", "?"]) - path = Pattern.new strexp + path = Pattern.new strexp, 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 eff96a0abc..14cb3a259c 100644 --- a/actionpack/test/journey/route_test.rb +++ b/actionpack/test/journey/route_test.rb @@ -27,7 +27,7 @@ module ActionDispatch def test_path_requirements_override_defaults strexp = Router::Strexp.build(':name', { name: /love/ }, ['/']) - path = Path::Pattern.new strexp + path = Path::Pattern.new strexp, 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 802fb93c69..cd8c77f1c1 100644 --- a/actionpack/test/journey/router_test.rb +++ b/actionpack/test/journey/router_test.rb @@ -33,7 +33,7 @@ module ActionDispatch router = Router.new(routes) exp = Router::Strexp.build '/foo-bar-baz', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true routes.add_route nil, path, {}, [], {:id => nil}, {} @@ -50,7 +50,7 @@ module ActionDispatch #match the escaped version of /ほげ exp = Router::Strexp.build '/%E3%81%BB%E3%81%92', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true routes.add_route nil, path, {}, [], {:id => nil}, {} @@ -69,7 +69,7 @@ module ActionDispatch requirements = { :hello => /world/ } exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -89,7 +89,7 @@ module ActionDispatch requirements = { :hello => /mom/ } exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true router.routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -116,7 +116,7 @@ module ActionDispatch router = Router.new(routes) exp = Router::Strexp.build '/bar', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true routes.add_route nil, path, {}, [], {}, {} @@ -152,8 +152,8 @@ module ActionDispatch def test_required_parts_verified_are_anchored add_routes @router, [ - Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?'], false) - ] + Router::Strexp.build("/foo/:id", { :id => /\d/ }, ['/', '.', '?']) + ], false assert_raises(ActionController::UrlGenerationError) do @formatter.generate(nil, { :id => '10' }, { }) @@ -162,8 +162,8 @@ module ActionDispatch def test_required_parts_are_verified_when_building add_routes @router, [ - Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) - ] + Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?']) + ], false path, _ = @formatter.generate(nil, { :id => '10' }, { }) assert_equal '/foo/10', path @@ -175,8 +175,8 @@ module ActionDispatch def test_only_required_parts_are_verified add_routes @router, [ - Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?'], false) - ] + Router::Strexp.build("/foo(/:id)", {:id => /\d/}, ['/', '.', '?']) + ], false path, _ = @formatter.generate(nil, { :id => '10' }, { }) assert_equal '/foo/10', path @@ -190,8 +190,8 @@ module ActionDispatch def test_knows_what_parts_are_missing_from_named_route route_name = "gorby_thunderhorse" - pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?'], false) - path = Path::Pattern.new pattern + pattern = Router::Strexp.build("/foo/:id", { :id => /\d+/ }, ['/', '.', '?']) + path = Path::Pattern.new pattern, false @router.routes.add_route nil, path, {}, [], {}, route_name error = assert_raises(ActionController::UrlGenerationError) do @@ -249,8 +249,8 @@ module ActionDispatch def test_recognize_with_unbound_regexp add_routes @router, [ - Router::Strexp.build("/foo", { }, ['/', '.', '?'], false) - ] + Router::Strexp.build("/foo", { }, ['/', '.', '?']) + ], false env = rails_env 'PATH_INFO' => '/foo/bar' @@ -262,8 +262,8 @@ module ActionDispatch def test_bound_regexp_keeps_path_info add_routes @router, [ - Router::Strexp.build("/foo", { }, ['/', '.', '?'], true) - ] + Router::Strexp.build("/foo", { }, ['/', '.', '?']) + ], true env = rails_env 'PATH_INFO' => '/foo' @@ -329,8 +329,8 @@ module ActionDispatch def test_generate_slash params = [ [:controller, "tasks"], [:action, "show"] ] - str = Router::Strexp.build("/", Hash[params], ['/', '.', '?'], true) - path = Path::Pattern.new str + str = Router::Strexp.build("/", Hash[params], ['/', '.', '?']) + path = Path::Pattern.new str, true @router.routes.add_route @app, path, {}, [], {}, {} @@ -503,7 +503,7 @@ module ActionDispatch { :controller => /.+?/ }, ["/", ".", "?"] ) - path = Path::Pattern.new strexp + path = Path::Pattern.new strexp, true app = Object.new route = @router.routes.add_route(app, path, {}, [], {}, {}) @@ -610,12 +610,12 @@ module ActionDispatch private - def add_routes router, paths + def add_routes router, paths, anchor = true paths.each do |path| if String === path path = Path::Pattern.from_string path else - path = Path::Pattern.new path + path = Path::Pattern.new path, anchor 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 b9dac8751c..c7555e68ee 100644 --- a/actionpack/test/journey/routes_test.rb +++ b/actionpack/test/journey/routes_test.rb @@ -10,7 +10,7 @@ module ActionDispatch def test_clear routes = Routes.new exp = Router::Strexp.build '/foo(/:id)', {}, ['/.?'] - path = Path::Pattern.new exp + path = Path::Pattern.new exp, true requirements = { :hello => /world/ } routes.add_route nil, path, requirements, [], {:id => nil}, {} @@ -52,7 +52,7 @@ module ActionDispatch strexp = Router::Strexp.build( "/hello/:who", { who: /\d/ }, ['/', '.', '?'] ) - path = Path::Pattern.new strexp + path = Path::Pattern.new strexp, true custom_route = @routes.add_route nil, path, {}, [], {}, {} assert_equal [custom_route], @routes.custom_routes -- cgit v1.2.3