kemko/liquid
1
0
Fork 0
mirror of https://github.com/kemko/liquid.git synced 2026-01-01 15:55:40 +03:00
Code Issues Actions Packages Projects Releases Wiki Activity

Fix performance regression from introduction of Template#disable_tags (#1274)

Browse Source
This commit is contained in:
Dylan Thacker-Smith
2020-08-18 11:25:51 -04:00
committed by GitHub
parent a1d982ca76
commit 2b11efc3ae
16 changed files with 164 additions and 199 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lib/liquid.rb
View File

@@ -63,6 +63,8 @@ require 'liquid/expression'
require 'liquid/context'
require 'liquid/parser_switching'
require 'liquid/tag'
require 'liquid/tag/disabler'
require 'liquid/tag/disableable'
require 'liquid/block'
require 'liquid/block_body'
require 'liquid/document'
@@ -86,4 +88,3 @@ require 'liquid/template_factory'
# Load all the tags of the standard library
#
Dir["#{__dir__}/liquid/tags/*.rb"].each { |f| require f }
Dir["#{__dir__}/liquid/registers/*.rb"].each { |f| require f }

13
lib/liquid/block_body.rb
View File

@@ -171,13 +171,7 @@ module Liquid
private
def render_node(context, output, node)
if node.disabled?(context)
output << node.disabled_error_message
return
end
disable_tags(context, node.disabled_tags) do
node.render_to_output_buffer(context, output)
end
node.render_to_output_buffer(context, output)
rescue UndefinedVariable, UndefinedDropMethod, UndefinedFilter => e
context.handle_error(e, node.line_number)
rescue ::StandardError => e
@@ -185,11 +179,6 @@ module Liquid
output << context.handle_error(e, line_number)
end
def disable_tags(context, tags, &block)
return yield if tags.empty?
context.registers[:disabled_tags].disable(tags, &block)
end
def raise_if_resource_limits_reached(context, length)
context.resource_limits.render_length += length
return unless context.resource_limits.reached?

19
lib/liquid/context.rb
View File

@@ -44,6 +44,7 @@ module Liquid
@interrupts = []
@filters = []
@global_filter = nil
@disabled_tags = {}
end
# rubocop:enable Metrics/ParameterLists
@@ -144,6 +145,7 @@ module Liquid
subcontext.strainer = nil
subcontext.errors = errors
subcontext.warnings = warnings
subcontext.disabled_tags = @disabled_tags
end
end
@@ -208,9 +210,24 @@ module Liquid
end
end
def with_disabled_tags(tag_names)
tag_names.each do |name|
@disabled_tags[name] = @disabled_tags.fetch(name, 0) + 1
end
yield
ensure
tag_names.each do |name|
@disabled_tags[name] -= 1
end
end
def tag_disabled?(tag_name)
@disabled_tags.fetch(tag_name, 0) > 0
end
protected
attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters
attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags
private

20
lib/liquid/profiler/hooks.rb
View File

@@ -1,25 +1,21 @@
# frozen_string_literal: true
module Liquid
class BlockBody
def render_node_with_profiling(context, output, node)
module BlockBodyProfilingHook
def render_node(context, output, node)
Profiler.profile_node_render(node) do
render_node_without_profiling(context, output, node)
super
end
end
alias_method :render_node_without_profiling, :render_node
alias_method :render_node, :render_node_with_profiling
end
BlockBody.prepend(BlockBodyProfilingHook)
class Include < Tag
def render_to_output_buffer_with_profiling(context, output)
module IncludeProfilingHook
def render_to_output_buffer(context, output)
Profiler.profile_children(context.evaluate(@template_name_expr).to_s) do
render_to_output_buffer_without_profiling(context, output)
super
end
end
alias_method :render_to_output_buffer_without_profiling, :render_to_output_buffer
alias_method :render_to_output_buffer, :render_to_output_buffer_with_profiling
end
Include.prepend(IncludeProfilingHook)
end

32
lib/liquid/registers/disabled_tags.rb
View File

@@ -1,32 +0,0 @@
# frozen_string_literal: true
module Liquid
class DisabledTags < Register
def initialize
@disabled_tags = {}
end
def disabled?(tag)
@disabled_tags.key?(tag) && @disabled_tags[tag] > 0
end
def disable(tags)
tags.each(&method(:increment))
yield
ensure
tags.each(&method(:decrement))
end
private
def increment(tag)
@disabled_tags[tag] ||= 0
@disabled_tags[tag] += 1
end
def decrement(tag)
@disabled_tags[tag] -= 1
end
end
Template.add_register(:disabled_tags, DisabledTags.new)
end

