Skip to content

Defaults #297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
May 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ bundle exec smithy build --debug

local build using smithy-ruby executable:
```
export SMITHY_PLUGIN_DIR=build/smithy/source/smithy-ruby
bundle exec smithy-ruby smith client --gem-name weather --gem-version 1.0.0 --destination-root projections/weather <<< $(smithy ast model/weather.smithy)
SMITHY_PLUGIN_DIR=build/smithy/source/smithy-ruby bundle exec smithy-ruby smith client --gem-name weather --gem-version 1.0.0 --destination-root projections/weather <<< $(smithy ast model/weather.smithy)
```

### IRB
Expand Down
26 changes: 10 additions & 16 deletions gems/smithy-cbor/lib/smithy-cbor/deserializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def list(ref, values, target = nil)
values.each do |value|
next if value.nil? && !sparse?(ref.shape)

target << (value.nil? ? nil : shape(ref.shape.member, value))
target << shape(ref.shape.member, value)
end
target
end
Expand All @@ -48,37 +48,31 @@ def map(ref, values, target = nil)
values.each do |key, value|
next if value.nil? && !sparse?(ref.shape)

target[key] = value.nil? ? nil : shape(ref.shape.value, value)
target[key] = shape(ref.shape.value, value)
end
target
end

def structure(ref, values, target = nil)
return Schema::EmptyStructure.new if ref.shape == Prelude::Unit

target = ref.shape.type.new if target.nil?
ref.shape.members.each do |member_name, member_ref|
key = member_ref.member_name
next unless values.key?(key)

target[member_name] = shape(member_ref, values[key])
value = values[member_ref.member_name]
target[member_name] = shape(member_ref, value) unless value.nil?
end
target
end

def union(ref, values, target = nil) # rubocop:disable Metrics/AbcSize
raise ArgumentError, "union value includes more than one key, received: #{values.keys}" if values.size > 1

key, value = values.first
return nil if key.nil?

ref.shape.members.each do |member_name, member_ref|
name = member_ref.member_name
next unless values.key?(name)
value = values[member_ref.member_name]
next if value.nil?

target = ref.shape.member_type(member_name) if target.nil?
return target.new(shape(member_ref, values[name]))
return target.new(shape(member_ref, value))
end

values.delete('__type')
key, value = values.first
ref.shape.member_type(:unknown).new(key, value)
end

Expand Down
30 changes: 16 additions & 14 deletions gems/smithy-cbor/lib/smithy-cbor/serializer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def initialize(options = {})

def serialize(shape, data)
ref = shape.is_a?(ShapeRef) ? shape : ShapeRef.new(shape: shape)
return nil if ref.shape == Prelude::Unit
return if ref.shape == Prelude::Unit

CBOR.encode(shape(ref, data))
end
Expand All @@ -33,38 +33,40 @@ def shape(ref, value)
end

def blob(value)
value.is_a?(String) ? value : value.read
value.respond_to?(:read) ? value.read : value
end

def list(ref, values)
shape = ref.shape
values.collect do |value|
next if value.nil? && !sparse?(ref.shape)
next if value.nil? && !sparse?(shape.traits)

value.nil? ? nil : shape(ref.shape.member, value)
value.nil? ? nil : shape(shape.member, value)
end
end

def map(ref, values)
shape = ref.shape
values.each.with_object({}) do |(key, value), data|
next if value.nil? && !sparse?(ref.shape)
next if value.nil? && !sparse?(shape.traits)

data[key] = value.nil? ? nil : shape(ref.shape.value, value)
data[key] = value.nil? ? nil : shape(shape.value, value)
end
end

def structure(ref, values)
values.each_pair.with_object({}) do |(key, value), data|
if ref.shape.member?(key) && !value.nil?
member_ref = ref.shape.member(key)
data[member_ref.member_name] = shape(member_ref, value)
end
ref.shape.members.each_with_object({}) do |(member_name, member_ref), data|
value = values[member_name]
next if value.nil?

data[member_ref.member_name] = shape(member_ref, value)
end
end

def union(ref, values) # rubocop:disable Metrics/AbcSize
data = {}
if values.is_a?(Schema::Union)
member_ref = ref.shape.member_by_type(values.class)
_name, member_ref = ref.shape.member_by_type(values.class)
data[member_ref.member_name] = shape(member_ref, values).value
else
key, value = values.first
Expand All @@ -76,8 +78,8 @@ def union(ref, values) # rubocop:disable Metrics/AbcSize
data
end

