From 833a90594e9fe25819c5111c853adc91eb6071c2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Mar 2011 15:15:40 -0700 Subject: RegexpWithNamedGroups is only used for path_info, so only apply to path info --- actionpack/lib/action_dispatch/routing/route.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb index eae9d4ea6d..0f90b2169d 100644 --- a/actionpack/lib/action_dispatch/routing/route.rb +++ b/actionpack/lib/action_dispatch/routing/route.rb @@ -23,7 +23,8 @@ module ActionDispatch conditions[:path_info] = ::Rack::Mount::Strexp.compile(path, requirements, SEPARATORS, anchor) end - @conditions = Hash[conditions.map { |k,v| [k, Rack::Mount::RegexpWithNamedGroups.new(v)] }] + @conditions = conditions.dup + @conditions[:path_info] = Rack::Mount::RegexpWithNamedGroups.new(@conditions[:path_info]) if @conditions[:path_info] @conditions.delete_if{ |k,v| k != :path_info && !valid_condition?(k) } @requirements.delete_if{ |k,v| !valid_condition?(k) } end -- cgit v1.2.3 From 9ba2d422f58e17b9e3d2a91d9979017aa36d13bd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Mar 2011 16:17:05 -0700 Subject: stop splatting arguments so that we can understand wtf is going on --- actionpack/lib/action_dispatch/routing/mapper.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index f67708722b..e7b267da81 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -365,8 +365,9 @@ module ActionDispatch # # See Scoping#defaults for its scope equivalent. def match(path, options=nil) - mapping = Mapping.new(@set, @scope, path, options || {}).to_route - @set.add_route(*mapping) + mapping = Mapping.new(@set, @scope, path, options || {}) + app, conditions, requirements, defaults, as, anchor = mapping.to_route + @set.add_route(app, conditions, requirements, defaults, as, anchor) self end -- cgit v1.2.3 From 9f2706d770a2a5513d54dac9d0fb536f787a434b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Mar 2011 18:26:30 -0700 Subject: use a list to represent the supported verbs for a route object --- actionpack/lib/action_dispatch/routing/mapper.rb | 4 ++-- actionpack/lib/action_dispatch/routing/route.rb | 20 +++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e7b267da81..cc6b8aa82d 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -195,8 +195,8 @@ module ActionDispatch def request_method_condition if via = @options[:via] - via = Array(via).map { |m| m.to_s.dasherize.upcase } - { :request_method => %r[^#{via.join('|')}$] } + list = Array(via).map { |m| m.to_s.dasherize.upcase } + { :request_method => list } else { } end diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb index 0f90b2169d..a049510182 100644 --- a/actionpack/lib/action_dispatch/routing/route.rb +++ b/actionpack/lib/action_dispatch/routing/route.rb @@ -12,6 +12,8 @@ module ActionDispatch @defaults = defaults @name = name + # FIXME: we should not be doing this much work in a constructor. + @requirements = requirements.merge(defaults) @requirements.delete(:controller) if @requirements[:controller].is_a?(Regexp) @requirements.delete_if { |k, v| @@ -23,22 +25,22 @@ module ActionDispatch conditions[:path_info] = ::Rack::Mount::Strexp.compile(path, requirements, SEPARATORS, anchor) end + @verbs = conditions[:request_method] || [] + @conditions = conditions.dup + + # Rack-Mount requires that :request_method be a regular expression. + # :request_method represents the HTTP verb that matches this route. + # + # Here we munge values before they get sent on to rack-mount. + @conditions[:request_method] = %r[^#{verb}$] unless @verbs.empty? @conditions[:path_info] = Rack::Mount::RegexpWithNamedGroups.new(@conditions[:path_info]) if @conditions[:path_info] @conditions.delete_if{ |k,v| k != :path_info && !valid_condition?(k) } @requirements.delete_if{ |k,v| !valid_condition?(k) } end def verb - if method = conditions[:request_method] - case method - when Regexp - source = method.source.upcase - source =~ /\A\^[-A-Z|]+\$\Z/ ? source[1..-2] : source - else - method.to_s.upcase - end - end + @verbs.join '|' end def segment_keys -- cgit v1.2.3 From 0eae62525696b57fe7fc4bbb0bf9c0bc7ee4e480 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Wed, 16 Mar 2011 19:03:25 +0100 Subject: fixes an issue with number_to_human when converting values which are less than 1 but greater than -1 [#6576 state:resolved] Signed-off-by: Santiago Pastorino --- actionpack/lib/action_view/helpers/number_helper.rb | 2 +- actionpack/test/template/number_helper_test.rb | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb index 4e44843c4b..05a9c5b4f1 100644 --- a/actionpack/lib/action_view/helpers/number_helper.rb +++ b/actionpack/lib/action_view/helpers/number_helper.rb @@ -472,7 +472,7 @@ module ActionView end.keys.map{|e_name| inverted_du[e_name] }.sort_by{|e| -e} number_exponent = number != 0 ? Math.log10(number.abs).floor : 0 - display_exponent = unit_exponents.find{|e| number_exponent >= e } + display_exponent = unit_exponents.find{ |e| number_exponent >= e } || 0 number /= 10 ** display_exponent unit = case units diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb index c1b4bab903..c8d50ebf75 100644 --- a/actionpack/test/template/number_helper_test.rb +++ b/actionpack/test/template/number_helper_test.rb @@ -195,7 +195,9 @@ class NumberHelperTest < ActionView::TestCase def test_number_to_human assert_equal '-123', number_to_human(-123) - assert_equal '0', number_to_human(0) + assert_equal '-0.5', number_to_human(-0.5) + assert_equal '0', number_to_human(0) + assert_equal '0.5', number_to_human(0.5) assert_equal '123', number_to_human(123) assert_equal '1.23 Thousand', number_to_human(1234) assert_equal '12.3 Thousand', number_to_human(12345) -- cgit v1.2.3 From de1fe5e8a723baa095e83bb3ddca6af73b0aab5d Mon Sep 17 00:00:00 2001 From: Chris Kowalik Date: Sat, 19 Mar 2011 12:13:59 +0800 Subject: [action_view] added custom patterns to template resolver --- actionpack/lib/action_view/template/resolver.rb | 67 ++++++++++++++++------ actionpack/lib/action_view/testing/resolvers.rb | 12 ++-- .../test/fixtures/custom_pattern/another.html.erb | 1 + .../test/fixtures/custom_pattern/html/another.erb | 1 + .../test/fixtures/custom_pattern/html/path.erb | 1 + actionpack/test/template/resolver_patterns_test.rb | 31 ++++++++++ 6 files changed, 89 insertions(+), 24 deletions(-) create mode 100644 actionpack/test/fixtures/custom_pattern/another.html.erb create mode 100644 actionpack/test/fixtures/custom_pattern/html/another.erb create mode 100644 actionpack/test/fixtures/custom_pattern/html/path.erb create mode 100644 actionpack/test/template/resolver_patterns_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 589b2a1a76..0966970a68 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -5,6 +5,25 @@ require "action_view/template" module ActionView # = Action View Resolver class Resolver + + class Path < String + attr_reader :name, :prefix, :partial, :virtual + alias_method :partial?, :partial + + def initialize(name, prefix, partial) + @name, @prefix, @partial = name, prefix, partial + rebuild(@name, @prefix, @partial) + end + + def rebuild(name, prefix, partial) + @virtual = "" + @virtual << "#{prefix}/" unless prefix.empty? + @virtual << (partial ? "_#{name}" : name) + + self.replace(@virtual) + end + end + cattr_accessor :caching self.caching = true @@ -41,10 +60,7 @@ module ActionView # Helpers that builds a path. Useful for building virtual paths. def build_path(name, prefix, partial) - path = "" - path << "#{prefix}/" unless prefix.empty? - path << (partial ? "_#{name}" : name) - path + Path.new(name, prefix, partial) end # Handles templates caching. If a key is given and caching is on @@ -97,25 +113,24 @@ module ActionView end class PathResolver < Resolver - EXTENSION_ORDER = [:locale, :formats, :handlers] + EXTENSIONS = [:locale, :formats, :handlers] + DEFAULT_PATTERN = ":prefix/:action{.:locale,}{.:formats,}{.:handlers,}" + + def initialize(pattern=nil) + @pattern = pattern || DEFAULT_PATTERN + super() + end private def find_templates(name, prefix, partial, details) path = build_path(name, prefix, partial) - query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats]) + extensions = Hash[EXTENSIONS.map { |ext| [ext, details[ext]] }.flatten(0)] + query(path, extensions, details[:formats]) end def query(path, exts, formats) - query = File.join(@path, path) - - query << exts.map { |ext| - "{#{ext.compact.map { |e| ".#{e}" }.join(',')},}" - }.join - - query.gsub!(/\{\.html,/, "{.html,.text.html,") - query.gsub!(/\{\.text,/, "{.text,.text.plain,") - + query = build_query(path, exts) templates = [] sanitizer = Hash.new { |h,k| h[k] = Dir["#{File.dirname(k)}/*"] } @@ -126,12 +141,28 @@ module ActionView contents = File.open(p, "rb") {|io| io.read } templates << Template.new(contents, File.expand_path(p), handler, - :virtual_path => path, :format => format, :updated_at => mtime(p)) + :virtual_path => path.virtual, :format => format, :updated_at => mtime(p)) end templates end + # Helper for building query glob string based on resolver's pattern. + def build_query(path, exts) + query = @pattern.dup + query.gsub!(/\:prefix(\/)?/, path.prefix.empty? ? "" : "#{path.prefix}\\1") # prefix can be empty... + query.gsub!(/\:action/, path.partial? ? "_#{path.name}" : path.name) + + exts.each { |ext, variants| + query.gsub!(/\:#{ext}/, "{#{variants.compact.uniq.join(',')}}") + } + + query.gsub!(/\.{html,/, ".{html,text.html,") + query.gsub!(/\.{text,/, ".{text,text.plain,") + + File.expand_path(query, @path) + end + # Returns the file mtime from the filesystem. def mtime(p) File.stat(p).mtime @@ -151,9 +182,9 @@ module ActionView # A resolver that loads files from the filesystem. class FileSystemResolver < PathResolver - def initialize(path) + def initialize(path, pattern=nil) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) - super() + super(pattern) @path = File.expand_path(path) end diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index 5c5cab7c7d..773dfcbb1d 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -8,8 +8,8 @@ module ActionView #:nodoc: class FixtureResolver < PathResolver attr_reader :hash - def initialize(hash = {}) - super() + def initialize(hash = {}, pattern=nil) + super(pattern) @hash = hash end @@ -21,8 +21,8 @@ module ActionView #:nodoc: def query(path, exts, formats) query = "" - exts.each do |ext| - query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)' + EXTENSIONS.each do |ext| + query << '(' << exts[ext].map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)' end query = /^(#{Regexp.escape(path)})#{query}$/ @@ -32,9 +32,9 @@ module ActionView #:nodoc: next unless _path =~ query handler, format = extract_handler_and_format(_path, formats) templates << Template.new(source, _path, handler, - :virtual_path => $1, :format => format, :updated_at => updated_at) + :virtual_path => path.virtual, :format => format, :updated_at => updated_at) end - + templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size } end end diff --git a/actionpack/test/fixtures/custom_pattern/another.html.erb b/actionpack/test/fixtures/custom_pattern/another.html.erb new file mode 100644 index 0000000000..6d7f3bafbb --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/another.html.erb @@ -0,0 +1 @@ +Hello custom patterns! \ No newline at end of file diff --git a/actionpack/test/fixtures/custom_pattern/html/another.erb b/actionpack/test/fixtures/custom_pattern/html/another.erb new file mode 100644 index 0000000000..dbd7e96ab6 --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/html/another.erb @@ -0,0 +1 @@ +Another template! \ No newline at end of file diff --git a/actionpack/test/fixtures/custom_pattern/html/path.erb b/actionpack/test/fixtures/custom_pattern/html/path.erb new file mode 100644 index 0000000000..6d7f3bafbb --- /dev/null +++ b/actionpack/test/fixtures/custom_pattern/html/path.erb @@ -0,0 +1 @@ +Hello custom patterns! \ No newline at end of file diff --git a/actionpack/test/template/resolver_patterns_test.rb b/actionpack/test/template/resolver_patterns_test.rb new file mode 100644 index 0000000000..97b1bad055 --- /dev/null +++ b/actionpack/test/template/resolver_patterns_test.rb @@ -0,0 +1,31 @@ +require 'abstract_unit' + +class ResolverPatternsTest < ActiveSupport::TestCase + def setup + path = File.expand_path("../../fixtures/", __FILE__) + pattern = ":prefix/{:formats/,}:action{.:formats,}{.:handlers,}" + @resolver = ActionView::FileSystemResolver.new(path, pattern) + end + + def test_should_return_empty_list_for_unknown_path + templates = @resolver.find_all("unknown", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]}) + assert_equal [], templates, "expected an empty list of templates" + end + + def test_should_return_template_for_declared_path + templates = @resolver.find_all("path", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]}) + assert_equal 1, templates.size, "expected one template" + assert_equal "Hello custom patterns!", templates.first.source + assert_equal "custom_pattern/path", templates.first.virtual_path + assert_equal [:html], templates.first.formats + end + + def test_should_return_all_templates_when_ambigous_pattern + templates = @resolver.find_all("another", "custom_pattern", false, {:locale => [], :formats => [:html], :handlers => [:erb]}) + assert_equal 2, templates.size, "expected two templates" + assert_equal "Another template!", templates[0].source + assert_equal "custom_pattern/another", templates[0].virtual_path + assert_equal "Hello custom patterns!", templates[1].source + assert_equal "custom_pattern/another", templates[1].virtual_path + end +end -- cgit v1.2.3 From d76fadbd7d4f33b7fc208ce5242b37f0350ad493 Mon Sep 17 00:00:00 2001 From: Chris Kowalik Date: Sun, 20 Mar 2011 06:21:11 +0800 Subject: [action_view] docs for FileSystemResolver --- actionpack/lib/action_view/template/resolver.rb | 32 +++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 0966970a68..f9c52e3228 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -5,7 +5,7 @@ require "action_view/template" module ActionView # = Action View Resolver class Resolver - + # Keeps all information about view path and builds virtual path. class Path < String attr_reader :name, :prefix, :partial, :virtual alias_method :partial?, :partial @@ -180,7 +180,35 @@ module ActionView end end - # A resolver that loads files from the filesystem. + # A resolver that loads files from the filesystem. It allows to set your own + # resolving pattern. Such pattern can be a glob string supported by some variables. + # + # ==== Examples + # + # Default pattern, loads views the same way as previous versions of rails, eg. when you're + # looking for `users/new` it will produce query glob: `users/new{.{en},}{.{html,js},}{.{erb,haml,rjs},}` + # + # FileSystemResolver.new("/path/to/views", ":prefix/:action{.:locale,}{.:formats,}{.:handlers,}") + # + # This one allows you to keep files with different formats in seperated subdirectories, + # eg. `users/new.html` will be loaded from `users/html/new.erb` or `users/new.html.erb`, + # `users/new.js` from `users/js/new.erb` or `users/new.js.erb`, etc. + # + # FileSystemResolver.new("/path/to/views", ":prefix/{:formats/,}:action{.:locale,}{.:formats,}{.:handlers,}") + # + # If you don't specify pattern then the default will be used. + # + # ==== Pattern format and variables + # + # Pattern have to be a valid glob string, and it allows you to use the + # following variables: + # + # * :prefix - usualy the controller path + # * :action - name of the action + # * :locale - possible locale versions + # * :formats - possible file formats + # * :handlers - possible handlers + # class FileSystemResolver < PathResolver def initialize(path, pattern=nil) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) -- cgit v1.2.3 From 1413c9b1d51414b72c4efb8583a3acae04a481dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 19 Mar 2011 16:06:50 -0700 Subject: Improved resolver docs a bit --- actionpack/lib/action_view/template/resolver.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index f9c52e3228..6c1063592f 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -198,6 +198,14 @@ module ActionView # # If you don't specify pattern then the default will be used. # + # In order to use any of the customized resolvers above in a Rails application, you just need + # to configure ActionController::Base.view_paths in an initializer, for example: + # + # ActionController::Base.view_paths = FileSystemResolver.new( + # Rails.root.join("app/views"), + # ":prefix{/:locale}/:action{.:formats,}{.:handlers,}" + # ) + # # ==== Pattern format and variables # # Pattern have to be a valid glob string, and it allows you to use the @@ -206,8 +214,8 @@ module ActionView # * :prefix - usualy the controller path # * :action - name of the action # * :locale - possible locale versions - # * :formats - possible file formats - # * :handlers - possible handlers + # * :formats - possible request formats (for example html, json, xml...) + # * :handlers - possible handlers (for example erb, haml, builder...) # class FileSystemResolver < PathResolver def initialize(path, pattern=nil) -- cgit v1.2.3 From 55bf087da1d93eda93f9b9632fc363fd4788bed2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 21 Mar 2011 10:00:30 -0700 Subject: SJIS is an alias to Windows-31J in ruby trunk. Use SHIFT_JIS for this test --- actionpack/test/template/render_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 034fb6c210..dd86bfed04 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -381,7 +381,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase end def test_render_utf8_template_with_incompatible_external_encoding - with_external_encoding Encoding::SJIS do + with_external_encoding Encoding::SHIFT_JIS do begin result = @view.render(:file => "test/utf8.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' @@ -392,7 +392,7 @@ class LazyViewRenderTest < ActiveSupport::TestCase end def test_render_utf8_template_with_partial_with_incompatible_encoding - with_external_encoding Encoding::SJIS do + with_external_encoding Encoding::SHIFT_JIS do begin result = @view.render(:file => "test/utf8_magic_with_bare_partial.html.erb", :layouts => "layouts/yield") flunk 'Should have raised incompatible encoding error' -- cgit v1.2.3 From 2ddfdba9a0dab7d8499c3ad0d13583bddbac4f69 Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Tue, 22 Mar 2011 22:19:31 +0700 Subject: Do not show optional (.:format) block for wildcard route [#6605 state:resolved] This will make the output of `rake routes` to be correctly match to the behavior of the application, as the regular expression used to match the path is greedy and won't capture the format part by default --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- actionpack/test/action_dispatch/routing/mapper_test.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index cc6b8aa82d..1dba1d416c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -107,7 +107,7 @@ module ActionDispatch if @options[:format] == false @options.delete(:format) path - elsif path.include?(":format") || path.end_with?('/') + elsif path.include?(":format") || path.end_with?('/') || path.match(/^\/?\*/) path else "#{path}(.:format)" diff --git a/actionpack/test/action_dispatch/routing/mapper_test.rb b/actionpack/test/action_dispatch/routing/mapper_test.rb index 9966234f1b..e21b271907 100644 --- a/actionpack/test/action_dispatch/routing/mapper_test.rb +++ b/actionpack/test/action_dispatch/routing/mapper_test.rb @@ -46,6 +46,13 @@ module ActionDispatch mapper.match '/one/two/', :to => 'posts#index', :as => :main assert_equal '/one/two(.:format)', fakeset.conditions.first[:path_info] end + + def test_map_wildcard + fakeset = FakeSet.new + mapper = Mapper.new fakeset + mapper.match '/*path', :to => 'pages#show', :as => :page + assert_equal '/*path', fakeset.conditions.first[:path_info] + end end end end -- cgit v1.2.3 From 5da9a74bd35284cf5124793c1f7558e506b52592 Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Fri, 25 Feb 2011 12:34:46 +0100 Subject: Add a failing test case for an implicit action with a before filter. Signed-off-by: Andrew White --- actionpack/test/controller/filters_test.rb | 22 ++++++++++++++++++++++ .../filter_test/implicit_actions/edit.html.erb | 1 + .../filter_test/implicit_actions/show.html.erb | 1 + 3 files changed, 24 insertions(+) create mode 100644 actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb create mode 100644 actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb (limited to 'actionpack') diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index 330fa276d0..c95332220b 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -505,6 +505,16 @@ class FilterTest < ActionController::TestCase end end + class ImplicitActionsController < ActionController::Base + before_filter :find_user, :only => :edit + + private + + def find_user + @user = 'Jenny' + end + end + def test_sweeper_should_not_block_rendering response = test_process(SweeperTestController) assert_equal 'hello world', response.body @@ -783,6 +793,18 @@ class FilterTest < ActionController::TestCase assert_equal("I rescued this: #", response.body) end + def test_filter_runs_for_implicitly_defined_action_when_needed + test_process(ImplicitActionsController, 'edit') + assert_equal 'Jenny', assigns(:user) + assert_equal 'edit', response.body + end + + def test_filter_does_not_run_for_implicity_defined_action_when_not_needed + test_process(ImplicitActionsController, 'show') + assert_nil assigns(:user) + assert_equal 'show', response.body + end + private def test_process(controller, action = "show") @controller = controller.is_a?(Class) ? controller.new : controller diff --git a/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb b/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb new file mode 100644 index 0000000000..8491ab9f80 --- /dev/null +++ b/actionpack/test/fixtures/filter_test/implicit_actions/edit.html.erb @@ -0,0 +1 @@ +edit \ No newline at end of file diff --git a/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb b/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb new file mode 100644 index 0000000000..0a89cecf05 --- /dev/null +++ b/actionpack/test/fixtures/filter_test/implicit_actions/show.html.erb @@ -0,0 +1 @@ +show \ No newline at end of file -- cgit v1.2.3 From 9772de8d459960cc114c5b214343b7ce08fea21c Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 23 Mar 2011 23:09:00 +0000 Subject: Fix filter :only and :except with implicit actions The method_name argument is "default_render" for implicit actions so use the action_name attribute to determine which callbacks to run. [#5673 state:resolved] --- actionpack/lib/abstract_controller/callbacks.rb | 2 +- actionpack/test/controller/filters_test.rb | 27 +++++++++++++++---------- 2 files changed, 17 insertions(+), 12 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb index 95992c2698..1943ca4436 100644 --- a/actionpack/lib/abstract_controller/callbacks.rb +++ b/actionpack/lib/abstract_controller/callbacks.rb @@ -14,7 +14,7 @@ module AbstractController # Override AbstractController::Base's process_action to run the # process_action callbacks around the normal behavior. def process_action(method_name, *args) - run_callbacks(:process_action, method_name) do + run_callbacks(:process_action, action_name) do super end end diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index c95332220b..9e44e8e088 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -506,12 +506,17 @@ class FilterTest < ActionController::TestCase end class ImplicitActionsController < ActionController::Base - before_filter :find_user, :only => :edit + before_filter :find_only, :only => :edit + before_filter :find_except, :except => :edit private - def find_user - @user = 'Jenny' + def find_only + @only = 'Only' + end + + def find_except + @except = 'Except' end end @@ -793,16 +798,16 @@ class FilterTest < ActionController::TestCase assert_equal("I rescued this: #", response.body) end - def test_filter_runs_for_implicitly_defined_action_when_needed - test_process(ImplicitActionsController, 'edit') - assert_equal 'Jenny', assigns(:user) - assert_equal 'edit', response.body - end - - def test_filter_does_not_run_for_implicity_defined_action_when_not_needed + def test_filters_obey_only_and_except_for_implicit_actions test_process(ImplicitActionsController, 'show') - assert_nil assigns(:user) + assert_equal 'Except', assigns(:except) + assert_nil assigns(:only) assert_equal 'show', response.body + + test_process(ImplicitActionsController, 'edit') + assert_equal 'Only', assigns(:only) + assert_nil assigns(:except) + assert_equal 'edit', response.body end private -- cgit v1.2.3 From 5170d210e84ab38fa93a1346ebe32b12e75d72bc Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Thu, 24 Mar 2011 23:24:47 +0100 Subject: correction to the outputted controller name in the diagnostics error template, test included Signed-off-by: Santiago Pastorino --- .../middleware/templates/rescues/diagnostics.erb | 2 +- actionpack/test/dispatch/show_exceptions_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb index 50d8ca9484..2099fd069a 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb @@ -1,7 +1,7 @@

