JsonEncoder produces invalid JSON

[fsteeg]

The JsonEncoder (encode-json) produces invalid JSON for multiple entries. Each entry is a valid JSON object, but they are simply serialized one after the other, not wrapped into a JSON array.

[blackwinter]

Duplicate of #152?

[dr0i]

As @blackwinter noted:
does using 'encode-json | write("stdout", header="[ ", footer=" ]", separator=",\n")' fixes the problem @fsteeg ?

[fsteeg]

Hm, I don't think the argument from #152 is (still) true.

First, the current behavior is not consistent, since encode-xml does produce valid XML for multiple records, while encode-json does not. See http://test.lobid.org/fix, click Load sample, Process, then change Flux to encode-json, Process again. Here are direct links for the produced XML and JSON.

Second, I don't get the argument from #152 that the encoder should (only) produce valid JSON for single records so they can be embedded. Why not produce singe JSON objects for single records, and JSON arrays for multiple records? That would not change the current behavior for single records.

[blackwinter]

Why not produce singe JSON objects for single records, and JSON arrays for multiple records? That would not change the current behavior for single records.

I'd consider producing JSON Lines output a valid use case (in fact, that's what we rely on; and so do you, if I'm not mistaken). This presents a compatibility issue.

As for the consistency with SimpleXmlEncoder, I agree. @cboehme's objection that it would pass incomplete/invalid serializations is already true for XML; either that should be "fixed" (which probably nobody wants) or the JSON encoder should be allowed the same discretion (only with reversed defaults to remain compatible).

[fsteeg]

[...] that it would pass incomplete/invalid serializations is already true for XML; either that should be "fixed" (which probably nobody wants) or the JSON encoder should be allowed the same discretion (only with reversed defaults to remain compatible).

OK, so that would mean we add a boolean writeJsonArray = false option to JsonEncoder?

[blackwinter]

If you're asking me, then yes, that would be my preference. Maybe even invert the logic here: boolean writeJsonLines = true to emphasize that it outputs JSON Lines format by default; then allow the user to call setJsonLines(false) to force a JSON array instead (the array merely being the default representation or an implementation detail, not particularly noteworthy).

[fsteeg]

Maybe even invert the logic here: boolean writeJsonLines = true to emphasize that it outputs JSON Lines format by default; then allow the user to call setJsonLines(false) to force a JSON array instead (the array merely being the default representation or an implementation detail, not particularly noteworthy).

Sounds good to me. We should then consider the behavior of the prettyPrinting = true option, which makes no sense with a new default writeJsonLines = true. Processing should probably abort with an error if both are true.

[blackwinter]

Well, not so fast ;) I mean, you certainly have a point, but we actually do use pretty printing with the current output format (example). So maybe we shouldn't call it JSON Lines after all...

[fsteeg]

So maybe boolean writeJsonArray = false is the better option?

[blackwinter]

Yeah, it probably is... Sorry for the noise.

[fsteeg]

No, it was a good idea, thanks for contributing. In particular it made me aware that by default, the JSON encoder actually writes JSON lines for multiple records, which makes a lot of sense. I'm now thinking that your original suggestion – to use write("stdout", header="[ ", footer=" ]", separator=",\n") after encode-json – is actually the best approach. We' won't be totally consistent with encode-xml anyway (due to to reversed defaults), and it does make sense to view the serialisation of individual JSON documents as an implementation detail of the writer (even if that serialization is itself JSON again). Closing this, if anyone objects feel free to comment or reopen.