From 21c4155082c541eb15d186731e595446366c0ddf Mon Sep 17 00:00:00 2001 From: Michael Dowling <mtdowling@gmail.com> Date: Fri, 15 Oct 2021 13:03:09 -0700 Subject: [PATCH] Add filterSuppressions model transform This commit adds support for filtering suppressions by removing unused suppressions, removing the reason from suppressions, removing suppressions based on an event ID allow/deny list, and removing suppressions based on a namespace allow/deny list. This model transform is useful for projecting models with internal information for external customers so that suppressions can be retained in the model without exposing internal information. To support this change, a Suppression abstraction had to be exposed in smithy-model that was previously never exposed. However, for now, the abstraction is as bare-bones as possible, and the concrete suppression types are package-private. ModelValidator was also significantly refactored to adapt to these Suppression changes, but remains package-private until we have a compelling reason to expose it outside of just using ModelAssembler for validation. smithy-build required a significant refactor to support passing in the encountered validation events of a projection to ModelTransformers via the TransformContext object. This gave the FilterSuppressions transform that ability to access ValidationEvents without needing to do another expensive round of validation. Only the validation events encountered for the model at the start of each projection, including loaded imports, are passed to transforms (that is, intermediate transforms of the model are not re-validated). However, FilterSuppressions performs a diff of the validators defined in the original model and validators defined in the projected model up to that point to determine if suppressions for any removed validators should also be removed. The implementation of the apply transform in smithy-build was also technically changed in a backward-incompatible way, though this change will likely not impact any code other than that in the smithy-build package itself. The transform still works in the same way, it was just refactored to be implemented just like any other transform. --- .../guides/building-models/build-config.rst | 205 ++++++++++ .../amazon/smithy/build/SmithyBuildImpl.java | 110 +++--- .../amazon/smithy/build/TransformContext.java | 66 +++- .../amazon/smithy/build/transforms/Apply.java | 34 +- .../build/transforms/FilterSuppressions.java | 351 ++++++++++++++++++ ....amazon.smithy.build.ProjectionTransformer | 2 + .../amazon/smithy/build/SmithyBuildTest.java | 5 +- .../transforms/FilterSuppressionsTest.java | 102 +++++ .../namespaces.detectsValidatorRemoval.json | 21 ++ .../namespaces.detectsValidatorRemoval.smithy | 7 + ...namespaces.filterByNamespaceAllowList.json | 15 + ...mespaces.filterByNamespaceAllowList.smithy | 30 ++ ...paces.filterWithProjectionImports.2.smithy | 17 + ...amespaces.filterWithProjectionImports.json | 16 + ...espaces.filterWithProjectionImports.smithy | 36 ++ ...espaces.filterWithTopLevelImports.2.smithy | 17 + .../namespaces.filterWithTopLevelImports.json | 16 + ...amespaces.filterWithTopLevelImports.smithy | 36 ++ .../namespaces.namespaceDenyList.json | 15 + .../namespaces.namespaceDenyList.smithy | 31 ++ .../namespaces.removeReasons.json | 15 + .../namespaces.removeReasons.smithy | 37 ++ .../namespaces.removeUnused.json | 15 + .../namespaces.removeUnused.smithy | 26 ++ .../filtersuppressions/namespaces.smithy | 39 ++ .../traits.eventIdAllowList.json | 15 + .../traits.eventIdAllowList.smithy | 22 ++ .../traits.eventIdDenyList.json | 15 + .../traits.eventIdDenyList.smithy | 23 ++ .../traits.removeUnused.json | 15 + .../traits.removeUnused.smithy | 21 ++ .../filtersuppressions/traits.smithy | 23 ++ .../software/amazon/smithy/model/Model.java | 5 + .../smithy/model/loader/ModelAssembler.java | 25 +- .../smithy/model/loader/ModelValidator.java | 315 +++++++++------- .../amazon/smithy/model/node/Node.java | 5 +- .../suppressions/MetadataSuppression.java | 70 ++++ .../validation/suppressions/Suppression.java | 72 ++++ .../suppressions/TraitSuppression.java | 39 ++ 39 files changed, 1664 insertions(+), 265 deletions(-) create mode 100644 smithy-build/src/main/java/software/amazon/smithy/build/transforms/FilterSuppressions.java create mode 100644 smithy-build/src/test/java/software/amazon/smithy/build/transforms/FilterSuppressionsTest.java create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.2.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.2.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.json create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.smithy create mode 100644 smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.smithy create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/MetadataSuppression.java create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/Suppression.java create mode 100644 smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/TraitSuppression.java diff --git a/docs/source/1.0/guides/building-models/build-config.rst b/docs/source/1.0/guides/building-models/build-config.rst index 06604faf84b..cb007378af2 100644 --- a/docs/source/1.0/guides/building-models/build-config.rst +++ b/docs/source/1.0/guides/building-models/build-config.rst @@ -675,6 +675,211 @@ orphaned shapes. This transformer does not remove shapes from the prelude. +.. _filterSuppressions-transform: + +filterSuppressions +------------------ + +Removes and modifies suppressions found in :ref:`metadata <suppression-definition>` +and the :ref:`suppress-trait`. + +.. list-table:: + :header-rows: 1 + :widths: 10 20 70 + + * - Property + - Type + - Description + * - removeUnused + - ``boolean`` + - Set to true to remove suppressions that have no effect. + + Shapes and validators are often removed when creating a filtered + version of model. After removing shapes and validators, suppressions + could be left in the model that no longer have any effect. These + suppressions could inadvertently disclose information about private + or unreleased features. + + If a validation event ID is never emitted, then ``@suppress`` traits + will be updated to no longer refer to the ID and removed if they no + longer refer to any event. Metadata suppressions are also removed if + they have no effect. + * - removeReasons + - ``boolean`` + - Set to true to remove the ``reason`` property from metadata suppressions. + The reason for a suppression could reveal internal or sensitive + information. Removing the "reason" from metadata suppressions is an + extra step teams can take to ensure they do not leak internal + information when publishing models outside of their organization. + * - eventIdAllowList + - ``[string]`` + - Sets a list of event IDs that can be referred to in suppressions. + Suppressions that refer to any other event ID will be updated to + no longer refer to them, or removed if they no longer refer to any + events. + + This setting cannot be used in tandem with ``eventIdDenyList``. + * - eventIdDenyList + - ``[string]`` + - Sets a list of event IDs that cannot be referred to in suppressions. + Suppressions that refer to any of these event IDs will be updated to + no longer refer to them, or removed if they no longer refer to any + events. + + This setting cannot be used in tandem with ``eventIdAllowList``. + * - namespaceAllowList + - ``[string]`` + - Sets a list of namespaces that can be referred to in metadata + suppressions. Metadata suppressions that refer to namespaces + outside of this list, including "*", will be removed. + + This setting cannot be used in tandem with ``namespaceDenyList``. + * - namespaceDenyList + - ``[string]`` + - Sets a list of namespaces that cannot be referred to in metadata + suppressions. Metadata suppressions that refer to namespaces + in this list, including "*", will be removed. + + This setting cannot be used in tandem with ``namespaceAllowList``. + +The following example removes suppressions that have no effect in the +``exampleProjection``: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } + } + +The following example removes suppressions from metadata that refer to +deny-listed namespaces: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "namespaceDenyList": ["com.internal"] + } + } + ] + } + } + } + +The following example removes suppressions from metadata that refer to +namespaces outside of the allow-listed namespaces: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "namespaceAllowList": ["com.external"] + } + } + ] + } + } + } + +The following example removes suppressions that refer to deny-listed event IDs: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "eventIdDenyList": ["MyInternalValidator"] + } + } + ] + } + } + } + +The following example removes suppressions that refer to event IDs outside +of the event ID allow list: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "eventIdAllowList": ["A", "B", "C"] + } + } + ] + } + } + } + +The following example removes the ``reason`` property from metadata +suppressions: + +.. tabs:: + + .. code-tab:: json + + { + "version": "1.0", + "projections": { + "exampleProjection": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeReasons": true + } + } + ] + } + } + } + + .. _includeTags-transform: includeTags diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java index c2f566a2a9c..caa6928dfc9 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/SmithyBuildImpl.java @@ -41,7 +41,6 @@ import software.amazon.smithy.build.model.ProjectionConfig; import software.amazon.smithy.build.model.SmithyBuildConfig; import software.amazon.smithy.build.model.TransformConfig; -import software.amazon.smithy.build.transforms.Apply; import software.amazon.smithy.model.Model; import software.amazon.smithy.model.loader.ModelAssembler; import software.amazon.smithy.model.node.ObjectNode; @@ -157,7 +156,7 @@ void applyAllProjections( Consumer<ProjectionResult> projectionResultConsumer, BiConsumer<String, Throwable> projectionExceptionConsumer ) { - Model resolvedModel = createBaseModel(); + ValidatedResult<Model> resolvedModel = createBaseModel(); // The projections are being split up here because we need to be able // to break out non-parallelizeable plugins. Right now the only @@ -209,7 +208,7 @@ void applyAllProjections( } private void executeSerialProjection( - Model resolvedModel, + ValidatedResult<Model> baseModel, String name, ProjectionConfig config, Consumer<ProjectionResult> projectionResultConsumer, @@ -220,7 +219,7 @@ private void executeSerialProjection( ProjectionResult result = null; try { - result = applyProjection(name, config, resolvedModel); + result = applyProjection(name, config, baseModel); } catch (Throwable e) { projectionExceptionConsumer.accept(name, e); } @@ -255,53 +254,60 @@ private void executeParallelProjections( } } - private Model createBaseModel() { - Model resolvedModel = model; - + private ValidatedResult<Model> createBaseModel() { if (!config.getImports().isEmpty()) { LOGGER.fine(() -> "Merging the following imports into the loaded model: " + config.getImports()); - ModelAssembler assembler = modelAssemblerSupplier.get().addModel(model); - config.getImports().forEach(assembler::addImport); - resolvedModel = assembler.assemble().unwrap(); } - return resolvedModel; + ModelAssembler assembler = modelAssemblerSupplier.get().addModel(model); + config.getImports().forEach(assembler::addImport); + return assembler.assemble(); } - private ProjectionResult applyProjection(String projectionName, ProjectionConfig projection, Model resolvedModel) { + private ProjectionResult applyProjection( + String projectionName, + ProjectionConfig projection, + ValidatedResult<Model> baseModel + ) { + Model resolvedModel = baseModel.unwrap(); LOGGER.fine(() -> String.format("Creating the `%s` projection", projectionName)); - // Resolve imports. + // Resolve imports, and overwrite baseModel. if (!projection.getImports().isEmpty()) { LOGGER.fine(() -> String.format( "Merging the following `%s` projection imports into the loaded model: %s", projectionName, projection.getImports())); ModelAssembler assembler = modelAssemblerSupplier.get().addModel(resolvedModel); projection.getImports().forEach(assembler::addImport); - ValidatedResult<Model> resolvedResult = assembler.assemble(); + baseModel = assembler.assemble(); // Fail if the model can't be merged with the imports. - if (!resolvedResult.getResult().isPresent()) { + if (!baseModel.getResult().isPresent()) { LOGGER.severe(String.format( "The model could not be merged with the following imports: [%s]", projection.getImports())); return ProjectionResult.builder() .projectionName(projectionName) - .events(resolvedResult.getValidationEvents()) + .events(baseModel.getValidationEvents()) .build(); } - resolvedModel = resolvedResult.unwrap(); + resolvedModel = baseModel.unwrap(); } // Create the base directory where all projection artifacts are stored. Path baseProjectionDir = outputDirectory.resolve(projectionName); - // Transform the model and collect the results. - Model projectedModel = applyProjectionTransforms( - resolvedModel, resolvedModel, projectionName, Collections.emptySet()); + Model projectedModel = resolvedModel; + ValidatedResult<Model> modelResult = baseModel; - ValidatedResult<Model> modelResult = modelAssemblerSupplier.get().addModel(projectedModel).assemble(); + // Don't do another round of validation and transforms if there are no transforms. + // This is the case on the source projection, for example. + if (!projection.getTransforms().isEmpty()) { + projectedModel = applyProjectionTransforms( + baseModel, resolvedModel, projectionName, Collections.emptySet()); + modelResult = modelAssemblerSupplier.get().addModel(projectedModel).assemble(); + } ProjectionResult.Builder resultBuilder = ProjectionResult.builder() .projectionName(projectionName) @@ -319,28 +325,28 @@ private ProjectionResult applyProjection(String projectionName, ProjectionConfig } private Model applyProjectionTransforms( - Model inputModel, - Model originalModel, + ValidatedResult<Model> baseModel, + Model currentModel, String projectionName, Set<String> visited ) { - // Transform the model and collect the results. - Model projectedModel = inputModel; + Model originalModel = baseModel.unwrap(); for (Pair<ObjectNode, ProjectionTransformer> transformerBinding : transformers.get(projectionName)) { TransformContext context = TransformContext.builder() - .model(projectedModel) + .model(currentModel) .originalModel(originalModel) + .originalModelValidationEvents(baseModel.getValidationEvents()) .transformer(modelTransformer) .projectionName(projectionName) .sources(sources) .settings(transformerBinding.left) - .visited(visited) .build(); - projectedModel = transformerBinding.right.transform(context); + currentModel = transformerBinding.right.transform(context); + currentModel = applyQueuedProjections(context, currentModel, visited); } - return projectedModel; + return currentModel; } private void applyPlugin( @@ -408,12 +414,6 @@ private List<Pair<ObjectNode, ProjectionTransformer>> createTransformers( for (TransformConfig transformConfig : config.getTransforms()) { String name = transformConfig.getName(); - - if (name.equals("apply")) { - resolved.add(createApplyTransformer(projectionName, transformConfig)); - continue; - } - ProjectionTransformer transformer = transformFactory.apply(name) .orElseThrow(() -> new UnknownTransformException(String.format( "Unable to find a transform named `%s` in the `%s` projection. Is this the correct " @@ -425,30 +425,30 @@ private List<Pair<ObjectNode, ProjectionTransformer>> createTransformers( return resolved; } - // This is a somewhat hacky special case that allows the "apply" transform to apply - // transformations defined on other projections. - private Pair<ObjectNode, ProjectionTransformer> createApplyTransformer( - String projectionName, - TransformConfig transformConfig - ) { - Apply.ApplyCallback callback = (currentModel, projectionTarget, visited) -> { - if (projectionTarget.equals(projectionName)) { - throw new SmithyBuildException( - "Cannot recursively apply the same projection: " + projectionName); - } else if (visited.contains(projectionTarget)) { - visited.add(projectionTarget); - throw new SmithyBuildException(String.format( - "Cycle found in apply transforms: %s -> ...", String.join(" -> ", visited))); + private Model applyQueuedProjections(TransformContext context, Model currentModel, Set<String> visited) { + for (String projectionTarget : context.getQueuedProjections()) { + Set<String> updatedVisited = new LinkedHashSet<>(visited); + if (context.getProjectionName().equals(projectionTarget)) { + throw new SmithyBuildException("Cannot recursively apply the same projection: " + projectionTarget); } else if (!transformers.containsKey(projectionTarget)) { throw new UnknownProjectionException(String.format( - "Unable to find projection named `%s` referenced by `%s`", - projectionTarget, projectionName)); + "Unable to find projection named `%s` referenced by the `%s` projection", + projectionTarget, context.getProjectionName())); + } else if (visited.contains(projectionTarget)) { + updatedVisited.add(projectionTarget); + throw new SmithyBuildException(String.format( + "Cycle found in apply transforms: %s -> ...", String.join(" -> ", updatedVisited))); } - Set<String> updatedVisited = new LinkedHashSet<>(visited); + updatedVisited.add(projectionTarget); - return applyProjectionTransforms(currentModel, model, projectionTarget, updatedVisited); - }; + currentModel = applyProjectionTransforms( + new ValidatedResult<>(context.getOriginalModel().orElse(currentModel), + context.getOriginalModelValidationEvents()), + currentModel, + projectionTarget, + updatedVisited); + } - return Pair.of(transformConfig.getArgs(), new Apply(callback)); + return currentModel; } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java b/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java index c161959855a..3218baa95db 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/TransformContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -16,8 +16,10 @@ package software.amazon.smithy.build; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; -import java.util.LinkedHashSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -25,6 +27,7 @@ import software.amazon.smithy.model.node.Node; import software.amazon.smithy.model.node.ObjectNode; import software.amazon.smithy.model.transform.ModelTransformer; +import software.amazon.smithy.model.validation.ValidationEvent; import software.amazon.smithy.utils.SetUtils; import software.amazon.smithy.utils.SmithyBuilder; import software.amazon.smithy.utils.ToSmithyBuilder; @@ -43,7 +46,8 @@ public final class TransformContext implements ToSmithyBuilder<TransformContext> private final Set<Path> sources; private final String projectionName; private final ModelTransformer transformer; - private final Set<String> visited; + private final List<String> queuedProjections = new ArrayList<>(); + private final List<ValidationEvent> originalModelValidationEvents; private TransformContext(Builder builder) { model = SmithyBuilder.requiredState("model", builder.model); @@ -52,7 +56,7 @@ private TransformContext(Builder builder) { originalModel = builder.originalModel; sources = SetUtils.copyOf(builder.sources); projectionName = builder.projectionName; - visited = new LinkedHashSet<>(builder.visited); + originalModelValidationEvents = builder.originalModelValidationEvents; } /** @@ -71,7 +75,8 @@ public Builder toBuilder() { .sources(sources) .projectionName(projectionName) .transformer(transformer) - .visited(visited); + .queuedProjections(queuedProjections) + .originalModelValidationEvents(originalModelValidationEvents); } /** @@ -138,15 +143,40 @@ public ModelTransformer getTransformer() { } /** - * Gets the set of previously visited transforms. + * Gets the queue of projections that need to be applied. * - * <p>This method is used as bookkeeping for the {@code apply} - * plugin to detect cycles. + * <p>This queue can be added to from transformers to invoke + * other projections. It's used by the apply transform, but is + * generic enough to be used by other transforms. * - * @return Returns the ordered set of visited projections. + * @return Returns the queue of projections to apply. */ - public Set<String> getVisited() { - return visited; + public Collection<String> getQueuedProjections() { + return queuedProjections; + } + + /** + * Adds a projection to the queue of projections to apply to the + * model. + * + * @param projection Projection to enqueue. + */ + public void enqueueProjection(String projection) { + if (projection.equals(getProjectionName())) { + throw new SmithyBuildException("Cannot recursively apply the same projection: " + projection); + } + + queuedProjections.add(projection); + } + + /** + * Gets an immutable list of {@link ValidationEvent}s that were + * encountered when loading the source model. + * + * @return Returns the encountered validation events. + */ + public List<ValidationEvent> getOriginalModelValidationEvents() { + return originalModelValidationEvents; } /** @@ -160,7 +190,8 @@ public static final class Builder implements SmithyBuilder<TransformContext> { private Set<Path> sources = Collections.emptySet(); private String projectionName = "source"; private ModelTransformer transformer; - private Set<String> visited = Collections.emptySet(); + private final List<ValidationEvent> originalModelValidationEvents = new ArrayList<>(); + private final List<String> queuedProjections = new ArrayList<>(); private Builder() {} @@ -199,8 +230,15 @@ public Builder transformer(ModelTransformer transformer) { return this; } - public Builder visited(Set<String> visited) { - this.visited = visited; + public Builder originalModelValidationEvents(List<ValidationEvent> originalModelValidationEvents) { + this.originalModelValidationEvents.clear(); + this.originalModelValidationEvents.addAll(originalModelValidationEvents); + return this; + } + + public Builder queuedProjections(Collection<String> queuedProjections) { + this.queuedProjections.clear(); + this.queuedProjections.addAll(queuedProjections); return this; } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/transforms/Apply.java b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/Apply.java index 74d5861f0d3..5103acb328d 100644 --- a/smithy-build/src/main/java/software/amazon/smithy/build/transforms/Apply.java +++ b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/Apply.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -16,18 +16,11 @@ package software.amazon.smithy.build.transforms; import java.util.List; -import java.util.Set; import software.amazon.smithy.build.TransformContext; import software.amazon.smithy.model.Model; /** - * Recursively applies transforms of other projections. - * - * <p>Note: this transform is special cased and not created using a - * normal factory. This is because this transformer needs to - * recursively transform models based on projections, and no other - * transform needs this functionality. We could *maybe* address - * this later if we really care that much. + * Applies transforms of other projections. */ public final class Apply extends BackwardCompatHelper<Apply.Config> { @@ -56,22 +49,6 @@ public void setProjections(List<String> projections) { } } - @FunctionalInterface - public interface ApplyCallback { - Model apply(Model inputModel, String projectionName, Set<String> visited); - } - - private final ApplyCallback applyCallback; - - /** - * Sets the function used to apply projections. - * - * @param applyCallback Takes the projection name, model, and returns the updated model. - */ - public Apply(ApplyCallback applyCallback) { - this.applyCallback = applyCallback; - } - @Override public Class<Config> getConfigType() { return Config.class; @@ -89,13 +66,10 @@ String getBackwardCompatibleNameMapping() { @Override protected Model transformWithConfig(TransformContext context, Config config) { - Model current = context.getModel(); - Set<String> visited = context.getVisited(); - for (String projection : config.getProjections()) { - current = applyCallback.apply(current, projection, visited); + context.enqueueProjection(projection); } - return current; + return context.getModel(); } } diff --git a/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FilterSuppressions.java b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FilterSuppressions.java new file mode 100644 index 00000000000..6b7f8fd4fcc --- /dev/null +++ b/smithy-build/src/main/java/software/amazon/smithy/build/transforms/FilterSuppressions.java @@ -0,0 +1,351 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.build.transforms; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import software.amazon.smithy.build.SmithyBuildException; +import software.amazon.smithy.build.TransformContext; +import software.amazon.smithy.model.Model; +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.shapes.Shape; +import software.amazon.smithy.model.traits.SuppressTrait; +import software.amazon.smithy.model.validation.Severity; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.suppressions.Suppression; + +/** + * Filters suppressions found in metadata and {@link SuppressTrait} traits. + */ +public final class FilterSuppressions extends ConfigurableProjectionTransformer<FilterSuppressions.Config> { + + private static final Logger LOGGER = Logger.getLogger(FilterSuppressions.class.getName()); + + /** + * {@code filterSuppressions} configuration settings. + */ + public static final class Config { + + private boolean removeUnused = false; + private boolean removeReasons = false; + private Set<String> eventIdAllowList = Collections.emptySet(); + private Set<String> eventIdDenyList = Collections.emptySet(); + private Set<String> namespaceAllowList = Collections.emptySet(); + private Set<String> namespaceDenyList = Collections.emptySet(); + + /** + * Gets whether unused suppressions are removed. + * + * @return Returns true if unused suppressions are removed. + */ + public boolean getRemoveUnused() { + return removeUnused; + } + + /** + * Set to true to remove suppressions that have no effect. + * + * <p>If a validation event ID is never emitted, then {@code suppress} traits + * will be updated to no longer refer to the ID and removed if they no longer + * refer to any event. Metadata suppressions are also removed if they have no + * effect. + * + * @param removeUnused Set to true to remove unused suppressions. + */ + public void setRemoveUnused(boolean removeUnused) { + this.removeUnused = removeUnused; + } + + /** + * Gets whether suppression reasons are removed. + * + * @return Returns true if suppression reasons are removed. + */ + public boolean getRemoveReasons() { + return removeReasons; + } + + /** + * Set to true to remove the {@code reason} property from metadata suppressions. + * + * <p>The reason for a suppression could reveal internal or sensitive information. + * Removing the "reason" from metadata suppressions is an extra step teams can + * take to ensure they do not leak internal information when publishing models + * outside of their organization. + * + * @param removeReasons Set to true to remove reasons. + */ + public void setRemoveReasons(boolean removeReasons) { + this.removeReasons = removeReasons; + } + + /** + * Gets a list of allowed event IDs. + * + * @return Returns the allow list of event IDs. + */ + public Set<String> getEventIdAllowList() { + return eventIdAllowList; + } + + /** + * Sets a list of event IDs that can be referred to in suppressions. + * + * <p>Suppressions that refer to any other event ID will be updated to + * no longer refer to them, or removed if they no longer refer to any events. + * + * <p>This setting cannot be used in tandem with {@code eventIdDenyList}. + * + * @param eventIdAllowList IDs to allow. + */ + public void setEventIdAllowList(Set<String> eventIdAllowList) { + this.eventIdAllowList = eventIdAllowList; + } + + /** + * Gets a list of denied event IDs. + * + * @return Gets the event ID deny list. + */ + public Set<String> getEventIdDenyList() { + return eventIdDenyList; + } + + /** + * Sets a list of event IDs that cannot be referred to in suppressions. + * + * <p>Suppressions that refer to any of these event IDs will be updated to + * no longer refer to them, or removed if they no longer refer to any events. + * + * <p>This setting cannot be used in tandem with {@code eventIdAllowList}. + * + * @param eventIdDenyList IDs to deny. + */ + public void setEventIdDenyList(Set<String> eventIdDenyList) { + this.eventIdDenyList = eventIdDenyList; + } + + /** + * Gets the metadata suppression namespace allow list. + * + * @return The metadata suppression namespace allow list. + */ + public Set<String> getNamespaceAllowList() { + return namespaceAllowList; + } + + /** + * Sets a list of namespaces that can be referred to in metadata suppressions. + * + * <p>Metadata suppressions that refer to namespaces outside of this list, + * including "*", will be removed. + * + * <p>This setting cannot be used in tandem with {@code namespaceDenyList}. + * + * @param namespaceAllowList Namespaces to allow. + */ + public void setNamespaceAllowList(Set<String> namespaceAllowList) { + this.namespaceAllowList = namespaceAllowList; + } + + /** + * Gets the metadata suppression namespace deny list. + * + * @return The metadata suppression namespace deny list. + */ + public Set<String> getNamespaceDenyList() { + return namespaceDenyList; + } + + /** + * Sets a list of namespaces that cannot be referred to in metadata suppressions. + * + * <p>Metadata suppressions that refer to namespaces in this list, + * including "*", will be removed. + * + * <p>This setting cannot be used in tandem with {@code namespaceAllowList}. + * + * @param namespaceDenyList Namespaces to deny. + */ + public void setNamespaceDenyList(Set<String> namespaceDenyList) { + this.namespaceDenyList = namespaceDenyList; + } + } + + @Override + public Class<Config> getConfigType() { + return Config.class; + } + + @Override + public String getName() { + return "filterSuppressions"; + } + + @Override + protected Model transformWithConfig(TransformContext context, Config config) { + if (!config.getEventIdAllowList().isEmpty() && !config.getEventIdDenyList().isEmpty()) { + throw new SmithyBuildException(getName() + ": cannot set both eventIdAllowList values and " + + "eventIdDenyList values at the same time"); + } + + if (!config.getNamespaceAllowList().isEmpty() && !config.getNamespaceDenyList().isEmpty()) { + throw new SmithyBuildException(getName() + ": cannot set both namespaceAllowList values and " + + "namespaceDenyList values at the same time"); + } + + Model model = context.getModel(); + Model.Builder builder = model.toBuilder(); + Set<String> removedValidators = getRemovedValidators(context, config); + List<ValidationEvent> suppressedEvents = context.getOriginalModelValidationEvents().stream() + .filter(event -> event.getSeverity() == Severity.SUPPRESSED) + .filter(event -> !removedValidators.contains(event.getId())) + .collect(Collectors.toList()); + filterSuppressionTraits(model, builder, config, suppressedEvents); + filterMetadata(model, builder, config, suppressedEvents, removedValidators); + return builder.build(); + } + + private Set<String> getRemovedValidators(TransformContext context, Config config) { + // Validators could have been removed by other transforms in this projection, + // and if they were removed, then validation events referring to them are + // no longer relevant. We don't want to keep suppressions around for validators + // that were removed. + if (!context.getOriginalModel().isPresent()) { + return Collections.emptySet(); + } + + Set<String> originalValidators = getValidatorNames(context.getOriginalModel().get()); + Set<String> updatedValidators = getValidatorNames(context.getModel()); + + if (originalValidators.equals(updatedValidators)) { + return Collections.emptySet(); + } + + originalValidators.removeAll(updatedValidators); + + if (config.getRemoveUnused()) { + LOGGER.info(() -> "Detected the removal of the following validators: " + + originalValidators + + ". Suppressions that refer to these validators will be removed."); + } + + return originalValidators; + } + + private Set<String> getValidatorNames(Model model) { + ArrayNode validators = model.getMetadata() + .getOrDefault("validators", Node.arrayNode()) + .expectArrayNode(); + Set<String> metadataSuppressions = new HashSet<>(); + for (Node validator : validators) { + ObjectNode validatorObject = validator.expectObjectNode(); + String id = validatorObject + .getStringMember("id") + .orElseGet(() -> validatorObject.expectStringMember("name")) + .getValue(); + metadataSuppressions.add(id); + } + return metadataSuppressions; + } + + private void filterSuppressionTraits( + Model model, + Model.Builder builder, + Config config, + List<ValidationEvent> suppressedEvents + ) { + // First filter and '@suppress' traits that didn't suppress anything. + for (Shape shape : model.getShapesWithTrait(SuppressTrait.class)) { + SuppressTrait trait = shape.expectTrait(SuppressTrait.class); + List<String> allowed = new ArrayList<>(trait.getValues()); + allowed.removeIf(value -> !isAllowed(value, config.getEventIdAllowList(), config.getEventIdDenyList())); + + // Only keep IDs that actually acted to suppress an event. + if (config.getRemoveUnused()) { + Set<String> matched = suppressedEvents.stream() + .filter(event -> Objects.equals(shape.getId(), event.getShapeId().orElse(null))) + .map(ValidationEvent::getId) + .collect(Collectors.toSet()); + allowed.removeIf(value -> !matched.contains(value)); + } + + if (allowed.isEmpty()) { + builder.addShape(Shape.shapeToBuilder(shape).removeTrait(SuppressTrait.ID).build()); + } else if (!allowed.equals(trait.getValues())) { + trait = trait.toBuilder().values(allowed).build(); + builder.addShape(Shape.shapeToBuilder(shape).addTrait(trait).build()); + } + } + } + + private void filterMetadata( + Model model, + Model.Builder builder, + Config config, + List<ValidationEvent> suppressedEvents, + Set<String> removedValidators + ) { + // Next remove metadata suppressions that didn't suppress anything. + ArrayNode suppressionsNode = model.getMetadata() + .getOrDefault("suppressions", Node.arrayNode()).expectArrayNode(); + builder.removeMetadataProperty("suppressions"); + List<ObjectNode> updatedMetadataSuppressions = new ArrayList<>(); + + for (Node suppressionNode : suppressionsNode) { + ObjectNode object = suppressionNode.expectObjectNode(); + String id = object.getStringMemberOrDefault("id", ""); + String namespace = object.getStringMemberOrDefault("namespace", ""); + if (config.getRemoveReasons()) { + object = object.withoutMember("reason"); + } + + // Only keep the suppression if it passes each filter. + if (isAllowed(id, config.getEventIdAllowList(), config.getEventIdDenyList()) + && isAllowed(namespace, config.getNamespaceAllowList(), config.getNamespaceDenyList())) { + if (!config.getRemoveUnused()) { + updatedMetadataSuppressions.add(object); + } else { + Suppression suppression = Suppression.fromMetadata(object); + for (ValidationEvent event : suppressedEvents) { + if (!removedValidators.contains(event.getId()) && suppression.test(event)) { + updatedMetadataSuppressions.add(object); + break; + } + } + } + } + } + + if (!updatedMetadataSuppressions.isEmpty()) { + builder.putMetadataProperty("suppressions", Node.fromNodes(updatedMetadataSuppressions)); + } + } + + private boolean isAllowed(String value, Collection<String> allowList, Collection<String> denyList) { + return (allowList.isEmpty() || allowList.contains(value)) + && (denyList.isEmpty() || !denyList.contains(value)); + } +} diff --git a/smithy-build/src/main/resources/META-INF/services/software.amazon.smithy.build.ProjectionTransformer b/smithy-build/src/main/resources/META-INF/services/software.amazon.smithy.build.ProjectionTransformer index 9203083422e..bca84cd9797 100644 --- a/smithy-build/src/main/resources/META-INF/services/software.amazon.smithy.build.ProjectionTransformer +++ b/smithy-build/src/main/resources/META-INF/services/software.amazon.smithy.build.ProjectionTransformer @@ -1,3 +1,4 @@ +software.amazon.smithy.build.transforms.Apply software.amazon.smithy.build.transforms.ExcludeMetadata software.amazon.smithy.build.transforms.ExcludeShapesByTag software.amazon.smithy.build.transforms.ExcludeShapesByTrait @@ -5,6 +6,7 @@ software.amazon.smithy.build.transforms.ExcludeTags software.amazon.smithy.build.transforms.ExcludeTraits software.amazon.smithy.build.transforms.ExcludeTraitsByTag software.amazon.smithy.build.transforms.FlattenNamespaces +software.amazon.smithy.build.transforms.FilterSuppressions software.amazon.smithy.build.transforms.IncludeMetadata software.amazon.smithy.build.transforms.IncludeNamespaces software.amazon.smithy.build.transforms.IncludeServices diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java index ee586253a81..5b1d504af85 100644 --- a/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java +++ b/smithy-build/src/test/java/software/amazon/smithy/build/SmithyBuildTest.java @@ -451,7 +451,8 @@ public void detectsMissingApplyProjection() throws Exception { new SmithyBuild().config(config).build(); }); - assertThat(thrown.getMessage(), containsString("Unable to find projection named `bar` referenced by `foo`")); + assertThat(thrown.getMessage(), + containsString("Unable to find projection named `bar` referenced by the `foo` projection")); } @Test @@ -463,7 +464,7 @@ public void detectsDirectlyRecursiveApply() throws Exception { new SmithyBuild().config(config).build(); }); - assertThat(thrown.getMessage(), containsString("Cannot recursively apply the same projection:")); + assertThat(thrown.getMessage(), containsString("Cannot recursively apply the same projection: foo")); } @Test diff --git a/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FilterSuppressionsTest.java b/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FilterSuppressionsTest.java new file mode 100644 index 00000000000..c51135029e2 --- /dev/null +++ b/smithy-build/src/test/java/software/amazon/smithy/build/transforms/FilterSuppressionsTest.java @@ -0,0 +1,102 @@ +package software.amazon.smithy.build.transforms; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.nio.file.Paths; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import software.amazon.smithy.build.MockManifest; +import software.amazon.smithy.build.ProjectionResult; +import software.amazon.smithy.build.SmithyBuild; +import software.amazon.smithy.build.SmithyBuildException; +import software.amazon.smithy.build.SmithyBuildResult; +import software.amazon.smithy.build.TransformContext; +import software.amazon.smithy.build.model.SmithyBuildConfig; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.ModelSerializer; + +public class FilterSuppressionsTest { + @Test + public void cannotSetBothEventIdAllowAndDenyList() { + SmithyBuildException thrown = Assertions.assertThrows(SmithyBuildException.class, () -> { + Model model = Model.builder().build(); + TransformContext context = TransformContext.builder() + .model(model) + .settings(Node.objectNode() + .withMember("eventIdAllowList", Node.fromStrings("a")) + .withMember("eventIdDenyList", Node.fromStrings("b"))) + .build(); + new FilterSuppressions().transform(context); + }); + + assertThat(thrown.getMessage(), containsString("cannot set both eventIdAllowList values")); + } + + @Test + public void cannotSetBothNamespaceAllowAndDenyList() { + SmithyBuildException thrown = Assertions.assertThrows(SmithyBuildException.class, () -> { + Model model = Model.builder().build(); + TransformContext context = TransformContext.builder() + .model(model) + .settings(Node.objectNode() + .withMember("namespaceAllowList", Node.fromStrings("a")) + .withMember("namespaceDenyList", Node.fromStrings("b"))) + .build(); + new FilterSuppressions().transform(context); + }); + + assertThat(thrown.getMessage(), containsString("cannot set both namespaceAllowList values")); + } + + @ParameterizedTest + @CsvSource({ + "traits,removeUnused", + "traits,eventIdAllowList", + "traits,eventIdDenyList", + "namespaces,filterByNamespaceAllowList", + "namespaces,removeReasons", + "namespaces,removeUnused", + "namespaces,namespaceDenyList", + "namespaces,filterWithTopLevelImports", + "namespaces,filterWithProjectionImports", + "namespaces,detectsValidatorRemoval" + }) + public void runTransformTests(String modelFile, String testName) throws Exception { + Model model = Model.assembler() + .addImport(getClass().getResource("filtersuppressions/" + modelFile + ".smithy")) + .assemble() + .unwrap(); + + SmithyBuild builder = new SmithyBuild() + .model(model) + .fileManifestFactory(MockManifest::new); + + SmithyBuildConfig.Builder configBuilder = SmithyBuildConfig.builder(); + configBuilder.load(Paths.get( + getClass().getResource("filtersuppressions/" + modelFile + "." + testName + ".json").toURI())); + configBuilder.outputDirectory("/mocked/is/not/used"); + builder.config(configBuilder.build()); + + SmithyBuildResult results = builder.build(); + assertTrue(results.getProjectionResult("foo").isPresent()); + + ProjectionResult projectionResult = results.getProjectionResult("foo").get(); + MockManifest manifest = (MockManifest) projectionResult.getPluginManifest("model").get(); + String modelText = manifest.getFileString("model.json").get(); + Model resultModel = Model.assembler().addUnparsedModel("/model.json", modelText).assemble().unwrap(); + Model expectedModel = Model.assembler() + .addImport(getClass().getResource("filtersuppressions/" + modelFile + "." + testName + ".smithy")) + .assemble() + .unwrap(); + + Node resultNode = ModelSerializer.builder().build().serialize(resultModel); + Node expectedNode = ModelSerializer.builder().build().serialize(expectedModel); + + Node.assertEquals(resultNode, expectedNode); + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.json new file mode 100644 index 00000000000..c1d4bfd27ea --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.json @@ -0,0 +1,21 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "excludeMetadata", + "args": { + "keys": ["validators"] + } + }, + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.smithy new file mode 100644 index 00000000000..4956dfd6592 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.detectsValidatorRemoval.smithy @@ -0,0 +1,7 @@ +$version: "1.0" + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.json new file mode 100644 index 00000000000..e503f027dd2 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "namespaceAllowList": ["smithy.example", "*"] + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.smithy new file mode 100644 index 00000000000..f7fbab2d13a --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterByNamespaceAllowList.smithy @@ -0,0 +1,30 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +metadata suppressions = [ + { + "id": "Test", + "namespace": "smithy.example", + "reason": "reason..." + }, + { + "id": "Ipsum", + "namespace": "*" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.2.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.2.smithy new file mode 100644 index 00000000000..b33d17d5065 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.2.smithy @@ -0,0 +1,17 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test2", + severity: "WARNING", + configuration: { + selector: ":is([id|name^=Foo])" + } + } +] + +namespace smithy.example + +@suppress(["Test"]) // unused +structure Foo2 {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.json new file mode 100644 index 00000000000..16bddb58164 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.json @@ -0,0 +1,16 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "imports": ["namespaces.filterWithTopLevelImports.2.smithy"], + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.smithy new file mode 100644 index 00000000000..ca7f8776e0b --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithProjectionImports.smithy @@ -0,0 +1,36 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + }, + { + name: "EmitEachSelector", + id: "Test2", + severity: "WARNING", + configuration: { + selector: ":is([id|name^=Foo])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + reason: "reason...", + namespace: "smithy.example" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} + +structure Foo2 {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.2.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.2.smithy new file mode 100644 index 00000000000..b33d17d5065 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.2.smithy @@ -0,0 +1,17 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test2", + severity: "WARNING", + configuration: { + selector: ":is([id|name^=Foo])" + } + } +] + +namespace smithy.example + +@suppress(["Test"]) // unused +structure Foo2 {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.json new file mode 100644 index 00000000000..2a38d0fb4a3 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.json @@ -0,0 +1,16 @@ +{ + "version": "1.0", + "imports": ["namespaces.filterWithProjectionImports.2.smithy"], + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.smithy new file mode 100644 index 00000000000..ca7f8776e0b --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.filterWithTopLevelImports.smithy @@ -0,0 +1,36 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + }, + { + name: "EmitEachSelector", + id: "Test2", + severity: "WARNING", + configuration: { + selector: ":is([id|name^=Foo])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + reason: "reason...", + namespace: "smithy.example" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} + +structure Foo2 {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.json new file mode 100644 index 00000000000..4f27aee6a4d --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "namespaceDenyList": ["smithy.foo", "*"] + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.smithy new file mode 100644 index 00000000000..1159cceba1b --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.namespaceDenyList.smithy @@ -0,0 +1,31 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + reason: "reason...", + namespace: "smithy.example" + }, + { + id: "Test", + namespace: "smithy.example.nested", + reason: "reason..." + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.json new file mode 100644 index 00000000000..a014324a611 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeReasons": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.smithy new file mode 100644 index 00000000000..576d19b7b2a --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeReasons.smithy @@ -0,0 +1,37 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + namespace: "smithy.example" + }, + { + id: "Lorem", + namespace: "smithy.foo" + }, + { + id: "Test", + namespace: "smithy.example.nested" + }, + { + id: "Ipsum", + namespace: "*" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.json new file mode 100644 index 00000000000..5c6f2aa7b32 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.smithy new file mode 100644 index 00000000000..e180c2981f5 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.removeUnused.smithy @@ -0,0 +1,26 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + reason: "reason...", + namespace: "smithy.example" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.smithy new file mode 100644 index 00000000000..422bcf8d72e --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/namespaces.smithy @@ -0,0 +1,39 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +metadata suppressions = [ + { + id: "Test", + reason: "reason...", + namespace: "smithy.example" + }, + { + id: "Lorem", + namespace: "smithy.foo" + }, + { + id: "Test", + namespace: "smithy.example.nested", + reason: "reason..." + }, + { + id: "Ipsum", + namespace: "*" + } +] + +namespace smithy.example + +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.json new file mode 100644 index 00000000000..27c0b38a8a5 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "eventIdAllowList": ["Test"] + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.smithy new file mode 100644 index 00000000000..7dfb28ea20f --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdAllowList.smithy @@ -0,0 +1,22 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +namespace smithy.example + +@suppress(["Test"]) +string NoMatches + +@suppress(["Test"]) +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.json new file mode 100644 index 00000000000..d4e9578dc82 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "eventIdDenyList": ["Test"] + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.smithy new file mode 100644 index 00000000000..8db256c6184 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.eventIdDenyList.smithy @@ -0,0 +1,23 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +namespace smithy.example + +@suppress(["NeverUsed"]) +string NoMatches + +@suppress(["NeverUsed"]) +structure Foo {} + +@suppress(["NeverUsed"]) +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.json b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.json new file mode 100644 index 00000000000..5c6f2aa7b32 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.json @@ -0,0 +1,15 @@ +{ + "version": "1.0", + "projections": { + "foo": { + "transforms": [ + { + "name": "filterSuppressions", + "args": { + "removeUnused": true + } + } + ] + } + } +} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.smithy new file mode 100644 index 00000000000..698fed11b51 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.removeUnused.smithy @@ -0,0 +1,21 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +namespace smithy.example + +string NoMatches + +@suppress(["Test"]) +structure Foo {} + +structure Baz {} diff --git a/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.smithy b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.smithy new file mode 100644 index 00000000000..18b4d5b2f25 --- /dev/null +++ b/smithy-build/src/test/resources/software/amazon/smithy/build/transforms/filtersuppressions/traits.smithy @@ -0,0 +1,23 @@ +$version: "1.0" + +metadata validators = [ + { + name: "EmitEachSelector", + id: "Test", + severity: "WARNING", + configuration: { + selector: ":is([id=smithy.example#Foo], [id=smithy.example#Baz])" + } + } +] + +namespace smithy.example + +@suppress(["NeverUsed", "Test"]) +string NoMatches + +@suppress(["NeverUsed", "Test"]) +structure Foo {} + +@suppress(["NeverUsed"]) +structure Baz {} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/Model.java b/smithy-model/src/main/java/software/amazon/smithy/model/Model.java index 9dd82670400..dec83d16f7a 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/Model.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/Model.java @@ -862,6 +862,11 @@ public Builder putMetadataProperty(String key, Node value) { return this; } + public Builder removeMetadataProperty(String key) { + metadata.remove(key); + return this; + } + public Builder clearMetadata() { metadata.clear(); return this; diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java index 0c3f526710e..149b4b9bde2 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelAssembler.java @@ -104,12 +104,6 @@ public final class ModelAssembler { private boolean disablePrelude; private Consumer<ValidationEvent> validationEventListener = DEFAULT_EVENT_LISTENER; - // Lazy initialization holder class idiom to hold a default validator factory. - private static final class LazyValidatorFactoryHolder { - static final ValidatorFactory INSTANCE = ValidatorFactory.createServiceFactory( - ModelAssembler.class.getClassLoader()); - } - // Lazy initialization holder class idiom to hold a default trait factory. static final class LazyTraitFactoryHolder { static final TraitFactory INSTANCE = TraitFactory.createServiceFactory(ModelAssembler.class.getClassLoader()); @@ -634,14 +628,14 @@ private ValidatedResult<Model> validate(Model model, TraitContainer traits, List return new ValidatedResult<>(model, events); } - if (validatorFactory == null) { - validatorFactory = LazyValidatorFactoryHolder.INSTANCE; - } - // Validate the model based on the explicit validators and model metadata. // Note the ModelValidator handles emitting events to the validationEventListener. - List<ValidationEvent> mergedEvents = ModelValidator - .validate(model, validatorFactory, assembleValidators(), validationEventListener); + List<ValidationEvent> mergedEvents = new ModelValidator() + .validators(validators) + .validatorFactory(validatorFactory) + .eventListener(validationEventListener) + .createValidator() + .validate(model); mergedEvents.addAll(events); return new ValidatedResult<>(model, mergedEvents); @@ -682,11 +676,4 @@ private boolean areUnknownTraitsAllowed() { Object allowUnknown = properties.get(ModelAssembler.ALLOW_UNKNOWN_TRAITS); return allowUnknown != null && (boolean) allowUnknown; } - - private List<Validator> assembleValidators() { - // Find and register built-in validators with the validator. - List<Validator> copiedValidators = new ArrayList<>(validatorFactory.loadBuiltinValidators()); - copiedValidators.addAll(validators); - return copiedValidators; - } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java index ecfac469a6e..819fd7ef2a6 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/ModelValidator.java @@ -1,5 +1,5 @@ /* - * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"). * You may not use this file except in compliance with the License. @@ -18,9 +18,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; @@ -28,24 +27,20 @@ import software.amazon.smithy.model.Model; import software.amazon.smithy.model.SourceLocation; import software.amazon.smithy.model.node.ObjectNode; -import software.amazon.smithy.model.shapes.Shape; import software.amazon.smithy.model.traits.SuppressTrait; 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.Validator; import software.amazon.smithy.model.validation.ValidatorFactory; +import software.amazon.smithy.model.validation.suppressions.Suppression; 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 - * traits. - * - * <p>ModelValidator is used to apply validation to a loaded Model. This - * class is used in tandem with {@link ModelAssembler}. + * traits and metadata. * * <p>Validators found in metadata and suppressions found in traits are * automatically created and applied to the model. Explicitly provided @@ -55,12 +50,12 @@ final class ModelValidator { private static final String SUPPRESSIONS = "suppressions"; - private static final String ID = "id"; - private static final String NAMESPACE = "namespace"; - private static final String REASON = "reason"; - private static final String STAR = "*"; - private static final String EMPTY_REASON = ""; - private static final Collection<String> SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON); + + // Lazy initialization holder class idiom to hold a default validator factory. + private static final class LazyValidatorFactoryHolder { + static final ValidatorFactory INSTANCE = ValidatorFactory.createServiceFactory( + ModelAssembler.class.getClassLoader()); + } /** If these validators fail, then many others will too. Validate these first. */ private static final Set<Class<? extends Validator>> CORE_VALIDATORS = SetUtils.of( @@ -68,75 +63,141 @@ final class ModelValidator { ResourceCycleValidator.class ); - private final List<Validator> validators; - private final ArrayList<ValidationEvent> events = new ArrayList<>(); - private final ValidatorFactory validatorFactory; - private final Model model; - private final Map<String, Map<String, String>> namespaceSuppressions = new HashMap<>(); - private final Consumer<ValidationEvent> eventListener; + private final List<Validator> validators = new ArrayList<>(); + private final List<Suppression> suppressions = new ArrayList<>(); + private ValidatorFactory validatorFactory; + private Consumer<ValidationEvent> eventListener; - private ModelValidator( - Model model, - ValidatorFactory validatorFactory, - List<Validator> validators, - Consumer<ValidationEvent> eventListener - ) { - this.model = model; + /** + * Sets the custom {@link Validator}s to use when running the ModelValidator. + * + * @param validators Validators to set. + * @return Returns the ModelValidator. + */ + public ModelValidator validators(Collection<? extends Validator> validators) { + this.validators.clear(); + validators.forEach(this::addValidator); + return this; + } + + /** + * Adds a custom {@link Validator} to the ModelValidator. + * + * @param validator Validator to add. + * @return Returns the ModelValidator. + */ + public ModelValidator addValidator(Validator validator) { + validators.add(Objects.requireNonNull(validator)); + return this; + } + + /** + * Sets the {@link Suppression}s to use with the validator. + * + * @param suppressions Suppressions to set. + * @return Returns the ModelValidator. + */ + public ModelValidator suppressions(Collection<? extends Suppression> suppressions) { + this.suppressions.clear(); + suppressions.forEach(this::addSuppression); + return this; + } + + /** + * Adds a custom {@link Suppression} to the validator. + * + * @param suppression Suppression to add. + * @return Returns the ModelValidator. + */ + public ModelValidator addSuppression(Suppression suppression) { + suppressions.add(Objects.requireNonNull(suppression)); + return this; + } + + /** + * Sets the factory used to find built-in {@link Validator}s and to load + * validators found in model metadata. + * + * @param validatorFactory Factory to use to load {@code Validator}s. + * + * @return Returns the ModelValidator. + */ + public ModelValidator validatorFactory(ValidatorFactory validatorFactory) { this.validatorFactory = validatorFactory; - this.validators = new ArrayList<>(validators); - this.validators.removeIf(v -> CORE_VALIDATORS.contains(v.getClass())); - this.eventListener = eventListener; + return this; } /** - * Validates the given Model using validators configured explicitly and - * detected through metadata. + * Sets a custom event listener that receives each {@link ValidationEvent} + * as it is emitted. * - * @param model Model to validate. - * @param validatorFactory Factory used to find ValidatorService providers. - * @param validators Additional validators to use. - * @param eventListener Consumer invoked each time a validation event is encountered. - * @return Returns the encountered validation events. + * @param eventListener Event listener that consumes each event. + * @return Returns the ModelValidator. */ - static List<ValidationEvent> validate( - Model model, - ValidatorFactory validatorFactory, - List<Validator> validators, - Consumer<ValidationEvent> eventListener - ) { - return new ModelValidator(model, validatorFactory, validators, eventListener).doValidate(); + public ModelValidator eventListener(Consumer<ValidationEvent> eventListener) { + this.eventListener = eventListener; + return this; } - private List<ValidationEvent> doValidate() { - assembleNamespaceSuppressions(); - 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)); - // Emit any events that have already occurred. - events.forEach(eventListener); - - if (LoaderUtils.containsErrorEvents(events)) { - return events; + /** + * Creates a reusable Model Validator that uses every registered validator, + * suppression, and extracts validators and suppressions from each + * provided model. + * + * @return Returns the created {@link Validator}. + */ + public Validator createValidator() { + if (validatorFactory == null) { + validatorFactory = LazyValidatorFactoryHolder.INSTANCE; } - List<ValidationEvent> result = validators - .parallelStream() - .flatMap(validator -> validator.validate(model).stream()) - .map(this::suppressEvent) - .filter(ModelValidator::filterPrelude) - // Emit events as they occur during validation. - .peek(eventListener) - .collect(Collectors.toList()); + List<Validator> staticValidators = resolveStaticValidators(); + + return model -> { + List<ValidationEvent> coreEvents = new ArrayList<>(); + + // Add suppressions found in the model via metadata. + List<Suppression> modelSuppressions = new ArrayList<>(suppressions); + loadModelSuppressions(modelSuppressions, model); + + // Add validators defined in the model through metadata. + List<Validator> modelValidators = new ArrayList<>(staticValidators); + loadModelValidators(validatorFactory, modelValidators, model, coreEvents, modelSuppressions); + + // 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. + coreEvents.addAll(new TargetValidator().validate(model)); + coreEvents.addAll(new ResourceCycleValidator().validate(model)); + // Emit any events that have already occurred. + coreEvents.forEach(eventListener); - // Add in events encountered while building up validators and suppressions. - result.addAll(events); + if (LoaderUtils.containsErrorEvents(coreEvents)) { + return coreEvents; + } + + List<ValidationEvent> result = modelValidators + .parallelStream() + .flatMap(validator -> validator.validate(model).stream()) + .filter(ModelValidator::filterPrelude) + .map(event -> suppressEvent(model, event, modelSuppressions)) + // Emit events as they occur during validation. + .peek(eventListener) + .collect(Collectors.toList()); + + // Add in events encountered while building up validators and suppressions. + result.addAll(coreEvents); + + return result; + }; + } - return result; + private List<Validator> resolveStaticValidators() { + List<Validator> resolvedValidators = new ArrayList<>(validatorFactory.loadBuiltinValidators()); + resolvedValidators.addAll(validators); + // These core validators are applied first, so don't run them again. + resolvedValidators.removeIf(v -> CORE_VALIDATORS.contains(v.getClass())); + return resolvedValidators; } private static boolean filterPrelude(ValidationEvent event) { @@ -149,25 +210,17 @@ private static boolean filterPrelude(ValidationEvent event) { .isPresent(); } - /** - * Load validator definitions, aggregating any errors along the way. - * - * @return Returns the loaded validator definitions. - */ - private List<ValidatorDefinition> assembleValidatorDefinitions() { - ValidatedResult<List<ValidatorDefinition>> result = ValidationLoader.loadValidators(model.getMetadata()); - events.addAll(result.getValidationEvents()); - return result.getResult().orElseGet(Collections::emptyList); - } - - /** - * Loads validators from model metadata, combines with explicit - * validators, and aggregates errors. - * - * @param definitions List of validator definitions to resolve - * using the validator factory. - */ - private void assembleValidators(List<ValidatorDefinition> definitions) { + private static void loadModelValidators( + ValidatorFactory validatorFactory, + List<Validator> validators, + Model model, + List<ValidationEvent> events, + List<Suppression> suppressions + ) { + // Load validators defined in metadata. + ValidatedResult<List<ValidatorDefinition>> loaded = ValidationLoader.loadValidators(model.getMetadata()); + events.addAll(loaded.getValidationEvents()); + List<ValidatorDefinition> definitions = loaded.getResult().orElseGet(Collections::emptyList); ValidatorFromDefinitionFactory factory = new ValidatorFromDefinitionFactory(validatorFactory); // Attempt to create the Validator instances and collect errors along the way. @@ -176,13 +229,14 @@ private void assembleValidators(List<ValidatorDefinition> definitions) { result.getResult().ifPresent(validators::add); events.addAll(result.getValidationEvents()); if (result.getValidationEvents().isEmpty() && !result.getResult().isPresent()) { - events.add(suppressEvent(unknownValidatorError(val.name, val.sourceLocation))); + ValidationEvent event = unknownValidatorError(val.name, val.sourceLocation); + events.add(suppressEvent(model, event, suppressions)); } } } // Unknown validators don't fail the build! - private ValidationEvent unknownValidatorError(String name, SourceLocation location) { + private static ValidationEvent unknownValidatorError(String name, SourceLocation location) { return ValidationEvent.builder() // Per the spec, the eventID is "UnknownValidator_<validatorName>". .id("UnknownValidator_" + name) @@ -192,81 +246,56 @@ private ValidationEvent unknownValidatorError(String name, SourceLocation locati .build(); } - // Find all namespace suppressions. - private void assembleNamespaceSuppressions() { + private static void loadModelSuppressions(List<Suppression> suppressions, Model model) { model.getMetadataProperty(SUPPRESSIONS).ifPresent(value -> { List<ObjectNode> values = value.expectArrayNode().getElementsAs(ObjectNode.class); for (ObjectNode rule : values) { - rule.warnIfAdditionalProperties(SUPPRESSION_KEYS); - String id = rule.expectStringMember(ID).getValue(); - String namespace = rule.expectStringMember(NAMESPACE).getValue(); - String reason = rule.getStringMemberOrDefault(REASON, EMPTY_REASON); - namespaceSuppressions.computeIfAbsent(id, i -> new HashMap<>()).put(namespace, reason); + suppressions.add(Suppression.fromMetadata(rule)); } }); } - private ValidationEvent suppressEvent(ValidationEvent event) { + private static ValidationEvent suppressEvent(Model model, ValidationEvent event, List<Suppression> suppressions) { // ERROR and SUPPRESSED events cannot be suppressed. if (!event.getSeverity().canSuppress()) { return event; } - String reason = resolveReason(event); + Suppression matchedSuppression = findMatchingSuppression(model, event, suppressions); - // The event is not suppressed, return as-is. - if (reason == null) { + if (matchedSuppression == null) { return event; } // The event was suppressed so change the severity and reason. ValidationEvent.Builder builder = event.toBuilder(); builder.severity(Severity.SUPPRESSED); - if (!reason.equals(EMPTY_REASON)) { - builder.suppressionReason(reason); - } + matchedSuppression.getReason().ifPresent(builder::suppressionReason); return builder.build(); } - // Get the reason as a String if it is suppressed, or null otherwise. - private String resolveReason(ValidationEvent event) { + private static Suppression findMatchingSuppression( + Model model, + ValidationEvent event, + List<Suppression> suppressions + ) { return event.getShapeId() .flatMap(model::getShape) - .flatMap(shape -> matchSuppression(shape, event.getId())) - // This is always evaluated if a reason hasn't been found. - .orElseGet(() -> matchWildcardNamespaceSuppressions(event.getId())); - } - - private Optional<String> matchSuppression(Shape shape, String eventId) { - // Traits take precedent over service suppressions. - if (shape.getTrait(SuppressTrait.class).isPresent()) { - if (shape.expectTrait(SuppressTrait.class).getValues().contains(eventId)) { - // The "" is filtered out before being passed to the - // updated ValidationEvent. - return Optional.of(EMPTY_REASON); - } - } - - // Check namespace-wide suppressions. - if (namespaceSuppressions.containsKey(eventId)) { - Map<String, String> namespaces = namespaceSuppressions.get(eventId); - if (namespaces.containsKey(shape.getId().getNamespace())) { - return Optional.of(namespaces.get(shape.getId().getNamespace())); - } - } - - return Optional.empty(); - } - - private String matchWildcardNamespaceSuppressions(String eventId) { - if (namespaceSuppressions.containsKey(eventId)) { - Map<String, String> namespaces = namespaceSuppressions.get(eventId); - if (namespaces.containsKey(STAR)) { - return namespaces.get(STAR); - } - } - - return null; + // First check for trait based suppressions. + .flatMap(shape -> shape.hasTrait(SuppressTrait.class) + ? Optional.of(Suppression.fromSuppressTrait(shape)) + : Optional.empty()) + // Try to suppress it. + .flatMap(suppression -> suppression.test(event) ? Optional.of(suppression) : Optional.empty()) + // If it wasn't suppressed, then try the rules loaded from metadata. + .orElseGet(() -> { + for (Suppression suppression : suppressions) { + if (suppression.test(event)) { + return suppression; + } + } + return null; + }); } } diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/node/Node.java b/smithy-model/src/main/java/software/amazon/smithy/model/node/Node.java index 92e2d7f43e5..1ed731f3214 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/node/Node.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/node/Node.java @@ -201,8 +201,9 @@ public static BooleanNode from(boolean value) { * @param values String values to add to the ArrayNode. * @return Returns the created ArrayNode. */ - public static ArrayNode fromNodes(List<Node> values) { - return new ArrayNode(values, SourceLocation.none()); + @SuppressWarnings("unchecked") + public static ArrayNode fromNodes(List<? extends Node> values) { + return new ArrayNode((List<Node>) values, SourceLocation.none()); } /** diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/MetadataSuppression.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/MetadataSuppression.java new file mode 100644 index 00000000000..0c0c6570196 --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/MetadataSuppression.java @@ -0,0 +1,70 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.validation.suppressions; + +import java.util.Collection; +import java.util.Optional; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.node.ObjectNode; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.utils.ListUtils; + +/** + * A suppression created from metadata. + * + * <p>"*" is used as a wildcard to suppress events in any namespace. + */ +final class MetadataSuppression implements Suppression { + + private static final String ID = "id"; + private static final String NAMESPACE = "namespace"; + private static final String REASON = "reason"; + private static final Collection<String> SUPPRESSION_KEYS = ListUtils.of(ID, NAMESPACE, REASON); + + private final String id; + private final String namespace; + private final String reason; + + MetadataSuppression(String id, String namespace, String reason) { + this.id = id; + this.namespace = namespace; + this.reason = reason; + } + + static MetadataSuppression fromNode(Node node) { + ObjectNode rule = node.expectObjectNode(); + rule.warnIfAdditionalProperties(SUPPRESSION_KEYS); + String id = rule.expectStringMember(ID).getValue(); + String namespace = rule.expectStringMember(NAMESPACE).getValue(); + String reason = rule.getStringMemberOrDefault(REASON, null); + return new MetadataSuppression(id, namespace, reason); + } + + @Override + public boolean test(ValidationEvent event) { + return event.getId().equals(id) && matchesNamespace(event); + } + + @Override + public Optional<String> getReason() { + return Optional.ofNullable(reason); + } + + private boolean matchesNamespace(ValidationEvent event) { + return namespace.equals("*") + || event.getShapeId().filter(id -> id.getNamespace().equals(namespace)).isPresent(); + } +} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/Suppression.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/Suppression.java new file mode 100644 index 00000000000..dd54aa83bcb --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/Suppression.java @@ -0,0 +1,72 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.validation.suppressions; + +import java.util.Optional; +import software.amazon.smithy.model.node.ExpectationNotMetException; +import software.amazon.smithy.model.node.Node; +import software.amazon.smithy.model.shapes.Shape; +import software.amazon.smithy.model.traits.SuppressTrait; +import software.amazon.smithy.model.validation.ValidationEvent; +import software.amazon.smithy.model.validation.Validator; + +/** + * Suppresses {@link ValidationEvent}s emitted from {@link Validator}s. + */ +@FunctionalInterface +public interface Suppression { + + /** + * Determines if the suppression applies to the given event. + * + * @param event Event to test. + * @return Returns true if the suppression applies. + */ + boolean test(ValidationEvent event); + + /** + * Gets the optional reason for the suppression. + * + * @return Returns the optional suppression reason. + */ + default Optional<String> getReason() { + return Optional.empty(); + } + + /** + * Creates a suppression using the {@link SuppressTrait} of + * the given shape. + * + * @param shape Shape to get the {@link SuppressTrait} from. + * @return Returns the created suppression. + * @throws ExpectationNotMetException if the shape has no {@link SuppressTrait}. + */ + static Suppression fromSuppressTrait(Shape shape) { + return new TraitSuppression(shape.getId(), shape.expectTrait(SuppressTrait.class)); + } + + /** + * Creates a suppression from a {@link Node} found in the + * "suppressions" metadata of a Smithy model. + * + * @param node Node to parse. + * @return Returns the loaded suppression. + * @throws ExpectationNotMetException if the suppression node is malformed. + */ + static Suppression fromMetadata(Node node) { + return MetadataSuppression.fromNode(node); + } +} diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/TraitSuppression.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/TraitSuppression.java new file mode 100644 index 00000000000..ea6e5c1beb7 --- /dev/null +++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/suppressions/TraitSuppression.java @@ -0,0 +1,39 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.smithy.model.validation.suppressions; + +import software.amazon.smithy.model.shapes.ShapeId; +import software.amazon.smithy.model.traits.SuppressTrait; +import software.amazon.smithy.model.validation.ValidationEvent; + +/** + * A suppression based on the {@link SuppressTrait}. + */ +final class TraitSuppression implements Suppression { + + private final ShapeId shape; + private final SuppressTrait trait; + + TraitSuppression(ShapeId shape, SuppressTrait trait) { + this.shape = shape; + this.trait = trait; + } + + @Override + public boolean test(ValidationEvent event) { + return event.getShapeId().filter(shape::equals).isPresent() && trait.getValues().contains(event.getId()); + } +}