Skip to content

Commit ff0b532

Browse files
author
Fahad Zubair
committed
Python: Required fields should precede optional
1 parent 3991879 commit ff0b532

File tree

8 files changed

+229
-29
lines changed

8 files changed

+229
-29
lines changed

‎codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/CoreRustSettings.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ open class CoreCodegenConfig(
4646
) {
4747
companion object {
4848
const val DEFAULT_FORMAT_TIMEOUT_SECONDS = 20
49-
const val DEFAULT_DEBUG_MODE = false
49+
const val DEFAULT_DEBUG_MODE = true
5050
const val DEFAULT_FLATTEN_MODE = false
5151

5252
fun fromNode(node: Optional<ObjectNode>): CoreCodegenConfig =

‎codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerModuleGenerator.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class PythonServerModuleGenerator(
4141
rustBlockTemplate(
4242
"""
4343
##[#{pyo3}::pymodule]
44-
##[#{pyo3}(name = "$libName")]
44+
##[pyo3(name = "$libName")]
4545
pub fn python_library(py: #{pyo3}::Python<'_>, m: &#{pyo3}::types::PyModule) -> #{pyo3}::PyResult<()>
4646
""",
4747
*codegenScope,

‎codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerStructureGenerator.kt

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rustInlineTemplate
1919
import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate
2020
import software.amazon.smithy.rust.codegen.core.rustlang.writable
2121
import software.amazon.smithy.rust.codegen.core.smithy.generators.StructureGenerator
22+
import software.amazon.smithy.rust.codegen.core.smithy.isOptional
2223
import software.amazon.smithy.rust.codegen.core.smithy.rustType
2324
import software.amazon.smithy.rust.codegen.core.util.hasTrait
2425
import software.amazon.smithy.rust.codegen.core.util.isEventStream
@@ -125,24 +126,32 @@ class PythonServerStructureGenerator(
125126
)
126127
}
127128

129+
// Python function parameters require that all required parameters appear before optional ones.
130+
// This function sorts the member fields to ensure required fields precede optional fields.
131+
private fun sortedMembers() =
132+
members.sortedBy { member ->
133+
val memberSymbol = symbolProvider.toSymbol(member)
134+
memberSymbol.isOptional()
135+
}
136+
128137
private fun renderStructSignatureMembers(): Writable =
129138
writable {
130-
forEachMember(members) { _, memberName, memberSymbol ->
139+
forEachMember(sortedMembers()) { _, memberName, memberSymbol ->
131140
val memberType = memberSymbol.rustType()
132141
rust("$memberName: ${memberType.render()},")
133142
}
134143
}
135144

136145
private fun renderStructBodyMembers(): Writable =
137146
writable {
138-
forEachMember(members) { _, memberName, _ ->
147+
forEachMember(sortedMembers()) { _, memberName, _ ->
139148
rust("$memberName,")
140149
}
141150
}
142151

143152
private fun renderConstructorSignature(): Writable =
144153
writable {
145-
forEachMember(members) { member, memberName, memberSymbol ->
154+
forEachMember(sortedMembers()) { member, memberName, memberSymbol ->
146155
val memberType = memberPythonType(member, memberSymbol)
147156
rust("/// :param $memberName ${memberType.renderAsDocstring()}:")
148157
}

‎codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerRequiredPrecedeOptionalTest.kt

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
package software.amazon.smithy.rust.codegen.server.python.smithy.generators
2+
3+
import org.junit.jupiter.api.Test
4+
import software.amazon.smithy.rust.codegen.core.rustlang.RustWriter
5+
import software.amazon.smithy.rust.codegen.core.rustlang.rust
6+
import software.amazon.smithy.rust.codegen.core.testutil.asSmithyModel
7+
import software.amazon.smithy.rust.codegen.core.testutil.unitTest
8+
import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.cargoTest
9+
import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.executePythonServerCodegenVisitor
10+
import software.amazon.smithy.rust.codegen.server.python.smithy.testutil.generatePythonServerPluginContext
11+
import kotlin.io.path.appendText
12+
13+
internal class PythonServerRequiredPrecedeOptionalTest {
14+
@Test
15+
fun `mandatory fields are reordered to be before optional`() {
16+
val model =
17+
"""
18+
namespace test
19+
20+
use aws.protocols#restJson1
21+
use smithy.framework#ValidationException
22+
23+
@restJson1
24+
service SampleService {
25+
operations: [
26+
OpWithIncorrectOrder, OpWithCorrectOrder, OpWithDefaults
27+
],
28+
}
29+
30+
@http(method: "POST", uri: "/opIncorrect")
31+
operation OpWithIncorrectOrder {
32+
input:= {
33+
a: String
34+
@required
35+
b: String
36+
c: String
37+
@required
38+
d: String
39+
}
40+
output:= {
41+
a: String
42+
@required
43+
b: String
44+
c: String
45+
@required
46+
d: String
47+
}
48+
errors: [ValidationException]
49+
}
50+
51+
@http(method: "POST", uri: "/opCorrect")
52+
operation OpWithCorrectOrder {
53+
input:= {
54+
@required
55+
b: String
56+
@required
57+
d: String
58+
a: String
59+
c: String
60+
}
61+
output:= {
62+
@required
63+
b: String
64+
@required
65+
d: String
66+
a: String
67+
c: String
68+
}
69+
errors: [ValidationException]
70+
}
71+
72+
@http(method: "POST", uri: "/opWithDefaults")
73+
operation OpWithDefaults {
74+
input:= {
75+
a: String,
76+
b: String = "hi"
77+
}
78+
}
79+
""".asSmithyModel(smithyVersion = "2")
80+
81+
val (pluginCtx, testDir) = generatePythonServerPluginContext(model)
82+
executePythonServerCodegenVisitor(pluginCtx)
83+
84+
val writer = RustWriter.forModule("service")
85+
writer.unitTest("test_required_fields") {
86+
rust(
87+
"""
88+
use crate::input;
89+
use pyo3::{types::IntoPyDict, Python};
90+
91+
pyo3::prepare_freethreaded_python();
92+
Python::with_gil(|py| {
93+
let globals = [
94+
(
95+
"OpWithIncorrectOrderInput",
96+
py.get_type::<input::OpWithIncorrectOrderInput>(),
97+
),
98+
(
99+
"OpWithCorrectOrderInput",
100+
py.get_type::<input::OpWithCorrectOrderInput>(),
101+
),
102+
(
103+
"OpWithDefaultsInput",
104+
py.get_type::<input::OpWithDefaultsInput>(),
105+
)]
106+
.into_py_dict(py);
107+
let locals = [(
108+
"OpWithIncorrectOrderInput",
109+
py.get_type::<input::OpWithIncorrectOrderInput>(),
110+
)]
111+
.into_py_dict(py);
112+
113+
py.run(
114+
"input = OpWithIncorrectOrderInput(\"b\", \"d\")",
115+
Some(globals),
116+
Some(locals),
117+
)
118+
.unwrap();
119+
120+
// Python should have been able to construct input.
121+
let input = locals
122+
.get_item("input")
123+
.expect("Python exception occurred during dictionary lookup")
124+
.unwrap()
125+
.extract::<input::OpWithIncorrectOrderInput>()
126+
.unwrap();
127+
assert_eq!(input.b, "b");
128+
assert_eq!(input.d, "d");
129+
130+
py.run(
131+
"input = OpWithCorrectOrderInput(\"b\", \"d\")",
132+
Some(globals),
133+
Some(locals),
134+
)
135+
.unwrap();
136+
137+
// Python should have been able to construct input.
138+
let input = locals
139+
.get_item("input")
140+
.expect("Python exception occurred during dictionary lookup")
141+
.unwrap()
142+
.extract::<input::OpWithCorrectOrderInput>()
143+
.unwrap();
144+
assert_eq!(input.b, "b");
145+
assert_eq!(input.d, "d");
146+
147+
py.run(
148+
"input = OpWithDefaultsInput(\"a\")",
149+
Some(globals),
150+
Some(locals),
151+
)
152+
.unwrap();
153+
154+
// KanchaBilla
155+
// Python should have been able to construct input.
156+
let input = locals
157+
.get_item("input")
158+
.expect("Python exception occurred during dictionary lookup")
159+
.unwrap()
160+
.extract::<input::OpWithDefaultsInput>()
161+
.unwrap();
162+
assert_eq!(input.a, Some("a".to_string()));
163+
assert_eq!(input.b, "hi");
164+
});
165+
""",
166+
)
167+
}
168+
169+
testDir.resolve("src/service.rs").appendText(writer.toString())
170+
171+
cargoTest(testDir)
172+
}
173+
}

‎codegen-server/python/src/test/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerTypesTest.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ internal class PythonServerTypesTest {
119119
120120
locals
121121
.get_item("output")
122+
.expect("Python exception occurred during dictionary lookup")
122123
.unwrap()
123124
.extract::<output::EchoOutput>()
124125
.unwrap()
@@ -212,6 +213,7 @@ internal class PythonServerTypesTest {
212213
213214
locals
214215
.get_item("output")
216+
.expect("Python exception occurred during dictionary lookup")
215217
.unwrap()
216218
.extract::<output::EchoOutput>()
217219
.unwrap()

0 commit comments

Comments
 (0)