diff --git a/History.md b/History.md index 01536c76..940f2b5a 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,14 @@ # Liquid Change Log +## 5.3.1 (unreleased) + +### Fixes +* `PartialCache` now shares snippet cache with subcontexts by default (#1553) [Chris AtLee] +* Hash registers no longer leak into subcontexts as static registers (#1564) [Chris AtLee] + +### Changed +* Liquid::Context#registers now always returns a Liquid::StaticRegisters object, though supports the most used Hash functions for compatibility (#1553) + ## 5.3.0 2022-03-22 ### Fixes diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 3d6ffc96..016bd309 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -28,7 +28,7 @@ module Liquid @static_environments = [static_environments].flat_map(&:freeze).freeze @scopes = [(outer_scope || {})] - @registers = registers + @registers = registers.is_a?(StaticRegisters) ? registers : StaticRegisters.new(registers) @errors = [] @partial = false @strict_variables = false @@ -39,6 +39,10 @@ module Liquid @global_filter = nil @disabled_tags = {} + @registers.static[:cached_partials] ||= {} + @registers.static[:file_system] ||= Liquid::Template.file_system + @registers.static[:template_factory] ||= Liquid::TemplateFactory.new + self.exception_renderer = Template.default_exception_renderer if rethrow_errors self.exception_renderer = Liquid::RAISE_EXCEPTION_LAMBDA diff --git a/lib/liquid/partial_cache.rb b/lib/liquid/partial_cache.rb index 856d3074..56e10805 100644 --- a/lib/liquid/partial_cache.rb +++ b/lib/liquid/partial_cache.rb @@ -3,16 +3,16 @@ module Liquid class PartialCache def self.load(template_name, context:, parse_context:) - cached_partials = (context.registers[:cached_partials] ||= {}) + cached_partials = context.registers[:cached_partials] cached = cached_partials[template_name] return cached if cached - file_system = (context.registers[:file_system] ||= Liquid::Template.file_system) + file_system = context.registers[:file_system] source = file_system.read_template_file(template_name) parse_context.partial = true - template_factory = (context.registers[:template_factory] ||= Liquid::TemplateFactory.new) + template_factory = context.registers[:template_factory] template = template_factory.for(template_name) partial = template.parse(source, parse_context) diff --git a/lib/liquid/template.rb b/lib/liquid/template.rb index cb8919ab..39bde833 100644 --- a/lib/liquid/template.rb +++ b/lib/liquid/template.rb @@ -167,15 +167,14 @@ module Liquid output = nil - context_register = context.registers.is_a?(StaticRegisters) ? context.registers.static : context.registers - case args.last when Hash options = args.pop output = options[:output] if options[:output] + static_registers = context.registers.static options[:registers]&.each do |key, register| - context_register[key] = register + static_registers[key] = register end apply_options_to_context(context, options) diff --git a/test/integration/context_test.rb b/test/integration/context_test.rb index 58f1c331..7598c715 100644 --- a/test/integration/context_test.rb +++ b/test/integration/context_test.rb @@ -618,6 +618,20 @@ class ContextTest < Minitest::Test end end + def test_context_always_uses_static_registers + registers = { + my_register: :my_value, + } + c = Context.new({}, {}, registers) + assert_instance_of(StaticRegisters, c.registers) + assert_equal(:my_value, c.registers[:my_register]) + + r = StaticRegisters.new(registers) + c = Context.new({}, {}, r) + assert_instance_of(StaticRegisters, c.registers) + assert_equal(:my_value, c.registers[:my_register]) + end + private def assert_no_object_allocations diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 0342742a..f413e6f9 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -32,7 +32,7 @@ class TestDrop < Liquid::Drop attr_reader :value def registers - @context.registers + { @value => @context.registers[@value] } end end @@ -440,7 +440,7 @@ class StandardFiltersTest < Minitest::Test end def test_map_calls_context= - model = TestModel.new(value: "test") + model = TestModel.new(value: :test) template = Template.parse('{{ foo | map: "registers" }}') template.registers[:test] = 1234 diff --git a/test/unit/partial_cache_unit_test.rb b/test/unit/partial_cache_unit_test.rb index 27912c04..00052359 100644 --- a/test/unit/partial_cache_unit_test.rb +++ b/test/unit/partial_cache_unit_test.rb @@ -125,4 +125,35 @@ class PartialCacheUnitTest < Minitest::Test assert_equal('my partial body', partial.render) assert_equal(1, template_factory.count) end + + def test_cache_state_is_shared_for_subcontexts + parse_context = Liquid::ParseContext.new + shared_file_system = StubFileSystem.new( + 'my_partial' => 'my shared value' + ) + context = Liquid::Context.build( + registers: Liquid::StaticRegisters.new( + file_system: shared_file_system, + ) + ) + subcontext = context.new_isolated_subcontext + + assert_equal(subcontext.registers[:cached_partials].object_id, context.registers[:cached_partials].object_id) + + 2.times do + Liquid::PartialCache.load( + 'my_partial', + context: context, + parse_context: parse_context + ) + + Liquid::PartialCache.load( + 'my_partial', + context: subcontext, + parse_context: parse_context + ) + end + + assert_equal(1, shared_file_system.file_read_count) + end end