Skip to content

Commit 3f6039d

Browse files
committed
Improve number normalization in NumberNode
This change now makes the BigDecimal get created eagerly, and accounts for NaN and Infinity. The previous implementation would have failed in these cases. The node validation visitor now uses the computed BigDecimal rather than create a one-off each time. It also doesn't recreate the already available BigDecimcal of the range trait.
1 parent 1412661 commit 3f6039d

File tree

2 files changed

+89
-56
lines changed

2 files changed

+89
-56
lines changed

smithy-model/src/main/java/software/amazon/smithy/model/node/NumberNode.java

Lines changed: 84 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License").
55
* You may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616
package software.amazon.smithy.model.node;
1717

1818
import java.math.BigDecimal;
19-
import java.math.BigInteger;
2019
import java.util.Objects;
2120
import java.util.Optional;
2221
import java.util.function.Supplier;
@@ -31,17 +30,48 @@
3130
*/
3231
public final class NumberNode extends Node {
3332

34-
private final Number value;
33+
private final BigDecimal value;
34+
private final Number originalValue;
3535
private final String stringCache;
36-
private volatile BigDecimal equality;
36+
private boolean isNaN;
37+
private boolean isPositiveInfinity;
38+
private boolean isNegativeInfinity;
3739

3840
public NumberNode(Number value, SourceLocation sourceLocation) {
3941
super(sourceLocation);
40-
this.value = Objects.requireNonNull(value);
42+
originalValue = value;
4143
stringCache = value.toString();
44+
this.value = toBigDecimal(originalValue);
45+
}
4246

47+
private BigDecimal toBigDecimal(Number value) {
4348
if (value instanceof BigDecimal) {
44-
equality = (BigDecimal) value;
49+
return (BigDecimal) value;
50+
} else if (value instanceof Integer) {
51+
return BigDecimal.valueOf(value.intValue());
52+
} else if (value instanceof Short) {
53+
return BigDecimal.valueOf(value.shortValue());
54+
} else if (value instanceof Byte) {
55+
return BigDecimal.valueOf(value.byteValue());
56+
} else if (value instanceof Long) {
57+
return BigDecimal.valueOf(value.longValue());
58+
} else if (value instanceof Float || value instanceof Double) {
59+
double d = value.doubleValue();
60+
if (Double.isNaN(d)) {
61+
isNaN = true;
62+
return null;
63+
} else if (Double.isInfinite(d)) {
64+
if (stringCache.startsWith("-")) {
65+
isNegativeInfinity = true;
66+
} else {
67+
isPositiveInfinity = true;
68+
}
69+
return null;
70+
} else {
71+
return BigDecimal.valueOf(d);
72+
}
73+
} else {
74+
return new BigDecimal(stringCache);
4575
}
4676
}
4777

@@ -51,7 +81,18 @@ public NumberNode(Number value, SourceLocation sourceLocation) {
5181
* @return Returns a number.
5282
*/
5383
public Number getValue() {
54-
return value;
84+
return originalValue;
85+
}
86+
87+
/**
88+
* Gets the number value as a BigDecimal if possible.
89+
*
90+
* <p>NaN and infinite numbers will return an empty Optional.
91+
*
92+
* @return Returns the BigDecimal value of the wrapped number.
93+
*/
94+
public Optional<BigDecimal> asBigDecimal() {
95+
return Optional.ofNullable(value);
5596
}
5697

5798
/**
@@ -69,10 +110,30 @@ public boolean isNaturalNumber() {
69110
* @return Returns true if the node contains a floating point number.
70111
*/
71112
public boolean isFloatingPointNumber() {
72-
return value instanceof Float
73-
|| value instanceof Double
74-
|| value instanceof BigDecimal
75-
|| stringCache.contains(".");
113+
return toString().contains(".")
114+
|| isInfinite()
115+
|| isNaN()
116+
|| value == null
117+
|| value.scale() > 0
118+
|| value.stripTrailingZeros().scale() > 0;
119+
}
120+
121+
/**
122+
* Returns true if the number is a floating point NaN.
123+
*
124+
* @return Return true if NaN.
125+
*/
126+
public boolean isNaN() {
127+
return isNaN;
128+
}
129+
130+
/**
131+
* Returns true if the number is infinite.
132+
*
133+
* @return Return true if infinite.
134+
*/
135+
public boolean isInfinite() {
136+
return isPositiveInfinity || isNegativeInfinity;
76137
}
77138

78139
@Override
@@ -117,18 +178,15 @@ public Optional<NumberNode> asNumberNode() {
117178
* @return Returns true if set to zero.
118179
*/
119180
public boolean isZero() {
120-
// Do a cheap test based on the serialized value of the number first.
121-
// This test covers byte, short, integer, and long.
122-
if (toString().equals("0") || toString().equals("0.0")) {
181+
if (isNegativeInfinity || isPositiveInfinity || isNaN) {
182+
return false;
183+
} else if (toString().equals("0") || toString().equals("0.0")) {
123184
return true;
124-
} else if (value instanceof BigDecimal) {
125-
return value.equals(BigDecimal.ZERO);
126-
} else if (value instanceof BigInteger) {
127-
return value.equals(BigInteger.ZERO);
128-
} else if (value instanceof Float) {
129-
return value.floatValue() == 0f;
185+
} else if (value == null) {
186+
// This case should never happen.
187+
return false;
130188
} else {
131-
return value.doubleValue() == 0d;
189+
return value.compareTo(BigDecimal.ZERO) == 0;
132190
}
133191
}
134192

@@ -139,42 +197,12 @@ public boolean equals(Object other) {
139197
} else if (other == this) {
140198
return true;
141199
} else {
142-
NumberNode otherNode = (NumberNode) other;
143-
144-
// This only works if both values are the same type.
145-
if (value.equals(otherNode.value)) {
146-
return true;
147-
}
148-
149-
// Attempt a cheap check based on the string cache.
150-
if (stringCache.equals(otherNode.stringCache)) {
151-
return true;
152-
}
153-
154-
// Convert both to BigDecimal and compare equality.
155-
return getEquality().equals(otherNode.getEquality());
156-
}
157-
}
158-
159-
private BigDecimal getEquality() {
160-
BigDecimal e = equality;
161-
162-
if (e == null) {
163-
if (value instanceof Integer) {
164-
e = BigDecimal.valueOf(value.intValue());
165-
} else if (value instanceof Short) {
166-
e = BigDecimal.valueOf(value.shortValue());
167-
} else if (value instanceof Byte) {
168-
e = BigDecimal.valueOf(value.byteValue());
169-
} else if (value instanceof Long) {
170-
e = BigDecimal.valueOf(value.longValue());
171-
} else {
172-
e = new BigDecimal(stringCache);
173-
}
174-
equality = e;
200+
NumberNode o = (NumberNode) other;
201+
return isNaN == o.isNaN
202+
&& isPositiveInfinity == o.isPositiveInfinity
203+
&& isNegativeInfinity == o.isNegativeInfinity
204+
&& Objects.equals(value, o.value);
175205
}
176-
177-
return e;
178206
}
179207

180208
@Override

smithy-model/src/test/java/software/amazon/smithy/model/node/NumberNodeTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,11 @@ public float floatValue() {
199199
public double doubleValue() {
200200
return 0;
201201
}
202+
203+
@Override
204+
public String toString() {
205+
return "0.000";
206+
}
202207
}, true);
203208
cases.put(new Number() {
204209
@Override

0 commit comments

Comments
 (0)