Unverified Commit 9570983e authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Fix `SDK::Endpoint` built-in (#2935)

Setting `endpoint_url` on a SDK Config currently has no effect due to a
bug in deciding if a parameter is a built-in or not in
EndpointBuiltinsDecorator. The generated code is placing the endpoint
URL into the ConfigBag with the correct
`aws_types::endpoint_config::EndpointUrl` type, but then the operation
runtime plugin is trying to pull it out of the ConfigBag with the
`crate::config::EndpointUrl` type, which always yields nothing.

Or, to illustrate with the generated code itself:
```
    pub fn set_endpoint_url(&mut self, endpoint_url: Option<::std::string::String>) -> &mut Self {
        self.config.store_or_unset(endpoint_url.map(::aws_types::endpoint_config::EndpointUrl));
        self
    }
```
vs.
```
let params = crate::config::endpoint::Params::builder()
            .set_endpoint(cfg.load::<crate::config::EndpointUrl>().map(|ty| ty.0.clone()))
```

This PR adds a unit test that reproduces the issue, and then fixes it.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent c742b5f6
Loading
Loading
Loading
Loading
+7 −2
Original line number Diff line number Diff line
@@ -57,7 +57,9 @@ fun ClientCodegenContext.getBuiltIn(builtIn: String): Parameter? {
}

private fun promotedBuiltins(parameter: Parameter) =
    parameter == Builtins.FIPS || parameter == Builtins.DUALSTACK || parameter == Builtins.SDK_ENDPOINT
    parameter.builtIn == Builtins.FIPS.builtIn ||
        parameter.builtIn == Builtins.DUALSTACK.builtIn ||
        parameter.builtIn == Builtins.SDK_ENDPOINT.builtIn

private fun configParamNewtype(parameter: Parameter, name: String, runtimeConfig: RuntimeConfig): RuntimeType {
    val type = parameter.symbol().mapRustType { t -> t.stripOuter<RustType.Option>() }
@@ -204,6 +206,9 @@ val PromotedBuiltInsDecorators =
        decoratorForBuiltIn(Builtins.DUALSTACK),
        decoratorForBuiltIn(
            Builtins.SDK_ENDPOINT,
            ConfigParam.Builder().name("endpoint_url").type(RuntimeType.String.toSymbol()).setterDocs(endpointUrlDocs),
            ConfigParam.Builder()
                .name("endpoint_url")
                .type(RuntimeType.String.toSymbol())
                .setterDocs(endpointUrlDocs),
        ),
    ).toTypedArray()
+119 −0
Original line number Diff line number Diff line
/*
 * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 * SPDX-License-Identifier: Apache-2.0
 */

package software.amazon.smithy.rustsdk

import org.junit.jupiter.api.Test
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
import software.amazon.smithy.rust.codegen.core.testutil.integrationTest

class EndpointBuiltInsDecoratorTest {
    private val endpointUrlModel = """
        namespace test

        use aws.api#service
        use aws.auth#sigv4
        use aws.protocols#restJson1
        use smithy.rules#endpointRuleSet

        @service(sdkId: "dontcare")
        @restJson1
        @sigv4(name: "dontcare")
        @auth([sigv4])
        @endpointRuleSet({
            "version": "1.0"
            "parameters": {
                "endpoint": { "required": false, "type": "string", "builtIn": "SDK::Endpoint" },
                "region": { "required": false, "type": "String", "builtIn": "AWS::Region" },
            }
            "rules": [
                {
                    "type": "endpoint"
                    "conditions": [
                        {"fn": "isSet", "argv": [{"ref": "endpoint"}]},
                        {"fn": "isSet", "argv": [{"ref": "region"}]}
                    ],
                    "endpoint": {
                        "url": "{endpoint}"
                        "properties": {
                            "authSchemes": [{"name": "sigv4","signingRegion": "{region}"}]
                        }
                    }
                },
                {
                    "type": "endpoint"
                    "conditions": [
                        {"fn": "isSet", "argv": [{"ref": "region"}]},
                    ],
                    "endpoint": {
                        "url": "https://WRONG/"
                        "properties": {
                            "authSchemes": [{"name": "sigv4", "signingRegion": "{region}"}]
                        }
                    }
                }
            ]
        })
        service TestService {
            version: "2023-01-01",
            operations: [SomeOperation]
        }

        structure SomeOutput {
            someAttribute: Long,
            someVal: String
        }

        @http(uri: "/SomeOperation", method: "GET")
        @optionalAuth
        operation SomeOperation {
            output: SomeOutput
        }
    """.asSmithyModel()

    @Test
    fun endpointUrlBuiltInWorksEndToEnd() {
        awsSdkIntegrationTest(
            endpointUrlModel,
            generateOrchestrator = true,
        ) { codegenContext, rustCrate ->
            rustCrate.integrationTest("endpoint_url_built_in_works") {
                val module = codegenContext.moduleUseName()
                rustTemplate(
                    """
                    use $module::{Config, Client, config::Region};

                    ##[#{tokio}::test]
                    async fn endpoint_url_built_in_works() {
                        let connector = #{TestConnection}::new(vec![(
                            http::Request::builder()
                                .uri("https://RIGHT/SomeOperation")
                                .body(#{SdkBody}::empty())
                                .unwrap(),
                            http::Response::builder().status(200).body("").unwrap(),
                        )]);
                        let config = Config::builder()
                            .http_connector(connector.clone())
                            .region(Region::new("us-east-1"))
                            .endpoint_url("https://RIGHT")
                            .build();
                        let client = Client::from_conf(config);
                        dbg!(client.some_operation().send().await).expect("success");
                        connector.assert_requests_match(&[]);
                    }
                    """,
                    "tokio" to CargoDependency.Tokio.toDevDependency().withFeature("rt").withFeature("macros").toType(),
                    "TestConnection" to CargoDependency.smithyClient(codegenContext.runtimeConfig)
                        .toDevDependency().withFeature("test-util").toType()
                        .resolve("test_connection::TestConnection"),
                    "SdkBody" to RuntimeType.sdkBody(codegenContext.runtimeConfig),
                )
            }
        }
    }
}
+2 −0
Original line number Diff line number Diff line
@@ -6,6 +6,7 @@
package software.amazon.smithy.rustsdk

import software.amazon.smithy.model.Model
import software.amazon.smithy.model.node.BooleanNode
import software.amazon.smithy.model.node.ObjectNode
import software.amazon.smithy.model.node.StringNode
import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext
@@ -65,6 +66,7 @@ fun awsSdkIntegrationTest(
                        .withMember("includeFluentClient", false)
                        .letIf(generateOrchestrator) {
                            it.withMember("enableNewSmithyRuntime", StringNode.from("orchestrator"))
                                .withMember("includeEndpointUrlConfig", BooleanNode.from(false))
                        }
                        .build(),
                ).build(),