From ca5e23ed4d8818d81314953aadd422b2cbde63b0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 25 Feb 2019 13:18:44 -0800 Subject: Templates have one format Templates only have one format. Before this commit, templates would be constructed with a single element array that contained the format. This commit eliminates the single element array and just implements a `format` method. This saves one array allocation per template. --- actionmailer/lib/action_mailer/base.rb | 4 ++-- actionview/lib/action_view/file_template.rb | 4 ++-- actionview/lib/action_view/helpers/cache_helper.rb | 2 +- .../lib/action_view/renderer/abstract_renderer.rb | 2 +- .../lib/action_view/renderer/template_renderer.rb | 2 +- actionview/lib/action_view/template.rb | 13 +++++++------ actionview/lib/action_view/template/html.rb | 19 ++++++++++++++----- actionview/lib/action_view/template/text.rb | 8 +++++--- .../test/actionpack/controller/view_paths_test.rb | 2 +- actionview/test/template/digestor_test.rb | 2 +- actionview/test/template/html_test.rb | 6 +++--- actionview/test/template/lookup_context_test.rb | 2 +- actionview/test/template/resolver_patterns_test.rb | 2 +- .../test/template/testing/fixture_resolver_test.rb | 2 +- .../test/template/testing/null_resolver_test.rb | 2 +- actionview/test/template/text_test.rb | 2 +- 16 files changed, 43 insertions(+), 31 deletions(-) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 033a7f5b9e..52360d40fe 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -973,7 +973,7 @@ module ActionMailer templates_name = headers[:template_name] || action_name each_template(Array(templates_path), templates_name).map do |template| - self.formats = template.formats + self.formats = [template.format] { body: render(template: template), content_type: template.type.to_s @@ -986,7 +986,7 @@ module ActionMailer if templates.empty? raise ActionView::MissingTemplate.new(paths, name, paths, false, "mailer") else - templates.uniq(&:formats).each(&block) + templates.uniq(&:format).each(&block) end end diff --git a/actionview/lib/action_view/file_template.rb b/actionview/lib/action_view/file_template.rb index 4921074383..43206f02e5 100644 --- a/actionview/lib/action_view/file_template.rb +++ b/actionview/lib/action_view/file_template.rb @@ -22,11 +22,11 @@ module ActionView # 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 ] + [ @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variants ] end def marshal_load(array) # :nodoc: - @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @formats, @variants = *array + @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variants = *array @compile_mutex = Mutex.new end end diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb index 6b69b71947..020aebeea3 100644 --- a/actionview/lib/action_view/helpers/cache_helper.rb +++ b/actionview/lib/action_view/helpers/cache_helper.rb @@ -217,7 +217,7 @@ module ActionView end def digest_path_from_template(template) # :nodoc: - digest = Digestor.digest(name: template.virtual_path, format: template.formats.first, finder: lookup_context, dependencies: view_cache_dependencies) + digest = Digestor.digest(name: template.virtual_path, format: template.format, finder: lookup_context, dependencies: view_cache_dependencies) if digest.present? "#{template.virtual_path}:#{digest}" diff --git a/actionview/lib/action_view/renderer/abstract_renderer.rb b/actionview/lib/action_view/renderer/abstract_renderer.rb index f1b4c9b92d..475452f1bb 100644 --- a/actionview/lib/action_view/renderer/abstract_renderer.rb +++ b/actionview/lib/action_view/renderer/abstract_renderer.rb @@ -68,7 +68,7 @@ module ActionView end def format - template.formats.first + template.format end EMPTY_SPACER = Struct.new(:body).new diff --git a/actionview/lib/action_view/renderer/template_renderer.rb b/actionview/lib/action_view/renderer/template_renderer.rb index 87f6cf3de3..83b990b081 100644 --- a/actionview/lib/action_view/renderer/template_renderer.rb +++ b/actionview/lib/action_view/renderer/template_renderer.rb @@ -8,7 +8,7 @@ module ActionView @details = extract_details(options) template = determine_template(options) - prepend_formats(template.formats) + prepend_formats(template.format) render_template(context, template, options[:layout], options[:locals] || {}) end diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb index 8ba03027d3..cc5e0cbb4f 100644 --- a/actionview/lib/action_view/template.rb +++ b/actionview/lib/action_view/template.rb @@ -125,8 +125,7 @@ module ActionView attr_accessor :locals, :variants, :virtual_path attr_reader :source, :identifier, :handler, :original_encoding, :updated_at - - attr_reader :variable, :formats + attr_reader :variable, :format def initialize(source, identifier, handler, format: nil, **details) unless format @@ -149,7 +148,7 @@ module ActionView end @updated_at = details[:updated_at] || Time.now - @formats = Array(format) + @format = format @variants = [details[:variant]] @compile_mutex = Mutex.new end @@ -158,6 +157,8 @@ module ActionView end deprecate :formats= + deprecate def formats; Array(format); end + # Returns whether the underlying handler supports streaming. If so, # a streaming buffer *may* be passed when it starts rendering. def supports_streaming? @@ -180,7 +181,7 @@ module ActionView end def type - @type ||= Types[@formats.first] if @formats.first + @type ||= Types[format] end # Receives a view object and return a template similar to self by using @virtual_path. @@ -257,11 +258,11 @@ module ActionView # 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: - [ @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @formats, @variants ] + [ @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variants ] end def marshal_load(array) # :nodoc: - @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @formats, @variants = *array + @source, @identifier, @handler, @compiled, @original_encoding, @locals, @virtual_path, @updated_at, @format, @variants = *array @compile_mutex = Mutex.new end diff --git a/actionview/lib/action_view/template/html.rb b/actionview/lib/action_view/template/html.rb index a262c6d9ad..ecd1c31e79 100644 --- a/actionview/lib/action_view/template/html.rb +++ b/actionview/lib/action_view/template/html.rb @@ -1,15 +1,21 @@ # frozen_string_literal: true +require "active_support/deprecation" + module ActionView #:nodoc: # = Action View HTML Template class Template #:nodoc: class HTML #:nodoc: - attr_accessor :type + attr_reader :type def initialize(string, type = nil) + unless type + ActiveSupport::Deprecation.warn "ActionView::Template::HTML#initialize requires a type parameter" + type = :html + end + @string = string.to_s - @type = Types[type] || type if type - @type ||= Types[:html] + @type = type end def identifier @@ -26,9 +32,12 @@ module ActionView #:nodoc: to_str end - def formats - [@type.respond_to?(:ref) ? @type.ref : @type.to_s] + def format + @type end + + def formats; Array(format); end + deprecate :formats end end end diff --git a/actionview/lib/action_view/template/text.rb b/actionview/lib/action_view/template/text.rb index f8d6c2811f..c5fd55f1b3 100644 --- a/actionview/lib/action_view/template/text.rb +++ b/actionview/lib/action_view/template/text.rb @@ -8,7 +8,6 @@ module ActionView #:nodoc: def initialize(string) @string = string.to_s - @type = Types[:text] end def identifier @@ -25,9 +24,12 @@ module ActionView #:nodoc: to_str end - def formats - [@type.ref] + def format + :text end + + def formats; Array(format); end + deprecate :formats end end end diff --git a/actionview/test/actionpack/controller/view_paths_test.rb b/actionview/test/actionpack/controller/view_paths_test.rb index c5238dd746..a7a58a78cc 100644 --- a/actionview/test/actionpack/controller/view_paths_test.rb +++ b/actionview/test/actionpack/controller/view_paths_test.rb @@ -144,7 +144,7 @@ class ViewLoadPathsTest < ActionController::TestCase template.identifier, template.handler, virtual_path: template.virtual_path, - format: template.formats + format: template.format ) end end diff --git a/actionview/test/template/digestor_test.rb b/actionview/test/template/digestor_test.rb index 91861edf11..7affd3c005 100644 --- a/actionview/test/template/digestor_test.rb +++ b/actionview/test/template/digestor_test.rb @@ -361,7 +361,7 @@ class TemplateDigestorTest < ActionView::TestCase def tree_template_formats(template_name) tree = ActionView::Digestor.tree(template_name, finder) - tree.flatten.map(&:template).compact.flat_map(&:formats) + tree.flatten.map(&:template).compact.map(&:format) end def disable_resolver_caching diff --git a/actionview/test/template/html_test.rb b/actionview/test/template/html_test.rb index 5cdff74d60..c5fc8f906c 100644 --- a/actionview/test/template/html_test.rb +++ b/actionview/test/template/html_test.rb @@ -4,16 +4,16 @@ require "abstract_unit" class HTMLTest < ActiveSupport::TestCase test "formats returns symbol for recognized MIME type" do - assert_equal [:html], ActionView::Template::HTML.new("", :html).formats + assert_equal :html, ActionView::Template::HTML.new("", :html).format end test "formats returns string for recognized MIME type when MIME does not have symbol" do foo = Mime::Type.lookup("foo") assert_nil foo.to_sym - assert_equal ["foo"], ActionView::Template::HTML.new("", foo).formats + assert_equal "foo", ActionView::Template::HTML.new("", foo).format end test "formats returns string for unknown MIME type" do - assert_equal ["foo"], ActionView::Template::HTML.new("", "foo").formats + assert_equal "foo", ActionView::Template::HTML.new("", "foo").format end end diff --git a/actionview/test/template/lookup_context_test.rb b/actionview/test/template/lookup_context_test.rb index 5298afb694..a763e24226 100644 --- a/actionview/test/template/lookup_context_test.rb +++ b/actionview/test/template/lookup_context_test.rb @@ -125,7 +125,7 @@ class LookupContextTest < ActiveSupport::TestCase assert_called(ActionView::Template::Handlers::Builder, :default_format, returns: nil) do @lookup_context.formats = [:text] template = @lookup_context.find("hello", %w(test)) - assert_equal [:text], template.formats + assert_equal :text, template.format end end diff --git a/actionview/test/template/resolver_patterns_test.rb b/actionview/test/template/resolver_patterns_test.rb index 1e1a4c5063..38357aa10b 100644 --- a/actionview/test/template/resolver_patterns_test.rb +++ b/actionview/test/template/resolver_patterns_test.rb @@ -19,7 +19,7 @@ class ResolverPatternsTest < ActiveSupport::TestCase assert_equal 1, templates.size, "expected one template" assert_equal "Hello custom patterns!", templates.first.source assert_equal "custom_pattern/path", templates.first.virtual_path - assert_equal [:html], templates.first.formats + assert_equal :html, templates.first.format end def test_should_return_all_templates_when_ambiguous_pattern diff --git a/actionview/test/template/testing/fixture_resolver_test.rb b/actionview/test/template/testing/fixture_resolver_test.rb index 9954e3500d..4361824a71 100644 --- a/actionview/test/template/testing/fixture_resolver_test.rb +++ b/actionview/test/template/testing/fixture_resolver_test.rb @@ -15,6 +15,6 @@ class FixtureResolverTest < ActiveSupport::TestCase assert_equal 1, templates.size, "expected one template" assert_equal "this text", templates.first.source assert_equal "arbitrary/path", templates.first.virtual_path - assert_equal [:html], templates.first.formats + assert_equal :html, templates.first.format end end diff --git a/actionview/test/template/testing/null_resolver_test.rb b/actionview/test/template/testing/null_resolver_test.rb index 53364c1d90..dad8d0966d 100644 --- a/actionview/test/template/testing/null_resolver_test.rb +++ b/actionview/test/template/testing/null_resolver_test.rb @@ -9,6 +9,6 @@ class NullResolverTest < ActiveSupport::TestCase assert_equal 1, templates.size, "expected one template" assert_equal "Template generated by Null Resolver", templates.first.source assert_equal "arbitrary/path.erb", templates.first.virtual_path.to_s - assert_equal [:html], templates.first.formats + assert_equal :html, templates.first.format end end diff --git a/actionview/test/template/text_test.rb b/actionview/test/template/text_test.rb index 0c6470df21..e7e3b6aae8 100644 --- a/actionview/test/template/text_test.rb +++ b/actionview/test/template/text_test.rb @@ -4,7 +4,7 @@ require "abstract_unit" class TextTest < ActiveSupport::TestCase test "formats always return :text" do - assert_equal [:text], ActionView::Template::Text.new("").formats + assert_equal :text, ActionView::Template::Text.new("").format end test "identifier should return 'text template'" do -- cgit v1.2.3