Minor code cleanup for JSON additions #935
Conversation
Normally, `jsonString()` delegates to `jsonUTF8Data()`, which relies on our efficient general JSON encoder. But when the message type has a custom JSON encoding, that encoding is produced directly as a String, so the delegation unnecessarily translated string->data->string. While here, make `init(jsonUTF8Data:options:)` a bit easier to read.
This will help ensure that both work exactly the same. There have been a couple of minor bugs recently where preconditions were being checked differently in the two paths. While here, I noticed that we were not correctly verifying that decoded JSON was correctly re-encoded. Fixing that in turn revealed a test bug: I was asserting that signed zeros were always preserved, but they are currently not. By my reading of the spec, because -0.0 == +0.0, signed zeros should be treated as default zero values and be omitted.
|
While fixing the handling of empty JSON decoding for the data-based API, I became nervous that there might be other cases where string- and data-based JSON APIs behaved differently. So I expanded the test helpers to run all JSON tests through both string- and data-based APIs, which should catch any future bugs along these lines. Of course, that change in turn uncovered a test inconsistency around floating-point signed zeros. By my reading of the spec, a negative zero should be considered a default value and not encoded in proto3. It's easy to change this if the protobuf folks disagree. |
|
Re negative zero - go ahead and open an issue upstream asking about it. I believe they were having some discussions internally around the floating point edge cases, so filing issues to get this clarified seems like a good idea. |
Normally,
jsonString()delegates tojsonUTF8Data(), which relieson our efficient general JSON encoder. But when the message type
has a custom JSON encoding, that encoding is produced directly as a String,
so the delegation unnecessarily translated string->data->string.
While here, make
init(jsonUTF8Data:options:)a bit easier to read.