Add logic to handle number acceptors #5813
Conversation
RanVaknin
changed the title
RanVaknin
force-pushed
the
rvaknin/number-support-for-waiters
branch
from
4fb548f to
cd55716
Compare
...es-test/src/test/java/software/amazon/awssdk/services/waiters/WaitersSyncFunctionalTest.java
Outdated
Show resolved
Hide resolved
|
| @@ -57,6 +57,7 @@ | |||
| * this interpreter make heavy use of the {@code JmesPathRuntime}. | |||
| */ | |||
| public class JmesPathAcceptorGenerator { | |||
| private static final ClassName BIG_DECIMAL = ClassName.get("java.math", "BigDecimal"); | |||
There was a problem hiding this comment.
You can simplify this to ClassName.get(BigDecimal.class)
codegen/src/main/java/software/amazon/awssdk/codegen/jmespath/component/Literal.java
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,6 @@ | |||
| { | |||
There was a problem hiding this comment.
This feature doesn't need a changelog entry, since the changelog is for customer facing changes; this is more of an internal feature (though it does mean it will allow us to potentially offer more waiters for customers in the future).
| @@ -36,7 +36,9 @@ public final class Jackson { | |||
| .disable(JSON.Feature.FAIL_ON_UNKNOWN_BEAN_PROPERTY) | |||
| .enable(JSON.Feature.PRETTY_PRINT_OUTPUT) | |||
| .enable(JSON.Feature.READ_JSON_ARRAYS_AS_JAVA_ARRAYS) | |||
| .treeCodec(new JacksonJrsTreeCodec()); | |||
| .enable(JSON.Feature.USE_BIG_DECIMAL_FOR_FLOATS) | |||
| .register(new JrSimpleTreeExtension()); | |||
There was a problem hiding this comment.
Can we just add a small comment here why we're adding JrSimpleTreeExtension? It's not obvious that it's to enable the BigDecimal support.
codegen/src/main/java/software/amazon/awssdk/codegen/poet/waiters/BaseWaiterClassSpec.java
Show resolved
Hide resolved
| if (value instanceof BigDecimal) { | ||
| this.numberValue = (BigDecimal) value; | ||
| } else { | ||
| this.numberValue = new BigDecimal(value.toString()); |
There was a problem hiding this comment.
BigDecimal has constructors that take either long or double. Can we check for those types and use those constructors instead? It will be more efficient than converting them to a string then back to a number
if (value instanceof Double || value instanceof Float) {
...| @@ -375,8 +375,7 @@ | |||
| "Integer":{"type":"integer"}, | |||
| // Shape is customized to BigDecimal in customization.config | |||
| @@ -62,8 +62,7 @@ public void clientBuilder_settingRetryModeInOverrideConfigurationConsumer() { | |||
| // Configuring the client using RetryMode should support AWS retryable conditions. | |||
| ProtocolRestJsonClient client = client(b -> b.overrideConfiguration(o -> o.retryStrategy(RetryMode.STANDARD))); | |||
| assertThrows(ProtocolRestJsonException.class, () -> callAllTypes(client)); | |||
| // Three requests, i.e., there were retries. | |||
| verifyRequestCount(3); | |||
| // Three requests, i.e., th | |||
There was a problem hiding this comment.
Probably didn't mean to delete this line?
Motivation and Context
Currently the Java SDK waiter matcher acceptor only support Integers. According to the cross SDK spec waiter matcher acceptor must support all number types.
Modifications
Testing