Experimenting with #1467: Support for @JsonUnwrapped in @JsonCreators

[fxshlein]

I was more or less just messing around with an approach to solve this for now without the big refactor mentioned in the issue. Perhaps this is already fine? The test I added is green at least, but I have no idea what I might have missed. After all, if an "object with three properties" test covered a considerable amount of jackson's feature set, I wouldn't be using it... 😆

[cowtowncoder]

Whoa! Impressive work. I share your suspicion wrt test coverage (lack thereof), but it is what it is.

I'd need to read this through with more thought and focus to see if there's anything specifically that wouldn't work (or be against design). In the meantime I added some notes on code style issues (which are obviously not super important but might as well mention now).

Also, assuming we get through this to merge it, one thing needed (if not already done) is CLA:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

which we need before merging the first contribution. It's usually easiest to print, fill & sign, scan/photo, email to cla at fasterxml dot com. Only needed once, good for all future contributions.

[fxshlein]

@cowtowncoder I sent the signed CLA to cla@.... 🙂

Just to verify, here it says to email the CLA to info@..., are both fine?

[fxshlein]

I noticed a minor issue with the placeholder name: If you use two @JsonUnwrapped parameters in a creator without explicitly giving it a name @JsonProperty, they will be marked as duplicates, as they both get the placeholder:

Caused by: java.lang.IllegalArgumentException: Duplicate creator property "@JsonUnwrapped" (index 2 vs 3) for type ...

I added a test case here: https://github.com/FasterXML/jackson-databind/pull/4271/files#diff-1b40e79714a359efac38026f7dfe9dbf45f4def1877d805437ba48955ae9478eR68-R96

From what I can see, there are three solutions to this:

1. Just let it happen, maybe add a special exception message that tells users to simply distinguish the properties using @JsonProperty. On one hand this should fit right in, as the user probably has to have @JsonProperty on all parameters anyways, but on the other hand it would be kind of weird since the name is never actually used.
2. Change the placeholder to include the index ("unnamed @JsonUnwrapped property at N"). This is the one I implemented now, which makes the test case work. This is nice since the name is more unique, and the names also double as human-readable identifiers in error messages.
3. Do a refactor to allow unwrapped properties to bypass all the places where properties are placed in a map, using their name as a key. I started this here: https://github.com/fxshlein/jackson-databind/commit/3bd10a06548260445cfcde0643f885de5537a09d, but it's already a pretty massive undertaking without even touching everything.

[cowtowncoder]

To me (2), adding index for bogus name, sounds reasonable at this point. +1.

[wingsofovnia]

While looking for a stop-gap solution for this in GitHub, I found this (usage) rather straightforward solution for @JsonUnwrapped support in Kotlin data classes. Perhaps this might be helpful for you or others who seek a temp solution.

Looking forward to having this merged. Thank you for the contribution!