From 626881577949ffce8370e097aa3efb3fec47d4f5 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 12:40:01 -0300 Subject: Initialize @view_context_class and cache view_context_class value. --- actionpack/lib/abstract_controller/rendering.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 5d9b35d297..918def3e81 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -66,7 +66,7 @@ module AbstractController attr_writer :view_context_class def view_context_class - @view_context_class || self.class.view_context_class + @view_context_class ||= self.class.view_context_class end def initialize(*) -- cgit v1.2.3 From bc0e7f4e374d4f72d1254f51e8c116f358826855 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 11:10:38 -0300 Subject: Test correct method behaviour. --- actionpack/test/controller/capture_test.rb | 6 +++++- actionpack/test/fixtures/test/proper_block_detection.erb | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/capture_test.rb b/actionpack/test/controller/capture_test.rb index eb426e855b..d78acb8ce8 100644 --- a/actionpack/test/controller/capture_test.rb +++ b/actionpack/test/controller/capture_test.rb @@ -25,6 +25,10 @@ class CaptureController < ActionController::Base render :layout => "talk_from_action" end + def proper_block_detection + @todo = "some todo" + end + def rescue_action(e) raise end end @@ -66,8 +70,8 @@ class CaptureTest < ActionController::TestCase end def test_proper_block_detection - @todo = "some todo" get :proper_block_detection + assert_equal "some todo", @response.body end private diff --git a/actionpack/test/fixtures/test/proper_block_detection.erb b/actionpack/test/fixtures/test/proper_block_detection.erb index 23564dbcee..b55efbb25d 100644 --- a/actionpack/test/fixtures/test/proper_block_detection.erb +++ b/actionpack/test/fixtures/test/proper_block_detection.erb @@ -1 +1 @@ -<%= @todo %> +<%= @todo %> \ No newline at end of file -- cgit v1.2.3 From 152580ee00205b42de45950d69349c6eab6dd291 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 11:37:08 -0300 Subject: Don't try to interpolate string if there's no interpolation point at all. --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0cb02c5b80..189da138d8 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -349,7 +349,7 @@ module ActionDispatch options = args.last.is_a?(Hash) ? args.pop : {} path = args.shift || block - path_proc = path.is_a?(Proc) ? path : proc { |params| params.empty? ? path : (path % params) } + path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%{\w*}/)) ? path : (path % params) } status = options[:status] || 301 lambda do |env| -- cgit v1.2.3 From 8823b85010a217df555b981a453384e24ce7da47 Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 12:13:58 -0300 Subject: Remove redundant conditional. --- actionpack/lib/action_dispatch/testing/assertions/response.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 10b122557a..e381b9abdf 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -84,11 +84,7 @@ module ActionDispatch when %r{^\w[\w\d+.-]*:.*} fragment when String - if fragment =~ %r{^\w[\w\d+.-]*:.*} - fragment - else - @request.protocol + @request.host_with_port + fragment - end + @request.protocol + @request.host_with_port + fragment when :back raise RedirectBackError unless refer = @request.headers["Referer"] refer -- cgit v1.2.3 From c37800aae123d21d53a49433cac2e0a2479c6bbd Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 12:55:43 -0300 Subject: _ is not a valid scheme name character, \w includes it and also is redundant with \d. 'The scheme name consists of a letter followed by any combination of letters, digits, and the plus ("+"), period ("."), or hyphen ("-") characters; and is terminated by a colon (":").' --- actionpack/lib/action_dispatch/testing/assertions/response.rb | 2 +- actionpack/test/controller/action_pack_assertions_test.rb | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index e381b9abdf..1558c3ae05 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -81,7 +81,7 @@ module ActionDispatch def normalize_argument_to_redirection(fragment) case fragment - when %r{^\w[\w\d+.-]*:.*} + when %r{^\w[A-Za-z\d+.-]*:.*} fragment when String @request.protocol + @request.host_with_port + fragment diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index d9d258e593..5a8b763717 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -32,6 +32,8 @@ class ActionPackAssertionsController < ActionController::Base def redirect_to_path() redirect_to '/some/path' end + def redirect_invalid_external_route() redirect_to 'ht_tp://www.rubyonrails.org' end + def redirect_to_named_route() redirect_to route_one_url end def redirect_external() redirect_to "http://www.rubyonrails.org"; end @@ -368,6 +370,11 @@ class ActionPackAssertionsControllerTest < ActionController::TestCase end end + def test_redirect_invalid_external_route + process :redirect_invalid_external_route + assert_redirected_to "http://test.hostht_tp://www.rubyonrails.org" + end + def test_redirected_to_url_full_url process :redirect_to_path assert_redirected_to 'http://test.host/some/path' -- cgit v1.2.3 From 6371e5b99f933ebe462784b5272fdf12e9602e5a Mon Sep 17 00:00:00 2001 From: Emilio Tagua Date: Wed, 29 Sep 2010 14:35:24 -0300 Subject: We can't assign @view_context_class here, define super() in test instead if we want to avoid warnings. --- actionpack/lib/abstract_controller/rendering.rb | 2 +- actionpack/test/controller/filters_test.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 918def3e81..5d9b35d297 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -66,7 +66,7 @@ module AbstractController attr_writer :view_context_class def view_context_class - @view_context_class ||= self.class.view_context_class + @view_context_class || self.class.view_context_class end def initialize(*) diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index d13ebc705a..dfc90943e1 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -314,6 +314,7 @@ class FilterTest < ActionController::TestCase def initialize @@execution_log = "" + super() end before_filter { |c| c.class.execution_log << " before procfilter " } -- cgit v1.2.3 From 1c0be7baac8e736e2ddd659ac2d6ef1845f756c7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 11:34:51 -0700 Subject: fixing space error --- actionpack/test/controller/log_subscriber_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index b5bc0e9e9a..10873708fe 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -23,7 +23,7 @@ module Another def with_fragment_cache render :inline => "<%= cache('foo'){ 'bar' } %>" end - + def with_fragment_cache_and_percent_in_key render :inline => "<%= cache('foo%bar'){ 'Contains % sign in key' } %>" end -- cgit v1.2.3 From a5f8f5908116081a3461409eda53e62d997d5b40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 11:47:27 -0700 Subject: dry up action_methods --- actionpack/lib/abstract_controller/base.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 85270d84d8..ce88803a9c 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -61,13 +61,13 @@ module AbstractController def action_methods @action_methods ||= begin # All public instance methods of this class, including ancestors - methods = public_instance_methods(true).map { |m| m.to_s }.to_set - + methods = (public_instance_methods(true) - # Except for public instance methods of Base and its ancestors - internal_methods.map { |m| m.to_s } + + internal_methods + # Be sure to include shadowed public instance methods of this class - public_instance_methods(false).map { |m| m.to_s } - + public_instance_methods(false)).map { |x| x.to_s } - # And always exclude explicitly hidden actions - hidden_actions + hidden_actions.to_a # Clear out AS callback method pollution methods.reject { |method| method =~ /_one_time_conditions/ } -- cgit v1.2.3 From 1d9a219307ec5cc506635ee0bd3d368955d5b4a6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 11:53:34 -0700 Subject: oops, missed a uniq --- actionpack/lib/abstract_controller/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index ce88803a9c..f9f6eb945e 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -65,7 +65,7 @@ module AbstractController # Except for public instance methods of Base and its ancestors internal_methods + # Be sure to include shadowed public instance methods of this class - public_instance_methods(false)).map { |x| x.to_s } - + public_instance_methods(false)).uniq.map { |x| x.to_s } - # And always exclude explicitly hidden actions hidden_actions.to_a -- cgit v1.2.3 From 2437356cdacf2b774147b4da38de1d137e0a8b26 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 15:37:38 -0700 Subject: removing lollerject --- actionpack/lib/action_controller/metal/http_authentication.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 6a7e170306..53444f31a8 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -214,7 +214,7 @@ module ActionController def encode_credentials(http_method, credentials, password, password_is_ha1) credentials[:response] = expected_response(http_method, credentials[:uri], credentials, password, password_is_ha1) - "Digest " + credentials.sort_by {|x| x[0].to_s }.inject([]) {|a, v| a << "#{v[0]}='#{v[1]}'" }.join(', ') + "Digest " + credentials.sort_by {|x| x[0].to_s }.map {|v| "#{v[0]}='#{v[1]}'" }.join(', ') end def decode_credentials_header(request) -- cgit v1.2.3 From 3f88f26d1e17277dfa85a22bb01c1db558b6addf Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 15:41:06 -0700 Subject: removing more lolinject --- .../lib/action_controller/metal/http_authentication.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 53444f31a8..e0c5eaca84 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -423,14 +423,13 @@ module ActionController # Returns nil if no token is found. def token_and_options(request) if header = request.authorization.to_s[/^Token (.*)/] - values = $1.split(','). - inject({}) do |memo, value| - value.strip! # remove any spaces between commas and values - key, value = value.split(/\=\"?/) # split key=value pairs - value.chomp!('"') # chomp trailing " in value - value.gsub!(/\\\"/, '"') # unescape remaining quotes - memo.update(key => value) - end + values = Hash[$1.split(',').map do |value| + value.strip! # remove any spaces between commas and values + key, value = value.split(/\=\"?/) # split key=value pairs + value.chomp!('"') # chomp trailing " in value + value.gsub!(/\\\"/, '"') # unescape remaining quotes + [key, value] + end] [values.delete("token"), values.with_indifferent_access] end end -- cgit v1.2.3 From ab0d216b670e13d6f65e82dfdeb3d08c75101274 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 15:43:27 -0700 Subject: reduce function calls on Array --- actionpack/lib/action_controller/metal/http_authentication.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index e0c5eaca84..547cec7081 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -441,9 +441,8 @@ module ActionController # # Returns String. def encode_credentials(token, options = {}) - values = ["token=#{token.to_s.inspect}"] - options.each do |key, value| - values << "#{key}=#{value.to_s.inspect}" + values = ["token=#{token.to_s.inspect}"] + options.map do |key, value| + "#{key}=#{value.to_s.inspect}" end "Token #{values * ", "}" end -- cgit v1.2.3 From 78ac9c2be738ff48c847a26ae8fc4464e881e184 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 16:09:58 -0700 Subject: dry up method checking in the request object --- actionpack/lib/action_dispatch/http/request.rb | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 7a28228817..09d6ba8223 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -54,11 +54,7 @@ module ActionDispatch # the application should use), this \method returns the overridden # value, not the original. def request_method - @request_method ||= begin - method = env["REQUEST_METHOD"] - HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") - method - end + @request_method ||= check_method(env["REQUEST_METHOD"]) end # Returns a symbol form of the #request_method @@ -70,11 +66,7 @@ module ActionDispatch # even if it was overridden by middleware. See #request_method for # more information. def method - @method ||= begin - method = env["rack.methodoverride.original_method"] || env['REQUEST_METHOD'] - HTTP_METHOD_LOOKUP[method] || raise(ActionController::UnknownHttpMethod, "#{method}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") - method - end + @method ||= check_method(env["rack.methodoverride.original_method"] || env['REQUEST_METHOD']) end # Returns a symbol form of the #method @@ -246,5 +238,12 @@ module ActionDispatch def local? LOCALHOST.any? { |local_ip| local_ip === remote_addr && local_ip === remote_ip } end + + private + + def check_method(name) + HTTP_METHOD_LOOKUP[name] || raise(ActionController::UnknownHttpMethod, "#{name}, accepted HTTP methods are #{HTTP_METHODS.to_sentence(:locale => :en)}") + name + end end end -- cgit v1.2.3 From 2eef53b163353898a0f96d559e3f80600eb6ba15 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 16:17:56 -0700 Subject: removing useless code --- actionpack/lib/action_view/helpers/form_options_helper.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 83434a9340..7d6aca0470 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -297,7 +297,6 @@ module ActionView def options_for_select(container, selected = nil) return container if String === container - container = container.to_a if Hash === container selected, disabled = extract_selected_and_disabled(selected).map do | r | Array.wrap(r).map(&:to_s) end -- cgit v1.2.3 From a40e3c1a9604ab3737ad2465c8f6a6db0fe0cc78 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 16:40:47 -0700 Subject: removing crazy finalizer code until there is proof that we need it --- actionpack/lib/action_view/template.rb | 9 --------- 1 file changed, 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index a999a0b7d7..405e1736ba 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -101,14 +101,6 @@ module ActionView attr_reader :source, :identifier, :handler, :virtual_path, :formats, :original_encoding - Finalizer = proc do |method_name, mod| - proc do - mod.module_eval do - remove_possible_method method_name - end - end - end - def initialize(source, identifier, handler, details) @source = source @identifier = identifier @@ -253,7 +245,6 @@ module ActionView begin mod.module_eval(source, identifier, 0) - ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) method_name rescue Exception => e # errors from template code -- cgit v1.2.3 From 8efdffeda31b520b9b534dc614c6039404288c26 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 30 Sep 2010 02:22:00 +0800 Subject: no need of nil check --- actionpack/lib/action_controller/test_case.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 70a5de7f30..1af75fc2d7 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -394,7 +394,7 @@ module ActionController parameters ||= {} @request.assign_parameters(@routes, @controller.class.name.underscore.sub(/_controller$/, ''), action.to_s, parameters) - @request.session = ActionController::TestSession.new(session) unless session.nil? + @request.session = ActionController::TestSession.new(session) if session @request.session["flash"] = @request.flash.update(flash || {}) @request.session["flash"].sweep -- cgit v1.2.3 From 692f5184c405b3b0f9b6ac02c37aaefb7d2ffb62 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 30 Sep 2010 02:24:47 +0800 Subject: no need to check for nil? --- actionpack/lib/action_dispatch/middleware/session/abstract_store.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index ea49b30630..db0187c015 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -165,7 +165,7 @@ module ActionDispatch return response unless value cookie = { :value => value } - unless options[:expire_after].nil? + if options[:expire_after] cookie[:expires] = Time.now + options.delete(:expire_after) end -- cgit v1.2.3 From 08a08d97dd1307ab9544cc55fbd398c6acdc4c89 Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 30 Sep 2010 02:26:22 +0800 Subject: no need to check for nil? --- actionpack/lib/action_view/helpers/form_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index c47fac05ef..cabe272cc7 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -1015,7 +1015,7 @@ module ActionView module ClassMethods def value(object, method_name) - object.send method_name unless object.nil? + object.send method_name if object end def value_before_type_cast(object, method_name) @@ -1055,7 +1055,7 @@ module ActionView private def add_default_name_and_id_for_value(tag_value, options) - unless tag_value.nil? + if tag_value pretty_tag_value = tag_value.to_s.gsub(/\s/, "_").gsub(/[^-\w]/, "").downcase specified_id = options["id"] add_default_name_and_id(options) -- cgit v1.2.3 From 618407db56b39a4130e05779339534ae2ebf883e Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 30 Sep 2010 02:27:13 +0800 Subject: another case of extra nil? check --- actionpack/lib/action_view/helpers/text_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 3bc5afc2c4..94348cf9fa 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -365,7 +365,7 @@ module ActionView # <% end %> def current_cycle(name = "default") cycle = get_cycle(name) - cycle.current_value unless cycle.nil? + cycle.current_value if cycle end # Resets a cycle so that it starts from the first element the next time -- cgit v1.2.3 From 29c32e832908e13f0e360bbe47c80d5578bffba1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 17:57:39 -0700 Subject: tag value can be false, so nil? check is necessary --- actionpack/lib/action_view/helpers/form_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index cabe272cc7..1836baaf12 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -1055,7 +1055,7 @@ module ActionView private def add_default_name_and_id_for_value(tag_value, options) - if tag_value + unless tag_value.nil? pretty_tag_value = tag_value.to_s.gsub(/\s/, "_").gsub(/[^-\w]/, "").downcase specified_id = options["id"] add_default_name_and_id(options) -- cgit v1.2.3 From 7e057d11aa21383394e570d0de8f4d5f3729d024 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 18:23:33 -0700 Subject: fixing regexp warnings --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 189da138d8..0c6e1b37a7 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -349,7 +349,7 @@ module ActionDispatch options = args.last.is_a?(Hash) ? args.pop : {} path = args.shift || block - path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%{\w*}/)) ? path : (path % params) } + path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } status = options[:status] || 301 lambda do |env| -- cgit v1.2.3 From 31752f3516e5977b459cc713ae50515b20fda67b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 29 Sep 2010 18:32:51 -0700 Subject: avoid creating a block if possible --- actionpack/lib/action_dispatch/routing/mapper.rb | 4 ++-- 1 file changed, 2 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 0c6e1b37a7..0a888505d2 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -345,10 +345,10 @@ module ActionDispatch # Redirect any path to another path: # # match "/stories" => redirect("/posts") - def redirect(*args, &block) + def redirect(*args) options = args.last.is_a?(Hash) ? args.pop : {} - path = args.shift || block + path = args.shift || Proc.new path_proc = path.is_a?(Proc) ? path : proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) } status = options[:status] || 301 -- cgit v1.2.3 From 69f97f469747777ed1c457715f5361f6b8a0ab7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 30 Sep 2010 07:25:06 +0200 Subject: Use .find here as it is simpler and faster. --- actionpack/lib/action_dispatch/routing/mapper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 0a888505d2..47aed0273c 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -1130,7 +1130,7 @@ module ActionDispatch end candidate = name.select(&:present?).join("_").presence - candidate unless as.nil? && @set.routes.map(&:name).include?(candidate) + candidate unless as.nil? && @set.routes.find { |r| r.name == candidate } end end -- cgit v1.2.3 From 22b11a41cc764bc0f7b0c0f518a5289230428597 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Sat, 25 Sep 2010 19:22:32 +0200 Subject: Allow mounting engines at '/' Without that commit script_name always become '/', which results in paths like //posts/1 instead of /posts/1 --- .../lib/action_dispatch/routing/route_set.rb | 2 +- actionpack/test/dispatch/prefix_generation_test.rb | 93 ++++++++++++++++++---- 2 files changed, 78 insertions(+), 17 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5d18dfe369..99a3019f3a 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -511,7 +511,7 @@ module ActionDispatch end script_name = options.delete(:script_name) - path = (script_name.blank? ? _generate_prefix(options) : script_name).to_s + path = (script_name.blank? ? _generate_prefix(options) : script_name.chomp('/')).to_s 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 26d76557dd..18f28deee4 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -1,8 +1,23 @@ require 'abstract_unit' +require 'rack/test' module TestGenerationPrefix + class Post + extend ActiveModel::Naming + + def to_param + "1" + end + + def self.model_name + klass = "Post" + def klass.name; self end + + ActiveModel::Name.new(klass) + end + end + class WithMountedEngine < ActionDispatch::IntegrationTest - require 'rack/test' include Rack::Test::Methods class BlogEngine @@ -55,21 +70,6 @@ module TestGenerationPrefix # force draw RailsApplication.routes - class Post - extend ActiveModel::Naming - - def to_param - "1" - end - - def self.model_name - klass = "Post" - def klass.name; self end - - ActiveModel::Name.new(klass) - end - end - class ::InsideEngineGeneratingController < ActionController::Base include BlogEngine.routes.url_helpers include RailsApplication.routes.mounted_helpers @@ -253,4 +253,65 @@ module TestGenerationPrefix assert_equal "http://www.example.com/awesome/blog/posts/1", path end end + + class EngineMountedAtRoot < ActionDispatch::IntegrationTest + include Rack::Test::Methods + + class BlogEngine + def self.routes + @routes ||= begin + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + match "/posts/:id", :to => "posts#show", :as => :post + end + + routes + end + end + + def self.call(env) + env['action_dispatch.routes'] = routes + routes.call(env) + end + end + + class RailsApplication + def self.routes + @routes ||= begin + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + mount BlogEngine => "/" + end + + routes + end + end + + def self.call(env) + env['action_dispatch.routes'] = routes + routes.call(env) + end + end + + # force draw + RailsApplication.routes + + class ::PostsController < ActionController::Base + include BlogEngine.routes.url_helpers + include RailsApplication.routes.mounted_helpers + + def show + render :text => post_path(:id => params[:id]) + end + end + + def app + RailsApplication + end + + test "generating path inside engine" do + get "/posts/1" + assert_equal "/posts/1", last_response.body + end + end end -- cgit v1.2.3 From ec5d846ac6137e60d81257041e4fde82c0480b32 Mon Sep 17 00:00:00 2001 From: Piotr Sarnacki Date: Sun, 26 Sep 2010 00:17:06 +0200 Subject: Properly reload routes defined in class definition Sometimes it's easier to define routes inside Engine or Application class definition (e.g. one file applications). The problem with such case is that if there is a plugin that has config/routes.rb file, it will trigger routes reload on application. Since routes definition for application is not in config/routes.rb file routes_reloader will fail to reload application's routes properly. With this commit you can pass routes definition as a block to routes method, which will allow to properly reload it: class MyApp::Application < Rails::Application routes do resources :users end end --- actionpack/lib/action_dispatch/routing/route_set.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 99a3019f3a..32f41934f1 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,6 +1,7 @@ require 'rack/mount' require 'forwardable' require 'active_support/core_ext/object/to_query' +require 'active_support/core_ext/hash/slice' module ActionDispatch module Routing -- cgit v1.2.3 From 6a55ca346e543e4e112648cca2a01230c32b21ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 30 Sep 2010 11:40:51 +0200 Subject: Revert "removing crazy finalizer code until there is proof that we need it" This reverts commit a40e3c1a9604ab3737ad2465c8f6a6db0fe0cc78. --- actionpack/lib/action_view/template.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 405e1736ba..164d177dcc 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -101,6 +101,16 @@ module ActionView attr_reader :source, :identifier, :handler, :virtual_path, :formats, :original_encoding + # This finalizer is needed (and exactly with a proc inside another proc) + # otherwise templates leak in development. + Finalizer = proc do |method_name, mod| + proc do + mod.module_eval do + remove_possible_method method_name + end + end + end + def initialize(source, identifier, handler, details) @source = source @identifier = identifier @@ -245,6 +255,7 @@ module ActionView begin mod.module_eval(source, identifier, 0) + ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) method_name rescue Exception => e # errors from template code -- cgit v1.2.3 From ff2fdcc52b391514cb62c2a1ef29827ac94914c6 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:22:42 -0700 Subject: removing AS::Testing::Default in favor of just undefing default_test --- actionpack/lib/action_dispatch/testing/performance_test.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/testing/performance_test.rb b/actionpack/lib/action_dispatch/testing/performance_test.rb index 33a5c68b9d..d6c98b4db7 100644 --- a/actionpack/lib/action_dispatch/testing/performance_test.rb +++ b/actionpack/lib/action_dispatch/testing/performance_test.rb @@ -11,9 +11,8 @@ begin # formats are written, so you'll have two output files per test method. class PerformanceTest < ActionDispatch::IntegrationTest include ActiveSupport::Testing::Performance - include ActiveSupport::Testing::Default end end rescue NameError $stderr.puts "Specify ruby-prof as application's dependency in Gemfile to run benchmarks." -end \ No newline at end of file +end -- cgit v1.2.3 From dfa331ae154c0475dfc631528071bdb06947acc2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 17:54:50 -0700 Subject: use a method that actually exists --- actionpack/test/controller/integration_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index 4ff39fb76c..f0d62b0b13 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -167,7 +167,7 @@ end class IntegrationTestTest < Test::Unit::TestCase def setup - @test = ::ActionDispatch::IntegrationTest.new(:default_test) + @test = ::ActionDispatch::IntegrationTest.new(:app) @test.class.stubs(:fixture_table_names).returns([]) @session = @test.open_session end -- cgit v1.2.3 From 44f85678e967f1eccfaf448f82ca81111c9584af Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:01:34 -0700 Subject: delete repeated code --- actionpack/test/abstract_unit.rb | 18 ++++++++++++++++++ actionpack/test/controller/redirect_test.rb | 18 ------------------ actionpack/test/template/url_helper_test.rb | 18 ------------------ 3 files changed, 18 insertions(+), 36 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 3540af13ac..bf6d43da08 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -308,3 +308,21 @@ module ActionView end end end + +class Workshop + extend ActiveModel::Naming + include ActiveModel::Conversion + attr_accessor :id + + def initialize(id) + @id = id + end + + def persisted? + id.present? + end + + def to_s + id.to_s + end +end diff --git a/actionpack/test/controller/redirect_test.rb b/actionpack/test/controller/redirect_test.rb index b00142c92d..92d4a6d98b 100644 --- a/actionpack/test/controller/redirect_test.rb +++ b/actionpack/test/controller/redirect_test.rb @@ -3,24 +3,6 @@ require 'abstract_unit' class WorkshopsController < ActionController::Base end -class Workshop - extend ActiveModel::Naming - include ActiveModel::Conversion - attr_accessor :id - - def initialize(id) - @id = id - end - - def persisted? - id.present? - end - - def to_s - id.to_s - end -end - class RedirectController < ActionController::Base def simple_redirect redirect_to :action => "hello_world" diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index db8fd82aeb..98276da559 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -547,24 +547,6 @@ class LinkToUnlessCurrentWithControllerTest < ActionController::TestCase end end -class Workshop - extend ActiveModel::Naming - include ActiveModel::Conversion - attr_accessor :id - - def initialize(id) - @id = id - end - - def persisted? - id.present? - end - - def to_s - id.to_s - end -end - class Session extend ActiveModel::Naming include ActiveModel::Conversion -- cgit v1.2.3 From ffbcb84c215bb615a3db4bb8bf8dcb977e72e32b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:05:59 -0700 Subject: removing more duplicate code --- actionpack/test/abstract_unit.rb | 17 +++++++++++++++++ actionpack/test/controller/rescue_test.rb | 16 ---------------- actionpack/test/dispatch/show_exceptions_test.rb | 14 -------------- 3 files changed, 17 insertions(+), 30 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index bf6d43da08..470b36dbe2 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -326,3 +326,20 @@ class Workshop id.to_s end end + +module ActionDispatch + class ShowExceptions + private + remove_method :public_path + def public_path + "#{FIXTURE_LOAD_PATH}/public" + end + + remove_method :logger + # Silence logger + def logger + nil + end + end +end + diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb index a2418bb7c0..c445285538 100644 --- a/actionpack/test/controller/rescue_test.rb +++ b/actionpack/test/controller/rescue_test.rb @@ -1,21 +1,5 @@ require 'abstract_unit' -module ActionDispatch - class ShowExceptions - private - remove_method :public_path - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end - - remove_method :logger - # Silence logger - def logger - nil - end - end -end - class RescueController < ActionController::Base class NotAuthorized < StandardError end diff --git a/actionpack/test/dispatch/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index 4ede1ab47c..ce6c397e32 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -1,19 +1,5 @@ require 'abstract_unit' -module ActionDispatch - class ShowExceptions - private - def public_path - "#{FIXTURE_LOAD_PATH}/public" - end - - # Silence logger - def logger - nil - end - end -end - class ShowExceptionsTest < ActionDispatch::IntegrationTest Boomer = lambda do |env| req = ActionDispatch::Request.new(env) -- cgit v1.2.3 From 50cf5c11a1b33cd082bbdf4f253581109955797c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:07:19 -0700 Subject: fixing warnings with regexps on assert_match --- actionpack/test/controller/log_subscriber_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 10873708fe..90c944d890 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -151,8 +151,8 @@ class ACLogSubscriberTest < ActionController::TestCase wait assert_equal 4, logs.size - assert_match /Exist fragment\? views\/foo%bar/, logs[1] - assert_match /Write fragment views\/foo%bar/, logs[2] + assert_match(/Exist fragment\? views\/foo%bar/, logs[1]) + assert_match(/Write fragment views\/foo%bar/, logs[2]) ensure @controller.config.perform_caching = true end -- cgit v1.2.3 From 3eb7f9adee4f606ac65e8e3d25098411b5a787b7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Oct 2010 18:09:37 -0700 Subject: removing more duplicate code. :'( --- actionpack/test/controller/record_identifier_test.rb | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/record_identifier_test.rb b/actionpack/test/controller/record_identifier_test.rb index 835a0e970b..e46e317e31 100644 --- a/actionpack/test/controller/record_identifier_test.rb +++ b/actionpack/test/controller/record_identifier_test.rb @@ -1,17 +1,5 @@ require 'abstract_unit' - -class Comment - extend ActiveModel::Naming - include ActiveModel::Conversion - - attr_reader :id - def to_key; id ? [id] : nil end - def save; @id = 1 end - def new_record?; @id.nil? end - def name - @id.nil? ? 'new comment' : "comment ##{@id}" - end -end +require 'controller/fake_models' class Sheep extend ActiveModel::Naming -- cgit v1.2.3 From b95152201871f076a0d5c95e9e6387f68feab94e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 2 Oct 2010 08:36:55 +0200 Subject: Revert "Perf: refactor _assign method to avoid inject and defining unneeded local var." _assigns must return a hash. This reverts commit e66c1cee86aba1c81152f3d0872313e65cec85a9. --- actionpack/lib/action_view/test_case.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 915c2f90d7..0eb4a663de 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -192,7 +192,11 @@ module ActionView end def _assigns - _instance_variables.map { |var| [var[1..-1].to_sym, instance_variable_get(var)] } + _instance_variables.inject({}) do |hash, var| + name = var[1..-1].to_sym + hash[name] = instance_variable_get(var) + hash + end end def _routes -- cgit v1.2.3 From 8d1df887d32643f0aee2d71f7216ec98fdfe4fdb Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Wed, 29 Sep 2010 18:15:36 +0530 Subject: Fixing search_field to remove object attribute from options hash [#5730 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_view/helpers/form_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 1836baaf12..3cd8b02bc4 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -791,7 +791,7 @@ module ActionView options["incremental"] = true unless options.has_key?("incremental") end - InstanceTag.new(object_name, method, self, options.delete(:object)).to_input_field_tag("search", options) + InstanceTag.new(object_name, method, self, options.delete("object")).to_input_field_tag("search", options) end # Returns a text_field of type "tel". -- cgit v1.2.3 From 297cf0b26658ff9c7f19fc703de5bd8939e31d06 Mon Sep 17 00:00:00 2001 From: Aditya Sanghi Date: Sat, 2 Oct 2010 17:39:12 +0530 Subject: added test for form_for with search_field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/test/template/form_helper_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'actionpack') diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index abc98ebe69..8809e510fb 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -761,6 +761,20 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + + def test_form_for_with_search_field + # Test case for bug which would emit an "object" attribute + # when used with form_for using a search_field form helper + form_for(Post.new, :url => "/search", :html => { :id => 'search-post' }) do |f| + concat f.search_field(:title) + end + + expected = whole_form("/search", "search-post", "new_post") do + "" + end + + assert_dom_equal expected, output_buffer + end def test_form_for_with_remote form_for(@post, :url => '/', :remote => true, :html => { :id => 'create-post', :method => :put }) do |f| @@ -1737,4 +1751,5 @@ class FormHelperTest < ActionView::TestCase def protect_against_forgery? false end + end -- cgit v1.2.3 From f656796d05715174568536cfe119a3959a020f23 Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sat, 2 Oct 2010 12:35:17 -0500 Subject: Rename _assigns to view_assigns in AV::TC - also add tests - also deprecate _assigns [#5751 state:resolved] Signed-off-by: Santiago Pastorino --- actionpack/lib/action_view/test_case.rb | 30 ++++++++++++++++++++--------- actionpack/test/template/test_case_test.rb | 31 ++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 0eb4a663de..ac59c16d7c 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -103,7 +103,7 @@ module ActionView end def render(options = {}, local_assigns = {}, &block) - view.assign(_assigns) + view.assign(view_assigns) @rendered << output = view.render(options, local_assigns, &block) output end @@ -169,15 +169,19 @@ module ActionView alias_method :_view, :view - EXCLUDE_IVARS = %w{ + INTERNAL_IVARS = %w{ + @__name__ @_assertion_wrapped + @_assertions @_result + @_routes @controller @layouts @locals @method_name @output_buffer @partials + @passed @rendered @request @routes @@ -187,18 +191,26 @@ module ActionView @view_context_class } - def _instance_variables - instance_variables.map(&:to_s) - EXCLUDE_IVARS + def _user_defined_ivars + instance_variables.map(&:to_s) - INTERNAL_IVARS end - def _assigns - _instance_variables.inject({}) do |hash, var| - name = var[1..-1].to_sym - hash[name] = instance_variable_get(var) - hash + # Returns a Hash of instance variables and their values, as defined by + # the user in the test case, which are then assigned to the view being + # rendered. This is generally intended for internal use and extension + # frameworks. + def view_assigns + _user_defined_ivars.inject({}) do |hash, var| + hash.merge(var.sub('@','').to_sym => instance_variable_get(var)) end end + def _assigns + ActiveSupport::Deprecation.warn "ActionView::TestCase#_assigns is deprecated and will be removed in future versions. " << + "Please use view_assigns instead." + view_assigns + end + def _routes @controller._routes if @controller.respond_to?(:_routes) end diff --git a/actionpack/test/template/test_case_test.rb b/actionpack/test/template/test_case_test.rb index 8526db61cc..a745999622 100644 --- a/actionpack/test/template/test_case_test.rb +++ b/actionpack/test/template/test_case_test.rb @@ -116,6 +116,37 @@ module ActionView end end + class AssignsTest < ActionView::TestCase + setup do + ActiveSupport::Deprecation.stubs(:warn) + end + + test "_assigns delegates to user_defined_ivars" do + self.expects(:view_assigns) + _assigns + end + + test "_assigns is deprecated" do + ActiveSupport::Deprecation.expects(:warn) + _assigns + end + end + + class ViewAssignsTest < ActionView::TestCase + test "view_assigns returns a Hash of user defined ivars" do + @a = 'b' + @c = 'd' + assert_equal({:a => 'b', :c => 'd'}, view_assigns) + end + + test "view_assigns excludes internal ivars" do + INTERNAL_IVARS.each do |ivar| + assert defined?(ivar), "expected #{ivar} to be defined" + assert !view_assigns.keys.include?(ivar.sub('@','').to_sym), "expected #{ivar} to be excluded from view_assigns" + end + end + end + class HelperExposureTest < ActionView::TestCase helper(Module.new do def render_from_helper -- cgit v1.2.3 From c28bebef13b8a0e497fc7bbb83f542e9400e07e5 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 3 Oct 2010 13:34:34 -0200 Subject: PERF: Hash[] + map is faster than this silly inject, and var[1..-1] is faster than var.sub('@', '') --- actionpack/lib/action_view/test_case.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index ac59c16d7c..731f91df30 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -200,9 +200,9 @@ module ActionView # rendered. This is generally intended for internal use and extension # frameworks. def view_assigns - _user_defined_ivars.inject({}) do |hash, var| - hash.merge(var.sub('@','').to_sym => instance_variable_get(var)) - end + Hash[_user_defined_ivars.map do |var| + [var[1..-1].to_sym, instance_variable_get(var)] + end] end def _assigns -- cgit v1.2.3 From 50215f9525b6b5e3bfe703724b9f68177ed8565d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 20 Sep 2010 10:18:44 +0200 Subject: Rely on Rack::Session stores API for more compatibility across the Ruby world. --- actionpack/CHANGELOG | 6 +- actionpack/lib/action_controller/test_case.rb | 8 +- actionpack/lib/action_dispatch/http/url.rb | 5 - .../middleware/session/abstract_store.rb | 280 ++++----------------- .../middleware/session/cookie_store.rb | 64 ++--- .../middleware/session/mem_cache_store.rb | 53 +--- .../test/dispatch/session/cookie_store_test.rb | 12 - 7 files changed, 84 insertions(+), 344 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6f8314109b..6352b97a6b 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,12 +1,12 @@ *Rails 3.1.0 (unreleased)* +* Rely on Rack::Session stores API for more compatibility across the Ruby world. This is backwards incompatible since Rack::Session expects #get_session to accept 4 arguments and requires #destroy_session instead of simply #destroy. [José Valim] + * file_field automatically adds :multipart => true to the enclosing form. [Santiago Pastorino] * Renames csrf_meta_tag -> csrf_meta_tags, and aliases csrf_meta_tag for backwards compatibility. [fxn] -* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used - for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply - to the browser only. +* Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used for HTTP caching. Note that Rack::Cache will be used if you use #expires_in, #fresh_when or #stale with :public => true. Otherwise, the caching rules will apply to the browser only. [Yehuda Katz, Carl Lerche] *Rails 3.0.0 (August 29, 2010)* diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 1af75fc2d7..d7b54c2abc 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -187,15 +187,17 @@ module ActionController end end - class TestSession < ActionDispatch::Session::AbstractStore::SessionHash #:nodoc: - DEFAULT_OPTIONS = ActionDispatch::Session::AbstractStore::DEFAULT_OPTIONS + class TestSession < Rack::Session::Abstract::SessionHash #:nodoc: + DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) replace(session.stringify_keys) @loaded = true end - def exists?; true; end + def exists? + true + end end # Superclass for ActionController functional tests. Functional tests allow you to diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3e5cd6a2f9..cfee95eb4b 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -18,11 +18,6 @@ module ActionDispatch @protocol ||= ssl? ? 'https://' : 'http://' end - # Is this an SSL request? - def ssl? - @ssl ||= @env['HTTPS'] == 'on' || @env['HTTP_X_FORWARDED_PROTO'] == 'https' - end - # Returns the \host for this request, such as "example.com". def raw_host_with_port if forwarded = env["HTTP_X_FORWARDED_HOST"] diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index db0187c015..679ba7fc8e 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -1,5 +1,6 @@ require 'rack/utils' require 'rack/request' +require 'rack/session/abstract/id' require 'action_dispatch/middleware/cookies' require 'active_support/core_ext/object/blank' @@ -8,252 +9,69 @@ module ActionDispatch class SessionRestoreError < StandardError #:nodoc: end - class AbstractStore - ENV_SESSION_KEY = 'rack.session'.freeze - ENV_SESSION_OPTIONS_KEY = 'rack.session.options'.freeze - - # thin wrapper around Hash that allows us to lazily - # load session id into session_options - class OptionsHash < Hash - def initialize(by, env, default_options) - @by = by - @env = env - @session_id_loaded = false - merge!(default_options) - end - - def [](key) - if key == :id - load_session_id! unless key?(:id) || has_session_id? - end - super - end - - private - - def has_session_id? - @session_id_loaded - end - - def load_session_id! - self[:id] = @by.send(:extract_session_id, @env) - @session_id_loaded = true - end - end - - class SessionHash < Hash - def initialize(by, env) - super() - @by = by - @env = env - @loaded = false - end - - def [](key) - load_for_read! - super(key.to_s) - end - - def has_key?(key) - load_for_read! - super(key.to_s) - end - - def []=(key, value) - load_for_write! - super(key.to_s, value) - end - - def clear - load_for_write! - super - end - - def to_hash - load_for_read! - h = {}.replace(self) - h.delete_if { |k,v| v.nil? } - h - end - - def update(hash) - load_for_write! - super(hash.stringify_keys) - end - - def delete(key) - load_for_write! - super(key.to_s) - end - - def inspect - load_for_read! - super - end - - def exists? - return @exists if instance_variable_defined?(:@exists) - @exists = @by.send(:exists?, @env) - end - - def loaded? - @loaded - end - - def destroy - clear - @by.send(:destroy, @env) if defined?(@by) && @by - @env[ENV_SESSION_OPTIONS_KEY][:id] = nil if defined?(@env) && @env && @env[ENV_SESSION_OPTIONS_KEY] - @loaded = false - end - - private - - def load_for_read! - load! if !loaded? && exists? - end - - def load_for_write! - load! unless loaded? - end - - def load! - id, session = @by.send(:load_session, @env) - @env[ENV_SESSION_OPTIONS_KEY][:id] = id - replace(session.stringify_keys) - @loaded = true - end - + module DestroyableSession + def destroy + clear + options = @env[Rack::Session::Abstract::ENV_SESSION_OPTIONS_KEY] if @env + options ||= {} + @by.send(:destroy_session, @env, options[:id], options) if @by + options[:id] = nil + @loaded = false end + end - DEFAULT_OPTIONS = { - :key => '_session_id', - :path => '/', - :domain => nil, - :expire_after => nil, - :secure => false, - :httponly => true, - :cookie_only => true - } + ::Rack::Session::Abstract::SessionHash.send :include, DestroyableSession + module Compatibility def initialize(app, options = {}) - @app = app - @default_options = DEFAULT_OPTIONS.merge(options) - @key = @default_options.delete(:key).freeze - @cookie_only = @default_options.delete(:cookie_only) - ensure_session_key! + options[:key] ||= '_session_id' + super end - def call(env) - prepare!(env) - response = @app.call(env) - - session_data = env[ENV_SESSION_KEY] - options = env[ENV_SESSION_OPTIONS_KEY] - - if !session_data.is_a?(AbstractStore::SessionHash) || session_data.loaded? || options[:expire_after] - request = ActionDispatch::Request.new(env) - - return response if (options[:secure] && !request.ssl?) - - session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.loaded? - - sid = options[:id] || generate_sid - session_data = session_data.to_hash - - value = set_session(env, sid, session_data) - return response unless value - - cookie = { :value => value } - if options[:expire_after] - cookie[:expires] = Time.now + options.delete(:expire_after) - end - - set_cookie(request, cookie.merge!(options)) - end - - response + def generate_sid + ActiveSupport::SecureRandom.hex(16) end + end - private - - def prepare!(env) - env[ENV_SESSION_KEY] = SessionHash.new(self, env) - env[ENV_SESSION_OPTIONS_KEY] = OptionsHash.new(self, env, @default_options) - end - - def generate_sid - ActiveSupport::SecureRandom.hex(16) - end - - def set_cookie(request, options) - if request.cookie_jar[@key] != options[:value] || !options[:expires].nil? - request.cookie_jar[@key] = options - end - end - - def load_session(env) - stale_session_check! do - sid = current_session_id(env) - sid, session = get_session(env, sid) - [sid, session] - end - end - - def extract_session_id(env) - stale_session_check! do - request = ActionDispatch::Request.new(env) - sid = request.cookies[@key] - sid ||= request.params[@key] unless @cookie_only - sid - end - end - - def current_session_id(env) - env[ENV_SESSION_OPTIONS_KEY][:id] - end + module StaleSessionCheck + def load_session(env) + stale_session_check! { super } + end - def ensure_session_key! - if @key.blank? - raise ArgumentError, 'A key is required to write a ' + - 'cookie containing the session data. Use ' + - 'config.session_store SESSION_STORE, { :key => ' + - '"_myapp_session" } in config/application.rb' - end - end + def extract_session_id(env) + stale_session_check! { super } + end - def stale_session_check! - yield - rescue ArgumentError => argument_error - if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} - begin - # Note that the regexp does not allow $1 to end with a ':' - $1.constantize - rescue LoadError, NameError => const_error - raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" - end - retry - else - raise + def stale_session_check! + yield + rescue ArgumentError => argument_error + if argument_error.message =~ %r{undefined class/module ([\w:]*\w)} + begin + # Note that the regexp does not allow $1 to end with a ':' + $1.constantize + rescue LoadError, NameError => const_error + raise ActionDispatch::Session::SessionRestoreError, "Session contains objects whose class definition isn't available.\nRemember to require the classes for all objects kept in the session.\n(Original exception: #{const_error.message} [#{const_error.class}])\n" end + retry + else + raise end + end + end - def exists?(env) - current_session_id(env).present? - end - - def get_session(env, sid) - raise '#get_session needs to be implemented.' - end + class AbstractStore < Rack::Session::Abstract::ID + include Compatibility + include StaleSessionCheck - def set_session(env, sid, session_data) - raise '#set_session needs to be implemented and should return ' << - 'the value to be stored in the cookie (usually the sid)' - end + def destroy_session(env, sid, options) + ActiveSupport::Deprecation.warn "Implementing #destroy in session stores is deprecated. " << + "Please implement destroy_session(env, session_id, options) instead." + destroy(env) + end - def destroy(env) - raise '#destroy needs to be implemented.' - end + def destroy(env) + raise '#destroy needs to be implemented.' + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb index ca1494425f..9c9ccc62f5 100644 --- a/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/cookie_store.rb @@ -1,5 +1,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/object/blank' +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/cookie' module ActionDispatch module Session @@ -38,58 +40,32 @@ module ActionDispatch # "rake secret" and set the key in config/initializers/secret_token.rb. # # Note that changing digest or secret invalidates all existing sessions! - class CookieStore < AbstractStore - - def initialize(app, options = {}) - super(app, options.merge!(:cookie_only => true)) - freeze - end + class CookieStore < Rack::Session::Cookie + include Compatibility + include StaleSessionCheck private - def load_session(env) - data = unpacked_cookie_data(env) - data = persistent_session_id!(data) - [data["session_id"], data] - end - - def extract_session_id(env) - if data = unpacked_cookie_data(env) - data["session_id"] - else - nil - end - end - - def unpacked_cookie_data(env) - env["action_dispatch.request.unsigned_session_cookie"] ||= begin - stale_session_check! do - request = ActionDispatch::Request.new(env) - if data = request.cookie_jar.signed[@key] - data.stringify_keys! - end - data || {} + def unpacked_cookie_data(env) + env["action_dispatch.request.unsigned_session_cookie"] ||= begin + stale_session_check! do + request = ActionDispatch::Request.new(env) + if data = request.cookie_jar.signed[@key] + data.stringify_keys! end + data || {} end end + end - def set_cookie(request, options) - request.cookie_jar.signed[@key] = options - end - - def set_session(env, sid, session_data) - persistent_session_id!(session_data, sid) - end - - def destroy(env) - # session data is stored on client; nothing to do here - end + def set_session(env, sid, session_data, options) + persistent_session_id!(session_data, sid) + end - def persistent_session_id!(data, sid=nil) - data ||= {} - data["session_id"] ||= sid || generate_sid - data - end + def set_cookie(env, session_id, cookie) + request = ActionDispatch::Request.new(env) + request.cookie_jar.signed[@key] = cookie + end end end end diff --git a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb index 28e3dbd732..4dd9a946c2 100644 --- a/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/mem_cache_store.rb @@ -1,56 +1,17 @@ +require 'action_dispatch/middleware/session/abstract_store' +require 'rack/session/memcache' + module ActionDispatch module Session - class MemCacheStore < AbstractStore + class MemCacheStore < Rack::Session::Memcache + include Compatibility + include StaleSessionCheck + def initialize(app, options = {}) require 'memcache' - - # Support old :expires option options[:expire_after] ||= options[:expires] - - super - - @default_options = { - :namespace => 'rack:session', - :memcache_server => 'localhost:11211' - }.merge(@default_options) - - @pool = options[:cache] || MemCache.new(@default_options[:memcache_server], @default_options) - unless @pool.servers.any? { |s| s.alive? } - raise "#{self} unable to find server during initialization." - end - @mutex = Mutex.new - super end - - private - def get_session(env, sid) - sid ||= generate_sid - begin - session = @pool.get(sid) || {} - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - session = {} - end - [sid, session] - end - - def set_session(env, sid, session_data) - options = env['rack.session.options'] - expiry = options[:expire_after] || 0 - @pool.set(sid, session_data, expiry) - sid - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - - def destroy(env) - if sid = current_session_id(env) - @pool.delete(sid) - end - rescue MemCache::MemCacheError, Errno::ECONNREFUSED - false - end - end end end diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 3489f628ed..256d0781c7 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -53,18 +53,6 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def rescue_action(e) raise end end - def test_raises_argument_error_if_missing_session_key - assert_raise(ArgumentError, nil.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => nil, :secret => SessionSecret) - } - - assert_raise(ArgumentError, ''.inspect) { - ActionDispatch::Session::CookieStore.new(nil, - :key => '', :secret => SessionSecret) - } - end - def test_setting_session_value with_test_route_set do get '/set_session_value' -- cgit v1.2.3 From 74dd8a3681c6984ea35c879f88c6a87521b58ec2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 22 Sep 2010 21:40:14 +0200 Subject: Move ETag and ConditionalGet logic from AD::Response to the middleware stack. --- actionpack/lib/action_dispatch/http/cache.rb | 22 +---- actionpack/lib/action_dispatch/http/response.rb | 2 +- actionpack/test/controller/new_base/etag_test.rb | 46 --------- actionpack/test/controller/render_test.rb | 118 ----------------------- actionpack/test/dispatch/response_test.rb | 11 +-- 5 files changed, 5 insertions(+), 194 deletions(-) delete mode 100644 actionpack/test/controller/new_base/etag_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb index 047fab006e..4061222d11 100644 --- a/actionpack/lib/action_dispatch/http/cache.rb +++ b/actionpack/lib/action_dispatch/http/cache.rb @@ -50,8 +50,7 @@ module ActionDispatch if cache_control = self["Cache-Control"] cache_control.split(/,\s*/).each do |segment| first, last = segment.split("=") - last ||= true - @cache_control[first.to_sym] = last + @cache_control[first.to_sym] = last || true end end end @@ -88,28 +87,9 @@ module ActionDispatch def handle_conditional_get! if etag? || last_modified? || !@cache_control.empty? set_conditional_cache_control! - elsif nonempty_ok_response? - self.etag = body - - if request && request.etag_matches?(etag) - self.status = 304 - self.body = [] - end - - set_conditional_cache_control! - else - headers["Cache-Control"] = "no-cache" end end - def nonempty_ok_response? - @status == 200 && string_body? - end - - def string_body? - !@blank && @body.respond_to?(:all?) && @body.all? { |part| part.is_a?(String) } - end - DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate" def set_conditional_cache_control! diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 151c90167b..72871e328a 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -132,7 +132,7 @@ module ActionDispatch # :nodoc: # information. attr_accessor :charset, :content_type - CONTENT_TYPE = "Content-Type" + CONTENT_TYPE = "Content-Type" cattr_accessor(:default_charset) { "utf-8" } diff --git a/actionpack/test/controller/new_base/etag_test.rb b/actionpack/test/controller/new_base/etag_test.rb deleted file mode 100644 index 2bca5aec6a..0000000000 --- a/actionpack/test/controller/new_base/etag_test.rb +++ /dev/null @@ -1,46 +0,0 @@ -require 'abstract_unit' - -module Etags - class BasicController < ActionController::Base - self.view_paths = [ActionView::FixtureResolver.new( - "etags/basic/base.html.erb" => "Hello from without_layout.html.erb", - "layouts/etags.html.erb" => "teh <%= yield %> tagz" - )] - - def without_layout - render :action => "base" - end - - def with_layout - render :action => "base", :layout => "etags" - end - end - - class EtagTest < Rack::TestCase - describe "Rendering without any special etag options returns an etag that is an MD5 hash of its text" - - test "an action without a layout" do - get "/etags/basic/without_layout" - - body = "Hello from without_layout.html.erb" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - test "an action with a layout" do - get "/etags/basic/with_layout" - - body = "teh Hello from without_layout.html.erb tagz" - assert_body body - assert_header "Etag", etag_for(body) - assert_status 200 - end - - private - - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - end -end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 7ca784c467..fca8de60bc 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -99,11 +99,6 @@ class TestController < ActionController::Base render :template => "test/hello_world" end - def render_hello_world_with_etag_set - response.etag = "hello_world" - render :template => "test/hello_world" - end - # :ported: compatibility def render_hello_world_with_forward_slash render :template => "/test/hello_world" @@ -1386,119 +1381,6 @@ class ExpiresInRenderTest < ActionController::TestCase end end - -class EtagRenderTest < ActionController::TestCase - tests TestController - - def setup - super - @request.host = "www.nextangle.com" - @expected_bang_etag = etag_for(expand_key([:foo, 123])) - end - - def test_render_blank_body_shouldnt_set_etag - get :blank_response - assert !@response.etag? - end - - def test_render_200_should_set_etag - get :render_hello_world_from_variable - assert_equal etag_for("hello david"), @response.headers['ETag'] - assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control'] - end - - def test_render_against_etag_request_should_304_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - assert @response.body.empty? - end - - def test_render_against_etag_request_should_have_no_content_length_when_match - @request.if_none_match = etag_for("hello david") - get :render_hello_world_from_variable - assert !@response.headers.has_key?("Content-Length") - end - - def test_render_against_etag_request_should_200_when_no_match - @request.if_none_match = etag_for("hello somewhere else") - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - assert !@response.body.empty? - end - - def test_render_should_not_set_etag_when_last_modified_has_been_specified - get :render_hello_world_with_last_modified_set - assert_equal 200, @response.status.to_i - assert_not_nil @response.last_modified - assert_nil @response.etag - assert_present @response.body - end - - def test_render_with_etag - get :render_hello_world_from_variable - expected_etag = etag_for('hello david') - assert_equal expected_etag, @response.headers['ETag'] - @response = ActionController::TestResponse.new - - @request.if_none_match = expected_etag - get :render_hello_world_from_variable - assert_equal 304, @response.status.to_i - - @response = ActionController::TestResponse.new - @request.if_none_match = "\"diftag\"" - get :render_hello_world_from_variable - assert_equal 200, @response.status.to_i - end - - def render_with_404_shouldnt_have_etag - get :render_custom_code - assert_nil @response.headers['ETag'] - end - - def test_etag_should_not_be_changed_when_already_set - get :render_hello_world_with_etag_set - assert_equal etag_for("hello_world"), @response.headers['ETag'] - end - - def test_etag_should_govern_renders_with_layouts_too - get :builder_layout_test - assert_equal "\n\n

