From bf7d06190b7184e2b1fdaa953c57a4c5d23a784f Mon Sep 17 00:00:00 2001 From: Jim Gay Date: Fri, 20 Dec 2024 17:59:42 -0500 Subject: [PATCH 1/5] Cleanup DelegatedOpenStruct and ensure dup function Ensure that dup will copy the reference to the object for forwarding messages. --- lib/radius/delegating_open_struct.rb | 34 ++++++++++++---------------- test/context_test.rb | 8 +++++++ 2 files changed, 23 insertions(+), 19 deletions(-) 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/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 From e8b6dc35a6471c2d3bbb65f1b4b5e40ac8c16392 Mon Sep 17 00:00:00 2001 From: Jim Gay Date: Fri, 20 Dec 2024 18:00:02 -0500 Subject: [PATCH 2/5] Use dup instead of initializing via the class --- lib/radius/context.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From 90a5da931f4c8c157a34e51a9df27d0b7ec58f01 Mon Sep 17 00:00:00 2001 From: Jim Gay Date: Fri, 20 Dec 2024 18:01:40 -0500 Subject: [PATCH 3/5] Update changelog for dup fix --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) 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 From cb8dc55e632a612fabfb920baf859194d4810155 Mon Sep 17 00:00:00 2001 From: Jim Gay Date: Fri, 20 Dec 2024 18:17:41 -0500 Subject: [PATCH 4/5] Fixup aggregation of multithreaded test results --- test/multithreaded_test.rb | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) 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 From 13b4d6b241d360b2641f310d9ff0ebb4b0b0ed27 Mon Sep 17 00:00:00 2001 From: Jim Gay Date: Fri, 20 Dec 2024 18:17:56 -0500 Subject: [PATCH 5/5] Cleanup utility methods --- lib/radius/utility.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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