fix: require values for HTTP query- and queryParams-bound parameters (#697)

Author: ianbotsf

.changes/10bfb10c-9874-4480-bc75-26157b725e22.json

@@ -0,0 +1,5 @@
+{
+    "id": "10bfb10c-9874-4480-bc75-26157b725e22",
+    "type": "bugfix",
+    "description": "Require values for HTTP query- and queryParams-bound parameters"
+}

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

@@ -9,9 +9,13 @@ import software.amazon.smithy.codegen.core.Symbol
 import software.amazon.smithy.kotlin.codegen.core.*
 import software.amazon.smithy.kotlin.codegen.lang.KotlinTypes
 import software.amazon.smithy.kotlin.codegen.model.*
+import software.amazon.smithy.kotlin.codegen.utils.getOrNull
 import software.amazon.smithy.model.shapes.*
 import software.amazon.smithy.model.traits.ErrorTrait
 import software.amazon.smithy.model.traits.HttpLabelTrait
+import software.amazon.smithy.model.traits.HttpQueryParamsTrait
+import software.amazon.smithy.model.traits.HttpQueryTrait
+import software.amazon.smithy.model.traits.LengthTrait
 import software.amazon.smithy.model.traits.RetryableTrait
 import software.amazon.smithy.model.traits.SensitiveTrait
 import software.amazon.smithy.model.traits.StreamingTrait
@@ -82,17 +86,35 @@ class StructureGenerator(
                 writer.write("override val #1L: #2F = builder.#1L", memberName, memberSymbol)
             }
 
-            memberShape.hasTrait<HttpLabelTrait>() ->
+            memberShape.isRequiredInStruct -> {
                 writer.write(
                     """public val #1L: #2F = requireNotNull(builder.#1L) { "A non-null value must be provided for #1L" }""",
                     memberName,
                     memberSymbol,
                 )
+                if (memberShape.isNonBlankInStruct) {
+                    writer
+                        .indent()
+                        .write(
+                            """.apply { require(isNotBlank()) { "A non-blank value must be provided for #L" } }""",
+                            memberName,
+                        )
+                        .dedent()
+                }
+            }
 
             else -> writer.write("public val #1L: #2F = builder.#1L", memberName, memberSymbol)
         }
     }
 
+    private val MemberShape.isRequiredInStruct
+        get() =
+            hasTrait<HttpLabelTrait>() ||
+                isRequired && (hasTrait<HttpQueryTrait>() || hasTrait<HttpQueryParamsTrait>())
+
+    private val MemberShape.isNonBlankInStruct
+        get() = getTrait<LengthTrait>()?.min?.getOrNull()?.takeIf { it > 0 } != null
+
     private fun renderCompanionObject() {
         writer.withBlock("public companion object {", "}") {
             write("public operator fun invoke(block: Builder.() -> #Q): #Q = Builder().apply(block).build()", KotlinTypes.Unit, symbol)

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

@@ -280,7 +280,7 @@ class StructureGeneratorTest {
     }
 
     @Test
-    fun `it handles HTTP labels in initializers`() {
+    fun `it handles required HTTP fields in initializers`() {
         val model = """
             @http(method: "POST", uri: "/foo/{bar}/{baz}")
             operation Foo {
@@ -292,12 +292,29 @@ class StructureGeneratorTest {
                 @httpLabel
                 bar: String,
                 
-                @required
                 @httpLabel
+                @required
                 baz: Integer,
                 
                 @httpPayload
-                qux: String
+                qux: String,
+                
+                @required
+                @httpQuery("quux")
+                quux: Boolean,
+                
+                @httpQuery("corge")
+                corge: String,
+                
+                @required
+                @length(min: 0)
+                @httpQuery("grault")
+                grault: String,
+                
+                @required
+                @length(min: 3)
+                @httpQuery("garply")
+                garply: String
             }
         """.prependNamespaceAndService(operations = listOf("Foo")).toSmithyModel()
 
@@ -312,11 +329,54 @@ class StructureGeneratorTest {
             public class FooRequest private constructor(builder: Builder) {
                 public val bar: kotlin.String? = requireNotNull(builder.bar) { "A non-null value must be provided for bar" }
                 public val baz: kotlin.Int? = requireNotNull(builder.baz) { "A non-null value must be provided for baz" }
+                public val corge: kotlin.String? = builder.corge
+                public val garply: kotlin.String? = requireNotNull(builder.garply) { "A non-null value must be provided for garply" }
+                    .apply { require(isNotBlank()) { "A non-blank value must be provided for garply" } }
+                public val grault: kotlin.String? = requireNotNull(builder.grault) { "A non-null value must be provided for grault" }
+                public val quux: kotlin.Boolean? = requireNotNull(builder.quux) { "A non-null value must be provided for quux" }
                 public val qux: kotlin.String? = builder.qux
         """.formatForTest(indent = "")
         generated.shouldContainOnlyOnceWithDiff(expected)
     }
 
+    @Test
+    fun `it handles required query params in initializers`() {
+        val model = """
+            @http(method: "POST", uri: "/foo")
+            operation Foo {
+                input: FooRequest
+            }
+            
+            map MapOfStrings {
+                key: String,
+                value: String
+            }
+            
+            structure FooRequest {
+                @required
+                @httpQueryParams
+                bar: MapOfStrings
+                
+                @httpPayload
+                baz: String
+            }
+        """.prependNamespaceAndService(operations = listOf("Foo")).toSmithyModel()
+
+        val provider: SymbolProvider = KotlinCodegenPlugin.createSymbolProvider(model)
+        val writer = KotlinWriter(TestModelDefault.NAMESPACE)
+        val struct = model.expectShape<StructureShape>("com.test#FooRequest")
+        val renderingCtx = RenderingContext(writer, struct, model, provider, model.defaultSettings())
+        StructureGenerator(renderingCtx).render()
+
+        val generated = writer.toString()
+        val expected = """
+            public class FooRequest private constructor(builder: Builder) {
+                public val bar: Map<String, String>? = requireNotNull(builder.bar) { "A non-null value must be provided for bar" }
+                public val baz: kotlin.String? = builder.baz
+        """.formatForTest(indent = "")
+        generated.shouldContainOnlyOnceWithDiff(expected)
+    }
+
     @Test
     fun `it handles blob shapes`() {
         // blobs (with and without streaming) require special attention in equals() and hashCode() implementations