Skip to content

Commit

Permalink
More refactors and test passing
Browse files Browse the repository at this point in the history
  • Loading branch information
mullermp committed Apr 16, 2024
1 parent 14f2bfa commit 5070c61
Show file tree
Hide file tree
Showing 15 changed files with 39 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ def match_req_body(suite, test_case, http_req, it)
end

def match_resp_data(test_case, resp, it)
data = ProtocolTestsHelper.data_to_hash(resp.data)
resp_data = ProtocolTestsHelper.data_to_hash(resp.data)
expected_data =
if error_case?(test_case)
error_shape = resp.context.operation.errors.find do |err|
Expand All @@ -285,7 +285,7 @@ def match_resp_data(test_case, resp, it)
)
end
if test_case['response']['eventstream']
data.each do |member_name, value|
resp_data.each do |member_name, value|
if value.respond_to?(:each)
# event stream member
value.each do |event_struct|
Expand All @@ -300,7 +300,7 @@ def match_resp_data(test_case, resp, it)
end
end
else
it.expect(data).to it.eq(expected_data)
it.expect(resp_data).to it.eq(expected_data)
end
end

Expand All @@ -321,12 +321,4 @@ def error_case?(test_case)
end
end
end
end

# @api private
# RSpec::Matchers.define :match_cbor_float do |expected|
# match do |actual|
# expect(actual).to be_within(0.0001).of(expected)
# end
# diffable
# end
end
4 changes: 2 additions & 2 deletions gems/aws-sdk-core/lib/aws-sdk-core/cbor.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

require_relative 'cbor/cbor_engine'
require_relative 'cbor/default_cbor_engine'
require_relative 'cbor/engines/cbor_engine'
require_relative 'cbor/engines/default_cbor_engine'

module Aws
# @api private
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# frozen_string_literal: true

require_relative 'half'
require_relative 'tagged'
require_relative 'encoder'
require_relative 'decoder'
require_relative '../half'

This comment has been minimized.

Copy link
@alextwoods

alextwoods Apr 16, 2024

Contributor

This seems like a slightly weird file structure now. Maybe we should have a module for our engine and have these files there?

This comment has been minimized.

Copy link
@mullermp

mullermp Apr 16, 2024

Author Contributor

They're only used by this engine

This comment has been minimized.

Copy link
@alextwoods

alextwoods Apr 16, 2024

Contributor

Yeah, exactly they are only used by our engine, so maybe shouldn't be on the top level of cbor.

This comment has been minimized.

Copy link
@alextwoods

alextwoods Apr 16, 2024

Contributor

Well excepted Tagged, but see other comment....

require_relative '../tagged'
require_relative '../encoder'
require_relative '../decoder'

module Aws
module Cbor
Expand Down
6 changes: 0 additions & 6 deletions gems/aws-sdk-core/lib/aws-sdk-core/error_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ def error(context)
else
code, message, data = extract_error(body, context)
end
headers = context.http_response.headers
context[:request_id] = request_id(headers, body)
errors_module = context.client.class.errors_module
errors_module.error_class(code).new(context, message, data)
end
Expand All @@ -33,9 +31,5 @@ def http_status_error_code(context)
}[status_code] || "Http#{status_code}Error"
end

def request_id(headers, _body)
headers['x-amz-request-id'] || headers['x-amzn-requestid']
end

end
end
5 changes: 2 additions & 3 deletions gems/aws-sdk-core/lib/aws-sdk-core/json.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
# frozen_string_literal: true

require 'json'
require_relative 'json/builder'
require_relative 'json/error_handler'
require_relative 'json/handler'
require_relative 'json/parser'
require_relative 'json/json_engine'
require_relative 'json/oj_engine'
require_relative 'json/engines/json_engine'
require_relative 'json/engines/oj_engine'

module Aws
# @api private
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'json'

module Aws
module Json
module JSONEngine
Expand Down
3 changes: 1 addition & 2 deletions gems/aws-sdk-core/lib/aws-sdk-core/json/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ def call(context)
build_request(context)
response = @handler.call(context)
response.on(200..299) { |resp| parse_response(resp) }
response.on(200..599) { |resp| apply_request_id(context) }
response
response.on(200..599) { |_resp| apply_request_id(context) }
end

