Skip to content
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

Merged
merged 5 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 18 additions & 6 deletions docs/source-2.0/spec/documentation-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,22 @@ 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
:ref:`custom trait values <trait-node-values>`.
* - lowerInputValidationSeverity
- ``[string]``
- Lowers input validation events from ERROR to WARNING for specific
validation types. List can include: ``BLOB_LENGTH_WARNING``,
``MAP_LENGTH_WARNING``, ``PATTERN_TRAIT_WARNING``,
``RANGE_TRAIT_WARNING``, ``REQUIRED_TRAIT_WARNING``,
``STRING_LENGTH_WARNING``.


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. Specific validation events for the ``input`` can be
lowered to a ``WARNING`` by setting the appropriate
``lowerInputValidationSeverity`` value. ``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
MUST NOT be defined for the same example.
Expand Down Expand Up @@ -186,7 +197,8 @@ MUST NOT be defined for the same example.
content: {
message: "Invalid 'foo'. Special character not allowed."
}
}
},
lowerInputValidationSeverity: ["PATTERN_TRAIT_WARNING"]
hpmellema marked this conversation as resolved.
Show resolved Hide resolved
}
])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,15 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.node.ToNode;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.validators.ExamplesTraitValidator;
import software.amazon.smithy.utils.BuilderRef;
import software.amazon.smithy.utils.SmithyBuilder;
import software.amazon.smithy.utils.ToSmithyBuilder;

Expand Down Expand Up @@ -55,6 +58,24 @@ protected Node createNode() {
return examples.stream().map(Example::toNode).collect(ArrayNode.collect(getSourceLocation()));
}

@Override
public boolean equals(Object other) {
if (!(other instanceof ExamplesTrait)) {
return false;
} else if (other == this) {
return true;
} else {
ExamplesTrait trait = (ExamplesTrait) other;
return this.examples.size() == trait.examples.size() && this.examples.containsAll(trait.examples);
}
}

@Override
public int hashCode() {
return Objects.hash(toShapeId(), examples);
}


