search

FasterXML / jackson-module-kotlin

Module that adds support for serialization/deserialization of Kotlin (http://kotlinlang.org) classes and data classes.
Apache License 2.0
1.12k stars 175 forks source link

KotlinModule consturctor changed from 2.15.3 to 2.16.0 causing binary compatibility issues #729

Closed florianmutter closed 9 months ago

florianmutter commented 9 months ago

Search before asking

Describe the bug

We use a library (ktorm) that was built against jackson-module-kotlin v2.12.3.

Seems like this version was compatible up to release v2.15.3 but not with v2.16.0 because of the consturcor change. This yields the following error:

java.lang.NoSuchMethodError: 'void com.fasterxml.jackson.module.kotlin.KotlinModule.<init>(int, boolean, boolean, boolean, com.fasterxml.jackson.module.kotlin.SingletonSupport, boolean, int, kotlin.jvm.internal.DefaultConstructorMarker)'
    at org.ktorm.jackson.JsonSqlTypeKt.<clinit>(JsonSqlType.kt:34)

To Reproduce

Minimal example here: https://github.com/florianmutter/ktorm-jackson-sample

Expected behavior

Updating the minor version of jackson should not break existing implementations.

Versions

Kotlin: 1.9.10 Jackson-module-kotlin: 2.16.0 Jackson-databind: -

Additional context

Related to #610

k163377 commented 9 months ago

As I commented, this is closed because I don't think this is an issue that kotlin-module should fix. https://github.com/FasterXML/jackson-module-kotlin/issues/610#issuecomment-1817550354

We apologize for any miscommunication that may have occurred.

cowtowncoder commented 9 months ago

Apologies: I contributed to confusion here. While I do think it is better to file new issues, I should not have indicated reported problem will necessarily be fixed the way requested.

As to compatibility: it, unfortunately, depends. Wherever possible we do try to keep new versions backwards-compatibility within same major version -- and sometime oversights can be easily rectified. But unfortunately due to the way Kotlin (and Java) works, it is not easy (or necessarily even possible) to fully prevent some possible breakages: specifically, there is loose concept of "public API", wherein compatibility should be strictly maintained (think of public methods of ObjectMapper) versus "internal API" (set of methods or constructors in Jackson-databind classes that can be overridden but not designed to be overridden or called by objects outside component itself).

Case of constructors for module falls somewhere in-between these endpoints and I can see how they could be thought of as part of Public API. But sometimes there are reasons why it makes sense to use deprecation for a few releases but eventually drop them, if their continued use does not make sense (f.ex not possible to figure out reasonable default values for new arguments that are needed).

All of which is to say that every case of incompatibility needs to be evaluated separately. In this case it is up to @k163377 to decide and I support given reasoning. But I wanted to add some more context on complexity of these decisions.

cowtowncoder commented 9 months ago

One more quick note: as per note on #610: constructor in question was deprecated in 2.13, so while not optimal, removal was consistent with changes done for "internal" APIs -- method/field/accessor should be marked as deprecated for at least 1 minor version (but ideally 2 or more) before being eligible for removal from a minor version update.

But as per my earlier comment, we really try to avoid removing accessible things (including constructors) in minor releases.

florianmutter commented 9 months ago

Thanks for the explanations. I hope the ktorm devs fix it. Just for reference this is the PR @k163377 created: https://github.com/kotlin-orm/ktorm/pull/527