From e197d6f34b72cd96ffc5c6680ed8d433f1b1c4db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Tue, 24 Aug 2010 16:42:14 -0300 Subject: Finally fix the bug where symbols and strings were not having the same behavior in the router. If you were using symbols before for methods like match/get/post/put/delete, it is likely that this commit will break your routes. Everything should behave the same if you are using strings, if not, please open up a ticket. --- actionpack/lib/action_dispatch/routing/mapper.rb | 113 +++++++++-------------- 1 file changed, 46 insertions(+), 67 deletions(-) (limited to 'actionpack/lib/action_dispatch/routing/mapper.rb') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 234e34a24f..f3cbba243c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -44,7 +44,8 @@ module ActionDispatch IGNORE_OPTIONS = [:to, :as, :via, :on, :constraints, :defaults, :only, :except, :anchor, :shallow, :shallow_path, :shallow_prefix] def initialize(set, scope, path, options) - @set, @scope, @options = set, scope, options + @set, @scope = set, scope + @options = (@scope[:options] || {}).merge(options) @path = normalize_path(path) normalize_options! end @@ -57,18 +58,11 @@ module ActionDispatch def normalize_options! path_without_format = @path.sub(/\(\.:format\)$/, '') - @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 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)) @@ -80,8 +74,8 @@ module ActionDispatch end def normalize_path(path) - raise ArgumentError, "path is required" if @scope[:path].blank? && path.blank? - path = 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] @@ -93,7 +87,11 @@ module ActionDispatch @options.reverse_merge!(:controller => /.+?/) end - path + if path.include?(":format") + path + else + "#{path}(.:format)" + end end def app @@ -216,6 +214,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 @@ -324,13 +326,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] ||= {} @@ -362,8 +358,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 = {}) @@ -444,9 +441,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] @@ -700,41 +697,27 @@ module ActionDispatch return member { match(*args) } end - path = options.delete(:path) + if resource_scope? + raise ArgumentError, "can't define route directly in resource(s) scope" + end + action = args.first + path = path_for_action(action, options.delete(:path)) - if action.is_a?(Symbol) || (resource_method_scope? && action.to_s =~ /^[A-Za-z_]\w*$/) - path = path_for_action(action, path) - options[:action] ||= action + if action.to_s =~ /^[\w\/]+$/ + options[:action] ||= action unless action.to_s.include?("/") 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[: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" + else + options[:as] = name_for_action(options[:as]) end - args.push(options) - super + 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 @@ -871,7 +854,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? @@ -882,18 +865,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 @@ -913,6 +888,7 @@ module ActionDispatch 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 @@ -922,15 +898,18 @@ module ActionDispatch name = case @scope[:scope_level] when :collection - [name_prefix, collection_name] + [prefix, name_prefix, collection_name] when :new - [: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 - [shallow_scoping? ? @scope[:shallow_prefix] : name_prefix, member_name] + [name_prefix, member_name, prefix] end - name.unshift(prefix) - name.select(&:present?).join("_") + name.select(&:present?).join("_").presence end end -- cgit v1.2.3