-
Notifications
You must be signed in to change notification settings - Fork 218
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
Allow constraint trait errors on error example inputs #1949
Changes from 3 commits
a378fdb
38beb19
2655235
edef8b8
e5bcfd3
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 |
---|---|---|
|
@@ -139,10 +139,16 @@ Each ``example`` trait value is a structure with the following members: | |
- :ref:`examples-ErrorExample-structure` | ||
- Provides an error shape ID and example error parameters for the | ||
operation. | ||
|
||
The values provided for the ``input`` and ``output`` members MUST be | ||
compatible with the shapes and constraints of the corresponding structure. | ||
These values use the same semantics and format as | ||
* - disableConstraints | ||
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. Shouldn't this only be allowed for errors? 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. ExamplesTraitsValidator checks that this is only set when errors are also present. Updated the docs to make this clear. |
||
- ``boolean`` | ||
- Set to true to lower input constraint trait validations to warnings. | ||
|
||
When ``input`` and ``output`` members are present, both MUST be compatible | ||
with the shapes and constraints of the corresponding structure. When ``input`` | ||
and ``error`` members are present, input validation events will be emitted as | ||
an ``ERROR`` by default. Constraint trait validation events for the ``input`` | ||
can be lowered to a ``WARNING`` by setting ``disableConstraints`` to true. | ||
``input`` and ``output`` members use the same semantics and format as | ||
:ref:`custom trait values <trait-node-values>`. | ||
|
||
A value for ``output`` or ``error`` SHOULD be provided. However, both | ||
|
@@ -186,7 +192,8 @@ MUST NOT be defined for the same example. | |
content: { | ||
message: "Invalid 'foo'. Special character not allowed." | ||
} | ||
} | ||
}, | ||
disableConstraints: true | ||
} | ||
]) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,10 +104,21 @@ public enum Feature { | |
* Emit a warning when a range trait is incompatible with a default value of 0. | ||
* | ||
* <p>This was a common pattern in Smithy 1.0 and earlier. It implies that the value is effectively | ||
* required. However, chaning the type of the value by un-boxing it or adjusting the range trait would | ||
* be a lossy tranformation when migrating a model from 1.0 to 2.0. | ||
* required. However, changing the type of the value by un-boxing it or adjusting the range trait would | ||
* be a lossy transformation when migrating a model from 1.0 to 2.0. | ||
*/ | ||
RANGE_TRAIT_ZERO_VALUE_WARNING | ||
RANGE_TRAIT_ZERO_VALUE_WARNING, | ||
|
||
// Lowers severity of constraint trait validations to WARNING. | ||
DISABLE_CONSTRAINTS; | ||
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. This name seems weird because it doesn't disable validation of constraints but lowers the severity. DISABLE_CONSTRAINTS -> IGNORE_CONSTRAINTS? 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. Updated to |
||
|
||
public static Feature fromNode(Node node) { | ||
return Feature.valueOf(node.expectStringNode().getValue()); | ||
} | ||
|
||
public static Node toNode(Feature feature) { | ||
return StringNode.from(feature.toString()); | ||
} | ||
} | ||
|
||
public static Builder builder() { | ||
|
@@ -317,9 +328,11 @@ public List<ValidationEvent> structureShape(StructureShape shape) { | |
|
||
for (MemberShape member : members.values()) { | ||
if (member.isRequired() && !object.getMember(member.getMemberName()).isPresent()) { | ||
Severity severity = this.validationContext.hasFeature(Feature.DISABLE_CONSTRAINTS) | ||
? Severity.WARNING : Severity.ERROR; | ||
events.add(event(String.format( | ||
"Missing required structure member `%s` for `%s`", | ||
member.getMemberName(), shape.getId()))); | ||
member.getMemberName(), shape.getId()), severity)); | ||
} | ||
} | ||
return events; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated for consistency, since we use
boolean
elsewhere in the docs.