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.13k stars 174 forks source link

conflicting getter #473

Closed guai closed 3 years ago

guai commented 3 years ago

Hi There is a project https://github.com/kuberig-io/kuberig-dsl-openshift It generates and compiles some kotlin DSL based on openshift API spec. It uses fasterxml to produce some yaml from this DSL, which can be passed to openshift later.

In one of those DSL classes there are properties csi and iscsi With com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.11.4 there was no problems. But with com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.12.3 I got an exception: java.lang.IllegalArgumentException: Conflicting getter definitions for property "iscsi": io.k8s.api.core.v1.Volume#getIscsi() vs io.k8s.api.core.v1.Volume#getCsi()

I think there should be some global toggle to tell fasterxml not to check for is-getters. For backward compatibility sake. And also for code, which will never have is-getters, like generated kotlin. And annotations is not a good option here since it's not my code, I have no control over it.

And I don't really see why are those conflicting. AFAIK, according to bean spec its either boolean isProp or otherType getProp. So getIsProp can never clash with getProp

guai commented 3 years ago

in decompiled as java code this class have following declarations:

   private final CSIVolumeSource csi;
   private final ISCSIVolumeSource iscsi;
   public final CSIVolumeSource getCsi() {...}
   public final ISCSIVolumeSource getIscsi() {...}

Or maybe fasterxml could use kotlin annotations if they are present. Kotlin adds its @Metadata annotation where properties are declared.

cowtowncoder commented 3 years ago

I suspect this is due to Kotlin's handling of things, unifying potentially conflicting field/method names. Vanilla databind should not so this, I think -- it would infer 2 properties, csi and iscsi, since Bean Naming would suggest that. But Kotlin's rules of "is-getters" are different from Java.

Note, too, that there are configuration settings to prevent discovery of is-getters, for example:

MapperFeature.AUTO_DETECT_IS_GETTERS (enabled by default)

as well as options via @JsonAutoDetect (and matching configurability via ObjectMapper). But Kotlin does override some of that handling.

guai commented 3 years ago

disable(MapperFeature.AUTO_DETECT_IS_GETTERS) doesn't help

guai commented 3 years ago

I think it's a bug. The algorithm should not even try to check for is-getter unless the type is boolean/Boolean.

But Kotlin's rules of "is-getters" are different from Java.

But there are none of those in this case. There are only get-getters

Find all the get-getters, strip get from it. Here is your property (csi and iscsi). If it's a boolean, check if there is also an is-getter (isCsi and isIscsi). This is how it should work IMO. No checks for getIscsi should be involved.

cowtowncoder commented 3 years ago

@guai Please read more about Kotlin definition of "is-getter", naming, before making strong statements: while Java side has specific rules (including that type must be "boolean"), Kotlin EXPLICITLY differs.

I will now move this to Kotlin module since I do not think this would occur without it (happy to move back with a Java-only test that proves otherwise).

MartinX3 commented 3 years ago

I think I suffer by the same issue using kotlin.

My DTO has the constructor property var webinar: WebinarDto and a val isWebinar = true outside of the constructor as a constant value.

On deserialization the webinar disappears in the json and the isWebinar contains the object, instead of the boolean.

disable(MapperFeature.AUTO_DETECT_IS_GETTERS) doesn't help

Renaming the boolean now to iisWebinar as a workaround.

guai commented 3 years ago

@cowtowncoder I'd privide a decompiled java, not a kotlin. this error will occure in the similar java code written by hand.

dinomite commented 3 years ago

@MartinX3 Adding @JsonProperty("serializedName") and perhaps @JsonIgnore annotations should solve your problem.

As for this problem generally, we have other issues that cover it—the state of things is that whichever choice we make it will seem broken for some use cases. See: https://github.com/FasterXML/jackson-module-kotlin/issues/337

guai commented 3 years ago

looks like the work on #337 caused that issue.

dinomite commented 3 years ago

@guai The only work done as part of #337 was adding a test.

MartinX3 commented 3 years ago

@JsonProperty("serializedName")

Thank you @JsonProperty("isWebinar") does work.