Parameter Validation #256
Parameter Validation #256
Conversation
This reverts commit 1c626da.
| @@ -91,7 +91,30 @@ def member(name) | |||
| class IntegerShape < Shape; end | |||
|
|
|||
| # Represents an IntEnum shape. | |||
| class IntEnumShape < EnumShape; end | |||
| class IntEnumShape < Shape | |||
There was a problem hiding this comment.
Inheriting from EnumShape and StructureShape makes is_a? checks order dependent for visiting. While this is duplicating, it's safer.
There was a problem hiding this comment.
If we wanted to reduce the duplicaiton, we could do use an include with some sort of ShapeWithMembers module or something. But the duplication is minimal and fine.
| return PRELUDE_SHAPES_MAP[id] if PRELUDE_SHAPES_MAP.key?(id) | ||
|
|
||
| Model::Shape.relative_id(id) | ||
| Model::Shape.name(id).camelize |
There was a problem hiding this comment.
This fixes a bug where constants in add_member are not camelized and do not exist.
| @@ -4,95 +4,18 @@ | |||
|
|
|||
| module ClientHelper | |||
There was a problem hiding this comment.
I went back and forth on this but I'm happy with the result - I went too far with the refactor and dialed it back some. I think that the smithy spec_helper/client_helper should work only on fixtures. The purpose of those is to verify that the generated code/files are correct. I prefer the explicit module name declaration in the tests for smithy's spec_helper. I think having smithy-client rely on another gem's spec helper was funky, so I pulled out sample_shapes into smithy-clients client_helper, and I think it's ok to have two. At this point, when using this helper, we can assume the generated code is correct. The quick runtime client generation is great for smithy-client tests and does not need fixtures in my opinion, as we probably want to change model values during tests.
| @@ -91,7 +91,30 @@ def member(name) | |||
| class IntegerShape < Shape; end | |||
|
|
|||
| # Represents an IntEnum shape. | |||
| class IntEnumShape < EnumShape; end | |||
| class IntEnumShape < Shape | |||
There was a problem hiding this comment.
If we wanted to reduce the duplicaiton, we could do use an include with some sort of ShapeWithMembers module or something. But the duplication is minimal and fine.
| describe 'floats' do | ||
| it 'accepts a float' do | ||
| validate(float: 123.0) | ||
| validate({ float: 123 }, |
There was a problem hiding this comment.
This validation feels weird to me - it seems like we should accept an integer here?
There was a problem hiding this comment.
In v3 we do both param validation and param conversion. Param validation should ensure strictness-ish and param conversion can convert integers given to float shapes as floats. This makes error messaging cleaner maybe. Unless you have a different idea.
| Object.const_get(namespace) | ||
| rescue Exception => e # rubocop:disable Lint/RescueException | ||
| Object.const_get(module_name) | ||
| rescue LoadError => e |
There was a problem hiding this comment.
Does LoadError cover all of the syntax errors that could happen during eval?
There was a problem hiding this comment.
I think so. At this point we assume generated code is valid.
Adds parameter validation based on API rules.