feat: add support for sub-expressions and list indexing in JMESPath #912
Conversation
0marperez
added
the
no-changelog
| @@ -107,6 +108,25 @@ class WaiterTest { | |||
| GetEntityResponse { primitives = EntityPrimitives { intEnum = IntEnum.One } }, | |||
| ) | |||
|
|
|||
| // list indexing | |||
| @Test fun testBooleanListIndexZeroEquals() = successTest( | |||
There was a problem hiding this comment.
suggestion: Would be good to have some failure tests (e.g. doesn't match, index out of bounds, etc).
There was a problem hiding this comment.
Smithy wouldn't allow out of bounds indexing but I tried. I would get an error message:
comparator must return a boolean type, but this acceptor was statically determined to return a null type.
| @@ -193,6 +193,50 @@ service WaitersTestService { | |||
| ] | |||
| }, | |||
|
|
|||
| // list indexing | |||
There was a problem hiding this comment.
suggestion: Would be good to also test a more involved index expression like the struct list list.structs[1].strings[0]
| @Test fun testBooleanListIndexOneEquals() = successTest( | ||
| WaitersTestClient::waitUntilBooleanListIndexOneEquals, | ||
| GetEntityResponse { lists = EntityLists { booleans = listOf(false, false) } }, | ||
| GetEntityResponse { lists = EntityLists { booleans = listOf(true, true) } }, | ||
| ) |
There was a problem hiding this comment.
Nit: This doesn't test that the correct index is being observed since both values in the list are changing at the same time. I recommend testing [false, false], [true, false], and then finally [false, true].
|
Kudos, SonarCloud Quality Gate passed!
|
| @@ -336,8 +356,7 @@ class KotlinJmespathExpressionVisitor( | |||
|
|
|||
| private val Shape.isNullable: Boolean | |||
| get() = this is MemberShape && | |||
| ctx.model.expectShape(target).let { !it.hasTrait<OperationInput>() && !it.hasTrait<OperationOutput>() } && | |||
| nullableIndex.isMemberNullable(this, NullableIndex.CheckMode.CLIENT_ZERO_VALUE_V1_NO_INPUT) | |||
There was a problem hiding this comment.
was this nullableIndex.isMemberNullable check meant to be removed?
There was a problem hiding this comment.
Yeah, it was causing issues and the code behaves the same without it
| is FieldExpression -> subfield(left, parentName).identifier | ||
| is Subexpression -> subexpression(left, parentName).identifier | ||
| else -> throw CodegenException("Subexpression type $left is unsupported") |
There was a problem hiding this comment.
Could left also be an IndexExpression?
There was a problem hiding this comment.
I guess it could but it really shouldn't be. The expression would be trying to access an index in nothing i.e. "[2]..."
Issue #
Implement full support for JMESPath expression types #596
Description of changes
-The sub-expression function is now recursive and will be able to visit all expressions needed
-Visiting indexes is now available
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.