Add tests for lookup context.

actionpack/lib/action_view/lookup_context.rb

@@ -11,16 +11,26 @@ class LookupContext #:nodoc:
     @@fallbacks = [FileSystemResolver.new(""), FileSystemResolver.new("/")]
 
     mattr_accessor :registered_details
-    self.registered_details = {}
+    self.registered_details = []
 
     def self.register_detail(name, options = {})
-      registered_details[name] = lambda do |value|
+      self.registered_details << name
+
+      Setters.send :define_method, :"#{name}=" do |value|
         value = Array(value.presence || yield)
         value |= [nil] unless options[:allow_nil] == false
-        value
+
+        unless value == @details[name]
+          @details_key, @details = nil, @details.merge(name => value)
+          @details.freeze
+        end
       end
     end
 
+    # Holds raw setters for the registered details.
+    module Setters #:nodoc:
+    end
+
     register_detail(:formats) { Mime::SET.symbols }
     register_detail(:locale)  { [I18n.locale] }
 
@@ -40,7 +50,7 @@ def initialize(details)
     end
 
     def initialize(view_paths, details = {})
-      @details_key = nil
+      @details, @details_key = {}, nil
       self.view_paths = view_paths
       self.details = details
     end
@@ -55,18 +65,18 @@ def view_paths=(paths)
       end
 
       def find(name, prefix = nil, partial = false)
-        @view_paths.find(name, prefix, partial || false, details, details_key)
+        @view_paths.find(name, prefix, partial, details, details_key)
       end
 
       def find_all(name, prefix = nil, partial = false)
-        @view_paths.find_all(name, prefix, partial || false, details, details_key)
+        @view_paths.find_all(name, prefix, partial, details, details_key)
       end
 
       def exists?(name, prefix = nil, partial = false)
-        @view_paths.exists?(name, prefix, partial || false, details, details_key)
+        @view_paths.exists?(name, prefix, partial, details, details_key)
       end
 
-      # Add fallbacks to the view paths. Useful in cases you are rendering a file.
+      # Add fallbacks to the view paths. Useful in cases you are rendering a :file.
       def with_fallbacks
         added_resolvers = 0
         self.class.fallbacks.each do |resolver|
@@ -83,9 +93,8 @@ def with_fallbacks
     module Details
       attr_reader :details
 
-      def details=(details)
-        @details = normalize_details(details)
-        @details_key = nil if @details_key && @details_key.details != @details
+      def details=(given_details)
+        registered_details.each { |key| send(:"#{key}=", given_details[key]) }
       end
 
       def details_key
@@ -97,23 +106,25 @@ def formats
         @details[:formats].compact
       end
 
-      # Shortcut to set formats in details.
-      def formats=(value)
-        self.details = @details.merge(:formats => value)
+      # Overload formats= to reject [:"*/*"] values.
+      def formats=(value, freeze=true)
+        value = nil if value == [:"*/*"]
+        super(value)
       end
 
       # Shortcut to read locale.
       def locale
         I18n.locale
       end
 
-      # Shortcut to set locale in details and I18n.
+      # Overload locale= to also set the I18n.locale. If the current I18n.config object responds
+      # to i18n_config, it means that it's has a copy of the original I18n configuration and it's
+      # acting as proxy, which we need to skip.
       def locale=(value)
-        I18n.locale = value
-
-        unless I18n.config.respond_to?(:lookup_context)
-          self.details = @details.merge(:locale => value)
-        end
+        value = value.first if value.is_a?(Array)
+        config = I18n.config.respond_to?(:i18n_config) ? I18n.config.i18n_config : I18n.config
+        config.locale = value if value
+        super(I18n.locale)
       end
 
       # Update the details keys by merging the given hash into the current
@@ -127,24 +138,13 @@ def update_details(new_details)
           begin
             yield
           ensure
-            self.details = old_details
+            @details = old_details
           end
         end
       end
-
-    protected
-
-      def normalize_details(details)
-        details = details.dup
-        # TODO: Refactor this concern out of the resolver
-        details.delete(:formats) if details[:formats] == [:"*/*"]
-        self.class.registered_details.each do |k, v|
-          details[k] = v.call(details[k])
-        end
-        details.freeze
-      end
     end
 
+    include Setters
     include Details
     include ViewPaths
   end

actionpack/test/controller/new_base/render_rjs_test.rb

@@ -17,7 +17,7 @@ def index
     end
 
     def index_locale
-      old_locale, I18n.locale = I18n.locale, :da
+      self.locale = :da
     end
   end
 

actionpack/test/lib/fixture_template.rb

@@ -1,5 +1,7 @@
 module ActionView #:nodoc:
   class FixtureResolver < PathResolver
+    attr_reader :hash
+
     def initialize(hash = {})
       super()
       @hash = hash

actionpack/test/template/lookup_context_test.rb