<%=h @exception.class.to_s %> <% if @request.parameters['controller'] %> - in <%=h @request.parameters['controller'].humanize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> + in <%=h @request.parameters['controller'].classify.pluralize %>Controller<% if @request.parameters['action'] %>#<%=h @request.parameters['action'] %><% end %> <% end %> 

<%=h @exception.message %>
diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 2a478c214f..e453dd11ce 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -7,6 +7,8 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest case req.path when "/not_found" raise ActionController::UnknownAction + when "/runtime_error" + raise RuntimeError when "/method_not_allowed" raise ActionController::MethodNotAllowed when "/not_implemented" @@ -121,4 +123,18 @@ class ShowExceptionsTest < ActionDispatch::IntegrationTest assert_response 404 assert_match(/AbstractController::ActionNotFound/, body) end + + test "show the controller name in the diagnostics template when controller name is present" do + @app = ProductionApp + get("/runtime_error", {}, { + 'action_dispatch.show_exceptions' => true, + 'action_dispatch.request.parameters' => { + 'action' => 'show', + 'id' => 'unknown', + 'controller' => 'featured_tiles' + } + }) + assert_response 500 + assert_match(/RuntimeError\n in FeaturedTilesController/, body) + end end -- cgit v1.2.3 From 198f3883fab0a8b5ec96f5365286620344b0e6e7 Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Sat, 26 Mar 2011 11:34:05 +1100 Subject: Proc objects for cache_path for caches_action no longer need controller object, nor to use send when calling routing helpers --- actionpack/lib/action_controller/caching/actions.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/caching/actions.rb b/actionpack/lib/action_controller/caching/actions.rb index 2c8a6e4d4d..5fc6956266 100644 --- a/actionpack/lib/action_controller/caching/actions.rb +++ b/actionpack/lib/action_controller/caching/actions.rb @@ -56,19 +56,18 @@ module ActionController #:nodoc: # # caches_page :public # - # caches_action :index, :if => proc do |c| - # !c.request.format.json? # cache if is not a JSON request + # caches_action :index, :if => proc do + # !request.format.json? # cache if is not a JSON request # end # # caches_action :show, :cache_path => { :project => 1 }, # :expires_in => 1.hour # - # caches_action :feed, :cache_path => proc do |c| - # if c.params[:user_id] - # c.send(:user_list_url, - # c.params[:user_id], c.params[:id]) + # caches_action :feed, :cache_path => proc do + # if params[:user_id] + # user_list_url(params[:user_id, params[:id]) # else - # c.send(:list_url, c.params[:id]) + # list_url(params[:id]) # end # end # end -- cgit v1.2.3