diff --git a/CHANGELOG b/CHANGELOG index 21d6c76..2efad0d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ = Change Log == 0.8.0 +* Ensure that the DelegatingOpenStruct is a true copy of the original [saturnflyer] * Avoid errors with frozen strings in Ruby 3.4+ [jeremyevans] * Drop support for EOL versions of Ruby diff --git a/lib/radius/context.rb b/lib/radius/context.rb index 97096cd..211ebb9 100644 --- a/lib/radius/context.rb +++ b/lib/radius/context.rb @@ -96,7 +96,7 @@ def dup # :nodoc: def stack(name, attributes, block) previous = @tag_binding_stack.last previous_locals = previous.nil? ? globals : previous.locals - locals = DelegatingOpenStruct.new(previous_locals) + locals = previous_locals.dup binding = TagBinding.new(self, locals, name, attributes, block) @tag_binding_stack.push(binding) result = yield(binding) diff --git a/lib/radius/delegating_open_struct.rb b/lib/radius/delegating_open_struct.rb index 1a95bee..17a3196 100644 --- a/lib/radius/delegating_open_struct.rb +++ b/lib/radius/delegating_open_struct.rb @@ -8,30 +8,26 @@ def initialize(object = nil) end def dup - rv = self.class.new - rv.instance_variable_set(:@hash, @hash.dup) - rv + self.class.new.tap do |copy| + copy.instance_variable_set(:@hash, @hash.dup) + copy.object = @object + end end def method_missing(method, *args, &block) - symbol = (method.to_s =~ /^(.*?)=$/) ? $1.intern : method - if (0..1).include?(args.size) - if args.size == 1 - @hash[symbol] = args.first - else - if @hash.has_key?(symbol) - @hash[symbol] - else - unless object.nil? - @object.send(method, *args, &block) - else - nil - end - end - end + return super if args.size > 1 + + symbol = method.to_s.chomp('=').to_sym + + if method.to_s.end_with?('=') + @hash[symbol] = args.first else - super + @hash.fetch(symbol) { @object&.public_send(method, *args, &block) } end end + + def respond_to_missing?(method, include_private = false) + (args.size <= 1) || super + end end end diff --git a/lib/radius/utility.rb b/lib/radius/utility.rb index aab4874..d409575 100644 --- a/lib/radius/utility.rb +++ b/lib/radius/utility.rb @@ -22,15 +22,12 @@ def self.constantize(camelized_string) end def self.camelize(underscored_string) - string = '' - underscored_string.split('_').each { |part| string << part.capitalize } - string + underscored_string.split('_').map(&:capitalize).join end def self.array_to_s(c) - c.map do |x| - res = (x.is_a?(Array) ? array_to_s(x) : x.to_s) - +res + +c.map do |x| + x.is_a?(Array) ? array_to_s(x) : x.to_s end.join end end diff --git a/test/context_test.rb b/test/context_test.rb index 537468b..201fac9 100644 --- a/test/context_test.rb +++ b/test/context_test.rb @@ -35,6 +35,14 @@ def test_initialize_with_arguments assert_equal 'arg', @context.name end + def test_dup_preserves_delegated_values + @context = Radius::Context.new + @context.globals.object = Object.new.tap { |o| def o.special_method; "special"; end } + duped = @context.dup + + assert_equal "special", duped.globals.special_method, "Duped context should preserve delegated object methods" + end + def test_with got = @context.with do |c| assert_equal @context, c diff --git a/test/multithreaded_test.rb b/test/multithreaded_test.rb index 961f1d9..947a691 100644 --- a/test/multithreaded_test.rb +++ b/test/multithreaded_test.rb @@ -26,12 +26,17 @@ def test_runs_multithreaded local_results = [] iterations_per_thread.times do begin - parser = Radius::Parser.new(@context, :tag_prefix => 'r') + thread_context = @context.dup + parser = Radius::Parser.new(thread_context, :tag_prefix => 'r') parser.context.globals.thread_id = Thread.current.object_id expected = "#{Thread.current.object_id} / #{parser.context.globals.object_id}" result = parser.parse('') - local_results << result + local_results << { + result: result, + thread_id: Thread.current.object_id, + iteration: local_results.size + } failures << "Expected: #{expected}, Got: #{result}" unless result == expected rescue => e @@ -48,12 +53,26 @@ def test_runs_multithreaded failure_message = if failures.empty? nil else - "Thread failures detected:\n#{failures.size} times:\n#{failures.pop(5).join("\n")}" + failed_items = [] + 5.times { failed_items << failures.pop unless failures.empty? } + "Thread failures detected:\n#{failures.size} times:\n#{failed_items.join("\n")}" end assert(failures.empty?, failure_message) total_results = results.flatten.uniq.size expected_unique_results = thread_count * iterations_per_thread + + if total_results != expected_unique_results + duplicates = results.flatten.group_by { |r| r[:result] } + .select { |_, v| v.size > 1 } + + puts "\nDuplicates found:" + duplicates.each do |result, occurrences| + puts "\nResult: #{result[:result]}" + occurrences.each { |o| puts " Thread: #{o[:thread_id]}, Iteration: #{o[:iteration]}" } + end + end + assert_equal expected_unique_results, total_results, "Expected #{expected_unique_results} unique results (#{thread_count} threads × #{iterations_per_thread} iterations)" end