Merge pull request #35074 from rails/ro-lookup-context

Make the lookup context more "read-only"

3 files changed, 27 insertions, 25 deletions

diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb
index 554d223c0e..cc262dc2b7 100644
--- a/actionview/lib/action_view/lookup_context.rb
+++ b/actionview/lib/action_view/lookup_context.rb
@@ -106,12 +106,6 @@ module ActionView
     module ViewPaths
       attr_reader :view_paths, :html_fallback_for_js
 
-      # Whenever setting view paths, makes a copy so that we can manipulate them in
-      # instance objects as we wish.
-      def view_paths=(paths)
-        @view_paths = ActionView::PathSet.new(Array(paths))
-      end
-
       def find(name, prefixes = [], partial = false, keys = [], options = {})
         @view_paths.find(*args_for_lookup(name, prefixes, partial, keys, options))
       end
@@ -138,19 +132,21 @@ module ActionView
       # Adds fallbacks to the view paths. Useful in cases when you are rendering
       # a :file.
       def with_fallbacks
-        added_resolvers = 0
-        self.class.fallbacks.each do |resolver|
-          next if view_paths.include?(resolver)
-          view_paths.push(resolver)
-          added_resolvers += 1
-        end
+        view_paths = @view_paths
+        @view_paths = build_view_paths((view_paths.paths + self.class.fallbacks).uniq)
         yield
       ensure
-        added_resolvers.times { view_paths.pop }
+        @view_paths = view_paths
       end
 
     private
 
+      # Whenever setting view paths, makes a copy so that we can manipulate them in
+      # instance objects as we wish.
+      def build_view_paths(paths)
+        ActionView::PathSet.new(Array(paths))
+      end
+
       def args_for_lookup(name, prefixes, partial, keys, details_options)
         name, prefixes = normalize_name(name, prefixes)
         details, details_key = detail_args_for(details_options)
@@ -226,7 +222,7 @@ module ActionView
       @rendered_format = nil
 
       @details = initialize_details({}, details)
-      self.view_paths = view_paths
+      @view_paths = build_view_paths(view_paths)
     end
 
     def digest_cache
diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb
index 1fad08a689..d6203b95c5 100644
--- a/actionview/lib/action_view/testing/resolvers.rb
+++ b/actionview/lib/action_view/testing/resolvers.rb
@@ -8,13 +8,15 @@ module ActionView #:nodoc:
   # useful for testing extensions that have no way of knowing what the file
   # system will look like at runtime.
   class FixtureResolver < PathResolver
-    attr_reader :hash
-
     def initialize(hash = {}, pattern = nil)
       super(pattern)
       @hash = hash
     end
 
+    def data
+      @hash
+    end
+
     def to_s
       @hash.keys.join(", ")
     end
diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb
index 68e151f154..0894925c9f 100644
--- a/actionview/test/template/lookup_context_test.rb
+++ b/actionview/test/template/lookup_context_test.rb
@@ -5,10 +5,14 @@ require "abstract_controller/rendering"
 
 class LookupContextTest < ActiveSupport::TestCase
   def setup
-    @lookup_context = ActionView::LookupContext.new(FIXTURE_LOAD_PATH, {})
+    @lookup_context = build_lookup_context(FIXTURE_LOAD_PATH, {})
     ActionView::LookupContext::DetailsKey.clear
   end
 
+  def build_lookup_context(paths, details)
+    ActionView::LookupContext.new(paths, details)
+  end
+
   def teardown
     I18n.locale = :en
   end
@@ -156,13 +160,13 @@ class LookupContextTest < ActiveSupport::TestCase
   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")
+    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {})
     template = @lookup_context.find("foo", %w(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"
+    @lookup_context.view_paths.first.data["test/_foo.erb"] = "Bar"
     template = @lookup_context.find("foo", %w(test), true)
     assert_equal "Foo", template.source
 
@@ -185,7 +189,7 @@ class LookupContextTest < ActiveSupport::TestCase
   end
 
   test "can disable the cache on demand" do
-    @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo")
+    @lookup_context = build_lookup_context(ActionView::FixtureResolver.new("test/_foo.erb" => "Foo"), {})
     old_template = @lookup_context.find("foo", %w(test), true)
 
     template = @lookup_context.find("foo", %w(test), true)
@@ -221,12 +225,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase
 
       # 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"
+      @resolver.data["test/_foo.erb"][0] = "Bar"
       template = @lookup_context.find("foo", %w(test), true)
       assert_equal "Foo", template.source
 
       # Now update the timestamp.
-      @resolver.hash["test/_foo.erb"][1] = Time.now.utc
+      @resolver.data["test/_foo.erb"][1] = Time.now.utc
       template = @lookup_context.find("foo", %w(test), true)
       assert_equal "Bar", template.source
     end
@@ -237,7 +241,7 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase
       template = @lookup_context.find("foo", %w(test), true)
       assert_equal "Foo", template.source
 
-      @resolver.hash.clear
+      @resolver.data.clear
       assert_raise ActionView::MissingTemplate do
         @lookup_context.find("foo", %w(test), true)
       end
@@ -246,12 +250,12 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase
 
   test "if no template was cached in the first lookup, retrieval should work in the second call" do
     ActionView::Resolver.stub(:caching?, false) do
-      @resolver.hash.clear
+      @resolver.data.clear
       assert_raise ActionView::MissingTemplate do
         @lookup_context.find("foo", %w(test), true)
       end
 
-      @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)]
+      @resolver.data["test/_foo.erb"] = ["Foo", Time.utc(2000)]
       template = @lookup_context.find("foo", %w(test), true)
       assert_equal "Foo", template.source
     end