private
Expand Down
8 changes: 4 additions & 4 deletions gems/aws-sdk-core/lib/aws-sdk-core/query/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ class Handler < Seahorse::Client::Handler
# @return [Seahorse::Client::Response]
def call(context)
build_request(context)
@handler.call(context).on_success do |response|
response.error = nil
@handler.call(context).on_success do |resp|
resp.error = nil
parsed = parse_xml(context)
if parsed.nil? || parsed == EmptyStructure
response.data = EmptyStructure.new
resp.data = EmptyStructure.new
else
response.data = parsed
resp.data = parsed
end
end
end
Expand Down
7 changes: 3 additions & 4 deletions gems/aws-sdk-core/lib/aws-sdk-core/rest/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ class Handler < Seahorse::Client::Handler

def call(context)
Rest::Request::Builder.new.apply(context)
resp = @handler.call(context)
resp.on(200..299) { |response| Response::Parser.new.apply(response) }
resp.on(200..599) { |response| apply_request_id(context) }
resp
response = @handler.call(context)
response.on(200..299) { |resp| Response::Parser.new.apply(resp) }
response.on(200..599) { |_resp| apply_request_id(context) }
end

private
Expand Down
6 changes: 0 additions & 6 deletions gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,3 @@
require_relative 'rpc_v2/error_handler'
require_relative 'rpc_v2/builder'
require_relative 'rpc_v2/parser'

module Aws
# @api private
module RpcV2
end
end
4 changes: 4 additions & 0 deletions gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ def initialize(rules, _options = {})
end

def serialize(params)
# If the input shape is empty, do not set a body. This is
# different than if the input shape is a structure with no members.
return nil if @rules.shape.struct_class == EmptyStructure

Cbor.encode(format(@rules, params))
end

Expand Down
18 changes: 11 additions & 7 deletions gems/aws-sdk-core/lib/aws-sdk-core/rpc_v2/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ class Handler < Seahorse::Client::Handler
def call(context)
build_request(context)
response = @handler.call(context)
response.on(200..299) { |resp| resp.data = parse_body(resp.context) }
response.on(200..299) { |resp| resp.data = parse_body(context) }
response.on(200..599) { |_resp| apply_request_id(context) }
response
end

Expand All @@ -19,8 +20,8 @@ def build_request(context)
build_url(context)
context.http_request.headers['smithy-protocol'] = 'rpc-v2-cbor'
context.http_request.headers['Accept'] = 'application/cbor' # remove?
context.http_request.headers['Content-Type'] ||= 'application/cbor' # specific to input
context.http_request.body = build_body(context)
apply_content_type_header(context)
end

def build_url(context)
Expand All @@ -33,6 +34,14 @@ def build_body(context)
Builder.new(context.operation.input).serialize(context.params)
end

def apply_content_type_header(context)
# If the input shape is empty, do not set the content type. This is
# different than if the input shape is a structure with no members.
return if context.operation.input.shape.struct_class == EmptyStructure

context.http_request.headers['Content-Type'] = 'application/cbor'
end

def parse_body(context)
cbor_data = context.http_response.body_contents.force_encoding(Encoding::BINARY)
Parser.new(
Expand All @@ -41,11 +50,6 @@ def parse_body(context)
).parse(cbor_data)
end

def target(context)
prefix = context.config.api.metadata['targetPrefix']
"#{prefix}.#{context.operation.name}"
end

def apply_request_id(context)
context[:request_id] = context.http_response.headers['x-amzn-requestid']
end
Expand Down
3 changes: 2 additions & 1 deletion gems/aws-sdk-core/lib/aws-sdk-core/xml/error_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ def call(context)
private

def extract_error(body, context)
context[:request_id] = request_id(body)
code = error_code(body, context)
[
code,
Expand Down Expand Up @@ -84,7 +85,7 @@ def error_message(body)
end
end

def request_id(_headers, body)
def request_id(body)
if (matches = body.match(/<RequestId>(.+?)<\/RequestId>/m))
matches[1]
end
Expand Down

0 comments on commit 5070c61

Please sign in to comment.