diff options
| -rw-r--r-- | actionpack/lib/action_controller/metal/url_for.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/route_set.rb | 17 | ||||
| -rw-r--r-- | actionpack/test/dispatch/prefix_generation_test.rb | 18 | ||||
| -rw-r--r-- | railties/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | railties/lib/rails/engine.rb | 6 | ||||
| -rw-r--r-- | railties/test/railties/engine_test.rb | 48 | ||||
| -rw-r--r-- | railties/test/railties/mounted_engine_test.rb | 15 |
7 files changed, 78 insertions, 41 deletions
diff --git a/actionpack/lib/action_controller/metal/url_for.rb b/actionpack/lib/action_controller/metal/url_for.rb index dd5ceb3478..0cdd17bc2e 100644 --- a/actionpack/lib/action_controller/metal/url_for.rb +++ b/actionpack/lib/action_controller/metal/url_for.rb @@ -30,9 +30,15 @@ module ActionController :_recall => request.symbolized_path_parameters ).freeze - if _routes.equal?(env["action_dispatch.routes"]) + if (same_origin = _routes.equal?(env["action_dispatch.routes"])) || + (script_name = env["ROUTES_#{_routes.object_id}_SCRIPT_NAME"]) || + (original_script_name = env['SCRIPT_NAME']) @_url_options.dup.tap do |options| - options[:script_name] = request.script_name.dup + if original_script_name + options[:original_script_name] = original_script_name + else + options[:script_name] = same_origin ? request.script_name.dup : script_name + end options.freeze end else diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index cc53e298cc..d3f66f042c 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -163,9 +163,9 @@ module ActionDispatch private def define_named_route_methods(name, route) - define_url_helper route, :"#{name}_path", + define_url_helper route, :"#{name}_path", route.defaults.merge(:use_route => name, :only_path => true) - define_url_helper route, :"#{name}_url", + define_url_helper route, :"#{name}_url", route.defaults.merge(:use_route => name, :only_path => false) end @@ -465,7 +465,7 @@ module ActionDispatch def use_recall_for(key) if @recall[key] && (!@options.key?(key) || @options[key] == @recall[key]) if !named_route_exists? || segment_keys.include?(key) - @options[key] = @recall.delete(key) + @options[key] = @recall.delete(key) end end end @@ -574,7 +574,8 @@ module ActionDispatch end RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length, - :trailing_slash, :anchor, :params, :only_path, :script_name] + :trailing_slash, :anchor, :params, :only_path, :script_name, + :original_script_name] def mounted? false @@ -594,7 +595,13 @@ module ActionDispatch user, password = extract_authentication(options) recall = options.delete(:_recall) - script_name = options.delete(:script_name).presence || _generate_prefix(options) + + original_script_name = options.delete(:original_script_name).presence + script_name = options.delete(:script_name).presence || _generate_prefix(options) + + if script_name && original_script_name + script_name = original_script_name + script_name + end path_options = options.except(*RESERVED_OPTIONS) path_options = yield(path_options) if block_given? diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index ab2f7612ce..6d75c5ec7a 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -166,18 +166,6 @@ module TestGenerationPrefix assert_equal "/generate", last_response.body end - 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 - test "[ENGINE] generating engine's url with polymorphic path" do get "/pure-awesomeness/blog/polymorphic_path_for_engine" assert_equal "/pure-awesomeness/blog/posts/1", last_response.body @@ -200,12 +188,6 @@ module TestGenerationPrefix assert_equal "/something/awesome/blog/posts/1", last_response.body end - 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 "[APP] generating engine's url with polymorphic path" do get "/polymorphic_path_for_engine" assert_equal "/awesome/blog/posts/1", last_response.body diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 18c130e828..851f41249a 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Correctly handle SCRIPT_NAME when generating routes to engine in application + that's mounted at a sub-uri. With this behavior, you *should not* use + default_url_options[:script_name] to set proper application's mount point by + yourself. *Piotr Sarnacki* + * The migration generator will now produce AddXXXToYYY/RemoveXXXFromYYY migrations with references statements, for instance rails g migration AddReferencesToProducts user:references supplier:references{polymorphic} diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index f469c334a7..40f35ae5a6 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -494,7 +494,11 @@ module Rails # Define the Rack API for this engine. def call(env) - app.call(env.merge!(env_config)) + env.merge!(env_config) + if env['SCRIPT_NAME'] + env.merge! "ROUTES_#{routes.object_id}_SCRIPT_NAME" => env['SCRIPT_NAME'].dup + end + app.call(env) end # Defines additional Rack env configuration that is added on each call. diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 63814f7a04..e52b3efdab 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -1193,6 +1193,54 @@ YAML last_response.body.split("\n").map(&:strip) end + test "paths are properly generated when application is mounted at sub-path" do + @plugin.write "lib/bukkits.rb", <<-RUBY + module Bukkits + class Engine < ::Rails::Engine + isolate_namespace Bukkits + end + end + RUBY + + app_file "app/controllers/bar_controller.rb", <<-RUBY + class BarController < ApplicationController + def index + render :text => bukkits.bukkit_path + end + end + RUBY + + app_file "config/routes.rb", <<-RUBY + AppTemplate::Application.routes.draw do + get '/bar' => 'bar#index', :as => 'bar' + mount Bukkits::Engine => "/bukkits", :as => "bukkits" + end + RUBY + + @plugin.write "config/routes.rb", <<-RUBY + Bukkits::Engine.routes.draw do + get '/bukkit' => 'bukkit#index' + end + RUBY + + + @plugin.write "app/controllers/bukkits/bukkit_controller.rb", <<-RUBY + class Bukkits::BukkitController < ActionController::Base + def index + render :text => main_app.bar_path + end + end + RUBY + + boot_rails + + get("/bukkits/bukkit", {}, {'SCRIPT_NAME' => '/foo'}) + assert_equal '/foo/bar', last_response.body + + get("/bar", {}, {'SCRIPT_NAME' => '/foo'}) + assert_equal '/foo/bukkits/bukkit', last_response.body + end + private def app Rails.application diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb index 4c0fdee556..bd13c3aba3 100644 --- a/railties/test/railties/mounted_engine_test.rb +++ b/railties/test/railties/mounted_engine_test.rb @@ -163,24 +163,14 @@ module ApplicationTests 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" @@ -193,14 +183,11 @@ module ApplicationTests 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" @@ -210,10 +197,8 @@ module ApplicationTests 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 - reset_script_name! # test polymorphic routes get "/polymorphic_route" |