Merge pull request #35865 from jhawthorn/template_file_source

Introduce Template::Source::File, replacing FileTemplate and Template#refresh

7 files changed, 46 insertions, 83 deletions

diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb
index f398ba0ff0..5ee14bfc78 100644
--- a/actionview/lib/action_view.rb
+++ b/actionview/lib/action_view.rb
@@ -44,7 +44,6 @@ module ActionView
     autoload :Rendering
     autoload :RoutingUrlFor
     autoload :Template
-    autoload :FileTemplate
     autoload :ViewPaths
 
     autoload_under "renderer" do
diff --git a/actionview/lib/action_view/file_template.rb b/actionview/lib/action_view/file_template.rb
deleted file mode 100644
index dea02176eb..0000000000
--- a/actionview/lib/action_view/file_template.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-# frozen_string_literal: true
-
-require "action_view/template"
-
-module ActionView
-  class FileTemplate < Template
-    def initialize(filename, handler, details)
-      @filename = filename
-
-      super(nil, filename, handler, details)
-    end
-
-    def source
-      File.binread @filename
-    end
-
-    def refresh(_)
-      self
-    end
-
-    # Exceptions are marshalled when using the parallel test runner with DRb, so we need
-    # to ensure that references to the template object can be marshalled as well. This means forgoing
-    # the marshalling of the compiler mutex and instantiating that again on unmarshalling.
-    def marshal_dump # :nodoc:
-      [ @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant ]
-    end
-
-    def marshal_load(array) # :nodoc:
-      @identifier, @handler, @compiled, @locals, @virtual_path, @updated_at, @format, @variant = *array
-      @compile_mutex = Mutex.new
-    end
-  end
-end
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
index 94f8a194a0..b80bf56c1b 100644
--- a/actionview/lib/action_view/template.rb
+++ b/actionview/lib/action_view/template.rb
@@ -117,13 +117,14 @@ module ActionView
       autoload :Handlers
       autoload :HTML
       autoload :Inline
+      autoload :Sources
       autoload :Text
       autoload :Types
     end
 
     extend Template::Handlers
 
-    attr_reader :source, :identifier, :handler, :original_encoding, :updated_at
+    attr_reader :identifier, :handler, :original_encoding, :updated_at
     attr_reader :variable, :format, :variant, :locals, :virtual_path
 
     def initialize(source, identifier, handler, format: nil, variant: nil, locals: nil, virtual_path: nil, updated_at: nil)
@@ -164,6 +165,7 @@ module ActionView
     deprecate def formats; Array(format); end
     deprecate def variants=(_); end
     deprecate def variants; [variant]; end
+    deprecate def refresh(_); self; end
 
     # Returns whether the underlying handler supports streaming. If so,
     # a streaming buffer *may* be passed when it starts rendering.
@@ -190,25 +192,6 @@ module ActionView
       @type ||= Types[format]
     end
 
-    # Receives a view object and return a template similar to self by using @virtual_path.
-    #
-    # This method is useful if you have a template object but it does not contain its source
-    # anymore since it was already compiled. In such cases, all you need to do is to call
-    # refresh passing in the view object.
-    #
-    # Notice this method raises an error if the template to be refreshed does not have a
-    # virtual path set (true just for inline templates).
-    def refresh(view)
-      raise "A template needs to have a virtual path in order to be refreshed" unless @virtual_path
-      lookup  = view.lookup_context
-      pieces  = @virtual_path.split("/")
-      name    = pieces.pop
-      partial = !!name.sub!(/^_/, "")
-      lookup.disable_cache do
-        lookup.find_template(name, [ pieces.join("/") ], partial, @locals)
-      end
-    end
-
     def short_identifier
       @short_identifier ||= defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier
     end
@@ -217,6 +200,10 @@ module ActionView
       "#<#{self.class.name} #{short_identifier} locals=#{@locals.inspect}>"
     end
 
+    def source
+      @source.to_s
+    end
+
     # This method is responsible for properly setting the encoding of the
     # source. Until this point, we assume that the source is BINARY data.
     # If no additional information is supplied, we assume the encoding is
