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

Add support for overriding validation severity #1890

Merged
merged 1 commit into from
Aug 2, 2023
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
53 changes: 52 additions & 1 deletion docs/source-2.0/spec/model-validation.rst
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
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
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) {
mtdowling marked this conversation as resolved.
Show resolved Hide resolved
// 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