22
lib/liquid/tag.rb
View File

@@ -13,15 +13,13 @@ module Liquid
tag
end
def disable_tags(*tags)
disabled_tags.push(*tags)
def disable_tags(*tag_names)
@disabled_tags ||= []
@disabled_tags.concat(tag_names)
prepend(Disabler)
end
private :new
def disabled_tags
@disabled_tags ||= []
end
end
def initialize(tag_name, markup, parse_context)
@@ -46,14 +44,6 @@ module Liquid
''
end
def disabled?(context)
context.registers[:disabled_tags].disabled?(tag_name)
end
def disabled_error_message
"#{tag_name} #{options[:locale].t('errors.disabled.tag')}"
end
# For backwards compatibility with custom tags. In a future release, the semantics
# of the `render_to_output_buffer` method will become the default and the `render`
# method will be removed.
@@ -65,9 +55,5 @@ module Liquid
def blank?
false
end
def disabled_tags
self.class.disabled_tags
end
end
end

19
lib/liquid/tag/disableable.rb Normal file
View File

@@ -0,0 +1,19 @@
# frozen_string_literal: true
module Liquid
class Tag
module Disableable
def render_to_output_buffer(context, output)
if context.tag_disabled?(tag_name)
output << disabled_error_message
return
end
super
end
def disabled_error_message
"#{tag_name} #{parse_context[:locale].t('errors.disabled.tag')}"
end
end
end
end

21
lib/liquid/tag/disabler.rb Normal file
View File

@@ -0,0 +1,21 @@
# frozen_string_literal: true
module Liquid
class Tag
module Disabler
module ClassMethods
attr_reader :disabled_tags
end
def self.prepended(base)
base.extend(ClassMethods)
end
def render_to_output_buffer(context, output)
context.with_disabled_tags(self.class.disabled_tags) do
super
end
end
end
end
end

2
lib/liquid/tags/include.rb
View File

@@ -16,6 +16,8 @@ module Liquid
# {% include 'product' for products %}
#
class Include < Tag
prepend Tag::Disableable
SYNTAX = /(#{QuotedFragment}+)(\s+(?:with|for)\s+(#{QuotedFragment}+))?(\s+(?:as)\s+(#{VariableSegment}+))?/o
Syntax = SYNTAX

12
lib/liquid/template.rb
View File

@@ -80,14 +80,6 @@ module Liquid
tags[name.to_s] = klass
end
attr_accessor :registers
Template.registers = {}
private :registers=
def add_register(name, klass)
registers[name.to_sym] = klass
end
# Pass a module with filter methods which should be available
# to all liquid views. Good for registering the standard library
def register_filter(mod)
@@ -194,10 +186,6 @@ module Liquid
context.add_filters(args.pop)
end
Template.registers.each do |key, register|
context_register[key] = register unless context_register.key?(key)
end
# Retrying a render resets resource usage
context.resource_limits.reset

27
test/integration/registers/disabled_tags_test.rb
View File

@@ -1,27 +0,0 @@
# frozen_string_literal: true
require 'test_helper'
class DisabledTagsTest < Minitest::Test
include Liquid
class DisableRaw < Block
disable_tags "raw"
end
class DisableRawEcho < Block
disable_tags "raw", "echo"
end
def test_disables_raw
with_custom_tag('disable', DisableRaw) do
assert_template_result 'raw usage is not allowed in this contextfoo', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}'
end
end
def test_disables_echo_and_raw
with_custom_tag('disable', DisableRawEcho) do
assert_template_result 'raw usage is not allowed in this contextecho usage is not allowed in this context', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}'
end
end
end

49
test/integration/tag/disableable_test.rb Normal file
View File

@@ -0,0 +1,49 @@
# frozen_string_literal: true
require 'test_helper'
class TagDisableableTest < Minitest::Test
include Liquid
class DisableRaw < Block
disable_tags "raw"
end
class DisableRawEcho < Block
disable_tags "raw", "echo"
end
class DisableableRaw < Liquid::Raw
prepend Liquid::Tag::Disableable
end
class DisableableEcho < Liquid::Echo
prepend Liquid::Tag::Disableable
end
def test_disables_raw
with_disableable_tags do
with_custom_tag('disable', DisableRaw) do
assert_template_result 'raw usage is not allowed in this contextfoo', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}'
end
end
end
def test_disables_echo_and_raw
with_disableable_tags do
with_custom_tag('disable', DisableRawEcho) do
assert_template_result 'raw usage is not allowed in this contextecho usage is not allowed in this context', '{% disable %}{% raw %}Foobar{% endraw %}{% echo "foo" %}{% enddisable %}'
end
end
end
private
def with_disableable_tags
with_custom_tag('raw', DisableableRaw) do
with_custom_tag('echo', DisableableEcho) do
yield
end
end
end
end

