From f9bea6304dfba902b1937b3bc29b1ebc2f67e55b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 23 Jan 2019 14:19:50 -0800 Subject: [PATCH 01/10] Move templates to an anonymous subclass of AV::Base Now we can throw away the subclass and the generated methods will get GC'd too --- .../action_dispatch/middleware/debug_view.rb | 9 ++++++++ actionview/lib/action_view.rb | 1 - actionview/lib/action_view/base.rb | 16 +++++++------- actionview/lib/action_view/lookup_context.rb | 5 +++++ actionview/lib/action_view/rendering.rb | 17 ++++----------- actionview/test/abstract_unit.rb | 6 ++++-- .../activerecord/multifetch_cache_test.rb | 4 +++- .../test/template/compiled_templates_test.rb | 17 ++++++++++++--- .../test/template/log_subscriber_test.rb | 5 ++++- actionview/test/template/render_test.rb | 21 +++++++++++++++++-- .../test/template/streaming_render_test.rb | 5 ++++- actionview/test/template/template_test.rb | 4 +++- .../test/template/translation_helper_test.rb | 5 ++++- 13 files changed, 82 insertions(+), 33 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index 5a7010a1c2..499bb1b391 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -9,12 +9,21 @@ module ActionDispatch class DebugView < ActionView::Base # :nodoc: RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) + module CompiledTemplates + end + + include CompiledTemplates + def initialize(assigns) paths = [RESCUES_TEMPLATE_PATH] renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths) super(renderer, assigns) end + def compiled_method_container + CompiledTemplates + end + def debug_params(params) clean_params = params.clone clean_params.delete("action") diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index 6ff2d70e35..8cb4648a67 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -35,7 +35,6 @@ module ActionView eager_autoload do autoload :Base autoload :Context - autoload :CompiledTemplates, "action_view/context" autoload :Digestor autoload :Helpers autoload :LookupContext diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index b143e13c96..1e58004fcf 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -11,10 +11,6 @@ require "action_view/template" require "action_view/lookup_context" module ActionView #:nodoc: - module CompiledTemplates #:nodoc: - # holds compiled template code - end - # = Action View Base # # Action View templates can be written in several ways. @@ -146,8 +142,6 @@ module ActionView #:nodoc: class Base include Helpers, ::ERB::Util, Context - include CompiledTemplates - # Specify the proc used to decorate input tags that refer to attributes with errors. cattr_accessor :field_error_proc, default: Proc.new { |html_tag, instance| "
#{html_tag}
".html_safe } @@ -186,6 +180,14 @@ module ActionView #:nodoc: def xss_safe? #:nodoc: true end + + def with_empty_template_cache # :nodoc: + template_container = Module.new + Class.new(self) { + include template_container + define_method(:compiled_method_container) { template_container } + } + end end attr_reader :view_renderer @@ -260,7 +262,7 @@ module ActionView #:nodoc: end def compiled_method_container - CompiledTemplates + raise NotImplementedError end ActiveSupport.run_load_hooks(:action_view, self) diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index c2f6439e26..c3bb0a49fc 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -68,12 +68,17 @@ module ActionView end def self.clear + @view_context_class = nil @details_keys.clear end def self.digest_caches @details_keys.values end + + def self.view_context_class(klass) + @view_context_class ||= klass.with_empty_template_cache + end end # Add caching behavior on top of Details. diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 205665a8c6..01caa82ec6 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -36,21 +36,12 @@ module ActionView module ClassMethods def view_context_class - @view_context_class ||= begin - supports_path = supports_path? - routes = respond_to?(:_routes) && _routes - helpers = respond_to?(:_helpers) && _helpers + klass = ActionView::LookupContext::DetailsKey.view_context_class(ActionView::Base) - Class.new(ActionView::Base) do - if routes - include routes.url_helpers(supports_path) - include routes.mounted_helpers - end + @view_context_class ||= build_view_context_class(klass, supports_path?, _routes, _helpers) - if helpers - include helpers - end - end + if klass.changed?(@view_context_class) + @view_context_class = build_view_context_class(klass, supports_path?, _routes, _helpers) end end end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index acc2ed029b..b649b3c9dd 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -48,7 +48,8 @@ module RenderERBUtils @view ||= begin path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.with_view_paths(view_paths) + view = ActionView::Base.with_empty_template_cache + view.with_view_paths(view_paths) end end @@ -61,7 +62,8 @@ module RenderERBUtils ActionView::Template.handler_for_extension(:erb), {}) - template.render(ActionView::Base.empty, {}).strip + view = ActionView::Base.with_empty_template_cache + template.render(view.empty, {}).strip end end diff --git a/actionview/test/activerecord/multifetch_cache_test.rb b/actionview/test/activerecord/multifetch_cache_test.rb index 229b4e56d0..f56168bda5 100644 --- a/actionview/test/activerecord/multifetch_cache_test.rb +++ b/actionview/test/activerecord/multifetch_cache_test.rb @@ -10,8 +10,10 @@ class MultifetchCacheTest < ActiveRecordTestCase def setup view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) - @view = Class.new(ActionView::Base) do + @view = Class.new(ActionView::Base.with_empty_template_cache) do def view_cache_dependencies [] end diff --git a/actionview/test/template/compiled_templates_test.rb b/actionview/test/template/compiled_templates_test.rb index ded4786e62..3d4561b55f 100644 --- a/actionview/test/template/compiled_templates_test.rb +++ b/actionview/test/template/compiled_templates_test.rb @@ -3,7 +3,18 @@ require "abstract_unit" class CompiledTemplatesTest < ActiveSupport::TestCase - teardown do + attr_reader :view_class + + def setup + super + view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + @view_class = ActionView::Base.with_empty_template_cache + end + + def teardown + super ActionView::LookupContext::DetailsKey.clear end @@ -72,13 +83,13 @@ class CompiledTemplatesTest < ActiveSupport::TestCase def render_with_cache(*args) view_paths = ActionController::Base.view_paths - ActionView::Base.with_view_paths(view_paths, {}).render(*args) + view_class.with_view_paths(view_paths, {}).render(*args) end def render_without_cache(*args) path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) - ActionView::Base.with_view_paths(view_paths, {}).render(*args) + view_class.with_view_paths(view_paths, {}).render(*args) end def modify_template(template, content) diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 9fcf80bb24..7f98207e3d 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -12,9 +12,12 @@ class AVLogSubscriberTest < ActiveSupport::TestCase super view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + lookup_context = ActionView::LookupContext.new(view_paths, {}, ["test"]) renderer = ActionView::Renderer.new(lookup_context) - @view = ActionView::Base.new(renderer, {}) + @view = ActionView::Base.with_empty_template_cache.new(renderer, {}) ActionView::LogSubscriber.attach_to :action_view diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index f89d087c34..372b85a4a5 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -9,7 +9,8 @@ end module RenderTestCases def setup_view(paths) @assigns = { secret: "in the sauce" } - @view = Class.new(ActionView::Base) do + + @view = Class.new(ActionView::Base.with_empty_template_cache) do def view_cache_dependencies; []; end def combined_fragment_cache_key(key) @@ -17,7 +18,16 @@ module RenderTestCases end end.with_view_paths(paths, @assigns) - @controller_view = TestController.new.view_context + controller = TestController.new + view = @view + + @controller_view = Class.new(controller.view_context_class) do + include view.compiled_method_container + + define_method(:compiled_method_container) do + view.compiled_method_container + end + end.new(controller.view_renderer, controller.view_assigns, controller) # Reload and register danish language for testing I18n.backend.store_translations "da", {} @@ -629,6 +639,8 @@ class CachedViewRenderTest < ActiveSupport::TestCase # Ensure view path cache is primed def setup view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class setup_view(view_paths) end @@ -645,6 +657,9 @@ class LazyViewRenderTest < ActiveSupport::TestCase # Test the same thing as above, but make sure the view path # is not eager loaded def setup + view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) path = ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH) view_paths = ActionView::PathSet.new([path]) assert_equal ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH), view_paths.first @@ -704,6 +719,8 @@ class CachedCollectionViewRenderTest < ActiveSupport::TestCase setup do view_paths = ActionController::Base.view_paths assert_equal ActionView::OptimizedFileSystemResolver, view_paths.first.class + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) ActionView::PartialRenderer.collection_cache = ActiveSupport::Cache::MemoryStore.new diff --git a/actionview/test/template/streaming_render_test.rb b/actionview/test/template/streaming_render_test.rb index dda2095013..4567ee31b4 100644 --- a/actionview/test/template/streaming_render_test.rb +++ b/actionview/test/template/streaming_render_test.rb @@ -8,8 +8,11 @@ end class SetupFiberedBase < ActiveSupport::TestCase def setup view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + @assigns = { secret: "in the sauce", name: nil } - @view = ActionView::Base.with_view_paths(view_paths, @assigns) + @view = ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, @assigns) @controller_view = TestController.new.view_context end diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 8257d97b7c..8af6739e45 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -20,6 +20,7 @@ class TestERBTemplate < ActiveSupport::TestCase class Context < ActionView::Base def initialize + super @output_buffer = "original" @virtual_path = nil end @@ -63,7 +64,8 @@ class TestERBTemplate < ActiveSupport::TestCase end def setup - @context = Context.new + @context = Context.with_empty_template_cache.new + super end def test_basic_template diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index d04e68e182..7094d4c7fc 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -36,7 +36,10 @@ class TranslationHelperTest < ActiveSupport::TestCase } } ) - @view = ::ActionView::Base.with_view_paths(ActionController::Base.view_paths, {}) + view_paths = ActionController::Base.view_paths + view_paths.each(&:clear_cache) + ActionView::LookupContext.fallbacks.each(&:clear_cache) + @view = ::ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, {}) end teardown do From 5b4df9d0eb295d402489465f3600576038f5762d Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 23 Jan 2019 15:44:32 -0800 Subject: [PATCH 02/10] Regenerate AV::Base subclass when DetailsKey gets cleared --- actionview/lib/action_view/base.rb | 7 ++++++- actionview/lib/action_view/rendering.rb | 25 +++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 1e58004fcf..c5d8cdd409 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -185,9 +185,14 @@ module ActionView #:nodoc: template_container = Module.new Class.new(self) { include template_container - define_method(:compiled_method_container) { template_container } + define_method(:compiled_method_container) { template_container } + define_singleton_method(:compiled_method_container) { template_container } } end + + def changed?(other) # :nodoc: + compiled_method_container != other.compiled_method_container + end end attr_reader :view_renderer diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 01caa82ec6..8246e4ba6d 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -35,14 +35,35 @@ module ActionView end module ClassMethods + def _routes + end + + def _helpers + end + + def build_av_class(klass, supports_path, routes, helpers) + Class.new(klass) do + if routes + include routes.url_helpers(supports_path) + include routes.mounted_helpers + end + + if helpers + include helpers + end + end + end + def view_context_class klass = ActionView::LookupContext::DetailsKey.view_context_class(ActionView::Base) - @view_context_class ||= build_view_context_class(klass, supports_path?, _routes, _helpers) + @view_context_class ||= build_av_class(klass, supports_path?, _routes, _helpers) if klass.changed?(@view_context_class) - @view_context_class = build_view_context_class(klass, supports_path?, _routes, _helpers) + @view_context_class = build_av_class(klass, supports_path?, _routes, _helpers) end + + @view_context_class end end From 7d0ce785d4d0d3c8f31eb63b653d1c5d2a2bdad7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 23 Jan 2019 15:50:48 -0800 Subject: [PATCH 03/10] Remove finalizer and configuration --- actionview/lib/action_view/railtie.rb | 8 -------- actionview/lib/action_view/template.rb | 15 --------------- guides/source/configuring.md | 7 ------- .../templates/config/environments/test.rb.tt | 3 --- railties/test/application/configuration_test.rb | 17 ----------------- 5 files changed, 50 deletions(-) diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index 12d06bf376..12bdc642d4 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -10,7 +10,6 @@ module ActionView config.action_view.embed_authenticity_token_in_remote_forms = nil config.action_view.debug_missing_translation = true config.action_view.default_enforce_utf8 = nil - config.action_view.finalize_compiled_template_methods = true config.eager_load_namespaces << ActionView @@ -46,13 +45,6 @@ module ActionView end end - initializer "action_view.finalize_compiled_template_methods" do |app| - ActiveSupport.on_load(:action_view) do - ActionView::Template.finalize_compiled_template_methods = - app.config.action_view.delete(:finalize_compiled_template_methods) - end - end - initializer "action_view.logger" do ActiveSupport.on_load(:action_view) { self.logger ||= Rails.logger } end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 8a5407c622..69e28770eb 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -10,8 +10,6 @@ module ActionView class Template extend ActiveSupport::Autoload - mattr_accessor :finalize_compiled_template_methods, default: true - # === Encodings in ActionView::Template # # ActionView::Template is one of a few sources of potential @@ -118,16 +116,6 @@ module ActionView attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - # This finalizer is needed (and exactly with a proc inside another proc) - # otherwise templates leak in development. - Finalizer = proc do |method_name, mod| # :nodoc: - proc do - mod.module_eval do - remove_possible_method method_name - end - end - end - attr_reader :variable def initialize(source, identifier, handler, details) @@ -337,9 +325,6 @@ module ActionView end mod.module_eval(source, identifier, 0) - if finalize_compiled_template_methods - ObjectSpace.define_finalizer(self, Finalizer[method_name, mod]) - end end def handle_render_error(view, e) diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 2911b1f7c3..e7129dd274 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -590,13 +590,6 @@ Defaults to `'signed cookie'`. * `config.action_view.default_enforce_utf8` determines whether forms are generated with a hidden tag that forces older versions of Internet Explorer to submit forms encoded in UTF-8. This defaults to `false`. -* `config.action_view.finalize_compiled_template_methods` determines - whether the methods on `ActionView::CompiledTemplates` that templates - compile themselves to are removed when template instances are - destroyed by the garbage collector. This helps prevent memory leaks in - development mode, but for large test suites, disabling this option in - the test environment can improve performance. This defaults to `true`. - ### Configuring Action Mailbox diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 223aa56187..63ed3fa952 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -48,7 +48,4 @@ Rails.application.configure do # Raises error for missing translations. # config.action_view.raise_on_missing_translations = true - - # Prevent expensive template finalization at end of test suite runs. - config.action_view.finalize_compiled_template_methods = false end diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 7006b0855f..79bf7e6fd0 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2092,23 +2092,6 @@ module ApplicationTests assert_equal true, ActionView::Helpers::FormTagHelper.default_enforce_utf8 end - test "ActionView::Template.finalize_compiled_template_methods is true by default" do - app "test" - assert_equal true, ActionView::Template.finalize_compiled_template_methods - end - - test "ActionView::Template.finalize_compiled_template_methods can be configured via config.action_view.finalize_compiled_template_methods" do - app_file "config/environments/test.rb", <<-RUBY - Rails.application.configure do - config.action_view.finalize_compiled_template_methods = false - end - RUBY - - app "test" - - assert_equal false, ActionView::Template.finalize_compiled_template_methods - end - test "ActiveJob::Base.return_false_on_aborted_enqueue is true by default" do app "development" From 6dcd43b2a8a81b6131f5a569fffae6a17d337032 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 23 Jan 2019 16:11:54 -0800 Subject: [PATCH 04/10] =?UTF-8?q?=F0=9F=9A=A8=20Make=20the=20cops=20happy?= =?UTF-8?q?=20=F0=9F=9A=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- actionview/test/template/compiled_templates_test.rb | 2 +- actionview/test/template/log_subscriber_test.rb | 2 +- actionview/test/template/translation_helper_test.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/actionview/test/template/compiled_templates_test.rb b/actionview/test/template/compiled_templates_test.rb index 3d4561b55f..6375555bb4 100644 --- a/actionview/test/template/compiled_templates_test.rb +++ b/actionview/test/template/compiled_templates_test.rb @@ -7,7 +7,7 @@ class CompiledTemplatesTest < ActiveSupport::TestCase def setup super - view_paths = ActionController::Base.view_paths + view_paths = ActionController::Base.view_paths view_paths.each(&:clear_cache) ActionView::LookupContext.fallbacks.each(&:clear_cache) @view_class = ActionView::Base.with_empty_template_cache diff --git a/actionview/test/template/log_subscriber_test.rb b/actionview/test/template/log_subscriber_test.rb index 7f98207e3d..4574b798d9 100644 --- a/actionview/test/template/log_subscriber_test.rb +++ b/actionview/test/template/log_subscriber_test.rb @@ -11,7 +11,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase def setup super - view_paths = ActionController::Base.view_paths + view_paths = ActionController::Base.view_paths view_paths.each(&:clear_cache) ActionView::LookupContext.fallbacks.each(&:clear_cache) diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index 7094d4c7fc..23fc9850c4 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -36,7 +36,7 @@ class TranslationHelperTest < ActiveSupport::TestCase } } ) - view_paths = ActionController::Base.view_paths + view_paths = ActionController::Base.view_paths view_paths.each(&:clear_cache) ActionView::LookupContext.fallbacks.each(&:clear_cache) @view = ::ActionView::Base.with_empty_template_cache.with_view_paths(view_paths, {}) From 517b96207d0e083a42b81414efa9f9b6ba300e03 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 24 Jan 2019 12:43:07 -0800 Subject: [PATCH 05/10] Pull generated methods up in to the anonymous subclass Then we don't need the extra module. --- .../lib/action_dispatch/middleware/debug_view.rb | 7 +------ actionview/lib/action_view/base.rb | 11 ++++++----- actionview/test/template/render_test.rb | 9 +-------- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index 499bb1b391..f16484d1ea 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -9,11 +9,6 @@ module ActionDispatch class DebugView < ActionView::Base # :nodoc: RESCUES_TEMPLATE_PATH = File.expand_path("templates", __dir__) - module CompiledTemplates - end - - include CompiledTemplates - def initialize(assigns) paths = [RESCUES_TEMPLATE_PATH] renderer = ActionView::Renderer.new ActionView::LookupContext.new(paths) @@ -21,7 +16,7 @@ module ActionDispatch end def compiled_method_container - CompiledTemplates + self.class end def debug_params(params) diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index c5d8cdd409..71aa68499d 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -182,11 +182,12 @@ module ActionView #:nodoc: end def with_empty_template_cache # :nodoc: - template_container = Module.new - Class.new(self) { - include template_container - define_method(:compiled_method_container) { template_container } - define_singleton_method(:compiled_method_container) { template_container } + subclass = Class.new(self) { + # We can't implement these as self.class because subclasses will + # share the same template cache as superclasses, so "changed?" won't work + # correctly. + define_method(:compiled_method_container) { subclass } + define_singleton_method(:compiled_method_container) { subclass } } end diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 372b85a4a5..b8d8717db4 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -19,15 +19,8 @@ module RenderTestCases end.with_view_paths(paths, @assigns) controller = TestController.new - view = @view - @controller_view = Class.new(controller.view_context_class) do - include view.compiled_method_container - - define_method(:compiled_method_container) do - view.compiled_method_container - end - end.new(controller.view_renderer, controller.view_assigns, controller) + @controller_view = controller.view_context_class.with_empty_template_cache.new(controller.view_renderer, controller.view_assigns, controller) # Reload and register danish language for testing I18n.backend.store_translations "da", {} From 8fc73067e3c11810af374e886a41b9f3611a8873 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 24 Jan 2019 12:45:21 -0800 Subject: [PATCH 06/10] Rename method so it is more descriptive --- actionview/lib/action_view/rendering.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/actionview/lib/action_view/rendering.rb b/actionview/lib/action_view/rendering.rb index 8246e4ba6d..cf7d1105e0 100644 --- a/actionview/lib/action_view/rendering.rb +++ b/actionview/lib/action_view/rendering.rb @@ -41,7 +41,7 @@ module ActionView def _helpers end - def build_av_class(klass, supports_path, routes, helpers) + def build_view_context_class(klass, supports_path, routes, helpers) Class.new(klass) do if routes include routes.url_helpers(supports_path) @@ -57,10 +57,10 @@ module ActionView def view_context_class klass = ActionView::LookupContext::DetailsKey.view_context_class(ActionView::Base) - @view_context_class ||= build_av_class(klass, supports_path?, _routes, _helpers) + @view_context_class ||= build_view_context_class(klass, supports_path?, _routes, _helpers) if klass.changed?(@view_context_class) - @view_context_class = build_av_class(klass, supports_path?, _routes, _helpers) + @view_context_class = build_view_context_class(klass, supports_path?, _routes, _helpers) end @view_context_class From 7761ddbebb5627769cd9fa9d9f1ff5a072716b47 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 24 Jan 2019 13:50:24 -0800 Subject: [PATCH 07/10] Deprecate finalizer configuration (it doesn't do anything) Revert "Remove finalizer and configuration" This reverts commit 9e7b4a3173788ea43b11e74a4d2f69a5f1565daa. --- actionview/CHANGELOG.md | 6 ++++++ actionview/lib/action_view/railtie.rb | 13 ++++++++++++ actionview/lib/action_view/template.rb | 9 ++++++++ .../test/application/configuration_test.rb | 21 +++++++++++++++++++ 4 files changed, 49 insertions(+) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 5e17e65bde..ecc02d510b 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,9 @@ +* ActionView::Template.finalize_compiled_template_methods is deprecated with + no replacement. + +* config.action_view.finalize_compiled_template_methods is deprecated with + no replacement. + * Ensure unique DOM IDs for collection inputs with float values. Fixes #34974 diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb index 12bdc642d4..a25e1d3d02 100644 --- a/actionview/lib/action_view/railtie.rb +++ b/actionview/lib/action_view/railtie.rb @@ -6,10 +6,13 @@ require "rails" module ActionView # = Action View Railtie class Railtie < Rails::Engine # :nodoc: + NULL_OPTION = Object.new + config.action_view = ActiveSupport::OrderedOptions.new config.action_view.embed_authenticity_token_in_remote_forms = nil config.action_view.debug_missing_translation = true config.action_view.default_enforce_utf8 = nil + config.action_view.finalize_compiled_template_methods = NULL_OPTION config.eager_load_namespaces << ActionView @@ -45,6 +48,16 @@ module ActionView end end + initializer "action_view.finalize_compiled_template_methods" do |app| + ActiveSupport.on_load(:action_view) do + option = app.config.action_view.delete(:finalize_compiled_template_methods) + + if option != NULL_OPTION + ActiveSupport::Deprecation.warn "action_view.finalize_compiled_template_methods is deprecated and has no effect" + end + end + end + initializer "action_view.logger" do ActiveSupport.on_load(:action_view) { self.logger ||= Rails.logger } end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 69e28770eb..37f476e554 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/object/try" require "active_support/core_ext/kernel/singleton_class" +require "active_support/deprecation" require "thread" require "delegate" @@ -10,6 +11,14 @@ module ActionView class Template extend ActiveSupport::Autoload + def self.finalize_compiled_template_methods + ActiveSupport::Deprecation.warn "ActionView::Template.finalize_compiled_template_methods is deprecated and has no effect" + end + + def self.finalize_compiled_template_methods=(_) + ActiveSupport::Deprecation.warn "ActionView::Template.finalize_compiled_template_methods= is deprecated and has no effect" + end + # === Encodings in ActionView::Template # # ActionView::Template is one of a few sources of potential diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 79bf7e6fd0..37fba72ee3 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -2092,6 +2092,27 @@ module ApplicationTests assert_equal true, ActionView::Helpers::FormTagHelper.default_enforce_utf8 end + test "ActionView::Template.finalize_compiled_template_methods is true by default" do + app "test" + assert_deprecated do + ActionView::Template.finalize_compiled_template_methods + end + end + + test "ActionView::Template.finalize_compiled_template_methods can be configured via config.action_view.finalize_compiled_template_methods" do + app_file "config/environments/test.rb", <<-RUBY + Rails.application.configure do + config.action_view.finalize_compiled_template_methods = false + end + RUBY + + app "test" + + assert_deprecated do + ActionView::Template.finalize_compiled_template_methods + end + end + test "ActiveJob::Base.return_false_on_aborted_enqueue is true by default" do app "development" From 9aa9b6d6e12b1b900f783abc40f50e4771f55d6f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 24 Jan 2019 13:59:26 -0800 Subject: [PATCH 08/10] Add a message to help allocate AV::Base instances --- actionview/lib/action_view/base.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index 71aa68499d..df87de4406 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -268,7 +268,11 @@ module ActionView #:nodoc: end def compiled_method_container - raise NotImplementedError + raise NotImplementedError, <<~msg + Subclasses of ActionView::Base must implement `compiled_method_container` + or use the class method `with_empty_template_cache` for constructing + an ActionView::Base subclass thata has an empty cache. + msg end ActiveSupport.run_load_hooks(:action_view, self) From cf0dd4a71da69cf7f39da59a992f04b3c7bcbb19 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 24 Jan 2019 14:30:40 -0800 Subject: [PATCH 09/10] Fix some typos! --- actionview/CHANGELOG.md | 4 ++++ actionview/lib/action_view/base.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index ecc02d510b..5e7b271fb9 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,9 +1,13 @@ * ActionView::Template.finalize_compiled_template_methods is deprecated with no replacement. + *tenderlove* + * config.action_view.finalize_compiled_template_methods is deprecated with no replacement. + *tenderlove* + * Ensure unique DOM IDs for collection inputs with float values. Fixes #34974 diff --git a/actionview/lib/action_view/base.rb b/actionview/lib/action_view/base.rb index df87de4406..420136d6de 100644 --- a/actionview/lib/action_view/base.rb +++ b/actionview/lib/action_view/base.rb @@ -271,7 +271,7 @@ module ActionView #:nodoc: raise NotImplementedError, <<~msg Subclasses of ActionView::Base must implement `compiled_method_container` or use the class method `with_empty_template_cache` for constructing - an ActionView::Base subclass thata has an empty cache. + an ActionView::Base subclass that has an empty cache. msg end From 570bcdaa65987ac2f5cc84fdf83678cd5c0bb7d8 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 6 Feb 2019 16:50:06 -0800 Subject: [PATCH 10/10] Fix deprecation warnings and call super --- actionview/test/template/template_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb index 8af6739e45..a069c8f2d0 100644 --- a/actionview/test/template/template_test.rb +++ b/actionview/test/template/template_test.rb @@ -19,7 +19,7 @@ class TestERBTemplate < ActiveSupport::TestCase end class Context < ActionView::Base - def initialize + def initialize(*) super @output_buffer = "original" @virtual_path = nil @@ -64,7 +64,7 @@ class TestERBTemplate < ActiveSupport::TestCase end def setup - @context = Context.with_empty_template_cache.new + @context = Context.with_empty_template_cache.empty super end