Skip to content

Commit

Permalink
Add server RPC v2 CBOR support (#2544)
Browse files Browse the repository at this point in the history
RPC v2 CBOR is a new protocol that ~is being added~ has [recently been
added](https://smithy.io/2.0/additional-specs/protocols/smithy-rpc-v2.html)
to the Smithy
specification.

_(I'll add more details here as the patchset evolves)_

Credit goes to @jjant for initial implementation of the router, which I
built on top of from his
[`jjant/smithy-rpc-v2-exploration`](https://github.com/awslabs/smithy-rs/tree/jjant/smithy-rpc-v2-exploration)
branch.

Tracking issue: #3573.

## Caveats

`TODO`s are currently exhaustively sprinkled throughout the patch
documenting what remains to be done. Most of these need to be addressed
before this can be merged in; some can be punted on to not make this PR
bigger.

However, I'd like to call out the major caveats and blockers here. I'll
keep updating this list as the patchset evolves.

- [x] RPC v2 has still not been added to the Smithy specification. It is
currently being worked on over in the
[`smithy-rpc-v2`](https://github.com/awslabs/smithy/tree/smithy-rpc-v2)
branch. The following are prerrequisites for this PR to be merged;
**until they are done CI on this PR will fail**:
    - [x] Smithy merges in RPC v2 support.
    - [x] Smithy releases a new version incorporating RPC v2 support.
- Released in [Smithy
v1.47](https://github.com/smithy-lang/smithy/releases/tag/1.47.0)
    - [x] smithy-rs updates to the new version.
        - Updated in #3552
- [x] ~Protocol tests for the protocol do not currently exist in Smithy.
Until those get written~, this PR resorts to Rust unit tests and
integration tests that use `serde` to round-trip messages and compare
`serde`'s encoders and decoders with ours for correctness.
- Protocol tests are under the
[`smithy-protocol-tests`](https://github.com/smithy-lang/smithy/tree/main/smithy-protocol-tests/model/rpcv2Cbor)
directory in Smithy.
- We're keeping the `serde_cbor` round-trip tests for defense in depth.
- [ ] #3709 - Currently
only server-side support has been implemented, because that's what I'm
most familiar. However, we're almost all the way there to add
client-side support.
- ~[ ] [Smithy `document`
shapes](https://smithy.io/2.0/spec/simple-types.html#document) are not
supported. RPC v2's specification currently doesn't indicate how to
implement them.~
- [The
spec](https://smithy.io/2.0/additional-specs/protocols/smithy-rpc-v2.html#shape-serialization)
ended up leaving them as unsupported: "Document types are not currently
supported in this protocol."

## Prerequisite PRs

This section lists prerequisite PRs and issues that would make the diff
for this one lighter or easier to understand. It's preferable that these
PRs be merged prior to this one; some are hard prerequisites. They
mostly relate to parts of the codebase I've had to touch or ~pilfer~
inspect in this PR where I've made necessary changes, refactors and
"drive-by improvements" that are mostly unrelated, although some
directly unlock things I've needed in this patchset. It makes sense to
pull them out to ease reviewability and make this patch more
semantically self-contained.

- #2516
- #2517
- #2522
- #2524
- #2528
- #2536
- #2537
- #2531
- #2538
- #2539
- #2542
- #3684
- #3678
- #3690
- #3713
- #3726
- #3752

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->

~RPC v2 has still not been added to the Smithy specification. It is
currently being worked on over in the
[`smithy-rpc-v2`](https://github.com/awslabs/smithy/tree/smithy-rpc-v2)
branch.~

This can only be tested _locally_ following these steps:

~1. Clone [the Smithy
repository](https://github.com/smithy-lang/smithy/tree/smithy-rpc-v2)
and checkout the `smithy-rpc-v2` branch.
2. Inside your local checkout of smithy-rs pointing to this PR's branch,
make sure you've added `mavenLocal()` as a repository in the
`build.gradle.kts` files.
[Example](8df82fd).
4. Inside your local checkout of Smithy's `smithy-rpc-v2` branch:
1. Set `VERSION` to the current Smithy version used in smithy-rs (1.28.1
as of writing, but [check
here](https://github.com/awslabs/smithy-rs/blob/main/gradle.properties#L21)).
    2. Run `./gradlew clean build pTML`.~
~6.~ 1. In your local checkout of the smithy-rs's `smithy-rpc-v2`
branch, run `./gradlew codegen-server-test:build -P
modules='rpcv2Cbor'`.

~You can troubleshoot whether you have Smithy correctly set up locally
by inspecting
`~/.m2/repository/software/amazon/smithy/smithy-protocols-traits`.~

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [ ] 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
david-perez authored Jul 17, 2024
1 parent e4ced33 commit c63e792
Show file tree
Hide file tree
Showing 84 changed files with 4,479 additions and 246 deletions.
3 changes: 3 additions & 0 deletions .cargo-deny-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ exceptions = [
{ allow = ["OpenSSL"], name = "ring", version = "*" },
{ allow = ["OpenSSL"], name = "aws-lc-sys", version = "*" },
{ allow = ["OpenSSL"], name = "aws-lc-fips-sys", version = "*" },
{ allow = ["BlueOak-1.0.0"], name = "minicbor", version = "<=0.24.2" },
# Safe to bump as long as license does not change -------------^
# See D105255799.
]

[[licenses.clarify]]
Expand Down
2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ allprojects {
val allowLocalDeps: String by project
repositories {
if (allowLocalDeps.toBoolean()) {
mavenLocal()
mavenLocal()
}
mavenCentral()
google()
Expand Down
79 changes: 77 additions & 2 deletions buildSrc/src/main/kotlin/CodegenTestCommon.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,84 @@ fun generateImports(imports: List<String>): String =
if (imports.isEmpty()) {
""
} else {
"\"imports\": [${imports.map { "\"$it\"" }.joinToString(", ")}],"
"\"imports\": [${imports.joinToString(", ") { "\"$it\"" }}],"
}

val RustKeywords =
setOf(
"as",
"break",
"const",
"continue",
"crate",
"else",
"enum",
"extern",
"false",
"fn",
"for",
"if",
"impl",
"in",
"let",
"loop",
"match",
"mod",
"move",
"mut",
"pub",
"ref",
"return",
"self",
"Self",
"static",
"struct",
"super",
"trait",
"true",
"type",
"unsafe",
"use",
"where",
"while",
"async",
"await",
"dyn",
"abstract",
"become",
"box",
"do",
"final",
"macro",
"override",
"priv",
"typeof",
"unsized",
"virtual",
"yield",
"try",
)

fun toRustCrateName(input: String): String {
if (input.isBlank()) {
throw IllegalArgumentException("Rust crate name cannot be empty")
}
val lowerCased = input.lowercase()
// Replace any sequence of characters that are not lowercase letters, numbers, dashes, or underscores with a single underscore.
val sanitized = lowerCased.replace(Regex("[^a-z0-9_-]+"), "_")
// Trim leading or trailing underscores.
val trimmed = sanitized.trim('_')
// Check if the resulting string is empty, purely numeric, or a reserved name
val finalName =
when {
trimmed.isEmpty() -> throw IllegalArgumentException("Rust crate name after sanitizing cannot be empty.")
trimmed.matches(Regex("\\d+")) -> "n$trimmed" // Prepend 'n' if the name is purely numeric.
trimmed in RustKeywords -> "${trimmed}_" // Append an underscore if the name is reserved.
else -> trimmed
}
return finalName
}

private fun generateSmithyBuild(
projectDir: String,
pluginName: String,
Expand All @@ -48,7 +123,7 @@ private fun generateSmithyBuild(
${it.extraCodegenConfig ?: ""}
},
"service": "${it.service}",
"module": "${it.module}",
"module": "${toRustCrateName(it.module)}",
"moduleVersion": "0.0.1",
"moduleDescription": "test",
"moduleAuthors": ["[email protected]"]
Expand Down
1 change: 1 addition & 0 deletions buildSrc/src/main/kotlin/CrateSet.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ object CrateSet {
val SMITHY_RUNTIME_COMMON =
listOf(
"aws-smithy-async",
"aws-smithy-cbor",
"aws-smithy-checksums",
"aws-smithy-compression",
"aws-smithy-client",
Expand Down
3 changes: 2 additions & 1 deletion codegen-client/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ dependencies {
implementation("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-waiters:$smithyVersion")
implementation("software.amazon.smithy:smithy-rules-engine:$smithyVersion")
implementation("software.amazon.smithy:smithy-protocol-traits:$smithyVersion")

// `smithy.framework#ValidationException` is defined here, which is used in event stream
// marshalling/unmarshalling tests.
// marshalling/unmarshalling tests.
testImplementation("software.amazon.smithy:smithy-validation-model:$smithyVersion")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import software.amazon.smithy.rust.codegen.client.smithy.generators.error.ErrorC
import software.amazon.smithy.rust.codegen.core.rustlang.Writable
import software.amazon.smithy.rust.codegen.core.smithy.customize.CombinedCoreCodegenDecorator
import software.amazon.smithy.rust.codegen.core.smithy.customize.CoreCodegenDecorator
import software.amazon.smithy.rust.codegen.core.smithy.generators.protocol.ProtocolTestGenerator
import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolMap
import java.util.ServiceLoader
import java.util.logging.Logger
Expand Down Expand Up @@ -93,14 +92,6 @@ interface ClientCodegenDecorator : CoreCodegenDecorator<ClientCodegenContext, Cl
codegenContext: ClientCodegenContext,
baseCustomizations: List<ServiceRuntimePluginCustomization>,
): List<ServiceRuntimePluginCustomization> = baseCustomizations

/**
* Hook to override the protocol test generator
*/
fun protocolTestGenerator(
codegenContext: ClientCodegenContext,
baseGenerator: ProtocolTestGenerator,
): ProtocolTestGenerator = baseGenerator
}

/**
Expand Down Expand Up @@ -176,14 +167,6 @@ open class CombinedClientCodegenDecorator(decorators: List<ClientCodegenDecorato
decorator.serviceRuntimePluginCustomizations(codegenContext, customizations)
}

override fun protocolTestGenerator(
codegenContext: ClientCodegenContext,
baseGenerator: ProtocolTestGenerator,
): ProtocolTestGenerator =
combineCustomizations(baseGenerator) { decorator, gen ->
decorator.protocolTestGenerator(codegenContext, gen)
}

companion object {
fun fromClasspath(
context: PluginContext,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@ class ClientBuilderKindBehavior(val codegenContext: CodegenContext) : Instantiat
override fun doesSetterTakeInOption(memberShape: MemberShape): Boolean = true
}

class ClientInstantiator(private val codegenContext: ClientCodegenContext) : Instantiator(
class ClientInstantiator(private val codegenContext: ClientCodegenContext, withinTest: Boolean = false) : Instantiator(
codegenContext.symbolProvider,
codegenContext.model,
codegenContext.runtimeConfig,
ClientBuilderKindBehavior(codegenContext),
withinTest = false,
) {
fun renderFluentCall(
writer: RustWriter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class ClientProtocolTestGenerator(
get() = AppliesTo.CLIENT
override val expectFail: Set<FailingTest>
get() = ExpectFail
override val runOnly: Set<String>
override val generateOnly: Set<String>
get() = emptySet()
override val disabledTests: Set<String>
get() = emptySet()
Expand All @@ -128,7 +128,7 @@ class ClientProtocolTestGenerator(
private val inputShape = operationShape.inputShape(codegenContext.model)
private val outputShape = operationShape.outputShape(codegenContext.model)

private val instantiator = ClientInstantiator(codegenContext)
private val instantiator = ClientInstantiator(codegenContext, withinTest = true)

private val codegenScope =
arrayOf(
Expand All @@ -149,6 +149,8 @@ class ClientProtocolTestGenerator(
}

private fun RustWriter.renderHttpRequestTestCase(httpRequestTestCase: HttpRequestTestCase) {
logger.info("Generating request test: ${httpRequestTestCase.id}")

if (!protocolSupport.requestSerialization) {
rust("/* test case disabled for this protocol (not yet supported) */")
return
Expand Down Expand Up @@ -234,6 +236,8 @@ class ClientProtocolTestGenerator(
testCase: HttpResponseTestCase,
expectedShape: StructureShape,
) {
logger.info("Generating response test: ${testCase.id}")

if (!protocolSupport.responseDeserialization || (
!protocolSupport.errorDeserialization &&
expectedShape.hasTrait(
Expand Down Expand Up @@ -357,8 +361,8 @@ class ClientProtocolTestGenerator(
if (body == "") {
rustWriter.rustTemplate(
"""
// No body
#{AssertEq}(::std::str::from_utf8(body).unwrap(), "");
// No body.
#{AssertEq}(&body, &bytes::Bytes::new());
""",
*codegenScope,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import software.amazon.smithy.aws.traits.protocols.Ec2QueryTrait
import software.amazon.smithy.aws.traits.protocols.RestJson1Trait
import software.amazon.smithy.aws.traits.protocols.RestXmlTrait
import software.amazon.smithy.model.shapes.ServiceShape
import software.amazon.smithy.protocol.traits.Rpcv2CborTrait
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
import software.amazon.smithy.rust.codegen.client.smithy.generators.OperationGenerator
import software.amazon.smithy.rust.codegen.core.smithy.CodegenContext
Expand All @@ -28,6 +29,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolLoader
import software.amazon.smithy.rust.codegen.core.smithy.protocols.ProtocolMap
import software.amazon.smithy.rust.codegen.core.smithy.protocols.RestJson
import software.amazon.smithy.rust.codegen.core.smithy.protocols.RestXml
import software.amazon.smithy.rust.codegen.core.smithy.protocols.RpcV2Cbor
import software.amazon.smithy.rust.codegen.core.util.hasTrait

class ClientProtocolLoader(supportedProtocols: ProtocolMap<OperationGenerator, ClientCodegenContext>) :
Expand All @@ -41,6 +43,7 @@ class ClientProtocolLoader(supportedProtocols: ProtocolMap<OperationGenerator, C
Ec2QueryTrait.ID to ClientEc2QueryFactory(),
RestJson1Trait.ID to ClientRestJsonFactory(),
RestXmlTrait.ID to ClientRestXmlFactory(),
Rpcv2CborTrait.ID to ClientRpcV2CborFactory(),
)
val Default = ClientProtocolLoader(DefaultProtocols)
}
Expand Down Expand Up @@ -117,3 +120,12 @@ class ClientRestXmlFactory(

override fun support(): ProtocolSupport = CLIENT_PROTOCOL_SUPPORT
}

class ClientRpcV2CborFactory : ProtocolGeneratorFactory<OperationGenerator, ClientCodegenContext> {
override fun protocol(codegenContext: ClientCodegenContext): Protocol = RpcV2Cbor(codegenContext)

override fun buildProtocolGenerator(codegenContext: ClientCodegenContext): OperationGenerator =
OperationGenerator(codegenContext, protocol(codegenContext))

override fun support(): ProtocolSupport = CLIENT_PROTOCOL_SUPPORT
}
1 change: 1 addition & 0 deletions codegen-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ dependencies {
implementation("software.amazon.smithy:smithy-aws-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-protocol-test-traits:$smithyVersion")
implementation("software.amazon.smithy:smithy-waiters:$smithyVersion")
implementation("software.amazon.smithy:smithy-protocol-traits:$smithyVersion")
}

fun gitCommitHash(): String {
Expand Down
Loading

0 comments on commit c63e792

Please sign in to comment.