Supporting wildcard and keys fn in JmesPathRuntime

[cenedhryn]

Motivation and Context

Modifications

• Modifies the wildcard function, which has existed in the runtime but never been used in waiters, to accept lists which is according to specs: https://jmespath.org/specification.html#wildcard-expressions
• Adds a keys function which can return the names of SdkField members in an SdkPojo
• [INCORRECT] Catches errors when evaluating OperationContextParams at runtime and returns null instead, logging errors. It's complex functionality especially when sourcing parameters from a customization config. We can re-evaluate this behavior when model traits are available.
• Fixes a minor bug in project to filter null values from the result. Another alternative would be to just filter nulls when returning stringValues but I can't think of a Java use-case where we want to preserve the null when evaluating an expression.
• Also changed to filter nulls for stringValues, because it is correct.

Testing

Tested in codegen-generated-classes-test

Testing with incomplete objects where the projected field does not exist in a list projection member didn't work with the approximation of and SdkPojo in the test class. Instead, switched to a modeled pojo that correctly returns null for a field instead of not recognizing field.

[cenedhryn]

In general, wondering if we should add logic to only generate operation context params if they exist, potentially including the logger dependency. Do not have time to check exactly what is generated under which circumstances for this PR.

[cenedhryn]

Added removing empty methods to follow up task

[joviegas]

I believe that doing a try catch exception here will absorb any issue in this case we should be doing a fail-fast rather than fail safe for following reasons

1. We should immediately fail if there are issues while extracting the value using JmesPathRuntime

Example : Currently the customization was specified as "value": "Delete.Objects[*].key" , note that it should have have been "value": "Delete.Objects[*].Key"
Because of this the Binding is not happening and it gets ignored silently with a warning.
However , if we have an exception we get IllegalArgumentException: No such field: key upfront

java.lang.IllegalArgumentException: No such field: key
	at software.amazon.awssdk.services.s3.jmespath.internal.JmesPathRuntime$Value.lambda$field$4(JmesPathRuntime.java:297)
	at java.util.Optional.orElseThrow(Optional.java:290)

2. This might cause issue while AuthSchemes tries to use it , Auth scheme will not have a way to know if its params were Null because of Exception or if user had not set it.

[cenedhryn]

Agreed

[joviegas]

If there is a NonExistingMember defined should we get an exception?

[cenedhryn]

Removed

[joviegas]

Can we write a test case when only one of the List has entry

Source Json

{
 "bucket": "someBucket",
 "delete": {
   "objects": [
     {
       "id": "a",
       "key": "One"
     },
     {
       "id": "b"
     }
   ]
 }
}

JMESPath Expression : delete.objects[*].key

Output

[
  "One"
]

[cenedhryn]

Yes, had to update the test harness to handle this case.

[joviegas]

Can we please add test cases in Acceptor too.

aws-sdk-java-v2/codegen/src/test/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGeneratorTest.java

Lines 217 to 271 in e46761c

	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithNoNumbers2() {
	testConversion("[:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithNoNumbers3() {
	testConversion("[::]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartNumber2() {
	testConversion("[10:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStopNumber2() {
	testConversion("[:10]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopNumber2() {
	testConversion("[10:20]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartNumber3() {
	testConversion("[10::]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStopNumber3() {
	testConversion("[:10:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopNumber3() {
	testConversion("[10:20:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopStepNumber3() {
	testConversion("[10:20:30]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStepNumber3() {
	testConversion("[::30]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testCurrentNode() {
	testConversion("@", "");
	}

Since we are supporting it now can we add corresponding test for above cases .

Or if we only support [*] then add test for [*] and keep the remaining assertion as it was before

[cenedhryn]

Can we please add test cases in Acceptor too.

aws-sdk-java-v2/codegen/src/test/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGeneratorTest.java

Lines 217 to 271 in e46761c

	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithNoNumbers2() {
	testConversion("[:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithNoNumbers3() {
	testConversion("[::]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartNumber2() {
	testConversion("[10:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStopNumber2() {
	testConversion("[:10]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopNumber2() {
	testConversion("[10:20]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartNumber3() {
	testConversion("[10::]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStopNumber3() {
	testConversion("[:10:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopNumber3() {
	testConversion("[10:20:]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStartStopStepNumber3() {
	testConversion("[10:20:30]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testSliceExpressionWithStepNumber3() {
	testConversion("[::30]", "");
	}
	@Test(expected = UnsupportedOperationException.class)
	public void testCurrentNode() {
	testConversion("@", "");
	}

Since we are supporting it now can we add corresponding test for above cases .

Or if we only support [*] then add test for [*] and keep the remaining assertion as it was before

There a few things I want to clean up in a follow-up PR - some renaming, excluding codegen of empty methods etc. I will add this to that list.

[sonarcloud]

Quality Gate failed

Failed conditions
77.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud