diff options
Diffstat (limited to 'actionpack')
-rw-r--r-- | actionpack/CHANGELOG | 5 | ||||
-rw-r--r-- | actionpack/actionpack.gemspec | 2 | ||||
-rw-r--r-- | actionpack/lib/action_controller/test_case.rb | 1 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/http/parameters.rb | 4 | ||||
-rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 201 | ||||
-rw-r--r-- | actionpack/lib/action_pack/version.rb | 4 | ||||
-rw-r--r-- | actionpack/test/controller/layout_test.rb | 2 | ||||
-rw-r--r-- | actionpack/test/controller/test_test.rb | 8 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 77 |
9 files changed, 172 insertions, 132 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index dce2e30d3f..66141c1de7 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,3 +1,8 @@ +*Support routing constraints in functional tests. [Andrew White] + +*Add a header that tells Internet Explorer (all versions) to use the best available standards support. [Yehuda Katz] + + *Rails 3.0.0 [release candidate] (July 26th, 2010)* * Allow stylesheet/javascript extensions to be changed through railties. [Josh Kalderimis] diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index a5ebc18e2a..8fd77aeb17 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -25,7 +25,7 @@ Gem::Specification.new do |s| s.add_dependency('i18n', '~> 0.4.1') s.add_dependency('rack', '~> 1.2.1') s.add_dependency('rack-test', '~> 0.5.4') - s.add_dependency('rack-mount', '~> 0.6.10') + s.add_dependency('rack-mount', '~> 0.6.12') s.add_dependency('tzinfo', '~> 0.3.23') s.add_dependency('erubis', '~> 2.6.6') end diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index e02fe202e1..75ea6523f7 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -167,6 +167,7 @@ module ActionController @formats = nil @env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ } @env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ } + @symbolized_path_params = nil @method = @request_method = nil @fullpath = @ip = @remote_ip = nil @env['action_dispatch.request.query_parameters'] = {} diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 5e1a2405f7..ef5d207b26 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -15,14 +15,14 @@ module ActionDispatch alias :params :parameters def path_parameters=(parameters) #:nodoc: - @_symbolized_path_params = nil + @symbolized_path_params = nil @env.delete("action_dispatch.request.parameters") @env["action_dispatch.request.path_parameters"] = parameters end # The same as <tt>path_parameters</tt> with explicitly symbolized keys. def symbolized_path_parameters - @_symbolized_path_params ||= path_parameters.symbolize_keys + @symbolized_path_params ||= path_parameters.symbolize_keys end # Returns a hash with the \parameters used to form the \path of the request. diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 23b13d1d5d..9aa34e7ba5 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -43,9 +43,10 @@ module ActionDispatch class Mapping #:nodoc: IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] - def initialize(set, scope, args) - @set, @scope = set, scope - @path, @options = extract_path_and_options(args) + def initialize(set, scope, path, options) + @set, @scope = set, scope + @options = (@scope[:options] || {}).merge(options) + @path = normalize_path(path) normalize_options! end @@ -54,28 +55,6 @@ module ActionDispatch end private - def extract_path_and_options(args) - options = args.extract_options! - - if using_to_shorthand?(args, options) - path, to = options.find { |name, value| name.is_a?(String) } - options.merge!(:to => to).delete(path) if path - else - path = args.first - end - - if path.match(':controller') - raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] - - # Add a default constraint for :controller path segments that matches namespaced - # controllers with default routes like :controller/:action/:id(.:format), e.g: - # GET /admin/products/show/1 - # => { :controller => 'admin/products', :action => 'show', :id => '1' } - options.reverse_merge!(:controller => /.+?/) - end - - [ normalize_path(path), options ] - end def normalize_options! path_without_format = @path.sub(/\(\.:format\)$/, '') @@ -83,25 +62,39 @@ module ActionDispatch if using_match_shorthand?(path_without_format, @options) to_shorthand = @options[:to].blank? @options[:to] ||= path_without_format[1..-1].sub(%r{/([^/]*)$}, '#\1') - @options[:as] ||= path_without_format[1..-1].gsub("/", "_") + @options[:as] ||= Mapper.normalize_name(path_without_format) end @options.merge!(default_controller_and_action(to_shorthand)) end - # match "account" => "account#index" - def using_to_shorthand?(args, options) - args.empty? && options.present? - end - # match "account/overview" def using_match_shorthand?(path, options) path && options.except(:via, :anchor, :to, :as).empty? && path =~ %r{^/[\w\/]+$} end def normalize_path(path) - raise ArgumentError, "path is required" if @scope[:path].blank? && path.blank? - Mapper.normalize_path("#{@scope[:path]}/#{path}") + raise ArgumentError, "path is required" if path.blank? + path = Mapper.normalize_path(path) + + if path.match(':controller') + raise ArgumentError, ":controller segment is not allowed within a namespace block" if @scope[:module] + + # Add a default constraint for :controller path segments that matches namespaced + # controllers with default routes like :controller/:action/:id(.:format), e.g: + # GET /admin/products/show/1 + # => { :controller => 'admin/products', :action => 'show', :id => '1' } + @options.reverse_merge!(:controller => /.+?/) + end + + if @options[:format] == false + @options.delete(:format) + path + elsif path.include?(":format") + path + else + "#{path}(.:format)" + end end def app @@ -224,6 +217,10 @@ module ActionDispatch path end + def self.normalize_name(name) + normalize_path(name)[1..-1].gsub("/", "_") + end + module Base def initialize(set) #:nodoc: @set = set @@ -233,8 +230,8 @@ module ActionDispatch match '/', options.reverse_merge(:as => :root) end - def match(*args) - mapping = Mapping.new(@set, @scope, args).to_route + def match(path, options=nil) + mapping = Mapping.new(@set, @scope, path, options || {}).to_route @set.add_route(*mapping) self end @@ -250,7 +247,7 @@ module ActionDispatch raise "A rack application must be specified" unless path - match(path, options.merge(:to => app, :anchor => false)) + match(path, options.merge(:to => app, :anchor => false, :format => false)) self end @@ -332,13 +329,7 @@ module ActionDispatch ActiveSupport::Deprecation.warn ":name_prefix was deprecated in the new router syntax. Use :as instead.", caller end - case args.first - when String - options[:path] = args.first - when Symbol - options[:controller] = args.first - end - + options[:path] = args.first if args.first.is_a?(String) recover = {} options[:constraints] ||= {} @@ -370,8 +361,9 @@ module ActionDispatch @scope[:blocks] = recover[:block] end - def controller(controller) - scope(controller.to_sym) { yield } + def controller(controller, options={}) + options[:controller] = controller + scope(options) { yield } end def namespace(path, options = {}) @@ -389,21 +381,6 @@ module ActionDispatch scope(:defaults => defaults) { yield } end - def match(*args) - options = args.extract_options! - - options = (@scope[:options] || {}).merge(options) - - if @scope[:as] && !options[:as].blank? - options[:as] = "#{@scope[:as]}_#{options[:as]}" - elsif @scope[:as] && options[:as] == "" - options[:as] = @scope[:as].to_s - end - - args.push(options) - super(*args) - end - private def scope_options @scope_options ||= private_methods.grep(/^merge_(.+)_scope$/) { $1.to_sym } @@ -467,9 +444,9 @@ module ActionDispatch module Resources # CANONICAL_ACTIONS holds all actions that does not need a prefix or # a path appended since they fit properly in their scope level. - VALID_ON_OPTIONS = [:new, :collection, :member] - CANONICAL_ACTIONS = [:index, :create, :new, :show, :update, :destroy] - RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except] + VALID_ON_OPTIONS = [:new, :collection, :member] + RESOURCE_OPTIONS = [:as, :controller, :path, :only, :except] + CANONICAL_ACTIONS = %w(index create new show update destroy) class Resource #:nodoc: DEFAULT_ACTIONS = [:index, :create, :new, :show, :update, :destroy, :edit] @@ -723,42 +700,27 @@ module ActionDispatch return member { match(*args) } end - path = options.delete(:path) - action = args.first - - if action.is_a?(Symbol) - path = path_for_action(action, path) - options[:to] ||= action - options[:as] = name_for_action(action, options[:as]) - - with_exclusive_scope do - return super(path, options) - end - elsif resource_method_scope? - path = path_for_custom_action - options[:action] ||= action - options[:as] = name_for_action(options[:as]) if options[:as] - args.push(options) - - with_exclusive_scope do - scope(path) do - return super - end - end - end - if resource_scope? raise ArgumentError, "can't define route directly in resource(s) scope" end - args.push(options) - super + action = args.first + path = path_for_action(action, options.delete(:path)) + + if action.to_s =~ /^[\w\/]+$/ + options[:action] ||= action unless action.to_s.include?("/") + options[:as] = name_for_action(action, options[:as]) + else + options[:as] = name_for_action(options[:as]) + end + + super(path, options) end def root(options={}) if @scope[:scope_level] == :resources - with_scope_level(:nested) do - scope(parent_resource.path, :as => parent_resource.collection_name) do + with_scope_level(:root) do + scope(parent_resource.path) do super(options) end end @@ -895,7 +857,7 @@ module ActionDispatch end def canonical_action?(action, flag) - flag && CANONICAL_ACTIONS.include?(action) + flag && resource_method_scope? && CANONICAL_ACTIONS.include?(action.to_s) end def shallow_scoping? @@ -906,18 +868,10 @@ module ActionDispatch prefix = shallow_scoping? ? "#{@scope[:shallow_path]}/#{parent_resource.path}/:id" : @scope[:path] - if canonical_action?(action, path.blank?) - "#{prefix}(.:format)" - else - "#{prefix}/#{action_path(action, path)}(.:format)" - end - end - - def path_for_custom_action - if shallow_scoping? - "#{@scope[:shallow_path]}/#{parent_resource.path}/:id" + path = if canonical_action?(action, path.blank?) + prefix.to_s else - @scope[:path] + "#{prefix}/#{action_path(action, path)}" end end @@ -927,44 +881,59 @@ module ActionDispatch def prefix_name_for_action(action, as) if as.present? - "#{as}_" + as.to_s elsif as - "" + nil elsif !canonical_action?(action, @scope[:scope_level]) - "#{action}_" + action.to_s end end def name_for_action(action, as=nil) prefix = prefix_name_for_action(action, as) + prefix = Mapper.normalize_name(prefix) if prefix name_prefix = @scope[:as] if parent_resource collection_name = parent_resource.collection_name member_name = parent_resource.member_name - name_prefix = "#{name_prefix}_" if name_prefix.present? end - case @scope[:scope_level] + name = case @scope[:scope_level] when :collection - "#{prefix}#{name_prefix}#{collection_name}" + [prefix, name_prefix, collection_name] when :new - "#{prefix}new_#{name_prefix}#{member_name}" + [prefix, :new, name_prefix, member_name] + when :member + [prefix, shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name] + when :root + [name_prefix, collection_name, prefix] else - if shallow_scoping? - shallow_prefix = "#{@scope[:shallow_prefix]}_" if @scope[:shallow_prefix].present? - "#{prefix}#{shallow_prefix}#{member_name}" - else - "#{prefix}#{name_prefix}#{member_name}" - end + [name_prefix, member_name, prefix] end + + name.select(&:present?).join("_").presence end end + module Shorthand + def match(*args) + if args.size == 1 && args.last.is_a?(Hash) + options = args.pop + path, to = options.find { |name, value| name.is_a?(String) } + options.merge!(:to => to).delete(path) + super(path, options) + else + super + end + end + end + include Base include HttpHelpers include Scoping include Resources + include Shorthand end end end diff --git a/actionpack/lib/action_pack/version.rb b/actionpack/lib/action_pack/version.rb index 7eaf7d0534..73e16daa29 100644 --- a/actionpack/lib/action_pack/version.rb +++ b/actionpack/lib/action_pack/version.rb @@ -1,9 +1,9 @@ module ActionPack module VERSION #:nodoc: MAJOR = 3 - MINOR = 0 + MINOR = 1 TINY = 0 - BUILD = "rc" + BUILD = "beta" STRING = [MAJOR, MINOR, TINY, BUILD].join('.') end diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index e8751747d1..32a0f40614 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -208,7 +208,7 @@ class LayoutStatusIsRenderedTest < ActionController::TestCase end end -unless Config::CONFIG['host_os'] =~ /mswin|mingw/ +unless RbConfig::CONFIG['host_os'] =~ /mswin|mingw/ class LayoutSymlinkedTest < LayoutTest layout "symlinked/symlinked_layout" end diff --git a/actionpack/test/controller/test_test.rb b/actionpack/test/controller/test_test.rb index de2cab3e79..e959b41219 100644 --- a/actionpack/test/controller/test_test.rb +++ b/actionpack/test/controller/test_test.rb @@ -549,6 +549,14 @@ XML assert_blank @request.params[:foo] end + def test_symbolized_path_params_reset_after_request + get :test_params, :id => "foo" + assert_equal "foo", @request.symbolized_path_parameters[:id] + @request.recycle! + get :test_params + assert_nil @request.symbolized_path_parameters[:id] + end + def test_should_have_knowledge_of_client_side_cookie_state_even_if_they_are_not_set @request.cookies['foo'] = 'bar' get :no_op diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index c529db4771..5b2547e700 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -40,6 +40,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get :new, :path => "build" post :create, :path => "create", :as => "" put :update + get :remove, :action => :destroy, :as => :remove + end + + scope "pagemark", :controller => "pagemarks", :as => :pagemark do + get "new", :path => "build" + post "create", :as => "" + put "update" get "remove", :action => :destroy, :as => :remove end @@ -188,7 +195,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - resources :sheep + resources :sheep do + get "_it", :on => :member + end resources :clients do namespace :google do @@ -201,22 +210,27 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end resources :customers do - get "recent" => "customers#recent", :as => :recent, :on => :collection - get "profile" => "customers#profile", :as => :profile, :on => :member - post "preview" => "customers#preview", :as => :preview, :on => :new + get :recent, :on => :collection + get "profile", :on => :member + get "secret/profile" => "customers#secret", :on => :member + post "preview" => "customers#preview", :as => :another_preview, :on => :new resource :avatar do - get "thumbnail(.:format)" => "avatars#thumbnail", :as => :thumbnail, :on => :member + get "thumbnail" => "avatars#thumbnail", :as => :thumbnail, :on => :member end resources :invoices do - get "outstanding" => "invoices#outstanding", :as => :outstanding, :on => :collection + get "outstanding" => "invoices#outstanding", :on => :collection get "overdue", :to => :overdue, :on => :collection get "print" => "invoices#print", :as => :print, :on => :member post "preview" => "invoices#preview", :as => :preview, :on => :new + get "aged/:months", :on => :collection, :action => :aged, :as => :aged end resources :notes, :shallow => true do get "preview" => "notes#preview", :as => :preview, :on => :new get "print" => "notes#print", :as => :print, :on => :member end + get "inactive", :on => :collection + post "deactivate", :on => :member + get "old", :on => :collection, :as => :stale end namespace :api do @@ -642,15 +656,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest with_test_routes do get '/bookmark/build' assert_equal 'bookmarks#new', @response.body - assert_equal '/bookmark/build', new_bookmark_path + assert_equal '/bookmark/build', bookmark_new_path post '/bookmark/create' assert_equal 'bookmarks#create', @response.body assert_equal '/bookmark/create', bookmark_path - put '/bookmark' + put '/bookmark/update' assert_equal 'bookmarks#update', @response.body - assert_equal '/bookmark', update_bookmark_path + assert_equal '/bookmark/update', bookmark_update_path get '/bookmark/remove' assert_equal 'bookmarks#destroy', @response.body @@ -658,6 +672,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_pagemarks + with_test_routes do + get '/pagemark/build' + assert_equal 'pagemarks#new', @response.body + assert_equal '/pagemark/build', pagemark_new_path + + post '/pagemark/create' + assert_equal 'pagemarks#create', @response.body + assert_equal '/pagemark/create', pagemark_path + + put '/pagemark/update' + assert_equal 'pagemarks#update', @response.body + assert_equal '/pagemark/update', pagemark_update_path + + get '/pagemark/remove' + assert_equal 'pagemarks#destroy', @response.body + assert_equal '/pagemark/remove', pagemark_remove_path + end + end + def test_admin with_test_routes do get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'} @@ -992,6 +1026,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/sheep/1', sheep_path(1) assert_equal '/sheep/new', new_sheep_path assert_equal '/sheep/1/edit', edit_sheep_path(1) + assert_equal '/sheep/1/_it', _it_sheep_path(1) end end @@ -1557,7 +1592,8 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest with_test_routes do assert_equal '/customers/recent', recent_customers_path assert_equal '/customers/1/profile', profile_customer_path(:id => '1') - assert_equal '/customers/new/preview', preview_new_customer_path + assert_equal '/customers/1/secret/profile', secret_profile_customer_path(:id => '1') + assert_equal '/customers/new/preview', another_preview_new_customer_path assert_equal '/customers/1/avatar/thumbnail.jpg', thumbnail_customer_avatar_path(:customer_id => '1', :format => :jpg) assert_equal '/customers/1/invoices/outstanding', outstanding_customer_invoices_path(:customer_id => '1') assert_equal '/customers/1/invoices/2/print', print_customer_invoice_path(:customer_id => '1', :id => '2') @@ -1570,6 +1606,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/customers/1/invoices/overdue' assert_equal 'invoices#overdue', @response.body + + get '/customers/1/secret/profile' + assert_equal 'customers#secret', @response.body end end @@ -2034,6 +2073,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest verify_redirect 'http://www.example.com/countries/all/cities' end + def test_custom_resource_actions_defined_using_string + get '/customers/inactive' + assert_equal 'customers#inactive', @response.body + assert_equal '/customers/inactive', inactive_customers_path + + post '/customers/1/deactivate' + assert_equal 'customers#deactivate', @response.body + assert_equal '/customers/1/deactivate', deactivate_customer_path(:id => '1') + + get '/customers/old' + assert_equal 'customers#old', @response.body + assert_equal '/customers/old', stale_customers_path + + get '/customers/1/invoices/aged/3' + assert_equal 'invoices#aged', @response.body + assert_equal '/customers/1/invoices/aged/3', aged_customer_invoices_path(:customer_id => '1', :months => '3') + end + private def with_test_routes yield |