Fix #3906 by forcing Record constructors to be visible regardless of visibility overrides

[cowtowncoder]

Fixes #3906 as per title.

[cowtowncoder]

@yihtserns WDYT?

[yihtserns]

As noted by this comment, private canonical constructor (i.e. for private record XXX(...) { }) is visible for deserialization before #3724 (tested & confirmed using 2.14.2).

Do we want to change NON_PRIVATE to ANY to preserve that behaviour?

[yihtserns]

(Sorry for going MIA with regards to #3906 - besides day job, I was under the impression that it wasn't urgent so I've been researching for a different way to fix that)

[cowtowncoder]

@yihtserns Hmmh. Good question. I don't know to be sure; perhaps?

But I also noticed that there's one regression here, for RecordImplicitCreatorsTest: doing this will now expose implicit factory methods (valueOf()).

And NP for no follow up: I agree that this is not the most important thing to tackle. And with the issues there are maybe another approach would be useful.

[CatarinaRibas]

Hi,

Can you please fix this problem? I think this is the reason for some troubles that we have been having.

[cowtowncoder]

Unfortunately this breaks some tests. So someone with time should figure out what the problem is, and whether this fix is actually safe to apply or not. I can't (and shouldn't) fix in PR that breaks existing tests.

@CatarinaRibas perhaps you have time to dig into this to help?

Discussion on #3906 side may be a good place to continue discussions on people involved; I created PR but am not most knowledgeable here (nor have time right now)

[yuzawa-san]

@cowtowncoder looks the failures are related to this. the expected result for that is causing a single test (based around that specific disabled AUTO_DETECT_CREATORS case) to fail.

i got the tests to pass when i changed the condition in the new code to

ClassUtil.isRecordType(baseType) && isEnabled(MapperFeature.AUTO_DETECT_CREATORS)

[cowtowncoder]

Superceded by #4057 (targeting 2.16, too risky for patch release)