Add from_json()/to_json() Fix functions.

[blackwinter]

Open questions:

1. shouldConvertNullFromJson() seems counterintuitive; but fixing it would require changing the behaviour of Record.transform(String, BiConsumer). Also, we'd have to differentiate between value being null and error_string being null. One option might be to generate an empty string which could then get at least cleaned up by vacuum().
2. to_json() error behaviour is untested; I have no idea how to conjure up an invalid object ;)
3. Should we somehow utilize JsonDecoder/JsonEncoder instead of interacting with Jackson directly? Would certainly allow for greater compatibility/configurability.

@TobiasNx: Do you also want to do a functional review?

[fsteeg]

shouldConvertNullFromJson() seems counterintuitive. [...]

Hm, seems I'm missing something here. That one seems correct, null could not be parsed as JSON, no error_string is given, result is null, seems good. What seems wrong to me is shouldNotConvertInvalidObjectFromJson, which produces "eeny meeny miny moe", but shouldn't it produce null (input is not a quoted string, no error_string given)? And the next one, shouldConvertInvalidObjectFromJsonWithErrorString, should that not be called shouldNotConvertInvalidObjectFromJsonWithErrorString? Are these related to the issue you described here?

Should we somehow utilize JsonDecoder/JsonEncoder instead of interacting with Jackson directly?

Sounds like a good idea. I'm also happy with the direct implementation though, which seems very straightforward. Perhaps we could switch it if/when we first need some config option or encounter a inconsistency.

[blackwinter]

That one seems correct, null could not be parsed as JSON, no error_string is given, result is null, seems good.

null is a valid JSON value. It should be converted into the Java value null (incidentally homonymous), not the string (literal) "null".

What seems wrong to me is shouldNotConvertInvalidObjectFromJson, which produces "eeny meeny miny moe", but shouldn't it produce null (input is not a quoted string, no error_string given)?

It does indeed produce null, but returning null as new value leaves the old value untouched.

That's also what I was referring to: New value produces new value, null produces old value (does nothing), but a new "marker" (other than new value or null) is required to produce an actual null (which we don't accept, of course, so the value effectively gets deleted; similar to lookup(): null with delete: true removes the value, otherwise keeps the old value).

So basically: How should JSON null behave? Produce string "null" (current behaviour) or delete itself from the record (because we don't accept null values)?

Perhaps we could switch it if/when we first need some config option or encounter a inconsistency.

Okay.

[fsteeg]

So basically: How should JSON null behave? Produce string "null" (current behaviour) or delete itself from the record (because we don't accept null values)?

Alright, got it, thanks. I think the current behavior (produce string "null") is good.

[TobiasNx]

Had a look at the function, the docu and the unit test.
Did not play around with it yet.

What would a use case for this look like? Perhaps a short description of a use case could be added in the docu as example. (not the code)

[blackwinter]

What would a use case for this look like?

Our actual use case for from_json() (in combination with flatten(), see #244) is lookup() in a multi-valued map where values are encoded as JSON (because maps can only have string values).

to_json() was more or less a by-product, but already proved itself useful for print_record() (see #238).

[TobiasNx]

Then go for it! +1

[blackwinter]

Then go for it! +1

You mean describing this rather complex use case in the documentation for from_json()? I'm not so sure.

[fsteeg]

You mean describing this rather complex use case in the documentation for from_json()? I'm not so sure.

I think it was a general go for the PR. I agree that our internal use case is not a good example for the documentation. We can always add an example later if we come across something good (like an API or some data that does basically the reverse of this edit: like JSON over OAI-PMH, or embedded JSON in HTML).

[blackwinter]

Okay, thanks!