Hello

\n

This is grand!

\n\n\n", @response.body - assert_equal etag_for("\n\n

Hello

\n

This is grand!

\n\n\n"), @response.headers['ETag'] - end - - def test_etag_with_bang_should_set_etag - get :conditional_hello_with_bangs - assert_equal @expected_bang_etag, @response.headers["ETag"] - assert_response :success - end - - def test_etag_with_bang_should_obey_if_none_match - @request.if_none_match = @expected_bang_etag - get :conditional_hello_with_bangs - assert_response :not_modified - end - - def test_etag_with_public_true_should_set_header - get :conditional_hello_with_public_header - assert_equal "public", @response.headers['Cache-Control'] - end - - def test_etag_with_public_true_should_set_header_and_retain_other_headers - get :conditional_hello_with_public_header_and_expires_at - assert_equal "max-age=60, public", @response.headers['Cache-Control'] - end - - protected - def etag_for(text) - %("#{Digest::MD5.hexdigest(text)}") - end - - def expand_key(args) - ActiveSupport::Cache.expand_cache_key(args) - end -end - class LastModifiedRenderTest < ActionController::TestCase tests TestController diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index cd0418c338..be6398fead 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -11,9 +11,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"' + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] @@ -27,9 +25,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "max-age=0, private, must-revalidate", - "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"' + "Content-Type" => "text/html; charset=utf-8" }, headers) end @@ -41,8 +37,7 @@ class ResponseTest < ActiveSupport::TestCase status, headers, body = @response.to_a assert_equal 200, status assert_equal({ - "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "no-cache" + "Content-Type" => "text/html; charset=utf-8" }, headers) parts = [] -- cgit v1.2.3 From 653acac069e66f53b791caa4838a1e25de905f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 3 Oct 2010 21:45:27 +0200 Subject: Solve some warnings and a failing test. --- actionpack/lib/action_controller/test_case.rb | 1 + actionpack/lib/action_dispatch/http/request.rb | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index d7b54c2abc..6061945622 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -191,6 +191,7 @@ module ActionController DEFAULT_OPTIONS = Rack::Session::Abstract::ID::DEFAULT_OPTIONS def initialize(session = {}) + @env, @by = nil, nil replace(session.stringify_keys) @loaded = true end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 09d6ba8223..bbcdefb190 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -199,7 +199,7 @@ module ActionDispatch # TODO This should be broken apart into AD::Request::Session and probably # be included by the session middleware. def reset_session - session.destroy if session + session.destroy if session && session.respond_to?(:destroy) self.session = {} @env['action_dispatch.request.flash_hash'] = nil end -- cgit v1.2.3 From 18a7b767e8ad545702c1025fc9cc7a1cc3c64f28 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 14:11:34 -0700 Subject: moving fake model to the correct file --- actionpack/test/controller/record_identifier_test.rb | 13 ------------- actionpack/test/lib/controller/fake_models.rb | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/record_identifier_test.rb b/actionpack/test/controller/record_identifier_test.rb index e46e317e31..f3e5ff8a47 100644 --- a/actionpack/test/controller/record_identifier_test.rb +++ b/actionpack/test/controller/record_identifier_test.rb @@ -1,19 +1,6 @@ require 'abstract_unit' require 'controller/fake_models' -class Sheep - extend ActiveModel::Naming - include ActiveModel::Conversion - - attr_reader :id - def to_key; id ? [id] : nil end - def save; @id = 1 end - def new_record?; @id.nil? end - def name - @id.nil? ? 'new sheep' : "sheep ##{@id}" - end -end - class RecordIdentifierTest < Test::Unit::TestCase include ActionController::RecordIdentifier diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index 8cb3b4940a..cf3f2f7fa4 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -130,6 +130,20 @@ class CommentRelevance end end +class Sheep + extend ActiveModel::Naming + include ActiveModel::Conversion + + attr_reader :id + def to_key; id ? [id] : nil end + def save; @id = 1 end + def new_record?; @id.nil? end + def name + @id.nil? ? 'new sheep' : "sheep ##{@id}" + end +end + + class TagRelevance extend ActiveModel::Naming include ActiveModel::Conversion -- cgit v1.2.3 From a6c42c82678f11271fe71923a200eb135f2f1c0e Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Sun, 3 Oct 2010 15:38:17 -0700 Subject: two argument String#slice is faster than single argument, also avoid creating a Range object --- actionpack/lib/action_view/test_case.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 731f91df30..4026f7a40e 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -201,7 +201,7 @@ module ActionView # frameworks. def view_assigns Hash[_user_defined_ivars.map do |var| - [var[1..-1].to_sym, instance_variable_get(var)] + [var[1, var.length].to_sym, instance_variable_get(var)] end] end -- cgit v1.2.3 From 3986fcb935c8d5b89ecb86b2f1cbb463460182de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 08:47:36 +0200 Subject: Initialize sid should just skip instance variables. --- .../lib/action_dispatch/middleware/session/abstract_store.rb | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb index 679ba7fc8e..64d3a87fd0 100644 --- a/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb +++ b/actionpack/lib/action_dispatch/middleware/session/abstract_store.rb @@ -31,6 +31,13 @@ module ActionDispatch def generate_sid ActiveSupport::SecureRandom.hex(16) end + + protected + + def initialize_sid + @default_options.delete(:sidbits) + @default_options.delete(:secure_random) + end end module StaleSessionCheck -- cgit v1.2.3 From 0b51f3cc73ac21ed56b45a15fcce1d31beb7170c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Mon, 4 Oct 2010 18:06:04 +0200 Subject: Ensure the proper content type is returned for static files. --- .../lib/action_dispatch/middleware/static.rb | 6 +-- actionpack/test/dispatch/static_test.rb | 47 +++++++++++++--------- 2 files changed, 30 insertions(+), 23 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index cf13938331..913b899e20 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -6,13 +6,13 @@ module ActionDispatch @at, @root = at.chomp('/'), root.chomp('/') @compiled_at = (Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank?) @compiled_root = Regexp.compile(/^#{Regexp.escape(root)}/) - @file_server = ::Rack::File.new(root) + @file_server = ::Rack::File.new(@root) end def match?(path) path = path.dup - if @compiled_at.blank? || path.sub!(@compiled_at, '') - full_path = File.join(@root, ::Rack::Utils.unescape(path)) + if !@compiled_at || path.sub!(@compiled_at, '') + full_path = path.empty? ? @root : File.join(@root, ::Rack::Utils.unescape(path)) paths = "#{full_path}#{ext}" matches = Dir[paths] diff --git a/actionpack/test/dispatch/static_test.rb b/actionpack/test/dispatch/static_test.rb index 2eb82fc5d8..655745a848 100644 --- a/actionpack/test/dispatch/static_test.rb +++ b/actionpack/test/dispatch/static_test.rb @@ -2,30 +2,37 @@ require 'abstract_unit' module StaticTests def test_serves_dynamic_content - assert_equal "Hello, World!", get("/nofile") + assert_equal "Hello, World!", get("/nofile").body end def test_serves_static_index_at_root - assert_equal "/index.html", get("/index.html") - assert_equal "/index.html", get("/index") - assert_equal "/index.html", get("/") + assert_html "/index.html", get("/index.html") + assert_html "/index.html", get("/index") + assert_html "/index.html", get("/") + assert_html "/index.html", get("") end def test_serves_static_file_in_directory - assert_equal "/foo/bar.html", get("/foo/bar.html") - assert_equal "/foo/bar.html", get("/foo/bar/") - assert_equal "/foo/bar.html", get("/foo/bar") + assert_html "/foo/bar.html", get("/foo/bar.html") + assert_html "/foo/bar.html", get("/foo/bar/") + assert_html "/foo/bar.html", get("/foo/bar") end def test_serves_static_index_file_in_directory - assert_equal "/foo/index.html", get("/foo/index.html") - assert_equal "/foo/index.html", get("/foo/") - assert_equal "/foo/index.html", get("/foo") + assert_html "/foo/index.html", get("/foo/index.html") + assert_html "/foo/index.html", get("/foo/") + assert_html "/foo/index.html", get("/foo") end private + + def assert_html(body, response) + assert_equal body, response.body + assert_equal "text/html", response.headers["Content-Type"] + end + def get(path) - Rack::MockRequest.new(@app).request("GET", path).body + Rack::MockRequest.new(@app).request("GET", path) end end @@ -59,16 +66,16 @@ class MultipleDirectorisStaticTest < ActiveSupport::TestCase include StaticTests test "serves files from other mounted directories" do - assert_equal "/blog/index.html", get("/blog/index.html") - assert_equal "/blog/index.html", get("/blog/index") - assert_equal "/blog/index.html", get("/blog/") + assert_html "/blog/index.html", get("/blog/index.html") + assert_html "/blog/index.html", get("/blog/index") + assert_html "/blog/index.html", get("/blog/") - assert_equal "/blog/blog.html", get("/blog/blog/") - assert_equal "/blog/blog.html", get("/blog/blog.html") - assert_equal "/blog/blog.html", get("/blog/blog") + assert_html "/blog/blog.html", get("/blog/blog/") + assert_html "/blog/blog.html", get("/blog/blog.html") + assert_html "/blog/blog.html", get("/blog/blog") - assert_equal "/blog/subdir/index.html", get("/blog/subdir/index.html") - assert_equal "/blog/subdir/index.html", get("/blog/subdir/") - assert_equal "/blog/subdir/index.html", get("/blog/subdir") + assert_html "/blog/subdir/index.html", get("/blog/subdir/index.html") + assert_html "/blog/subdir/index.html", get("/blog/subdir/") + assert_html "/blog/subdir/index.html", get("/blog/subdir") end end -- cgit v1.2.3 From 6644675831a5a87c77f66697124d95ab37202509 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Fri, 24 Sep 2010 14:06:47 -0700 Subject: Template::Error is also used if rendering fails. --- actionpack/lib/action_view/template/error.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index 423e1e0bf5..ff256738a9 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -43,8 +43,9 @@ module ActionView end class Template - # The Template::Error exception is raised when the compilation of the template fails. This exception then gathers a - # bunch of intimate details and uses it to report a very precise exception message. + # The Template::Error exception is raised when the compilation or rendering of the template + # fails. This exception then gathers a bunch of intimate details and uses it to report a + # precise exception message. class Error < ActionViewError #:nodoc: SOURCE_CODE_RADIUS = 3 -- cgit v1.2.3 From b1047888fb4ddf9ba2cbdd4cd596ad4f1aca8645 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 4 Oct 2010 11:14:42 -0700 Subject: `render :text => proc { ... }` is no longer supported. --- actionpack/lib/action_controller/metal/streaming.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/streaming.rb b/actionpack/lib/action_controller/metal/streaming.rb index 2013da3adb..312dc8eb3e 100644 --- a/actionpack/lib/action_controller/metal/streaming.rb +++ b/actionpack/lib/action_controller/metal/streaming.rb @@ -101,10 +101,6 @@ module ActionController #:nodoc: # send_data image.data, :type => image.content_type, :disposition => 'inline' # # See +send_file+ for more information on HTTP Content-* headers and caching. - # - # Tip: if you want to stream large amounts of on-the-fly generated - # data to the browser, then use render :text => proc { ... } - # instead. See ActionController::Base#render for more information. def send_data(data, options = {}) #:doc: send_file_headers! options.dup render options.slice(:status, :content_type).merge(:text => data) -- cgit v1.2.3 From 28bb1885f5a35d0adecd35d38b73751d737891c4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:08:01 -0700 Subject: avoid method call to compact --- actionpack/lib/action_dispatch/routing/mapper.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 47aed0273c..bf10f81127 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -171,13 +171,13 @@ module ActionDispatch end def blocks + block = @scope[:blocks] || [] + if @options[:constraints].present? && !@options[:constraints].is_a?(Hash) - block = @options[:constraints] - else - block = nil + block << @options[:constraints] end - ((@scope[:blocks] || []) + [ block ]).compact + block end def constraints -- cgit v1.2.3 From f9734f2b0f5326c399d1c1cccba8b6d8e7d9bdd4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:30:16 -0700 Subject: adding tests for uploaded file --- actionpack/test/dispatch/uploaded_file_test.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 actionpack/test/dispatch/uploaded_file_test.rb (limited to 'actionpack') diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb new file mode 100644 index 0000000000..def6289fb3 --- /dev/null +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -0,0 +1,25 @@ +require 'abstract_unit' + +module ActionDispatch + class UploadedFileTest < ActiveSupport::TestCase + def test_original_filename + uf = Http::UploadedFile.new(:filename => 'foo') + assert_equal 'foo', uf.original_filename + end + + def test_content_type + uf = Http::UploadedFile.new(:type => 'foo') + assert_equal 'foo', uf.content_type + end + + def test_headers + uf = Http::UploadedFile.new(:head => 'foo') + assert_equal 'foo', uf.headers + end + + def test_tempfile + uf = Http::UploadedFile.new(:tempfile => 'foo') + assert_equal 'foo', uf.tempfile + end + end +end -- cgit v1.2.3 From 2a3022db7f2ddc0fc0e678ea757f97902c5f5c01 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:56:45 -0700 Subject: delegate to the @tempfile instance variable --- actionpack/lib/action_dispatch/http/upload.rb | 18 +++++------------- actionpack/test/dispatch/uploaded_file_test.rb | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 13 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 49465d820e..bfbe7c5305 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -2,27 +2,19 @@ require 'active_support/core_ext/object/blank' module ActionDispatch module Http - class UploadedFile < Tempfile + class UploadedFile attr_accessor :original_filename, :content_type, :tempfile, :headers def initialize(hash) @original_filename = hash[:filename] @content_type = hash[:type] @headers = hash[:head] - - # To the untrained eye, this may appear as insanity. Given the alternatives, - # such as busting the method cache on every request or breaking backwards - # compatibility with is_a?(Tempfile), this solution is the best available - # option. - # - # TODO: Deprecate is_a?(Tempfile) and define a real API for this parameter - tempfile = hash[:tempfile] - tempfile.instance_variables.each do |ivar| - instance_variable_set(ivar, tempfile.instance_variable_get(ivar)) - end + @tempfile = hash[:tempfile] end - alias local_path path + def method_missing(name, *args, &block) + @tempfile.send(name, *args, &block) + end end module Upload diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index def6289fb3..d04d5a8650 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -21,5 +21,23 @@ module ActionDispatch uf = Http::UploadedFile.new(:tempfile => 'foo') assert_equal 'foo', uf.tempfile end + + def test_delegates_to_tempfile + tf = Class.new { def tenderlove; 'thunderhorse' end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal 'thunderhorse', uf.tenderlove + end + + def test_delegates_to_tempfile_with_params + tf = Class.new { def tenderlove *args; args end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal %w{ thunder horse }, uf.tenderlove(*%w{ thunder horse }) + end + + def test_delegates_to_tempfile_with_block + tf = Class.new { def tenderlove; yield end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) + end end end -- cgit v1.2.3 From 876acf001a32887679e259f83a2fbad750ac2e67 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 16:57:11 -0700 Subject: if it walks like a duck and talks like a duck, it must be a duck --- actionpack/test/dispatch/request/multipart_params_parsing_test.rb | 7 ------- 1 file changed, 7 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb index 073dd3ddad..3ff558ec5a 100644 --- a/actionpack/test/dispatch/request/multipart_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/multipart_params_parsing_test.rb @@ -36,7 +36,6 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of Tempfile, file assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type assert_equal 'contents', file.read @@ -49,8 +48,6 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest file = params['file'] foo = params['foo'] - assert_kind_of Tempfile, file - assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type @@ -64,8 +61,6 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest file = params['file'] - assert_kind_of Tempfile, file - assert_equal 'file.txt', file.original_filename assert_equal "text/plain", file.content_type assert_equal(('a' * 20480), file.read) @@ -77,13 +72,11 @@ class MultipartParamsParsingTest < ActionDispatch::IntegrationTest assert_equal 'bar', params['foo'] file = params['file'] - assert_kind_of Tempfile, file assert_equal 'file.csv', file.original_filename assert_nil file.content_type assert_equal 'contents', file.read file = params['flowers'] - assert_kind_of Tempfile, file assert_equal 'flowers.jpg', file.original_filename assert_equal "image/jpeg", file.content_type assert_equal 19512, file.size -- cgit v1.2.3 From 8a9747021085c569f0118db1093bc12cfa2ba915 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:08:25 -0700 Subject: raising an argument error if tempfile is not provided --- actionpack/lib/action_dispatch/http/upload.rb | 1 + actionpack/test/dispatch/uploaded_file_test.rb | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index bfbe7c5305..d4fabe1eaf 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -10,6 +10,7 @@ module ActionDispatch @content_type = hash[:type] @headers = hash[:head] @tempfile = hash[:tempfile] + raise(ArgumentError, ':tempfile is required') unless @tempfile end def method_missing(name, *args, &block) diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index d04d5a8650..4173ce0a44 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -2,18 +2,24 @@ require 'abstract_unit' module ActionDispatch class UploadedFileTest < ActiveSupport::TestCase + def test_constructor_with_argument_error + assert_raises(ArgumentError) do + Http::UploadedFile.new({}) + end + end + def test_original_filename - uf = Http::UploadedFile.new(:filename => 'foo') + uf = Http::UploadedFile.new(:filename => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.original_filename end def test_content_type - uf = Http::UploadedFile.new(:type => 'foo') + uf = Http::UploadedFile.new(:type => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.content_type end def test_headers - uf = Http::UploadedFile.new(:head => 'foo') + uf = Http::UploadedFile.new(:head => 'foo', :tempfile => Object.new) assert_equal 'foo', uf.headers end -- cgit v1.2.3 From 3370ad0b1e883c9ec24c771f6c52b296a71eff40 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 17:11:50 -0700 Subject: making sure respond_to? works properly --- actionpack/lib/action_dispatch/http/upload.rb | 5 +++++ actionpack/test/dispatch/uploaded_file_test.rb | 15 +++++++++++++++ 2 files changed, 20 insertions(+) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index d4fabe1eaf..53f8039121 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,7 +13,12 @@ module ActionDispatch raise(ArgumentError, ':tempfile is required') unless @tempfile end + def respond_to?(name) + super || @tempfile.respond_to?(name) + end + def method_missing(name, *args, &block) + return super unless respond_to?(name) @tempfile.send(name, *args, &block) end end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index 4173ce0a44..c81797a73f 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -45,5 +45,20 @@ module ActionDispatch uf = Http::UploadedFile.new(:tempfile => tf.new) assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) end + + def test_delegate_respects_respond_to? + tf = Class.new { def tenderlove; yield end; private :tenderlove } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert_raises(NoMethodError) do + uf.tenderlove + end + end + + def test_respond_to? + tf = Class.new { def tenderlove; yield end } + uf = Http::UploadedFile.new(:tempfile => tf.new) + assert uf.respond_to?(:headers), 'responds to headers' + assert uf.respond_to?(:tenderlove), 'responds to tenderlove' + end end end -- cgit v1.2.3 From 12173396163616e077f761e190c13beb43d536bd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:28:40 -0700 Subject: only forwarding enough methods to work. People should grab the delegate tempfile if they really need to do hard work --- actionpack/lib/action_dispatch/http/upload.rb | 13 ++++++++----- actionpack/test/dispatch/uploaded_file_test.rb | 22 ++++++++-------------- 2 files changed, 16 insertions(+), 19 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 53f8039121..84e58d7d6a 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -13,13 +13,16 @@ module ActionDispatch raise(ArgumentError, ':tempfile is required') unless @tempfile end - def respond_to?(name) - super || @tempfile.respond_to?(name) + def read(*args) + @tempfile.read(*args) end - def method_missing(name, *args, &block) - return super unless respond_to?(name) - @tempfile.send(name, *args, &block) + def rewind + @tempfile.rewind + end + + def size + @tempfile.size end end diff --git a/actionpack/test/dispatch/uploaded_file_test.rb b/actionpack/test/dispatch/uploaded_file_test.rb index c81797a73f..b51697b930 100644 --- a/actionpack/test/dispatch/uploaded_file_test.rb +++ b/actionpack/test/dispatch/uploaded_file_test.rb @@ -29,36 +29,30 @@ module ActionDispatch end def test_delegates_to_tempfile - tf = Class.new { def tenderlove; 'thunderhorse' end } + tf = Class.new { def read; 'thunderhorse' end } uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal 'thunderhorse', uf.tenderlove + assert_equal 'thunderhorse', uf.read end def test_delegates_to_tempfile_with_params - tf = Class.new { def tenderlove *args; args end } + tf = Class.new { def read *args; args end } uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal %w{ thunder horse }, uf.tenderlove(*%w{ thunder horse }) - end - - def test_delegates_to_tempfile_with_block - tf = Class.new { def tenderlove; yield end } - uf = Http::UploadedFile.new(:tempfile => tf.new) - assert_equal('thunderhorse', uf.tenderlove { 'thunderhorse' }) + assert_equal %w{ thunder horse }, uf.read(*%w{ thunder horse }) end def test_delegate_respects_respond_to? - tf = Class.new { def tenderlove; yield end; private :tenderlove } + tf = Class.new { def read; yield end; private :read } uf = Http::UploadedFile.new(:tempfile => tf.new) assert_raises(NoMethodError) do - uf.tenderlove + uf.read end end def test_respond_to? - tf = Class.new { def tenderlove; yield end } + tf = Class.new { def read; yield end } uf = Http::UploadedFile.new(:tempfile => tf.new) assert uf.respond_to?(:headers), 'responds to headers' - assert uf.respond_to?(:tenderlove), 'responds to tenderlove' + assert uf.respond_to?(:read), 'responds to read' end end end -- cgit v1.2.3 From 5769636663c2849fd132fd593c0f6d29702b7cf1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:32:49 -0700 Subject: fixing a few test warnings --- actionpack/test/controller/filters_test.rb | 4 ++-- actionpack/test/lib/controller/fake_models.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index dfc90943e1..3a8a37d967 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -758,12 +758,12 @@ class ControllerWithSymbolAsFilter < PostsController def without_exception # Do stuff... - 1 + 1 + wtf = 1 + 1 yield # Do stuff... - 1 + 1 + wtf += 1 end end diff --git a/actionpack/test/lib/controller/fake_models.rb b/actionpack/test/lib/controller/fake_models.rb index cf3f2f7fa4..dba632e6df 100644 --- a/actionpack/test/lib/controller/fake_models.rb +++ b/actionpack/test/lib/controller/fake_models.rb @@ -55,6 +55,11 @@ class Post < Struct.new(:title, :author_name, :body, :secret, :written_on, :cost alias_method :secret?, :secret + def initialize(*args) + super + @persisted = false + end + def persisted=(boolean) @persisted = boolean end -- cgit v1.2.3 From 333a5659e8663049618386e3fa45248d388070fd Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:46:38 -0700 Subject: dry up some crazy codes --- actionpack/lib/action_view/helpers/url_helper.rb | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 1c3ca78d28..440e0162cd 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -476,18 +476,16 @@ module ActionView html_options = html_options.stringify_keys encode = html_options.delete("encode").to_s - cc, bcc, subject, body = html_options.delete("cc"), html_options.delete("bcc"), html_options.delete("subject"), html_options.delete("body") - extras = [] - extras << "cc=#{Rack::Utils.escape(cc).gsub("+", "%20")}" unless cc.nil? - extras << "bcc=#{Rack::Utils.escape(bcc).gsub("+", "%20")}" unless bcc.nil? - extras << "body=#{Rack::Utils.escape(body).gsub("+", "%20")}" unless body.nil? - extras << "subject=#{Rack::Utils.escape(subject).gsub("+", "%20")}" unless subject.nil? + extras = %w{ cc bcc body subject }.map { |item| + option = html_options.delete(item) || next + "#{item}=#{Rack::Utils.escape(option).gsub("+", "%20")}" + }.compact extras = extras.empty? ? '' : '?' + html_escape(extras.join('&')) email_address_obfuscated = email_address.dup - email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.has_key?("replace_at") - email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.has_key?("replace_dot") + email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.key?("replace_at") + email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.key?("replace_dot") string = '' -- cgit v1.2.3 From 714fea4540c96f70d044e2bd1be92b504d1f8fa3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 19:52:17 -0700 Subject: deleting more crazy --- actionpack/lib/action_view/helpers/url_helper.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 440e0162cd..0eed3ea259 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -495,13 +495,11 @@ module ActionView end "".html_safe elsif encode == "hex" - email_address_encoded = '' - email_address_obfuscated.each_byte do |c| - email_address_encoded << sprintf("&#%d;", c) - end + email_address_encoded = email_address_obfuscated.unpack('C*').map {|c| + sprintf("&#%d;", c) + }.join - protocol = 'mailto:' - protocol.each_byte { |c| string << sprintf("&#%d;", c) } + string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join email_address.each_byte do |c| char = c.chr -- cgit v1.2.3 From 839e2f96647d5b29cc2865555a6f615c31429109 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:00:55 -0700 Subject: cleaning up more crazy! --- actionpack/lib/action_view/helpers/url_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 0eed3ea259..651240a122 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -501,10 +501,10 @@ module ActionView string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join - email_address.each_byte do |c| + string += email_address.unpack('C*').map do |c| char = c.chr - string << (char =~ /\w/ ? sprintf("%%%x", c) : char) - end + char =~ /\w/ ? sprintf("%%%x", c) : char + end.join content_tag "a", name || email_address_encoded.html_safe, html_options.merge("href" => "#{string}#{extras}".html_safe) else content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe) -- cgit v1.2.3 From e3acdcfbf349e416c63f4c56170e9d82f7b1b1d0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 4 Oct 2010 20:05:56 -0700 Subject: refactoring to use fewer intermediate variables --- actionpack/lib/action_view/helpers/url_helper.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 651240a122..da42d94318 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -487,24 +487,25 @@ module ActionView email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.key?("replace_at") email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.key?("replace_dot") - string = '' - - if encode == "javascript" - "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe))}');".each_byte do |c| - string << sprintf("%%%x", c) - end + case encode + when "javascript" + string = + "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe))}');".unpack('C*').map { |c| + sprintf("%%%x", c) + }.join "".html_safe - elsif encode == "hex" + when "hex" email_address_encoded = email_address_obfuscated.unpack('C*').map {|c| sprintf("&#%d;", c) }.join - string += 'mailto:'.unpack('C*').map { |c| sprintf("&#%d;", c) }.join - - string += email_address.unpack('C*').map do |c| + string = 'mailto:'.unpack('C*').map { |c| + sprintf("&#%d;", c) + }.join + email_address.unpack('C*').map { |c| char = c.chr char =~ /\w/ ? sprintf("%%%x", c) : char - end.join + }.join + content_tag "a", name || email_address_encoded.html_safe, html_options.merge("href" => "#{string}#{extras}".html_safe) else content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe) -- cgit v1.2.3 From d649bf158be130515566aed987f83d36ac9b0ae8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 6 Oct 2010 17:18:59 +0200 Subject: Provide a cleaner syntax for paths configuration that does not rely on method_missing. --- actionpack/lib/action_controller/railtie.rb | 8 ++++---- actionpack/lib/action_controller/railties/paths.rb | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/railtie.rb b/actionpack/lib/action_controller/railtie.rb index 0ade42ba2d..c5a661f2b0 100644 --- a/actionpack/lib/action_controller/railtie.rb +++ b/actionpack/lib/action_controller/railtie.rb @@ -21,10 +21,10 @@ module ActionController paths = app.config.paths options = app.config.action_controller - options.assets_dir ||= paths.public.to_a.first - options.javascripts_dir ||= paths.public.javascripts.to_a.first - options.stylesheets_dir ||= paths.public.stylesheets.to_a.first - options.page_cache_directory ||= paths.public.to_a.first + options.assets_dir ||= paths["public"].first + options.javascripts_dir ||= paths["public/javascripts"].first + options.stylesheets_dir ||= paths["public/stylesheets"].first + options.page_cache_directory ||= paths["public"].first # make sure readers methods get compiled options.asset_path ||= nil diff --git a/actionpack/lib/action_controller/railties/paths.rb b/actionpack/lib/action_controller/railties/paths.rb index fa71d55946..7a59d4f2f3 100644 --- a/actionpack/lib/action_controller/railties/paths.rb +++ b/actionpack/lib/action_controller/railties/paths.rb @@ -5,12 +5,14 @@ module ActionController Module.new do define_method(:inherited) do |klass| super(klass) + if namespace = klass.parents.detect {|m| m.respond_to?(:_railtie) } - klass.helpers_path = namespace._railtie.config.paths.app.helpers.to_a + paths = namespace._railtie.paths["app/helpers"].existent else - klass.helpers_path = app.config.helpers_paths + paths = app.config.helpers_paths end + klass.helpers_path = paths klass.helper :all if klass.superclass == ActionController::Base end end -- cgit v1.2.3 From 243513f4d17e62186ef0499edece1588c79220b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 13:26:58 +0200 Subject: Get rid of ruby warnings in Resolvers. Move a few methods up to the abstract class. --- actionpack/lib/action_view/template/resolver.rb | 45 +++++++++++++------------ actionpack/lib/action_view/testing/resolvers.rb | 4 +-- 2 files changed, 26 insertions(+), 23 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a261e08dbc..a031d0dc33 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -6,7 +6,6 @@ module ActionView # = Action View Resolver class Resolver def initialize - @path = nil @cached = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| h2[k2] = Hash.new { |h3, k3| h3[k3] = {} } } } end @@ -35,6 +34,23 @@ module ActionView raise NotImplementedError end + # Helpers that builds a path. Useful for building virtual paths. + def build_path(name, prefix, partial, details) + path = "" + path << "#{prefix}/" unless prefix.empty? + path << (partial ? "_#{name}" : name) + path + end + + # Get the handler and format from the given parameters. + def retrieve_handler_and_format(handler, format, default_formats=nil) + handler = Template.handler_class_for_extension(handler) + format = format && Mime[format] + format ||= handler.default_format if handler.respond_to?(:default_format) + format ||= default_formats + [handler, format] + end + def cached(key, prefix, name, partial) return yield unless key && caching? @cached[key][prefix][name][partial] ||= yield @@ -44,25 +60,13 @@ module ActionView class PathResolver < Resolver EXTENSION_ORDER = [:locale, :formats, :handlers] - def to_s - @path.to_s - end - alias :to_path :to_s - - private + private def find_templates(name, prefix, partial, details) path = build_path(name, prefix, partial, details) query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats]) end - def build_path(name, prefix, partial, details) - path = "" - path << "#{prefix}/" unless prefix.empty? - path << (partial ? "_#{name}" : name) - path - end - def query(path, exts, formats) query = File.join(@path, path) @@ -86,13 +90,7 @@ module ActionView def extract_handler_and_format(path, default_formats) pieces = File.basename(path).split(".") pieces.shift - - handler = Template.handler_class_for_extension(pieces.pop) - format = pieces.last && Mime[pieces.last] && pieces.pop.to_sym - format ||= handler.default_format if handler.respond_to?(:default_format) - format ||= default_formats - - [handler, format] + retrieve_handler_and_format(pieces.pop, pieces.pop, default_formats) end end @@ -103,6 +101,11 @@ module ActionView @path = File.expand_path(path) end + def to_s + @path.to_s + end + alias :to_path :to_s + def eql?(resolver) self.class.equal?(resolver.class) && to_path == resolver.to_path end diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index b2b62528a9..6d82fa0971 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -13,7 +13,7 @@ module ActionView #:nodoc: @hash = hash end - private + private def query(path, exts, formats) query = Regexp.escape(path) @@ -32,7 +32,7 @@ module ActionView #:nodoc: end end - class NullResolver < ActionView::PathResolver + class NullResolver < PathResolver def query(path, exts, formats) handler, format = extract_handler_and_format(path, formats) [ActionView::Template.new("Template generated by Null Resolver", path, handler, :virtual_path => path, :format => format)] -- cgit v1.2.3 From b2600bfc181664fcfe448d100ca054017b0576dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 15:50:20 +0200 Subject: Remove locals dependency from template. This means that templates does not need to store its source anymore, allowing us to reduce the ammount of memory taken by our Rails processes. Naively speaking, if your app/views contains 2MB of files, each of your processes (after being hit by a bunch of requests) will take 2MB less of memory after this commit. This is extremely important for the upcoming features. Since Rails will also render CSS and JS files, their source won't be stored as well allowing us to decrease the ammount of memory taken. --- actionpack/lib/action_view/lookup_context.rb | 16 ++--- actionpack/lib/action_view/paths.rb | 4 +- actionpack/lib/action_view/render/layouts.rb | 4 +- actionpack/lib/action_view/render/partials.rb | 80 ++++++++++++++----------- actionpack/lib/action_view/render/rendering.rb | 10 ++-- actionpack/lib/action_view/template.rb | 66 +++++++++++++------- actionpack/lib/action_view/template/resolver.rb | 30 +++++++--- actionpack/lib/action_view/template/text.rb | 4 -- actionpack/test/template/template_test.rb | 1 + 9 files changed, 130 insertions(+), 85 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 3ea8b86af1..794fdeae64 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -77,17 +77,17 @@ module ActionView @view_paths = ActionView::Base.process_view_paths(paths) end - def find(name, prefix = nil, partial = false) - @view_paths.find(*args_for_lookup(name, prefix, partial)) + def find(name, prefix = nil, partial = false, keys = []) + @view_paths.find(*args_for_lookup(name, prefix, partial, keys)) end alias :find_template :find - def find_all(name, prefix = nil, partial = false) - @view_paths.find_all(*args_for_lookup(name, prefix, partial)) + def find_all(name, prefix = nil, partial = false, keys = []) + @view_paths.find_all(*args_for_lookup(name, prefix, partial, keys)) end - def exists?(name, prefix = nil, partial = false) - @view_paths.exists?(*args_for_lookup(name, prefix, partial)) + def exists?(name, prefix = nil, partial = false, keys = []) + @view_paths.exists?(*args_for_lookup(name, prefix, partial, keys)) end alias :template_exists? :exists? @@ -106,9 +106,9 @@ module ActionView protected - def args_for_lookup(name, prefix, partial) #:nodoc: + def args_for_lookup(name, prefix, partial, keys) #:nodoc: name, prefix = normalize_name(name, prefix) - [name, prefix, partial || false, @details, details_key] + [name, prefix, partial || false, @details, keys, details_key] end # Support legacy foo.erb names even though we now ignore .erb diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 9857d688d2..dc26d75ff3 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -10,8 +10,8 @@ module ActionView #:nodoc: METHOD end - def find(path, prefix = nil, partial = false, details = {}, key = nil) - template = find_all(path, prefix, partial, details, key).first + def find(path, prefix = nil, partial = false, details = {}, keys = [], key = nil) + template = find_all(path, prefix, partial, details, keys, key).first raise MissingTemplate.new(self, "#{prefix}/#{path}", details, partial) unless template template end diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb index 8882acca31..08162f7fd5 100644 --- a/actionpack/lib/action_view/render/layouts.rb +++ b/actionpack/lib/action_view/render/layouts.rb @@ -62,11 +62,11 @@ module ActionView # This is the method which actually finds the layout using details in the lookup # context object. If no layout is found, it checks if at least a layout with # the given name exists across all details before raising the error. - def find_layout(layout) + def find_layout(layout, keys) begin with_layout_format do layout =~ /^\// ? - with_fallbacks { find_template(layout) } : find_template(layout) + with_fallbacks { find_template(layout, nil, false, keys) } : find_template(layout, nil, false, keys) end rescue ActionView::MissingTemplate => e update_details(:formats => nil) do diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index cc9b444837..07e844afc2 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -218,7 +218,6 @@ module ActionView def initialize(view_context, options, block) @view = view_context @partial_names = PARTIAL_NAMES[@view.controller.class.name] - setup(options, block) end @@ -237,16 +236,28 @@ module ActionView @object = partial if @collection = collection - paths = @collection_paths = @collection.map { |o| partial_path(o) } + paths = @collection_data = @collection.map { |o| partial_path(o) } @path = paths.uniq.size == 1 ? paths.first : nil else @path = partial_path end end + + if @path + @variable, @variable_counter = retrieve_variable(@path) + else + paths.map! { |path| retrieve_variable(path).unshift(path) } + end + end + + def retrieve_variable(path) + variable = @options[:as] || path[%r'_?(\w+)(\.\w+)*$', 1].to_sym + variable_counter = :"#{variable}_counter" if @collection + [variable, variable_counter] end def render - identifier = ((@template = find_template) ? @template.identifier : @path) + identifier = ((@template = find_partial) ? @template.identifier : @path) if @collection ActiveSupport::Notifications.instrument("render_collection.action_view", @@ -254,15 +265,16 @@ module ActionView render_collection end else + if !@block && (layout = @options[:layout]) + layout = find_template(layout) + end + content = ActiveSupport::Notifications.instrument("render_partial.action_view", :identifier => identifier) do render_partial end - if !@block && (layout = @options[:layout]) - content = @view._render_layout(find_template(layout), @locals){ content } - end - + content = @view._render_layout(layout, @locals){ content } if layout content end end @@ -278,16 +290,9 @@ module ActionView result.join(spacer).html_safe end - def collection_with_template(template = @template) + def collection_with_template segments, locals, template = [], @locals, @template - - if @options[:as] - as = @options[:as] - counter = "#{as}_counter".to_sym - else - as = template.variable_name - counter = template.counter_name - end + as, counter = @variable, @variable_counter locals[counter] = -1 @@ -300,20 +305,16 @@ module ActionView segments end - def collection_without_template(collection_paths = @collection_paths) - segments, locals = [], @locals + def collection_without_template + segments, locals, collection_data = [], @locals, @collection_data index, template = -1, nil - - if @options[:as] - as = @options[:as] - counter = "#{as}_counter" - end + keys = @locals.keys @collection.each_with_index do |object, i| - template = find_template(collection_paths[i]) - locals[as || template.variable_name] = object - locals[counter || template.counter_name] = (index += 1) - + path, *data = collection_data[i] + template = find_template(path, keys + data) + locals[data[0]] = object + locals[data[1]] = (index += 1) segments << template.render(@view, locals) end @@ -321,13 +322,14 @@ module ActionView segments end - def render_partial(object = @object) - locals, view, template = @locals, @view, @template + def render_partial + locals, view = @locals, @view + object, as = @object, @variable - object ||= locals[template.variable_name] - locals[@options[:as] || template.variable_name] = object + object ||= locals[as] + locals[as] = object - template.render(view, locals) do |*name| + @template.render(view, locals) do |*name| view._layout_for(*name, &@block) end end @@ -342,10 +344,18 @@ module ActionView end end - def find_template(path=@path) - return path unless path.is_a?(String) + def find_partial + if path = @path + locals = @locals.keys + locals << @variable + locals << @variable_counter if @collection + find_template(path, locals) + end + end + + def find_template(path=@path, locals=@locals.keys) prefix = @view.controller_path unless path.include?(?/) - @view.find_template(path, prefix, true) + @view.find_template(path, prefix, true, locals) end def partial_path(object = @object) diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 5320245173..0771b40c37 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -34,16 +34,18 @@ module ActionView # Determine the template to be rendered using the given options. def _determine_template(options) #:nodoc: + keys = (options[:locals] ||= {}).keys + if options.key?(:inline) handler = Template.handler_class_for_extension(options[:type] || "erb") - Template.new(options[:inline], "inline template", handler, {}) + Template.new(options[:inline], "inline template", handler, { :locals => keys }) elsif options.key?(:text) Template::Text.new(options[:text], formats.try(:first)) elsif options.key?(:file) - with_fallbacks { find_template(options[:file], options[:prefix]) } + with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } elsif options.key?(:template) options[:template].respond_to?(:render) ? - options[:template] : find_template(options[:template], options[:prefix]) + options[:template] : find_template(options[:template], options[:prefix], false, keys) end end @@ -51,7 +53,7 @@ module ActionView # supplied as well. def _render_template(template, layout = nil, options = {}) #:nodoc: locals = options[:locals] || {} - layout = find_layout(layout) if layout + layout = find_layout(layout, locals.keys) if layout ActiveSupport::Notifications.instrument("render_template.action_view", :identifier => template.identifier, :layout => layout.try(:virtual_path)) do diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 164d177dcc..7e65654d0b 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -98,6 +98,8 @@ module ActionView extend Template::Handlers + attr_accessor :locals + attr_reader :source, :identifier, :handler, :virtual_path, :formats, :original_encoding @@ -117,23 +119,17 @@ module ActionView @handler = handler @original_encoding = nil @method_names = {} - - format = details[:format] || :html - @formats = Array.wrap(format).map(&:to_sym) - @virtual_path = details[:virtual_path].try(:sub, ".#{format}", "") + @locals = details[:locals] || [] + @formats = Array.wrap(details[:format] || :html).map(&:to_sym) + @virtual_path = details[:virtual_path].try(:sub, ".#{@formats.first}", "") + @compiled = false end def render(view, locals, &block) # Notice that we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do - if view.is_a?(ActionView::CompiledTemplates) - mod = ActionView::CompiledTemplates - else - mod = view.singleton_class - end - - method_name = compile(locals, view, mod) + compile!(view) view.send(method_name, locals, &block) end rescue Exception => e @@ -141,7 +137,7 @@ module ActionView e.sub_template_of(self) raise e else - raise Template::Error.new(self, view.respond_to?(:assigns) ? view.assigns : {}, e) + raise Template::Error.new(refresh(view) || self, view.respond_to?(:assigns) ? view.assigns : {}, e) end end @@ -149,10 +145,12 @@ module ActionView @mime_type ||= Mime::Type.lookup_by_extension(@formats.first.to_s) if @formats.first end + # TODO Remove me def variable_name @variable_name ||= @virtual_path[%r'_?(\w+)(\.\w+)*$', 1].to_sym end + # TODO Remove me def counter_name @counter_name ||= "#{variable_name}_counter".to_sym end @@ -166,7 +164,25 @@ module ActionView end end - private + def compile!(view) + return if @compiled + + if view.is_a?(ActionView::CompiledTemplates) + mod = ActionView::CompiledTemplates + else + mod = view.singleton_class + end + + compile(view, mod) + + # Just discard the source if we have a virtual path. This + # means we can get the template back. + @source = nil if @virtual_path + @compiled = true + end + + protected + # Among other things, this method is responsible for properly setting # the encoding of the source. Until this point, we assume that the # source is BINARY data. If no additional information is supplied, @@ -187,11 +203,9 @@ module ActionView # encode the source into Encoding.default_internal. In general, # this means that templates will be UTF-8 inside of Rails, # regardless of the original source encoding. - def compile(locals, view, mod) - method_name = build_method_name(locals) - return method_name if view.respond_to?(method_name) - - locals_code = locals.keys.map! { |key| "#{key} = local_assigns[:#{key}];" }.join + def compile(view, mod) + method_name = self.method_name + locals_code = @locals.map { |key| "#{key} = local_assigns[:#{key}];" }.join if source.encoding_aware? # Look for # encoding: *. If we find one, we'll encode the @@ -256,8 +270,6 @@ module ActionView begin mod.module_eval(source, identifier, 0) ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) - - method_name rescue Exception => e # errors from template code if logger = (view && view.logger) logger.debug "ERROR: compiling #{method_name} RAISED #{e}" @@ -269,12 +281,20 @@ module ActionView end end - def build_method_name(locals) - @method_names[locals.keys.hash] ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}_#{locals.keys.hash}".gsub('-', "_") + def refresh(view) + return unless @virtual_path + pieces = @virtual_path.split("/") + name = pieces.pop + partial = name.sub!(/^_/, "") + view.find_template(name, pieces.join, partial || false, @locals) + end + + def method_name + @method_name ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}".gsub('-', "_") end def identifier_method_name - @identifier_method_name ||= inspect.gsub(/[^a-z_]/, '_') + inspect.gsub(/[^a-z_]/, '_') end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a031d0dc33..9ec39b16f0 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -6,8 +6,8 @@ module ActionView # = Action View Resolver class Resolver def initialize - @cached = Hash.new { |h1,k1| h1[k1] = - Hash.new { |h2,k2| h2[k2] = Hash.new { |h3, k3| h3[k3] = {} } } } + @cached = Hash.new { |h1,k1| h1[k1] = Hash.new { |h2,k2| + h2[k2] = Hash.new { |h3,k3| h3[k3] = Hash.new { |h4,k4| h4[k4] = {} } } } } end def clear_cache @@ -15,8 +15,8 @@ module ActionView end # Normalizes the arguments and passes it on to find_template. - def find_all(name, prefix=nil, partial=false, details={}, key=nil) - cached(key, prefix, name, partial) do + def find_all(name, prefix=nil, partial=false, details={}, locals=[], key=nil) + cached(key, prefix, name, partial, locals) do find_templates(name, prefix, partial, details) end end @@ -51,9 +51,25 @@ module ActionView [handler, format] end - def cached(key, prefix, name, partial) - return yield unless key && caching? - @cached[key][prefix][name][partial] ||= yield + def cached(key, prefix, name, partial, locals) + locals = sort_locals(locals) + unless key && caching? + yield.each { |t| t.locals = locals } + else + @cached[key][prefix][name][partial][locals] ||= yield.each { |t| t.locals = locals } + end + end + + if :locale.respond_to?("<=>") + def sort_locals(locals) + locals.sort.freeze + end + else + def sort_locals(locals) + locals = locals.map{ |l| l.to_s } + locals.sort! + locals.freeze + end end end diff --git a/actionpack/lib/action_view/template/text.rb b/actionpack/lib/action_view/template/text.rb index 51be831dfb..4261c3b5e2 100644 --- a/actionpack/lib/action_view/template/text.rb +++ b/actionpack/lib/action_view/template/text.rb @@ -25,10 +25,6 @@ module ActionView #:nodoc: def formats [@mime_type.to_sym] end - - def partial? - false - end end end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index c7c33af670..210ba673f0 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -51,6 +51,7 @@ class TestERBTemplate < ActiveSupport::TestCase def test_locals @template = new_template("<%= my_local %>") + @template.locals = [:my_local] assert_equal "I'm a local", render(:my_local => "I'm a local") end -- cgit v1.2.3 From 8f9e9118e402ea2fe1eec6fcb9a2d3f0c84b3b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 16:25:13 +0200 Subject: Make collection rendering faster. --- actionpack/lib/abstract_controller/rendering.rb | 2 +- actionpack/lib/action_view/render/partials.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 5d9b35d297..b4ab3481d5 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -50,7 +50,7 @@ module AbstractController if controller.respond_to?(:_helpers) include controller._helpers - if controller.respond_to?(:_routes) + if controller.respond_to?(:_routes) && controller._routes include controller._routes.url_helpers include controller._routes.mounted_helpers end diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 07e844afc2..f7bdbd6917 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -307,12 +307,12 @@ module ActionView def collection_without_template segments, locals, collection_data = [], @locals, @collection_data - index, template = -1, nil + index, template, cache = -1, nil, {} keys = @locals.keys @collection.each_with_index do |object, i| path, *data = collection_data[i] - template = find_template(path, keys + data) + template = (cache[path] ||= find_template(path, keys + data)) locals[data[0]] = object locals[data[1]] = (index += 1) segments << template.render(@view, locals) -- cgit v1.2.3 From c563f10f3e8083bebe32200fa065748c8bcb65c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 20:48:21 +0200 Subject: render :template => 'foo/bar.json' now works as it should. --- actionpack/lib/abstract_controller/rendering.rb | 1 - actionpack/lib/action_view/render/partials.rb | 14 ++++++++------ actionpack/lib/action_view/render/rendering.rb | 21 ++++++++++++++++++--- actionpack/lib/action_view/template.rb | 4 ++-- .../test/controller/new_base/render_partial_test.rb | 17 +++++++++++++++-- .../controller/new_base/render_template_test.rb | 15 ++++++++++++++- 6 files changed, 57 insertions(+), 15 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index b4ab3481d5..e88e5aefc0 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -12,7 +12,6 @@ module AbstractController # This is a class to fix I18n global state. Whenever you provide I18n.locale during a request, # it will trigger the lookup_context and consequently expire the cache. - # TODO Add some deprecation warnings to remove I18n.locale from controllers class I18nProxy < ::I18n::Config #:nodoc: attr_reader :i18n_config, :lookup_context diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index f7bdbd6917..24d9e9ffb5 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -371,13 +371,15 @@ module ActionView end def _render_partial(options, &block) #:nodoc: - if defined?(@renderer) - @renderer.setup(options, block) - else - @renderer = PartialRenderer.new(self, options, block) - end + _wrap_formats(options[:partial]) do + if defined?(@renderer) + @renderer.setup(options, block) + else + @renderer = PartialRenderer.new(self, options, block) + end - @renderer.render + @renderer.render + end end end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 0771b40c37..8e599c71df 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -21,9 +21,11 @@ module ActionView elsif options.key?(:partial) _render_partial(options) else - template = _determine_template(options) - lookup_context.freeze_formats(template.formats, true) - _render_template(template, options[:layout], options) + _wrap_formats(options[:template] || options[:file]) do + template = _determine_template(options) + lookup_context.freeze_formats(template.formats, true) + _render_template(template, options[:layout], options) + end end when :update update_page(&block) @@ -32,6 +34,19 @@ module ActionView end end + # Checks if the given path contains a format and if so, change + # the lookup context to take this new format into account. + def _wrap_formats(value) + return yield unless value.is_a?(String) + @@formats_regexp ||= /\.(#{Mime::SET.symbols.join('|')})$/ + + if value.sub!(@@formats_regexp, "") + update_details(:formats => [$1.to_sym]){ yield } + else + yield + end + end + # Determine the template to be rendered using the given options. def _determine_template(options) #:nodoc: keys = (options[:locals] ||= {}).keys diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 7e65654d0b..c15a0eb568 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -121,7 +121,7 @@ module ActionView @method_names = {} @locals = details[:locals] || [] @formats = Array.wrap(details[:format] || :html).map(&:to_sym) - @virtual_path = details[:virtual_path].try(:sub, ".#{@formats.first}", "") + @virtual_path = details[:virtual_path] @compiled = false end @@ -286,7 +286,7 @@ module ActionView pieces = @virtual_path.split("/") name = pieces.pop partial = name.sub!(/^_/, "") - view.find_template(name, pieces.join, partial || false, @locals) + view.find_template(name, pieces.join, partial || false, ["unlikely_local_key"]) end def method_name diff --git a/actionpack/test/controller/new_base/render_partial_test.rb b/actionpack/test/controller/new_base/render_partial_test.rb index 5c7e66dba2..d800ea264d 100644 --- a/actionpack/test/controller/new_base/render_partial_test.rb +++ b/actionpack/test/controller/new_base/render_partial_test.rb @@ -5,10 +5,17 @@ module RenderPartial class BasicController < ActionController::Base self.view_paths = [ActionView::FixtureResolver.new( - "render_partial/basic/_basic.html.erb" => "BasicPartial!", - "render_partial/basic/basic.html.erb" => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'basic' %><%= @test_unchanged %>" + "render_partial/basic/_basic.html.erb" => "BasicPartial!", + "render_partial/basic/basic.html.erb" => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'basic' %><%= @test_unchanged %>", + "render_partial/basic/with_json.html.erb" => "<%= render 'with_json.json' %>", + "render_partial/basic/_with_json.json.erb" => "<%= render 'final' %>", + "render_partial/basic/_final.json.erb" => "{ final: json }" )] + def html_with_json_inside_json + render :action => "with_json" + end + def changing @test_unchanged = 'hello' render :action => "basic" @@ -22,6 +29,12 @@ module RenderPartial get :changing assert_response("goodbyeBasicPartial!goodbye") end + + test "rendering a template with renders another partial with other format that renders other partial in the same format" do + get :html_with_json_inside_json + assert_content_type "text/html; charset=utf-8" + assert_response "{ final: json }" + end end end diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index fea98d6bbd..8f5f51d70b 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -8,13 +8,20 @@ module RenderTemplate "shared.html.erb" => "Elastica", "locals.html.erb" => "The secret is <%= secret %>", "xml_template.xml.builder" => "xml.html do\n xml.p 'Hello'\nend", - "with_raw.html.erb" => "Hello <%=raw 'this is raw' %>" + "with_raw.html.erb" => "Hello <%=raw 'this is raw' %>", + "test/with_json.html.erb" => "<%= render :template => 'test/with_json.json' %>", + "test/with_json.json.erb" => "<%= render :template => 'test/final' %>", + "test/final.json.erb" => "{ final: json }" )] def index render :template => "test/basic" end + def html_with_json_inside_json + render :template => "test/with_json" + end + def index_without_key render "test/basic" end @@ -88,6 +95,12 @@ module RenderTemplate assert_body "Hello this is raw" assert_status 200 end + + test "rendering a template with renders another template with other format that renders other template in the same format" do + get :html_with_json_inside_json + assert_content_type "text/html; charset=utf-8" + assert_response "{ final: json }" + end end class WithLayoutController < ::ApplicationController -- cgit v1.2.3 From c7760809bfc8e19362272b71b23a294d48195d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 7 Oct 2010 21:30:19 +0200 Subject: Allow cache to be temporarily disabled through lookup_context. --- actionpack/lib/action_view/lookup_context.rb | 13 ++++++++++++- actionpack/lib/action_view/template.rb | 4 +++- actionpack/lib/action_view/testing/resolvers.rb | 8 +++++--- .../controller/new_base/render_template_test.rb | 13 ++++++++++++- actionpack/test/template/lookup_context_test.rb | 21 +++++++++++++++++++++ 5 files changed, 53 insertions(+), 6 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 794fdeae64..0cff888ac1 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -61,6 +61,7 @@ module ActionView def initialize(view_paths, details = {}) @details, @details_key = { :handlers => default_handlers }, nil @frozen_formats, @skip_default_locale = false, false + @cache = details.key?(:cache) ? details.delete(:cache) : true self.view_paths = view_paths self.registered_detail_setters.each do |key, setter| @@ -130,10 +131,20 @@ module ActionView end module Details + attr_accessor :cache + # Calculate the details key. Remove the handlers from calculation to improve performance # since the user cannot modify it explicitly. def details_key #:nodoc: - @details_key ||= DetailsKey.get(@details) + @details_key ||= DetailsKey.get(@details) if @cache + end + + # Temporary skip passing the details_key forward. + def disable_cache + old_value, @cache = @cache, false + yield + ensure + @cache = old_value end # Freeze the current formats in the lookup context. By freezing them, you are guaranteeing diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index c15a0eb568..1268b2f426 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -286,7 +286,9 @@ module ActionView pieces = @virtual_path.split("/") name = pieces.pop partial = name.sub!(/^_/, "") - view.find_template(name, pieces.join, partial || false, ["unlikely_local_key"]) + view.lookup_context.disable_cache do + view.find_template(name, pieces.join, partial || false, @locals) + end end def method_name diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index 6d82fa0971..ec0db48379 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -16,16 +16,18 @@ module ActionView #:nodoc: private def query(path, exts, formats) - query = Regexp.escape(path) + query = "" exts.each do |ext| query << '(' << ext.map {|e| e && Regexp.escape(".#{e}") }.join('|') << '|)' end + query = /^(#{Regexp.escape(path)})#{query}$/ templates = [] - @hash.select { |k,v| k =~ /^#{query}$/ }.each do |_path, source| + @hash.each do |_path, source| + next unless _path =~ query handler, format = extract_handler_and_format(_path, formats) templates << Template.new(source, _path, handler, - :virtual_path => _path, :format => format) + :virtual_path => $1, :format => format) end templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size } diff --git a/actionpack/test/controller/new_base/render_template_test.rb b/actionpack/test/controller/new_base/render_template_test.rb index 8f5f51d70b..d31193a063 100644 --- a/actionpack/test/controller/new_base/render_template_test.rb +++ b/actionpack/test/controller/new_base/render_template_test.rb @@ -11,7 +11,8 @@ module RenderTemplate "with_raw.html.erb" => "Hello <%=raw 'this is raw' %>", "test/with_json.html.erb" => "<%= render :template => 'test/with_json.json' %>", "test/with_json.json.erb" => "<%= render :template => 'test/final' %>", - "test/final.json.erb" => "{ final: json }" + "test/final.json.erb" => "{ final: json }", + "test/with_error.html.erb" => "<%= idontexist %>" )] def index @@ -49,6 +50,10 @@ module RenderTemplate def with_raw render :template => "with_raw" end + + def with_error + render :template => "test/with_error" + end end class TestWithoutLayout < Rack::TestCase @@ -101,6 +106,12 @@ module RenderTemplate assert_content_type "text/html; charset=utf-8" assert_response "{ final: json }" end + + test "rendering a template with error properly exceprts the code" do + get :with_error + assert_status 500 + assert_match "undefined local variable or method `idontexist'", response.body + end end class WithLayoutController < ::ApplicationController diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index cc71cb42d0..55d581e512 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -163,4 +163,25 @@ class LookupContextTest < ActiveSupport::TestCase template = @lookup_context.find("foo", "test", true) assert_equal "Bar", template.source end + + test "can disable the cache on demand" do + @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") + old_template = @lookup_context.find("foo", "test", true) + + template = @lookup_context.find("foo", "test", true) + assert_equal template, old_template + + assert @lookup_context.cache + template = @lookup_context.disable_cache do + assert !@lookup_context.cache + @lookup_context.find("foo", "test", true) + end + assert @lookup_context.cache + + assert_not_equal template, old_template + end + + test "can have cache disabled on initialization" do + assert !ActionView::LookupContext.new(FIXTURE_LOAD_PATH, :cache => false).cache + end end \ No newline at end of file -- cgit v1.2.3 From 581b2b68368e3330cc725a305d0e2465c2e71e1c Mon Sep 17 00:00:00 2001 From: Michael Koziarski Date: Fri, 8 Oct 2010 11:12:11 +1300 Subject: fix rendering a partial with an array as its :object [#5746 state:resolved] Signed-off-by: Michael Koziarski Conflicts: actionpack/lib/action_view/render/partials.rb --- actionpack/lib/action_view/render/partials.rb | 10 +++++++--- actionpack/test/fixtures/test/_object_inspector.erb | 1 + actionpack/test/template/render_test.rb | 4 ++++ 3 files changed, 12 insertions(+), 3 deletions(-) create mode 100644 actionpack/test/fixtures/test/_object_inspector.erb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 24d9e9ffb5..4e03d43358 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -235,7 +235,7 @@ module ActionView else @object = partial - if @collection = collection + if @collection = collection_from_object || collection paths = @collection_data = @collection.map { |o| partial_path(o) } @path = paths.uniq.size == 1 ? paths.first : nil else @@ -337,10 +337,14 @@ module ActionView private def collection + if @options.key?(:collection) + @options[:collection] || [] + end + end + + def collection_from_object if @object.respond_to?(:to_ary) @object - elsif @options.key?(:collection) - @options[:collection] || [] end end diff --git a/actionpack/test/fixtures/test/_object_inspector.erb b/actionpack/test/fixtures/test/_object_inspector.erb new file mode 100644 index 0000000000..53af593821 --- /dev/null +++ b/actionpack/test/fixtures/test/_object_inspector.erb @@ -0,0 +1 @@ +<%= object_inspector.inspect -%> \ No newline at end of file diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 205fdcf345..756d8d05d2 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -127,6 +127,10 @@ module RenderTestCases assert_equal "Hello: david", @view.render(:partial => "test/customer", :object => Customer.new("david")) end + def test_render_object_with_array + assert_equal "[1, 2, 3]", @view.render(:partial => "test/object_inspector", :object => [1, 2, 3]) + end + def test_render_partial_collection assert_equal "Hello: davidHello: mary", @view.render(:partial => "test/customer", :collection => [ Customer.new("david"), Customer.new("mary") ]) end -- cgit v1.2.3 From 54b09e17d4527bb4e5508d7dcb286040c3e3ea3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 8 Oct 2010 09:24:06 +0200 Subject: Fix 1.9.2 failures. --- actionpack/test/template/template_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 210ba673f0..27b42006e3 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -32,8 +32,8 @@ class TestERBTemplate < ActiveSupport::TestCase end end - def new_template(body = "<%= hello %>", handler = ERBHandler, details = {}) - ActionView::Template.new(body, "hello template", ERBHandler, {:virtual_path => "hello"}) + def new_template(body = "<%= hello %>", details = {}) + ActionView::Template.new(body, "hello template", ERBHandler, {:virtual_path => "hello"}.merge!(details)) end def render(locals = {}) @@ -102,7 +102,7 @@ class TestERBTemplate < ActiveSupport::TestCase # inside Rails. def test_lying_with_magic_comment assert_raises(ActionView::Template::Error) do - @template = new_template("# encoding: UTF-8\nhello \xFCmlat") + @template = new_template("# encoding: UTF-8\nhello \xFCmlat", :virtual_path => nil) render end end @@ -118,7 +118,7 @@ class TestERBTemplate < ActiveSupport::TestCase def test_error_when_template_isnt_valid_utf8 assert_raises(ActionView::Template::Error, /\xFC/) do - @template = new_template("hello \xFCmlat") + @template = new_template("hello \xFCmlat", :virtual_path => nil) render end end -- cgit v1.2.3 From 067c1aa0e098f293a99953c50babaf201bba60cd Mon Sep 17 00:00:00 2001 From: Andrew White Date: Fri, 8 Oct 2010 22:00:27 +0100 Subject: Refactor resource action scope methods --- actionpack/lib/action_dispatch/routing/mapper.rb | 58 ++++++++---------------- 1 file changed, 20 insertions(+), 38 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index bf10f81127..3c7dcea003 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -735,15 +735,15 @@ module ActionDispatch resource_scope(SingletonResource.new(resources.pop, options)) do yield if block_given? - collection_scope do + collection do post :create end if parent_resource.actions.include?(:create) - new_scope do + new do get :new end if parent_resource.actions.include?(:new) - member_scope do + member do get :edit if parent_resource.actions.include?(:edit) get :show if parent_resource.actions.include?(:show) put :update if parent_resource.actions.include?(:update) @@ -780,16 +780,16 @@ module ActionDispatch resource_scope(Resource.new(resources.pop, options)) do yield if block_given? - collection_scope do + collection do get :index if parent_resource.actions.include?(:index) post :create if parent_resource.actions.include?(:create) end - new_scope do + new do get :new end if parent_resource.actions.include?(:new) - member_scope do + member do get :edit if parent_resource.actions.include?(:edit) get :show if parent_resource.actions.include?(:show) put :update if parent_resource.actions.include?(:update) @@ -813,12 +813,14 @@ module ActionDispatch # create the search_photos_url and search_photos_path # route helpers. def collection - unless @scope[:scope_level] == :resources - raise ArgumentError, "can't use collection outside resources scope" + unless resource_scope? + raise ArgumentError, "can't use collection outside resource(s) scope" end - collection_scope do - yield + with_scope_level(:collection) do + scope(parent_resource.collection_scope) do + yield + end end end @@ -838,8 +840,10 @@ module ActionDispatch raise ArgumentError, "can't use member outside resource(s) scope" end - member_scope do - yield + with_scope_level(:member) do + scope(parent_resource.member_scope) do + yield + end end end @@ -848,8 +852,10 @@ module ActionDispatch raise ArgumentError, "can't use new outside resource(s) scope" end - new_scope do - yield + with_scope_level(:new) do + scope(parent_resource.new_scope(action_path(:new))) do + yield + end end end @@ -1034,30 +1040,6 @@ module ActionDispatch end end - def new_scope - with_scope_level(:new) do - scope(parent_resource.new_scope(action_path(:new))) do - yield - end - end - end - - def collection_scope - with_scope_level(:collection) do - scope(parent_resource.collection_scope) do - yield - end - end - end - - def member_scope - with_scope_level(:member) do - scope(parent_resource.member_scope) do - yield - end - end - end - def nested_options {}.tap do |options| options[:as] = parent_resource.member_name -- cgit v1.2.3 From e695c40e11135e54e40aef97718f88d20a0967c2 Mon Sep 17 00:00:00 2001 From: Andrea Campi Date: Sat, 9 Oct 2010 12:11:57 +0200 Subject: Fix example that became outdated after a code change. [#5770 state:resolved] Signed-off-by: Santiago Pastorino --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index c1dfbe5dc3..3ff80579e2 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -152,7 +152,7 @@ module ActionView # # # Normally you'd calculate RELEASE_NUMBER at startup. # RELEASE_NUMBER = 12345 - # config.action_controller.asset_path_template = proc { |asset_path| + # config.action_controller.asset_path = proc { |asset_path| # "/release-#{RELEASE_NUMBER}#{asset_path}" # } # -- cgit v1.2.3 From 64c7f7e39244129e9330afed82da8a7ffeb948b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sat, 9 Oct 2010 23:56:08 +0200 Subject: Add more docs and tests to templates. --- actionpack/lib/action_view/template.rb | 95 ++++++++++++++++++------------- actionpack/test/template/template_test.rb | 12 ++++ 2 files changed, 66 insertions(+), 41 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 1268b2f426..04ff752e64 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -125,34 +125,41 @@ module ActionView @compiled = false end + # Render a template. If the template was not compiled yet, it is done + # exactly before rendering. + # + # This method is instrumented as "!render_template.action_view". Notice that + # we use a bang in this instrumentation because you don't want to + # consume this in production. This is only slow if it's being listened to. def render(view, locals, &block) - # Notice that we use a bang in this instrumentation because you don't want to - # consume this in production. This is only slow if it's being listened to. ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do compile!(view) view.send(method_name, locals, &block) end rescue Exception => e - if e.is_a?(Template::Error) - e.sub_template_of(self) - raise e - else - raise Template::Error.new(refresh(view) || self, view.respond_to?(:assigns) ? view.assigns : {}, e) - end + handle_render_error(view, e) end def mime_type @mime_type ||= Mime::Type.lookup_by_extension(@formats.first.to_s) if @formats.first end - # TODO Remove me - def variable_name - @variable_name ||= @virtual_path[%r'_?(\w+)(\.\w+)*$', 1].to_sym - end - - # TODO Remove me - def counter_name - @counter_name ||= "#{variable_name}_counter".to_sym + # Receives a view object and return a template similar to self by using @virtual_path. + # + # This method is useful if you have a template object but it does not contain its source + # anymore since it was already compiled. In such cases, all you need to do is to call + # refresh passing in the view object. + # + # Notice this method raises an error if the template to be refreshed does not have a + # virtual path set (true just for inline templates). + def refresh(view) + raise "A template need to have a virtual path in order to be refreshed" unless @virtual_path + pieces = @virtual_path.split("/") + name = pieces.pop + partial = name.sub!(/^_/, "") + view.lookup_context.disable_cache do + view.find_template(name, pieces.join, partial || false, @locals) + end end def inspect @@ -164,24 +171,26 @@ module ActionView end end - def compile!(view) - return if @compiled + protected - if view.is_a?(ActionView::CompiledTemplates) - mod = ActionView::CompiledTemplates - else - mod = view.singleton_class - end + # Compile a template. This method ensures a template is compiled + # just once and removes the source after it is compiled. + def compile!(view) #:nodoc: + return if @compiled - compile(view, mod) + if view.is_a?(ActionView::CompiledTemplates) + mod = ActionView::CompiledTemplates + else + mod = view.singleton_class + end - # Just discard the source if we have a virtual path. This - # means we can get the template back. - @source = nil if @virtual_path - @compiled = true - end + compile(view, mod) - protected + # Just discard the source if we have a virtual path. This + # means we can get the template back. + @source = nil if @virtual_path + @compiled = true + end # Among other things, this method is responsible for properly setting # the encoding of the source. Until this point, we assume that the @@ -203,9 +212,8 @@ module ActionView # encode the source into Encoding.default_internal. In general, # this means that templates will be UTF-8 inside of Rails, # regardless of the original source encoding. - def compile(view, mod) + def compile(view, mod) #:nodoc: method_name = self.method_name - locals_code = @locals.map { |key| "#{key} = local_assigns[:#{key}];" }.join if source.encoding_aware? # Look for # encoding: *. If we find one, we'll encode the @@ -281,21 +289,26 @@ module ActionView end end - def refresh(view) - return unless @virtual_path - pieces = @virtual_path.split("/") - name = pieces.pop - partial = name.sub!(/^_/, "") - view.lookup_context.disable_cache do - view.find_template(name, pieces.join, partial || false, @locals) + def handle_render_error(view, e) #:nodoc: + if e.is_a?(Template::Error) + e.sub_template_of(self) + raise e + else + assigns = view.respond_to?(:assigns) ? view.assigns : {} + template = @virtual_path ? refresh(view) : self + raise Template::Error.new(template, assigns, e) end end - def method_name + def locals_code #:nodoc: + @locals.map { |key| "#{key} = local_assigns[:#{key}];" }.join + end + + def method_name #:nodoc: @method_name ||= "_#{identifier_method_name}__#{@identifier.hash}_#{__id__}".gsub('-', "_") end - def identifier_method_name + def identifier_method_name #:nodoc: inspect.gsub(/[^a-z_]/, '_') end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 27b42006e3..00bfbbccd6 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -49,6 +49,18 @@ class TestERBTemplate < ActiveSupport::TestCase assert_equal "Hello", render end + def test_template_loses_its_source_after_rendering + @template = new_template + render + assert_nil @template.source + end + + def test_template_does_not_lose_its_source_after_rendering_if_it_does_not_have_a_virtual_path + @template = new_template("Hello", :virtual_path => nil) + render + assert_equal "Hello", @template.source + end + def test_locals @template = new_template("<%= my_local %>") @template.locals = [:my_local] -- cgit v1.2.3 From c7408a0e40545558872efb4129fe4bf097c9ce2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 01:14:50 +0200 Subject: Deprecate old template handler API. Remove old handlers. --- actionpack/lib/action_view/template/handler.rb | 10 ++++++++- actionpack/lib/action_view/template/handlers.rb | 10 +++------ .../lib/action_view/template/handlers/builder.rb | 8 +++---- .../lib/action_view/template/handlers/erb.rb | 25 +++++++++++++--------- .../lib/action_view/template/handlers/rjs.rb | 12 ++++------- actionpack/test/controller/content_type_test.rb | 24 ++++++++++----------- actionpack/test/controller/layout_test.rb | 18 ++++++++-------- .../test/fixtures/layout_tests/alt/hello.erb | 1 + .../test/fixtures/layout_tests/alt/hello.rhtml | 1 - .../test/fixtures/layout_tests/alt/layouts/alt.erb | 0 .../fixtures/layout_tests/alt/layouts/alt.rhtml | 0 .../layouts/controller_name_space/nested.erb | 1 + .../layouts/controller_name_space/nested.rhtml | 1 - .../test/fixtures/layout_tests/layouts/item.erb | 1 + .../test/fixtures/layout_tests/layouts/item.rhtml | 1 - .../fixtures/layout_tests/layouts/layout_test.erb | 1 + .../layout_tests/layouts/layout_test.rhtml | 1 - .../test/fixtures/layout_tests/views/goodbye.erb | 1 + .../test/fixtures/layout_tests/views/goodbye.rhtml | 1 - .../test/fixtures/layout_tests/views/hello.erb | 1 + .../test/fixtures/layout_tests/views/hello.rhtml | 1 - .../render_default_for_builder.builder | 1 + .../old_content_type/render_default_for_erb.erb | 1 + .../render_default_for_rhtml.rhtml | 1 - .../old_content_type/render_default_for_rxml.rxml | 1 - actionpack/test/template/template_test.rb | 4 ++-- 26 files changed, 66 insertions(+), 61 deletions(-) create mode 100644 actionpack/test/fixtures/layout_tests/alt/hello.erb delete mode 100644 actionpack/test/fixtures/layout_tests/alt/hello.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/alt/layouts/alt.erb delete mode 100644 actionpack/test/fixtures/layout_tests/alt/layouts/alt.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.erb delete mode 100644 actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/layouts/item.erb delete mode 100644 actionpack/test/fixtures/layout_tests/layouts/item.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/layouts/layout_test.erb delete mode 100644 actionpack/test/fixtures/layout_tests/layouts/layout_test.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/views/goodbye.erb delete mode 100644 actionpack/test/fixtures/layout_tests/views/goodbye.rhtml create mode 100644 actionpack/test/fixtures/layout_tests/views/hello.erb delete mode 100644 actionpack/test/fixtures/layout_tests/views/hello.rhtml create mode 100644 actionpack/test/fixtures/old_content_type/render_default_for_builder.builder create mode 100644 actionpack/test/fixtures/old_content_type/render_default_for_erb.erb delete mode 100644 actionpack/test/fixtures/old_content_type/render_default_for_rhtml.rhtml delete mode 100644 actionpack/test/fixtures/old_content_type/render_default_for_rxml.rxml (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template/handler.rb b/actionpack/lib/action_view/template/handler.rb index c6a1bc6235..0a9d299807 100644 --- a/actionpack/lib/action_view/template/handler.rb +++ b/actionpack/lib/action_view/template/handler.rb @@ -1,4 +1,4 @@ -require "action_dispatch/http/mime_type" +require 'action_dispatch/http/mime_type' require 'active_support/core_ext/class/attribute' # Legacy TemplateHandler stub @@ -7,6 +7,8 @@ module ActionView module Handlers #:nodoc: module Compilable def self.included(base) + ActiveSupport::Deprecation.warn "Including Compilable in your template handler is deprecated. " << + "All the API your template handler needs to implement is to respond to #call." base.extend(ClassMethods) end @@ -26,6 +28,12 @@ module ActionView class_attribute :default_format self.default_format = Mime::HTML + def self.inherited(base) + ActiveSupport::Deprecation.warn "Inheriting from ActionView::Template::Handler is deprecated. " << + "All the API your template handler needs to implement is to respond to #call." + super + end + def self.call(template) raise "Need to implement #{self.class.name}#call(template)" end diff --git a/actionpack/lib/action_view/template/handlers.rb b/actionpack/lib/action_view/template/handlers.rb index ed397699b0..60347e2a95 100644 --- a/actionpack/lib/action_view/template/handlers.rb +++ b/actionpack/lib/action_view/template/handlers.rb @@ -7,13 +7,9 @@ module ActionView #:nodoc: autoload :Builder, 'action_view/template/handlers/builder' def self.extended(base) - base.register_default_template_handler :erb, ERB - base.register_template_handler :rjs, RJS - base.register_template_handler :builder, Builder - - # TODO: Depreciate old template extensions - base.register_template_handler :rhtml, ERB - base.register_template_handler :rxml, Builder + base.register_default_template_handler :erb, ERB.new + base.register_template_handler :rjs, RJS.new + base.register_template_handler :builder, Builder.new end @@template_handlers = {} diff --git a/actionpack/lib/action_view/template/handlers/builder.rb b/actionpack/lib/action_view/template/handlers/builder.rb index a93cfca8aa..2c52cfd90e 100644 --- a/actionpack/lib/action_view/template/handlers/builder.rb +++ b/actionpack/lib/action_view/template/handlers/builder.rb @@ -1,11 +1,11 @@ module ActionView module Template::Handlers - class Builder < Template::Handler - include Compilable - + class Builder + # Default format used by Builder. + class_attribute :default_format self.default_format = Mime::XML - def compile(template) + def call(template) require 'builder' "xml = ::Builder::XmlMarkup.new(:indent => 2);" + "self.output_buffer = xml.target!;" + diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb index 24e1e44c1d..b827610456 100644 --- a/actionpack/lib/action_view/template/handlers/erb.rb +++ b/actionpack/lib/action_view/template/handlers/erb.rb @@ -1,6 +1,7 @@ require 'active_support/core_ext/class/attribute_accessors' require 'active_support/core_ext/string/output_safety' -require "action_view/template" +require 'action_view/template' +require 'action_view/template/handler' require 'erubis' module ActionView @@ -47,28 +48,31 @@ module ActionView end end - class ERB < Handler - include Compilable - - ## - # :singleton-method: + class ERB # Specify trim mode for the ERB compiler. Defaults to '-'. # See ERb documentation for suitable values. - cattr_accessor :erb_trim_mode + class_attribute :erb_trim_mode self.erb_trim_mode = '-' + # Default format used by ERB. + class_attribute :default_format self.default_format = Mime::HTML - cattr_accessor :erb_implementation + # Default implemenation used. + class_attribute :erb_implementation self.erb_implementation = Erubis ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*") - def self.handles_encoding? + def self.call(template) + new.call(template) + end + + def handles_encoding? true end - def compile(template) + def call(template) if template.source.encoding_aware? # First, convert to BINARY, so in case the encoding is # wrong, we can still find an encoding tag @@ -94,6 +98,7 @@ module ActionView end private + def valid_encoding(string, encoding) # If a magic encoding comment was found, tag the # String with this encoding. This is for a case diff --git a/actionpack/lib/action_view/template/handlers/rjs.rb b/actionpack/lib/action_view/template/handlers/rjs.rb index 128be5077c..9d71059134 100644 --- a/actionpack/lib/action_view/template/handlers/rjs.rb +++ b/actionpack/lib/action_view/template/handlers/rjs.rb @@ -1,17 +1,13 @@ module ActionView module Template::Handlers - class RJS < Template::Handler - include Compilable - + class RJS + # Default format used by RJS. + class_attribute :default_format self.default_format = Mime::JS - def compile(template) + def call(template) "update_page do |page|;#{template.source}\nend" end - - def default_format - Mime::JS - end end end end diff --git a/actionpack/test/controller/content_type_test.rb b/actionpack/test/controller/content_type_test.rb index 967107853b..9500c25a32 100644 --- a/actionpack/test/controller/content_type_test.rb +++ b/actionpack/test/controller/content_type_test.rb @@ -29,18 +29,18 @@ class OldContentTypeController < ActionController::Base render :text => "hello world!" end - def render_default_for_rhtml + def render_default_for_erb end - def render_default_for_rxml + def render_default_for_builder end def render_default_for_rjs end - def render_change_for_rxml + def render_change_for_builder response.content_type = Mime::HTML - render :action => "render_default_for_rxml" + render :action => "render_default_for_builder" end def render_default_content_types_for_respond_to @@ -108,23 +108,23 @@ class ContentTypeTest < ActionController::TestCase assert_equal "utf-8", @response.charset, @response.headers.inspect end - def test_nil_default_for_rhtml + def test_nil_default_for_erb OldContentTypeController.default_charset = nil - get :render_default_for_rhtml + get :render_default_for_erb assert_equal Mime::HTML, @response.content_type assert_nil @response.charset, @response.headers.inspect ensure OldContentTypeController.default_charset = "utf-8" end - def test_default_for_rhtml - get :render_default_for_rhtml + def test_default_for_erb + get :render_default_for_erb assert_equal Mime::HTML, @response.content_type assert_equal "utf-8", @response.charset end - def test_default_for_rxml - get :render_default_for_rxml + def test_default_for_builder + get :render_default_for_builder assert_equal Mime::XML, @response.content_type assert_equal "utf-8", @response.charset end @@ -135,8 +135,8 @@ class ContentTypeTest < ActionController::TestCase assert_equal "utf-8", @response.charset end - def test_change_for_rxml - get :render_change_for_rxml + def test_change_for_builder + get :render_change_for_builder assert_equal Mime::HTML, @response.content_type assert_equal "utf-8", @response.charset end diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 32a0f40614..cafe2b9320 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -46,13 +46,13 @@ class LayoutAutoDiscoveryTest < ActionController::TestCase def test_application_layout_is_default_when_no_controller_match @controller = ProductController.new get :hello - assert_equal 'layout_test.rhtml hello.rhtml', @response.body + assert_equal 'layout_test.erb hello.erb', @response.body end def test_controller_name_layout_name_match @controller = ItemController.new get :hello - assert_equal 'item.rhtml hello.rhtml', @response.body + assert_equal 'item.erb hello.erb', @response.body end def test_third_party_template_library_auto_discovers_layout @@ -65,13 +65,13 @@ class LayoutAutoDiscoveryTest < ActionController::TestCase def test_namespaced_controllers_auto_detect_layouts1 @controller = ControllerNameSpace::NestedController.new get :hello - assert_equal 'controller_name_space/nested.rhtml hello.rhtml', @response.body + assert_equal 'controller_name_space/nested.erb hello.erb', @response.body end def test_namespaced_controllers_auto_detect_layouts2 @controller = MultipleExtensions.new get :hello - assert_equal 'multiple_extensions.html.erb hello.rhtml', @response.body.strip + assert_equal 'multiple_extensions.html.erb hello.erb', @response.body.strip end end @@ -79,7 +79,7 @@ class DefaultLayoutController < LayoutTest end class AbsolutePathLayoutController < LayoutTest - layout File.expand_path(File.expand_path(__FILE__) + '/../../fixtures/layout_tests/layouts/layout_test.rhtml') + layout File.expand_path(File.expand_path(__FILE__) + '/../../fixtures/layout_tests/layouts/layout_test.erb') end class HasOwnLayoutController < LayoutTest @@ -137,7 +137,7 @@ class LayoutSetInResponseTest < ActionController::TestCase def test_layout_only_exception_when_excepted @controller = OnlyLayoutController.new get :goodbye - assert !@response.body.include?("item.rhtml"), "#{@response.body.inspect} included 'item.rhtml'" + assert !@response.body.include?("item.erb"), "#{@response.body.inspect} included 'item.erb'" end def test_layout_except_exception_when_included @@ -149,7 +149,7 @@ class LayoutSetInResponseTest < ActionController::TestCase def test_layout_except_exception_when_excepted @controller = ExceptLayoutController.new get :goodbye - assert !@response.body.include?("item.rhtml"), "#{@response.body.inspect} included 'item.rhtml'" + assert !@response.body.include?("item.erb"), "#{@response.body.inspect} included 'item.erb'" end def test_layout_set_when_using_render @@ -173,7 +173,7 @@ class LayoutSetInResponseTest < ActionController::TestCase def test_absolute_pathed_layout @controller = AbsolutePathLayoutController.new get :hello - assert_equal "layout_test.rhtml hello.rhtml", @response.body.strip + assert_equal "layout_test.erb hello.erb", @response.body.strip end end @@ -184,7 +184,7 @@ class RenderWithTemplateOptionController < LayoutTest end class SetsNonExistentLayoutFile < LayoutTest - layout "nofile.rhtml" + layout "nofile.erb" end class LayoutExceptionRaised < ActionController::TestCase diff --git a/actionpack/test/fixtures/layout_tests/alt/hello.erb b/actionpack/test/fixtures/layout_tests/alt/hello.erb new file mode 100644 index 0000000000..1055c36659 --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/alt/hello.erb @@ -0,0 +1 @@ +alt/hello.erb diff --git a/actionpack/test/fixtures/layout_tests/alt/hello.rhtml b/actionpack/test/fixtures/layout_tests/alt/hello.rhtml deleted file mode 100644 index fcda6cf97a..0000000000 --- a/actionpack/test/fixtures/layout_tests/alt/hello.rhtml +++ /dev/null @@ -1 +0,0 @@ -alt/hello.rhtml diff --git a/actionpack/test/fixtures/layout_tests/alt/layouts/alt.erb b/actionpack/test/fixtures/layout_tests/alt/layouts/alt.erb new file mode 100644 index 0000000000..e69de29bb2 diff --git a/actionpack/test/fixtures/layout_tests/alt/layouts/alt.rhtml b/actionpack/test/fixtures/layout_tests/alt/layouts/alt.rhtml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.erb b/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.erb new file mode 100644 index 0000000000..121bc079a1 --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.erb @@ -0,0 +1 @@ +controller_name_space/nested.erb <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.rhtml b/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.rhtml deleted file mode 100644 index 5f86a7de4d..0000000000 --- a/actionpack/test/fixtures/layout_tests/layouts/controller_name_space/nested.rhtml +++ /dev/null @@ -1 +0,0 @@ -controller_name_space/nested.rhtml <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/layouts/item.erb b/actionpack/test/fixtures/layout_tests/layouts/item.erb new file mode 100644 index 0000000000..60f04d77d5 --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/layouts/item.erb @@ -0,0 +1 @@ +item.erb <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/layouts/item.rhtml b/actionpack/test/fixtures/layout_tests/layouts/item.rhtml deleted file mode 100644 index 1bc7cbda06..0000000000 --- a/actionpack/test/fixtures/layout_tests/layouts/item.rhtml +++ /dev/null @@ -1 +0,0 @@ -item.rhtml <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/layouts/layout_test.erb b/actionpack/test/fixtures/layout_tests/layouts/layout_test.erb new file mode 100644 index 0000000000..b74ac0840d --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/layouts/layout_test.erb @@ -0,0 +1 @@ +layout_test.erb <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/layouts/layout_test.rhtml b/actionpack/test/fixtures/layout_tests/layouts/layout_test.rhtml deleted file mode 100644 index c0f2642b40..0000000000 --- a/actionpack/test/fixtures/layout_tests/layouts/layout_test.rhtml +++ /dev/null @@ -1 +0,0 @@ -layout_test.rhtml <%= yield %> \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/views/goodbye.erb b/actionpack/test/fixtures/layout_tests/views/goodbye.erb new file mode 100644 index 0000000000..4ee911188e --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/views/goodbye.erb @@ -0,0 +1 @@ +hello.erb \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/views/goodbye.rhtml b/actionpack/test/fixtures/layout_tests/views/goodbye.rhtml deleted file mode 100644 index bbccf0913e..0000000000 --- a/actionpack/test/fixtures/layout_tests/views/goodbye.rhtml +++ /dev/null @@ -1 +0,0 @@ -hello.rhtml \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/views/hello.erb b/actionpack/test/fixtures/layout_tests/views/hello.erb new file mode 100644 index 0000000000..4ee911188e --- /dev/null +++ b/actionpack/test/fixtures/layout_tests/views/hello.erb @@ -0,0 +1 @@ +hello.erb \ No newline at end of file diff --git a/actionpack/test/fixtures/layout_tests/views/hello.rhtml b/actionpack/test/fixtures/layout_tests/views/hello.rhtml deleted file mode 100644 index bbccf0913e..0000000000 --- a/actionpack/test/fixtures/layout_tests/views/hello.rhtml +++ /dev/null @@ -1 +0,0 @@ -hello.rhtml \ No newline at end of file diff --git a/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder b/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder new file mode 100644 index 0000000000..598d62e2fc --- /dev/null +++ b/actionpack/test/fixtures/old_content_type/render_default_for_builder.builder @@ -0,0 +1 @@ +xml.p "Hello world!" \ No newline at end of file diff --git a/actionpack/test/fixtures/old_content_type/render_default_for_erb.erb b/actionpack/test/fixtures/old_content_type/render_default_for_erb.erb new file mode 100644 index 0000000000..c7926d48bb --- /dev/null +++ b/actionpack/test/fixtures/old_content_type/render_default_for_erb.erb @@ -0,0 +1 @@ +<%= 'hello world!' %> \ No newline at end of file diff --git a/actionpack/test/fixtures/old_content_type/render_default_for_rhtml.rhtml b/actionpack/test/fixtures/old_content_type/render_default_for_rhtml.rhtml deleted file mode 100644 index c7926d48bb..0000000000 --- a/actionpack/test/fixtures/old_content_type/render_default_for_rhtml.rhtml +++ /dev/null @@ -1 +0,0 @@ -<%= 'hello world!' %> \ No newline at end of file diff --git a/actionpack/test/fixtures/old_content_type/render_default_for_rxml.rxml b/actionpack/test/fixtures/old_content_type/render_default_for_rxml.rxml deleted file mode 100644 index 598d62e2fc..0000000000 --- a/actionpack/test/fixtures/old_content_type/render_default_for_rxml.rxml +++ /dev/null @@ -1 +0,0 @@ -xml.p "Hello world!" \ No newline at end of file diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 00bfbbccd6..eb2d4aab36 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -1,7 +1,7 @@ require "abstract_unit" class TestERBTemplate < ActiveSupport::TestCase - ERBHandler = ActionView::Template::Handlers::ERB + ERBHandler = ActionView::Template::Handlers::ERB.new class Context def initialize @@ -121,7 +121,7 @@ class TestERBTemplate < ActiveSupport::TestCase def test_encoding_can_be_specified_with_magic_comment_in_erb with_external_encoding Encoding::UTF_8 do - @template = new_template("<%# encoding: ISO-8859-1 %>hello \xFCmlat") + @template = new_template("<%# encoding: ISO-8859-1 %>hello \xFCmlat", :virtual_path => nil) result = render assert_equal Encoding::UTF_8, render.encoding assert_equal "hello \u{fc}mlat", render -- cgit v1.2.3 From 38d78f99d52801d8392a7229b40edae74cc3d142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 09:24:17 +0200 Subject: Resolvers now consider timestamps. Before this patch, every request in development caused the template to be compiled, regardless if it was updated in the filesystem or not. This patch now checks the timestamp and only compiles it again if any change was done. While this probably won't show any difference for current setups, but it will be useful for asset template handlers (like SASS), as compiling their templates is slower than ERb, Haml, etc. --- actionpack/lib/action_view/template.rb | 25 ++++----- actionpack/lib/action_view/template/resolver.rb | 67 +++++++++++++++++-------- actionpack/lib/action_view/testing/resolvers.rb | 5 +- actionpack/test/template/lookup_context_test.rb | 45 +++++++++++++++++ 4 files changed, 107 insertions(+), 35 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 04ff752e64..3cc9bb2710 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -98,10 +98,9 @@ module ActionView extend Template::Handlers - attr_accessor :locals + attr_accessor :locals, :formats, :virtual_path - attr_reader :source, :identifier, :handler, :virtual_path, :formats, - :original_encoding + attr_reader :source, :identifier, :handler, :original_encoding, :updated_at # This finalizer is needed (and exactly with a proc inside another proc) # otherwise templates leak in development. @@ -114,15 +113,17 @@ module ActionView end def initialize(source, identifier, handler, details) - @source = source - @identifier = identifier - @handler = handler - @original_encoding = nil - @method_names = {} - @locals = details[:locals] || [] - @formats = Array.wrap(details[:format] || :html).map(&:to_sym) - @virtual_path = details[:virtual_path] - @compiled = false + format = details[:format] || (handler.default_format if handler.respond_to?(:default_format)) + + @source = source + @identifier = identifier + @handler = handler + @compiled = false + @original_encoding = nil + @locals = details[:locals] || [] + @virtual_path = details[:virtual_path] + @updated_at = details[:updated_at] || Time.now + @formats = Array.wrap(format).map(&:to_sym) end # Render a template. If the template was not compiled yet, it is done diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 9ec39b16f0..5c6877a923 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -16,7 +16,7 @@ module ActionView # Normalizes the arguments and passes it on to find_template. def find_all(name, prefix=nil, partial=false, details={}, locals=[], key=nil) - cached(key, prefix, name, partial, locals) do + cached(key, [name, prefix, partial], details, locals) do find_templates(name, prefix, partial, details) end end @@ -35,37 +35,55 @@ module ActionView end # Helpers that builds a path. Useful for building virtual paths. - def build_path(name, prefix, partial, details) + def build_path(name, prefix, partial) path = "" path << "#{prefix}/" unless prefix.empty? path << (partial ? "_#{name}" : name) path end - # Get the handler and format from the given parameters. - def retrieve_handler_and_format(handler, format, default_formats=nil) - handler = Template.handler_class_for_extension(handler) - format = format && Mime[format] - format ||= handler.default_format if handler.respond_to?(:default_format) - format ||= default_formats - [handler, format] - end - - def cached(key, prefix, name, partial, locals) + # Hnadles templates caching. If a key is given and caching is on + # always check the cache before hitting the resolver. Otherwise, + # it always hits the resolver but check if the resolver is fresher + # before returning it. + def cached(key, path_info, details, locals) #:nodoc: + name, prefix, partial = path_info locals = sort_locals(locals) - unless key && caching? - yield.each { |t| t.locals = locals } + + if key && caching? + @cached[key][name][prefix][partial][locals] ||= decorate(yield, path_info, details, locals) else - @cached[key][prefix][name][partial][locals] ||= yield.each { |t| t.locals = locals } + fresh = decorate(yield, path_info, details, locals) + return fresh unless key + + scope = @cached[key][name][prefix][partial] + cache = scope[locals] + mtime = cache && cache.map(&:updated_at).max + + if !mtime || fresh.empty? || fresh.any? { |t| t.updated_at > mtime } + scope[locals] = fresh + else + cache + end + end + end + + # Ensures all the resolver information is set in the template. + def decorate(templates, path_info, details, locals) #:nodoc: + cached = nil + templates.each do |t| + t.locals = locals + t.formats = details[:formats] || [:html] if t.formats.empty? + t.virtual_path ||= (cached ||= build_path(*path_info)) end end - if :locale.respond_to?("<=>") - def sort_locals(locals) + if :symbol.respond_to?("<=>") + def sort_locals(locals) #:nodoc: locals.sort.freeze end else - def sort_locals(locals) + def sort_locals(locals) #:nodoc: locals = locals.map{ |l| l.to_s } locals.sort! locals.freeze @@ -79,7 +97,7 @@ module ActionView private def find_templates(name, prefix, partial, details) - path = build_path(name, prefix, partial, details) + path = build_path(name, prefix, partial) query(path, EXTENSION_ORDER.map { |ext| details[ext] }, details[:formats]) end @@ -96,17 +114,24 @@ module ActionView contents = File.open(p, "rb") {|io| io.read } Template.new(contents, File.expand_path(p), handler, - :virtual_path => path, :format => format) + :virtual_path => path, :format => format, :updated_at => mtime(p)) end end + # Returns the file mtime from the filesystem. + def mtime(p) + File.stat(p).mtime + end + # Extract handler and formats from path. If a format cannot be a found neither # from the path, or the handler, we should return the array of formats given # to the resolver. def extract_handler_and_format(path, default_formats) pieces = File.basename(path).split(".") pieces.shift - retrieve_handler_and_format(pieces.pop, pieces.pop, default_formats) + handler = Template.handler_class_for_extension(pieces.pop) + format = pieces.last && Mime[pieces.last] + [handler, format] end end diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index ec0db48379..55583096e0 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -23,11 +23,12 @@ module ActionView #:nodoc: query = /^(#{Regexp.escape(path)})#{query}$/ templates = [] - @hash.each do |_path, source| + @hash.each do |_path, array| + source, updated_at = array next unless _path =~ query handler, format = extract_handler_and_format(_path, formats) templates << Template.new(source, _path, handler, - :virtual_path => $1, :format => format) + :virtual_path => $1, :format => format, :updated_at => updated_at) end templates.sort_by {|t| -t.identifier.match(/^#{query}$/).captures.reject(&:blank?).size } diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index 55d581e512..23dfc1ba75 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -184,4 +184,49 @@ class LookupContextTest < ActiveSupport::TestCase test "can have cache disabled on initialization" do assert !ActionView::LookupContext.new(FIXTURE_LOAD_PATH, :cache => false).cache end +end + +class LookupContextWithFalseCaching < ActiveSupport::TestCase + def setup + @resolver = ActionView::FixtureResolver.new("test/_foo.erb" => ["Foo", Time.utc(2000)]) + @resolver.stubs(:caching?).returns(false) + @lookup_context = ActionView::LookupContext.new(@resolver, {}) + end + + test "templates are always found in the resolver but timestamp is checked before being compiled" do + template = @lookup_context.find("foo", "test", true) + assert_equal "Foo", template.source + + # Now we are going to change the template, but it won't change the returned template + # since the timestamp is the same. + @resolver.hash["test/_foo.erb"][0] = "Bar" + template = @lookup_context.find("foo", "test", true) + assert_equal "Foo", template.source + + # Now update the timestamp. + @resolver.hash["test/_foo.erb"][1] = Time.now.utc + template = @lookup_context.find("foo", "test", true) + assert_equal "Bar", template.source + end + + test "if no template was found in the second lookup, give it higher preference" do + template = @lookup_context.find("foo", "test", true) + assert_equal "Foo", template.source + + @resolver.hash.clear + assert_raise ActionView::MissingTemplate do + @lookup_context.find("foo", "test", true) + end + end + + test "if no template was cached in the first lookup, do not use the cache in the second" do + @resolver.hash.clear + assert_raise ActionView::MissingTemplate do + @lookup_context.find("foo", "test", true) + end + + @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] + template = @lookup_context.find("foo", "test", true) + assert_equal "Foo", template.source + end end \ No newline at end of file -- cgit v1.2.3 From 8cb2cfbf71092f95090335cbdde0340cc74db748 Mon Sep 17 00:00:00 2001 From: wycats Date: Sun, 10 Oct 2010 00:51:52 -0700 Subject: Fix a few bugs when trying to use Head standalone --- actionpack/lib/abstract_controller/base.rb | 1 + actionpack/lib/action_controller/metal.rb | 5 +++++ actionpack/lib/action_controller/metal/head.rb | 6 ++---- actionpack/test/controller/new_base/bare_metal_test.rb | 15 +++++++++++++++ 4 files changed, 23 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index f9f6eb945e..f83eaded88 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -13,6 +13,7 @@ module AbstractController class Base attr_internal :response_body attr_internal :action_name + attr_internal :formats include ActiveSupport::Configurable extend ActiveSupport::DescendantsTracker diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index ace1aabe03..329798e84f 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -119,6 +119,11 @@ module ActionController headers["Location"] = url end + # basic url_for that can be overridden for more robust functionality + def url_for(string) + string + end + def status @_status end diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 2b4a3b9392..8abcad55a2 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -2,8 +2,6 @@ module ActionController module Head extend ActiveSupport::Concern - include ActionController::UrlFor - # Return a response that has no content (merely headers). The options # argument is interpreted to be a hash of header names and values. # This allows you to easily return a response that consists only of @@ -27,8 +25,8 @@ module ActionController self.status = status self.location = url_for(location) if location - self.content_type = Mime[formats.first] + self.content_type = Mime[formats.first] if formats self.response_body = " " end end -end \ No newline at end of file +end diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index 8a06e8d2a0..44922cecff 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -24,4 +24,19 @@ module BareMetalTest assert_equal "Hello world", string end end + + class HeadController < ActionController::Metal + include ActionController::Head + + def index + head :not_found + end + end + + class HeadTest < ActiveSupport::TestCase + test "head works on its own" do + status, headers, body = HeadController.action(:index).call(Rack::MockRequest.env_for("/")) + assert_equal 404, status + end + end end -- cgit v1.2.3 From b3cadf338b948640e2203d677cc2b19fd2422ae8 Mon Sep 17 00:00:00 2001 From: wycats Date: Sun, 10 Oct 2010 00:52:43 -0700 Subject: Rendering doesn't need RackDelegation --- actionpack/lib/action_controller/metal/rendering.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 86bb810947..e524e546ad 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -2,7 +2,6 @@ module ActionController module Rendering extend ActiveSupport::Concern - include ActionController::RackDelegation include AbstractController::Rendering # Before processing, set the request formats in current controller formats. -- cgit v1.2.3 From ab2f2b22a6d0656f719c294d40e35d21c752ba8c Mon Sep 17 00:00:00 2001 From: David Calavera Date: Fri, 8 Oct 2010 18:16:40 +0530 Subject: prevent rake test to run the test suite three times when ENV['TEST'] is set [#3572 state:resolved] Signed-off-by: Xavier Noria --- actionpack/RUNNING_UNIT_TESTS | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/RUNNING_UNIT_TESTS b/actionpack/RUNNING_UNIT_TESTS index d6a1ccf871..1e3ba7abe7 100644 --- a/actionpack/RUNNING_UNIT_TESTS +++ b/actionpack/RUNNING_UNIT_TESTS @@ -8,15 +8,18 @@ Rake can be found at http://rake.rubyforge.org == Running by hand -If you only want to run a single test suite, or don't want to bother with Rake, -you can do so with something like: +To run a single test suite - ruby -Itest test/controller/base_tests.rb + rake test TEST=path/to/test.rb -== Dependency on ActiveRecord and database setup +which can be further narrowed down to one test: + + rake test TEST=path/to/test.rb TESTOPTS="--name=test_something" + +== Dependency on Active Record and database setup Test cases in the test/controller/active_record/ directory depend on having -activerecord and sqlite installed. If ActiveRecord is not in +activerecord and sqlite installed. If Active Record is not in actionpack/../activerecord directory, or the sqlite rubygem is not installed, these tests are skipped. -- cgit v1.2.3 From b88f4ca93bcaef9a6bfd21d95acc8f432a3c8e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 11:03:18 +0200 Subject: Clean up the house before moving in the new furniture. This commit moves all the template rendering logic that was hanging around AV::Base to renderer objects. --- actionpack/lib/action_view.rb | 8 +- actionpack/lib/action_view/base.rb | 9 +- .../lib/action_view/helpers/prototype_helper.rb | 2 +- actionpack/lib/action_view/render/layouts.rb | 83 ---------- actionpack/lib/action_view/render/partials.rb | 177 +-------------------- actionpack/lib/action_view/render/rendering.rb | 101 +++++++----- .../lib/action_view/renderer/abstract_renderer.rb | 36 +++++ .../lib/action_view/renderer/partial_renderer.rb | 167 +++++++++++++++++++ .../lib/action_view/renderer/template_renderer.rb | 66 ++++++++ actionpack/lib/action_view/template.rb | 5 +- 10 files changed, 348 insertions(+), 306 deletions(-) delete mode 100644 actionpack/lib/action_view/render/layouts.rb create mode 100644 actionpack/lib/action_view/renderer/abstract_renderer.rb create mode 100644 actionpack/lib/action_view/renderer/partial_renderer.rb create mode 100644 actionpack/lib/action_view/renderer/template_renderer.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 5f9dc70766..ad96f6c66d 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -38,11 +38,17 @@ module ActionView autoload :Helpers autoload :Base autoload :LookupContext + autoload :Render autoload :PathSet, "action_view/paths" autoload :TestCase, "action_view/test_case" + autoload_under "renderer" do + autoload :AbstractRenderer + autoload :PartialRenderer + autoload :TemplateRenderer + end + autoload_under "render" do - autoload :Layouts autoload :Partials autoload :Rendering end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 0bef3e3a08..2a601c7cee 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -158,7 +158,7 @@ module ActionView #:nodoc: module Subclasses end - include Helpers, Rendering, Partials, Layouts, ::ERB::Util, Context + include Helpers, Rendering, Partials, ::ERB::Util, Context # Specify whether RJS responses should be wrapped in a try/catch block # that alert()s the caught exception (and then re-raises it). @@ -180,8 +180,7 @@ module ActionView #:nodoc: attr_accessor :base_path, :assigns, :template_extension, :lookup_context attr_internal :captures, :request, :controller, :template, :config - delegate :find_template, :template_exists?, :formats, :formats=, :locale, :locale=, - :view_paths, :view_paths=, :with_fallbacks, :update_details, :with_layout_format, :to => :lookup_context + delegate :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, :to => :lookup_context delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, :flash, :action_name, :controller_name, :to => :controller @@ -220,6 +219,10 @@ module ActionView #:nodoc: @lookup_context.formats = formats if formats end + def store_content_for(key, value) + @_content_for[key] = value + end + def controller_path @controller_path ||= controller && controller.controller_path end diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index 300207dfac..41cd8d5171 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -546,7 +546,7 @@ module ActionView end def with_formats(*args) - @context ? @context.update_details(:formats => args) { yield } : yield + @context ? @context.lookup_context.update_details(:formats => args) { yield } : yield end def javascript_object_for(object) diff --git a/actionpack/lib/action_view/render/layouts.rb b/actionpack/lib/action_view/render/layouts.rb deleted file mode 100644 index 08162f7fd5..0000000000 --- a/actionpack/lib/action_view/render/layouts.rb +++ /dev/null @@ -1,83 +0,0 @@ -module ActionView - # = Action View Layouts - module Layouts - # Returns the contents that are yielded to a layout, given a name or a block. - # - # You can think of a layout as a method that is called with a block. If the user calls - # yield :some_name, the block, by default, returns content_for(:some_name). - # If the user calls simply +yield+, the default block returns content_for(:layout). - # - # The user can override this default by passing a block to the layout: - # - # # The template - # <%= render :layout => "my_layout" do %> - # Content - # <% end %> - # - # # The layout - # - # <%= yield %> - # - # - # In this case, instead of the default block, which would return content_for(:layout), - # this method returns the block that was passed in to render :layout, and the response - # would be - # - # - # Content - # - # - # Finally, the block can take block arguments, which can be passed in by +yield+: - # - # # The template - # <%= render :layout => "my_layout" do |customer| %> - # Hello <%= customer.name %> - # <% end %> - # - # # The layout - # - # <%= yield Struct.new(:name).new("David") %> - # - # - # In this case, the layout would receive the block passed into render :layout, - # and the struct specified would be passed into the block as an argument. The result - # would be - # - # - # Hello David - # - # - def _layout_for(*args, &block) #:nodoc: - name = args.first - - if name.is_a?(Symbol) - @_content_for[name].html_safe - elsif block - capture(*args, &block) - else - @_content_for[:layout].html_safe - end - end - - # This is the method which actually finds the layout using details in the lookup - # context object. If no layout is found, it checks if at least a layout with - # the given name exists across all details before raising the error. - def find_layout(layout, keys) - begin - with_layout_format do - layout =~ /^\// ? - with_fallbacks { find_template(layout, nil, false, keys) } : find_template(layout, nil, false, keys) - end - rescue ActionView::MissingTemplate => e - update_details(:formats => nil) do - raise unless template_exists?(layout) - end - end - end - - # Contains the logic that actually renders the layout. - def _render_layout(layout, locals, &block) #:nodoc: - layout.render(self, locals){ |*name| _layout_for(*name, &block) } - end - end -end diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 4e03d43358..844c3e9572 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -210,181 +210,12 @@ module ActionView # <%- end -%> # <% end %> module Partials - extend ActiveSupport::Concern - - class PartialRenderer - PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} } - - def initialize(view_context, options, block) - @view = view_context - @partial_names = PARTIAL_NAMES[@view.controller.class.name] - setup(options, block) - end - - def setup(options, block) - partial = options[:partial] - - @options = options - @locals = options[:locals] || {} - @block = block - - if String === partial - @object = options[:object] - @path = partial - @collection = collection - else - @object = partial - - if @collection = collection_from_object || collection - paths = @collection_data = @collection.map { |o| partial_path(o) } - @path = paths.uniq.size == 1 ? paths.first : nil - else - @path = partial_path - end - end - - if @path - @variable, @variable_counter = retrieve_variable(@path) - else - paths.map! { |path| retrieve_variable(path).unshift(path) } - end - end - - def retrieve_variable(path) - variable = @options[:as] || path[%r'_?(\w+)(\.\w+)*$', 1].to_sym - variable_counter = :"#{variable}_counter" if @collection - [variable, variable_counter] - end - - def render - identifier = ((@template = find_partial) ? @template.identifier : @path) - - if @collection - ActiveSupport::Notifications.instrument("render_collection.action_view", - :identifier => identifier || "collection", :count => @collection.size) do - render_collection - end - else - if !@block && (layout = @options[:layout]) - layout = find_template(layout) - end - - content = ActiveSupport::Notifications.instrument("render_partial.action_view", - :identifier => identifier) do - render_partial - end - - content = @view._render_layout(layout, @locals){ content } if layout - content - end - end - - def render_collection - return nil if @collection.blank? - - if @options.key?(:spacer_template) - spacer = find_template(@options[:spacer_template]).render(@view, @locals) - end - - result = @template ? collection_with_template : collection_without_template - result.join(spacer).html_safe - end - - def collection_with_template - segments, locals, template = [], @locals, @template - as, counter = @variable, @variable_counter - - locals[counter] = -1 - - @collection.each do |object| - locals[counter] += 1 - locals[as] = object - segments << template.render(@view, locals) - end - - segments - end - - def collection_without_template - segments, locals, collection_data = [], @locals, @collection_data - index, template, cache = -1, nil, {} - keys = @locals.keys - - @collection.each_with_index do |object, i| - path, *data = collection_data[i] - template = (cache[path] ||= find_template(path, keys + data)) - locals[data[0]] = object - locals[data[1]] = (index += 1) - segments << template.render(@view, locals) - end - - @template = template - segments - end - - def render_partial - locals, view = @locals, @view - object, as = @object, @variable - - object ||= locals[as] - locals[as] = object - - @template.render(view, locals) do |*name| - view._layout_for(*name, &@block) - end - end - - private - - def collection - if @options.key?(:collection) - @options[:collection] || [] - end - end - - def collection_from_object - if @object.respond_to?(:to_ary) - @object - end - end - - def find_partial - if path = @path - locals = @locals.keys - locals << @variable - locals << @variable_counter if @collection - find_template(path, locals) - end - end - - def find_template(path=@path, locals=@locals.keys) - prefix = @view.controller_path unless path.include?(?/) - @view.find_template(path, prefix, true, locals) - end - - def partial_path(object = @object) - @partial_names[object.class.name] ||= begin - object = object.to_model if object.respond_to?(:to_model) - - object.class.model_name.partial_path.dup.tap do |partial| - path = @view.controller_path - partial.insert(0, "#{File.dirname(path)}/") if partial.include?(?/) && path.include?(?/) - end - end - end - end - def _render_partial(options, &block) #:nodoc: - _wrap_formats(options[:partial]) do - if defined?(@renderer) - @renderer.setup(options, block) - else - @renderer = PartialRenderer.new(self, options, block) - end - - @renderer.render - end + _partial_renderer.setup(options, block).render end + def _partial_renderer #:nodoc: + @_partial_renderer ||= PartialRenderer.new(self) + end end end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 8e599c71df..adbb6bc626 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -21,11 +21,7 @@ module ActionView elsif options.key?(:partial) _render_partial(options) else - _wrap_formats(options[:template] || options[:file]) do - template = _determine_template(options) - lookup_context.freeze_formats(template.formats, true) - _render_template(template, options[:layout], options) - end + _render_template(options) end when :update update_page(&block) @@ -34,51 +30,70 @@ module ActionView end end - # Checks if the given path contains a format and if so, change - # the lookup context to take this new format into account. - def _wrap_formats(value) - return yield unless value.is_a?(String) - @@formats_regexp ||= /\.(#{Mime::SET.symbols.join('|')})$/ + # Returns the contents that are yielded to a layout, given a name or a block. + # + # You can think of a layout as a method that is called with a block. If the user calls + # yield :some_name, the block, by default, returns content_for(:some_name). + # If the user calls simply +yield+, the default block returns content_for(:layout). + # + # The user can override this default by passing a block to the layout: + # + # # The template + # <%= render :layout => "my_layout" do %> + # Content + # <% end %> + # + # # The layout + # + # <%= yield %> + # + # + # In this case, instead of the default block, which would return content_for(:layout), + # this method returns the block that was passed in to render :layout, and the response + # would be + # + # + # Content + # + # + # Finally, the block can take block arguments, which can be passed in by +yield+: + # + # # The template + # <%= render :layout => "my_layout" do |customer| %> + # Hello <%= customer.name %> + # <% end %> + # + # # The layout + # + # <%= yield Struct.new(:name).new("David") %> + # + # + # In this case, the layout would receive the block passed into render :layout, + # and the struct specified would be passed into the block as an argument. The result + # would be + # + # + # Hello David + # + # + def _layout_for(*args, &block) + name = args.first - if value.sub!(@@formats_regexp, "") - update_details(:formats => [$1.to_sym]){ yield } + if name.is_a?(Symbol) + @_content_for[name].html_safe + elsif block + capture(*args, &block) else - yield + @_content_for[:layout].html_safe end end - # Determine the template to be rendered using the given options. - def _determine_template(options) #:nodoc: - keys = (options[:locals] ||= {}).keys - - if options.key?(:inline) - handler = Template.handler_class_for_extension(options[:type] || "erb") - Template.new(options[:inline], "inline template", handler, { :locals => keys }) - elsif options.key?(:text) - Template::Text.new(options[:text], formats.try(:first)) - elsif options.key?(:file) - with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } - elsif options.key?(:template) - options[:template].respond_to?(:render) ? - options[:template] : find_template(options[:template], options[:prefix], false, keys) - end + def _render_template(options) #:nodoc: + _template_renderer.render(options) end - # Renders the given template. An string representing the layout can be - # supplied as well. - def _render_template(template, layout = nil, options = {}) #:nodoc: - locals = options[:locals] || {} - layout = find_layout(layout, locals.keys) if layout - - ActiveSupport::Notifications.instrument("render_template.action_view", - :identifier => template.identifier, :layout => layout.try(:virtual_path)) do - - content = template.render(self, locals) { |*name| _layout_for(*name) } - @_content_for[:layout] = content if layout - - content = _render_layout(layout, locals) if layout - content - end + def _template_renderer #:nodoc: + @_template_renderer ||= TemplateRenderer.new(self) end end end diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb new file mode 100644 index 0000000000..f9fa63ce7f --- /dev/null +++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb @@ -0,0 +1,36 @@ +module ActionView + class AbstractRenderer #:nodoc: + attr_reader :vew, :lookup_context + + delegate :find_template, :template_exists?, :with_fallbacks, :update_details, + :with_layout_format, :formats, :to => :lookup_context + + def initialize(view) + @view = view + @lookup_context = view.lookup_context + end + + def render + raise NotImplementedError + end + + # Contains the logic that actually renders the layout. + def render_layout(layout, locals, &block) #:nodoc: + view = @view + layout.render(view, locals){ |*name| view._layout_for(*name, &block) } + end + + # Checks if the given path contains a format and if so, change + # the lookup context to take this new format into account. + def wrap_formats(value) + return yield unless value.is_a?(String) + @@formats_regexp ||= /\.(#{Mime::SET.symbols.join('|')})$/ + + if value.sub!(@@formats_regexp, "") + update_details(:formats => [$1.to_sym]){ yield } + else + yield + end + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb new file mode 100644 index 0000000000..3be1702f9e --- /dev/null +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -0,0 +1,167 @@ +require 'action_view/renderer/abstract_renderer' + +module ActionView + class PartialRenderer < AbstractRenderer #:nodoc: + N = ::ActiveSupport::Notifications + PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} } + + def initialize(view) + super + @partial_names = PARTIAL_NAMES[@view.controller.class.name] + end + + def setup(options, block) + partial = options[:partial] + + @options = options + @locals = options[:locals] || {} + @block = block + + if String === partial + @object = options[:object] + @path = partial + @collection = collection + else + @object = partial + + if @collection = collection_from_object || collection + paths = @collection_data = @collection.map { |o| partial_path(o) } + @path = paths.uniq.size == 1 ? paths.first : nil + else + @path = partial_path + end + end + + if @path + @variable, @variable_counter = retrieve_variable(@path) + else + paths.map! { |path| retrieve_variable(path).unshift(path) } + end + + self + end + + def render + wrap_formats(@path) do + identifier = ((@template = find_partial) ? @template.identifier : @path) + + if @collection + N.instrument("render_collection.action_view", :identifier => identifier || "collection", :count => @collection.size) do + render_collection + end + else + N.instrument("render_partial.action_view", :identifier => identifier) do + render_partial + end + end + end + end + + def render_collection + return nil if @collection.blank? + + if @options.key?(:spacer_template) + spacer = find_template(@options[:spacer_template]).render(@view, @locals) + end + + result = @template ? collection_with_template : collection_without_template + result.join(spacer).html_safe + end + + def render_partial + locals, view, block = @locals, @view, @block + object, as = @object, @variable + + if !block && (layout = @options[:layout]) + layout = find_template(layout) + end + + object ||= locals[as] + locals[as] = object + + content = @template.render(view, locals) do |*name| + view._layout_for(*name, &block) + end + + content = render_layout(layout, locals){ content } if layout + content + end + + private + + def collection + if @options.key?(:collection) + @options[:collection] || [] + end + end + + def collection_from_object + if @object.respond_to?(:to_ary) + @object + end + end + + def find_partial + if path = @path + locals = @locals.keys + locals << @variable + locals << @variable_counter if @collection + find_template(path, locals) + end + end + + def find_template(path=@path, locals=@locals.keys) + prefix = @view.controller_path unless path.include?(?/) + @lookup_context.find_template(path, prefix, true, locals) + end + + def collection_with_template + segments, locals, template = [], @locals, @template + as, counter = @variable, @variable_counter + + locals[counter] = -1 + + @collection.each do |object| + locals[counter] += 1 + locals[as] = object + segments << template.render(@view, locals) + end + + segments + end + + def collection_without_template + segments, locals, collection_data = [], @locals, @collection_data + index, template, cache = -1, nil, {} + keys = @locals.keys + + @collection.each_with_index do |object, i| + path, *data = collection_data[i] + template = (cache[path] ||= find_template(path, keys + data)) + locals[data[0]] = object + locals[data[1]] = (index += 1) + segments << template.render(@view, locals) + end + + @template = template + segments + end + + def partial_path(object = @object) + @partial_names[object.class.name] ||= begin + object = object.to_model if object.respond_to?(:to_model) + + object.class.model_name.partial_path.dup.tap do |partial| + path = @view.controller_path + partial.insert(0, "#{File.dirname(path)}/") if partial.include?(?/) && path.include?(?/) + end + end + end + + def retrieve_variable(path) + variable = @options[:as] || path[%r'_?(\w+)(\.\w+)*$', 1].to_sym + variable_counter = :"#{variable}_counter" if @collection + [variable, variable_counter] + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb new file mode 100644 index 0000000000..3acc68dcac --- /dev/null +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -0,0 +1,66 @@ +require 'action_view/renderer/abstract_renderer' + +module ActionView + class TemplateRenderer < AbstractRenderer #:nodoc: + def render(options) + wrap_formats(options[:template] || options[:file]) do + template = determine_template(options) + lookup_context.freeze_formats(template.formats, true) + render_template(template, options[:layout], options) + end + end + + # Determine the template to be rendered using the given options. + def determine_template(options) #:nodoc: + keys = (options[:locals] ||= {}).keys + + if options.key?(:inline) + handler = Template.handler_class_for_extension(options[:type] || "erb") + Template.new(options[:inline], "inline template", handler, { :locals => keys }) + elsif options.key?(:text) + Template::Text.new(options[:text], formats.try(:first)) + elsif options.key?(:file) + with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } + elsif options.key?(:template) + options[:template].respond_to?(:render) ? + options[:template] : find_template(options[:template], options[:prefix], false, keys) + end + end + + # Renders the given template. An string representing the layout can be + # supplied as well. + def render_template(template, layout = nil, options = {}) #:nodoc: + view, locals = @view, options[:locals] || {} + layout = find_layout(layout, locals.keys) if layout + + ActiveSupport::Notifications.instrument("render_template.action_view", + :identifier => template.identifier, :layout => layout.try(:virtual_path)) do + + content = template.render(view, locals) { |*name| view._layout_for(*name) } + + if layout + view.store_content_for(:layout, content) + content = render_layout(layout, locals) + end + + content + end + end + + # This is the method which actually finds the layout using details in the lookup + # context object. If no layout is found, it checks if at least a layout with + # the given name exists across all details before raising the error. + def find_layout(layout, keys) + begin + with_layout_format do + layout =~ /^\// ? + with_fallbacks { find_template(layout, nil, false, keys) } : find_template(layout, nil, false, keys) + end + rescue ActionView::MissingTemplate => e + update_details(:formats => nil) do + raise unless template_exists?(layout) + end + end + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 3cc9bb2710..074daa5d28 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -155,11 +155,12 @@ module ActionView # virtual path set (true just for inline templates). def refresh(view) raise "A template need to have a virtual path in order to be refreshed" unless @virtual_path + lookup = view.lookup_context pieces = @virtual_path.split("/") name = pieces.pop partial = name.sub!(/^_/, "") - view.lookup_context.disable_cache do - view.find_template(name, pieces.join, partial || false, @locals) + lookup.disable_cache do + lookup.find_template(name, pieces.join, partial || false, @locals) end end -- cgit v1.2.3 From 940b57789fb9166658974c591e68d22ecab29f34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 12:34:31 +0200 Subject: Add support to render :once. This will be used internally by sprockets to ensure requires are executed just once. --- actionpack/lib/abstract_controller/rendering.rb | 2 +- actionpack/lib/action_view/render/rendering.rb | 7 +++ .../lib/action_view/renderer/abstract_renderer.rb | 12 ++-- .../lib/action_view/renderer/partial_renderer.rb | 7 +-- .../lib/action_view/renderer/template_renderer.rb | 67 ++++++++++++++------ .../test/controller/new_base/render_once_test.rb | 73 ++++++++++++++++++++++ 6 files changed, 139 insertions(+), 29 deletions(-) create mode 100644 actionpack/test/controller/new_base/render_once_test.rb (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index e88e5aefc0..1c63fb2d14 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -155,7 +155,7 @@ module AbstractController options[:partial] = action_name end - if (options.keys & [:partial, :file, :template]).empty? + if (options.keys & [:partial, :file, :template, :once]).empty? options[:prefix] ||= _prefix end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index adbb6bc626..0cd5d9d437 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -10,6 +10,7 @@ module ActionView # * :file - Renders an explicit template file (this used to be the old default), add :locals to pass in those. # * :inline - Renders an inline template similar to how it's done in the controller. # * :text - Renders the text passed in out. + # * :once - Receives :template paths and ensures they are rendered just once. # # If no options hash is passed or :update specified, the default is to render a partial and use the second parameter # as the locals hash. @@ -20,6 +21,8 @@ module ActionView _render_partial(options.merge(:partial => options[:layout]), &block) elsif options.key?(:partial) _render_partial(options) + elsif options.key?(:once) + _render_once(options) else _render_template(options) end @@ -88,6 +91,10 @@ module ActionView end end + def _render_once(options) #:nodoc: + _template_renderer.render_once(options) + end + def _render_template(options) #:nodoc: _template_renderer.render(options) end diff --git a/actionpack/lib/action_view/renderer/abstract_renderer.rb b/actionpack/lib/action_view/renderer/abstract_renderer.rb index f9fa63ce7f..77cfa51dff 100644 --- a/actionpack/lib/action_view/renderer/abstract_renderer.rb +++ b/actionpack/lib/action_view/renderer/abstract_renderer.rb @@ -14,12 +14,6 @@ module ActionView raise NotImplementedError end - # Contains the logic that actually renders the layout. - def render_layout(layout, locals, &block) #:nodoc: - view = @view - layout.render(view, locals){ |*name| view._layout_for(*name, &block) } - end - # Checks if the given path contains a format and if so, change # the lookup context to take this new format into account. def wrap_formats(value) @@ -32,5 +26,11 @@ module ActionView yield end end + + protected + + def instrument(name, options={}) + ActiveSupport::Notifications.instrument("render_#{name}.action_view", options){ yield } + end end end \ No newline at end of file diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 3be1702f9e..eff425687b 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -2,7 +2,6 @@ require 'action_view/renderer/abstract_renderer' module ActionView class PartialRenderer < AbstractRenderer #:nodoc: - N = ::ActiveSupport::Notifications PARTIAL_NAMES = Hash.new {|h,k| h[k] = {} } def initialize(view) @@ -46,11 +45,11 @@ module ActionView identifier = ((@template = find_partial) ? @template.identifier : @path) if @collection - N.instrument("render_collection.action_view", :identifier => identifier || "collection", :count => @collection.size) do + instrument(:collection, :identifier => identifier || "collection", :count => @collection.size) do render_collection end else - N.instrument("render_partial.action_view", :identifier => identifier) do + instrument(:partial, :identifier => identifier) do render_partial end end @@ -83,7 +82,7 @@ module ActionView view._layout_for(*name, &block) end - content = render_layout(layout, locals){ content } if layout + content = layout.render(view, locals){ content } if layout content end diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index 3acc68dcac..9f9df15347 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -1,26 +1,51 @@ +require 'set' +require 'active_support/core_ext/object/try' +require 'active_support/core_ext/array/wrap' require 'action_view/renderer/abstract_renderer' module ActionView class TemplateRenderer < AbstractRenderer #:nodoc: + attr_reader :rendered + + def initialize(view) + super + @rendered = Set.new + end + def render(options) wrap_formats(options[:template] || options[:file]) do template = determine_template(options) - lookup_context.freeze_formats(template.formats, true) - render_template(template, options[:layout], options) + render_template(template, options[:layout], options[:locals]) + end + end + + def render_once(options) + paths, locals = options[:once], options[:locals] || {} + layout, keys, prefix = options[:layout], locals.keys, options[:prefix] + + raise "render :once expects a String or an Array to be given" unless paths + + render_with_layout(layout, locals) do + contents = [] + Array.wrap(paths).each do |path| + template = find_template(path, prefix, false, keys) + contents << render_template(template, nil, locals) if @rendered.add?(template) + end + contents.join("\n") end end # Determine the template to be rendered using the given options. def determine_template(options) #:nodoc: - keys = (options[:locals] ||= {}).keys + keys = options[:locals].try(:keys) || [] - if options.key?(:inline) - handler = Template.handler_class_for_extension(options[:type] || "erb") - Template.new(options[:inline], "inline template", handler, { :locals => keys }) - elsif options.key?(:text) + if options.key?(:text) Template::Text.new(options[:text], formats.try(:first)) elsif options.key?(:file) with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } + elsif options.key?(:inline) + handler = Template.handler_class_for_extension(options[:type] || "erb") + Template.new(options[:inline], "inline template", handler, { :locals => keys }) elsif options.key?(:template) options[:template].respond_to?(:render) ? options[:template] : find_template(options[:template], options[:prefix], false, keys) @@ -29,20 +54,26 @@ module ActionView # Renders the given template. An string representing the layout can be # supplied as well. - def render_template(template, layout = nil, options = {}) #:nodoc: - view, locals = @view, options[:locals] || {} - layout = find_layout(layout, locals.keys) if layout + def render_template(template, layout_name = nil, locals = {}) #:nodoc: + lookup_context.freeze_formats(template.formats, true) + view, locals = @view, locals || {} - ActiveSupport::Notifications.instrument("render_template.action_view", - :identifier => template.identifier, :layout => layout.try(:virtual_path)) do - - content = template.render(view, locals) { |*name| view._layout_for(*name) } - - if layout - view.store_content_for(:layout, content) - content = render_layout(layout, locals) + render_with_layout(layout_name, locals) do |layout| + instrument(:template, :identifier => template.identifier, :layout => layout.try(:virtual_path)) do + template.render(view, locals) { |*name| view._layout_for(*name) } end + end + end + + def render_with_layout(path, locals) #:nodoc: + layout = path && find_layout(path, locals.keys) + content = yield(layout) + if layout + view = @view + view.store_content_for(:layout, content) + layout.render(view, locals){ |*name| view._layout_for(*name) } + else content end end diff --git a/actionpack/test/controller/new_base/render_once_test.rb b/actionpack/test/controller/new_base/render_once_test.rb new file mode 100644 index 0000000000..12892b7255 --- /dev/null +++ b/actionpack/test/controller/new_base/render_once_test.rb @@ -0,0 +1,73 @@ +require 'abstract_unit' + +module RenderTemplate + class RenderOnceController < ActionController::Base + layout false + + RESOLVER = ActionView::FixtureResolver.new( + "test/a.html.erb" => "a", + "test/b.html.erb" => "<>", + "test/c.html.erb" => "c", + "test/one.html.erb" => "<%= render :once => 'test/result' %>", + "test/two.html.erb" => "<%= render :once => 'test/result' %>", + "test/three.html.erb" => "<%= render :once => 'test/result' %>", + "test/result.html.erb" => "YES!", + "layouts/test.html.erb" => "l<%= yield %>l" + ) + + self.view_paths = [RESOLVER] + + def multiple + render :once => %w(test/a test/b test/c) + end + + def once + render :once => %w(test/one test/two test/three) + end + + def duplicate + render :once => %w(test/a test/a test/a) + end + + def with_layout + render :once => %w(test/a test/b test/c), :layout => "test" + end + end + + module Tests + def test_mutliple_arguments_get_all_rendered + get :multiple + assert_response "a\n<>\nc" + end + + def test_referenced_templates_get_rendered_once + get :once + assert_response "YES!\n\n" + end + + def test_duplicated_templates_get_rendered_once + get :duplicate + assert_response "a" + end + + def test_layout_wraps_all_rendered_templates + get :with_layout + assert_response "la\n<>\ncl" + end + end + + class TestWithResolverCache < Rack::TestCase + testing RenderTemplate::RenderOnceController + include Tests + end + + # TODO This still needs to be implemented and supported. + # class TestWithoutResolverCache < Rack::TestCase + # testing RenderTemplate::RenderOnceController + # include Tests + # + # def setup + # RenderTemplate::RenderOnceController::RESOLVER.stubs(:caching?).returns(false) + # end + # end +end -- cgit v1.2.3 From cba395dab9812d86d48cc0574061cd6dcd18009b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 12:40:47 +0200 Subject: Update CHANGELOG. --- actionpack/CHANGELOG | 8 ++++++++ actionpack/lib/action_view/render/rendering.rb | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6352b97a6b..3f8188b1f7 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,13 @@ *Rails 3.1.0 (unreleased)* +* Added render :once. You can pass either a string or an array of strings and Rails will ensure they each of them are rendered just once. [José Valim] + +* Deprecate old template handler API. The new API simply requires a template handler to respond to call. [José Valim] + +* :rhtml and :rxml were finally removed as template handlers. [José Valim] + +* Moved etag responsibility from ActionDispatch::Response to the middleware stack. [José Valim] + * Rely on Rack::Session stores API for more compatibility across the Ruby world. This is backwards incompatible since Rack::Session expects #get_session to accept 4 arguments and requires #destroy_session instead of simply #destroy. [José Valim] * file_field automatically adds :multipart => true to the enclosing form. [Santiago Pastorino] diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 0cd5d9d437..baa5d2c3fd 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -10,7 +10,7 @@ module ActionView # * :file - Renders an explicit template file (this used to be the old default), add :locals to pass in those. # * :inline - Renders an inline template similar to how it's done in the controller. # * :text - Renders the text passed in out. - # * :once - Receives :template paths and ensures they are rendered just once. + # * :once - Accepts a string or an array of strings and Rails will ensure they each of them are rendered just once. # # If no options hash is passed or :update specified, the default is to render a partial and use the second parameter # as the locals hash. -- cgit v1.2.3 From ffa32714bd339ae32355a6827750e1af81454a1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 13:17:01 +0200 Subject: Add some unit tests to Template#refresh. --- actionpack/test/template/template_test.rb | 32 +++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index eb2d4aab36..34679962b2 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -1,9 +1,16 @@ require "abstract_unit" +require "logger" class TestERBTemplate < ActiveSupport::TestCase ERBHandler = ActionView::Template::Handlers::ERB.new class Context + class LookupContext + def disable_cache + yield + end + end + def initialize @output_buffer = "original" @_virtual_path = nil @@ -22,8 +29,11 @@ class TestERBTemplate < ActiveSupport::TestCase ) end + def lookup_context + @lookup_context ||= LookupContext.new + end + def logger - require "logger" Logger.new(STDERR) end @@ -37,11 +47,11 @@ class TestERBTemplate < ActiveSupport::TestCase end def render(locals = {}) - @template.render(@obj, locals) + @template.render(@context, locals) end def setup - @obj = Context.new + @context = Context.new end def test_basic_template @@ -70,7 +80,7 @@ class TestERBTemplate < ActiveSupport::TestCase def test_restores_buffer @template = new_template assert_equal "Hello", render - assert_equal "original", @obj.my_buffer + assert_equal "original", @context.my_buffer end def test_virtual_path @@ -80,6 +90,20 @@ class TestERBTemplate < ActiveSupport::TestCase assert_equal "hellopartialhello", render end + def test_refresh + @template = new_template("Hello", :virtual_path => "test/foo") + @template.locals = [:key] + @context.lookup_context.expects(:find_template).with("foo", "test", false, [:key]).returns("template") + assert_equal "template", @template.refresh(@context) + end + + def test_refresh_raises_an_error_without_virtual_path + @template = new_template("Hello", :virtual_path => nil) + assert_raise RuntimeError, /OMG/ do + @template.refresh(@context) + end + end + if "ruby".encoding_aware? def test_resulting_string_is_utf8 @template = new_template -- cgit v1.2.3 From 11aa5157355d06ddbc9cad8fd0aa43a75ac8431e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 13:43:32 +0200 Subject: Add expire! and rerender to the template API. This will be used by SASS template handler. --- actionpack/lib/action_view/template.rb | 23 +++++++++++++-- actionpack/test/template/template_test.rb | 49 +++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 5 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 074daa5d28..5c3e0e899b 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -154,13 +154,30 @@ module ActionView # Notice this method raises an error if the template to be refreshed does not have a # virtual path set (true just for inline templates). def refresh(view) - raise "A template need to have a virtual path in order to be refreshed" unless @virtual_path + raise "A template needs to have a virtual path in order to be refreshed" unless @virtual_path lookup = view.lookup_context pieces = @virtual_path.split("/") name = pieces.pop - partial = name.sub!(/^_/, "") + partial = !!name.sub!(/^_/, "") lookup.disable_cache do - lookup.find_template(name, pieces.join, partial || false, @locals) + lookup.find_template(name, pieces.join, partial, @locals) + end + end + + # Expires this template by setting his updated_at date to Jan 1st, 1970. + def expire! + @updated_at = Time.utc(1970) + end + + # Receives a view context and renders a template exactly like self by using + # the @virtual_path. It raises an error if no @virtual_path was given. + def rerender(view) + raise "A template needs to have a virtual path in order to be rerendered" unless @virtual_path + name = @virtual_path.dup + if name.sub!(/(^|\/)_([^\/]*)$/, '\1\2') + view.render :partial => name + else + view.render :template => @virtual_path end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 34679962b2..22401dd145 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -90,20 +90,65 @@ class TestERBTemplate < ActiveSupport::TestCase assert_equal "hellopartialhello", render end - def test_refresh + def test_refresh_with_templates @template = new_template("Hello", :virtual_path => "test/foo") @template.locals = [:key] @context.lookup_context.expects(:find_template).with("foo", "test", false, [:key]).returns("template") assert_equal "template", @template.refresh(@context) end + def test_refresh_with_partials + @template = new_template("Hello", :virtual_path => "test/_foo") + @template.locals = [:key] + @context.lookup_context.expects(:find_template).with("foo", "test", true, [:key]).returns("partial") + assert_equal "partial", @template.refresh(@context) + end + def test_refresh_raises_an_error_without_virtual_path @template = new_template("Hello", :virtual_path => nil) - assert_raise RuntimeError, /OMG/ do + assert_raise RuntimeError do @template.refresh(@context) end end + def test_template_expire_sets_the_timestamp_to_1970 + @template = new_template("Hello", :updated_at => Time.utc(2010)) + assert_equal Time.utc(2010), @template.updated_at + @template.expire! + assert_equal Time.utc(1970), @template.updated_at + end + + def test_template_rerender_renders_a_template_like_self + @template = new_template("Hello", :virtual_path => "test/foo_bar") + @context.expects(:render).with(:template => "test/foo_bar").returns("template") + assert_equal "template", @template.rerender(@context) + end + + def test_template_rerender_renders_a_root_template_like_self + @template = new_template("Hello", :virtual_path => "foo_bar") + @context.expects(:render).with(:template => "foo_bar").returns("template") + assert_equal "template", @template.rerender(@context) + end + + def test_template_rerender_renders_a_partial_like_self + @template = new_template("Hello", :virtual_path => "test/_foo_bar") + @context.expects(:render).with(:partial => "test/foo_bar").returns("partial") + assert_equal "partial", @template.rerender(@context) + end + + def test_template_rerender_renders_a_root_partial_like_self + @template = new_template("Hello", :virtual_path => "_foo_bar") + @context.expects(:render).with(:partial => "foo_bar").returns("partial") + assert_equal "partial", @template.rerender(@context) + end + + def test_rerender_raises_an_error_without_virtual_path + @template = new_template("Hello", :virtual_path => nil) + assert_raise RuntimeError do + @template.rerender(@context) + end + end + if "ruby".encoding_aware? def test_resulting_string_is_utf8 @template = new_template -- cgit v1.2.3 From 49b6f33f99a28d68f18203a696fb47854a9085d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 14:47:11 +0200 Subject: Clean up unused methods from AV::Base and pass in the template object on rendering. --- actionpack/lib/action_view/base.rb | 29 ++++++++++------------ .../lib/action_view/helpers/translation_helper.rb | 4 +-- actionpack/lib/action_view/template.rb | 7 ++++-- actionpack/test/template/template_test.rb | 18 ++++++++------ .../test/template/translation_helper_test.rb | 4 +-- actionpack/test/template/url_helper_test.rb | 2 +- 6 files changed, 33 insertions(+), 31 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 2a601c7cee..a7a6bbd3a4 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -155,9 +155,6 @@ module ActionView #:nodoc: # # See the ActionView::Helpers::PrototypeHelper::JavaScriptGenerator::GeneratorMethods documentation for more details. class Base - module Subclasses - end - include Helpers, Rendering, Partials, ::ERB::Util, Context # Specify whether RJS responses should be wrapped in a try/catch block @@ -177,12 +174,12 @@ module ActionView #:nodoc: delegate :logger, :to => 'ActionController::Base', :allow_nil => true end - attr_accessor :base_path, :assigns, :template_extension, :lookup_context - attr_internal :captures, :request, :controller, :template, :config + attr_accessor :_template + attr_internal :request, :controller, :config, :assigns, :lookup_context delegate :formats, :formats=, :locale, :locale=, :view_paths, :view_paths=, :to => :lookup_context - delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, + delegate :request_forgery_protection_token, :params, :session, :cookies, :response, :headers, :flash, :action_name, :controller_name, :to => :controller delegate :logger, :to => :controller, :allow_nil => true @@ -197,26 +194,26 @@ module ActionView #:nodoc: end def assign(new_assigns) # :nodoc: - self.assigns = new_assigns.each { |key, value| instance_variable_set("@#{key}", value) } + @_assigns = new_assigns.each { |key, value| instance_variable_set("@#{key}", value) } end def initialize(lookup_context = nil, assigns_for_first_render = {}, controller = nil, formats = nil) #:nodoc: assign(assigns_for_first_render) - self.helpers = self.class.helpers || Module.new - - if @_controller = controller - @_request = controller.request if controller.respond_to?(:request) - end - - @_config = controller && controller.respond_to?(:config) ? controller.config.inheritable_copy : {} + self.helpers = Module.new unless self.class.helpers + @_config = {} @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil @output_buffer = nil - @lookup_context = lookup_context.is_a?(ActionView::LookupContext) ? + if @_controller = controller + @_request = controller.request if controller.respond_to?(:request) + @_config = controller.config.inheritable_copy if controller.respond_to?(:config) + end + + @_lookup_context = lookup_context.is_a?(ActionView::LookupContext) ? lookup_context : ActionView::LookupContext.new(lookup_context) - @lookup_context.formats = formats if formats + @_lookup_context.formats = formats if formats end def store_content_for(key, value) diff --git a/actionpack/lib/action_view/helpers/translation_helper.rb b/actionpack/lib/action_view/helpers/translation_helper.rb index 13767a09f9..8574ca6595 100644 --- a/actionpack/lib/action_view/helpers/translation_helper.rb +++ b/actionpack/lib/action_view/helpers/translation_helper.rb @@ -45,8 +45,8 @@ module ActionView private def scope_key_by_partial(key) if key.to_s.first == "." - if @_virtual_path - @_virtual_path.gsub(%r{/_?}, ".") + key.to_s + if (path = @_template && @_template.virtual_path) + path.gsub(%r{/_?}, ".") + key.to_s else raise "Cannot use t(#{key.inspect}) shortcut because path is not available" end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 5c3e0e899b..7dd8acf37b 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -133,12 +133,15 @@ module ActionView # we use a bang in this instrumentation because you don't want to # consume this in production. This is only slow if it's being listened to. def render(view, locals, &block) + old_template, view._template = view._template, self ActiveSupport::Notifications.instrument("!render_template.action_view", :virtual_path => @virtual_path) do compile!(view) view.send(method_name, locals, &block) end rescue Exception => e handle_render_error(view, e) + ensure + view._template = old_template end def mime_type @@ -272,9 +275,9 @@ module ActionView # encoding of the code source = <<-end_src def #{method_name}(local_assigns) - _old_virtual_path, @_virtual_path = @_virtual_path, #{@virtual_path.inspect};_old_output_buffer = @output_buffer;#{locals_code};#{code} + _old_output_buffer = @output_buffer;#{locals_code};#{code} ensure - @_virtual_path, @output_buffer = _old_virtual_path, _old_output_buffer + @output_buffer = _old_output_buffer end end_src diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 22401dd145..63f792d328 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -4,12 +4,14 @@ require "logger" class TestERBTemplate < ActiveSupport::TestCase ERBHandler = ActionView::Template::Handlers::ERB.new - class Context - class LookupContext - def disable_cache - yield - end + class LookupContext + def disable_cache + yield end + end + + class Context + attr_accessor :_template def initialize @output_buffer = "original" @@ -22,7 +24,7 @@ class TestERBTemplate < ActiveSupport::TestCase def partial ActionView::Template.new( - "<%= @_virtual_path %>", + "<%= @_template.virtual_path %>", "partial", ERBHandler, :virtual_path => "partial" @@ -84,9 +86,9 @@ class TestERBTemplate < ActiveSupport::TestCase end def test_virtual_path - @template = new_template("<%= @_virtual_path %>" \ + @template = new_template("<%= @_template.virtual_path %>" \ "<%= partial.render(self, {}) %>" \ - "<%= @_virtual_path %>") + "<%= @_template.virtual_path %>") assert_equal "hellopartialhello", render end diff --git a/actionpack/test/template/translation_helper_test.rb b/actionpack/test/template/translation_helper_test.rb index 952719a589..763080550b 100644 --- a/actionpack/test/template/translation_helper_test.rb +++ b/actionpack/test/template/translation_helper_test.rb @@ -31,13 +31,13 @@ class TranslationHelperTest < ActiveSupport::TestCase def test_scoping_by_partial I18n.expects(:translate).with("test.translation.helper", :raise => true).returns("helper") - @view = ActionView::Base.new(ActionController::Base.view_paths, {}) + @view = ::ActionView::Base.new(ActionController::Base.view_paths, {}) assert_equal "helper", @view.render(:file => "test/translation") end def test_scoping_by_partial_of_an_array I18n.expects(:translate).with("test.scoped_translation.foo.bar", :raise => true).returns(["foo", "bar"]) - @view = ActionView::Base.new(ActionController::Base.view_paths, {}) + @view = ::ActionView::Base.new(ActionController::Base.view_paths, {}) assert_equal "foobar", @view.render(:file => "test/scoped_translation") end diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 98276da559..b8a7d4259d 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -9,7 +9,7 @@ class UrlHelperTest < ActiveSupport::TestCase # or request. # # In those cases, we'll set up a simple mock - attr_accessor :controller, :request + attr_accessor :controller, :request, :_template routes = ActionDispatch::Routing::RouteSet.new routes.draw do -- cgit v1.2.3 From 682368d4ba0bb4548f896d02bc4e038ee8ba6b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 22:40:13 +0200 Subject: Use identifiers for template equality. --- .../lib/action_view/renderer/template_renderer.rb | 2 +- actionpack/lib/action_view/template.rb | 9 +++++++++ actionpack/lib/action_view/template/inline.rb | 20 ++++++++++++++++++++ .../test/controller/new_base/render_once_test.rb | 17 ++++++++--------- actionpack/test/template/template_test.rb | 8 ++++++++ 5 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 actionpack/lib/action_view/template/inline.rb (limited to 'actionpack') diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index 9f9df15347..a9076760c5 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -45,7 +45,7 @@ module ActionView with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } elsif options.key?(:inline) handler = Template.handler_class_for_extension(options[:type] || "erb") - Template.new(options[:inline], "inline template", handler, { :locals => keys }) + Template::Inline.new(options[:inline], handler, :locals => keys) elsif options.key?(:template) options[:template].respond_to?(:render) ? options[:template] : find_template(options[:template], options[:prefix], false, keys) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 7dd8acf37b..3ba18cbfae 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -93,6 +93,7 @@ module ActionView autoload :Error autoload :Handler autoload :Handlers + autoload :Inline autoload :Text end @@ -184,6 +185,14 @@ module ActionView end end + def hash + identifier.hash + end + + def eql?(other) + other.is_a?(Template) && other.identifier == identifier + end + def inspect @inspect ||= if defined?(Rails.root) diff --git a/actionpack/lib/action_view/template/inline.rb b/actionpack/lib/action_view/template/inline.rb new file mode 100644 index 0000000000..be08065b6b --- /dev/null +++ b/actionpack/lib/action_view/template/inline.rb @@ -0,0 +1,20 @@ +require 'digest/md5' + +module ActionView + class Template + class Inline < ::ActionView::Template + def initialize(source, handler, options={}) + super(source, "inline template", handler, options) + end + + def md5_source + @md5_source ||= Digest::MD5.hexdigest(source) + end + + def eql?(other) + other.is_a?(Inline) && other.md5_source == md5_source + end + end + end +end + \ No newline at end of file diff --git a/actionpack/test/controller/new_base/render_once_test.rb b/actionpack/test/controller/new_base/render_once_test.rb index 12892b7255..63de25be52 100644 --- a/actionpack/test/controller/new_base/render_once_test.rb +++ b/actionpack/test/controller/new_base/render_once_test.rb @@ -61,13 +61,12 @@ module RenderTemplate include Tests end - # TODO This still needs to be implemented and supported. - # class TestWithoutResolverCache < Rack::TestCase - # testing RenderTemplate::RenderOnceController - # include Tests - # - # def setup - # RenderTemplate::RenderOnceController::RESOLVER.stubs(:caching?).returns(false) - # end - # end + class TestWithoutResolverCache < Rack::TestCase + testing RenderTemplate::RenderOnceController + include Tests + + def setup + RenderTemplate::RenderOnceController::RESOLVER.stubs(:caching?).returns(false) + end + end end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 63f792d328..f2156c31de 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -151,6 +151,14 @@ class TestERBTemplate < ActiveSupport::TestCase end end + def test_inline_template_is_only_equal_if_source_match + inline1 = ActionView::Template::Inline.new("sample", ERBHandler) + inline2 = ActionView::Template::Inline.new("sample", ERBHandler) + inline3 = ActionView::Template::Inline.new("other", ERBHandler) + assert inline1.eql?(inline2) + assert !inline1.eql?(inline3) + end + if "ruby".encoding_aware? def test_resulting_string_is_utf8 @template = new_template -- cgit v1.2.3 From 5ec27189b8b433145baa7270cf4219c5041f6a4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 10 Oct 2010 23:11:50 +0200 Subject: Do not allow templates coming from Fallback resolvers to store a virtual path. --- actionpack/lib/action_view.rb | 1 + actionpack/lib/action_view/lookup_context.rb | 2 +- actionpack/lib/action_view/template/resolver.rb | 13 +++++++++++++ actionpack/test/template/lookup_context_test.rb | 4 ++-- actionpack/test/template/render_test.rb | 13 ++++++++++++- 5 files changed, 29 insertions(+), 4 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index ad96f6c66d..0f9d35d062 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -57,6 +57,7 @@ module ActionView autoload :Resolver autoload :PathResolver autoload :FileSystemResolver + autoload :FallbackFileSystemResolver end autoload_at "action_view/template/error" do diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index 0cff888ac1..80451798b1 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -10,7 +10,7 @@ module ActionView # this key is generated just once during the request, it speeds up all cache accesses. class LookupContext #:nodoc: mattr_accessor :fallbacks - @@fallbacks = [FileSystemResolver.new(""), FileSystemResolver.new("/")] + @@fallbacks = FallbackFileSystemResolver.instances mattr_accessor :registered_details self.registered_details = [] diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 5c6877a923..7707dbcf98 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -135,6 +135,7 @@ module ActionView end end + # A resolver that loads files from the filesystem. class FileSystemResolver < PathResolver def initialize(path) raise ArgumentError, "path already is a Resolver class" if path.is_a?(Resolver) @@ -152,4 +153,16 @@ module ActionView end alias :== :eql? end + + # The same as FileSystemResolver but does not allow templates to store + # a virtual path since it is invalid for such resolvers. + class FallbackFileSystemResolver < FileSystemResolver + def self.instances + [new(""), new("/")] + end + + def decorate(*) + super.each { |t| t.virtual_path = nil } + end + end end diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index 23dfc1ba75..850589b13b 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -100,8 +100,8 @@ class LookupContextTest < ActiveSupport::TestCase @lookup_context.with_fallbacks do assert_equal 3, @lookup_context.view_paths.size - assert @lookup_context.view_paths.include?(ActionView::FileSystemResolver.new("")) - assert @lookup_context.view_paths.include?(ActionView::FileSystemResolver.new("/")) + assert @lookup_context.view_paths.include?(ActionView::FallbackFileSystemResolver.new("")) + assert @lookup_context.view_paths.include?(ActionView::FallbackFileSystemResolver.new("/")) end end diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index 756d8d05d2..17bb610b6a 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -114,7 +114,7 @@ module RenderTestCases end def test_render_sub_template_with_errors - @view.render(:file => "test/sub_template_raise") + @view.render(:template => "test/sub_template_raise") flunk "Render did not raise Template::Error" rescue ActionView::Template::Error => e assert_match %r!method.*doesnt_exist!, e.message @@ -123,6 +123,17 @@ module RenderTestCases assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name end + def test_render_file_with_errors + @view.render(:file => File.expand_path("test/_raise", FIXTURE_LOAD_PATH)) + flunk "Render did not raise Template::Error" + rescue ActionView::Template::Error => e + assert_match %r!method.*doesnt_exist!, e.message + assert_equal "", e.sub_template_message + assert_equal "1", e.line_number + assert_equal "1: <%= doesnt_exist %>", e.annoted_source_code.strip + assert_equal File.expand_path("#{FIXTURE_LOAD_PATH}/test/_raise.html.erb"), e.file_name + end + def test_render_object assert_equal "Hello: david", @view.render(:partial => "test/customer", :object => Customer.new("david")) end -- cgit v1.2.3 From cb26eee54d34eda966b4c9da810b700cce24f824 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 10 Oct 2010 21:07:53 -0200 Subject: Revert "Make InstanceTagMethods#value_before_type_cast raise if the model don't respond to attr_before_type_cast or attr method" And "Makes form_helper use overriden model accessors" This reverts commit 3ba8e3100548f10fce0c9784981a4589531476dd and fb0bd8c1092db51888ec4bb72af6c595e13c31fa. --- actionpack/lib/action_view/helpers/form_helper.rb | 11 +++------ actionpack/test/template/form_helper_test.rb | 30 ----------------------- 2 files changed, 3 insertions(+), 38 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 3cd8b02bc4..b34a74788e 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -1020,14 +1020,9 @@ module ActionView def value_before_type_cast(object, method_name) unless object.nil? - if object.respond_to?(method_name) - object.send(method_name) - # FIXME: this is AR dependent - elsif object.respond_to?(method_name + "_before_type_cast") - object.send(method_name + "_before_type_cast") - else - raise NoMethodError, "Model #{object.class} does not respond to #{method_name}" - end + object.respond_to?(method_name + "_before_type_cast") ? + object.send(method_name + "_before_type_cast") : + object.send(method_name) end end diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 8809e510fb..0bfdbeebd1 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -4,18 +4,6 @@ require 'controller/fake_models' class FormHelperTest < ActionView::TestCase tests ActionView::Helpers::FormHelper - class Developer - def name_before_type_cast - "David" - end - - def name - "Santiago" - end - - attr_writer :language - end - def form_for(*) @output_buffer = super end @@ -278,24 +266,6 @@ class FormHelperTest < ActionView::TestCase text_field("user", "email", :type => "email") end - def test_text_field_from_a_user_defined_method - @developer = Developer.new - assert_dom_equal( - '', text_field("developer", "name") - ) - end - - def test_text_field_on_a_model_with_undefined_attr_reader - @developer = Developer.new - @developer.language = 'ruby' - begin - text_field("developer", "language") - rescue NoMethodError => error - message = error.message - end - assert_equal "Model #{Developer} does not respond to language", message - end - def test_check_box assert_dom_equal( '', -- cgit v1.2.3 From ef74ad8e0c0a1c46f7e8983e9d5460dea9bec95e Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Mon, 11 Oct 2010 13:28:50 +1100 Subject: Remove mention to register_javascript_include_default in documentation --- actionpack/lib/action_view/helpers/asset_tag_helper.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index c1dfbe5dc3..b97c73aff9 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -292,9 +292,6 @@ module ActionView # # * = The application.js file is only referenced if it exists # - # Though it's not really recommended practice, if you need to extend the default JavaScript set for any reason - # (e.g., you're going to be using a certain .js file in every action), then take a look at the register_javascript_include_default method. - # # You can also include all javascripts in the +javascripts+ directory using :all as the source: # # javascript_include_tag :all # => -- cgit v1.2.3 From 234a4ca7ddb7794a92f266af48fd38edbdb03003 Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Mon, 11 Oct 2010 13:32:04 +1100 Subject: Add missing CHANGELOG entry about reset_javascript_include_default --- actionpack/CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 6352b97a6b..966d1ae47d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -33,6 +33,8 @@ * Upgrade to Rack 1.2.1 [Jeremy Kemper] +* Removed reset_javascript_include_default, use config.action_view.javascript_expansions in config/application.rb for this instead [José Valim] + * Allow :path to be given to match/get/post/put/delete instead of :path_names in the new router [Carlos Antônio da Silva] * Added resources_path_names to the new router DSL [José Valim] -- cgit v1.2.3 From e6b45b8111cc375be57a1e1ca2b2b47eb21a2e01 Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Mon, 11 Oct 2010 12:00:37 +0200 Subject: Revert "Add missing CHANGELOG entry about reset_javascript_include_default" This reverts commit 234a4ca7ddb7794a92f266af48fd38edbdb03003. Reason: No big deal Ryan, only there's a very strict policy in docrails which allows public write access on the other hand. CHANGELOGs can only be edited in master. If this is added I'll make sure you get the credit for the patch. --- actionpack/CHANGELOG | 2 -- 1 file changed, 2 deletions(-) (limited to 'actionpack') diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 966d1ae47d..6352b97a6b 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -33,8 +33,6 @@ * Upgrade to Rack 1.2.1 [Jeremy Kemper] -* Removed reset_javascript_include_default, use config.action_view.javascript_expansions in config/application.rb for this instead [José Valim] - * Allow :path to be given to match/get/post/put/delete instead of :path_names in the new router [Carlos Antônio da Silva] * Added resources_path_names to the new router DSL [José Valim] -- cgit v1.2.3 From 0744d36f479ad35a89a50efaadb66674cac0a323 Mon Sep 17 00:00:00 2001 From: Krekoten' Marjan Date: Tue, 12 Oct 2010 22:55:19 +0300 Subject: Fix small typo in documentation --- actionpack/lib/action_controller/metal/conditional_get.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 8930c13a68..a5e37172c9 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -66,7 +66,7 @@ module ActionController # Examples: # expires_in 20.minutes # expires_in 3.hours, :public => true - # expires in 3.hours, 'max-stale' => 5.hours, :public => true + # expires_in 3.hours, 'max-stale' => 5.hours, :public => true # # This method will overwrite an existing Cache-Control header. # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities. -- cgit v1.2.3