From a2be6ce3eb94516a57c07c98f3cb19502b05cff1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Jan 2019 11:43:23 -0800 Subject: Remove method named "hash" We can't use the FixtureResolver as a hash key because it doesn't implement `hash` correctly. This commit renames the method to "data" (which is just as unfortunately named :( ) --- actionview/lib/action_view/testing/resolvers.rb | 6 ++++-- actionview/test/template/lookup_context_test.rb | 12 ++++++------ 2 files changed, 10 insertions(+), 8 deletions(-) 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..02b5d48e5a 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -162,7 +162,7 @@ class LookupContextTest < ActiveSupport::TestCase # 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 @@ -221,12 +221,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 +237,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 +246,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 -- cgit v1.2.3 From 0b0412ab531590580cd303e54da68dadd4853de3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 28 Jan 2019 11:47:53 -0800 Subject: Make `@view_paths` on the lookup context mostly read-only The `with_fallbacks` method will temporarily mutate the lookup context instance, but nobody can call the setter, and we don't have to do a push / pop dance. --- actionview/lib/action_view/lookup_context.rb | 24 ++++++++++-------------- actionview/test/template/lookup_context_test.rb | 10 +++++++--- 2 files changed, 17 insertions(+), 17 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/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 02b5d48e5a..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,7 +160,7 @@ 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 @@ -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) -- cgit v1.2.3