From 2ab329d24ed9d02d98256911cb09b7ad3e9b50a3 Mon Sep 17 00:00:00 2001 From: Fahad Zubair Date: Thu, 22 May 2025 12:45:39 +0000 Subject: [PATCH 1/3] Upgrade pyo3 to 0.20 --- .../smithy/PythonServerCargoDependency.kt | 4 +- .../generators/PythonServerModuleGenerator.kt | 2 +- .../PythonServerStructureGenerator.kt | 15 +- ...PythonServerRequiredPrecedeOptionalTest.kt | 178 ++++++++++++++++++ .../aws-smithy-http-server-python/Cargo.toml | 8 +- .../src/context/layer.rs | 12 +- .../src/context/testing.rs | 1 + .../src/error.rs | 2 +- .../src/logging.rs | 2 +- .../src/middleware/pytests/layer.rs | 1 + .../src/middleware/pytests/response.rs | 7 +- .../src/middleware/response.rs | 2 +- .../src/socket.rs | 3 +- .../aws-smithy-http-server-python/src/tls.rs | 12 +- .../aws-smithy-http-server-python/src/util.rs | 32 +++- 15 files changed, 255 insertions(+), 26 deletions(-) create mode 100644 codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCargoDependency.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCargoDependency.kt index 2e8070d15e3..22c1ac0d98a 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCargoDependency.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/PythonServerCargoDependency.kt @@ -15,9 +15,9 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig * For a dependency that is used in the client, or in both the client and the server, use [CargoDependency] directly. */ object PythonServerCargoDependency { - val PyO3: CargoDependency = CargoDependency("pyo3", CratesIo("0.18")) + val PyO3: CargoDependency = CargoDependency("pyo3", CratesIo("0.20")) val PyO3Asyncio: CargoDependency = - CargoDependency("pyo3-asyncio", CratesIo("0.18"), features = setOf("attributes", "tokio-runtime")) + CargoDependency("pyo3-asyncio", CratesIo("0.20"), features = setOf("attributes", "tokio-runtime")) val Tokio: CargoDependency = CargoDependency("tokio", CratesIo("1.20.1"), features = setOf("full")) val TokioStream: CargoDependency = CargoDependency("tokio-stream", CratesIo("0.1.12")) val Tracing: CargoDependency = CargoDependency("tracing", CratesIo("0.1")) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt index d8dc07f0e1f..5635ac63053 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt @@ -41,7 +41,7 @@ class PythonServerModuleGenerator( rustBlockTemplate( """ ##[#{pyo3}::pymodule] - ##[#{pyo3}(name = "$libName")] + ##[pyo3(name = "$libName")] pub fn python_library(py: #{pyo3}::Python<'_>, m: &#{pyo3}::types::PyModule) -> #{pyo3}::PyResult<()> """, *codegenScope, diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerStructureGenerator.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerStructureGenerator.kt index 3a78b0ddd05..a7e67877d29 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerStructureGenerator.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerStructureGenerator.kt @@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustInlineTemplate import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator +import software.amazon.smithy.rust.codegen.core.smithy.isOptional import software.amazon.smithy.rust.codegen.core.smithy.rustType import software.amazon.smithy.rust.codegen.core.util.hasTrait import software.amazon.smithy.rust.codegen.core.util.isEventStream @@ -125,9 +126,17 @@ class PythonServerStructureGenerator( ) } + // Python function parameters require that all required parameters appear before optional ones. + // This function sorts the member fields to ensure required fields precede optional fields. + private fun sortedMembers() = + members.sortedBy { member -> + val memberSymbol = symbolProvider.toSymbol(member) + memberSymbol.isOptional() + } + private fun renderStructSignatureMembers(): Writable = writable { - forEachMember(members) { _, memberName, memberSymbol -> + forEachMember(sortedMembers()) { _, memberName, memberSymbol -> val memberType = memberSymbol.rustType() rust("$memberName: ${memberType.render()},") } @@ -135,14 +144,14 @@ class PythonServerStructureGenerator( private fun renderStructBodyMembers(): Writable = writable { - forEachMember(members) { _, memberName, _ -> + forEachMember(sortedMembers()) { _, memberName, _ -> rust("$memberName,") } } private fun renderConstructorSignature(): Writable = writable { - forEachMember(members) { member, memberName, memberSymbol -> + forEachMember(sortedMembers()) { member, memberName, memberSymbol -> val memberType = memberPythonType(member, memberSymbol) rust("/// :param $memberName ${memberType.renderAsDocstring()}:") } diff --git a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt new file mode 100644 index 00000000000..80ffef62519 --- /dev/null +++ b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt @@ -0,0 +1,178 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.rust.codegen.server.python.smithy.generators + +import org.junit.jupiter.api.Test +import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter +import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel +import software.amazon.smithy.rust.codegen.core.testutil.unitTest +import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.cargoTest +import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.executePythonServerCodegenVisitor +import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.generatePythonServerPluginContext +import kotlin.io.path.appendText + +internal class PythonServerRequiredPrecedeOptionalTest { + @Test + fun `mandatory fields are reordered to be before optional`() { + val model = + """ + namespace test + + use aws.protocols#restJson1 + use smithy.framework#ValidationException + + @restJson1 + service SampleService { + operations: [ + OpWithIncorrectOrder, OpWithCorrectOrder, OpWithDefaults + ], + } + + @http(method: "POST", uri: "/opIncorrect") + operation OpWithIncorrectOrder { + input:= { + a: String + @required + b: String + c: String + @required + d: String + } + output:= { + a: String + @required + b: String + c: String + @required + d: String + } + errors: [ValidationException] + } + + @http(method: "POST", uri: "/opCorrect") + operation OpWithCorrectOrder { + input:= { + @required + b: String + @required + d: String + a: String + c: String + } + output:= { + @required + b: String + @required + d: String + a: String + c: String + } + errors: [ValidationException] + } + + @http(method: "POST", uri: "/opWithDefaults") + operation OpWithDefaults { + input:= { + a: String, + b: String = "hi" + } + } + """.asSmithyModel(smithyVersion = "2") + + val (pluginCtx, testDir) = generatePythonServerPluginContext(model) + executePythonServerCodegenVisitor(pluginCtx) + + val writer = RustWriter.forModule("service") + writer.unitTest("test_required_fields") { + rust( + """ + use crate::input; + use pyo3::{types::IntoPyDict, Python}; + + pyo3::prepare_freethreaded_python(); + Python::with_gil(|py| { + let globals = [ + ( + "OpWithIncorrectOrderInput", + py.get_type::(), + ), + ( + "OpWithCorrectOrderInput", + py.get_type::(), + ), + ( + "OpWithDefaultsInput", + py.get_type::(), + )] + .into_py_dict(py); + let locals = [( + "OpWithIncorrectOrderInput", + py.get_type::(), + )] + .into_py_dict(py); + + py.run( + "input = OpWithIncorrectOrderInput(\"b\", \"d\")", + Some(globals), + Some(locals), + ) + .unwrap(); + + // Python should have been able to construct input. + let input = locals + .get_item("input") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .extract::() + .unwrap(); + assert_eq!(input.b, "b"); + assert_eq!(input.d, "d"); + + py.run( + "input = OpWithCorrectOrderInput(\"b\", \"d\")", + Some(globals), + Some(locals), + ) + .unwrap(); + + // Python should have been able to construct input. + let input = locals + .get_item("input") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .extract::() + .unwrap(); + assert_eq!(input.b, "b"); + assert_eq!(input.d, "d"); + + py.run( + "input = OpWithDefaultsInput(\"a\")", + Some(globals), + Some(locals), + ) + .unwrap(); + + // KanchaBilla + // Python should have been able to construct input. + let input = locals + .get_item("input") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .extract::() + .unwrap(); + assert_eq!(input.a, Some("a".to_string())); + assert_eq!(input.b, "hi"); + }); + """, + ) + } + + testDir.resolve("src/service.rs").appendText(writer.toString()) + + cargoTest(testDir) + } +} diff --git a/rust-runtime/aws-smithy-http-server-python/Cargo.toml b/rust-runtime/aws-smithy-http-server-python/Cargo.toml index 779772dddb4..495421fa2ac 100644 --- a/rust-runtime/aws-smithy-http-server-python/Cargo.toml +++ b/rust-runtime/aws-smithy-http-server-python/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "aws-smithy-http-server-python" -version = "0.65.0" +version = "0.66.0" authors = ["Smithy Rust Server "] edition = "2021" license = "Apache-2.0" @@ -29,8 +29,8 @@ lambda_http = { version = "0.8.3" } num_cpus = "1.13.1" parking_lot = "0.12.1" pin-project-lite = "0.2.14" -pyo3 = "0.18.2" -pyo3-asyncio = { version = "0.18.0", features = ["tokio-runtime"] } +pyo3 = "0.20" +pyo3-asyncio = { version = "0.20.0", features = ["tokio-runtime"] } signal-hook = { version = "0.3.14", features = ["extended-siginfo"] } socket2 = { version = "0.5.5", features = ["all"] } thiserror = "1.0.40" @@ -46,7 +46,7 @@ pretty_assertions = "1" futures-util = { version = "0.3.29", default-features = false } tower-test = "0.4" tokio-test = "0.4" -pyo3-asyncio = { version = "0.18.0", features = ["testing", "attributes", "tokio-runtime", "unstable-streams"] } +pyo3-asyncio = { version = "0.20.0", features = ["testing", "attributes", "tokio-runtime", "unstable-streams"] } rcgen = "0.10.0" hyper-rustls = { version = "0.24", features = ["http2"] } diff --git a/rust-runtime/aws-smithy-http-server-python/src/context/layer.rs b/rust-runtime/aws-smithy-http-server-python/src/context/layer.rs index 281dadaf971..ea1ab7e8e0c 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/context/layer.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/context/layer.rs @@ -107,8 +107,16 @@ counter = ctx.counter .unwrap(); ( - locals.get_item("req_id").unwrap().to_string(), - locals.get_item("counter").unwrap().to_string(), + locals + .get_item("req_id") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .to_string(), + locals + .get_item("counter") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .to_string(), ) }); Ok::<_, Infallible>(Response::new((req_id, counter))) diff --git a/rust-runtime/aws-smithy-http-server-python/src/context/testing.rs b/rust-runtime/aws-smithy-http-server-python/src/context/testing.rs index 31d18d4d10c..b0e05c98658 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/context/testing.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/context/testing.rs @@ -25,6 +25,7 @@ pub fn get_context(code: &str) -> PyContext { py.run(code, Some(globals), Some(locals))?; let context = locals .get_item("ctx") + .expect("Python exception occurred during dictionary lookup") .expect("you should assing your context class to `ctx` variable") .into_py(py); Ok::<_, PyErr>(context) diff --git a/rust-runtime/aws-smithy-http-server-python/src/error.rs b/rust-runtime/aws-smithy-http-server-python/src/error.rs index 92e095d1c08..081d4fec278 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/error.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/error.rs @@ -44,7 +44,6 @@ impl From for PyErr { /// :param status_code typing.Optional\[int\]: /// :rtype None: #[pyclass(name = "MiddlewareException", extends = BasePyException)] -#[pyo3(text_signature = "($self, message, status_code=None)")] #[derive(Debug, Clone)] pub struct PyMiddlewareException { /// :type str: @@ -59,6 +58,7 @@ pub struct PyMiddlewareException { #[pymethods] impl PyMiddlewareException { /// Create a new [PyMiddlewareException]. + #[pyo3(text_signature = "($self, message, status_code=None)")] #[new] fn newpy(message: String, status_code: Option) -> Self { Self { diff --git a/rust-runtime/aws-smithy-http-server-python/src/logging.rs b/rust-runtime/aws-smithy-http-server-python/src/logging.rs index 57fa2873aa2..fe3b40f3108 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/logging.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/logging.rs @@ -127,7 +127,6 @@ fn setup_tracing_subscriber( /// :param format typing.Optional\[typing.Literal\['compact', 'pretty', 'json'\]\]: /// :rtype None: #[pyclass(name = "TracingHandler")] -#[pyo3(text_signature = "($self, level=None, logfile=None, format=None)")] #[derive(Debug)] pub struct PyTracingHandler { _guard: Option, @@ -135,6 +134,7 @@ pub struct PyTracingHandler { #[pymethods] impl PyTracingHandler { + #[pyo3(text_signature = "($self, level=None, logfile=None, format=None)")] #[new] fn newpy( py: Python, diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/layer.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/layer.rs index ead40320adf..ae267821287 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/layer.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/layer.rs @@ -321,6 +321,7 @@ fn py_handler(code: &str) -> PyMiddlewareHandler { py.run(code, Some(globals), Some(locals))?; let handler = locals .get_item("middleware") + .expect("Python exception occurred during dictionary lookup") .expect("your handler must be named `middleware`") .into(); PyMiddlewareHandler::new(py, handler) diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/response.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/response.rs index e9f1a480668..43b50a5fdab 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/response.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/pytests/response.rs @@ -27,7 +27,12 @@ response = Response(200, {"Content-Type": "application/json"}, b"hello world") ) .unwrap(); - let py_response: Py = locals.get_item("response").unwrap().extract().unwrap(); + let py_response: Py = locals + .get_item("response") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .extract() + .unwrap(); let response = py_response.borrow_mut(py).take_inner(); response.unwrap() }); diff --git a/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs b/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs index 2e97af5e49b..263d2689aae 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/middleware/response.rs @@ -23,7 +23,6 @@ use super::{PyHeaderMap, PyMiddlewareError}; /// :param body typing.Optional[bytes]: /// :rtype None: #[pyclass(name = "Response")] -#[pyo3(text_signature = "($self, status, headers=None, body=None)")] pub struct PyResponse { parts: Option, headers: PyHeaderMap, @@ -61,6 +60,7 @@ impl PyResponse { #[pymethods] impl PyResponse { /// Python-compatible [Response] object from the Python side. + #[pyo3(text_signature = "($self, status, headers=None, body=None)")] #[new] fn newpy( status: u16, diff --git a/rust-runtime/aws-smithy-http-server-python/src/socket.rs b/rust-runtime/aws-smithy-http-server-python/src/socket.rs index 13900ff8c8f..fd0b73f6720 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/socket.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/socket.rs @@ -25,7 +25,7 @@ use std::net::SocketAddr; /// :param port int: /// :param backlog typing.Optional\[int\]: /// :rtype None: -#[pyclass(text_signature = "($self, address, port, backlog=None)")] +#[pyclass] #[derive(Debug)] pub struct PySocket { pub(crate) inner: Socket, @@ -35,6 +35,7 @@ pub struct PySocket { impl PySocket { /// Create a new UNIX `SharedSocket` from an address, port and backlog. /// If not specified, the backlog defaults to 1024 connections. + #[pyo3(text_signature = "($self, address, port, backlog=None)")] #[new] pub fn new(address: String, port: i32, backlog: Option) -> PyResult { let address: SocketAddr = format!("{}:{}", address, port).parse()?; diff --git a/rust-runtime/aws-smithy-http-server-python/src/tls.rs b/rust-runtime/aws-smithy-http-server-python/src/tls.rs index c817cabacae..4a5f989323b 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/tls.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/tls.rs @@ -25,10 +25,7 @@ pub mod listener; /// :param cert_path pathlib.Path: /// :param reload_secs int: /// :rtype None: -#[pyclass( - name = "TlsConfig", - text_signature = "($self, *, key_path, cert_path, reload_secs=86400)" -)] +#[pyclass(name = "TlsConfig")] #[derive(Clone)] pub struct PyTlsConfig { /// Absolute path of the RSA or PKCS private key. @@ -105,6 +102,7 @@ impl PyTlsConfig { #[pymethods] impl PyTlsConfig { #[new] + #[pyo3(text_signature = "($self, *, key_path, cert_path, reload_secs=86400)")] #[pyo3(signature = (key_path, cert_path, reload_secs=86400))] fn py_new(key_path: PathBuf, cert_path: PathBuf, reload_secs: u64) -> Self { // TODO(BugOnUpstream): `reload: &PyDelta` segfaults, create an issue on PyO3 @@ -172,7 +170,11 @@ config = TlsConfig(key_path=TEST_KEY, cert_path=TEST_CERT, reload_secs=1000) Some(globals), Some(locals), )?; - locals.get_item("config").unwrap().extract::() + locals + .get_item("config") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + .extract::() })?; assert_eq!(PathBuf::from_str(TEST_KEY).unwrap(), config.key_path); diff --git a/rust-runtime/aws-smithy-http-server-python/src/util.rs b/rust-runtime/aws-smithy-http-server-python/src/util.rs index 4cac7f06d27..81c99001eab 100644 --- a/rust-runtime/aws-smithy-http-server-python/src/util.rs +++ b/rust-runtime/aws-smithy-http-server-python/src/util.rs @@ -158,19 +158,43 @@ class Types: assert_eq!( true, - is_optional_of::(py, type_hints.get_item("opt_of_str").unwrap())? + is_optional_of::( + py, + type_hints + .get_item("opt_of_str") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + )? ); assert_eq!( false, - is_optional_of::(py, type_hints.get_item("regular_str").unwrap())? + is_optional_of::( + py, + type_hints + .get_item("regular_str") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + )? ); assert_eq!( true, - is_optional_of::(py, type_hints.get_item("opt_of_bool").unwrap())? + is_optional_of::( + py, + type_hints + .get_item("opt_of_bool") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + )? ); assert_eq!( false, - is_optional_of::(py, type_hints.get_item("opt_of_bool").unwrap())? + is_optional_of::( + py, + type_hints + .get_item("opt_of_bool") + .expect("Python exception occurred during dictionary lookup") + .unwrap() + )? ); Ok(()) From aa5c29a987f7f91566c4691a6049ccdb8d69ba9a Mon Sep 17 00:00:00 2001 From: Fahad Zubair Date: Thu, 22 May 2025 16:29:30 +0100 Subject: [PATCH 2/3] Add tests for output --- ...PythonServerRequiredPrecedeOptionalTest.kt | 154 ++++++++++++------ .../generators/PythonServerTypesTest.kt | 2 + 2 files changed, 102 insertions(+), 54 deletions(-) diff --git a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt index 80ffef62519..779dd160849 100644 --- a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt +++ b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt @@ -8,8 +8,11 @@ package software.amazon.smithy.rust.codegen.server.python.smithy.generators import org.junit.jupiter.api.Test import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter import software.amazon.smithy.rust.codegen.core.rustlang.rust +import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate +import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel import software.amazon.smithy.rust.codegen.core.testutil.unitTest +import software.amazon.smithy.rust.codegen.server.python.smithy.PythonServerCargoDependency import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.cargoTest import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.executePythonServerCodegenVisitor import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.generatePythonServerPluginContext @@ -77,8 +80,12 @@ internal class PythonServerRequiredPrecedeOptionalTest { @http(method: "POST", uri: "/opWithDefaults") operation OpWithDefaults { input:= { - a: String, - b: String = "hi" + a: String, + b: String = "hi" + } + output:= { + a: String, + b: String = "hi" } } """.asSmithyModel(smithyVersion = "2") @@ -88,86 +95,125 @@ internal class PythonServerRequiredPrecedeOptionalTest { val writer = RustWriter.forModule("service") writer.unitTest("test_required_fields") { - rust( - """ - use crate::input; - use pyo3::{types::IntoPyDict, Python}; - - pyo3::prepare_freethreaded_python(); - Python::with_gil(|py| { - let globals = [ - ( - "OpWithIncorrectOrderInput", - py.get_type::(), - ), - ( - "OpWithCorrectOrderInput", - py.get_type::(), - ), - ( - "OpWithDefaultsInput", - py.get_type::(), - )] - .into_py_dict(py); - let locals = [( - "OpWithIncorrectOrderInput", - py.get_type::(), - )] - .into_py_dict(py); - + fun createInstanceWithRequiredFieldsOnly( + module: String, + typeName: String, + ) = writable { + rustTemplate( + """ py.run( - "input = OpWithIncorrectOrderInput(\"b\", \"d\")", + "data = $typeName(\"b\", \"d\")", Some(globals), Some(locals), - ) - .unwrap(); + ).unwrap(); // Python should have been able to construct input. - let input = locals - .get_item("input") + let data = locals + .get_item("data") .expect("Python exception occurred during dictionary lookup") .unwrap() - .extract::() + .extract::<$module::$typeName>() .unwrap(); - assert_eq!(input.b, "b"); - assert_eq!(input.d, "d"); + assert_eq!(data.b, "b"); + assert_eq!(data.d, "d"); + """, + ) + } + fun createInstance( + module: String, + typeName: String, + ) = writable { + rustTemplate( + """ py.run( - "input = OpWithCorrectOrderInput(\"b\", \"d\")", + "data = $typeName(\"b\", \"d\", a = \"a\", c = \"c\")", Some(globals), Some(locals), - ) - .unwrap(); + ).unwrap(); // Python should have been able to construct input. - let input = locals - .get_item("input") + let data = locals + .get_item("data") .expect("Python exception occurred during dictionary lookup") .unwrap() - .extract::() + .extract::<$module::$typeName>() .unwrap(); - assert_eq!(input.b, "b"); - assert_eq!(input.d, "d"); + assert_eq!(data.b, "b"); + assert_eq!(data.d, "d"); + assert_eq!(data.a, Some("a".to_string())); + assert_eq!(data.c, Some("c".to_string())); + """, + ) + } + fun createDefaultInstance( + module: String, + typeName: String, + ) = writable { + rustTemplate( + """ py.run( - "input = OpWithDefaultsInput(\"a\")", + "data = $typeName(\"a\")", Some(globals), Some(locals), - ) - .unwrap(); + ).unwrap(); - // KanchaBilla // Python should have been able to construct input. - let input = locals - .get_item("input") + let data = locals + .get_item("data") .expect("Python exception occurred during dictionary lookup") .unwrap() - .extract::() + .extract::<$module::$typeName>() .unwrap(); - assert_eq!(input.a, Some("a".to_string())); - assert_eq!(input.b, "hi"); + assert_eq!(data.a, Some("a".to_string())); + assert_eq!(data.b, "hi"); + """, + ) + } + + rustTemplate( + """ + use crate::{input, output}; + use #{pyo3}::{types::IntoPyDict, Python}; + + pyo3::prepare_freethreaded_python(); + Python::with_gil(|py| { + let globals = [ + ("OpWithIncorrectOrderInput", py.get_type::()), + ("OpWithCorrectOrderInput", py.get_type::()), + ("OpWithDefaultsInput", py.get_type::()), + ("OpWithIncorrectOrderOutput", py.get_type::()), + ("OpWithCorrectOrderOutput", py.get_type::()), + ("OpWithDefaultsOutput", py.get_type::()) + ] + .into_py_dict(py); + + let locals = [("OpWithIncorrectOrderInput", py.get_type::())].into_py_dict(py); + + #{IncorrectOrderInputRequiredOnly} + #{CorrectOrderInputRequiredOnly} + #{IncorrectOrderOutputRequiredOnly} + #{CorrectOrderOutputRequiredOnly} + #{IncorrectOrderInput} + #{CorrectOrderInput} + #{IncorrectOrderOutput} + #{CorrectOrderOutput} + #{DefaultsInput} + #{DefaultsOutput} }); """, + "pyo3" to PythonServerCargoDependency.PyO3.toDevDependency().toType(), + "IncorrectOrderInputRequiredOnly" to createInstanceWithRequiredFieldsOnly("input", "OpWithIncorrectOrderInput"), + "CorrectOrderInputRequiredOnly" to createInstanceWithRequiredFieldsOnly("input", "OpWithCorrectOrderInput"), + "IncorrectOrderOutputRequiredOnly" to createInstanceWithRequiredFieldsOnly("output", "OpWithIncorrectOrderOutput"), + "CorrectOrderOutputRequiredOnly" to createInstanceWithRequiredFieldsOnly("output", "OpWithCorrectOrderOutput"), + "IncorrectOrderInput" to createInstance("input", "OpWithIncorrectOrderInput"), + "CorrectOrderInput" to createInstance("input", "OpWithCorrectOrderInput"), + "IncorrectOrderOutput" to createInstance("output", "OpWithIncorrectOrderOutput"), + "CorrectOrderOutput" to createInstance("output", "OpWithCorrectOrderOutput"), + "DefaultsInput" to createDefaultInstance("input", "OpWithDefaultsInput"), + "DefaultsOutput" to createDefaultInstance("output", "OpWithDefaultsOutput"), ) } diff --git a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt index 02c93347eb6..425f5c3b3d0 100644 --- a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt +++ b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt @@ -119,6 +119,7 @@ internal class PythonServerTypesTest { locals .get_item("output") + .expect("Python exception occurred during dictionary lookup") .unwrap() .extract::() .unwrap() @@ -212,6 +213,7 @@ internal class PythonServerTypesTest { locals .get_item("output") + .expect("Python exception occurred during dictionary lookup") .unwrap() .extract::() .unwrap() From a275b94d8e521cbb8ca1d32a6b69b269be25b60e Mon Sep 17 00:00:00 2001 From: Fahad Zubair Date: Thu, 22 May 2025 18:54:57 +0100 Subject: [PATCH 3/3] Fix default value test --- .../generators/PythonServerRequiredPrecedeOptionalTest.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt index 779dd160849..d604c3cadcc 100644 --- a/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt +++ b/codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt @@ -153,8 +153,10 @@ internal class PythonServerRequiredPrecedeOptionalTest { ) = writable { rustTemplate( """ + // Default values are not exported from Rust. However, they + // are marked as non-optional. py.run( - "data = $typeName(\"a\")", + "data = $typeName(\"b\", \"a\")", Some(globals), Some(locals), ).unwrap(); @@ -167,7 +169,7 @@ internal class PythonServerRequiredPrecedeOptionalTest { .extract::<$module::$typeName>() .unwrap(); assert_eq!(data.a, Some("a".to_string())); - assert_eq!(data.b, "hi"); + assert_eq!(data.b, "b"); """, ) }