Unstable serde support #15
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
…xt` (#1951) * Re-export `DisplayErrorContext` * Update changelog
* Do not use deprecated from_timestamp from chrono CI is failing because [from_timestamp](https://docs.rs/chrono/latest/chrono/naive/struct.NaiveDateTime.html#method.from_timestamp) is now deprecated. Signed-off-by: Daniele Ahmed <[email protected]> * Update changelog Signed-off-by: Daniele Ahmed <[email protected]> * Fix error Signed-off-by: Daniele Ahmed <[email protected]> * Use with_ymd_and_hms Signed-off-by: Daniele Ahmed <[email protected]> Signed-off-by: Daniele Ahmed <[email protected]>
* feature: make HTTP connectors configurable * add: test for HTTP connector configuration customization add: impl<B> From<TestConnection<B>> for HttpConnector add: impl From<CaptureRequestHandler> for HttpConnector add: impl From<NeverConnector> for HttpConnector add: impl From<ReplayingConnection> for HttpConnector * add: to_vec method to AggregatedBytes update: method param names of FluentClientGenerics.sendBounds to be more explicit update: restructure s3/s3control tests to be uniform in structure * update: CHANGELOG.next.toml update: codegen `impl From<&SdkConfig> for Builder` to support HTTP connectors * update: CHANGELOG entry references * add: missing copyright header * fix: clippy lint * format: run cargo fmt * format: run cargo fmt on aws_smithy_client::dvr modules * format: run ktlintFormat * refactor: use from_conf instead of from_conf_conn remove: from_conf_conn * update: impl From<SmithyConnector> for HttpConnector remove: other From<T> for HttpConnector impls update: HttpConnector config setter examples * update: CHANGELOG.next.toml * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * update: CHANGELOG.next.toml remove: obsolete test update: `ConfigLoader::http_connector` setter method * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Update CHANGELOG.next.toml Co-authored-by: John DiSanti <[email protected]> * Apply suggestions from code review Co-authored-by: John DiSanti <[email protected]> * update: aws_config::loader::ConfigLoader doc comments update: CHANGELOG.next.toml examples * fix: doc issues add: reëxport aws_smithy_types::endpoint module to aws-config * format: run rustfmt in the weird CI way to get it to actually format. * fix: incorrect reëxport * add: "aws_smithy_http::endpoint" to allowed external types for aws-config * update: move `hyper-rustls` to deps so that it doesn't break exotic arch CI check * remove: `hyper-rustls` dep because it's not actually needed * fix: aws-types dep issue blocking exotic arch CI check * fix: broken doc comment Co-authored-by: John DiSanti <[email protected]>
* Service with ConnectInfo Signed-off-by: Daniele Ahmed <[email protected]>
This patchset, affectionately called "Builders of builders", lays the groundwork for fully implementing [Constraint traits] in the server SDK generator. [The RFC] illustrates what the end goal looks like, and is recommended prerrequisite reading to understanding this cover letter. This commit makes the sever deserializers work with _unconstrained_ types during request parsing, and only after the entire request is parsed are constraints enforced. Values for a constrained shape are stored in the correspondingly unconstrained shape, and right before the operation input is built, the values are constrained via a `TryFrom<UnconstrainedShape> for ConstrainedShape` implementation that all unconstrained types enjoy. The service owner only interacts with constrained types, the unconstrained ones are `pub(crate)` and for use by the framework only. In the case of structure shapes, the corresponding unconstrained shape is their builders. This is what gives this commit its title: during request deserialization, arbitrarily nested structures are parsed into _builders that hold builders_. Builders keep track of whether their members are constrained or not by storing its members in a `MaybeConstrained` [Cow](https://doc.rust-lang.org/std/borrow/enum.Cow.html)-like `enum` type: ```rust pub(crate) trait Constrained { type Unconstrained; } #[derive(Debug, Clone)] pub(crate) enum MaybeConstrained<T: Constrained> { Constrained(T), Unconstrained(T::Unconstrained), } ``` Consult the documentation for the generator in `ServerBuilderGenerator.kt` for more implementation details and for the differences with the builder types the server has been using, generated by `BuilderGenerator.kt`, which after this commit are exclusively used by clients. Other shape types, when they are constrained, get generated with their correspondingly unconstrained counterparts. Their Rust types are essentially wrapper newtypes, and similarly enjoy `TryFrom` converters to constrain them. See the documentation in `UnconstrainedShapeSymbolProvider.kt` for details and an example. When constraints are not met, the converters raise _constraint violations_. These are currently `enum`s holding the _first_ encountered violation. When a shape is _transitively but not directly_ constrained, newtype wrappers are also generated to hold the nested constrained values. To illustrate their need, consider for example a list of `@length` strings. Upon request parsing, the server deserializers need a way to hold a vector of unconstrained regular `String`s, and a vector of the constrained newtyped `LengthString`s. The former requirement is already satisfied by the generated unconstrained types, but for the latter we need to generate an intermediate constrained `ListUnconstrained(Vec<LengthString>)` newtype that will eventually be unwrapped into the `Vec<LengthString>` the user is handed. This is the purpose of the `PubCrate*` generators: consult the documentation in `PubCrateConstrainedShapeSymbolProvider.kt`, `PubCrateConstrainedCollectionGenerator.kt`, and `PubCrateConstrainedMapGenerator.kt` for more details. As their name implies, all of these types are `pub(crate)`, and the user never interacts with them. For users that would not like their application code to make use of constrained newtypes for their modeled constrained shapes, a `codegenConfig` setting `publicConstrainedTypes` has been added. They opt out of these by setting it to `false`, and use the inner types directly: the framework will still enforce constraints upon request deserialization, but once execution enters an application handler, the user is on their own to honor (or not) the modeled constraints. No user interest has been expressed for this feature, but I expect we will see demand for it. Moreover, it's a good stepping stone for users that want their services to honor constraints, but are not ready to migrate their application code to constrained newtypes. As for how it's implemented, several parts of the codebase inspect the setting and toggle or tweak generators based on its value. Perhaps the only detail worth mentioning in this commit message is that the structure shape builder types are generated by a much simpler and entirely different generator, in `ServerBuilderGeneratorWithoutPublicConstrainedTypes.kt`. Note that this builder _does not_ enforce constraints, except for `required` and `enum`, which are always (and already) baked into the type system. When `publicConstrainedTypes` is disabled, this is the builder that end users interact with, while the one that enforces all constraints, `ServerBuilderGenerator`, is now generated as `pub(crate)` and left for exclusive use by the deserializers. See the relevant documentation for the details and differences among the builder types. As proof that these foundations are sound, this commit also implements the `length` constraint trait on Smithy map and string shapes. Likewise, the `required` and `enum` traits, which were already baked in the generated types as non-`Option`al and `enum` Rust types, respectively, are now also treated like the rest of constraint traits upon request deserialization. See the documentation in `ConstrainedMapGenerator.kt` and `ConstrainedStringGenerator.kt` for details. The rest of the constraint traits and target shapes are left as an exercise to the reader, but hopefully the reader has been convinced that all of them can be enforced within this framework, paving the way for straightforward implementations. The diff is already large as it is. Any reamining work is being tracked in #1401; this and other issues are referenced in the code as TODOs. So as to not give users the impression that the server SDK plugin _fully_ honors constraints as per the Smithy specification, a validator in `ValidateUnsupportedConstraintsAreNotUsed.kt` has been added. This traverses the model and detects yet-unsupported parts of the spec, aborting code generation and printing informative warnings referencing the relevant tracking issues. This is a regression in that models that used constraint traits previously built fine (even though the constraint traits were silently not being honored), and now they will break. To unblock generation of these models, this commit adds another `codegenConfig` setting, `ignoreUnsupportedConstraints`, that users can opt into. Closes #1714. Testing ------- Several Kotlin unit test classes exercising the finer details of the added generators and symbol providers have been added. However, the best way to test is to generate server SDKs from models making use of constraint traits. The biggest assurances come from the newly added `constraints.smithy` model, an "academic" service that _heavily_ exercises constraint traits. It's a `restJson1` service that also tests binding of constrained shapes to different parts of the HTTP message. Deeply nested hierarchies and recursive shapes are also featured. ```sh ./gradlew -P modules='constraints' codegen-server-test:build ``` This model is _additionally_ generated in CI with the `publicConstrainedTypes` setting disabled: ```sh ./gradlew -P modules='constraints_without_public_constrained_types' codegen-server-test:build `````` Similarly, models using currently unsupported constraints are now being generated with the `ignoreUnsupportedConstraints` setting enabled. See `codegen-server-test/build.gradle.kts` for more details. [Constraint traits]: https://awslabs.github.io/smithy/2.0/spec/constraint-traits.html [The RFC]: #1199
Revamp errors in: * `aws-types` * `aws-endpoint` * `aws-http` * `aws-sig-auth` * `aws-inlineable`
* Implement RFC 23, with the exception of PluginBuilder * Update documentation. * Elide implementation details in `Upgradable`. * Update wording in docs to avoid personal pronouns. * Update `Upgradable`'s documentation. * Template the service builder name. * Template MissingOperationsError directly. * Code-generate an array directly. * Update design/src/server/anatomy.md Co-authored-by: david-perez <[email protected]> * Use `rust` where we do not need templating. * Remove unused variable. * Add `expect` message to point users at our issue board in case a panic slips through. * Typo. * Update `expect` error message. * Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call. * Fix new pokemon bin example. * Fix new pokemon bin example. * Fix unresolved symbolProvider. * Assign the `expect` error message to a variable. * Omit additional generic parameters in Upgradable when it's first introduced. Co-authored-by: david-perez <[email protected]>
Signed-off-by: Daniele Ahmed <[email protected]>
* Add support for Endpoints 2.0 Parameters This commit adds `EndpointDecorator` which injects Endpoint parameters in to the operation property bag. These can come from an ordered list of sources—this wires them all up. To facilitate testing, this diff also writes the parameters into the property bag during operation generation. * remove println * CR Feedback
* Implement RFC 23, with the exception of PluginBuilder * Update documentation. * Avoid star re-exports. * Elide implementation details in `Upgradable`. * Update wording in docs to avoid personal pronouns. * Update `Upgradable`'s documentation. * Template the service builder name. * Template MissingOperationsError directly. * Code-generate an array directly. * Sketch out the implementation of `PluginPipeline`. Remove `PluginExt`. Add a public constructor for `FilterByOperationName`. * Ask for a `PluginPipeline` as input for the generated builder. * Rename `add` to `push`. * Remove Pluggable. Rename `composer` module to `pipeline`. * Remove all mentions of `Pluggable` from docs and examples. * Fix punctuation. * Rename variable from `composer` to `pipeline` in doc examples. * Add a free-standing function for filtering. * Rename. * Rename. * Update design/src/server/anatomy.md Co-authored-by: david-perez <[email protected]> * Use `rust` where we do not need templating. * Remove unused variable. * Add `expect` message to point users at our issue board in case a panic slips through. * Typo. * Update `expect` error message. * Refactor the `for` loop in ``requestSpecMap` into an `associateWith` call. * Remove unnecessary bound - since `new` is private, the condition is already enforced via `filter_by_operation_name`. * Use `Self` in `new`. * Rename `inner` to `into_inner` * Rename `concat` to `append` to correctly mirror Vec's API terminology. * Fix codegen to use renamed method. * Cut down the public API surface to key methods for a sequence-like interface. * Add a note about ordering. * Add method docs. * Add Default implementation. * Fix new pokemon bin example. * Fix new pokemon bin example. * Fix code-generated builder. * Fix unresolved symbolProvider. * Assign the `expect` error message to a variable. * Do not require a PluginPipeline as input to `builder_with_plugins`. * Reverse plugin application order. * Upgrade documentation. * Add a test to verify that plugin layers are executed in registration order. * Add license header. * Update middleware.md * Typo. * Fix more builder() calls. Co-authored-by: david-perez <[email protected]>
* Make enum forward-compatible This commit implements the suggested approach described in #627 (comment) The idea is that once the user writes a match expression against an enum and assumes that an execution path comes to a particular match arm, we should guarantee that when the user upgrades a version of SDK, the execution path should come to the same match arm as before. * Add unit test to ensure enums are forward-compatible The test first mimics the user's interaction with the enum generated from a model where the user writes a match expression on the enum, wishing to hit the match arm corresponding to Variant3, which is not yet supported by the model. The test then simulates a scenario where the user now has access to the updated enum generated from the next version of the model that does support Variant3. The user's code should compile in both of the use cases. * Generate rustdoc for enum's forward-compatibility This commits generates rustdoc explaining to the users how they might write a match expression against a generated enum so their code is forward-compatible. While it is explained in #627 (comment), it can be helpful if the users can read it in rustdoc right below where the enum's definition is shown. * Make snippet in rustdoc text This commit updates a rustdoc code snippet generated for an enum from `rust,no_run` to `text`. We observed that the snippet fails to compile in CI because the name of the enum was not fully qualified; code in rustdoc is treated in a similar way that code in integration tests is treated as being outside of a crate, but not part of the crate, which means the name of the crate needs to be spelled out for a `use` statement if we want to import the enum to the code in rustdoc. However, the name of the crate is usually one-off, generated on the fly for testing, making it not obvious to hard-code it or obtain it programmatically from within Kotlin code. * Suppress missing doc lint for UnknownVariantValue This commit marks UnknownVariantValue as `#[allow(missing_docs)]` because failing to do so will cause tests in CI to fail. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: Russell Cohen <[email protected]> * Generate UnknownVariantValue via forInlineFun This commit attempts to create UnknownVariantValue using RuntimeType.forInlineFun. Prior to this commit, it was generated per enum but that caused multiple definitions of UnknownVariantValue in a single file as there can be multiple enums in the file. By using RuntimeType.forInlineFun, we can generate a single UnknownVariantValue per module and the enums within the module can refer to the same instance. This commit, however, fails to pass the unit tests in EnumGeneratorTest. The issue seems to be that a RustWriter used in the tests does not end up calling the finalize method, failing to render inline functions. * Replace "target == CodegenTarget.CLIENT" with a helper This commit addresses #1945 (comment) but uses an existing helper CodegenTarget.renderUnknownVariant. * Update EnumGeneratorTests to use TestWorkspace This commit addresses failures for unit tests in EnumGeneratorTests. Now that we use RuntimeType.forInlineFcn in EnumGenerator, we need to ensure that the inline functions should be rendered to a Rust source file during testing. RustWriter, used in EnumGeneratorTests prior to this commit, was not capable of doing so. We thus update EnumGeneratorTests to use TestWorkspace by which we ensure that the inline functions are rendered during testing. * Make sure to use the passed-in variable for shapeId This commit fixes a copy-and-paste error introduced in 712c983. The function should use the passed-in variable rather than the hard-coded literal "test#SomeEnum". * Address https://github.com/awslabs/smithy-rs/pull/1945\#discussion_r1014389852 * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml This commit addresses #1945 (comment) * Avoid potential name collisions by UnknownVariantValue This commit addresses #1945 (comment). It changes the module in which the `UnknownVariantValue` is rendered. Before the commit, it was emitted to the `model` module but that might cause name collisions in the future when a Smithy model has a shape named `UnknownVariantValue`. After this commit, we render it to the `types` module. * Move re-exports from lib.rs to types.rs This commit moves re-export statements in client crates from `lib.rs` to `types.rs`. The motivation is that now that we render the struct `UnknownVariantValue` in a separate file for the `types` module, we can no longer have a `pub mod types {...}` block in `lib.rs` as it cannot coexist with `pub mod types;`. * Add docs on UnknownVariantValue This commit adds public docs on `UnknownVariantValue`. Since it is rendered in `types.rs`, the users may not find the `Unknown` variant of an enum in the same file and may wonder what this struct is for when seeing it in isolation. * Update CHANGELOG.next.toml This commit addresses #1945 (comment) * Update the module documentation for types This commit updates rustdoc for the types module to reflect that fact that the module now contains not only re-exports but also an auxiliary struct `UnknownVariantValue`. * Add extensions to run code block depending on the target This commit introduces extensions on CodegenTarget that execute the block of code depending on the target is for client or for server. These extensions are intended to replace if-else expressions throughout the codebase explicitly checking whether the target is for client or for server. We do not update such if-else expressions all at once in this commit. Rather, we are planning to make incremental changes to update them opportunistically. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/EnumGenerator.kt Co-authored-by: John DiSanti <[email protected]> * Move extensions into CodegenTarget as methods This commit addresses #1945 (comment). By moving extensions into the CodegenTarget enum, no separate import is required to use them. Co-authored-by: Saito <[email protected]> Co-authored-by: Russell Cohen <[email protected]> Co-authored-by: Yuki Saito <[email protected]> Co-authored-by: John DiSanti <[email protected]>
* formatting: run code cleanup on entire project * revert: deletion of SdkSettings.defaultsConfigPath
- `Endpoint::set_endpoint` no longer panics when called on an endpoint without a scheme - `Endpoint::mutable` and `Endpoint::immutable` now both return a result so that constructing an endpoint without a scheme is an error - `Endpoint::mutable` and `Endpoint::immutable` both now take a string instead of a `Uri` as a convenience - `Endpoint::mutable_uri` and `Endpoint::immutable_uri` were added to construct an endpoint directly from a `Uri`
RestJson1 implementations can denote errors in responses in several ways. New server-side protocol implementations MUST use a header field named `X-Amzn-Errortype`. Note that the spec says that implementations SHOULD strip the error shape ID's namespace. However, our server implementation renders the full shape ID (including namespace), since some existing clients rely on it to deserialize the error shape and fail if only the shape name is present. This is compliant with the spec, see smithy-lang/smithy#1493. See smithy-lang/smithy#1494 too.
update: crate-hasher sha256 dep to 1.1
…1996) * update: CargoDependency companion fn names for smithy runtime crates rename: CargoDependency.asType to CargoDependency.toType fix: errors in InlineDependency doc comment formatting: run import optimizer for all kotlin files fix: gradle issue with incorrectly named tasks * fix: server test broken by removal of rustName
Co-authored-by: Julian Antonielli <[email protected]> Co-authored-by: Luca Palmieri <[email protected]>
* Multiple FromParts in handlers
Handlers can have up to 8 extensions:
```
pub async fn handler(
input: input::Input,
ext0: Extension<...>,
ext1: Extension<...>,
ext2: Extension<...>,
ext3: Extension<...>,
ext4: Extension<...>,
ext5: Extension<...>,
ext6: Extension<...>,
ext7: Extension<...>,
) -> Result<output::Output, error::Error> { ... }
```
Signed-off-by: Daniele Ahmed <[email protected]>
Co-authored-by: Harry Barber <[email protected]>
…2287) * Fix if condition. * Make sure that the changes are visible to the push step after the Docker action has executed by persisting the modified repository as an artifact. * Give a name to the argument.
* Use larger machines for the slowest CI jobs. * Fix invocation.
* Fix paths. * Fix commit logic - it should commit when there were changes.
* Fix the reference that gets checked out * Fix
* Fix broken doc link to `Stream` This commit fixes broken doc link to `Stream` in codegen clients. That target is `tokio_stream::Stream`, which in turn is a re-export of `futures_core::Stream`. Since we do not have a good way to link to the re-export, we remove a hyper link and just put `Stream`. * Update CHANGELOG.next.toml --------- Co-authored-by: Yuki Saito <[email protected]>
Login to ECR when credentials are available to improve CI performance
Co-authored-by: Russell Cohen <[email protected]>
* Fix CI on main and don't acquire Docker login for forks * Convert empty env vars into `None` * Optimize base image acquisition on main
* Fix working directory. * Build the Docker image using the latest commit on the invocation branch. * We need a Docker image with the same SHA later in the process.
…gradle.properties update being lost. (#2299)
* Do not alter Operation shape ID * Add OperationExtensionExt test * Add CHANGELOG.next.toml entry * Apply suggestions from code review Co-authored-by: Luca Palmieri <[email protected]> --------- Co-authored-by: Luca Palmieri <[email protected]>
Co-authored-by: AWS SDK Rust Bot <[email protected]>
The writable calculates a string that it never writes. This was added in #2131.
* fn set_fields * add fn set_fields to fluent builder * better docs * fix * improve document cfg to Attribute class
* add CfgUnstable for feature-gate * add features * add feature-gate * strings for feature gate * Revert "strings for feature gate" This reverts commit 1f33a5c. * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/RuntimeType.kt Co-authored-by: Zelda Hessler <[email protected]> * fix dependency thing on cargo * add OutputShape to builder * EnumGenerator * StructureGenerator * UnionGenerator * todo * fixed? * SerdeDecorator * codegen stuff * update * fix * Apply suggestions from code review Co-authored-by: Zelda Hessler <[email protected]> * - refactoring - pre-commit - https://github.com/awslabs/smithy-rs/pull/2183/files#r1080594621 * adds serde-serialize/deserialize * this one causes null pointer exception * interim solution * new push * fix * add Sensitive Warning * add test for CargoTomlGeneratorTest pre-commit fix * Update codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt Co-authored-by: Zelda Hessler <[email protected]> --------- Co-authored-by: Zelda Hessler <[email protected]>
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.
Motivation and Context
Description
Testing
Checklist
CHANGELOG.next.tomlif I made changes to the smithy-rs codegen or runtime cratesCHANGELOG.next.tomlif I made changes to the AWS SDK, generated SDK code, or SDK runtime cratesBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.