For now, AutoBeans expects that the input matches the expected format (and value ranges). The behavior seems kind of "undefined" when that's not the case, and not consistent between the VM and translatable implementations: * null values in some cases: dates in Java * exceptions in others: calling asNumber on a JsonSplittable without checking for isNumber can throw a NullPointerException; and Long.parseLong can throw a NumberFormatException * or even NaN: dates in JS; calling asNumber on a JsoSplittable without checking for isNumber can return NaN * or coercion to boolean: asBoolean on a JsoSplittable (note that on a JsonSplittable it could instead throw a NullPointerException)
If you want this to be fixed (which would be a good thing), I'd suggest doing it in a subsequent CL addressing all of these: error cases and discrepancy between Java and JS implementations. http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java File user/src/com/google/web/bindery/autobean/shared/ValueCodex.java (right): http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java#newcode107 user/src/com/google/web/bindery/autobean/shared/ValueCodex.java:107: return new Date((long) value.asNumber()); On 2012/04/05 20:34:10, skybrian wrote:
Since asNumber() returns a double, not a long, I expect this will lose
precision
once we are above 2^53. (What's that as a date, anyway?)
That's theoretically right, except: * this is about decoding, so the loss of precision if any would be in Splittable, not ValueCodex. If that's a problem, then Splittable could have an added isLong/asLong (and then we'd wonder about BigInteger/BigDecimal ;-) ). If you look at JsonSplittable, it explicitly rounds values to doubles (calls doubleValue() on any Number) * this would only happen on a Java client, because a Number in JavaScript is equivalent to a double in Java; a JSON input with a numeric value outside JS Number range would raise an exception at parsing time, before we reach this code * a Date in JavaScript has an even smaller range than a Number. ECMAScript 5 says (§15.9.1.1, p. 174): """ECMAScript Number values can represent all integers from –9,007,199,254,740,991 to 9,007,199,254,740,991; this range suffices to measure times to millisecond precision for any instant that is within approximately 285,616 years, either forward or backward, from 01 January, 1970 UTC. """The actual range of times supported by ECMAScript Date objects is slightly smaller: exactly –100,000,000 days to 100,000,000 days measured relative to midnight at the beginning of 01 January, 1970 UTC. This gives a range of 8,640,000,000,000,000 milliseconds to either side of 01 January, 1970 UTC.""" If you ask me, I'd be OK to leave the behavior undefined for such "out of range" values, on the basis that no-one would use them anyway. The behavior is already kind of undefined in the case of a String, as it'd try to create a JsDate (when compiled in JS) from the String, which is implementation-dependent (ECMAScript 5 added a specific format inspired by ISO8601, but allows implementation to use a fallback: "If the String does not conform to that format the function may fall back to any implementation-specific heuristics or implementation-specific date formats."). Furthermore, in JS, neither the JsDate.create() nor the Date(long) emulation should throw, so in case of an "invalid" String value, you'd get a "NaN Date"; in Java though, StringQuoter.tryParseDate uses Long.parseLong, ISO8601 and RFC2822 formats and then falls back to a 'null' value (which, as I said, you'd never get in JS). It might be something to fix, but that's a distinct issue (if any). http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/src/com/google/web/bindery/autobean/shared/ValueCodex.java#newcode170 user/src/com/google/web/bindery/autobean/shared/ValueCodex.java:170: return Long.valueOf((long) value.asNumber()); On 2012/04/05 20:34:10, skybrian wrote:
I expect this breaks once we get above 2^53?
Actually, it'll lose precision in Java (JsonSplittable would call doubleValue() on the Long parsed by org.json), and fail while parsing the JSON in JavaScript (Number cannot be represented). It's a shame that JSON (RFC 4627) doesn't define a clear and interoperable range for numbers, but I guess everyone using JSON sticks to using numeric values that can be parsed in JavaScript, i.e. falls in the java.lang.Double value space. http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/test/com/google/web/bindery/autobean/benchmarks/ValueCodexBenchmark.java File user/test/com/google/web/bindery/autobean/benchmarks/ValueCodexBenchmark.java (right): http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/test/com/google/web/bindery/autobean/benchmarks/ValueCodexBenchmark.java#newcode45 user/test/com/google/web/bindery/autobean/benchmarks/ValueCodexBenchmark.java:45: public void testEncode() { On 2012/04/05 20:34:10, skybrian wrote:
Empty test... not sure what's going on here.
AFAICT, these zero-arg methods are needed so that JUnit sees the tests, and then the BenchmarkShell will actually run the methods with annotated arguments. Will add a comment. http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/test/com/google/web/bindery/autobean/shared/AutoBeanCodexTest.java File user/test/com/google/web/bindery/autobean/shared/AutoBeanCodexTest.java (right): http://gwt-code-reviews.appspot.com/1601805/diff/2004/user/test/com/google/web/bindery/autobean/shared/AutoBeanCodexTest.java#newcode237 user/test/com/google/web/bindery/autobean/shared/AutoBeanCodexTest.java:237: AutoBean<HasLong> decodedBean = AutoBeanCodex.decode(f, HasLong.class, "{\"long\": 42}"); On 2012/04/05 20:34:10, skybrian wrote:
Let's try a wider range. I expect it breaks for Long.MAX_VALUE and Long.MIN_VALUE. (And below.)
See comments in ValueCodex. I can add tests using "8_640_000_000_000_000L" (value space for JS dates), or (long)Double.MAX_VALUE and (long)Double.MIN_VALUE in testLong, that are expected to pass both in JS and a Java VM, but for values outside those ranges the behavior is currently "undefined" (or at least inconsistent between JS and Java). I think I simply copied from testSimple() which doesn't exercise "edge cases" (also notice that the whole class only exercises the "happy paths", should we add tests for expected failures? may I suggest we do it in a subsequent CL?) http://gwt-code-reviews.appspot.com/1601805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
