From ab1503e49ba7476d34b1e737192c2382b72e5676 Mon Sep 17 00:00:00 2001 From: Matt Muller <53055821+mullermp@users.noreply.github.com> Date: Thu, 20 Jun 2024 16:54:43 -0400 Subject: [PATCH] Instance plugins (#3051) --- .../spec/interfaces/resources_spec.rb | 1 - .../templates/client_class.mustache | 5 + build_tools/services.rb | 4 +- gems/aws-sdk-core/CHANGELOG.md | 2 + .../plugins/global_configuration.rb | 17 ++- .../lib/aws-sdk-core/plugins/retry_errors.rb | 1 - gems/aws-sdk-core/lib/seahorse/client/base.rb | 24 +++- .../lib/seahorse/client/plugins/endpoint.rb | 1 - .../aws/plugins/global_configuration_spec.rb | 54 +++++++- .../spec/seahorse/client/base_spec.rb | 126 +++++++++++++----- gems/aws-sdk-s3/spec/client_spec.rb | 58 ++++---- gems/aws-sdk-sqs/spec/client_spec.rb | 12 +- 12 files changed, 206 insertions(+), 99 deletions(-) diff --git a/build_tools/aws-sdk-code-generator/spec/interfaces/resources_spec.rb b/build_tools/aws-sdk-code-generator/spec/interfaces/resources_spec.rb index b0713b1caf2..815883e2a65 100644 --- a/build_tools/aws-sdk-code-generator/spec/interfaces/resources_spec.rb +++ b/build_tools/aws-sdk-code-generator/spec/interfaces/resources_spec.rb @@ -5,7 +5,6 @@ describe 'Interfaces' do before(:all) do - # TODO : support Aws.config[:sample] = { ... } @tmpdir = SpecHelper.generate_service(['Sample'], multiple_files: true) end diff --git a/build_tools/aws-sdk-code-generator/templates/client_class.mustache b/build_tools/aws-sdk-code-generator/templates/client_class.mustache index 089ec838323..4aea5de7e32 100644 --- a/build_tools/aws-sdk-code-generator/templates/client_class.mustache +++ b/build_tools/aws-sdk-code-generator/templates/client_class.mustache @@ -37,6 +37,11 @@ module {{module_name}} {{#client_constructor}} # @overload initialize(options) # @param [Hash] options + # + # @option options [Array] :plugins ([]]) + # A list of plugins to apply to the client. Each plugin is either a + # class name or an instance of a plugin class. + # {{>documentation}} {{/client_constructor}} def initialize(*args) diff --git a/build_tools/services.rb b/build_tools/services.rb index 2f9f17fa955..b2ddb9ec9db 100644 --- a/build_tools/services.rb +++ b/build_tools/services.rb @@ -10,10 +10,10 @@ class ServiceEnumerator MANIFEST_PATH = File.expand_path('../../services.json', __FILE__) # Minimum `aws-sdk-core` version for new gem builds - MINIMUM_CORE_VERSION = "3.197.0" + MINIMUM_CORE_VERSION = "3.198.0" # Minimum `aws-sdk-core` version for new S3 gem builds - MINIMUM_CORE_VERSION_S3 = "3.197.0" + MINIMUM_CORE_VERSION_S3 = "3.198.0" EVENTSTREAM_PLUGIN = "Aws::Plugins::EventStreamConfiguration" diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index b7f278a1b8e..3591308f23c 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Feature - Support `:plugins` option on all Clients or using `Aws.config[:plugins]`. + 3.197.2 (2024-06-20) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/global_configuration.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/global_configuration.rb index feeed59195f..77955eb3ee2 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/global_configuration.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/global_configuration.rb @@ -43,7 +43,7 @@ module Plugins # @api private class GlobalConfiguration < Seahorse::Client::Plugin - @identifiers = Set.new() + @identifiers = Set.new # @api private def before_initialize(client_class, options) @@ -55,17 +55,18 @@ def before_initialize(client_class, options) private def apply_service_defaults(client_class, options) - if defaults = Aws.config[client_class.identifier] - defaults.each do |option_name, default| - options[option_name] = default unless options.key?(option_name) - end + return unless (defaults = Aws.config[client_class.identifier]) + + defaults.each do |option_name, default| + options[option_name] = default unless options.key?(option_name) end end - def apply_aws_defaults(client_class, options) + def apply_aws_defaults(_client_class, options) Aws.config.each do |option_name, default| next if self.class.identifiers.include?(option_name) next if options.key?(option_name) + options[option_name] = default end end @@ -80,9 +81,7 @@ def add_identifier(identifier) # @return [Set] # @api private - def identifiers - @identifiers - end + attr_reader :identifiers end end diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index a50b98f27b2..c44a899c5fc 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -113,7 +113,6 @@ class RetryErrors < Seahorse::Client::Plugin functionality of `standard` mode along with automatic client side throttling. This is a provisional mode that may change behavior in the future. - DOCS resolve_retry_mode(cfg) end diff --git a/gems/aws-sdk-core/lib/seahorse/client/base.rb b/gems/aws-sdk-core/lib/seahorse/client/base.rb index 64c69ed1c20..c1f28196280 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/base.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/base.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require 'thread' - module Seahorse module Client class Base @@ -60,6 +58,7 @@ def operation_names def build_config(plugins, options) config = Configuration.new config.add_option(:api) + config.add_option(:plugins) plugins.each do |plugin| plugin.add_options(config) if plugin.respond_to?(:add_options) end @@ -96,9 +95,9 @@ def context_for(operation_name, params) class << self def new(options = {}) - plugins = build_plugins options = options.dup - before_initialize(plugins, options) + plugins = build_plugins(self.plugins + options.fetch(:plugins, [])) + plugins = before_initialize(plugins, options) client = allocate client.send(:initialize, plugins, options) client @@ -209,17 +208,28 @@ def define_operation_methods include(operations_module) end - def build_plugins + def build_plugins(plugins) plugins.map { |plugin| plugin.is_a?(Class) ? plugin.new : plugin } end def before_initialize(plugins, options) - plugins.each do |plugin| - plugin.before_initialize(self, options) if plugin.respond_to?(:before_initialize) + queue = Queue.new + plugins.each { |plugin| queue.push(plugin) } + until queue.empty? + plugin = queue.pop + next unless plugin.respond_to?(:before_initialize) + + plugins_before = options.fetch(:plugins, []) + plugin.before_initialize(self, options) + plugins_after = build_plugins(options.fetch(:plugins, []) - plugins_before) + # Plugins with before_initialize can add other plugins + plugins_after.each { |p| queue.push(p); plugins << p } end + plugins end def inherited(subclass) + super subclass.instance_variable_set('@plugins', PluginList.new(@plugins)) end diff --git a/gems/aws-sdk-core/lib/seahorse/client/plugins/endpoint.rb b/gems/aws-sdk-core/lib/seahorse/client/plugins/endpoint.rb index ffc29e5ce0e..c0bfe44e4f5 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/plugins/endpoint.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/plugins/endpoint.rb @@ -17,7 +17,6 @@ class Endpoint < Plugin 'http://example.com' 'https://example.com' 'http://example.com:123' - DOCS def add_handlers(handlers, config) diff --git a/gems/aws-sdk-core/spec/aws/plugins/global_configuration_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/global_configuration_spec.rb index e58082cec9a..6c349f18120 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/global_configuration_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/global_configuration_spec.rb @@ -13,10 +13,23 @@ module Plugins option(:property, 'plugin-default') end) + let(:plugin) do + p = double('plugin') + allow(p).to receive(:is_a?).with(kind_of(Class)).and_return(false) + p + end + + let(:options) do + { + region: 'us-east-1', + credentials: Credentials.new('akid', 'secret'), + plugins: [plugin] + } + end + before(:each) do Aws.config.clear - Aws.config[:region] = 'us-east-1' - Aws.config[:credentials] = Credentials.new('akid', 'secret') + Aws.config = options end before(:all) do @@ -45,6 +58,43 @@ module Plugins expect(GlobalConfigClient.new(property: 'arg').config.property).to eq('arg') end + context 'plugins' do + it 'instructs plugins to #before_initialize' do + expect(plugin).to receive(:before_initialize).with(GlobalConfigClient, options) + GlobalConfigClient.new + end + + it 'instructs plugins to #add_options' do + expect(plugin).to receive(:add_options) + GlobalConfigClient.new + end + + it 'instructs plugins to #add_handlers' do + expect(plugin).to receive(:add_handlers). + with(kind_of(Seahorse::Client::HandlerList), kind_of(Struct)) + GlobalConfigClient.new + end + + it 'instructs plugins to #after_initialize' do + expect(plugin).to receive(:after_initialize).with(kind_of(Seahorse::Client::Base)) + GlobalConfigClient.new + end + + it 'does not call methods that plugin does not respond to' do + plugin = Object.new + allow(plugin).to receive(:respond_to?).with(:before_initialize).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_options).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_handlers).and_return(false) + allow(plugin).to receive(:respond_to?).with(:after_initialize).and_return(false) + expect(plugin).not_to receive(:before_initialize) + expect(plugin).not_to receive(:add_options) + expect(plugin).not_to receive(:add_handlers) + expect(plugin).not_to receive(:after_initialize) + Aws.config[:plugins] = [plugin] + GlobalConfigClient.new + end + end + end end end diff --git a/gems/aws-sdk-core/spec/seahorse/client/base_spec.rb b/gems/aws-sdk-core/spec/seahorse/client/base_spec.rb index 88b7b4c6b28..e02e76bafd8 100644 --- a/gems/aws-sdk-core/spec/seahorse/client/base_spec.rb +++ b/gems/aws-sdk-core/spec/seahorse/client/base_spec.rb @@ -9,9 +9,9 @@ module Client let(:api) { Seahorse::Model::Api.new } - let(:client_class) { Base.define(api:api) } + let(:client_class) { Base.define(api: api) } - let(:client) { client_class.new(endpoint:'http://example.com') } + let(:client) { client_class.new(endpoint: 'http://example.com') } let(:plugin_a) { Class.new } @@ -143,10 +143,10 @@ module Client it 'builds and sends a request when it receives a request method' do expect(client).to receive(:build_request). - with(:operation_name, {foo:'bar'}). + with(:operation_name, { foo: 'bar' }). and_return(request) expect(request).to receive(:send_request) - client.operation_name(foo:'bar') + client.operation_name(foo: 'bar') end it 'passes block arguments to the request method' do @@ -155,7 +155,7 @@ module Client and_yield('chunk2'). and_yield('chunk3') chunks = [] - client.operation_name(foo:'bar') do |chunk| + client.operation_name(foo: 'bar') do |chunk| chunks << chunk end expect(chunks).to eq(%w(chunk1 chunk2 chunk3)) @@ -269,59 +269,111 @@ module Client it 'has a defualt list of plugins' do client_class = Class.new(Base) - expect(client_class.plugins.to_a).to eq([ + expected = [ Plugins::Endpoint, Plugins::NetHttp, Plugins::RaiseResponseErrors, Plugins::ResponseTarget, Plugins::RequestCallback - ]) + ] + expect(client_class.plugins.to_a).to eq(expected) end end describe '.new' do - let(:plugin) { + let(:plugin) do p = double('plugin') allow(p).to receive(:is_a?).with(kind_of(Class)).and_return(false) p - } - - it 'instructs plugins to #before_initialize' do - options = { endpoint: 'http://foo.com' } - expect(plugin).to receive(:before_initialize).with(client_class, options) - client_class.add_plugin(plugin) - client_class.new(options) end - it 'instructs plugins to #add_options' do - expect(plugin).to receive(:add_options) do |config| - config.add_option(:foo, 'bar') - config.add_option(:endpoint, 'http://foo.com') - config.add_option(:regional_endpoint, false) + context 'class level plugin' do + it 'instructs plugins to #before_initialize' do + options = { endpoint: 'http://foo.com' } + expect(plugin).to receive(:before_initialize).with(client_class, options) + client_class.add_plugin(plugin) + client_class.new(options) end - client_class.add_plugin(plugin) - expect(client_class.new.config.foo).to eq('bar') - end - it 'instructs plugins to #add_handlers' do - expect(plugin).to receive(:add_handlers). - with(kind_of(HandlerList), kind_of(Struct)) - client_class.add_plugin(plugin) - client_class.new(endpoint:'http://foo.com') - end + it 'instructs plugins to #add_options' do + expect(plugin).to receive(:add_options) do |config| + config.add_option(:foo, 'bar') + config.add_option(:endpoint, 'http://foo.com') + config.add_option(:regional_endpoint, false) + end + client_class.add_plugin(plugin) + expect(client_class.new.config.foo).to eq('bar') + end - it 'instructs plugins to #after_initialize' do - expect(plugin).to receive(:after_initialize).with(kind_of(Client::Base)) - client_class.add_plugin(plugin) - client_class.new(endpoint:'http://foo.com') + it 'instructs plugins to #add_handlers' do + expect(plugin).to receive(:add_handlers). + with(kind_of(HandlerList), kind_of(Struct)) + client_class.add_plugin(plugin) + client_class.new(endpoint: 'http://foo.com') + end + + it 'instructs plugins to #after_initialize' do + expect(plugin).to receive(:after_initialize).with(kind_of(Client::Base)) + client_class.add_plugin(plugin) + client_class.new(endpoint: 'http://foo.com') + end + + it 'does not call methods that plugin does not respond to' do + plugin = Object.new + allow(plugin).to receive(:respond_to?).with(:before_initialize).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_options).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_handlers).and_return(false) + allow(plugin).to receive(:respond_to?).with(:after_initialize).and_return(false) + expect(plugin).not_to receive(:before_initialize) + expect(plugin).not_to receive(:add_options) + expect(plugin).not_to receive(:add_handlers) + expect(plugin).not_to receive(:after_initialize) + client_class.add_plugin(plugin) + client_class.new(endpoint: 'http://foo.com') + end end - it 'does not call methods that plugin does not respond to' do - plugin = Object.new - client_class.add_plugin(plugin) - client_class.new(endpoint:'http://foo.com') + context 'instance level plugin' do + it 'instructs plugins to #before_initialize' do + options = { endpoint: 'http://foo.com', plugins: [plugin] } + expect(plugin).to receive(:before_initialize).with(client_class, options) + client_class.new(options) + end + + it 'instructs plugins to #add_options' do + expect(plugin).to receive(:add_options) do |config| + config.add_option(:foo, 'bar') + config.add_option(:endpoint, 'http://foo.com') + config.add_option(:regional_endpoint, false) + end + client_class.new(endpoint: 'http://foo.com', plugins: [plugin]) + end + + it 'instructs plugins to #add_handlers' do + expect(plugin).to receive(:add_handlers). + with(kind_of(HandlerList), kind_of(Struct)) + client_class.new(endpoint: 'http://foo.com', plugins: [plugin]) + end + + it 'instructs plugins to #after_initialize' do + expect(plugin).to receive(:after_initialize).with(kind_of(Client::Base)) + client_class.new(endpoint: 'http://foo.com', plugins: [plugin]) + end + + it 'does not call methods that plugin does not respond to' do + plugin = Object.new + allow(plugin).to receive(:respond_to?).with(:before_initialize).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_options).and_return(false) + allow(plugin).to receive(:respond_to?).with(:add_handlers).and_return(false) + allow(plugin).to receive(:respond_to?).with(:after_initialize).and_return(false) + expect(plugin).not_to receive(:before_initialize) + expect(plugin).not_to receive(:add_options) + expect(plugin).not_to receive(:add_handlers) + expect(plugin).not_to receive(:after_initialize) + client_class.new(endpoint: 'http://foo.com', plugins: [plugin]) + end end end diff --git a/gems/aws-sdk-s3/spec/client_spec.rb b/gems/aws-sdk-s3/spec/client_spec.rb index bbd262193d0..7e97440128b 100644 --- a/gems/aws-sdk-s3/spec/client_spec.rb +++ b/gems/aws-sdk-s3/spec/client_spec.rb @@ -7,10 +7,10 @@ module Aws module S3 describe Client do - let(:client) { Client.new } + let(:client) { Client.new(options) } - before do - Aws.config[:s3] = { + let(:options) do + { region: 'us-east-1', credentials: Credentials.new('akid', 'secret'), retry_backoff: ->(*args) {} @@ -18,7 +18,6 @@ module S3 end after do - Aws.config = {} Aws::S3.bucket_region_cache.clear end @@ -36,8 +35,8 @@ module S3 describe 'request ids' do it 'populates request id and host id in the response context' do - s3 = Client.new(stub_responses: true) - s3.handle(step: :send) do |context| + options.merge!(stub_responses: true) + client.handle(step: :send) do |context| context.http_response.signal_done( status_code: 200, headers: { @@ -57,7 +56,7 @@ module S3 XML Seahorse::Client::Response.new(context: context) end - resp = s3.list_buckets + resp = client.list_buckets expect(resp.context[:request_id]).to eq('BE9C18E622969B17') expect(resp.context[:s3_host_id]).to eq( 'H0vUEO2f4PyWtNjgcb3TSdyHaie8j4IgnuKIW2'\ @@ -287,18 +286,18 @@ module S3 describe 'https required for sse cpk' do it 'raises a runtime error when attempting SSE CPK over HTTP' do - s3 = Client.new(endpoint: 'http://s3.amazonaws.com') + options.merge!(endpoint: 'http://s3.amazonaws.com') # put_object expect do - s3.put_object( + client.put_object( bucket: 'aws-sdk', key: 'key', sse_customer_key: 'secret' ) end.to raise_error(/HTTPS/) # copy_object_source expect do - s3.copy_object( + client.copy_object( bucket: 'aws-sdk', key: 'key', copy_source: 'bucket#key', @@ -320,36 +319,35 @@ module S3 describe 'invalid Expires header' do %w[get_object head_object].each do |method| it "correctly handled invalid Expires header for #{method}" do - s3 = Client.new - s3.handle(step: :send) do |context| + client.handle(step: :send) do |context| context.http_response.signal_headers(200, 'Expires' => 'abc') context.http_response.signal_done Seahorse::Client::Response.new(context: context) end - resp = s3.send(method, bucket: 'b', key: 'k') + resp = client.send(method, bucket: 'b', key: 'k') expect(resp.expires).to be(nil) expect(resp.expires_string).to eq('abc') end it 'accepts a stubbed Expires header as a Time value' do now = Time.at(Time.now.to_i) - s3 = Client.new( + options.merge!( stub_responses: { method.to_sym => { expires: now } } ) - resp = s3.send(method, bucket: 'b', key: 'k') + resp = client.send(method, bucket: 'b', key: 'k') expect(resp.expires).to eq(now) expect(resp.expires_string).to eq(now.httpdate) end it 'accepts a stubbed Expires header as String value' do - s3 = Client.new( + options.merge!( stub_responses: { method.to_sym => { expires_string: 'abc' } } ) - resp = s3.send(method, bucket: 'b', key: 'k') + resp = client.send(method, bucket: 'b', key: 'k') expect(resp.expires).to be(nil) expect(resp.expires_string).to eq('abc') end @@ -358,22 +356,22 @@ module S3 describe '#create_bucket' do it 'omits location constraint for the classic region', rbs_test: :skip do - s3 = Client.new(region: 'us-east-1') - s3.handle(step: :send) do |context| + options.merge!(region: 'us-east-1') + client.handle(step: :send) do |context| context.http_response.status_code = 200 Seahorse::Client::Response.new(context: context) end - resp = s3.create_bucket(bucket: 'aws-sdk') + resp = client.create_bucket(bucket: 'aws-sdk') expect(resp.context.http_request.body_contents).to eq('') end it 'populates the bucket location constraint for non-classic regions', rbs_test: :skip do - s3 = Client.new(region: 'us-west-2') - s3.handle(step: :send) do |context| + options.merge!(region: 'us-west-2') + client.handle(step: :send) do |context| context.http_response.status_code = 200 Seahorse::Client::Response.new(context: context) end - resp = s3.create_bucket(bucket: 'aws-sdk') + resp = client.create_bucket(bucket: 'aws-sdk') expect(resp.context.http_request.body_contents.strip) .to eq(<<-XML.gsub(/(^\s+|\n)/, '')) @@ -382,13 +380,13 @@ module S3 XML end - it 'does not overide bucket location constraint params', rbs_test: :skip do - s3 = Client.new(region: 'eu-west-1') - s3.handle(step: :send) do |context| + it 'does not override bucket location constraint params', rbs_test: :skip do + options.merge!(region: 'eu-west-1') + client.handle(step: :send) do |context| context.http_response.status_code = 200 Seahorse::Client::Response.new(context: context) end - resp = s3.create_bucket( + resp = client.create_bucket( bucket: 'aws-sdk', create_bucket_configuration: { location_constraint: 'EU' @@ -403,12 +401,12 @@ module S3 end it 'does not apply location constraint if location is set', rbs_test: :skip do - s3 = Client.new(region: 'eu-west-1') - s3.handle(step: :send) do |context| + options.merge!(region: 'eu-west-1') + client.handle(step: :send) do |context| context.http_response.status_code = 200 Seahorse::Client::Response.new(context: context) end - resp = s3.create_bucket( + resp = client.create_bucket( bucket: 'aws-sdk', create_bucket_configuration: { location: { type: 'AvailabilityZone', name: 'us-west-1a' } diff --git a/gems/aws-sdk-sqs/spec/client_spec.rb b/gems/aws-sdk-sqs/spec/client_spec.rb index cd915280d0e..51343e44c07 100644 --- a/gems/aws-sdk-sqs/spec/client_spec.rb +++ b/gems/aws-sdk-sqs/spec/client_spec.rb @@ -6,18 +6,12 @@ module Aws module SQS describe Client do - let(:client) { Client.new } - - before(:each) do - Aws.config[:sqs] = { + let(:client) do + Client.new( region: 'us-east-1', credentials: Credentials.new('akid', 'secret'), retry_limit: 0 - } - end - - after(:each) do - Aws.config = {} + ) end describe 'empty result element' do