From c7564d74e8a9b451f9fc78566ab0c734671f9612 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Sun, 7 Mar 2010 19:41:58 +0100
Subject: [PATCH] Added template lookup responsible to hold all information
 used in template lookup.

---
 actionmailer/lib/action_mailer/base.rb        |  1 -
 .../lib/abstract_controller/compatibility.rb  |  2 +-
 .../lib/abstract_controller/details_cache.rb  | 48 ------------
 actionpack/lib/abstract_controller/layouts.rb | 37 ++-------
 .../lib/abstract_controller/rendering.rb      | 42 +++++-----
 .../action_controller/metal/compatibility.rb  | 13 ----
 .../metal/implicit_render.rb                  |  1 +
 .../lib/action_controller/metal/rendering.rb  |  1 -
 actionpack/lib/action_view/base.rb            | 78 ++++++-------------
 .../action_view/helpers/prototype_helper.rb   |  4 +-
 actionpack/lib/action_view/paths.rb           |  4 +-
 actionpack/lib/action_view/render/partials.rb |  4 +-
 .../lib/action_view/render/rendering.rb       |  8 +-
 actionpack/lib/action_view/template.rb        |  1 +
 actionpack/lib/action_view/template/lookup.rb | 48 ++++++++++++
 .../test/abstract/details_cache_test.rb       | 57 --------------
 .../test/template/javascript_helper_test.rb   |  4 +-
 .../test/template/prototype_helper_test.rb    |  4 +-
 18 files changed, 118 insertions(+), 239 deletions(-)
 delete mode 100644 actionpack/lib/abstract_controller/details_cache.rb
 create mode 100644 actionpack/lib/action_view/template/lookup.rb
 delete mode 100644 actionpack/test/abstract/details_cache_test.rb

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])
+    def find_template(name, options)
+      template_lookup.find(name, options[:_prefix], options[:_partial])
     end
 
-    def template_exists?(name, details, options)
-      view_paths.exists?(name, details, 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