Consider dropping deprecated (in 2.12) `PropertyNamingStrategy` implementations from 2.18 or later

[cowtowncoder]

Describe your Issue

(note: inspired by #4109)

As per #2715 there is a nasty race condition possibility for anyone using constants like PropertyNamingStrategy.LOWER_CAMEL_CASE or classes themselves like PropertyNamingStrategy.SnakeCaseStrategy.class; and because of this, constants and implementations were marked as deprecated.

But while these are deprecated, problematic usage is still possible. Question then is, for existing legacy usage, which is worse:

1. Continuing potentially risky usage, leading to potential breakage (likely hard to diagnose)
2. Explicit breakage if these classes and constants are removed and Jackson dependency is upgraded

Arguably (1) is worse, as even new code could start using problematic classes, constants (when copy-pasting). At the same time, SemVer would suggest we only remove these from Jackson 3.0 (from which they are removed, see master.

I propose these classes, constants, would, however, be removed earlier than this, from Jackson 2.17. Please add comments on WDYT.

[JooHyukKim]

~~+1 on "Not" dropping deprecated class~~ updated my vote

🔍 Reasons

1. To follow SemVer : We want people to know that Jackson follows SemVer, not the opposite.
2. Minimize compatibility issues : Building(compiling) projects will give deprecation warnings which users are responsible to at least read them.
3. Avoid influx of inquiries : there will be. Even with best-effort warning announcements, people will come inquiring, filing issues, and making PRs, handling them and explaning over again will take up too much resources that could've been spent on improvements --compare this with recent CVE happening.

📑 Todo's (More like what I think we can do)

1. Amplify Warnings : If there is something more stonger than @Deprecated
2. Documentation: Enhance visibility of deprecation and related risks in documentation.
3. Encourage users to encourage each other : to use non-deprecated one.

In essence

Pragmatic perspective, not dropping the deprecated will improve things.

[takezoe]

Thank you for raising this topic.

I personally prefer to drop these fields even in 2.x series because impact of dead-lock issue caused by these deprecated classes is significant depending on the use case even though it could happen very rarely. However, I also understand importance of backwards compatibility in SemVer concept. That's reason why I raised #4109 which (slightly) mitigates the issue without breaking backwards compatibility.

One idea is that mentioning a potential dead-lock in the deprecated message. It may make risk of using deprecated classes easier to notice.

However, we have no ways to realize that deprecated classes are used unless recompile from the source code, in particular, if those classes are used inside third-party libraries. I initially thought that producing warning log in a constructor of deprecated classes might be helpful but Jackson doesn't have a logger, unfortunately.

Anyway, it would be great if we can do something to reduce risk of dead-lock issue even if deprecated classes will be maintained in 2.x series.

[cowtowncoder]

@takezoe Hmmmh. I'd be ok adding use of JDK-bundled Logger for constructors of deprecated instances. If you or anyone else wanted to do a PR.

Jackson tries to avoid use of loggers in general, but this would be reasonable case to use them.

We could even combine approach to get logging into 2.16, warning about imminent removal, then consider removal for a later 2.x version (whether 2.17 or later).

[takezoe]

I see. I have seen https://github.com/FasterXML/jackson-databind/pull/2913 that dropped a dependency on java.logging module. Is it okay to re-add?

[cowtowncoder]

Yes, if we add logging, would need that to be re-added as part of changes.

[JooHyukKim]

@takezoe does have a point also.

[takezoe]

Created a pull request for warning logs: https://github.com/FasterXML/jackson-databind/pull/4144

[JooHyukKim]

Created a pull request for warning logs: https://github.com/FasterXML/jackson-databind/pull/4144

+1 on work on more and merge #4109, if logging were to be considered.

Because of potential side effects around 'java.logging' deps. Thanks

[cowtowncoder]

Merged #4144 so Jackson 2.16 will start warning about usage.

We can then consider dropping from 2.17 or perhaps 2.18 (depending on how things go).

[genslein]

What is the workaround or replacement for using databind annotation `@JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class) then? I've not seen any documentation allowing equal changes

[takezoe]

@genslein @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) should be used.

[cowtowncoder]

Quick note: I don't think this should be yet done for 2.17, will update title.