-
Notifications
You must be signed in to change notification settings - Fork 7
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
Defaults #297
Changes from all commits
5e4c880
a4aac5b
8db39a1
688829d
ae78f16
67fa77d
2220b93
7cffe7d
5d37bc0
9662011
0b3bb17
7e1cadc
583e1a1
8301c53
3045d7f
927c26b
9cd1318
3da4fa0
c759002
7f8c65a
748557a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
module Smithy | ||
module CBOR | ||
# TODO: test all codec cases | ||
describe Codec do | ||
let(:shape) { SchemaHelper.sample_schema.const_get(:Structure) } | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason we're removing these tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'base64' | ||
require 'bigdecimal' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
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. | ||
# | ||
|
@@ -245,7 +246,6 @@ def each_base_class(shape_class, &) | |
end | ||
|
||
add(UnionShape, Hash) { |h, _| h.dup } | ||
add(UnionShape, Schema::Union) | ||
end | ||
end | ||
end |
Uh oh!
There was an error while loading. Please reload this page.