@@ -0,0 +1,167 @@
+require "abstract_unit"
+require "abstract_controller/rendering"
+
+ActionView::LookupContext::DetailsKey.class_eval do
+  def self.details_keys
+    @details_keys
+  end
+end
+
+class LookupContextTest < ActiveSupport::TestCase
+  def setup
+    @lookup_context = ActionView::LookupContext.new(FIXTURE_LOAD_PATH, {})
+  end
+
+  def teardown
+    I18n.locale = :en
+    ActionView::LookupContext::DetailsKey.details_keys.clear
+  end
+
+  test "process view paths on initialization" do
+    assert_kind_of ActionView::PathSet, @lookup_context.view_paths
+  end
+
+  test "normalizes details on initialization" do
+    formats = Mime::SET + [nil]
+    locale  = [I18n.locale, nil]
+    assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details
+  end
+
+  test "allows me to set details" do
+    @lookup_context.details = { :formats => [:html], :locale => :pt }
+    assert_equal Hash[:formats => [:html, nil], :locale => [:pt, nil]], @lookup_context.details
+  end
+
+  test "does not allow details to be modified in place" do
+    assert_raise TypeError do
+      @lookup_context.details.clear
+    end
+  end
+
+  test "allows me to update an specific detail" do
+    @lookup_context.update_details(:locale => :pt)
+    assert_equal :pt, I18n.locale
+    formats = Mime::SET + [nil]
+    locale  = [I18n.locale, nil]
+    assert_equal Hash[:formats => formats, :locale => locale], @lookup_context.details
+  end
+
+  test "allows me to change some details to execute an specific block of code" do
+    formats = Mime::SET + [nil]
+    @lookup_context.update_details(:locale => :pt) do
+      assert_equal Hash[:formats => formats, :locale => [:pt, nil]], @lookup_context.details
+    end
+    assert_equal Hash[:formats => formats, :locale => [:en, nil]], @lookup_context.details
+  end
+
+  test "provides getters and setters for formats" do
+    @lookup_context.formats = :html
+    assert_equal [:html], @lookup_context.formats
+  end
+
+  test "handles */* formats" do
+    @lookup_context.formats = [:"*/*"]
+    assert_equal Mime::SET, @lookup_context.formats
+  end
+
+  test "provides getters and setters for locale" do
+    @lookup_context.locale = :pt
+    assert_equal :pt, @lookup_context.locale
+  end
+
+  test "changing lookup_context locale, changes I18n.locale" do
+    @lookup_context.locale = :pt
+    assert_equal :pt, I18n.locale
+  end
+
+  test "delegates changing the locale to the I18n configuration object if it contains a lookup_context object" do
+    begin
+      I18n.config = AbstractController::I18nProxy.new(I18n.config, @lookup_context)
+      @lookup_context.locale = :pt
+      assert_equal :pt, I18n.locale
+      assert_equal :pt, @lookup_context.locale
+    ensure
+      I18n.config = I18n.config.i18n_config
+    end
+
+    assert_equal :pt, I18n.locale
+  end
+
+  test "find templates using the given view paths and configured details" do
+    template = @lookup_context.find("hello_world", "test")
+    assert_equal "Hello world!", template.source
+
+    @lookup_context.locale = :da
+    template = @lookup_context.find("hello_world", "test")
+    assert_equal "Hey verden", template.source
+  end
+
+  test "adds fallbacks to view paths when required" do
+    assert_equal 1, @lookup_context.view_paths.size
+
+    @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("/"))
+    end
+  end
+
+  test "add fallbacks just once in nested fallbacks calls" do
+    @lookup_context.with_fallbacks do
+      @lookup_context.with_fallbacks do
+        assert_equal 3, @lookup_context.view_paths.size
+      end
+    end
+  end
+
+  test "generates a new details key for each details hash" do
+    keys = []
+    keys << @lookup_context.details_key
+    assert_equal 1, keys.uniq.size
+
+    @lookup_context.locale = :da
+    keys << @lookup_context.details_key
+    assert_equal 2, keys.uniq.size
+
+    @lookup_context.locale = :en
+    keys << @lookup_context.details_key
+    assert_equal 2, keys.uniq.size
+
+    @lookup_context.formats = :html
+    keys << @lookup_context.details_key
+    assert_equal 3, keys.uniq.size
+
+    @lookup_context.formats = nil
+    keys << @lookup_context.details_key
+    assert_equal 3, keys.uniq.size
+  end
+
+  test "gives the key forward to the resolver, so it can be used as cache key" do
+    @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo")
+    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 we will hit the cache.
+    @lookup_context.view_paths.first.hash["test/_foo.erb"] = "Bar"
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+
+    # This time we will change the locale. The updated template should be picked since
+    # lookup_context generated a new key after we changed the locale.
+    @lookup_context.locale = :da
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Bar", template.source
+
+    # Now we will change back the locale and it will still pick the old template.
+    # This is expected because lookup_context will reuse the previous key for :en locale.
+    @lookup_context.locale = :en
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Foo", template.source
+
+    # Finally, we can expire the cache. And the expected template will be used.
+    @lookup_context.view_paths.first.clear_cache
+    template = @lookup_context.find("foo", "test", true)
+    assert_equal "Bar", template.source
+  end
+end