@Override
public Builder toBuilder() {
Builder builder = new Builder().sourceLocation(getSourceLocation());
Expand All @@ -70,7 +91,7 @@ public static Builder builder() {
}

/**
* Builds and examples trait.
* Builds an examples trait.
*/
public static final class Builder extends AbstractTraitBuilder<ExamplesTrait, Builder> {
private final List<Example> examples = new ArrayList<>();
Expand Down Expand Up @@ -100,13 +121,15 @@ public static final class Example implements ToNode, ToSmithyBuilder<Example> {
private final ObjectNode input;
private final ObjectNode output;
private final ErrorExample error;
private final List<NodeValidationVisitor.Feature> lowerInputValidationSeverity;

private Example(Builder builder) {
this.title = Objects.requireNonNull(builder.title, "Example title must not be null");
this.documentation = builder.documentation;
this.input = builder.input;
this.output = builder.output;
this.error = builder.error;
this.lowerInputValidationSeverity = builder.lowerInputValidationSeverity.get();
}

/**
Expand Down Expand Up @@ -144,6 +167,13 @@ public Optional<ErrorExample> getError() {
return Optional.ofNullable(error);
}

/**
* @return Gets the list of lowered input validation severities.
*/
public Optional<List<NodeValidationVisitor.Feature>> getLowerInputValidationSeverity() {
return Optional.ofNullable(lowerInputValidationSeverity);
hpmellema marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public Node toNode() {
ObjectNode.Builder builder = Node.objectNodeBuilder()
Expand All @@ -157,13 +187,20 @@ public Node toNode() {
if (this.getOutput().isPresent()) {
builder.withMember("output", output);
}
if (this.getLowerInputValidationSeverity().isPresent()) {
builder.withMember("lowerInputValidationSeverity", ArrayNode.fromNodes(lowerInputValidationSeverity
.stream()
.map(NodeValidationVisitor.Feature::toNode)
.collect(Collectors.toList())));
}

return builder.build();
}

@Override
public Builder toBuilder() {
return new Builder().documentation(documentation).title(title).input(input).output(output).error(error);
return new Builder().documentation(documentation).title(title).input(input).output(output).error(error)
.lowerInputValidationSeverity(lowerInputValidationSeverity);
}

public static Builder builder() {
Expand All @@ -179,6 +216,7 @@ public static final class Builder implements SmithyBuilder<Example> {
private ObjectNode input = Node.objectNode();
private ObjectNode output;
private ErrorExample error;
private BuilderRef<List<NodeValidationVisitor.Feature>> lowerInputValidationSeverity = BuilderRef.forList();

@Override
public Example build() {
Expand Down Expand Up @@ -209,6 +247,14 @@ public Builder error(ErrorExample error) {
this.error = error;
return this;
}

public Builder lowerInputValidationSeverity(
List<NodeValidationVisitor.Feature> lowerInputValidationSeverity
) {
this.lowerInputValidationSeverity.clear();
this.lowerInputValidationSeverity.get().addAll(lowerInputValidationSeverity);
return this;
}
}
}

Expand Down Expand Up @@ -302,7 +348,9 @@ private static Example exampleFromNode(ObjectNode node) {
.getStringMember("documentation", builder::documentation)
.getObjectMember("input", builder::input)
.getObjectMember("output", builder::output)
.getMember("error", ErrorExample::fromNode, builder::error);
.getMember("error", ErrorExample::fromNode, builder::error)
.getArrayMember("lowerInputValidationSeverity", NodeValidationVisitor.Feature::fromNode,
builder::lowerInputValidationSeverity);
return builder.build();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,36 @@ 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 Length Trait validations on blobs to a WARNING.
BLOB_LENGTH_WARNING,

// Lowers severity of Length Trait validations on maps to a WARNING.
MAP_LENGTH_WARNING,

// Lowers severity of Pattern Trait validations to a WARNING.
PATTERN_TRAIT_WARNING,

// Lowers severity of Range Trait validations to a WARNING.
RANGE_TRAIT_WARNING,

// Lowers severity of Required Trait validations to a WARNING.
REQUIRED_TRAIT_WARNING,

// Lowers severity of Length Trait validations on strings to a WARNING.
STRING_LENGTH_WARNING,;

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() {
Expand Down Expand Up @@ -317,9 +343,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.REQUIRED_TRAIT_WARNING)
? 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import software.amazon.smithy.model.shapes.BlobShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.utils.SmithyInternalApi;

/**
Expand All @@ -39,16 +41,23 @@ protected void check(Shape shape, LengthTrait trait, StringNode node, Context co

trait.getMin().ifPresent(min -> {
if (size < min) {
emitter.accept(node, "Value provided for `" + shape.getId() + "` must have at least "
+ min + " bytes, but the provided value only has " + size + " bytes");
emitter.accept(node, getSeverity(context), "Value provided for `" + shape.getId()
+ "` must have at least " + min + " bytes, but the provided value only has " + size
+ " bytes");
}
});

trait.getMax().ifPresent(max -> {
if (value.getBytes(StandardCharsets.UTF_8).length > max) {
emitter.accept(node, "Value provided for `" + shape.getId() + "` must have no more than "
+ max + " bytes, but the provided value has " + size + " bytes");
emitter.accept(node, getSeverity(context), "Value provided for `" + shape.getId()
+ "` must have no more than " + max + " bytes, but the provided value has " + size
+ " bytes");
}
});
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.BLOB_LENGTH_WARNING)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import software.amazon.smithy.model.shapes.MapShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.traits.LengthTrait;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.utils.SmithyInternalApi;

/**
Expand All @@ -35,18 +37,23 @@ final class MapLengthPlugin extends MemberAndShapeTraitPlugin<MapShape, ObjectNo
protected void check(Shape shape, LengthTrait trait, ObjectNode node, Context context, Emitter emitter) {
trait.getMin().ifPresent(min -> {
if (node.size() < min) {
emitter.accept(node, String.format(
emitter.accept(node, getSeverity(context), String.format(
"Value provided for `%s` must have at least %d entries, but the provided value only "
+ "has %d entries", shape.getId(), min, node.size()));
}
});

trait.getMax().ifPresent(max -> {
if (node.size() > max) {
emitter.accept(node, String.format(
emitter.accept(node, getSeverity(context), String.format(
"Value provided for `%s` must have no more than %d entries, but the provided value "
+ "has %d entries", shape.getId(), max, node.size()));
}
});
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.MAP_LENGTH_WARNING)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StringShape;
import software.amazon.smithy.model.traits.PatternTrait;
import software.amazon.smithy.model.validation.NodeValidationVisitor;
import software.amazon.smithy.model.validation.Severity;

/**
* Validates the pattern trait on string shapes or members that target them.
Expand All @@ -32,9 +34,15 @@ final class PatternTraitPlugin extends MemberAndShapeTraitPlugin<StringShape, St
@Override
protected void check(Shape shape, PatternTrait trait, StringNode node, Context context, Emitter emitter) {
if (!trait.getPattern().matcher(node.getValue()).find()) {
emitter.accept(node, String.format(
emitter.accept(node, getSeverity(context), String.format(
"String value provided for `%s` must match regular expression: %s",
shape.getId(), trait.getPattern().pattern()));
}
}


private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.PATTERN_TRAIT_WARNING)
? Severity.WARNING : Severity.ERROR;
}
}
Loading