From e65d41a47af0c56cb2d60868e6ad2ba01d324d48 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 6 Sep 2022 18:31:16 -0700 Subject: [PATCH 1/2] View path strict type casting --- actionview/lib/action_view/lookup_context.rb | 6 +++++- actionview/lib/action_view/path_set.rb | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 0616f7efd78..ab4dc12188e 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -152,7 +152,11 @@ module ActionView # 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)) + if ActionView::PathSet === paths + paths + else + ActionView::PathSet.new(Array(paths)) + end end # Compute details hash and key according to user options (e.g. passed from #render). diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb index d081d714f08..fdec2ef1090 100644 --- a/actionview/lib/action_view/path_set.rb +++ b/actionview/lib/action_view/path_set.rb @@ -76,8 +76,10 @@ module ActionView # :nodoc: case path when Pathname, String FileSystemResolver.new path.to_s - else + when Resolver path + else + raise TypeError, "#{path.inspect} is not a valid path: must be a String, Pathname, or Resolver" end end end From 0371317e253e4a3517e3e24a5803b60e64413f62 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Tue, 6 Sep 2022 18:36:17 -0700 Subject: [PATCH 2/2] Make ActionView::PathSet immutable Previously we would mutate the PathSet in order to implement {append,prepend}_view_path. This removes the mutation methods we had and instead constructs a new PathSet whenever we are modifying it. This should allow flexibility in the classes holding a view_path to know that their view paths isn't modified (for example, to allow caching). --- actionview/lib/action_view/lookup_context.rb | 8 ++++++++ actionview/lib/action_view/path_set.rb | 14 +++----------- actionview/lib/action_view/view_paths.rb | 8 ++++---- .../test/actionpack/controller/view_paths_test.rb | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index ab4dc12188e..cbfb627892a 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -148,6 +148,14 @@ module ActionView end alias :any_templates? :any? + def append_view_paths(paths) + @view_paths = build_view_paths(@view_paths.to_a + paths) + end + + def prepend_view_paths(paths) + @view_paths = build_view_paths(paths + @view_paths.to_a) + end + private # Whenever setting view paths, makes a copy so that we can manipulate them in # instance objects as we wish. diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb index fdec2ef1090..7cb1f25ad80 100644 --- a/actionview/lib/action_view/path_set.rb +++ b/actionview/lib/action_view/path_set.rb @@ -13,14 +13,14 @@ module ActionView # :nodoc: attr_reader :paths - delegate :[], :include?, :pop, :size, :each, to: :paths + delegate :[], :include?, :size, :each, to: :paths def initialize(paths = []) - @paths = typecast paths + @paths = typecast(paths).freeze end def initialize_copy(other) - @paths = other.paths.dup + @paths = other.paths.dup.freeze self end @@ -36,14 +36,6 @@ module ActionView # :nodoc: PathSet.new(paths + array) end - %w(<< concat push insert unshift).each do |method| - class_eval <<-METHOD, __FILE__, __LINE__ + 1 - def #{method}(*args) - paths.#{method}(*typecast(args)) - end - METHOD - end - def find(path, prefixes, partial, details, details_key, locals) find_all(path, prefixes, partial, details, details_key, locals).first || raise(MissingTemplate.new(self, path, prefixes, partial, details, details_key, locals)) diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index ef5701d5bca..e08c64ba1ab 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -35,7 +35,7 @@ module ActionView # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def append_view_path(path) - self._view_paths = view_paths + Array(path) + self._view_paths = ActionView::PathSet.new(view_paths.to_a + Array(path)) end # Prepend a path to the list of view paths for this controller. @@ -45,7 +45,7 @@ module ActionView # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def prepend_view_path(path) - self._view_paths = ActionView::PathSet.new(Array(path) + view_paths) + self._view_paths = ActionView::PathSet.new(Array(path) + view_paths.to_a) end # A list of all of the default view paths for this controller. @@ -110,7 +110,7 @@ module ActionView # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def append_view_path(path) - lookup_context.view_paths.push(*path) + lookup_context.append_view_paths(Array(path)) end # Prepend a path to the list of view paths for the current LookupContext. @@ -120,7 +120,7 @@ module ActionView # the default view path. You may also provide a custom view path # (see ActionView::PathSet for more information) def prepend_view_path(path) - lookup_context.view_paths.unshift(*path) + lookup_context.prepend_view_paths(Array(path)) end end end diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb index 1c4171fb089..7beab4b5574 100644 --- a/actionview/test/actionpack/controller/view_paths_test.rb +++ b/actionview/test/actionpack/controller/view_paths_test.rb @@ -155,7 +155,7 @@ class ViewLoadPathsTest < ActionController::TestCase end decorator = decorator_class.new(TestController.view_paths) - TestController.view_paths = ActionView::PathSet.new.push(decorator) + TestController.view_paths = ActionView::PathSet.new([decorator]) get :hello_world assert_response :success