stevedlawrence commented on code in PR #1298:
URL: https://github.com/apache/daffodil/pull/1298#discussion_r1761601919
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetInputter.scala:
##########
@@ -120,10 +125,13 @@ class JsonInfosetInputter(input: java.io.InputStream)
extends InfosetInputter {
): String = {
if (jsp.getCurrentToken() == JsonToken.VALUE_NULL) {
null
- } else {
- Assert.invariant(jsp.getCurrentToken() == JsonToken.VALUE_STRING)
+ } else if (jsp.getCurrentToken() == JsonToken.VALUE_STRING) {
// this handles unescaping any escaped characters
jsp.getText()
Review Comment:
Can we just always use `getText()` if the token is one of
STRING/FLOAT/INT/TRUE/FALSE? I assume that returns the actual text in the json
data and lets our infoset inputter logic convert it to the underlying type.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetInputter.scala:
##########
@@ -120,10 +125,13 @@ class JsonInfosetInputter(input: java.io.InputStream)
extends InfosetInputter {
): String = {
if (jsp.getCurrentToken() == JsonToken.VALUE_NULL) {
null
- } else {
- Assert.invariant(jsp.getCurrentToken() == JsonToken.VALUE_STRING)
+ } else if (jsp.getCurrentToken() == JsonToken.VALUE_STRING) {
// this handles unescaping any escaped characters
jsp.getText()
+ } else if (jsp.getCurrentToken() == JsonToken.VALUE_TRUE ||
jsp.getCurrentToken() == JsonToken.VALUE_FALSE) {
+ jsp.getBooleanValue().toString()
+ } else {
+ jsp.getNumberValue().toString()
Review Comment:
I think our previous logic (and now this else block) is incorrect. We
previously required that the current token to be STRING or NULL, and assert
otherswise. But I think that was wrong. If getSimpleText is called then we
require that the current token is one of the simple types (i.e. string, int,
float, true, false null). If it's not one of those simple types then it means
it was an object or array token, and it means this json doesn't match the
infoset and we we need to throw a `NonTextFoundInSimpleContentException`.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetInputter.scala:
##########
@@ -172,7 +180,13 @@ class JsonInfosetInputter(input: java.io.InputStream)
extends InfosetInputter {
objectDepth -= 1; exitNow = true
// start of a simple type or null
- case JsonToken.VALUE_STRING | JsonToken.VALUE_NULL => exitNow = true
+ case JsonToken.VALUE_STRING |
+ JsonToken.VALUE_NUMBER_INT |
+ JsonToken.VALUE_NUMBER_FLOAT |
+ JsonToken.VALUE_TRUE |
+ JsonToken.VALUE_FALSE |
+ JsonToken.VALUE_NULL =>
+ exitNow = true
Review Comment:
It looks like there is a `JsonToken.isScalar()` function that I think
returns true if it's one of these VALUE_* tokens. So this case can probably be
simplified to `case token if token.isScalar => exitNow = true`. Need to double
check that's the case.
##########
daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetOutputter.scala:
##########
@@ -112,9 +112,17 @@ class JsonInfosetOutputter private (writer:
java.io.BufferedWriter, pretty: Bool
} else {
simple.getText
}
- writer.write('"')
- writer.write(text)
- writer.write('"')
+ if (simple.metadata.dfdlType == DFDLPrimType.String ||
+ simple.metadata.dfdlType == DFDLPrimType.HexBinary ||
+ simple.metadata.dfdlType == DFDLPrimType.AnyURI ||
+ simple.metadata.dfdlType == DFDLPrimType.DateTime ||
+ simple.metadata.dfdlType == DFDLPrimType.Time) {
Review Comment:
Sounds like JSON oesn't support infinity or NaN. So we also need quotes if
dfdlType is float or double and the value is inifinity or nan.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
