Skip to content

Commit

Permalink
Add support for overriding validation severity
Browse files Browse the repository at this point in the history 
severityOverride cannot set an event to SUPPRESSED. Use suppressions
for that. It cannot lower the severity of an event, only elevate. This
prevents a backdoor for disabling built-in validation or ignoring
ERROR events.

Closes #856
  • Loading branch information
@mtdowling
mtdowling committed Aug 2, 2023
1 parent d8a1282 commit a8c4203
Show file tree
Hide file tree
Showing 34 changed files with 804 additions and 339 deletions.
53 changes: 52 additions & 1 deletion docs/source-2.0/spec/model-validation.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ following properties:
- Description
* - id
- ``string``
- **Required**. The validation event ID to suppress.
- **Required**. The hierarchical validation event ID to suppress.
* - namespace
- ``string``
- **Required**. The validation event is only suppressed if it matches the
Expand Down Expand Up @@ -315,6 +315,57 @@ specific. Further, a suppression ID of "ABC" does not match an event ID of
- No


------------------
Severity overrides
------------------

The ``severityOverrides`` metadata property is used to elevate the severity
of non-suppressed validation events. This property contains an array of
severity override objects that support the following properties:

.. list-table::
:header-rows: 1
:widths: 20 20 60

* - Property
- Type
- Description
* - id
- ``string``
- **Required**. The hierarchical validation event ID to elevate.
* - namespace
- ``string``
- **Required**. The validation event is only elevated if it matches the
supplied namespace. A value of ``*`` can be provided to match any namespace.
* - severity
- ``string``
- Defines the :ref:`severity <severity-definition>` to elevate matching
events to. This value can only be set to ``WARNING`` or ``DANGER``.

The following example elevates the events of ``SomeValidator`` to ``DANGER``
in any namespace, and ``OtherValidator`` is elevated to ``WARNING`` but only
for events emitted for shapes in the ``smithy.example`` namespace:

.. code-block:: smithy
$version: "2"
metadata severityOverrides = [
{
namespace: "*"
id: "SomeValidator"
severity: "DANGER"
}
{
namespace: "smithy.example"
id: "OtherValidator"
severity: "WARNING"
}
]
namespace smithy.example
-------------------
Built-in validators
-------------------
Expand Down
10 changes: 7 additions & 3 deletions smithy-model/src/main/java/software/amazon/smithy/model/loader/LoadOperationProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import software.amazon.smithy.model.shapes.ShapeType;
import software.amazon.smithy.model.traits.TraitFactory;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidationEventDecorator;