def sparse?(shape)
shape.traits.include?('smithy.api#sparse')
def sparse?(traits)
traits.include?('smithy.api#sparse')
end
end
end
Expand Down
19 changes: 1 addition & 18 deletions gems/smithy-cbor/spec/smithy-cbor/codec_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

module Smithy
module CBOR
# TODO: test all codec cases
describe Codec do
let(:shape) { SchemaHelper.sample_schema.const_get(:Structure) }

Expand Down Expand Up @@ -83,31 +84,13 @@ module CBOR
expect(subject.deserialize(shape, bytes).union).to eq(nil)
end

it 'serializes an empty union' do
data = { union: {} }
bytes = subject.serialize(shape, data)
expect(subject.deserialize(shape, bytes).union).to eq(nil)
end

it 'serializes nil union values' do
data = { union: { string: nil } }
bytes = subject.serialize(shape, data)
expect(subject.deserialize(shape, bytes).to_h).to eq(data)
end

it 'deserializes unknown union members' do
unknown_union_type = shape.member(:union).shape.member_type(:unknown)
data = { union: { 'someThing' => 'someValue' } }
deserialized = subject.deserialize(shape, CBOR.encode(data))
expect(deserialized.union).to be_a(unknown_union_type)
expect(deserialized.union.to_h).to eq(unknown: { name: 'someThing', value: 'someValue' })
end

it 'raises when deserializing unions with more than one member' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason we're removing these tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was wrong behavior when I had chatted with Kevin. I'm not really sure if we should be raising. Let me think on this.

data = { union: { string: 'string', integer: 1 } }
expect { subject.deserialize(shape, CBOR.encode(data)) }
.to raise_error(ArgumentError, /union value includes more than one key/)
end
end

context 'lists' do
Expand Down
2 changes: 2 additions & 0 deletions gems/smithy-client/lib/smithy-client.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'base64'
require 'bigdecimal'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be adding bigdecimal as a required, general dependency? If I remember it can cause some issues for users without compilers/jruby and SDKs dont generally support it. Maybe add it as a dependency in client gems only when the model requires it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @hsbt has work to make it compatible everywhere. We can check.

require 'jmespath'

require 'smithy-cbor'
Expand Down
4 changes: 2 additions & 2 deletions gems/smithy-client/lib/smithy-client/handler_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class HandlerContext
# @option options [Symbol] :operation_name (nil)
# @option options [OperationShape] :operation (nil)
# @option options [Base] :client (nil)
# @option options [Hash] :params ({})
# @option options [Hash, Struct] :params ({})
# @option options [Configuration] :config (nil)
# @option options [Request] :request (HTTP::Request.new)
# @option options [Response] :response (HTTP::Response.new)
Expand All @@ -33,7 +33,7 @@ def initialize(options = {})
# @return [Base]
attr_accessor :client

# @return [Hash] The hash of request parameters.
# @return [Hash, Struct] The request parameters as a Hash or a Struct.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume Struct here would be the runtime input shape?

This is a little confusing - I assume the reason for the union of types here is that at the start of the request we have hash params and after the parameter conversion handler runs, we have runtime shapes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, at the start of the request, we would have a hash OR struct, but eventually it will always be a struct. I'm not sure if we want to do this though - but it allows us to get values from types (like defaults) if we use the input type.

attr_accessor :params

# @return [Struct] The client configuration.
Expand Down
76 changes: 38 additions & 38 deletions gems/smithy-client/lib/smithy-client/param_converter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ class ParamConverter
@mutex = Mutex.new
@converters = Hash.new { |h, k| h[k] = {} }

def initialize(schema)
def initialize(schema, convert_structures: true)
@schema = schema
@convert_structures = convert_structures
@opened_files = []
end

Expand All @@ -39,67 +40,67 @@ def c(ref, value)
self.class.c(ref.shape.class, value, self)
end

def shape(ref, value)
case ref.shape
when ListShape then list(ref, value)
when MapShape then map(ref, value)
when StructureShape then structure(ref, value)
when UnionShape then union(ref, value)
else c(ref, value)
end
end

