Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON output no longer quotes all values #1298

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@
jsp.getCurrentToken() match {
case JsonToken.START_OBJECT => if (objectDepth == 1) StartDocument else StartElement
case JsonToken.END_OBJECT => EndElement
case JsonToken.VALUE_STRING | JsonToken.VALUE_NULL => {
case JsonToken.VALUE_STRING |
JsonToken.VALUE_NUMBER_INT |
JsonToken.VALUE_NUMBER_FLOAT |
JsonToken.VALUE_TRUE |
JsonToken.VALUE_FALSE |
JsonToken.VALUE_NULL => {
// we don't want to start faking element end yet, but signify that
// after a call to next(), we will want to fake it
nextEventShouldBeFakeEnd = true
Expand Down Expand Up @@ -118,10 +123,11 @@
primType: NodeInfo.Kind,
runtimeProperties: java.util.Map[String, String]
): String = {
if (jsp.getCurrentToken() == JsonToken.VALUE_NULL) {
if (!jsp.getCurrentToken().isScalarValue()) {
throw new NonTextFoundInSimpleContentException("Unexpected array or object '" + getLocalName + "' on line " + jsp.getTokenLocation().getLineNr())

Check warning on line 127 in daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetInputter.scala

View check run for this annotation

Codecov / codecov/patch

daffodil-runtime1/src/main/scala/org/apache/daffodil/runtime1/infoset/JsonInfosetInputter.scala#L127

Added line #L127 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests for this to make sure this works as expected?

} else if (jsp.getCurrentToken() == JsonToken.VALUE_NULL) {
null
} else {
Assert.invariant(jsp.getCurrentToken() == JsonToken.VALUE_STRING)
// this handles unescaping any escaped characters
jsp.getText()
stevedlawrence marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -172,7 +178,7 @@
objectDepth -= 1; exitNow = true

// start of a simple type or null
case JsonToken.VALUE_STRING | JsonToken.VALUE_NULL => exitNow = true
case token if token.isScalarValue() => exitNow = true

// skip field names, jackson makes these available via
// getCurrentName(), except for array elements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,40 @@ class JsonInfosetOutputter private (writer: java.io.BufferedWriter, pretty: Bool
} else {
simple.getText
}
writer.write('"')
writer.write(text)
writer.write('"')
if (needsQuote(simple)) {
writer.write('"')
writer.write(text)
writer.write('"')
} else {
writer.write(text)
}
} else {
writer.write("null")
}
}

private def needsQuote(simple: InfosetSimpleElement): Boolean = {
simple.metadata.dfdlType match {
case DFDLPrimType.String => true
case DFDLPrimType.HexBinary => true
case DFDLPrimType.AnyURI => true
case DFDLPrimType.DateTime => true
case DFDLPrimType.Date => true
case DFDLPrimType.Time => true

// json does not support inf/nan double/float so they must be quoted
case DFDLPrimType.Double => {
val d = simple.getDouble
d.isInfinite || d.isNaN
}
case DFDLPrimType.Float => {
val f = simple.getFloat.toDouble
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to convert the float to a double, the Float class has isInfinite and isNan functions.

f.isInfinite || f.isNaN
}
case _ => false
}
}

override def endSimple(se: InfosetSimpleElement): Unit = {
// nothing to do
}
Expand Down
Loading