Add Unit type and @input and @output traits #980
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 13, 2021
JordonPhillips
requested changes
Nov 16, 2021
smithy-model/src/main/java/software/amazon/smithy/model/traits/InputTrait.java
Show resolved
Hide resolved
mtdowling
force-pushed
the
refactor-operation-io-and-unit-types
branch
from
f0e7dde to
019a61f
Compare
mtdowling
force-pushed
the
refactor-operation-io-and-unit-types
branch
from
019a61f to
e771467
Compare
JordonPhillips
approved these changes
Nov 17, 2021
mtdowling
force-pushed
the
refactor-operation-io-and-unit-types
branch
from
98cad07 to
403ed0b
Compare
JordonPhillips
approved these changes
Nov 23, 2021
kstich
requested changes
Dec 2, 2021
smithy-aws-protocol-tests/model/restJson1/empty-input-output.smithy
Outdated
Show resolved
Hide resolved
smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/AddedOperationInputOutput.java
Outdated
Show resolved
Hide resolved
smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/ChangedOperationInput.java
Outdated
Show resolved
Hide resolved
smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/RemovedOperationInput.java
Outdated
Show resolved
Hide resolved
smithy-diff/src/main/java/software/amazon/smithy/diff/evaluators/RemovedOperationOutput.java
Outdated
Show resolved
Hide resolved
smithy-diff/src/main/resources/META-INF/services/software.amazon.smithy.diff.DiffEvaluator
Show resolved
Hide resolved
...rc/main/java/software/amazon/smithy/model/validation/validators/HttpLabelTraitValidator.java
Outdated
Show resolved
Hide resolved
smithy-mqtt-traits/src/main/java/software/amazon/smithy/mqtt/traits/TopicBinding.java
Outdated
Show resolved
Hide resolved
...del/src/main/java/software/amazon/smithy/model/validation/validators/OperationValidator.java
Outdated
Show resolved
Hide resolved
This commit introduces the `smithy.api#Unit` type that that is used for operations with no meaningful input or output, and for tagged union members with no meaningful value. Operations will now, by default, target `smithy.api#Union` if they do not define input or output. This commit also introduces the `@input` and `@output` traits that are used to specialize structures as for use only as the input or output of a single operation. The `@input` trait provides structures with more relaxed backward compatibility constraints so that, for example, when things like the `@default` value trait are added, it is considered a backward compatible change to remove the `@required` trait from the member of a structure marked with the `@input` trait. New methods were added to smithy-model to account for the fact that operations always now have input and output shapes. The existing methods that return Optional shape IDs and structures remain but are marked as deprecated. Any usage of these deprecated methods were updated throughout the monorepo to use the newly introduced methods (this accounts for a lot of the churn). If an operation defines no input or output, a WARNING is emitted. If an operation defines input or output other than `smithy.api#Unit` and the targeted shapes aren't marked with `@input` or `@output` a validation event is emitted. This also accounts for a lot of churn in test cases (either to add suppressions or to add input and output). Due to the suppressed events, Smithy's errorfiles test runner was updated to no longer require that SUPPRESSED validation events are explicitly listed in errorfiles (they can be but don't have to be). Along with updating every operation to define input and output shapes, this change also removes many JSON AST examples. These examples were unnecessary and a liability for the team to maintain. The appropriate place to document how the IDL and JSON AST interact is in the specification language around the IDL and JSON AST, not spread across the entire specification. Alternatively, I could have edited each impacted AST example, but I worried that I would add mistakes and they are not validated against the IDL examples.
These validators nudged modelers to define explicit input and output references for every operation, but added a lot of noise and don't achieve the core objective of defining a dedicated input and output shape for every operation. Better validation could be added through a linter that discourages the use of Unit for operations at all.
Because we validate neighbors before invoking other validators, this is safe. We should make a similar change in other places.
Adds a ModelTransformer transform that ensures every shape defines a dedicated input and output shape marked with the @input and @output traits and uses a consistent naming strategy. The goal of this transform is to consolidate much of the logic that has been repeated in various AWS SDK code generators to generate synthetic input and output shapes for operations. It provides a consisten name for these shapes so that generators know that they can use the name as-is if they'd like. Because shapes are sometimes renamed with this transform, it's important to know what the original shape ID was. This might matter for protocols that need to know the name of the input shape when serializing a request or response (like AWS rest-xml). To accomplish this, I added the concept of ephemeral traits to the model that allow traits to be added to shapes that are only defined in code rather than through a trait definition. Ephemeral traits are not persisted when a model is serialized, but otherwise can be used just like any other trait. When a shape is renamed, an ephemeral trait named OriginalShapeIdTrait is added to the shape to provide the original shape ID of the shape so that things like code generators can query it.
NodeMapper is used to serialize a few more complex traits, and it's picking up the "isEphemeral" method and serializing it as an "ephemeral" attribute. This commit updates setOmitEmptyValues() to also omit boolean false values to prevent this from happening.
This commit improves how Smithy diff reports adding/removing/changing shapes and traits so that it's more readable. This was something I had to do in order to make sense of the impact of this transform on AWS models, so it's included in this PR. I added logging to the transform, and renamed "ephemeral" traits to "synthetic" traits to better match other existing Smithy nomenclature.
mtdowling
force-pushed
the
refactor-operation-io-and-unit-types
branch
from
1bb2e52 to
abad0c9
Compare
kstich
approved these changes
Dec 2, 2021
ganeshnj
pushed a commit
to smithy-lang/smithy-swift
that referenced
this pull request
Aug 2, 2022
## Motivation awslabs/aws-sdk-swift#597 is failing due to unresolved shape `smithy.api#Unit` which was introduced in smithy-lang/smithy#980 ## Changes Updated smithyVersion to 1.22.0. ## Result AWS SDK handles `smithy.api#Unit` for operation correctly.
2 tasks
ganeshnj
pushed a commit
to awslabs/aws-sdk-swift
that referenced
this pull request
Aug 2, 2022
## Motivation #597 is failing due to unresolved shape `smithy.api#Unit` which was introduced in smithy-lang/smithy#980 ## Changes Updated smithyVersion to 1.22.0. ## Result AWS SDK handles `smithy.api#Unit` for operation correctly.
ganeshnj
added a commit
to smithy-lang/smithy-swift
that referenced
this pull request
Aug 11, 2022
## Motivation awslabs/aws-sdk-swift#597 is failing due to unresolved shape smithy.api#Unit which was introduced in smithy-lang/smithy#980 ## Changes - Updated smithyVersion to 1.22.0. - Update smithyGradleVersion to 0.6.0 - Fix issues with header list parsing. The parsing was always correct but the test setup was not, where expected result always assumed that the list is a date list. To support this, I took kotlin implementation of string list parsing and translated to Swift with test cases. - JsonName trait now has no effect in AwsJson1_0 and AwsJson1_1, this change is covered at chore: update smithy version to 1.22.0 aws-sdk-swift#600 - serde change: in case where the top container is sparse map which contains a dense list as value, the dense list was considered as sparse. Therefore, added a way to track the member parent to fix the nullability. - Sensitive trait fixes because now it can't target member directly. ## Result awslabs/aws-sdk-swift#600
ganeshnj
pushed a commit
to awslabs/aws-sdk-swift
that referenced
this pull request
Aug 11, 2022
## Motivation #597 is failing due to unresolved shape `smithy.api#Unit` which was introduced in smithy-lang/smithy#980 ## Changes Updated smithyVersion to 1.22.0. ## Result AWS SDK handles `smithy.api#Unit` for operation correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: A design doc further explaining the change is part of this PR.
This commit introduces the
smithy.api#Unittype that that is used foroperations with no meaningful input or output, and for tagged union
members with no meaningful value. Operations will now, by default,
target
smithy.api#Unitif they do not define input or output.This commit also introduces the
@inputand@outputtraits that areused to specialize structures as for use only as the input or output of
a single operation. The
@inputtrait provides structures with morerelaxed backward compatibility constraints so that, for example, when
things like the
@defaultvalue trait are added, it is considered abackward compatible change to remove the
@requiredtrait from themember of a structure marked with the
@inputtrait.New methods were added to smithy-model to account for the fact that
operations always now have input and output shapes. The existing methods
that return Optional shape IDs and structures remain but are marked as
deprecated. Any usage of these deprecated methods were updated
throughout the monorepo to use the newly introduced methods (this
accounts for a lot of the churn).
Along with updating operations to define input and output shapes,
this change also removes many JSON AST examples. These examples were
unnecessary and a liability for the team to maintain. The appropriate
place to document how the IDL and JSON AST interact is in the
specification language around the IDL and JSON AST, not spread across
the entire specification. Alternatively, I could have edited each impacted AST
example, but I worried that I would add mistakes and they are not validated
against the IDL examples.
Closes #928
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.