diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index a0e66128a20ba..92e72a73157e0 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -245,11 +245,9 @@ def spot(exc) def build_backtrace built_methods = {} - ActionView::ViewPaths.all_view_paths.each do |path_set| - path_set.each do |resolver| - resolver.built_templates.each do |template| - built_methods[template.method_name] = template - end + ActionView::ViewPaths::Registry.all_resolvers.each do |resolver| + resolver.built_templates.each do |template| + built_methods[template.method_name] = template end end diff --git a/actionview/lib/action_view/cache_expiry.rb b/actionview/lib/action_view/cache_expiry.rb index cab052b66bda4..c50a0ed2fa6d4 100644 --- a/actionview/lib/action_view/cache_expiry.rb +++ b/actionview/lib/action_view/cache_expiry.rb @@ -1,65 +1,65 @@ # frozen_string_literal: true module ActionView - class CacheExpiry - class Executor - def initialize(watcher:) - @execution_lock = Concurrent::ReentrantReadWriteLock.new - @cache_expiry = ViewModificationWatcher.new(watcher: watcher) do - clear_cache + module CacheExpiry # :nodoc: all + class ViewReloader + def initialize(watcher:, &block) + @mutex = Mutex.new + @watcher_class = watcher + @watched_dirs = nil + @watcher = nil + @previous_change = false + + rebuild_watcher + + _self = self + ActionView::ViewPaths::Registry.singleton_class.set_callback(:build_file_system_resolver, :after) do + _self.send(:rebuild_watcher) end end - def run - ActiveSupport::Dependencies.interlock.permit_concurrent_loads do - @cache_expiry.execute_if_updated - @execution_lock.acquire_read_lock - end + def updated? + @previous_change || @watcher.updated? end - def complete(_) - @execution_lock.release_read_lock + def execute + watcher = nil + @mutex.synchronize do + @previous_change = false + watcher = @watcher + end + watcher.execute end private - def clear_cache - @execution_lock.with_write_lock do - ActionView::LookupContext::DetailsKey.clear - end + def reload! + ActionView::LookupContext::DetailsKey.clear end - end - class ViewModificationWatcher - def initialize(watcher:, &block) - @watched_dirs = nil - @watcher_class = watcher - @watcher = nil - @mutex = Mutex.new - @block = block - end + def rebuild_watcher + @mutex.synchronize do + old_watcher = @watcher - def execute_if_updated - @mutex.synchronize do - watched_dirs = dirs_to_watch - return if watched_dirs.empty? + if @watched_dirs != dirs_to_watch + @watched_dirs = dirs_to_watch + new_watcher = @watcher_class.new([], @watched_dirs) do + reload! + end + @watcher = new_watcher - if watched_dirs != @watched_dirs - @watched_dirs = watched_dirs - @watcher = @watcher_class.new([], watched_dirs, &@block) - @watcher.execute - else - @watcher.execute_if_updated + # We must check the old watcher after initializing the new one to + # ensure we don't miss any events + @previous_change ||= old_watcher&.updated? + end end end - end - private def dirs_to_watch - all_view_paths.grep(FileSystemResolver).map!(&:path).tap(&:uniq!).sort! + all_view_paths.uniq.sort end def all_view_paths - ActionView::ViewPaths.all_view_paths.flat_map(&:paths) + ActionView::ViewPaths::Registry.all_file_system_resolvers.map(&:path) end end end diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index d2167174720a3..7f3f420a3e50e 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -71,8 +71,8 @@ def self.details_cache_key(details) end def self.clear - ActionView::ViewPaths.all_view_paths.each do |path_set| - path_set.each(&:clear_cache) + ActionView::ViewPaths::Registry.all_resolvers.each do |resolver| + resolver.clear_cache end @view_context_class = nil @details_keys.clear diff --git a/actionview/lib/action_view/path_set.rb b/actionview/lib/action_view/path_set.rb index 7cb1f25ad803a..7d2615898f3ef 100644 --- a/actionview/lib/action_view/path_set.rb +++ b/actionview/lib/action_view/path_set.rb @@ -67,7 +67,7 @@ def typecast(paths) paths.map do |path| case path when Pathname, String - FileSystemResolver.new path.to_s + ViewPaths::Registry.file_system_resolver(path.to_s) when Resolver path else diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index dd3df1613780b..e2626566c1387 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -107,7 +107,14 @@ class Railtie < Rails::Engine # :nodoc: end unless enable_caching - app.executor.register_hook ActionView::CacheExpiry::Executor.new(watcher: app.config.file_watcher) + view_reloader = ActionView::CacheExpiry::ViewReloader.new(watcher: app.config.file_watcher) + + app.reloaders << view_reloader + view_reloader.execute + app.reloader.to_run do + require_unload_lock! + view_reloader.execute + end end end diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index e08c64ba1abff..e8c9b4bb01d77 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -5,7 +5,7 @@ module ViewPaths extend ActiveSupport::Concern included do - ViewPaths.set_view_paths(self, ActionView::PathSet.new.freeze) + ViewPaths::Registry.set_view_paths(self, ActionView::PathSet.new.freeze) end delegate :template_exists?, :any_templates?, :view_paths, :formats, :formats=, @@ -13,11 +13,11 @@ module ViewPaths module ClassMethods def _view_paths - ViewPaths.get_view_paths(self) + ViewPaths::Registry.get_view_paths(self) end def _view_paths=(paths) - ViewPaths.set_view_paths(self, paths) + ViewPaths::Registry.set_view_paths(self, paths) end def _prefixes # :nodoc: @@ -70,21 +70,46 @@ def local_prefixes end end - # :stopdoc: - @all_view_paths = {} + module Registry # :nodoc: + @view_paths_by_class = {} + @file_system_resolvers = Concurrent::Map.new - def self.get_view_paths(klass) - @all_view_paths[klass] || get_view_paths(klass.superclass) - end + class << self + include ActiveSupport::Callbacks + define_callbacks :build_file_system_resolver + end - def self.set_view_paths(klass, paths) - @all_view_paths[klass] = paths - end + def self.get_view_paths(klass) + @view_paths_by_class[klass] || get_view_paths(klass.superclass) + end + + def self.set_view_paths(klass, paths) + @view_paths_by_class[klass] = paths + end - def self.all_view_paths - @all_view_paths.values.uniq + def self.file_system_resolver(path) + path = File.expand_path(path) + resolver = @file_system_resolvers[path] + unless resolver + run_callbacks(:build_file_system_resolver) do + resolver = @file_system_resolvers.fetch_or_store(path) do + FileSystemResolver.new(path) + end + end + end + resolver + end + + def self.all_resolvers + resolvers = [all_file_system_resolvers] + resolvers.concat @view_paths_by_class.values.map(&:to_a) + resolvers.flatten.uniq + end + + def self.all_file_system_resolvers + @file_system_resolvers.values + end end - # :startdoc: # The prefixes used in render "foo" shortcuts. def _prefixes # :nodoc: diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb index 7ab394f0bd299..964875c1236a3 100644 --- a/actionview/test/actionpack/controller/render_test.rb +++ b/actionview/test/actionpack/controller/render_test.rb @@ -1471,35 +1471,40 @@ def test_render_call_to_partial_with_layout_in_main_layout_and_within_content_fo assert_equal "Before (Anthony)\nInside from partial (Anthony)\nAfter\nBefore (David)\nInside from partial (David)\nAfter\nBefore (Ramm)\nInside from partial (Ramm)\nAfter", @response.body end - def test_template_annotations + def with_annotations_enabled ActionView::Base.annotate_rendered_view_with_filenames = true + ActionView::LookupContext::DetailsKey.clear + yield + ensure + ActionView::Base.annotate_rendered_view_with_filenames = false + ActionView::LookupContext::DetailsKey.clear + end - get :greeting + def test_template_annotations + with_annotations_enabled do + get :greeting + end assert_includes @response.body, "