Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add filterSuppressions model transform #940

Merged
merged 1 commit into from
Oct 19, 2021
Merged

Conversation

mtdowling
Copy link
Member

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.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mtdowling mtdowling requested a review from a team as a code owner October 15, 2021 20:03
@mtdowling mtdowling requested a review from kstich October 19, 2021 02:49
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.
@mtdowling mtdowling dismissed JordonPhillips’s stale review October 19, 2021 22:39

Resolved the requested changes

@mtdowling mtdowling merged commit 3a6a9a6 into main Oct 19, 2021
@mtdowling mtdowling deleted the filter-suppressions branch January 7, 2022 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants