Allow exposing CBOR undefined value as JsonToken.VALUE_EMBEDDED_OBJECT

[iifawzi]

Hello, this PR resolves #137 by

• Adding a feature flag to decode undefined value as JsonToken.VALUE_EMBEDDED_OBJECT
• Adding isUndefined method that could be used for both legacy handling and with embedded objects, as an indicator if the last token parsed is undefined or not.

[iifawzi]

Defaults for 3.0 should be true, I believe, so it could take place in a separate PR

[iifawzi]

I'm not sure if we need to touch getEmbeddedObject, now it will return null, I couldn't think of any other alternative than returning the string "undefined" but not feeling so strong about it. I think null maybe remains valid..

[cowtowncoder]

Could use a Javadoc

[cowtowncoder]

I'm not sure if we need to touch getEmbeddedObject, now it will return null, I couldn't think of any other alternative than returning the string "undefined" but not feeling so strong about it. I think null maybe remains valid..

Good question. I don't think constant String would be any better.

But somehow not setting anything does seem odd. I think we can go with that; as long as behavior is tested I think we are ok. Will add one specific suggestion on code, for clearing _binaryValue explicitly: while redundant, works as sort of documentation.

The only (?) concern I have is about future embedded values we might want to return, and whether null having specific semantics might be problematic. But that's hypothetical for now so... no need to solve problem we do not have.

[cowtowncoder]

Hmmmh. Not a fan of this approach but... I guess it'll do. Let's preface it by

(_currToken == JsonToken.VALUE_EMBEDDED_OBJECT || _currToken == JsonToken.VALUE_NULL)

to avoid likelihood of incorrect deterimation (binary segment ending with byte 0xF7).

Also: not 100% sure if bounds checks would be needed.
Actually, must check that _inputBuffer is not null -- _releaseBuffers() clears it.

[cowtowncoder]

Actually I can add that.

[cowtowncoder]

Let's add _binaryValue = null; just as sort of documentation of intent. I know it should be cleared by nextToken() and other places but it seems better be explicit in this particular case.

[cowtowncoder]

Ok I added release notes and made minor tweaks -- actually can add Javadoc too and then merge.

[cowtowncoder]

@iifawzi Merged into 2.20, will merge forward into 3.x, so separate PR just needed for defaults change and unit tests (likely).

[iifawzi]

I'm not sure if we need to touch getEmbeddedObject, now it will return null, I couldn't think of any other alternative than returning the string "undefined" but not feeling so strong about it. I think null maybe remains valid..

Good question. I don't think constant String would be any better.

But somehow not setting anything does seem odd. I think we can go with that; as long as behavior is tested I think we are ok. Will add one specific suggestion on code, for clearing _binaryValue explicitly: while redundant, works as sort of documentation.

The only (?) concern I have is about future embedded values we might want to return, and whether null having specific semantics might be problematic. But that's hypothetical for now so... no need to solve problem we do not have.

Thanks for doing the changes, I also believe for future embedded values (SimpleValue) we need to return the value itself as null would be meaningless and unexpected, but yes, maybe we discuss it in the corresponding pr later

Thank you for completing the missing points