diff options
18 files changed, 118 insertions, 239 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 48d6bbc8d9..14165e17a7 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -267,7 +267,6 @@ module ActionMailer #:nodoc: include AbstractController::Logger include AbstractController::Rendering - include AbstractController::DetailsCache include AbstractController::Layouts include AbstractController::Helpers include AbstractController::Translation diff --git a/actionpack/lib/abstract_controller/compatibility.rb b/actionpack/lib/abstract_controller/compatibility.rb index 7fb93a0eb5..85fb1364b7 100644 --- a/actionpack/lib/abstract_controller/compatibility.rb +++ b/actionpack/lib/abstract_controller/compatibility.rb @@ -11,7 +11,7 @@ module AbstractController def _default_layout(details, require_layout = false) super rescue ActionView::MissingTemplate - _find_layout(_layout({}), {}) + _layout_for_name(_layout({}), {}) nil end end diff --git a/actionpack/lib/abstract_controller/details_cache.rb b/actionpack/lib/abstract_controller/details_cache.rb deleted file mode 100644 index be1a1c0f34..0000000000 --- a/actionpack/lib/abstract_controller/details_cache.rb +++ /dev/null @@ -1,48 +0,0 @@ -module AbstractController - class HashKey - @hash_keys = Hash.new {|h,k| h[k] = {} } - - def self.get(klass, details) - @hash_keys[klass][details] ||= new(klass, details) - end - - attr_reader :hash - alias_method :eql?, :equal? - - def initialize(klass, details) - @details, @hash = details, details.hash - end - - def inspect - "#<HashKey -- details: #{@details.inspect}>" - end - end - - module DetailsCache - extend ActiveSupport::Concern - - module ClassMethods - def clear_template_caches! - ActionView::Partials::PartialRenderer::TEMPLATES.clear - template_cache.clear - super - end - - def template_cache - @template_cache ||= Hash.new {|h,k| h[k] = {} } - end - end - - def render_to_body(*args) - Thread.current[:format_locale_key] = HashKey.get(self.class, details_for_render) - super - end - - private - - def with_template_cache(name, details) - self.class.template_cache[HashKey.get(self.class, details)][name] ||= super - end - - end -end diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index beda4e633e..c26593dd19 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -170,27 +170,7 @@ module AbstractController module ClassMethods def inherited(klass) super - klass.class_eval do - _write_layout_method - @found_layouts = {} - end - end - - def clear_template_caches! - @found_layouts.clear if defined? @found_layouts - super - end - - def cache_layout(details) - layout = @found_layouts - key = Thread.current[:format_locale_key] - - # Cache nil - if layout.key?(key) - return layout[key] - else - layout[key] = yield - end + klass._write_layout_method end # This module is mixed in if layout conditions are provided. This means @@ -282,12 +262,10 @@ module AbstractController if name self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def _layout(details) - self.class.cache_layout(details) do - if template_exists?("#{_implied_layout_name}", details, :_prefix => "layouts") - "#{_implied_layout_name}" - else - super - end + if template_exists?("#{_implied_layout_name}", :_prefix => "layouts") + "#{_implied_layout_name}" + else + super end end RUBY @@ -372,9 +350,10 @@ module AbstractController # ==== Returns # Template:: A template object matching the name and details def _find_layout(name, details) - # TODO: Make prefix actually part of details in ViewPath#find_by_parts prefix = details.key?(:prefix) ? details.delete(:prefix) : "layouts" - find_template(name, details, :_prefix => prefix) + # TODO This should happen automatically + template_lookup.details = template_lookup.details.merge(:formats => details[:formats]) + find_template(name, :_prefix => prefix) end # Returns the default layout for this controller and a given set of details. diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 2ea4f02871..df33e8b480 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -90,10 +90,22 @@ module AbstractController view_context.render_partial(options) end + def template_lookup + @template_lookup ||= ActionView::Template::Lookup.new(_view_paths, details_for_render) + end + # The list of view paths for this controller. See ActionView::ViewPathSet for # more details about writing custom view paths. def view_paths - _view_paths + template_lookup.view_paths + end + + def append_view_path(path) + template_lookup.view_paths.push(*path) + end + + def prepend_view_path(path) + template_lookup.view_paths.unshift(*path) end # The prefix used in render "foo" shortcuts. @@ -162,10 +174,9 @@ module AbstractController options[:_prefix] ||= _prefix if (options.keys & [:partial, :file, :template]).empty? details = _normalize_details(options) + template_lookup.details = details - options[:_template] ||= with_template_cache(name, details) do - find_template(name, details, options) - end + options[:_template] ||= find_template(name, options) end def details_for_render @@ -179,16 +190,12 @@ module AbstractController details end - def find_template(name, details, options) - view_paths.find(name, details, options[:_prefix], options[:_partial]) - end - - def template_exists?(name, details, options) - view_paths.exists?(name, details, options[:_prefix], options[:_partial]) + def find_template(name, options) + template_lookup.find(name, options[:_prefix], options[:_partial]) end - def with_template_cache(name, details) - yield + def template_exists?(name, options) + template_lookup.exists?(name, options[:_prefix], options[:_partial]) end def format_for_text @@ -196,9 +203,6 @@ module AbstractController end module ClassMethods - def clear_template_caches! - end - # Append a path to the list of view paths for this controller. # # ==== Parameters @@ -206,7 +210,7 @@ module AbstractController # the default view path. You may also provide a custom view path # (see ActionView::ViewPathSet for more information) def append_view_path(path) - self.view_paths = view_paths.dup + Array.wrap(path) + self.view_paths = view_paths.dup + Array(path) end # Prepend a path to the list of view paths for this controller. @@ -216,8 +220,7 @@ module AbstractController # the default view path. You may also provide a custom view path # (see ActionView::ViewPathSet for more information) def prepend_view_path(path) - clear_template_caches! - self.view_paths = Array.wrap(path) + view_paths.dup + self.view_paths = Array(path) + view_paths.dup end # A list of all of the default view paths for this controller. @@ -231,9 +234,8 @@ module AbstractController # paths<ViewPathSet, Object>:: If a ViewPathSet is provided, use that; # otherwise, process the parameter into a ViewPathSet. def view_paths=(paths) - clear_template_caches! self._view_paths = paths.is_a?(ActionView::PathSet) ? paths : ActionView::Base.process_view_paths(paths) - _view_paths.freeze + self._view_paths.freeze end end end diff --git a/actionpack/lib/action_controller/metal/compatibility.rb b/actionpack/lib/action_controller/metal/compatibility.rb index 317a9fd951..518272b4cf 100644 --- a/actionpack/lib/action_controller/metal/compatibility.rb +++ b/actionpack/lib/action_controller/metal/compatibility.rb @@ -73,18 +73,5 @@ module ActionController def performed? response_body end - - # ==== Request only view path switching ==== - def append_view_path(path) - view_paths.push(*path) - end - - def prepend_view_path(path) - view_paths.unshift(*path) - end - - def view_paths - view_context.view_paths - end end end diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index ba2d9b686e..da717fcce2 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -12,6 +12,7 @@ module ActionController def method_for_action(action_name) super || begin + # TODO This should use template lookup if view_paths.exists?(action_name.to_s, details_for_render, controller_path) "default_render" end diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 00a09309bf..0a4215028b 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -4,7 +4,6 @@ module ActionController include ActionController::RackDelegation include AbstractController::Rendering - include AbstractController::DetailsCache def process_action(*) self.formats = request.formats.map {|x| x.to_sym } diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 76f9eb2b0d..22bc390dc5 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -180,64 +180,21 @@ module ActionView #:nodoc: attr_accessor :base_path, :assigns, :template_extension attr_internal :captures - def reset_formats(formats) - old_formats, self.formats = self.formats, formats - reset_hash_key - yield if block_given? - ensure - if block_given? - self.formats = old_formats - reset_hash_key - end - end - - def reset_hash_key - if defined?(AbstractController::HashKey) - # This is expensive, but we need to reset this when the format is updated, - # which currently only happens - Thread.current[:format_locale_key] = - AbstractController::HashKey.get(self.class, :formats => formats, :locale => [I18n.locale]) - end - end - - def formats - controller ? controller.formats : @formats - end - - def formats=(val) - if controller - controller.formats = val - else - @formats = val - end - end - class << self delegate :erb_trim_mode=, :to => 'ActionView::Template::Handlers::ERB' delegate :logger, :to => 'ActionController::Base', :allow_nil => true end - @@debug_rjs = false - ## - # :singleton-method: # Specify whether RJS responses should be wrapped in a try/catch block # that alert()s the caught exception (and then re-raises it). cattr_accessor :debug_rjs - - # Specify whether templates should be cached. Otherwise the file we be read everytime it is accessed. - # Automatically reloading templates are not thread safe and should only be used in development mode. - @@cache_template_loading = nil - cattr_accessor :cache_template_loading + @@debug_rjs = false # :nodoc: def self.xss_safe? true end - def self.cache_template_loading? - ActionController::Base.allow_concurrency || (cache_template_loading.nil? ? !ActiveSupport::Dependencies.load? : cache_template_loading) - end - attr_internal :request, :layout def controller_path @@ -249,8 +206,6 @@ module ActionView #:nodoc: delegate :logger, :to => :controller, :allow_nil => true - delegate :find, :to => :view_paths - include Context def self.process_view_paths(value) @@ -287,10 +242,10 @@ module ActionView #:nodoc: klass = self end - klass.new(controller.class.view_paths, {}, controller) + klass.new(controller.template_lookup, {}, controller) end - def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil, formats = nil)#:nodoc: + def initialize(template_lookup = nil, assigns_for_first_render = {}, controller = nil, formats = nil) #:nodoc: @config = nil @formats = formats @assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) } @@ -298,16 +253,33 @@ module ActionView #:nodoc: @_controller = controller @_config = ActiveSupport::InheritableOptions.new(controller.config) if controller - @_content_for = Hash.new {|h,k| h[k] = ActiveSupport::SafeBuffer.new } + @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil - self.view_paths = view_paths + + @template_lookup = template_lookup.is_a?(ActionView::Template::Lookup) ? + template_lookup : ActionView::Template::Lookup.new(template_lookup) end attr_internal :controller, :template, :config - attr_reader :view_paths - def view_paths=(paths) - @view_paths = self.class.process_view_paths(paths) + attr_reader :template_lookup + delegate :find, :view_paths, :view_paths=, :to => :template_lookup + + def formats=(formats) + update_details(:formats => Array(formats)) + end + + def update_details(details) + old_details = template_lookup.details + template_lookup.details = old_details.merge(details) + + if block_given? + begin + yield + ensure + template_lookup.details = old_details + end + end end def punctuate_body!(part) diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index 67a7586699..be49b5cc28 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -182,7 +182,7 @@ module ActionView def initialize(context, &block) #:nodoc: context._evaluate_assigns_and_ivars @context, @lines = context, [] - @context.reset_formats([:js, :html]) do + @context.update_details(:formats => [:js, :html]) do include_helpers_from_context @context.with_output_buffer(@lines) do @context.instance_exec(self, &block) @@ -583,7 +583,7 @@ module ActionView end def with_formats(*args) - @context ? @context.reset_formats(args) { yield } : yield + @context ? @context.update_details(:formats => args) { yield } : yield end def javascript_object_for(object) diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index 459b6bba54..154a79a8f1 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -9,7 +9,7 @@ module ActionView #:nodoc: METHOD end - def find(path, details = {}, prefix = nil, partial = false) + def find(path, details = {}, prefix = nil, partial = false, key=nil) each do |resolver| if template = resolver.find(path, details, prefix, partial) return template @@ -19,7 +19,7 @@ module ActionView #:nodoc: raise ActionView::MissingTemplate.new(self, "#{prefix}/#{path}", details, partial) end - def exists?(path, details = {}, prefix = nil, partial = false) + def exists?(path, details = {}, prefix = nil, partial = false, key=nil) each do |resolver| if resolver.find(path, details, prefix, partial) return true diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index 8b6dce0c1c..74513935a7 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -309,7 +309,7 @@ module ActionView prefix = controller.controller_path unless path.include?(?/) end - @view.find(path, {:formats => @view.formats}, prefix, true) + @view.find(path, prefix, true) end def partial_path(object = @object) @@ -329,7 +329,7 @@ module ActionView details = options[:_details] - # Is this needed + # TODO This should happen automatically as well self.formats = details[:formats] if details renderer = PartialRenderer.new(self, options, nil) text = renderer.render diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index 28b79bfcb7..1be5675e37 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -25,7 +25,7 @@ module ActionView end template = if options[:file] - find(options[:file], details_for_render) + find(options[:file]) elsif options[:inline] handler = Template.handler_class_for_extension(options[:type] || "erb") Template.new(options[:inline], "inline template", handler, {}) @@ -34,7 +34,7 @@ module ActionView end if template - layout = find(layout, details_for_render) if layout + layout = find(layout) if layout _render_template(template, layout, :locals => options[:locals]) end when :update @@ -44,10 +44,6 @@ module ActionView end end - def details_for_render - controller.try(:details_for_render) || {:formats => formats} - end - # You can think of a layout as a method that is called with a block. _layout_for # returns the contents that are yielded to the layout. If the user calls yield # :some_name, the block, by default, returns content_for(:some_name). If the user diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index cd6b1930a1..c176359253 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -13,6 +13,7 @@ module ActionView autoload :Handler autoload :Handlers autoload :Text + autoload :Lookup end extend Template::Handlers diff --git a/actionpack/lib/action_view/template/lookup.rb b/actionpack/lib/action_view/template/lookup.rb new file mode 100644 index 0000000000..ea3a12615b --- /dev/null +++ b/actionpack/lib/action_view/template/lookup.rb @@ -0,0 +1,48 @@ +module ActionView + class Template + class Lookup + attr_reader :details, :view_paths + + class DetailsKey + attr_reader :details + alias :eql? :equal? + + @details_keys = Hash.new + + def self.get(details) + @details_keys[details] ||= new(details) + end + + def initialize(details) + @details, @hash = details, details.hash + end + end + + def initialize(view_paths, details = {}) + @details = details + self.view_paths = view_paths + end + + def view_paths=(paths) + @view_paths = ActionView::Base.process_view_paths(paths) + end + + def details=(details) + @details = details + @details_key = nil if @details_key && @details_key.details != details + end + + def details_key + @details_key ||= DetailsKey.get(details) unless details.empty? + end + + def find(name, prefix = nil, partial = false) + @view_paths.find(name, details, prefix, partial || false, details_key) + end + + def exists?(name, prefix = nil, partial = false) + @view_paths.exists?(name, details, prefix, partial || false, details_key) + end + end + end +end
\ No newline at end of file diff --git a/actionpack/test/abstract/details_cache_test.rb b/actionpack/test/abstract/details_cache_test.rb deleted file mode 100644 index ee746c1bb0..0000000000 --- a/actionpack/test/abstract/details_cache_test.rb +++ /dev/null @@ -1,57 +0,0 @@ -require 'abstract_unit' - -module AbstractController - module Testing - - class CachedController < AbstractController::Base - include AbstractController::Rendering - include AbstractController::DetailsCache - - self.view_paths = [ActionView::FixtureResolver.new( - "default.erb" => "With Default", - "template.erb" => "With Template", - "some/file.erb" => "With File", - "template_name.erb" => "With Template Name" - )] - end - - class TestLocalizedCache < ActiveSupport::TestCase - - def setup - @controller = CachedController.new - CachedController.clear_template_caches! - end - - def test_templates_are_cached - @controller.render :template => "default.erb" - assert_equal "With Default", @controller.response_body - - cached = @controller.class.template_cache - assert_equal 1, cached.size - assert_kind_of ActionView::Template, cached.values.first["default.erb"] - end - - def test_cache_is_used - CachedController.new.render :template => "default.erb" - - @controller.expects(:find_template).never - @controller.render :template => "default.erb" - - assert_equal 1, @controller.class.template_cache.size - end - - def test_cache_changes_with_locale - CachedController.new.render :template => "default.erb" - - I18n.locale = :es - @controller.render :template => "default.erb" - - assert_equal 2, @controller.class.template_cache.size - ensure - I18n.locale = :en - end - - end - - end -end diff --git a/actionpack/test/template/javascript_helper_test.rb b/actionpack/test/template/javascript_helper_test.rb index 368a9c2514..6604f1c095 100644 --- a/actionpack/test/template/javascript_helper_test.rb +++ b/actionpack/test/template/javascript_helper_test.rb @@ -7,8 +7,8 @@ class JavaScriptHelperTest < ActionView::TestCase attr_accessor :formats, :output_buffer - def reset_formats(format) - @format = format + def update_details(details) + @details = details yield if block_given? end diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb index 6811d3aaf3..619293dc43 100644 --- a/actionpack/test/template/prototype_helper_test.rb +++ b/actionpack/test/template/prototype_helper_test.rb @@ -39,8 +39,8 @@ class Author::Nested < Author; end class PrototypeHelperBaseTest < ActionView::TestCase attr_accessor :formats, :output_buffer - def reset_formats(format) - @format = format + def update_details(details) + @details = details yield if block_given? end |