Skip to content

Commit 95b30dc

Browse files
authored
Add logic to handle number acceptors (#5813)
* add logic to handle number acceptors * Use BigDecimal for all JMESPath numbers to overcome JacksonJr lack of precision * Bumping jacksonjr version to add arbitrary precision * Fixing tests and adding coverage for pathAny and pathAll matchers * Fix checkstyle * Added changlog * Removing incorrect waiter definition and undoing incorrect changes in query folder * Removing unnecessary changes * Remove unnecessary imports * Fixing CR suggestions, better support for Big Decimal * Adding numeric waiter definition to trigger codegen and fixed codegen test * Remove dependency, refactor pathAnyAcceptorBody --------- Co-authored-by: Ran Vaknin <[email protected]>
1 parent f6c5e9f commit 95b30dc

File tree

17 files changed

+1195
-64
lines changed

17 files changed

+1195
-64
lines changed

codegen/src/main/java/software/amazon/awssdk/codegen/internal/Jackson.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import com.fasterxml.jackson.databind.ObjectWriter;
2121
import com.fasterxml.jackson.databind.SerializationFeature;
2222
import com.fasterxml.jackson.jr.ob.JSON;
23-
import com.fasterxml.jackson.jr.stree.JacksonJrsTreeCodec;
23+
import com.fasterxml.jackson.jr.stree.JrSimpleTreeExtension;
2424
import com.fasterxml.jackson.jr.stree.JrsValue;
2525
import java.io.File;
2626
import java.io.IOException;
@@ -36,7 +36,10 @@ public final class Jackson {
3636
.disable(JSON.Feature.FAIL_ON_UNKNOWN_BEAN_PROPERTY)
3737
.enable(JSON.Feature.PRETTY_PRINT_OUTPUT)
3838
.enable(JSON.Feature.READ_JSON_ARRAYS_AS_JAVA_ARRAYS)
39-
.treeCodec(new JacksonJrsTreeCodec());
39+
// enable BigDecimal Support
40+
.enable(JSON.Feature.USE_BIG_DECIMAL_FOR_FLOATS)
41+
.register(new JrSimpleTreeExtension());
42+
4043

4144
mapperBuilder.streamFactory().enable(JsonParser.Feature.ALLOW_COMMENTS);
4245

codegen/src/main/java/software/amazon/awssdk/codegen/poet/waiters/BaseWaiterClassSpec.java

Lines changed: 49 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import com.squareup.javapoet.TypeSpec;
3333
import com.squareup.javapoet.TypeVariableName;
3434
import com.squareup.javapoet.WildcardTypeName;
35+
import java.math.BigDecimal;
3536
import java.time.Duration;
3637
import java.util.ArrayList;
3738
import java.util.Comparator;
@@ -557,15 +558,30 @@ private String waiterState(Acceptor acceptor) {
557558

558559
private CodeBlock pathAcceptorBody(Acceptor acceptor) {
559560
String expected = acceptor.getExpected().asText();
560-
String expectedType = acceptor.getExpected() instanceof JrsString ? "$S" : "$L";
561-
return CodeBlock.builder()
561+
boolean isString = acceptor.getExpected() instanceof JrsString;
562+
boolean isNumber = acceptor.getExpected().isNumber();
563+
564+
CodeBlock.Builder block = CodeBlock.builder();
565+
if (isNumber) {
566+
block.add("new $T($S)", BigDecimal.class, expected);
567+
} else if (isString) {
568+
block.add("$S", expected);
569+
} else {
570+
block.add("$L", expected);
571+
}
572+
CodeBlock rhs = block.build();
573+
574+
CodeBlock.Builder builder = CodeBlock.builder()
562575
.add("response -> {")
563576
.add("$1T input = new $1T(response);", poetExtensions.jmesPathRuntimeClass().nestedClass("Value"))
564577
.add("return $T.equals(", Objects.class)
565578
.add(jmesPathAcceptorGenerator.interpret(acceptor.getArgument(), "input"))
566-
.add(".value(), " + expectedType + ");", expected)
567-
.add("}")
568-
.build();
579+
.add(".value(), ")
580+
.add(rhs)
581+
.add(");")
582+
.add("}");
583+
584+
return builder.build();
569585
}
570586

571587
private CodeBlock pathAllAcceptorBody(Acceptor acceptor) {
@@ -586,18 +602,34 @@ private CodeBlock pathAllAcceptorBody(Acceptor acceptor) {
586602

587603
private CodeBlock pathAnyAcceptorBody(Acceptor acceptor) {
588604
String expected = acceptor.getExpected().asText();
589-
String expectedType = acceptor.getExpected() instanceof JrsString ? "$S" : "$L";
590-
return CodeBlock.builder()
591-
.add("response -> {")
592-
.add("$1T input = new $1T(response);", poetExtensions.jmesPathRuntimeClass().nestedClass("Value"))
593-
.add("$T<$T> resultValues = ", List.class, Object.class)
594-
.add(jmesPathAcceptorGenerator.interpret(acceptor.getArgument(), "input"))
595-
.add(".values();")
596-
.add("return !resultValues.isEmpty() && "
597-
+ "resultValues.stream().anyMatch(v -> $T.equals(v, " + expectedType + "));",
598-
Objects.class, expected)
599-
.add("}")
600-
.build();
605+
boolean isString = acceptor.getExpected() instanceof JrsString;
606+
boolean isNumber = acceptor.getExpected().isNumber();
607+
608+
CodeBlock.Builder block = CodeBlock.builder();
609+
if (isNumber) {
610+
block.add("new $T($S)", BigDecimal.class, expected);
611+
} else if (isString) {
612+
block.add("$S", expected);
613+
} else {
614+
block.add("$L", expected);
615+
}
616+
CodeBlock rhs = block.build();
617+
618+
619+
CodeBlock.Builder builder = CodeBlock.builder()
620+
.add("response -> {")
621+
.add("$1T input = new $1T(response);", poetExtensions.jmesPathRuntimeClass().nestedClass("Value"))
622+
.add("$T<$T> resultValues = ", List.class, Object.class)
623+
.add(jmesPathAcceptorGenerator.interpret(acceptor.getArgument(), "input"))
624+
.add(".values();")
625+
.add("return !resultValues.isEmpty() &&"
626+
+ " resultValues.stream().anyMatch(v " + "-> $T.equals"
627+
+ "(v, ", Objects.class)
628+
.add(rhs)
629+
.add("));")
630+
.add("}");
631+
632+
return builder.build();
601633
}
602634

603635
private CodeBlock trueForAllResponse() {

codegen/src/main/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGenerator.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.fasterxml.jackson.jr.stree.JrsValue;
2020
import com.squareup.javapoet.ClassName;
2121
import com.squareup.javapoet.CodeBlock;
22+
import java.math.BigDecimal;
2223
import java.util.ArrayDeque;
2324
import java.util.Deque;
2425
import java.util.List;
@@ -57,6 +58,7 @@
5758
* this interpreter make heavy use of the {@code JmesPathRuntime}.
5859
*/
5960
public class JmesPathAcceptorGenerator {
61+
private static final ClassName BIG_DECIMAL = ClassName.get(BigDecimal.class);
6062
private final ClassName runtimeClass;
6163

6264
public JmesPathAcceptorGenerator(ClassName runtimeClass) {
@@ -273,7 +275,7 @@ public void visitRawString(String input) {
273275
public void visitLiteral(Literal input) {
274276
JrsValue jsonValue = input.jsonValue();
275277
if (jsonValue.isNumber()) {
276-
codeBlock.add(".constant($L)", Integer.parseInt(jsonValue.asText()));
278+
codeBlock.add(".constant(new $T($S))", BIG_DECIMAL, jsonValue.asText());
277279
} else if (jsonValue instanceof JrsBoolean) {
278280
codeBlock.add(".constant($L)", ((JrsBoolean) jsonValue).booleanValue());
279281
} else {

codegen/src/main/resources/software/amazon/awssdk/codegen/jmespath/JmesPathRuntime.java.resource

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import static java.util.stream.Collectors.toList;
22
import static java.util.stream.Collectors.toMap;
33

4+
import java.math.BigDecimal;
45
import java.util.ArrayList;
56
import java.util.Collection;
67
import java.util.Collections;
@@ -47,9 +48,9 @@ public final class JmesPathRuntime {
4748
private SdkPojo pojoValue;
4849

4950
/**
50-
* The value if this is an {@link Type#INTEGER} (or null otherwise).
51+
* The value if this is an {@link Type#NUMBER} (or null otherwise).
5152
*/
52-
private Integer integerValue;
53+
private BigDecimal numberValue;
5354

5455
/**
5556
* The value if this is an {@link Type#STRING} (or null otherwise).
@@ -105,9 +106,25 @@ public final class JmesPathRuntime {
105106
} else if (value instanceof String) {
106107
this.type = Type.STRING;
107108
this.stringValue = (String) value;
108-
} else if (value instanceof Integer) {
109-
this.type = Type.INTEGER;
110-
this.integerValue = (Integer) value;
109+
} else if (value instanceof Number) {
110+
this.type = Type.NUMBER;
111+
if (value instanceof BigDecimal) {
112+
this.numberValue = (BigDecimal) value;
113+
} else if (value instanceof Double) {
114+
this.numberValue = BigDecimal.valueOf((Double) value);
115+
} else if (value instanceof Float) {
116+
this.numberValue = BigDecimal.valueOf((Float) value);
117+
} else if (value instanceof Long) {
118+
this.numberValue = BigDecimal.valueOf((Long) value);
119+
} else if (value instanceof Integer) {
120+
this.numberValue = BigDecimal.valueOf((Integer) value);
121+
} else if (value instanceof Short) {
122+
this.numberValue = BigDecimal.valueOf((Short) value);
123+
} else if (value instanceof Byte) {
124+
this.numberValue = BigDecimal.valueOf((Byte) value);
125+
} else {
126+
this.numberValue = new BigDecimal(value.toString());
127+
}
111128
} else if (value instanceof Collection) {
112129
this.type = Type.LIST;
113130
this.listValue = new ArrayList<>(((Collection<?>) value));
@@ -143,7 +160,7 @@ public final class JmesPathRuntime {
143160
switch (type) {
144161
case NULL: return null;
145162
case POJO: return pojoValue;
146-
case INTEGER: return integerValue;
163+
case NUMBER: return numberValue;
147164
case STRING: return stringValue;
148165
case BOOLEAN: return booleanValue;
149166
case LIST: return listValue;
@@ -207,8 +224,8 @@ public final class JmesPathRuntime {
207224
switch (type) {
208225
case NULL:
209226
return null;
210-
case INTEGER:
211-
return integerValue.toString();
227+
case NUMBER:
228+
return numberValue.toString();
212229
case STRING:
213230
return stringValue;
214231
case BOOLEAN:
@@ -490,14 +507,14 @@ public final class JmesPathRuntime {
490507
return new Value(false);
491508
}
492509

493-
if (type == Type.INTEGER) {
510+
if (type == Type.NUMBER) {
494511
switch (comparison) {
495-
case "<": return new Value(integerValue < rhs.integerValue);
496-
case "<=": return new Value(integerValue <= rhs.integerValue);
497-
case ">": return new Value(integerValue > rhs.integerValue);
498-
case ">=": return new Value(integerValue >= rhs.integerValue);
499-
case "==": return new Value(Objects.equals(integerValue, rhs.integerValue));
500-
case "!=": return new Value(!Objects.equals(integerValue, rhs.integerValue));
512+
case "<": return new Value(numberValue.compareTo(rhs.numberValue) < 0);
513+
case "<=": return new Value(numberValue.compareTo(rhs.numberValue) <= 0);
514+
case ">": return new Value(numberValue.compareTo(rhs.numberValue) > 0);
515+
case ">=": return new Value(numberValue.compareTo(rhs.numberValue) >= 0);
516+
case "==": return new Value(numberValue.compareTo(rhs.numberValue) == 0);
517+
case "!=": return new Value(numberValue.compareTo(rhs.numberValue) != 0);
501518
default: throw new IllegalArgumentException("Unsupported comparison: " + comparison);
502519
}
503520
}
@@ -635,7 +652,7 @@ public final class JmesPathRuntime {
635652
MAP,
636653
BOOLEAN,
637654
STRING,
638-
INTEGER,
655+
NUMBER,
639656
NULL
640657
}
641658

codegen/src/test/java/software/amazon/awssdk/codegen/poet/waiters/JmesPathAcceptorGeneratorTest.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import com.squareup.javapoet.ClassName;
2323
import org.junit.jupiter.api.BeforeEach;
2424
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.CsvSource;
2527

2628
class JmesPathAcceptorGeneratorTest {
2729
private JmesPathAcceptorGenerator acceptorGenerator;
@@ -42,9 +44,10 @@ void testAutoScalingComplexExpression() {
4244
@Test
4345
void testEcsComplexExpression() {
4446
testConversion("length(services[?!(length(deployments) == `1` && runningCount == desiredCount)]) == `0`",
45-
"input.field(\"services\").filter(x0 -> x0.constant(x0.field(\"deployments\").length().compare(\"==\", "
46-
+ "x0.constant(1)).and(x0.field(\"runningCount\").compare(\"==\", x0.field(\"desiredCount\"))).not()))"
47-
+ ".length().compare(\"==\", input.constant(0))");
47+
"input.field(\"services\").filter(x0 -> x0.constant(x0.field(\"deployments\")"
48+
+ ".length().compare(\"==\", x0.constant(new java.math.BigDecimal(\"1\")))"
49+
+ ".and(x0.field(\"runningCount\").compare(\"==\", x0.field(\"desiredCount\")))"
50+
+ ".not())).length().compare(\"==\", input.constant(new java.math.BigDecimal(\"0\")))");
4851
}
4952

5053
@Test
@@ -326,6 +329,24 @@ void testNegativeNumber() {
326329
testConversion("foo[-10]", "input.field(\"foo\").index(-10)");
327330
}
328331

332+
@ParameterizedTest
333+
@CsvSource({
334+
"123132.81289319837183771465876127837183719837123, 123132.81289319837183771465876127837183719837123",
335+
"42, 42",
336+
"127, 127",
337+
"32767, 32767",
338+
"9223372036854775807, 9223372036854775807",
339+
"42.5, 42.5",
340+
"-42, -42",
341+
"0, 0",
342+
"1e-10, 1E-10"
343+
})
344+
void testNumericValues(String input, String expectedNumber) {
345+
String jmesPath = String.format("value == `%s`", input);
346+
String expected = String.format("input.field(\"value\").compare(\"==\", input.constant(new java.math.BigDecimal(\"%s\")))", expectedNumber);
347+
testConversion(jmesPath, expected);
348+
}
349+
329350
private void testConversion(String jmesPathString, String expectedCode) {
330351
assertThat(acceptorGenerator.interpret(jmesPathString, "input").toString()).isEqualTo((expectedCode));
331352
}

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/c2j/query/waiters-2.json

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,58 @@
2323
"state": "success"
2424
}
2525
]
26+
},
27+
"FloatValueTest": {
28+
"delay": 15,
29+
"operation": "APostOperation",
30+
"maxAttempts": 40,
31+
"acceptors": [
32+
{
33+
"matcher": "path",
34+
"expected": 42.5,
35+
"argument": "FloatValue",
36+
"state": "success"
37+
}
38+
]
39+
},
40+
"BigDecimalValueTest": {
41+
"delay": 15,
42+
"operation": "APostOperation",
43+
"maxAttempts": 40,
44+
"acceptors": [
45+
{
46+
"matcher": "path",
47+
"expected": 123132.81289319837183771465876127837183719837123,
48+
"argument": "BigDecimalValue",
49+
"state": "success"
50+
}
51+
]
52+
},
53+
"LongValueTest": {
54+
"delay": 15,
55+
"operation": "APostOperation",
56+
"maxAttempts": 40,
57+
"acceptors": [
58+
{
59+
"matcher": "path",
60+
"expected": 9223372036854775807,
61+
"argument": "LongValue",
62+
"state": "success"
63+
}
64+
]
65+
},
66+
"DoubleValueTest": {
67+
"delay": 15,
68+
"operation": "APostOperation",
69+
"maxAttempts": 40,
70+
"acceptors": [
71+
{
72+
"matcher": "path",
73+
"expected": 1.7976931348623157E308,
74+
"argument": "DoubleValue",
75+
"state": "success"
76+
}
77+
]
2678
}
2779
}
2880
}

0 commit comments

Comments
 (0)