Add validation for intEnum shapes #654
Conversation
milesziemer
force-pushed
the
add-int-enum-validation
branch
from
b43c93c to
7acd0e1
Compare
This PR adds an IntegerEnumValidator, IntegerEnumValidationFailure and the relevant tests to the ssdk libs. StructuredMemberWriter was updated to use IntegerEnumValidator when generating shape validators for intEnum shapes. Local testing: - Update js sdk to use Smithy 1.26.4, which has the protocol tests for intEnum - Update js sdk to use local version of smithy-typescript-ssdk-libs, specificaly the server protocol test packages - Run `yarn generate-clients --server-artifacts` and `yarn test:server-protocols`
milesziemer
force-pushed
the
add-int-enum-validation
branch
from
7acd0e1 to
5cd89ef
Compare
| if (constraints.isEmpty()) { | ||
| boolean shouldWriteIntEnumValidator = shape.isIntEnumShape(); | ||
|
|
||
| if (constraints.isEmpty() && !shouldWriteIntEnumValidator) { |
There was a problem hiding this comment.
Are you not able to add it as a constraint instead of adding an override here?
There was a problem hiding this comment.
Not from what I can tell. The constraints are Traits, but there is no trait for intEnum. The EnumTrait is deprecated too, so ideally we would handle enum and intEnum shapes like the other shapes in writeMemberValidator. The issue with that is, there may still be constraint traits on the shape, and for enum shapes, that will still include the @enum trait, so we'd have to like exclude it from the constraints or something. I agree though that handling intEnum like the way I did is a little funky, I'm open to other options. Both intEnum and enum shapes will trigger this guard, which would make it weird to try doing a shape.isIntEnumShape() or shape.isEnumShape() in writeMemberValidator.
This PR adds an IntegerEnumValidator, IntegerEnumValidationFailure and the relevant tests to the ssdk libs. StructuredMemberWriter was updated to use IntegerEnumValidator when generating shape validators for intEnum shapes.
Local testing:
yarn generate-clients --server-artifactsandyarn test:server-protocolsBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.