Skip to content

Commit 65027f8

Browse files
authored
fix: validate that members bound to URI paths are non-null at object construction (#666)
1 parent 0e19c06 commit 65027f8

File tree

4 files changed

+85
-5
lines changed

4 files changed

+85
-5
lines changed
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"id": "ec8ea669-5999-45d9-ad8e-dbb8d35d3c4e",
3+
"type": "bugfix",
4+
"description": "Validate that members bound to URI paths are non-null at object construction",
5+
"issues": [
6+
"awslabs/smithy-kotlin#139"
7+
]
8+
}

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGenerator.kt

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import software.amazon.smithy.model.shapes.MemberShape
1414
import software.amazon.smithy.model.shapes.ShapeType
1515
import software.amazon.smithy.model.shapes.StructureShape
1616
import software.amazon.smithy.model.traits.ErrorTrait
17+
import software.amazon.smithy.model.traits.HttpLabelTrait
1718
import software.amazon.smithy.model.traits.RetryableTrait
1819
import software.amazon.smithy.model.traits.SensitiveTrait
1920
import software.amazon.smithy.model.traits.StreamingTrait
@@ -69,16 +70,29 @@ class StructureGenerator(
6970
val (memberName, memberSymbol) = memberNameSymbolIndex[it]!!
7071
writer.renderMemberDocumentation(model, it)
7172
writer.renderAnnotations(it)
72-
if (shape.isError && "message" == memberName) {
73-
val targetShape = model.expectShape(it.target)
73+
renderImmutableProperty(it, memberName, memberSymbol)
74+
}
75+
}
76+
77+
private fun renderImmutableProperty(memberShape: MemberShape, memberName: String, memberSymbol: Symbol) {
78+
when {
79+
shape.isError && memberName == "message" -> {
80+
val targetShape = model.expectShape(memberShape.target)
7481
if (!targetShape.isStringShape) {
7582
throw CodegenException("Message is a reserved name for exception types and cannot be used for any other property")
7683
}
7784
// override Throwable's message property
7885
writer.write("override val #1L: #2F = builder.#1L", memberName, memberSymbol)
79-
} else {
80-
writer.write("val #1L: #2F = builder.#1L", memberName, memberSymbol)
8186
}
87+
88+
memberShape.hasTrait<HttpLabelTrait>() ->
89+
writer.write(
90+
"""val #1L: #2F = requireNotNull(builder.#1L) { "A non-null value must be provided for #1L" }""",
91+
memberName,
92+
memberSymbol,
93+
)
94+
95+
else -> writer.write("val #1L: #2F = builder.#1L", memberName, memberSymbol)
8296
}
8397
}
8498

smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/rendering/protocol/HttpProtocolUnitTestResponseGenerator.kt

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44
*/
55
package software.amazon.smithy.kotlin.codegen.rendering.protocol
66

7+
import software.amazon.smithy.codegen.core.CodegenException
78
import software.amazon.smithy.codegen.core.Symbol
89
import software.amazon.smithy.kotlin.codegen.core.*
910
import software.amazon.smithy.kotlin.codegen.model.hasStreamingMember
1011
import software.amazon.smithy.kotlin.codegen.model.hasTrait
12+
import software.amazon.smithy.kotlin.codegen.model.shape
1113
import software.amazon.smithy.kotlin.codegen.rendering.ShapeValueGenerator
1214
import software.amazon.smithy.model.shapes.*
15+
import software.amazon.smithy.model.traits.HttpLabelTrait
1316
import software.amazon.smithy.model.traits.StreamingTrait
1417
import software.amazon.smithy.protocoltests.traits.HttpResponseTestCase
1518

@@ -80,8 +83,15 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder:
8083
val inputShape = model.expectShape(it)
8184
val inputSymbol = symbolProvider.toSymbol(inputShape)
8285

86+
val labelMembers = inputShape.members().filter { it.hasTrait<HttpLabelTrait>() }
87+
8388
// invoke the DSL builder for the input type
84-
writer.write("val input = ${inputSymbol.name}{}")
89+
writer.withBlock("val input = #T {", "}", inputSymbol) {
90+
labelMembers.forEach { memberShape ->
91+
val memberSymbol = symbolProvider.toSymbol(memberShape)
92+
write("#L = #L", memberShape.defaultName(), memberSymbol.defaultUnboxedValue())
93+
}
94+
}
8595
}
8696

8797
val service = symbolProvider.toSymbol(serviceShape)
@@ -178,3 +188,13 @@ open class HttpProtocolUnitTestResponseGenerator protected constructor(builder:
178188
HttpProtocolUnitTestResponseGenerator(this)
179189
}
180190
}
191+
192+
private fun Symbol.defaultUnboxedValue(): String = when (shape) {
193+
is LongShape -> "0L"
194+
is FloatShape -> "0.0f"
195+
is DoubleShape -> "0.0"
196+
is NumberShape -> "0"
197+
is StringShape -> "\"\""
198+
is BooleanShape -> "false"
199+
else -> throw CodegenException("Cannot determine default value for unsupported shape $shape")
200+
}

smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/rendering/StructureGeneratorTest.kt

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,44 @@ class StructureGeneratorTest {
280280
generated.shouldContainOnlyOnceWithDiff(expected)
281281
}
282282

283+
@Test
284+
fun `it handles HTTP labels in initializers`() {
285+
val model = """
286+
@http(method: "POST", uri: "/foo/{bar}/{baz}")
287+
operation Foo {
288+
input: FooRequest
289+
}
290+
291+
structure FooRequest {
292+
@required
293+
@httpLabel
294+
bar: String,
295+
296+
@required
297+
@httpLabel
298+
baz: Integer,
299+
300+
@httpPayload
301+
qux: String
302+
}
303+
""".prependNamespaceAndService(operations = listOf("Foo")).toSmithyModel()
304+
305+
val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
306+
val writer = KotlinWriter(TestModelDefault.NAMESPACE)
307+
val struct = model.expectShape<StructureShape>("com.test#FooRequest")
308+
val renderingCtx = RenderingContext(writer, struct, model, provider, model.defaultSettings())
309+
StructureGenerator(renderingCtx).render()
310+
311+
val generated = writer.toString()
312+
val expected = """
313+
class FooRequest private constructor(builder: Builder) {
314+
val bar: kotlin.String? = requireNotNull(builder.bar) { "A non-null value must be provided for bar" }
315+
val baz: kotlin.Int? = requireNotNull(builder.baz) { "A non-null value must be provided for baz" }
316+
val qux: kotlin.String? = builder.qux
317+
""".formatForTest(indent = "")
318+
generated.shouldContainOnlyOnceWithDiff(expected)
319+
}
320+
283321
@Test
284322
fun `it handles blob shapes`() {
285323
// blobs (with and without streaming) require special attention in equals() and hashCode() implementations

0 commit comments

Comments
 (0)