From 28f88e0074f473f58c2d3fd54cb3daff81027c12 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 1 Feb 2019 16:10:02 -0800 Subject: [PATCH] Pass source to template handler and deprecate old style handler This commit passes the mutated source to the template handler as a parameter and deprecates the old handlers. Old handlers required that templates contain a reference to mutated source code, but we would like to make template objects "read only". This change lets the template remain "read only" while still giving template handlers access to the source code after mutations. --- actionview/lib/action_view/template.rb | 3 +- .../lib/action_view/template/handlers.rb | 28 ++++++++++++++++++- .../action_view/template/handlers/builder.rb | 4 +-- .../lib/action_view/template/handlers/erb.rb | 10 +++---- .../lib/action_view/template/handlers/html.rb | 2 +- .../lib/action_view/template/handlers/raw.rb | 4 +-- actionview/test/abstract_unit.rb | 2 +- .../test/actionpack/controller/layout_test.rb | 4 +-- .../test/template/dependency_tracker_test.rb | 4 +-- actionview/test/template/render_test.rb | 15 ++++++++-- 10 files changed, 55 insertions(+), 21 deletions(-) diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 2d1762e568610..3b2c264ed45b4 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -2,7 +2,6 @@ require "active_support/core_ext/object/try" require "active_support/core_ext/kernel/singleton_class" -require "active_support/deprecation" require "thread" require "delegate" @@ -304,7 +303,7 @@ def initialize(template, source) # regardless of the original source encoding. def compile(mod) source = encode! - code = @handler.call(LegacyTemplate.new(self, source)) + code = @handler.call(self, source) # Make sure that the resulting String to be eval'd is in the # encoding of the code diff --git a/actionview/lib/action_view/template/handlers.rb b/actionview/lib/action_view/template/handlers.rb index 7ec76dcc3f759..f2a2341b8eba1 100644 --- a/actionview/lib/action_view/template/handlers.rb +++ b/actionview/lib/action_view/template/handlers.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/deprecation" + module ActionView #:nodoc: # = Action View Template Handlers class Template #:nodoc: @@ -14,7 +16,7 @@ def self.extended(base) base.register_template_handler :erb, ERB.new base.register_template_handler :html, Html.new base.register_template_handler :builder, Builder.new - base.register_template_handler :ruby, :source.to_proc + base.register_template_handler :ruby, lambda { |_, source| source } end @@template_handlers = {} @@ -24,11 +26,35 @@ def self.extensions @@template_extensions ||= @@template_handlers.keys end + class LegacyHandlerWrapper < SimpleDelegator # :nodoc: + def call(view, source) + __getobj__.call(ActionView::Template::LegacyTemplate.new(view, source)) + end + end + # Register an object that knows how to handle template files with the given # extensions. This can be used to implement new template types. # The handler must respond to +:call+, which will be passed the template # and should return the rendered template as a String. def register_template_handler(*extensions, handler) + params = if handler.is_a?(Proc) + handler.parameters + else + handler.method(:call).parameters + end + + unless params.find_all { |type, _| type == :req }.length >= 2 + ActiveSupport::Deprecation.warn <<~eowarn + Single arity template handlers are deprecated. Template handlers must + now accept two parameters, the view object and the source for the view object. + Change: + >> #{handler.class}#call(#{params.map(&:last).join(", ")}) + To: + >> #{handler.class}#call(#{params.map(&:last).join(", ")}, source) + eowarn + handler = LegacyHandlerWrapper.new(handler) + end + raise(ArgumentError, "Extension is required") if extensions.empty? extensions.each do |extension| @@template_handlers[extension.to_sym] = handler diff --git a/actionview/lib/action_view/template/handlers/builder.rb b/actionview/lib/action_view/template/handlers/builder.rb index 61492ce448c98..e5413cd2a0d50 100644 --- a/actionview/lib/action_view/template/handlers/builder.rb +++ b/actionview/lib/action_view/template/handlers/builder.rb @@ -5,11 +5,11 @@ module Template::Handlers class Builder class_attribute :default_format, default: :xml - def call(template) + def call(template, source) require_engine "xml = ::Builder::XmlMarkup.new(:indent => 2);" \ "self.output_buffer = xml.target!;" + - template.source + + source + ";xml.target!;" end diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb index 7d1a6767d7e83..b6314a5bc39fb 100644 --- a/actionview/lib/action_view/template/handlers/erb.rb +++ b/actionview/lib/action_view/template/handlers/erb.rb @@ -28,8 +28,8 @@ class ERB ENCODING_TAG = Regexp.new("\\A(<%#{ENCODING_FLAG}-?%>)[ \\t]*") - def self.call(template) - new.call(template) + def self.call(template, source) + new.call(template, source) end def supports_streaming? @@ -40,17 +40,17 @@ def handles_encoding? true end - def call(template) + def call(template, source) # First, convert to BINARY, so in case the encoding is # wrong, we can still find an encoding tag # (<%# encoding %>) inside the String using a regular # expression - template_source = template.source.dup.force_encoding(Encoding::ASCII_8BIT) + template_source = source.dup.force_encoding(Encoding::ASCII_8BIT) erb = template_source.gsub(ENCODING_TAG, "") encoding = $2 - erb.force_encoding valid_encoding(template.source.dup, encoding) + erb.force_encoding valid_encoding(source.dup, encoding) # Always make sure we return a String in the default_internal erb.encode! diff --git a/actionview/lib/action_view/template/handlers/html.rb b/actionview/lib/action_view/template/handlers/html.rb index 27004a318cd2d..65857d8587cde 100644 --- a/actionview/lib/action_view/template/handlers/html.rb +++ b/actionview/lib/action_view/template/handlers/html.rb @@ -3,7 +3,7 @@ module ActionView module Template::Handlers class Html < Raw - def call(template) + def call(template, source) "ActionView::OutputBuffer.new #{super}" end end diff --git a/actionview/lib/action_view/template/handlers/raw.rb b/actionview/lib/action_view/template/handlers/raw.rb index 5cd23a006067c..57ebb169fcad7 100644 --- a/actionview/lib/action_view/template/handlers/raw.rb +++ b/actionview/lib/action_view/template/handlers/raw.rb @@ -3,8 +3,8 @@ module ActionView module Template::Handlers class Raw - def call(template) - "#{template.source.inspect}.html_safe;" + def call(template, source) + "#{source.inspect}.html_safe;" end end end diff --git a/actionview/test/abstract_unit.rb b/actionview/test/abstract_unit.rb index 56d617309c3cb..acc2ed029b567 100644 --- a/actionview/test/abstract_unit.rb +++ b/actionview/test/abstract_unit.rb @@ -58,7 +58,7 @@ def render_erb(string) template = ActionView::Template.new( string.strip, "test template", - ActionView::Template::Handlers::ERB, + ActionView::Template.handler_for_extension(:erb), {}) template.render(ActionView::Base.empty, {}).strip diff --git a/actionview/test/actionpack/controller/layout_test.rb b/actionview/test/actionpack/controller/layout_test.rb index 6d5c97b7fdd75..838b564c5de94 100644 --- a/actionview/test/actionpack/controller/layout_test.rb +++ b/actionview/test/actionpack/controller/layout_test.rb @@ -70,7 +70,7 @@ def test_controller_name_layout_name_match end def test_third_party_template_library_auto_discovers_layout - with_template_handler :mab, lambda { |template| template.source.inspect } do + with_template_handler :mab, lambda { |template, source| source.inspect } do @controller = ThirdPartyTemplateLibraryController.new get :hello assert_response :success @@ -212,7 +212,7 @@ def test_layout_except_exception_when_excepted end def test_layout_set_when_using_render - with_template_handler :mab, lambda { |template| template.source.inspect } do + with_template_handler :mab, lambda { |template, source| source.inspect } do @controller = SetsLayoutInRenderController.new get :hello assert_includes @response.body, "layouts/third_party_template_library.mab" diff --git a/actionview/test/template/dependency_tracker_test.rb b/actionview/test/template/dependency_tracker_test.rb index ef7aeac039133..42cb14a18a35b 100644 --- a/actionview/test/template/dependency_tracker_test.rb +++ b/actionview/test/template/dependency_tracker_test.rb @@ -17,8 +17,8 @@ def initialize(source, handler = Neckbeard) end end -Neckbeard = lambda { |template| template.source } -Bowtie = lambda { |template| template.source } +Neckbeard = lambda { |template, source| source } +Bowtie = lambda { |template, source| source } class DependencyTrackerTest < ActionView::TestCase def tracker diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb index 76ffe3415def4..f5d251da20d56 100644 --- a/actionview/test/template/render_test.rb +++ b/actionview/test/template/render_test.rb @@ -450,13 +450,22 @@ def test_render_fallbacks_to_erb_for_unknown_types assert_equal "Hello, World!", @view.render(inline: "Hello, World!", type: :bar) end - CustomHandler = lambda do |template| + CustomHandler = lambda do |template, source| "@output_buffer = ''.dup\n" \ - "@output_buffer << 'source: #{template.source.inspect}'\n" + "@output_buffer << 'source: #{source.inspect}'\n" end def test_render_inline_with_render_from_to_proc - ActionView::Template.register_template_handler :ruby_handler, :source.to_proc + ActionView::Template.register_template_handler :ruby_handler, lambda { |_, source| source } + assert_equal "3", @view.render(inline: "(1 + 2).to_s", type: :ruby_handler) + ensure + ActionView::Template.unregister_template_handler :ruby_handler + end + + def test_render_inline_with_render_from_to_proc_deprecated + assert_deprecated do + ActionView::Template.register_template_handler :ruby_handler, :source.to_proc + end assert_equal "3", @view.render(inline: "(1 + 2).to_s", type: :ruby_handler) ensure ActionView::Template.unregister_template_handler :ruby_handler