From bfd0f3628bb9621a491fe8a9b1e192bec0535da3 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Tue, 7 Jul 2026 14:15:57 -0700 Subject: [PATCH 1/2] Restore instance-level plugins and fix string-name resolution --- gems/smithy-client/lib/smithy-client/base.rb | 10 ++++++---- gems/smithy-client/lib/smithy-client/plugin_list.rb | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gems/smithy-client/lib/smithy-client/base.rb b/gems/smithy-client/lib/smithy-client/base.rb index 460011c48..173b6e791 100644 --- a/gems/smithy-client/lib/smithy-client/base.rb +++ b/gems/smithy-client/lib/smithy-client/base.rb @@ -47,6 +47,7 @@ def inspect def build_config(plugins, options) config = Configuration.new config.add_option(:service) + config.add_option(:plugins) plugins.each do |plugin| plugin.add_options(config) if plugin.respond_to?(:add_options) end @@ -87,9 +88,8 @@ def waiter(waiter_name, options = {}) class << self def new(options = {}) - plugins = build_plugins options = options.dup - options[:plugins]&.freeze + plugins = build_plugins(options[:plugins]) before_initialize(plugins, options) client = allocate client.send(:initialize, plugins, options) @@ -180,8 +180,10 @@ def define(options = {}) private - def build_plugins - plugins.map { |plugin| plugin.is_a?(Class) ? plugin.new : plugin } + def build_plugins(instance_plugins = nil) + list = PluginList.new(@plugins) + Array(instance_plugins).each { |plugin| list.add(plugin) } + list.map { |plugin| plugin.is_a?(Class) ? plugin.new : plugin }.freeze end def before_initialize(plugins, options) diff --git a/gems/smithy-client/lib/smithy-client/plugin_list.rb b/gems/smithy-client/lib/smithy-client/plugin_list.rb index 88c0ee0ad..1c9bc7993 100644 --- a/gems/smithy-client/lib/smithy-client/plugin_list.rb +++ b/gems/smithy-client/lib/smithy-client/plugin_list.rb @@ -129,7 +129,7 @@ def hash # @return [Class] def require_plugin require(@gem_name) if @gem_name - plugin_class = Plugin + plugin_class = Kernel @canonical_name.split('::').each do |const_name| plugin_class = plugin_class.const_get(const_name) end From 74e632490b298b2a6f691a1cada57f29ab6232f5 Mon Sep 17 00:00:00 2001 From: Juli Tera Date: Tue, 7 Jul 2026 14:16:29 -0700 Subject: [PATCH 2/2] Cover instance-level plugins and string-name resolution --- .../spec/smithy-client/base_spec.rb | 36 +++++++++++++++++++ .../spec/smithy-client/plugin_list_spec.rb | 24 +++++++++++++ 2 files changed, 60 insertions(+) diff --git a/gems/smithy-client/spec/smithy-client/base_spec.rb b/gems/smithy-client/spec/smithy-client/base_spec.rb index b76092bdc..02d00deb2 100644 --- a/gems/smithy-client/spec/smithy-client/base_spec.rb +++ b/gems/smithy-client/spec/smithy-client/base_spec.rb @@ -144,6 +144,42 @@ module Client client_class.new end end + + context 'instance level plugins' do + it 'applies plugins passed via :plugins' do + client = client_class.new(plugins: [DummySendPlugin]) + expect(client.handlers).to include(DummySendPlugin::Handler) + end + + it 'does not mutate the class plugin list' do + client_class.new(plugins: [DummySendPlugin]) + expect(client_class.plugins).not_to include(DummySendPlugin) + end + + it 'does not affect other clients built from the same class' do + with_plugin = client_class.new(plugins: [DummySendPlugin]) + without = client_class.new + expect(with_plugin.handlers).to include(DummySendPlugin::Handler) + expect(without.handlers).not_to include(DummySendPlugin::Handler) + end + + it 'de-duplicates a plugin already on the class (same class form)' do + client_class.add_plugin(DummySendPlugin) + client = client_class.new(plugins: [DummySendPlugin]) + handler_count = client.handlers.to_a.count(DummySendPlugin::Handler) + expect(handler_count).to eq(1) + end + + it 'resolves the plugin set once and freezes it' do + expect(client_class).to receive(:build_plugins).once.and_call_original + client_class.new(plugins: [DummySendPlugin]) + end + + it 'exposes the passed plugins on the config' do + client = client_class.new(plugins: [DummySendPlugin]) + expect(client.config.plugins).to eq([DummySendPlugin]) + end + end end describe '.add_plugin' do diff --git a/gems/smithy-client/spec/smithy-client/plugin_list_spec.rb b/gems/smithy-client/spec/smithy-client/plugin_list_spec.rb index d670efd70..a5d022334 100644 --- a/gems/smithy-client/spec/smithy-client/plugin_list_spec.rb +++ b/gems/smithy-client/spec/smithy-client/plugin_list_spec.rb @@ -76,12 +76,36 @@ def self.const_missing(name) expect(subject.to_a).to eq([plugin]) end + # TODO: address with team offline + # A class and an instance of it are NOT de-duplicated: a class + # canonicalizes by name, an instance by object_id. This matches V3 + # behavior. + it 'does not de-duplicate a class against an instance of it' do + subject.add(Plugin1) + subject.add(Plugin1.new) + expect(subject.to_a.size).to eq(2) + end + + it 'does not de-duplicate distinct instances of the same class' do + subject.add(Plugin1.new) + subject.add(Plugin1.new) + expect(subject.to_a.size).to eq(2) + end + it 'does not require plugins when added' do subject.add('Smithy::Client::LazyPlugin::Add') expect(LazyPlugin.const_defined?(:Add)).to eq(false) expect(subject.to_a).to eq([LazyPlugin::Add]) expect(LazyPlugin.const_defined?(:Add)).to eq(true) end + + it 'resolves a plugin name from the global namespace, not under Plugin' do + top_level = Class.new + stub_const('CollidingPlugin', top_level) + stub_const('Smithy::Client::Plugin::CollidingPlugin', Class.new) + subject.add('CollidingPlugin') + expect(subject.to_a).to eq([top_level]) + end end describe '#remove' do