Skip to content

Commit

Permalink
Move Message, Header, HeaderValue, and StrBytes from `aws-smi…
Browse files Browse the repository at this point in the history
…thy-eventstream` to `aws-smithy-types` (#3139)

## Motivation and Context
Fixes the following statement in
#3114

> A notable breaking change is the error type of the new recv method.
Previously, it was [SdkError<E,
RawMessage>>](https://docs.rs/aws-smithy-http/0.57.0/aws_smithy_http/event_stream/struct.Receiver.html#method.recv)
but it is now a BoxError. This means that upon an event receive error,
customers can only show what the error is, but not match on it and take
some actions.

so that the `recv` method on a new-type wrapper
`aws_smithy_http::event_stream::Receiver` can expose `SdkError<E,
RawMessage>>` in public API. It is the responsibility of the above PR to
move
[RawMessage](https://github.com/awslabs/smithy-rs/blob/c7f0d5dc33937d65841d74b5b828cc77ed1d2db4/rust-runtime/aws-smithy-http/src/event_stream/receiver.rs#L92-L98)
from `aws-smithy-http` to `aws-smithy-types` but doing so first requires
`Message` to be moved to `aws-smithy-types`, hence this PR.


## Description
Moving `Message` to `aws-smithy-types` also requires moving the types it
depends upon, namely `Header`, `HederValue`, and `StrBytes`. However,
their serialization/deserialization methods (`read_from` and `write_to`)
remain in `aws-smithy-eventstream` and they are converted to free
functions, with the aim of moving things as minimally as possible.

`aws-smithy-eventstream` has fuzz testing. It previously implemented the
`Arbitrary` trait for moved types, but since they are now in
`aws-smithy-types`, we need to have newtypes for them to enable the
trait due to the orphan rules. A newly added module
`aws-smithy-eventstream::arbitrary` is exactly for that purpose.

## Testing
Relied on the existing tests.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
  • Loading branch information
ysaito1001 authored Nov 6, 2023
1 parent 315d88b commit c296e8e
Show file tree
Hide file tree
Showing 30 changed files with 648 additions and 497 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,11 @@ message = "The HTTP `Request`, `Response`, `Headers`, and `HeaderValue` types ha
references = ["smithy-rs#3138"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" }
author = "jdisanti"

[[smithy-rs]]
message = """
`Message`, `Header`, `HeaderValue`, and `StrBytes` have been moved to `aws-smithy-types` from `aws-smithy-eventstream`. `Message::read_from` and `Message::write_to` remain in `aws-smithy-eventstream` but they are converted to free functions with the names `read_message_from` and `write_message_to` respectively.
"""
references = ["smithy-rs#3139"]
meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "all"}
author = "ysaito1001"
6 changes: 4 additions & 2 deletions aws/rust-runtime/aws-runtime/src/auth/sigv4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ mod event_stream {
use aws_sigv4::event_stream::{sign_empty_message, sign_message};
use aws_sigv4::sign::v4;
use aws_smithy_async::time::SharedTimeSource;
use aws_smithy_eventstream::frame::{Message, SignMessage, SignMessageError};
use aws_smithy_eventstream::frame::{SignMessage, SignMessageError};
use aws_smithy_runtime_api::client::identity::Identity;
use aws_smithy_types::event_stream::Message;
use aws_types::region::SigningRegion;
use aws_types::SigningName;

Expand Down Expand Up @@ -293,7 +294,8 @@ mod event_stream {
use crate::auth::sigv4::event_stream::SigV4MessageSigner;
use aws_credential_types::Credentials;
use aws_smithy_async::time::SharedTimeSource;
use aws_smithy_eventstream::frame::{HeaderValue, Message, SignMessage};
use aws_smithy_eventstream::frame::SignMessage;
use aws_smithy_types::event_stream::{HeaderValue, Message};

use aws_types::region::Region;
use aws_types::region::SigningRegion;
Expand Down
1 change: 1 addition & 0 deletions aws/rust-runtime/aws-sigv4/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ aws-credential-types = { path = "../aws-credential-types" }
aws-smithy-eventstream = { path = "../../../rust-runtime/aws-smithy-eventstream", optional = true }
aws-smithy-http = { path = "../../../rust-runtime/aws-smithy-http" }
aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api", features = ["client"] }
aws-smithy-types = { path = "../../../rust-runtime/aws-smithy-types" }
bytes = "1"
form_urlencoded = { version = "1.0", optional = true }
hex = "0.4"
Expand Down
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-sigv4/external-types.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ allowed_external_types = [
# TODO(refactorHttp): Remove this and remove the signing helpers
"http::request::Request",
# TODO(https://github.com/awslabs/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature
"aws_smithy_eventstream::frame::Message",
"aws_smithy_types::event_stream::Message",
"aws_smithy_runtime_api::client::identity::Identity"
]
12 changes: 7 additions & 5 deletions aws/rust-runtime/aws-sigv4/src/event_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//!
//! ```rust
//! use aws_sigv4::event_stream::sign_message;
//! use aws_smithy_eventstream::frame::{Header, HeaderValue, Message};
//! use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
//! use std::time::SystemTime;
//! use aws_credential_types::Credentials;
//! use aws_smithy_runtime_api::client::identity::Identity;
Expand Down Expand Up @@ -51,7 +51,8 @@ use crate::http_request::SigningError;
use crate::sign::v4::{calculate_signature, generate_signing_key, sha256_hex_string};
use crate::SigningOutput;
use aws_credential_types::Credentials;
use aws_smithy_eventstream::frame::{write_headers_to, Header, HeaderValue, Message};
use aws_smithy_eventstream::frame::{write_headers_to, write_message_to};
use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
use bytes::Bytes;
use std::io::Write;
use std::time::SystemTime;
Expand Down Expand Up @@ -102,7 +103,7 @@ pub fn sign_message<'a>(
) -> Result<SigningOutput<Message>, SigningError> {
let message_payload = {
let mut payload = Vec::new();
message.write_to(&mut payload).unwrap();
write_message_to(message, &mut payload).unwrap();
payload
};
sign_payload(Some(message_payload), last_signature, params)
Expand Down Expand Up @@ -161,7 +162,8 @@ mod tests {
use crate::event_stream::{calculate_string_to_sign, sign_message, SigningParams};
use crate::sign::v4::sha256_hex_string;
use aws_credential_types::Credentials;
use aws_smithy_eventstream::frame::{Header, HeaderValue, Message};
use aws_smithy_eventstream::frame::write_message_to;
use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
use std::time::{Duration, UNIX_EPOCH};

#[test]
Expand All @@ -171,7 +173,7 @@ mod tests {
HeaderValue::String("value".into()),
));
let mut message_payload = Vec::new();
message_to_sign.write_to(&mut message_payload).unwrap();
write_message_to(&message_to_sign, &mut message_payload).unwrap();

let params = SigningParams {
identity: &Credentials::for_tests().into(),
Expand Down
5 changes: 3 additions & 2 deletions aws/sdk/integration-tests/transcribestreaming/tests/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ use async_stream::stream;
use aws_sdk_transcribestreaming::config::{Credentials, Region};
use aws_sdk_transcribestreaming::error::SdkError;
use aws_sdk_transcribestreaming::operation::start_stream_transcription::StartStreamTranscriptionOutput;
use aws_sdk_transcribestreaming::primitives::event_stream::{HeaderValue, Message};
use aws_sdk_transcribestreaming::primitives::Blob;
use aws_sdk_transcribestreaming::types::error::{AudioStreamError, TranscriptResultStreamError};
use aws_sdk_transcribestreaming::types::{
AudioEvent, AudioStream, LanguageCode, MediaEncoding, TranscriptResultStream,
};
use aws_sdk_transcribestreaming::{Client, Config};
use aws_smithy_eventstream::frame::{DecodedFrame, HeaderValue, Message, MessageFrameDecoder};
use aws_smithy_eventstream::frame::{read_message_from, DecodedFrame, MessageFrameDecoder};
use aws_smithy_runtime::client::http::test_util::dvr::{Event, ReplayingClient};
use bytes::BufMut;
use futures_core::Stream;
Expand Down Expand Up @@ -132,7 +133,7 @@ fn decode_frames(mut body: &[u8]) -> Vec<(Message, Option<Message>)> {
let inner_msg = if msg.payload().is_empty() {
None
} else {
Some(Message::read_from(msg.payload().as_ref()).unwrap())
Some(read_message_from(msg.payload().as_ref()).unwrap())
};
result.push((msg, inner_msg));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,16 @@ object ClientRustModule {
val Meta = RustModule.public("meta")
val Input = RustModule.public("input")
val Output = RustModule.public("output")
val Primitives = RustModule.public("primitives")

/** crate::primitives */
val primitives = Primitives.self
object Primitives {
/** crate::primitives */
val self = RustModule.public("primitives")

/** crate::primitives::event_stream */
val EventStream = RustModule.public("event_stream", parent = self)
}

/** crate::types */
val types = Types.self
Expand Down Expand Up @@ -110,7 +119,8 @@ class ClientModuleDocProvider(
ClientRustModule.Meta -> strDoc("Information about this crate.")
ClientRustModule.Input -> PANIC("this module shouldn't exist in the new scheme")
ClientRustModule.Output -> PANIC("this module shouldn't exist in the new scheme")
ClientRustModule.Primitives -> strDoc("Primitives such as `Blob` or `DateTime` used by other types.")
ClientRustModule.primitives -> strDoc("Primitives such as `Blob` or `DateTime` used by other types.")
ClientRustModule.Primitives.EventStream -> strDoc("Event stream related primitives such as `Message` or `Header`.")
ClientRustModule.types -> strDoc("Data structures used by operation inputs/outputs.")
ClientRustModule.Types.Error -> strDoc("Error types that $serviceName can respond with.")
else -> TODO("Document this module: $module")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.smithy.customizations.AllowLintsCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customizations.CrateVersionCustomization
import software.amazon.smithy.rust.codegen.core.smithy.customizations.pubUseSmithyPrimitives
import software.amazon.smithy.rust.codegen.core.smithy.customizations.pubUseSmithyPrimitivesEventStream
import software.amazon.smithy.rust.codegen.core.smithy.generators.LibRsCustomization

val TestUtilFeature = Feature("test-util", false, listOf())
Expand Down Expand Up @@ -85,9 +86,12 @@ class RequiredCustomizations : ClientCodegenDecorator {
// Re-export resiliency types
ResiliencyReExportCustomization(codegenContext).extras(rustCrate)

rustCrate.withModule(ClientRustModule.Primitives) {
rustCrate.withModule(ClientRustModule.primitives) {
pubUseSmithyPrimitives(codegenContext, codegenContext.model, rustCrate)(this)
}
rustCrate.withModule(ClientRustModule.Primitives.EventStream) {
pubUseSmithyPrimitivesEventStream(codegenContext, codegenContext.model)(this)
}
rustCrate.withModule(ClientRustModule.Error) {
rustTemplate(
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,5 +174,5 @@ class ClientEnumGenerator(codegenContext: ClientCodegenContext, shape: StringSha
codegenContext.model,
codegenContext.symbolProvider,
shape,
InfallibleEnumType(ClientRustModule.Primitives),
InfallibleEnumType(ClientRustModule.primitives),
)
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
import software.amazon.smithy.rust.codegen.core.util.hasEventStreamMember
import software.amazon.smithy.rust.codegen.core.util.hasEventStreamOperations
import software.amazon.smithy.rust.codegen.core.util.hasStreamingMember

/** Returns true if the model has normal streaming operations (excluding event streams) */
Expand Down Expand Up @@ -80,3 +81,22 @@ fun pubUseSmithyPrimitives(codegenContext: CodegenContext, model: Model, rustCra
)
}
}

/** Adds re-export statements for event-stream-related Smithy primitives */
fun pubUseSmithyPrimitivesEventStream(codegenContext: CodegenContext, model: Model): Writable = writable {
val rc = codegenContext.runtimeConfig
if (codegenContext.serviceShape.hasEventStreamOperations(model)) {
rustTemplate(
"""
pub use #{Header};
pub use #{HeaderValue};
pub use #{Message};
pub use #{StrBytes};
""",
"Header" to RuntimeType.smithyTypes(rc).resolve("event_stream::Header"),
"HeaderValue" to RuntimeType.smithyTypes(rc).resolve("event_stream::HeaderValue"),
"Message" to RuntimeType.smithyTypes(rc).resolve("event_stream::Message"),
"StrBytes" to RuntimeType.smithyTypes(rc).resolve("str_bytes::StrBytes"),
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,15 @@ class EventStreamUnmarshallerGenerator(
symbolProvider.symbolForEventStreamError(unionShape)
}
private val smithyEventStream = RuntimeType.smithyEventStream(runtimeConfig)
private val smithyTypes = RuntimeType.smithyTypes(runtimeConfig)
private val eventStreamSerdeModule = RustModule.eventStreamSerdeModule()
private val codegenScope = arrayOf(
"Blob" to RuntimeType.blob(runtimeConfig),
"expect_fns" to smithyEventStream.resolve("smithy"),
"MarshallMessage" to smithyEventStream.resolve("frame::MarshallMessage"),
"Message" to smithyEventStream.resolve("frame::Message"),
"Header" to smithyEventStream.resolve("frame::Header"),
"HeaderValue" to smithyEventStream.resolve("frame::HeaderValue"),
"Message" to smithyTypes.resolve("event_stream::Message"),
"Header" to smithyTypes.resolve("event_stream::Header"),
"HeaderValue" to smithyTypes.resolve("event_stream::HeaderValue"),
"Error" to smithyEventStream.resolve("error::Error"),
"OpError" to errorSymbol,
"SmithyError" to RuntimeType.smithyTypes(runtimeConfig).resolve("Error"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class EventStreamErrorMarshallerGenerator(
payloadContentType: String,
) : EventStreamMarshallerGenerator(model, target, runtimeConfig, symbolProvider, unionShape, serializerGenerator, payloadContentType) {
private val smithyEventStream = RuntimeType.smithyEventStream(runtimeConfig)
private val smithyTypes = RuntimeType.smithyTypes(runtimeConfig)

private val operationErrorSymbol = if (target == CodegenTarget.SERVER && unionShape.eventStreamErrors().isEmpty()) {
RuntimeType.smithyHttp(runtimeConfig).resolve("event_stream::MessageStreamError").toSymbol()
Expand All @@ -54,9 +55,9 @@ class EventStreamErrorMarshallerGenerator(
private val errorsShape = unionShape.expectTrait<SyntheticEventStreamUnionTrait>()
private val codegenScope = arrayOf(
"MarshallMessage" to smithyEventStream.resolve("frame::MarshallMessage"),
"Message" to smithyEventStream.resolve("frame::Message"),
"Header" to smithyEventStream.resolve("frame::Header"),
"HeaderValue" to smithyEventStream.resolve("frame::HeaderValue"),
"Message" to smithyTypes.resolve("event_stream::Message"),
"Header" to smithyTypes.resolve("event_stream::Header"),
"HeaderValue" to smithyTypes.resolve("event_stream::HeaderValue"),
"Error" to smithyEventStream.resolve("error::Error"),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,13 @@ open class EventStreamMarshallerGenerator(
private val payloadContentType: String,
) {
private val smithyEventStream = RuntimeType.smithyEventStream(runtimeConfig)
private val smithyTypes = RuntimeType.smithyTypes(runtimeConfig)
private val eventStreamSerdeModule = RustModule.eventStreamSerdeModule()
private val codegenScope = arrayOf(
"MarshallMessage" to smithyEventStream.resolve("frame::MarshallMessage"),
"Message" to smithyEventStream.resolve("frame::Message"),
"Header" to smithyEventStream.resolve("frame::Header"),
"HeaderValue" to smithyEventStream.resolve("frame::HeaderValue"),
"Message" to smithyTypes.resolve("event_stream::Message"),
"Header" to smithyTypes.resolve("event_stream::Header"),
"HeaderValue" to smithyTypes.resolve("event_stream::HeaderValue"),
"Error" to smithyEventStream.resolve("error::Error"),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ object EventStreamMarshallTestCases {
val typesModule = codegenContext.symbolProvider.moduleForShape(codegenContext.model.lookup("test#TestStruct"))
rustTemplate(
"""
use aws_smithy_eventstream::frame::{Message, Header, HeaderValue, MarshallMessage};
use aws_smithy_eventstream::frame::MarshallMessage;
use aws_smithy_types::event_stream::{Message, Header, HeaderValue};
use std::collections::HashMap;
use aws_smithy_types::{Blob, DateTime};
use ${typesModule.fullyQualifiedPath()}::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ object EventStreamUnmarshallTestCases {
val typesModule = codegenContext.symbolProvider.moduleForShape(codegenContext.model.lookup("test#TestStruct"))
rust(
"""
use aws_smithy_eventstream::frame::{Header, HeaderValue, Message, UnmarshallMessage, UnmarshalledMessage};
use aws_smithy_eventstream::frame::{UnmarshallMessage, UnmarshalledMessage};
use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
use aws_smithy_types::{Blob, DateTime};
use $testStreamError;
use ${typesModule.fullyQualifiedPath()}::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

#![no_main]

use aws_smithy_eventstream::frame::Message;
use aws_smithy_eventstream::frame::read_message_from;
use bytes::BufMut;
use crc32fast::Hasher as Crc;
use libfuzzer_sys::fuzz_target;
Expand All @@ -30,7 +30,7 @@ fuzz_target!(|input: Input| {
message.put_u32(crc(&message));

let mut data = &mut &message[..];
let _ = Message::read_from(&mut data);
let _ = read_message_from(&mut data);
});

fn crc(input: &[u8]) -> u32 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

#![no_main]

use aws_smithy_eventstream::frame::{Header, HeaderValue, Message};
use aws_smithy_eventstream::frame::{read_message_from, write_message_to};
use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
use aws_smithy_types::DateTime;
use bytes::{Buf, BufMut};
use crc32fast::Hasher as Crc;
Expand All @@ -18,7 +19,7 @@ use libfuzzer_sys::{fuzz_mutator, fuzz_target};
// so that the fuzzer can actually explore the header parsing logic.
fn mutate(data: &mut [u8], size: usize, max_size: usize) -> usize {
let input = &mut &data[..size];
let message = if let Ok(message) = Message::read_from(input) {
let message = if let Ok(message) = read_message_from(input) {
message
} else {
Message::new(&b"some payload"[..])
Expand All @@ -44,7 +45,7 @@ fn mutate(data: &mut [u8], size: usize, max_size: usize) -> usize {
};

let mut bytes = Vec::new();
message.write_to(&mut bytes).unwrap();
write_message_to(&message, &mut bytes).unwrap();

let headers_len = (&bytes[4..8]).get_u32();
let non_header_len = bytes.len() - headers_len as usize;
Expand Down Expand Up @@ -72,7 +73,7 @@ fuzz_mutator!(

fuzz_target!(|data: &[u8]| {
let mut message = data;
let _ = Message::read_from(&mut message);
let _ = read_message_from(&mut message);
});

fn crc(input: &[u8]) -> u32 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

#![no_main]

use aws_smithy_eventstream::frame::{Header, HeaderValue, Message};
use aws_smithy_eventstream::frame::{read_message_from, write_message_to};
use aws_smithy_types::event_stream::{Header, HeaderValue, Message};
use bytes::{Buf, BufMut};
use crc32fast::Hasher as Crc;
use libfuzzer_sys::fuzz_target;
Expand All @@ -22,7 +23,7 @@ fuzz_target!(|input: Input| {
.add_header(Header::new("str", HeaderValue::String("some str".into())));

let mut bytes = Vec::new();
message.write_to(&mut bytes).unwrap();
write_message_to(&message, &mut bytes).unwrap();

let headers_len = (&bytes[4..8]).get_u32();
let headers = &bytes[12..(12 + headers_len as usize)];
Expand All @@ -35,7 +36,7 @@ fuzz_target!(|input: Input| {
mutated.put_slice(message.payload());
mutated.put_u32(crc(&mutated));

let _ = Message::read_from(&mut &mutated[..]);
let _ = read_message_from(&mut &mutated[..]);
});

fn crc(input: &[u8]) -> u32 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@

#![no_main]

use aws_smithy_eventstream::frame::Message;
use aws_smithy_eventstream::frame::read_message_from;
use libfuzzer_sys::fuzz_target;

fuzz_target!(|data: &[u8]| {
let mut message = data;
let _ = Message::read_from(&mut message);
let _ = read_message_from(&mut message);
});
Loading

0 comments on commit c296e8e

Please sign in to comment.