@@ -298,9 +285,6 @@ module ActionView
             compile(mod)
           end
 
-          # Just discard the source if we have a virtual path. This
-          # means we can get the template back.
-          @source = nil if @virtual_path
           @compiled = true
         end
       end
@@ -368,12 +352,7 @@ module ActionView
           e.sub_template_of(self)
           raise e
         else
-          template = self
-          unless template.source
-            template = refresh(view)
-            template.encode!
-          end
-          raise Template::Error.new(template)
+          raise Template::Error.new(self)
         end
       end
 
diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb
index d5cb3c823a..e291dc268a 100644
--- a/actionview/lib/action_view/template/resolver.rb
+++ b/actionview/lib/action_view/template/resolver.rb
@@ -191,7 +191,9 @@ module ActionView
       def build_template(template, virtual_path, locals)
         handler, format, variant = extract_handler_and_format_and_variant(template)
 
-        FileTemplate.new(File.expand_path(template), handler,
+        filename = File.expand_path(template)
+        source = Template::Sources::File.new(filename)
+        Template.new(source, filename, handler,
           virtual_path: virtual_path,
           format: format,
           variant: variant,
diff --git a/actionview/lib/action_view/template/sources.rb b/actionview/lib/action_view/template/sources.rb
new file mode 100644
index 0000000000..8b89535741
--- /dev/null
+++ b/actionview/lib/action_view/template/sources.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+module ActionView
+  class Template
+    module Sources
+      extend ActiveSupport::Autoload
+
+      eager_autoload do
+        autoload :File
+      end
+    end
+  end
+end
diff --git a/actionview/lib/action_view/template/sources/file.rb b/actionview/lib/action_view/template/sources/file.rb
new file mode 100644
index 0000000000..2d3a7dec7f
--- /dev/null
+++ b/actionview/lib/action_view/template/sources/file.rb
@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module ActionView
+  class Template
+    module Sources
+      class File
+        def initialize(filename)
+          @filename = filename
+        end
+
+        def to_s
+          ::File.binread @filename
+        end
+      end
+    end
+  end
+end
diff --git a/actionview/test/template/template_test.rb b/actionview/test/template/template_test.rb
index 71fb99115b..049e0bc129 100644
--- a/actionview/test/template/template_test.rb
+++ b/actionview/test/template/template_test.rb
@@ -91,10 +91,10 @@ class TestERBTemplate < ActiveSupport::TestCase
     assert_equal "<%= hello %>", render
   end
 
-  def test_template_loses_its_source_after_rendering
+  def test_template_does_not_lose_its_source_after_rendering
     @template = new_template
     render
-    assert_nil @template.source
+    assert_equal "<%= hello %>", @template.source
   end
 
   def test_template_does_not_lose_its_source_after_rendering_if_it_does_not_have_a_virtual_path
@@ -121,24 +121,10 @@ class TestERBTemplate < ActiveSupport::TestCase
     assert_equal "hellopartialhello", render
   end
 
-  def test_refresh_with_templates
+  def test_refresh_is_deprecated
     @template = new_template("Hello", virtual_path: "test/foo/bar", locals: [:key])
-    assert_called_with(@context.lookup_context, :find_template, ["bar", %w(test/foo), false, [:key]], returns: "template") do
-      assert_equal "template", @template.refresh(@context)
-    end
-  end
-
-  def test_refresh_with_partials
-    @template = new_template("Hello", virtual_path: "test/_foo", locals: [:key])
-    assert_called_with(@context.lookup_context, :find_template, ["foo", %w(test), true, [:key]], returns: "partial") do
-      assert_equal "partial", @template.refresh(@context)
-    end
-  end
-
-  def test_refresh_raises_an_error_without_virtual_path
-    @template = new_template("Hello", virtual_path: nil)
-    assert_raise RuntimeError do
-      @template.refresh(@context)
+    assert_deprecated do
+      assert_same @template, @template.refresh(@context)
     end
   end