From 0b42337f9859ad5eed0141b0a86bdec7b5461a82 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 5 Nov 2025 10:10:29 -0800 Subject: [PATCH 1/3] Use set --- .../lib/smithy-client/features.rb | 8 ++-- .../spec/smithy-client/features_spec.rb | 37 +++++++++++++------ projections/shapes/lib/shapes/client.rb | 3 +- projections/weather/lib/weather/client.rb | 3 +- 4 files changed, 32 insertions(+), 19 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index f60321fd6..97119105e 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -6,15 +6,15 @@ module Client module Features class << self def track(*features, &block) - Thread.current[:smithy_ruby_features] ||= [] - Thread.current[:smithy_ruby_features].concat(features) + Thread.current[:smithy_ruby_features] ||= Set.new + added = features.map { |f| Thread.current[:smithy_ruby_features].add?(f) } block.call ensure - Thread.current[:smithy_ruby_features].pop(features.size) + features.each_with_index { |f, i| Thread.current[:smithy_ruby_features].delete(f) if added[i] } end def tracked - Thread.current[:smithy_ruby_features] || [] + Thread.current[:smithy_ruby_features] || Set.new end end end diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb index 6f89bbaed..708c7f06b 100644 --- a/gems/smithy-client/spec/smithy-client/features_spec.rb +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -6,27 +6,27 @@ module Smithy module Client describe Features do it 'tracks and removes a feature' do - Features.track('A') { expect(Features.tracked).to eq(%w[A]) } + Features.track('A') { expect(Features.tracked).to eq(%w[A].to_set) } expect(Features.tracked).to be_empty end it 'tracks and removes multiple features' do - features = %w[A B C] + features = %w[A B C].to_set Features.track(*features) { expect(Features.tracked).to eq(features) } expect(Features.tracked).to be_empty end it 'tracks and removes features in stack order' do Features.track('A') do - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A].to_set) Features.track('B') do - expect(Features.tracked).to eq(%w[A B]) + expect(Features.tracked).to eq(%w[A B].to_set) Features.track('C') do - expect(Features.tracked).to eq(%w[A B C]) + expect(Features.tracked).to eq(%w[A B C].to_set) end - expect(Features.tracked).to eq(%w[A B]) + expect(Features.tracked).to eq(%w[A B].to_set) end - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A].to_set) end expect(Features.tracked).to be_empty end @@ -34,7 +34,7 @@ module Client it 'ensures that features are removed' do begin Features.track('A') do - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A].to_set) raise StandardError end rescue StandardError @@ -45,15 +45,30 @@ module Client it 'tracks features in multiple threads' do Features.track('A') do - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A].to_set) Thread.new do expect(Features.tracked).to be_empty Features.track('B') do - expect(Features.tracked).to eq(%w[B]) + expect(Features.tracked).to eq(%w[B].to_set) end expect(Features.tracked).to be_empty end.join - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A].to_set) + end + expect(Features.tracked).to be_empty + end + + it 'does not track duplicate features' do + Features.track('A') do + expect(Features.tracked).to eq(%w[A].to_set) + Features.track('B') do + expect(Features.tracked).to eq(%w[A B].to_set) + Features.track('A') do + expect(Features.tracked).to eq(%w[A B].to_set) + end + expect(Features.tracked).to eq(%w[A B].to_set) + end + expect(Features.tracked).to eq(%w[A].to_set) end expect(Features.tracked).to be_empty end diff --git a/projections/shapes/lib/shapes/client.rb b/projections/shapes/lib/shapes/client.rb index 455cceec2..68c72a96e 100644 --- a/projections/shapes/lib/shapes/client.rb +++ b/projections/shapes/lib/shapes/client.rb @@ -161,8 +161,7 @@ class Client < Smithy::Client::Base # @option options [Boolean] :stub_responses # When `true`, the client will return stubbed responses instead of networking requests. # By default fake responses are generated and returned. You can specify the response data - # to return or errors to raise by calling {Stubs#stub_responses}. - # @see Stubs + # to return or errors to raise by calling {Smithy::Client::Stubs#stub_responses}. # @option options [String] :user_agent_suffix # An optional string that is appended to the User-Agent header. # The default User-Agent includes the smithy-client version, diff --git a/projections/weather/lib/weather/client.rb b/projections/weather/lib/weather/client.rb index 059e2f5cc..1136a3d93 100644 --- a/projections/weather/lib/weather/client.rb +++ b/projections/weather/lib/weather/client.rb @@ -161,8 +161,7 @@ class Client < Smithy::Client::Base # @option options [Boolean] :stub_responses # When `true`, the client will return stubbed responses instead of networking requests. # By default fake responses are generated and returned. You can specify the response data - # to return or errors to raise by calling {Stubs#stub_responses}. - # @see Stubs + # to return or errors to raise by calling {Smithy::Client::Stubs#stub_responses}. # @option options [String] :user_agent_suffix # An optional string that is appended to the User-Agent header. # The default User-Agent includes the smithy-client version, From 08ad0112bb7be8885e97ed9c6f03bfd79dccb5e4 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 5 Nov 2025 13:43:26 -0800 Subject: [PATCH 2/3] Convert tracked to array --- .../lib/smithy-client/features.rb | 1 + .../spec/smithy-client/features_spec.rb | 32 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/features.rb b/gems/smithy-client/lib/smithy-client/features.rb index 97119105e..262e312f8 100644 --- a/gems/smithy-client/lib/smithy-client/features.rb +++ b/gems/smithy-client/lib/smithy-client/features.rb @@ -15,6 +15,7 @@ def track(*features, &block) def tracked Thread.current[:smithy_ruby_features] || Set.new + Thread.current[:smithy_ruby_features].to_a end end end diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb index 708c7f06b..b0b5e4b11 100644 --- a/gems/smithy-client/spec/smithy-client/features_spec.rb +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -6,27 +6,27 @@ module Smithy module Client describe Features do it 'tracks and removes a feature' do - Features.track('A') { expect(Features.tracked).to eq(%w[A].to_set) } + Features.track('A') { expect(Features.tracked).to eq(%w[A]) } expect(Features.tracked).to be_empty end it 'tracks and removes multiple features' do - features = %w[A B C].to_set + features = %w[A B C] Features.track(*features) { expect(Features.tracked).to eq(features) } expect(Features.tracked).to be_empty end it 'tracks and removes features in stack order' do Features.track('A') do - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) Features.track('B') do - expect(Features.tracked).to eq(%w[A B].to_set) + expect(Features.tracked).to eq(%w[A B]) Features.track('C') do - expect(Features.tracked).to eq(%w[A B C].to_set) + expect(Features.tracked).to eq(%w[A B C]) end - expect(Features.tracked).to eq(%w[A B].to_set) + expect(Features.tracked).to eq(%w[A B]) end - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) end expect(Features.tracked).to be_empty end @@ -34,7 +34,7 @@ module Client it 'ensures that features are removed' do begin Features.track('A') do - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) raise StandardError end rescue StandardError @@ -45,30 +45,30 @@ module Client it 'tracks features in multiple threads' do Features.track('A') do - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) Thread.new do expect(Features.tracked).to be_empty Features.track('B') do - expect(Features.tracked).to eq(%w[B].to_set) + expect(Features.tracked).to eq(%w[B]) end expect(Features.tracked).to be_empty end.join - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) end expect(Features.tracked).to be_empty end it 'does not track duplicate features' do Features.track('A') do - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) Features.track('B') do - expect(Features.tracked).to eq(%w[A B].to_set) + expect(Features.tracked).to eq(%w[A B]) Features.track('A') do - expect(Features.tracked).to eq(%w[A B].to_set) + expect(Features.tracked).to eq(%w[A B]) end - expect(Features.tracked).to eq(%w[A B].to_set) + expect(Features.tracked).to eq(%w[A B]) end - expect(Features.tracked).to eq(%w[A].to_set) + expect(Features.tracked).to eq(%w[A]) end expect(Features.tracked).to be_empty end From 9536ef9d888297a7503690e5afd3edb7522baba4 Mon Sep 17 00:00:00 2001 From: Richard Wang Date: Wed, 5 Nov 2025 14:42:41 -0800 Subject: [PATCH 3/3] Fix test --- .../spec/smithy-client/features_spec.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/gems/smithy-client/spec/smithy-client/features_spec.rb b/gems/smithy-client/spec/smithy-client/features_spec.rb index b0b5e4b11..df0f21839 100644 --- a/gems/smithy-client/spec/smithy-client/features_spec.rb +++ b/gems/smithy-client/spec/smithy-client/features_spec.rb @@ -59,16 +59,12 @@ module Client end it 'does not track duplicate features' do - Features.track('A') do - expect(Features.tracked).to eq(%w[A]) - Features.track('B') do - expect(Features.tracked).to eq(%w[A B]) - Features.track('A') do - expect(Features.tracked).to eq(%w[A B]) - end + Features.track('A', 'B') do + expect(Features.tracked).to eq(%w[A B]) + Features.track('A') do expect(Features.tracked).to eq(%w[A B]) end - expect(Features.tracked).to eq(%w[A]) + expect(Features.tracked).to eq(%w[A B]) end expect(Features.tracked).to be_empty end