From e9791bec823e42372eca095b946c93c1712a0613 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Thu, 15 Jul 2010 01:18:02 +0200 Subject: Routes refactoring: * added more tests for prefix generation * fixed bug with generating host for both prefix and url * refactored url_for method * organized tests for prefix generation --- actionpack/lib/action_dispatch/routing/mapper.rb | 3 +- .../lib/action_dispatch/routing/route_set.rb | 2 +- actionpack/lib/action_dispatch/routing/url_for.rb | 10 +- actionpack/test/dispatch/prefix_generation_test.rb | 117 +++++++++++-------- .../test/railties/mounted_engine_routes_test.rb | 129 -------------------- railties/test/railties/mounted_engine_test.rb | 130 +++++++++++++++++++++ 6 files changed, 207 insertions(+), 184 deletions(-) delete mode 100644 railties/test/railties/mounted_engine_routes_test.rb create mode 100644 railties/test/railties/mounted_engine_test.rb diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0e138524cd..88152ac290 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -288,8 +288,7 @@ module ActionDispatch _router = @set app.routes.class_eval do define_method :_generate_prefix do |options| - keys = _route.segment_keys + ActionDispatch::Routing::RouteSet::RESERVED_OPTIONS - prefix_options = options.slice(*keys) + prefix_options = options.slice(*_route.segment_keys) # we must actually delete prefix segment keys to avoid passing them to next url_for _route.segment_keys.each { |k| options.delete(k) } _router.url_helpers.send("#{name}_path", prefix_options) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index c94b00257b..b3945a4963 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -452,7 +452,7 @@ module ActionDispatch Generator.new(options, recall, self, extras).generate end - RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :script_name, :routes] + RESERVED_OPTIONS = [:anchor, :params, :only_path, :host, :protocol, :port, :trailing_slash, :script_name] def _generate_prefix(options = {}) nil diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index edcb7f9cbe..30b456f3df 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -123,19 +123,17 @@ module ActionDispatch # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :anchor => 'ok', :only_path => true # => '/tasks/testing#ok' # url_for :controller => 'tasks', :action => 'testing', :trailing_slash=>true # => 'http://somehost.org/tasks/testing/' # url_for :controller => 'tasks', :action => 'testing', :host=>'somehost.org', :number => '33' # => 'http://somehost.org/tasks/testing?number=33' - def url_for(*args) - if args.first.respond_to?(:routes) - app = args.shift - _with_routes(app.routes) do + def url_for(options = nil, *args) + if options.respond_to?(:routes) + _with_routes(options.routes) do if args.first.is_a? Symbol named_route = args.shift - url_for _routes.url_helpers.__send__("hash_for_#{named_route}", *args) + url_for _routes.url_helpers.send("hash_for_#{named_route}", *args) else url_for(*args) end end else - options = args.first case options when String options diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index c681642eda..2eb592c8d0 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -10,7 +10,8 @@ module TestGenerationPrefix @routes ||= begin routes = ActionDispatch::Routing::RouteSet.new routes.draw do - match "/posts/:id", :to => "inside_engine_generating#index", :as => :post + match "/posts/:id", :to => "inside_engine_generating#show", :as => :post + match "/posts", :to => "inside_engine_generating#index", :as => :posts match "/url_to_application", :to => "inside_engine_generating#url_to_application" end @@ -47,13 +48,19 @@ module TestGenerationPrefix end class ::InsideEngineGeneratingController < ActionController::Base + include BlogEngine.routes.url_helpers + def index - render :text => url_for(BlogEngine, :post_path, :id => params[:id]) + render :text => posts_path + end + + def show + render :text => post_path(:id => params[:id]) end def url_to_application - path = url_for( :routes => RailsApplication.routes, - :controller => "outside_engine_generating", + path = url_for( RailsApplication, + :controller => "outside_engine_generating", :action => "index", :only_path => true) render :text => path @@ -61,100 +68,118 @@ module TestGenerationPrefix end class ::OutsideEngineGeneratingController < ActionController::Base - include BlogEngine.routes.url_helpers def index render :text => url_for(BlogEngine, :post_path, :id => 1) end end - class Foo + class EngineObject include ActionDispatch::Routing::UrlFor include BlogEngine.routes.url_helpers - - def foo - post_path(42) - end end - class Bar + class AppObject include ActionDispatch::Routing::UrlFor include RailsApplication.routes.url_helpers - - def bar - root_path - end end - RailsApplication.routes # force draw - include BlogEngine.routes.url_helpers + # force draw + RailsApplication.routes def app RailsApplication end + def engine_object + @engine_object ||= EngineObject.new + end + + def app_object + @app_object ||= AppObject.new + end + def setup RailsApplication.routes.default_url_options = {} end - test "generating URL with prefix" do - assert_equal "/awesome/blog/posts/1", post_path(:id => 1) + # Inside Engine + test "[ENGINE] generating engine's url use SCRIPT_NAME from request" do + get "/pure-awesomeness/blog/posts/1" + assert_equal "/pure-awesomeness/blog/posts/1", last_response.body end - test "use SCRIPT_NAME inside the engine" do - get "/pure-awesomness/blog/posts/1" - assert_equal "/pure-awesomness/blog/posts/1", last_response.body + test "[ENGINE] generating application's url never uses SCRIPT_NAME from request" do + get "/pure-awesomeness/blog/url_to_application" + assert_equal "/generate", last_response.body end - test "prepend prefix outside the engine" do + test "[ENGINE] generating application's url includes default_url_options[:script_name]" do + RailsApplication.routes.default_url_options = {:script_name => "/something"} + get "/pure-awesomeness/blog/url_to_application" + assert_equal "/something/generate", last_response.body + end + + test "[ENGINE] generating application's url should give higher priority to default_url_options[:script_name]" do + RailsApplication.routes.default_url_options = {:script_name => "/something"} + get "/pure-awesomeness/blog/url_to_application", {}, 'SCRIPT_NAME' => '/foo' + assert_equal "/something/generate", last_response.body + end + + # Inside Application + test "[APP] generating engine's route includes prefix" do get "/generate" assert_equal "/awesome/blog/posts/1", last_response.body end - test "prepend prefix outside the engine and use default_url_options[:script_name]" do + test "[APP] generating engine's route includes default_url_options[:script_name]" do RailsApplication.routes.default_url_options = {:script_name => "/something"} get "/generate" assert_equal "/something/awesome/blog/posts/1", last_response.body end - test "give higher priority to default_url_options[:script_name]" do + test "[APP] generating engine's route should give higher priority to default_url_options[:script_name]" do RailsApplication.routes.default_url_options = {:script_name => "/something"} get "/generate", {}, 'SCRIPT_NAME' => "/foo" assert_equal "/something/awesome/blog/posts/1", last_response.body end - test "generating urls with options for prefix and named_route" do - assert_equal "/pure-awesomness/blog/posts/3", post_path(:id => 3, :omg => "pure-awesomness") + # Inside any Object + test "[OBJECT] generating engine's route includes prefix" do + assert_equal "/awesome/blog/posts/1", engine_object.post_path(:id => 1) end - test "generating urls with url_for should prepend the prefix" do - path = BlogEngine.routes.url_for(:omg => 'omg', :controller => "inside_engine_generating", :action => "index", :id => 1, :only_path => true) - assert_equal "/omg/blog/posts/1", path + test "[OBJECT] generating engine's route includes dynamic prefix" do + assert_equal "/pure-awesomeness/blog/posts/3", engine_object.post_path(:id => 3, :omg => "pure-awesomeness") end - test "generating urls from a regular class" do - assert_equal "/awesome/blog/posts/42", Foo.new.foo + test "[OBJECT] generating engine's route includes default_url_options[:script_name]" do + RailsApplication.routes.default_url_options = {:script_name => "/something"} + assert_equal "/something/pure-awesomeness/blog/posts/3", engine_object.post_path(:id => 3, :omg => "pure-awesomeness") end - test "generating application's url from engine" do - get "/pure-awesomness/blog/url_to_application" - assert_equal "/generate", last_response.body + test "[OBJECT] generating application's route" do + assert_equal "/", app_object.root_path end - test "generating application's url from engine with default_url_options[:script_name]" do + test "[OBJECT] generating application's route includes default_url_options[:script_name]" do RailsApplication.routes.default_url_options = {:script_name => "/something"} - get "/pure-awesomness/blog/url_to_application" - assert_equal "/something/generate", last_response.body + assert_equal "/something/", app_object.root_path end - test "generating application's url from engine should give higher priority to default_url_options[:script_name]" do - RailsApplication.routes.default_url_options = {:script_name => "/something"} - get "/pure-awesomness/blog/url_to_application", {}, 'SCRIPT_NAME' => '/foo' - assert_equal "/something/generate", last_response.body - end + test "[OBJECT] generating engine's route with url_for" do + path = engine_object.url_for(BlogEngine, + :controller => "inside_engine_generating", + :action => "show", + :only_path => true, + :omg => "omg", + :id => 1) + assert_equal "/omg/blog/posts/1", path - test "using default_url_options[:script_name] in regular classes" do - RailsApplication.routes.default_url_options = {:script_name => "/something"} - assert_equal "/something/", Bar.new.bar + path = engine_object.url_for(BlogEngine, :posts_path) + assert_equal "/awesome/blog/posts", path + + path = engine_object.url_for(BlogEngine, :posts_url, :host => "example.com") + assert_equal "http://example.com/awesome/blog/posts", path end end end diff --git a/railties/test/railties/mounted_engine_routes_test.rb b/railties/test/railties/mounted_engine_routes_test.rb deleted file mode 100644 index 319b99383c..0000000000 --- a/railties/test/railties/mounted_engine_routes_test.rb +++ /dev/null @@ -1,129 +0,0 @@ -require 'isolation/abstract_unit' - -module ApplicationTests - class ApplicationRoutingTest < Test::Unit::TestCase - require 'rack/test' - include Rack::Test::Methods - include ActiveSupport::Testing::Isolation - - def setup - build_app - - add_to_config("config.action_dispatch.show_exceptions = false") - - @plugin = engine "blog" - - app_file 'config/routes.rb', <<-RUBY - AppTemplate::Application.routes.draw do |map| - match "/engine_route" => "application_generating#engine_route" - match "/url_for_engine_route" => "application_generating#url_for_engine_route" - scope "/:user", :user => "anonymous" do - mount Blog::Engine => "/blog" - end - root :to => 'main#index' - end - RUBY - - @plugin.write "lib/blog.rb", <<-RUBY - module Blog - class Engine < ::Rails::Engine - end - end - RUBY - - app_file "config/initializers/bla.rb", <<-RUBY - Blog::Engine.eager_load! - RUBY - - @plugin.write "config/routes.rb", <<-RUBY - Blog::Engine.routes.draw do - resources :posts do - get :generate_application_route - end - end - RUBY - - @plugin.write "app/controllers/posts_controller.rb", <<-RUBY - class PostsController < ActionController::Base - def index - render :text => url_for(Blog::Engine, :post_path, 1) - end - - def generate_application_route - path = url_for( :routes => Rails.application.routes, - :controller => "main", - :action => "index", - :only_path => true) - render :text => path - end - end - RUBY - - app_file "app/controllers/application_generating_controller.rb", <<-RUBY - class ApplicationGeneratingController < ActionController::Base - def engine_route - render :text => url_for(Blog::Engine, :posts_path) - end - - def url_for_engine_route - render :text => url_for(:controller => "posts", :action => "index", :user => "john", :only_path => true, :routes => Blog::Engine.routes) - end - end - RUBY - - boot_rails - end - - def app - @app ||= begin - require "#{app_path}/config/environment" - Rails.application - end - end - - def reset_script_name! - Rails.application.routes.default_url_options = {} - end - - def script_name(script_name) - Rails.application.routes.default_url_options = {:script_name => script_name} - end - - test "routes generation in engine and application" do - # test generating engine's route from engine - get "/john/blog/posts" - assert_equal "/john/blog/posts/1", last_response.body - - # test generating engine's route from engine with default_url_options - script_name "/foo" - get "/john/blog/posts", {}, 'SCRIPT_NAME' => "/foo" - assert_equal "/foo/john/blog/posts/1", last_response.body - reset_script_name! - - # test generating engine's route from application - get "/engine_route" - assert_equal "/anonymous/blog/posts", last_response.body - get "/url_for_engine_route" - assert_equal "/john/blog/posts", last_response.body - - # test generating engine's route from application with default_url_options - script_name "/foo" - get "/engine_route", {}, 'SCRIPT_NAME' => "/foo" - assert_equal "/foo/anonymous/blog/posts", last_response.body - script_name "/foo" - get "/url_for_engine_route", {}, 'SCRIPT_NAME' => "/foo" - assert_equal "/foo/john/blog/posts", last_response.body - reset_script_name! - - # test generating application's route from engine - get "/someone/blog/generate_application_route" - assert_equal "/", last_response.body - - # test generating application's route from engine with default_url_options - script_name "/foo" - get "/someone/blog/generate_application_route", {}, 'SCRIPT_NAME' => '/foo' - assert_equal "/foo/", last_response.body - end - end -end - diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb new file mode 100644 index 0000000000..87bcf9b1f3 --- /dev/null +++ b/railties/test/railties/mounted_engine_test.rb @@ -0,0 +1,130 @@ +require 'isolation/abstract_unit' + +module ApplicationTests + class ApplicationRoutingTest < Test::Unit::TestCase + require 'rack/test' + include Rack::Test::Methods + include ActiveSupport::Testing::Isolation + + def setup + build_app + + add_to_config("config.action_dispatch.show_exceptions = false") + + @plugin = engine "blog" + + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do |map| + match "/engine_route" => "application_generating#engine_route" + match "/url_for_engine_route" => "application_generating#url_for_engine_route" + scope "/:user", :user => "anonymous" do + mount Blog::Engine => "/blog" + end + root :to => 'main#index' + end + RUBY + + @plugin.write "lib/blog.rb", <<-RUBY + module Blog + class Engine < ::Rails::Engine + end + end + RUBY + + app_file "config/initializers/bla.rb", <<-RUBY + Blog::Engine.eager_load! + RUBY + + @plugin.write "config/routes.rb", <<-RUBY + Blog::Engine.routes.draw do + resources :posts do + get :generate_application_route + end + end + RUBY + + @plugin.write "app/controllers/posts_controller.rb", <<-RUBY + class PostsController < ActionController::Base + def index + render :text => url_for(Blog::Engine, :post_path, 1) + end + + def generate_application_route + path = url_for(Rails.application, + :controller => "main", + :action => "index", + :only_path => true) + render :text => path + end + end + RUBY + + app_file "app/controllers/application_generating_controller.rb", <<-RUBY + class ApplicationGeneratingController < ActionController::Base + def engine_route + render :text => url_for(Blog::Engine, :posts_path) + end + + def url_for_engine_route + render :text => url_for(Blog::Engine, :controller => "posts", :action => "index", :user => "john", :only_path => true) + end + end + RUBY + + boot_rails + end + + def app + @app ||= begin + require "#{app_path}/config/environment" + Rails.application + end + end + + def reset_script_name! + Rails.application.routes.default_url_options = {} + end + + def script_name(script_name) + Rails.application.routes.default_url_options = {:script_name => script_name} + end + + test "routes generation in engine and application" do + # test generating engine's route from engine + get "/john/blog/posts" + assert_equal "/john/blog/posts/1", last_response.body + + # test generating engine's route from engine with default_url_options + script_name "/foo" + get "/john/blog/posts", {}, 'SCRIPT_NAME' => "/foo" + assert_equal "/foo/john/blog/posts/1", last_response.body + reset_script_name! + + # test generating engine's route from application + get "/engine_route" + assert_equal "/anonymous/blog/posts", last_response.body + get "/url_for_engine_route" + assert_equal "/john/blog/posts", last_response.body + + # test generating engine's route from application with default_url_options + script_name "/foo" + get "/engine_route", {}, 'SCRIPT_NAME' => "/foo" + assert_equal "/foo/anonymous/blog/posts", last_response.body + + script_name "/foo" + get "/url_for_engine_route", {}, 'SCRIPT_NAME' => "/foo" + assert_equal "/foo/john/blog/posts", last_response.body + reset_script_name! + + # test generating application's route from engine + get "/someone/blog/generate_application_route" + assert_equal "/", last_response.body + + # test generating application's route from engine with default_url_options + script_name "/foo" + get "/someone/blog/generate_application_route", {}, 'SCRIPT_NAME' => '/foo' + assert_equal "/foo/", last_response.body + end + end +end + -- cgit v1.2.3