Skip to content

Commit

Permalink
Introduce a file type template, deprecate Template#refresh
Browse files Browse the repository at this point in the history
Every template that specifies a "virtual path" loses the template source
when the template gets compiled:

  https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template.rb#L275

The "refresh" method seems to think that the source code for a template
can be recovered if there is a virtual path:

  https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template.rb#L171-L188

Every call site that allocates a template object *and* provides a
"virtual path" reads the template contents from the filesystem:

  https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/template/resolver.rb#L229-L231

Templates that are inline or literals don't provide a "virtual path":

  https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/renderer/template_renderer.rb#L34

This commit introduces a `FileTemplate` type that subclasses `Template`.
The `FileTemplate` keeps a reference to the filename, and reads the
source from the filesystem.  This effectively makes the template source
immutable.

Other classes depended on the source to be mutated while being compiled,
so this commit also introduces a temporary way to pass the mutated
source to the ERB (or whatever) compiler.  See `LegacyTemplate`.

I think we should consider it an error to provide a virtual path on a
non file type template an non-file templates can't recover their source.
Here is an example:

  https://github.com/rails/rails/blob/eda0f574f129fcd5ad1fc58b55cb6d1db71ea95c/actionview/lib/action_view/testing/resolvers.rb#L53

This provides a "virtual path" so the source code (a string literal) is
thrown away after compilation.  Clearly we can't recover that string, so
I think this should be an error.
  • Loading branch information
tenderlove committed Feb 1, 2019
1 parent 916b74d commit 2169bd3
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
1 change: 1 addition & 0 deletions actionview/lib/action_view.rb
Expand Up @@ -45,6 +45,7 @@ module ActionView
autoload :Rendering
autoload :RoutingUrlFor
autoload :Template
autoload :FileTemplate
autoload :ViewPaths

autoload_under "renderer" do
Expand Down
33 changes: 33 additions & 0 deletions actionview/lib/action_view/file_template.rb
@@ -0,0 +1,33 @@
# 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, @original_encoding, @locals, @virtual_path, @updated_at, @formats, @variants ]
end

def marshal_load(array) # :nodoc:
@identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @formats, @variants = *array
@compile_mutex = Mutex.new
end
end
end
13 changes: 12 additions & 1 deletion actionview/lib/action_view/template.rb
Expand Up @@ -2,7 +2,9 @@

require "active_support/core_ext/object/try"
require "active_support/core_ext/kernel/singleton_class"
require "active_support/deprecation"
require "thread"
require "delegate"

module ActionView
# = Action View Template
Expand Down Expand Up @@ -279,6 +281,15 @@ def compile!(view)
end
end

class LegacyTemplate < DelegateClass(Template) # :nodoc:
attr_reader :source

def initialize(template, source)
super(template)
@source = source
end
end

# Among other things, this method is responsible for properly setting
# the encoding of the compiled template.
#
Expand All @@ -293,7 +304,7 @@ def compile!(view)
# regardless of the original source encoding.
def compile(mod)
source = encode!
code = @handler.call(self)
code = @handler.call(LegacyTemplate.new(self, source))

# Make sure that the resulting String to be eval'd is in the
# encoding of the code
Expand Down
3 changes: 1 addition & 2 deletions actionview/lib/action_view/template/resolver.rb
Expand Up @@ -226,9 +226,8 @@ def query(path, details, formats, outside_app_allowed)

template_paths.map do |template|
handler, format, variant = extract_handler_and_format_and_variant(template)
contents = File.binread(template)

Template.new(contents, File.expand_path(template), handler,
FileTemplate.new(File.expand_path(template), handler,
virtual_path: path.virtual,
format: format,
variant: variant,
Expand Down

0 comments on commit 2169bd3

Please sign in to comment.