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

Stop validation on critical loading errors #775

Merged
merged 1 commit into from
Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
"Id": {
"target": "smithy.api#String",
"traits": {
"aws.api#endpointDiscoveryId": {},
"aws.api#clientEndpointDiscoveryId": {},
"smithy.api#required": {}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] -: Error creating `StandardOperationVerb` validator: Either verbs or suggestAlternatives must be set when configuring StandardOperationVerb | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"smithy": "1.0",
"metadata": {
"validators": [
{
"name": "StandardOperationVerb",
"id": "Pointless"
}
]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,3 @@
[DANGER] ns.foo#DestroyFoo: Operation shape `DestroyFoo` uses a non-standard verb, `Destroy`. Expected one of the following verbs: `Create`, `Delete`, `Update` | WithVerbsAndPrefixes
[DANGER] ns.foo#MakeFoo: Operation shape `MakeFoo` uses a non-standard verb, `Make`. Consider using one of the following verbs instead: `Create`, `Generate` | SuggestAlternatives
[DANGER] ns.foo#MakeFoo: Operation shape `MakeFoo` uses a non-standard verb, `Make`. Expected one of the following verbs: `Create`, `Delete`, `Update` | WithVerbsAndPrefixes
[ERROR] -: Error creating `StandardOperationVerb` validator: Either verbs or suggestAlternatives must be set when configuring StandardOperationVerb | Model
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@
},
"metadata": {
"validators": [
{
"name": "StandardOperationVerb",
"id": "Pointless"
},
{
"name": "StandardOperationVerb",
"id": "WithVerbsAndPrefixes",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,19 @@ static boolean isSameLocation(FromSourceLocation a, FromSourceLocation b) {
SourceLocation sb = b.getSourceLocation();
return sa != SourceLocation.NONE && sa.equals(sb);
}

/**
* Checks if a list of validation events contains an ERROR severity.
*
* @param events Events to check.
* @return Returns true if an ERROR event is present.
*/
static boolean containsErrorEvents(List<ValidationEvent> events) {
for (ValidationEvent event : events) {
if (event.getSeverity() == Severity.ERROR) {
return true;
}
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,9 @@ private List<ModelFile> createModelFiles() {
private ValidatedResult<Model> validate(Model model, TraitContainer traits, List<ValidationEvent> events) {
validateTraits(model.getShapeIds(), traits, events);

if (disableValidation) {
// If ERROR validation events occur while loading, then performing more
// granular semantic validation will only obscure the root cause of errors.
if (disableValidation || LoaderUtils.containsErrorEvents(events)) {
return new ValidatedResult<>(model, events);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.SourceLocation;
Expand All @@ -33,7 +34,10 @@
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.Validator;
import software.amazon.smithy.model.validation.ValidatorFactory;
import software.amazon.smithy.model.validation.validators.ResourceCycleValidator;
import software.amazon.smithy.model.validation.validators.TargetValidator;
import software.amazon.smithy.utils.ListUtils;
import software.amazon.smithy.utils.SetUtils;

/**
* Validates a model, including validators and suppressions loaded from
Expand All @@ -57,6 +61,12 @@ final class ModelValidator {
private static final String EMPTY_REASON = "";
private static final Collection<String> SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON);

/** If these validators fail, then many others will too. Validate these first. */
private static final Set<Class<? extends Validator>> CORE_VALIDATORS = SetUtils.of(
TargetValidator.class,
ResourceCycleValidator.class
);

private final List<Validator> validators;
private final ArrayList<ValidationEvent> events = new ArrayList<>();
private final ValidatorFactory validatorFactory;
Expand All @@ -71,6 +81,7 @@ private ModelValidator(
this.model = model;
this.validatorFactory = validatorFactory;
this.validators = new ArrayList<>(validators);
this.validators.removeIf(v -> CORE_VALIDATORS.contains(v.getClass()));
}

/**
Expand All @@ -95,6 +106,15 @@ private List<ValidationEvent> doValidate() {
List<ValidatorDefinition> assembledValidatorDefinitions = assembleValidatorDefinitions();
assembleValidators(assembledValidatorDefinitions);

// Perform critical validation before other more granular semantic validators.
// If these validators fail, then many other validators will fail as well,
// which will only obscure the root cause.
events.addAll(new TargetValidator().validate(model));
events.addAll(new ResourceCycleValidator().validate(model));
if (LoaderUtils.containsErrorEvents(events)) {
return events;
}

List<ValidationEvent> result = validators
.parallelStream()
.flatMap(validator -> validator.validate(model).stream())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class PaginatedIndexTest {
public void findDirectChildren() {
ValidatedResult<Model> result = Model.assembler()
.addImport(getClass().getResource(
"/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json"))
"/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-valid.json"))
.assemble();
Model model = result.getResult().get();
PaginatedIndex index = PaginatedIndex.of(model);
Expand Down Expand Up @@ -63,7 +63,7 @@ public void findDirectChildren() {
public void findIndirectChildren() {
ValidatedResult<Model> result = Model.assembler()
.addImport(getClass().getResource(
"/software/amazon/smithy/model/errorfiles/validators/paginated-trait-test.json"))
"/software/amazon/smithy/model/errorfiles/validators/paginated/paginated-valid.json"))
.assemble();
Model model = result.getResult().get();
PaginatedIndex index = PaginatedIndex.of(model);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] ns.foo#InvalidShapeId: Error creating trait `auth`: Invalid shape ID: not a shape ID | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"smithy": "1.0",
"shapes": {
"ns.foo#InvalidShapeId": {
"type": "operation",
"traits": {
"smithy.api#auth": ["not a shape ID"]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] ns.foo#Invalid1: Error validating trait `auth`.0: Shape ID `smithy.api#String` does not match selector `[trait|authDefinition]` | TraitValue
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,6 @@
}
},
"ns.foo#Invalid1": {
"type": "operation",
"traits": {
"smithy.api#auth": ["not a shape ID"]
}
},
"ns.foo#Invalid2": {
"type": "operation",
"traits": {
"smithy.api#auth": ["smithy.api#String"]
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[ERROR] ns.foo#Invalid: Error creating trait `externalDocumentation`: Each externalDocumentation value must be a valid URL. Found "invalid!" for name "Homepage" | Model
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"smithy": "1.0",
"shapes": {
"ns.foo#Invalid": {
"type": "service",
"version": "2017-01-17",
"traits": {
"smithy.api#externalDocumentation": {
"Homepage": "invalid!"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
[ERROR] ns.foo#Invalid: Error creating trait `externalDocumentation`: Each externalDocumentation value must be a valid URL. Found "invalid!" for name "Homepage" | Model
[ERROR] ns.foo#Invalid2: Error validating trait `externalDocumentation`: Value provided for `smithy.api#externalDocumentation` must have at least 1 entries, but the provided value only has 0 entries | TraitValue
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"smithy": "1.0",
"shapes": {
"ns.foo#Invalid2": {
"type": "service",
"version": "2017-01-17",
"traits": {
"smithy.api#externalDocumentation": {}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"smithy": "1.0",
"shapes": {
"ns.foo#Valid": {
"type": "service",
"version": "2017-01-17",
"traits": {
"smithy.api#externalDocumentation": {
"Homepage": "https://www.example.com"
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near ``: Unexpected selector EOF; expression: | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near ``: Unexpected selector EOF; expression: | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "!": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `!`: Unexpected selector character: !; expression: ! | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "'foo'": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `'foo'`: Unexpected selector character: '; expression: 'foo' | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "\"foo\"": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 1, near `"foo"`: Unexpected selector character: "; expression: "foo" | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "invalid": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Unknown shape type: invalid; expression: invalid | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 2, near `]`: Expected a valid identifier character, but found ']'; expression: [] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo|]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `]`: Expected a valid identifier character, but found ']'; expression: [foo|] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[|]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 2, near `|]`: Expected a valid identifier character, but found '|'; expression: [|] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[a=]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near `]`: Expected a valid identifier character, but found ']'; expression: [a=] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[a=b": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Expected: ']', but found '[EOF]'; expression: [a=b | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "string=b": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 7, near `=b`: Unexpected selector character: =; expression: string=b | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo=']": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Expected ' to close ]; expression: [foo='] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo=\"]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 8, near ``: Expected " to close ]; expression: [foo="] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo==value]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `=value]`: Expected a valid identifier character, but found '='; expression: [foo==value] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "[foo^foo]": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `foo]`: Expected: '=', but found 'f'; expression: [foo^foo] | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(:not(string) > list": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 24, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :is(:not(string) > list | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[]->": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ` -[]->`: Unknown shape type: foo; expression: foo -[]-> | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from "foo -[input]->": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ` -[input]->`: Unknown shape type: foo; expression: foo -[input]-> | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Expected: '(', but found '[EOF]'; expression: :not | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not(": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near ``: Unexpected selector EOF; expression: :not( | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not()": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `)`: Unexpected selector character: ); expression: :not() | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":not(string": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 12, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :not(string | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 4, near ``: Expected: '(', but found '[EOF]'; expression: :is | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 5, near ``: Unexpected selector EOF; expression: :is( | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":nay()": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 6, near `)`: Unexpected selector character: ); expression: :nay() | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 11, near ``: Found '[EOF]', but expected one of the following tokens: ')' ','; expression: :is(string | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, ": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 13, near ``: Unexpected selector EOF; expression: :is(string, | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, )": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 13, near `)`: Unexpected selector character: ); expression: :is(string, ) | Model
[ERROR] -: Error creating `EmitEachSelector` validator: Deserialization error at (/selector): unable to create software.amazon.smithy.model.selector.Selector from ":is(string, :not())": Unable to deserialize Node using fromNode method: Syntax error at line 1 column 18, near `))`: Unexpected selector character: ); expression: :is(string, :not()) | Model
Loading