Skip to content

Commit

Permalink
Combine disabling constraint validations
Browse files Browse the repository at this point in the history
  • Loading branch information
srchase committed Aug 31, 2023
1 parent 38beb19 commit 05d809a
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 108 deletions.
2 changes: 1 addition & 1 deletion docs/source-2.0/guides/building-models/build-config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ The configuration file accepts the following properties:
projection. Plugins are a mapping of :ref:`plugin IDs <plugin-id>` to
plugin-specific configuration objects.
* - ignoreMissingPlugins
- ``bool``
- ``boolean``
- If a plugin can't be found, Smithy will by default fail the build. This
setting can be set to ``true`` to allow the build to progress even if
a plugin can't be found on the classpath.
Expand Down
19 changes: 7 additions & 12 deletions docs/source-2.0/spec/documentation-traits.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,22 +139,17 @@ 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.
* - 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``.

* - disableConstraints
- ``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. 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>`.
an ``ERROR`` by default. Constraint trait validation events for the ``input``
can be lowered to a ``WARNING`` by setting the ``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
MUST NOT be defined for the same example.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@
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.BooleanNode;
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 @@ -121,15 +119,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 final boolean disableConstraints;

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();
this.disableConstraints = builder.disableConstraints;
}

/**
Expand Down Expand Up @@ -168,10 +166,10 @@ public Optional<ErrorExample> getError() {
}

/**
* @return Gets the list of lowered input validation severities.
* @return Returns true if input constraints validation has been disabled.
*/
public List<NodeValidationVisitor.Feature> getLowerInputValidationSeverity() {
return lowerInputValidationSeverity;
public boolean getDisableConstraints() {
return disableConstraints;
}

@Override
Expand All @@ -180,10 +178,7 @@ public Node toNode() {
.withMember("title", Node.from(title))
.withOptionalMember("documentation", getDocumentation().map(Node::from))
.withOptionalMember("error", getError().map(ErrorExample::toNode))
.withMember("lowerInputValidationSeverity", ArrayNode.fromNodes(lowerInputValidationSeverity
.stream()
.map(NodeValidationVisitor.Feature::toNode)
.collect(Collectors.toList())));
.withOptionalMember("disableConstraints", BooleanNode.from(disableConstraints).asBooleanNode());

if (!input.isEmpty()) {
builder.withMember("input", input);
Expand All @@ -198,7 +193,7 @@ public Node toNode() {
@Override
public Builder toBuilder() {
return new Builder().documentation(documentation).title(title).input(input).output(output).error(error)
.lowerInputValidationSeverity(lowerInputValidationSeverity);
.disableConstraints(disableConstraints);
}

public static Builder builder() {
Expand All @@ -214,7 +209,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();
private boolean disableConstraints;

@Override
public Example build() {
Expand Down Expand Up @@ -246,11 +241,8 @@ public Builder error(ErrorExample error) {
return this;
}

public Builder lowerInputValidationSeverity(
List<NodeValidationVisitor.Feature> lowerInputValidationSeverity
) {
this.lowerInputValidationSeverity.clear();
this.lowerInputValidationSeverity.get().addAll(lowerInputValidationSeverity);
public Builder disableConstraints(Boolean disableConstraints) {
this.disableConstraints = disableConstraints;
return this;
}
}
Expand Down Expand Up @@ -347,8 +339,7 @@ private static Example exampleFromNode(ObjectNode node) {
.getObjectMember("input", builder::input)
.getObjectMember("output", builder::output)
.getMember("error", ErrorExample::fromNode, builder::error)
.getArrayMember("lowerInputValidationSeverity", NodeValidationVisitor.Feature::fromNode,
builder::lowerInputValidationSeverity);
.getBooleanMember("disableConstraints", builder::disableConstraints);
return builder.build();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,23 +109,8 @@ public enum Feature {
*/
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,;
// Lowers severity of constraint trait validations to WARNING.
DISABLE_CONSTRAINTS;

public static Feature fromNode(Node node) {
return Feature.valueOf(node.expectStringNode().getValue());
Expand Down Expand Up @@ -343,7 +328,7 @@ 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 severity = this.validationContext.hasFeature(Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
events.add(event(String.format(
"Missing required structure member `%s` for `%s`",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected void check(Shape shape, LengthTrait trait, StringNode node, Context co
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.BLOB_LENGTH_WARNING)
return context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected void check(Shape shape, LengthTrait trait, ObjectNode node, Context co
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.MAP_LENGTH_WARNING)
return context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ protected void check(Shape shape, PatternTrait trait, StringNode node, Context c


private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.PATTERN_TRAIT_WARNING)
return context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ protected void check(Shape shape, Context context, RangeTrait trait, NumberNode
private Severity getSeverity(NumberNode node, Context context) {
boolean zeroValueWarning = context
.hasFeature(NodeValidationVisitor.Feature.RANGE_TRAIT_ZERO_VALUE_WARNING);
boolean rangeTraitWarning = context.hasFeature(NodeValidationVisitor.Feature.RANGE_TRAIT_WARNING);
boolean rangeTraitWarning = context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS);
return (zeroValueWarning && node.isZero()) || rangeTraitWarning ? Severity.WARNING : Severity.ERROR;
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.RANGE_TRAIT_WARNING)
return context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ protected void check(Shape shape, LengthTrait trait, StringNode node, Context co
}

private Severity getSeverity(Context context) {
return context.hasFeature(NodeValidationVisitor.Feature.STRING_LENGTH_WARNING)
return context.hasFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS)
? Severity.WARNING : Severity.ERROR;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,9 @@

package software.amazon.smithy.model.validation.validators;

import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.BLOB_LENGTH_WARNING;
import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.MAP_LENGTH_WARNING;
import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.PATTERN_TRAIT_WARNING;
import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.RANGE_TRAIT_WARNING;
import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.REQUIRED_TRAIT_WARNING;
import static software.amazon.smithy.model.validation.NodeValidationVisitor.Feature.STRING_LENGTH_WARNING;

import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.node.ObjectNode;
import software.amazon.smithy.model.shapes.OperationShape;
Expand All @@ -41,14 +32,6 @@
*/
public final class ExamplesTraitValidator extends AbstractValidator {

private static final Set<NodeValidationVisitor.Feature> ALLOWED_FEATURES = EnumSet.of(
BLOB_LENGTH_WARNING,
MAP_LENGTH_WARNING,
PATTERN_TRAIT_WARNING,
RANGE_TRAIT_WARNING,
REQUIRED_TRAIT_WARNING,
STRING_LENGTH_WARNING);

@Override
public List<ValidationEvent> validate(Model model) {
List<ValidationEvent> events = new ArrayList<>();
Expand All @@ -69,16 +52,12 @@ private List<ValidationEvent> validateExamples(Model model, OperationShape shape

model.getShape(shape.getInputShape()).ifPresent(input -> {
NodeValidationVisitor validator;
if (!example.getLowerInputValidationSeverity().isEmpty()) {
if (!isErrorDefined) {
events.add(error(shape, trait, String.format(
"Example: `%s` has lowerInputValidationSeverity defined, so error must also be defined.",
if (example.getDisableConstraints() && !isErrorDefined) {
events.add(error(shape, trait, String.format(
"Example: `%s` has disableConstraints enabled, so error must be defined.",
example.getTitle())));
}
validator = createVisitor("input", example.getInput(), model, shape, example, true);
} else {
validator = createVisitor("input", example.getInput(), model, shape, example, false);
}
validator = createVisitor("input", example.getInput(), model, shape, example);
List<ValidationEvent> inputValidationEvents = input.accept(validator);
events.addAll(inputValidationEvents);
});
Expand All @@ -90,15 +69,15 @@ private List<ValidationEvent> validateExamples(Model model, OperationShape shape
} else if (isOutputDefined) {
model.getShape(shape.getOutputShape()).ifPresent(output -> {
NodeValidationVisitor validator = createVisitor(
"output", example.getOutput().get(), model, shape, example, false);
"output", example.getOutput().get(), model, shape, example);
events.addAll(output.accept(validator));
});
} else if (isErrorDefined) {
ExamplesTrait.ErrorExample errorExample = example.getError().get();
Optional<Shape> errorShape = model.getShape(errorExample.getShapeId());
if (errorShape.isPresent() && shape.getErrors().contains(errorExample.getShapeId())) {
NodeValidationVisitor validator = createVisitor(
"error", errorExample.getContent(), model, shape, example, false);
"error", errorExample.getContent(), model, shape, example);
events.addAll(errorShape.get().accept(validator));
} else {
events.add(error(shape, trait, String.format(
Expand All @@ -116,19 +95,16 @@ private NodeValidationVisitor createVisitor(
ObjectNode value,
Model model,
Shape shape,
ExamplesTrait.Example example,
boolean enableFeatures
ExamplesTrait.Example example
) {
NodeValidationVisitor.Builder builder = NodeValidationVisitor.builder()
.model(model)
.eventShapeId(shape.getId())
.value(value)
.startingContext("Example " + name + " of `" + example.getTitle() + "`")
.eventId(getName());
if (enableFeatures) {
example.getLowerInputValidationSeverity().stream()
.filter(ALLOWED_FEATURES::contains)
.forEach(builder::addFeature);
if (example.getDisableConstraints()) {
builder.addFeature(NodeValidationVisitor.Feature.DISABLE_CONSTRAINTS);
}
return builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ structure Example {

error: ExampleError

lowerInputValidationSeverity: NonEmptyStringList
disableConstraints: Boolean
}

@private
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,11 @@
import software.amazon.smithy.model.node.ArrayNode;
import software.amazon.smithy.model.node.Node;
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.validation.NodeValidationVisitor;

public class ExamplesTraitTest {
@Test
public void loadsTrait() {
TraitFactory provider = TraitFactory.createServiceFactory();
ArrayNode lowerInputValidationSeverity = Node.fromStrings(
NodeValidationVisitor.Feature.REQUIRED_TRAIT_WARNING.name() );
ArrayNode node = Node.arrayNode(
Node.objectNode()
.withMember("title", Node.from("foo")),
Expand All @@ -44,7 +41,7 @@ public void loadsTrait() {
.withMember("error", Node.objectNode()
.withMember(Node.from("shapeId"), Node.from("smithy.example#FooError"))
.withMember(Node.from("content"), Node.objectNode().withMember("e", Node.from("f"))))
.withMember("lowerInputValidationSeverity", lowerInputValidationSeverity));
.withMember("disableConstraints", Node.from(true)));

Optional<Trait> trait = provider.createTrait(
ShapeId.from("smithy.api#examples"), ShapeId.from("ns.qux#foo"), node);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[WARNING] ns.foo#Operation: Example output of `Testing 2`: Invalid structure member `additional` found for `ns.foo#OperationOutput` | ExamplesTrait
[ERROR] ns.foo#Operation: Example output of `Testing 2`: Missing required structure member `bam` for `ns.foo#OperationOutput` | ExamplesTrait
[WARNING] ns.foo#Operation: Example error of `Testing 1`: Invalid structure member `extra` found for `ns.foo#OperationError` | ExamplesTrait
[ERROR] ns.foo#Operation: Example: `Testing 4` has lowerInputValidationSeverity defined, so error must also be defined. | ExamplesTrait
[ERROR] ns.foo#Operation: Example: `Testing 4` has disableConstraints enabled, so error must be defined. | ExamplesTrait
[WARNING] ns.foo#Operation: Example input of `Testing 5`.blobMin: Value provided for `ns.foo#OperationInput$blobMin` must have at least 3 bytes, but the provided value only has 1 bytes | ExamplesTrait
[WARNING] ns.foo#Operation: Example input of `Testing 5`.blobMax: Value provided for `ns.foo#OperationInput$blobMax` must have no more than 3 bytes, but the provided value has 6 bytes | ExamplesTrait
[WARNING] ns.foo#Operation: Example input of `Testing 5`: Missing required structure member `foo` for `ns.foo#OperationInput` | ExamplesTrait
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@
"output": {
"bam": "value3"
},
"lowerInputValidationSeverity": [
"REQUIRED_TRAIT_WARNING"
]
"disableConstraints": true
},
{
"title": "Testing 5",
Expand Down Expand Up @@ -93,14 +91,7 @@
"bat": "baz"
}
},
"lowerInputValidationSeverity": [
"BLOB_LENGTH_WARNING",
"MAP_LENGTH_WARNING",
"PATTERN_TRAIT_WARNING",
"RANGE_TRAIT_WARNING",
"REQUIRED_TRAIT_WARNING",
"STRING_LENGTH_WARNING"
]
"disableConstraints": true
}
]
}
Expand Down

0 comments on commit 05d809a

Please sign in to comment.