final class LoadOperationProcessor implements Consumer<LoadOperation> {

Expand All @@ -47,22 +48,25 @@ final class LoadOperationProcessor implements Consumer<LoadOperation> {
TraitFactory traitFactory,
Model prelude,
boolean allowUnknownTraits,
Consumer<ValidationEvent> validationEventListener
Consumer<ValidationEvent> validationEventListener,
ValidationEventDecorator decorator
) {
// Emit events as the come in.
this.events = new ArrayList<ValidationEvent>() {
@Override
public boolean add(ValidationEvent e) {
e = decorator.decorate(e);
validationEventListener.accept(e);
return super.add(e);
}

@Override
public boolean addAll(Collection<? extends ValidationEvent> validationEvents) {
ensureCapacity(size() + validationEvents.size());
for (ValidationEvent e : validationEvents) {
validationEventListener.accept(e);
add(e);
}
return super.addAll(validationEvents);
return true;
}
};

Expand Down
72 changes: 22 additions & 50 deletions smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@
import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Supplier;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import software.amazon.smithy.model.Model;
import software.amazon.smithy.model.SourceException;
Expand All @@ -47,7 +45,6 @@
import software.amazon.smithy.model.shapes.ShapeId;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.traits.TraitFactory;
import software.amazon.smithy.model.validation.Severity;
import software.amazon.smithy.model.validation.ValidatedResult;
import software.amazon.smithy.model.validation.ValidationEvent;
import software.amazon.smithy.model.validation.ValidationEventDecorator;
Expand Down Expand Up @@ -487,10 +484,9 @@ public ModelAssembler disableValidation() {
* while loading and validating the model.
*
* <p>The consumer could be invoked simultaneously by multiple threads. It's
* up to the consumer to perform any necessary synchronization. Providing
* an event listener is useful for things like CLIs so that events can
* be streamed to stdout as soon as they are encountered, rather than
* waiting until the entire model is parsed and validated.
* up to the consumer to perform any necessary synchronization. If a validator
* or decorator throws, then there is no guarantee that all validation events
* are emitted to the listener.
*
* @param eventListener Listener invoked for each ValidationEvent.
* @return Returns the assembler.
Expand All @@ -510,13 +506,19 @@ public ValidatedResult<Model> assemble() {
if (traitFactory == null) {
traitFactory = LazyTraitFactoryHolder.INSTANCE;
}

if (validatorFactory == null) {
validatorFactory = ModelValidator.defaultValidationFactory();
}

// Create a singular, composed event decorator used to modify events.
ValidationEventDecorator decorator = ValidationEventDecorator.compose(validatorFactory.loadDecorators());

Model prelude = disablePrelude ? null : Prelude.getPreludeModel();

// As issues are encountered, they are decorated and then emitted.
LoadOperationProcessor processor = new LoadOperationProcessor(
traitFactory, prelude, areUnknownTraitsAllowed(), validationEventListener);
traitFactory, prelude, areUnknownTraitsAllowed(), validationEventListener, decorator);
List<ValidationEvent> events = processor.events();

// Register manually added metadata.
Expand Down Expand Up @@ -560,32 +562,24 @@ public ValidatedResult<Model> assemble() {
}

Model processedModel = processor.buildModel();
Model transformed;

// Do the 1.0 -> 2.0 transform before full-model validation.
try {
transformed = new ModelInteropTransformer(processedModel, events, processor::getShapeVersion)
.transform();
} catch (SourceException e) {
// The transformation shouldn't throw, but if it does, return here with the original model.
LOGGER.log(Level.SEVERE, "Error in ModelInteropTransformer: ", e);
events.add(ValidationEvent.fromSourceException(e));
return new ValidatedResult<>(processedModel, events);
}
Model transformed = new ModelInteropTransformer(processedModel, events, processor::getShapeVersion).transform();

// If ERROR validation events occur while loading, then performing more
// granular semantic validation will only obscure the root cause of errors.
if (LoaderUtils.containsErrorEvents(events)) {
return returnOnlyErrors(transformed, events);
}

if (disableValidation) {
List<ValidationEventDecorator> decorators = validatorFactory.loadDecorators();
return new ValidatedResult<>(transformed, ModelValidator.decorateEvents(decorators, events));
if (disableValidation || LoaderUtils.containsErrorEvents(events)) {
// All events have been emitted and decorated at this point.
return new ValidatedResult<>(transformed, events);
}

try {
return validate(transformed, events);
List<ValidationEvent> mergedEvents = ModelValidator.builder()
.validators(validators)
.validatorFactory(validatorFactory, decorator)
.eventListener(validationEventListener)
.includeEvents(events)
.build()
.validate(transformed);
return new ValidatedResult<>(transformed, mergedEvents);
} catch (SourceException e) {
events.add(ValidationEvent.fromSourceException(e));
return new ValidatedResult<>(transformed, events);
Expand All @@ -598,28 +592,6 @@ private void addMetadataToProcessor(Map<String, Node> metadataMap, LoadOperation
}
}

private ValidatedResult<Model> returnOnlyErrors(Model model, List<ValidationEvent> events) {
List<ValidationEventDecorator> decorators = validatorFactory.loadDecorators();
return new ValidatedResult<>(model, events.stream()
.filter(event -> event.getSeverity() == Severity.ERROR)
.map(event -> ModelValidator.decorateEvent(decorators, event))
.collect(Collectors.toList()));
}

private ValidatedResult<Model> validate(Model model, List<ValidationEvent> events) {
// Validate the model based on the explicit validators and model metadata.
// Note the ModelValidator handles emitting events to the validationEventListener.
List<ValidationEvent> mergedEvents = new ModelValidator()
.validators(validators)
.validatorFactory(validatorFactory)
.eventListener(validationEventListener)
.includeEvents(events)
.createValidator()
.validate(model);

return new ValidatedResult<>(model, mergedEvents);
}

private boolean areUnknownTraitsAllowed() {
Object allowUnknown = properties.get(ModelAssembler.ALLOW_UNKNOWN_TRAITS);
return allowUnknown != null && (boolean) allowUnknown;
Expand Down
Loading

0 comments on commit a8c4203

Please sign in to comment.