44
test/integration/template_test.rb
View File

@@ -361,48 +361,4 @@ class TemplateTest < Minitest::Test
result = t.render('x' => 1, 'y' => 5)
assert_equal('12345', result)
end
def test_render_uses_correct_disabled_tags_instance
Liquid::Template.file_system = StubFileSystem.new(
'foo' => 'bar',
'test_include' => '{% include "foo" %}'
)
disabled_tags = DisabledTags.new
context = Context.build(registers: { disabled_tags: disabled_tags })
source = "{% render 'test_include' %}"
parse_context = Liquid::ParseContext.new(line_numbers: true, error_mode: :strict)
document = Document.parse(Liquid::Tokenizer.new(source, true), parse_context)
assert_equal("include usage is not allowed in this context", document.render(context))
end
def test_render_sets_context_static_register_when_register_key_does_exist
disabled_tags_for_test = DisabledTags.new
Template.add_register(:disabled_tags, disabled_tags_for_test)
t = Template.parse("{% if true %} Test Template {% endif %}")
context = Context.new
refute(context.registers.key?(:disabled_tags))
t.render(context)
assert(context.registers.key?(:disabled_tags))
assert_equal(disabled_tags_for_test, context.registers[:disabled_tags])
end
def test_render_does_not_override_context_static_register_when_register_key_exists
context = Context.new
context.registers[:random_register] = nil
Template.add_register(:random_register, {})
t = Template.parse("{% if true %} Test Template {% endif %}")
t.render(context)
assert_nil(context.registers[:random_register])
assert(context.registers.key?(:random_register))
end
end

15
test/test_helper.rb
View File

@@ -98,10 +98,17 @@ module Minitest
end
def with_custom_tag(tag_name, tag_class)
Liquid::Template.register_tag(tag_name, tag_class)
yield
ensure
Liquid::Template.tags.delete(tag_name)
old_tag = Liquid::Template.tags[tag_name]
begin
Liquid::Template.register_tag(tag_name, tag_class)
yield
ensure
if old_tag
Liquid::Template.tags[tag_name] = old_tag
else
Liquid::Template.tags.delete(tag_name)
end
end
end
end
end

29
test/unit/context_unit_test.rb
View File

@@ -563,6 +563,35 @@ class ContextUnitTest < Minitest::Test
assert_equal('my filter result', template.render(subcontext))
end
def test_disables_tag_specified
context = Context.new
context.with_disabled_tags(%w(foo bar)) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
assert_equal false, context.tag_disabled?("unknown")
end
end
def test_disables_nested_tags
context = Context.new
context.with_disabled_tags(["foo"]) do
context.with_disabled_tags(["foo"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal false, context.tag_disabled?("bar")
end
context.with_disabled_tags(["bar"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
context.with_disabled_tags(["foo"]) do
assert_equal true, context.tag_disabled?("foo")
assert_equal true, context.tag_disabled?("bar")
end
end
assert_equal true, context.tag_disabled?("foo")
assert_equal false, context.tag_disabled?("bar")
end
end
private
def assert_no_object_allocations

36
test/unit/registers/disabled_tags_unit_test.rb
View File

@@ -1,36 +0,0 @@
# frozen_string_literal: true
require 'test_helper'
class DisabledTagsUnitTest < Minitest::Test
include Liquid
def test_disables_tag_specified
register = DisabledTags.new
register.disable(%w(foo bar)) do
assert_equal true, register.disabled?("foo")
assert_equal true, register.disabled?("bar")
assert_equal false, register.disabled?("unknown")
end
end
def test_disables_nested_tags
register = DisabledTags.new
register.disable(["foo"]) do
register.disable(["foo"]) do
assert_equal true, register.disabled?("foo")
assert_equal false, register.disabled?("bar")
end
register.disable(["bar"]) do
assert_equal true, register.disabled?("foo")
assert_equal true, register.disabled?("bar")
register.disable(["foo"]) do
assert_equal true, register.disabled?("foo")
assert_equal true, register.disabled?("bar")
end
end
assert_equal true, register.disabled?("foo")
assert_equal false, register.disabled?("bar")
end
end
end