Allow exposing CBOR simple values as VALUE_EMBEDDED_OBJECT

[iifawzi]

Hello, this PR is resolving #587 to allow exposing simple values as value embedded objects

[iifawzi]

A separate PR is needed to change defaults for 3.0. Most probably after the merge of this PR I will create that PR with defaults changes for this one, and other two features added recently

[cowtowncoder]

Just realized that I'll probably want change naming here slightly; instead of "handle", either "expose" or maybe "read" or "return". Will go ahead and make that change for both new Feature names.

I think I'll call with READ_ since that is most commonly used by other Jackson components.

[iifawzi]

I think it could be better; I'm not good at naming these feature flags, though, so thank you! :-D

[cowtowncoder]

np, that's what I am ultimately responsible for :)
(not that it was bad name per se, just trying to keep uniform terminology etc)

[cowtowncoder]

There are a few other places this must be done (everywhere where _binaryValue = null; is done)

[iifawzi]

I thought clearing it once at the beginning of nextToken could be enough, given it's used only once for major 7, and there is no partial assignment (always one byte).

[cowtowncoder]

I wouldn't want to count on state not getting cleared for nextFieldName() variants, as they won't call nextToken().
It may be that a combination of things could keep things consistent (as in, not exposing stale simple value) but when adding new features invariants might not hold.

But if you are confident after looking, that's fine, just LMK.

[iifawzi]

on the side; so far I've been exploring the async impl of CBOR; and I'm almost done with my draft, building almost all major types to explore the spec. In next weeks, I will start work on the actual organized PR, However, I'd like to know if you think there are higher priority issues that the community would like to be fixed. I'm enjoying contributing to the project in my free time, so aligning with priorities will make it even more valuable.

After the async parser, I believe looking at Avro issues is worth it, as I think it's the most used (just a guess from the number of issues)

Would appreciate your thoughts.

[iifawzi]

I wouldn't want to count on state not getting cleared for nextFieldName() variants, as they won't call nextToken(). It may be that a combination of things could keep things consistent (as in, not exposing stale simple value) but when adding new features invariants might not hold.

But if you are confident after looking, that's fine, just LMK.

Aha, I got it, and well, I'm not 100% confident about the different combinations that might happen that can lead to exposing stale values, so yeah, better not to count on that. I will commit the change to clear it in places where binary values is cleared

[cowtowncoder]

@iifawzi On async impl: that sounds like something that would be well received so +1 for that. Avro does seem to get more attention that I'd have expected so that's a good thing to follow up on too. And maybe logical type support and whatever "new" (... I haven't been following up spec development) features for CBOR would be good as well.
(Avro, too, has logical type concept).

And ideally Proto3 would be supported by protobuf module. There's also question of replacing protoc-parser (reader for schema) that is deprecated with library that is to replace it (by same authors). I haven't had time to follow up on this, I think there's an issue filed to request it.

[iifawzi]

Thanks a lot for the insights @cowtowncoder! I’ll keep those points in mind as I move forward after the async impl

[cowtowncoder]

LGTM!

[cowtowncoder]

Thank you again, @iifawzi. I am now merging to 3.x, after which you can do PR to change defaults for features (which are now CBORReadFeatures in 3.0).