misc: pr feedback v1

Author: 0marperez

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

@@ -27,6 +27,18 @@ import software.amazon.smithy.model.shapes.*
 
 private val suffixSequence = sequenceOf("") + generateSequence(2) { it + 1 }.map(Int::toString) // "", "2", "3", etc.
 
+/*
+JMESPath has the concept of objects.
+This visitor assumes JMESPath objects will be Kotlin objects/classes.
+The smithy spec contains an instance where it's assumed a JMESPath object will be a Kotlin map.
+
+Specifically it's the keys function
+Smithy spec: https://smithy.io/2.0/additional-specs/rules-engine/parameters.html#smithy-rules-operationcontextparams-trait
+JMESPath spec: https://jmespath.org/specification.html#keys
+
+TODO: Proactively support JMESPath objects as Kotlin maps throughout thr entire visitor if more instances of this behavior start popping up
+ */
+
 /**
  * An [ExpressionVisitor] used for traversing a JMESPath expression to generate code for traversing an equivalent
  * modeled object. This visitor is passed to [JmespathExpression.accept], at which point specific expression methods
@@ -524,14 +536,16 @@ class KotlinJmespathExpressionVisitor(
             ".$expr"
         }
 
-    // TODO: Support maps as objects throughout the visitor
-    // This visitor assumes JMESPath 'objects' will be classes. DDB assumes JMESPath 'objects' will be maps
+    /*
+    Smithy spec expects a map, JMESPath spec expects an object
+    Smithy spec: https://smithy.io/2.0/additional-specs/rules-engine/parameters.html#smithy-rules-operationcontextparams-trait
+    JMESPath spec: https://jmespath.org/specification.html#keys
+     */
     private fun VisitedExpression.getKeys(): String =
-        if (this.shape?.targetOrSelf(ctx.model)?.type == ShapeType.MAP) {
-            "${this.identifier}?.keys?.toList()"
+        if (shape?.targetOrSelf(ctx.model)?.type == ShapeType.MAP) {
+            "$identifier?.keys?.map { it.toString() }?.toList()"
         } else {
-            this
-                .shape
+            shape
                 ?.targetOrSelf(ctx.model)
                 ?.allMembers
                 ?.keys

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

@@ -135,7 +135,7 @@ class OperationContextParamsTest {
             @Suppress("UNCHECKED_CAST")
             val input = request.context[HttpOperationContext.OperationInput] as TestOperationRequest
             val object = input.object
-            val keys = object?.keys?.toList()
+            val keys = object?.keys?.map { it.toString() }?.toList()
             builder.foo = keys
         """.formatForTest("    ")
 

tests/codegen/waiter-tests/model/function-keys.smithy

@@ -18,6 +18,20 @@ use smithy.waiters#waitable
             }
         ]
     },
+    KeysFunctionPrimitivesIntegerEquals: {
+        acceptors: [
+            {
+                state: "success",
+                matcher: {
+                    output: {
+                        path: "keys(primitives)",
+                        expected: "integer",
+                        comparator: "anyStringEquals"
+                    }
+                }
+            }
+        ]
+    },
     KeysFunctionMapStringEquals: {
         acceptors: [
             {

tests/codegen/waiter-tests/src/test/kotlin/com/test/FunctionKeysTest.kt

@@ -11,6 +11,7 @@ import com.test.model.GetFunctionKeysEqualsRequest
 import com.test.model.GetFunctionKeysEqualsResponse
 import com.test.utils.successTest
 import com.test.waiters.waitUntilKeysFunctionMapStringEquals
+import com.test.waiters.waitUntilKeysFunctionPrimitivesIntegerEquals
 import com.test.waiters.waitUntilKeysFunctionPrimitivesStringEquals
 import kotlin.test.Test
 
@@ -22,6 +23,13 @@ class FunctionKeysTest {
         GetFunctionKeysEqualsResponse { primitives = EntityPrimitives { } },
     )
 
+    @Test
+    fun testKeysFunctionPrimitivesIntegerEquals() = successTest(
+        GetFunctionKeysEqualsRequest { name = "test" },
+        WaitersTestClient::waitUntilKeysFunctionPrimitivesIntegerEquals,
+        GetFunctionKeysEqualsResponse { primitives = EntityPrimitives { } },
+    )
+
     @Test
     fun testKeysFunctionMapStringEquals() = successTest(
         GetFunctionKeysEqualsRequest { name = "test" },