From d8403af515bfe929d616044f87e0e48a4fae6c2b Mon Sep 17 00:00:00 2001 From: Mike Angell <53470248+shopmike@users.noreply.github.com> Date: Wed, 18 Sep 2019 13:25:55 +1000 Subject: [PATCH] Reimplementation of Static Registers (#1157) --- lib/liquid.rb | 1 + lib/liquid/context.rb | 9 +- lib/liquid/static_registers.rb | 34 ++++ test/unit/context_unit_test.rb | 16 +- test/unit/static_registers_unit_test.rb | 209 ++++++++++++++++++++++++ 5 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 lib/liquid/static_registers.rb create mode 100644 test/unit/static_registers_unit_test.rb diff --git a/lib/liquid.rb b/lib/liquid.rb index 668956635..d102530cc 100644 --- a/lib/liquid.rb +++ b/lib/liquid.rb @@ -78,6 +78,7 @@ module Liquid require 'liquid/parse_context' require 'liquid/partial_cache' require 'liquid/usage' +require 'liquid/static_registers' # Load all the tags of the standard library # diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 7e4350aaf..88e467134 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -18,18 +18,17 @@ class Context attr_accessor :exception_renderer, :template_name, :partial, :global_filter, :strict_variables, :strict_filters # rubocop:disable Metrics/ParameterLists - def self.build(environments: {}, outer_scope: {}, registers: {}, rethrow_errors: false, resource_limits: nil, static_registers: {}, static_environments: {}) - new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_registers, static_environments) + def self.build(environments: {}, outer_scope: {}, registers: {}, rethrow_errors: false, resource_limits: nil, static_environments: {}) + new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_environments) end - def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, static_registers = {}, static_environments = {}) + def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, static_environments = {}) @environments = [environments] @environments.flatten! @static_environments = [static_environments].flat_map(&:freeze).freeze @scopes = [(outer_scope || {})] @registers = registers - @static_registers = static_registers.freeze @errors = [] @partial = false @strict_variables = false @@ -137,7 +136,7 @@ def new_isolated_subcontext Context.build( resource_limits: resource_limits, static_environments: static_environments, - static_registers: static_registers + registers: StaticRegisters.new(registers) ).tap do |subcontext| subcontext.base_scope_depth = base_scope_depth + 1 subcontext.exception_renderer = exception_renderer diff --git a/lib/liquid/static_registers.rb b/lib/liquid/static_registers.rb new file mode 100644 index 000000000..b05056d70 --- /dev/null +++ b/lib/liquid/static_registers.rb @@ -0,0 +1,34 @@ +module Liquid + class StaticRegisters + attr_reader :static_registers, :registers + + def initialize(registers = {}) + @static_registers = registers.is_a?(StaticRegisters) ? registers.static_registers : registers.freeze + @registers = {} + end + + def []=(key, value) + @registers[key] = value + end + + def [](key) + if @registers.key?(key) + @registers[key] + else + @static_registers[key] + end + end + + def delete(key) + @registers.delete(key) + end + + def fetch(key, default = nil) + key?(key) ? self[key] : default + end + + def key?(key) + @registers.key?(key) || @static_registers.key?(key) + end + end +end diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index 67a8c9165..fe790cfb0 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -518,15 +518,23 @@ def test_new_isolated_subcontext_does_not_inherit_non_static_registers registers = { my_register: :my_value, } - super_context = Context.new({}, {}, registers) + super_context = Context.new({}, {}, StaticRegisters.new(registers)) + super_context.registers[:my_register] = :my_alt_value subcontext = super_context.new_isolated_subcontext - assert_nil subcontext.registers[:my_register] + assert_equal :my_value, subcontext.registers[:my_register] end def test_new_isolated_subcontext_inherits_static_registers - super_context = Context.build(static_registers: { my_register: :my_value }) + super_context = Context.build(registers: { my_register: :my_value }) subcontext = super_context.new_isolated_subcontext - assert_equal :my_value, subcontext.static_registers[:my_register] + assert_equal :my_value, subcontext.registers[:my_register] + end + + def test_new_isolated_subcontext_registers_do_not_pollute_context + super_context = Context.build(registers: { my_register: :my_value }) + subcontext = super_context.new_isolated_subcontext + subcontext.registers[:my_register] = :my_alt_value + assert_equal :my_value, super_context.registers[:my_register] end def test_new_isolated_subcontext_inherits_filters diff --git a/test/unit/static_registers_unit_test.rb b/test/unit/static_registers_unit_test.rb new file mode 100644 index 000000000..389cb3fb6 --- /dev/null +++ b/test/unit/static_registers_unit_test.rb @@ -0,0 +1,209 @@ +require 'test_helper' + +class StaticRegistersUnitTest < Minitest::Test + include Liquid + + def set + static_register = StaticRegisters.new + static_register[nil] = true + static_register[1] = :one + static_register[:one] = "one" + static_register["two"] = "three" + static_register["two"] = 3 + static_register[false] = nil + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.registers) + + static_register + end + + def test_get + static_register = set + + assert_equal true, static_register[nil] + assert_equal :one, static_register[1] + assert_equal "one", static_register[:one] + assert_equal 3, static_register["two"] + assert_nil static_register[false] + assert_nil static_register["unknown"] + end + + def test_delete + static_register = set + + assert_equal true, static_register.delete(nil) + assert_equal :one, static_register.delete(1) + assert_equal "one", static_register.delete(:one) + assert_equal 3, static_register.delete("two") + assert_nil static_register.delete(false) + assert_nil static_register.delete("unknown") + + assert_equal({}, static_register.registers) + end + + def test_fetch + static_register = set + + assert_equal true, static_register.fetch(nil) + assert_equal :one, static_register.fetch(1) + assert_equal "one", static_register.fetch(:one) + assert_equal 3, static_register.fetch("two") + assert_nil static_register.fetch(false) + assert_nil static_register.fetch("unknown") + end + + def test_fetch_default + static_register = StaticRegisters.new + + assert_equal true, static_register.fetch(nil, true) + assert_equal :one, static_register.fetch(1, :one) + assert_equal "one", static_register.fetch(:one, "one") + assert_equal 3, static_register.fetch("two", 3) + assert_nil static_register.fetch(false, nil) + end + + def test_key + static_register = set + + assert_equal true, static_register.key?(nil) + assert_equal true, static_register.key?(1) + assert_equal true, static_register.key?(:one) + assert_equal true, static_register.key?("two") + assert_equal true, static_register.key?(false) + assert_equal false, static_register.key?("unknown") + assert_equal false, static_register.key?(true) + end + + def set_with_static + static_register = StaticRegisters.new(nil => true, 1 => :one, :one => "one", "two" => 3, false => nil) + static_register[nil] = false + static_register["two"] = 4 + static_register[true] = "foo" + + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + assert_equal({ nil => false, "two" => 4, true => "foo" }, static_register.registers) + + static_register + end + + def test_get_with_static + static_register = set_with_static + + assert_equal false, static_register[nil] + assert_equal :one, static_register[1] + assert_equal "one", static_register[:one] + assert_equal 4, static_register["two"] + assert_equal "foo", static_register[true] + assert_nil static_register[false] + end + + def test_delete_with_static + static_register = set_with_static + + assert_equal false, static_register.delete(nil) + assert_equal 4, static_register.delete("two") + assert_equal "foo", static_register.delete(true) + assert_nil static_register.delete("unknown") + assert_nil static_register.delete(:one) + + assert_equal({}, static_register.registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + end + + def test_fetch_with_static + static_register = set_with_static + + assert_equal false, static_register.fetch(nil) + assert_equal :one, static_register.fetch(1) + assert_equal "one", static_register.fetch(:one) + assert_equal 4, static_register.fetch("two") + assert_equal "foo", static_register.fetch(true) + assert_nil static_register.fetch(false) + end + + def test_key_with_static + static_register = set_with_static + + assert_equal true, static_register.key?(nil) + assert_equal true, static_register.key?(1) + assert_equal true, static_register.key?(:one) + assert_equal true, static_register.key?("two") + assert_equal true, static_register.key?(false) + assert_equal false, static_register.key?("unknown") + assert_equal true, static_register.key?(true) + end + + def test_static_register_frozen + static_register = set_with_static + + static = static_register.static_registers + + assert_raises(RuntimeError) do + static["two"] = "foo" + end + + assert_raises(RuntimeError) do + static["unknown"] = "foo" + end + + assert_raises(RuntimeError) do + static.delete("two") + end + end + + def test_new_static_retains_static + static_register = StaticRegisters.new(nil => true, 1 => :one, :one => "one", "two" => 3, false => nil) + static_register["one"] = 1 + static_register["two"] = 2 + static_register["three"] = 3 + + new_register = StaticRegisters.new(static_register) + assert_equal({}, new_register.registers) + + new_register["one"] = 4 + new_register["two"] = 5 + new_register["three"] = 6 + + newest_register = StaticRegisters.new(new_register) + assert_equal({}, newest_register.registers) + + newest_register["one"] = 7 + newest_register["two"] = 8 + newest_register["three"] = 9 + + assert_equal({ "one" => 1, "two" => 2, "three" => 3 }, static_register.registers) + assert_equal({ "one" => 4, "two" => 5, "three" => 6 }, new_register.registers) + assert_equal({ "one" => 7, "two" => 8, "three" => 9 }, newest_register.registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, new_register.static_registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, newest_register.static_registers) + end + + def test_multiple_instances_are_unique + static_register = StaticRegisters.new(nil => true, 1 => :one, :one => "one", "two" => 3, false => nil) + static_register["one"] = 1 + static_register["two"] = 2 + static_register["three"] = 3 + + new_register = StaticRegisters.new(foo: :bar) + assert_equal({}, new_register.registers) + + new_register["one"] = 4 + new_register["two"] = 5 + new_register["three"] = 6 + + newest_register = StaticRegisters.new(bar: :foo) + assert_equal({}, newest_register.registers) + + newest_register["one"] = 7 + newest_register["two"] = 8 + newest_register["three"] = 9 + + assert_equal({ "one" => 1, "two" => 2, "three" => 3 }, static_register.registers) + assert_equal({ "one" => 4, "two" => 5, "three" => 6 }, new_register.registers) + assert_equal({ "one" => 7, "two" => 8, "three" => 9 }, newest_register.registers) + assert_equal({ nil => true, 1 => :one, :one => "one", "two" => 3, false => nil }, static_register.static_registers) + assert_equal({ foo: :bar }, new_register.static_registers) + assert_equal({ bar: :foo }, newest_register.static_registers) + end +end