diff options
| author | Aaron Patterson <aaron.patterson@gmail.com> | 2019-01-31 15:34:09 -0800 |
|---|---|---|
| committer | Aaron Patterson <aaron.patterson@gmail.com> | 2019-02-01 12:09:46 -0800 |
| commit | 2169bd3d2a9d2f331a5dd6e41d9d638e0da6117c (patch) | |
| tree | 9ce840ce73e54eb765018c5dbe32a4e3650528c8 /actionview/lib | |
| parent | 916b74d16a5ed369ba58c17630f593470ca502a8 (diff) | |
| download | rails-2169bd3d2a9d2f331a5dd6e41d9d638e0da6117c.tar.gz rails-2169bd3d2a9d2f331a5dd6e41d9d638e0da6117c.tar.bz2 rails-2169bd3d2a9d2f331a5dd6e41d9d638e0da6117c.zip | |
Introduce a file type template, deprecate `Template#refresh`
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.
Diffstat (limited to 'actionview/lib')
| -rw-r--r-- | actionview/lib/action_view.rb | 1 | ||||
| -rw-r--r-- | actionview/lib/action_view/file_template.rb | 33 | ||||
| -rw-r--r-- | actionview/lib/action_view/template.rb | 13 | ||||
| -rw-r--r-- | actionview/lib/action_view/template/resolver.rb | 3 |
4 files changed, 47 insertions, 3 deletions
diff --git a/actionview/lib/action_view.rb b/actionview/lib/action_view.rb index c4a614fa6d..6ff2d70e35 100644 --- a/actionview/lib/action_view.rb +++ b/actionview/lib/action_view.rb @@ -45,6 +45,7 @@ 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 new file mode 100644 index 0000000000..4921074383 --- /dev/null +++ b/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 diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index a283d2467c..2d1762e568 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -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 @@ -279,6 +281,15 @@ module ActionView 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. # @@ -293,7 +304,7 @@ module ActionView # 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 diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 12ae82f8c5..3b4594942b 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -226,9 +226,8 @@ module ActionView 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, |