def list(ref, values)
values = c(ref, values)
if values.is_a?(Array)
values.map { |v| member(ref.shape.member, v) }
else
values
end
return values unless values.is_a?(Array)

values.collect { |v| shape(ref.shape.member, v) }
end

def map(ref, values)
values = c(ref, values)
if values.is_a?(Hash)
values.each.with_object({}) do |(key, value), hash|
hash[member(ref.shape.key, key)] = member(ref.shape.value, value)
end
else
values
end
end
return values unless values.is_a?(Hash)

def member(ref, value)
case ref.shape
when StructureShape then structure(ref, value)
when UnionShape then union(ref, value)
when ListShape then list(ref, value)
when MapShape then map(ref, value)
else c(ref, value)
values.each.with_object({}) do |(key, value), hash|
hash[shape(ref.shape.key, key)] = shape(ref.shape.value, value)
end
end

def structure(ref, values)
values = c(ref, values)
if values.respond_to?(:each_pair)
values.each_pair do |k, v|
next if v.nil?
return if values.nil?

next unless ref.shape.member?(k)
type = @convert_structures ? ref.shape.type.new : values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When convert_structures is false, this should copy values right? Otherwise the input is being modified

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dup is called by the converter prior to this function call if it's a hash.

return type unless values.respond_to?(:each_pair)

values[k] = member(ref.shape.member(k), v)
end
values.each_pair do |k, v|
next if v.nil?
next unless ref.shape.member?(k)

type[k] = shape(ref.shape.member(k), v)
end
values
type
end

def union(ref, values)
def union(ref, values) # rubocop:disable Metrics/AbcSize
values = c(ref, values)
return if values.nil?

if values.is_a?(Schema::Union)
member_ref = ref.shape.member_by_type(values.class)
member(member_ref, values)
name, member_ref = ref.shape.member_by_type(values.class)
member_type = ref.shape.member_type(name)
member_type.new(shape(member_ref, values.value))
else
key, value = values.first
values[key] = member(ref.shape.member(key), value)
return { key => shape(ref.shape.member(key), value) } unless @convert_structures
return unless ref.shape.member?(key)

member_type = ref.shape.member_type(key)
member_type.new(shape(ref.shape.member(key), value))
end
values
end

class << self
def convert(shape, params)
new(shape).convert(params)
end

# Registers a new value converter. Converters run in the context
# of a shape and value class.
#
Expand Down Expand Up @@ -245,7 +246,6 @@ def each_base_class(shape_class, &)
end

add(UnionShape, Hash) { |h, _| h.dup }
add(UnionShape, Schema::Union)
end
end
end
5 changes: 3 additions & 2 deletions gems/smithy-client/lib/smithy-client/param_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def union(ref, values, errors, context)
return unless valid_union?(ref, values, errors, context)

if values.is_a?(Schema::Union)
member_ref = ref.shape.member_by_type(values.class)
_name, member_ref = ref.shape.member_by_type(values.class)
shape(member_ref, values.value, errors, context)
else
values.each_pair do |name, value|
Expand Down Expand Up @@ -176,7 +176,8 @@ def valid_union?(ref, values, errors, context)

def validate_required_members(ref, values, errors, context)
ref.shape.members.each do |name, member_ref|
next unless member_ref.traits.include?('smithy.api#required')
traits = member_ref.traits
next unless traits.include?('smithy.api#required') && !traits.include?('smithy.api#clientOptional')

if values[name].nil?
param = "#{context}[#{name.inspect}]"
Expand Down
4 changes: 2 additions & 2 deletions gems/smithy-client/lib/smithy-client/rpc_v2_cbor/protocol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def stub_data(service, operation, data)
ResponseStubber.new(@options).stub_data(service, operation, data)
end

def stub_error(error_code)
ResponseStubber.new(@options).stub_error(error_code)
def stub_error(service, error_code)
ResponseStubber.new(@options).stub_error(service, error_code)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def stub_data(_service, operation, data)
resp
end

def stub_error(error_code)
def stub_error(_service, error_code)
resp = HTTP::Response.new
resp.status_code = 400
resp.headers['Smithy-Protocol'] = 'rpc-v2-cbor'
Expand Down
2 changes: 2 additions & 0 deletions gems/smithy-client/lib/smithy-client/signers/http_basic.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require 'base64'

module Smithy
module Client
module Signers
Expand Down
Loading