From e7c7f1de08e322e6fb86722d0b7d23a21e45047e Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 25 Aug 2023 12:47:17 -0400 Subject: [PATCH 1/2] Move interceptor context out of middleware context. Enforce Signer interface --- hearth/lib/hearth/context.rb | 10 ------- hearth/lib/hearth/interceptor_list.rb | 13 ++++++-- hearth/lib/hearth/signers.rb | 4 +-- hearth/lib/hearth/signers/anonymous.rb | 10 ++++++- hearth/spec/hearth/context_spec.rb | 13 -------- hearth/spec/hearth/interceptor_list_spec.rb | 28 +++++++++++++----- hearth/spec/hearth/signers/anonymous_spec.rb | 31 ++++++++++++++++++++ hearth/spec/hearth/signers_spec.rb | 25 ++++++++++++++++ 8 files changed, 99 insertions(+), 35 deletions(-) create mode 100644 hearth/spec/hearth/signers/anonymous_spec.rb create mode 100644 hearth/spec/hearth/signers_spec.rb diff --git a/hearth/lib/hearth/context.rb b/hearth/lib/hearth/context.rb index 87a092717..c28f85c54 100644 --- a/hearth/lib/hearth/context.rb +++ b/hearth/lib/hearth/context.rb @@ -52,15 +52,5 @@ def [](key) def []=(key, value) @metadata[key] = value end - - # @api private - def interceptor_context(input, output) - Hearth::Interceptor::Context.new( - input: input, - request: request, - response: response, - output: output - ) - end end end diff --git a/hearth/lib/hearth/interceptor_list.rb b/hearth/lib/hearth/interceptor_list.rb index 305902755..b32c5955b 100644 --- a/hearth/lib/hearth/interceptor_list.rb +++ b/hearth/lib/hearth/interceptor_list.rb @@ -45,13 +45,13 @@ def add(interceptor) # error is encountered. # @return nil if successful, an exception otherwise def apply(hook:, input:, context:, output:, aggregate_errors: false) - ictx = context.interceptor_context(input, output) + i_ctx = interceptor_context(input, context, output) last_error = nil @interceptors.each do |i| next unless i.respond_to?(hook) begin - i.send(hook, ictx) + i.send(hook, i_ctx) rescue StandardError => e context.logger.error(last_error) if last_error last_error = e @@ -74,6 +74,15 @@ def each(&block) private + def interceptor_context(input, context, output) + Hearth::Interceptor::Context.new( + input: input, + request: context.request, + response: context.response, + output: output + ) + end + def set_output_error(last_error, context, output) return unless last_error && output diff --git a/hearth/lib/hearth/signers.rb b/hearth/lib/hearth/signers.rb index c8f49cd2b..5a0ee2d26 100644 --- a/hearth/lib/hearth/signers.rb +++ b/hearth/lib/hearth/signers.rb @@ -6,11 +6,11 @@ module Signers # Base class for all Signer classes. class Base def sign(request:, identity:, properties:) - # Do nothing by default + raise NotImplementedError end def reset(request:, properties:) - # Do nothing by default + raise NotImplementedError end end end diff --git a/hearth/lib/hearth/signers/anonymous.rb b/hearth/lib/hearth/signers/anonymous.rb index 8a697de3e..1c046c615 100644 --- a/hearth/lib/hearth/signers/anonymous.rb +++ b/hearth/lib/hearth/signers/anonymous.rb @@ -3,6 +3,14 @@ module Hearth module Signers # A signer that does not sign requests. - class Anonymous < Signers::Base; end + class Anonymous < Signers::Base + def sign(request:, identity:, properties:) + # Do nothing. + end + + def reset(request:, properties:) + # Do nothing. + end + end end end diff --git a/hearth/spec/hearth/context_spec.rb b/hearth/spec/hearth/context_spec.rb index e13de7926..e470dfcb4 100644 --- a/hearth/spec/hearth/context_spec.rb +++ b/hearth/spec/hearth/context_spec.rb @@ -47,19 +47,6 @@ module Hearth end end - describe '#interceptor_context' do - let(:input) { double('input') } - let(:output) { double('output') } - - it 'creates an interceptor context' do - ictx = subject.interceptor_context(input, output) - expect(ictx.input).to eq(input) - expect(ictx.request).to eq(request) - expect(ictx.response).to eq(response) - expect(ictx.output).to eq(output) - end - end - describe '#[]' do it 'returns the metadata for the given key' do expect(subject[:foo]).to eq('bar') diff --git a/hearth/spec/hearth/interceptor_list_spec.rb b/hearth/spec/hearth/interceptor_list_spec.rb index 92fb45524..513bcbbff 100644 --- a/hearth/spec/hearth/interceptor_list_spec.rb +++ b/hearth/spec/hearth/interceptor_list_spec.rb @@ -71,8 +71,17 @@ def read_before_execution(_ctx); end let(:logger) { double('logger') } let(:input) { double('input') } let(:output) { double('output', error: nil) } - let(:context) { double('context', logger: logger) } - let(:ictx) { double('interceptor_context') } + let(:request) { double('request') } + let(:response) { double('response') } + let(:context) do + double( + 'context', + request: request, + response: response, + logger: logger + ) + end + let(:i_ctx) { double('interceptor_context') } let(:error) { StandardError.new } let(:interceptors) do @@ -81,14 +90,19 @@ def read_before_execution(_ctx); end let(:hook) { :read_before_execution } before(:each) do - allow(context).to receive(:interceptor_context).and_return(ictx) + allow(context).to receive(:interceptor_context).and_return(i_ctx) end it 'calls each interceptor hook with context' do - expect(context).to receive(:interceptor_context) - .with(input, output).and_return(ictx) - expect(interceptor1).to receive(hook).with(ictx) - expect(interceptor2).to receive(hook).with(ictx) + expect(Interceptor::Context).to receive(:new).with( + input: input, + request: request, + response: response, + output: output + ).and_return(i_ctx) + + expect(interceptor1).to receive(hook).with(i_ctx) + expect(interceptor2).to receive(hook).with(i_ctx) out = interceptors.apply( hook: hook, diff --git a/hearth/spec/hearth/signers/anonymous_spec.rb b/hearth/spec/hearth/signers/anonymous_spec.rb new file mode 100644 index 000000000..4a95ec61d --- /dev/null +++ b/hearth/spec/hearth/signers/anonymous_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module Hearth + module Signers + describe Anonymous do + let(:request) { HTTP::Request.new } + let(:identity) { Identities::Anonymous.new } + let(:properties) { {} } + + describe '#sign' do + it 'does not modify the request' do + expect do + subject.sign( + request: request, + identity: identity, + properties: properties + ) + end.not_to(change { request }) + end + end + + describe '#reset' do + it 'does not modify the request' do + expect do + subject.reset(request: request, properties: properties) + end.not_to(change { request }) + end + end + end + end +end diff --git a/hearth/spec/hearth/signers_spec.rb b/hearth/spec/hearth/signers_spec.rb new file mode 100644 index 000000000..57494a42a --- /dev/null +++ b/hearth/spec/hearth/signers_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Hearth + module Signers + describe Base do + let(:request) { HTTP::Request.new } + let(:identity) { Identities::Anonymous.new } + let(:properties) { {} } + + it 'defines the interface' do + expect do + subject.sign( + request: request, + identity: identity, + properties: properties + ) + end.to raise_error(NotImplementedError) + + expect do + subject.reset(request: request, properties: properties) + end.to raise_error(NotImplementedError) + end + end + end +end From 90d1af1bbb4d6491611cf5cbdd391f0f4563fcbc Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 25 Aug 2023 12:49:33 -0400 Subject: [PATCH 2/2] Upgrade Smithy version --- codegen/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen/build.gradle.kts b/codegen/build.gradle.kts index f8e0cd5ae..01d9e9a1f 100644 --- a/codegen/build.gradle.kts +++ b/codegen/build.gradle.kts @@ -28,7 +28,7 @@ allprojects { version = "0.1.0" } -extra["smithyVersion"] = "1.33.0" +extra["smithyVersion"] = "1.37.0" // The root project doesn't produce a JAR. tasks["jar"].enabled = false