jackson-module-kotlin icon indicating copy to clipboard operation
jackson-module-kotlin copied to clipboard

enum @JsonCreator method called with wrong parameter type when using customized visibility ObjectMapper

Open rnpy opened this issue 7 years ago • 13 comments

I have a weird issue using @JsonCreator on enums, but only when using an ObjectMapper with customized visibility. Depending on the name of the parameters of the @JsonCreator and constructor methods, the deserialization will fail or succeed. Tested using Jackson 2.8.7 with Kotlin 1.1.2, adding kotlin-reflect or not does not change the results.

Short snippet to reproduce the issue: those 2 classes are exactly the same, except for the name of fromInt parameter (JacksonTest class uses the same name as the enum constructor, while JacksonTest2 uses a different name)

enum class JacksonTest(private val value: Int) {
    TEST(0), TEST2(1);
    companion object {
        @JvmStatic @JsonCreator
        fun fromInt(value: Int): JacksonTest {
            return JacksonTest.values().find { it.value == value } ?: JacksonTest.TEST
        }
    }
}

enum class JacksonTest2(private val value: Int) {
    TEST(0), TEST2(1);
    companion object {
        @JvmStatic @JsonCreator
        fun fromInt(intValue: Int): JacksonTest2 {
            return JacksonTest2.values().find { it.value == intValue } ?: JacksonTest2.TEST
        }
    }
}

Then running the following tests, the first one fails when using custom visibility on ObjectMapper, but only when using JacksonTest enum, it works well with JacksonTest2 (which is exactly the same class, with just a renamed parameter):

@Test
fun jacksonTest1_customMapper_fails() {
    val mapper = ObjectMapper()
    mapper.registerModule(KotlinModule())

    mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
    mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY)
    mapper.setVisibility(PropertyAccessor.CREATOR, JsonAutoDetect.Visibility.ANY)

    val deserialized = mapper.readValue("1", JacksonTest::class.java) // throws exception
    Assert.assertEquals(deserialized, JacksonTest.TEST2)
}

@Test
fun jacksonTest1_defaultMapper_succeeds() {
    val mapper = ObjectMapper()
    mapper.registerModule(KotlinModule())

    val deserialized = mapper.readValue("1", JacksonTest::class.java)
    Assert.assertEquals(deserialized, JacksonTest.TEST2)
}

@Test
fun jacksonTest2_customMapper_succeeds() {
    val mapper = ObjectMapper()
    mapper.registerModule(KotlinModule())

    mapper.setVisibility(PropertyAccessor.ALL, JsonAutoDetect.Visibility.NONE)
    mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY) // works with or without this
    mapper.setVisibility(PropertyAccessor.CREATOR, JsonAutoDetect.Visibility.ANY) // works with or without this

    val deserialized = mapper.readValue("1", JacksonTest2::class.java)
    Assert.assertEquals(deserialized, JacksonTest2.TEST2)
}

@Test
fun jacksonTest2_defaultMapper_succeeds() {
    val mapper = ObjectMapper()
    mapper.registerModule(KotlinModule())

    val deserialized = mapper.readValue("1", JacksonTest2::class.java)
    Assert.assertEquals(deserialized, JacksonTest2.TEST2)
}

It seems that in the first case, when both parameters have the same name and ObjectMapper uses customized visibility, then Jackson tries to call that @JsonCreator method with a String parameter instead of an Int.

com.fasterxml.jackson.databind.JsonMappingException: Can not construct instance of ***.model.JacksonTest, problem: argument type mismatch at [Source: 1; line: 1, column: 1]

But for some reason, if the parameters have different names, then everything is working as expected.

rnpy avatar Jul 03 '17 00:07 rnpy

@cowtowncoder this seems to be something in databind since the kotlin module doesn't do anything with types. If it is handed a string as a parameter it uses a string as a parameter. This seems familiar and maybe related to the other reported issue here about enum not working with int as the parameter type, but only as string

(this was one of the other issues, https://github.com/FasterXML/jackson-module-kotlin/issues/75 ... but I remember this coming up a while ago as well in another older)

apatrida avatar Jul 10 '17 13:07 apatrida

I think the first step would be to double-check that this occurs with latest patch, 2.8.9.

cowtowncoder avatar Jul 12 '17 21:07 cowtowncoder

Just tried running the tests with 2.8.9, same result:

com.fasterxml.jackson.databind.JsonMappingException: Can not construct instance of ***.model.JacksonTest, problem: argument type mismatch at [Source: 1; line: 1, column: 1]

rnpy avatar Jul 13 '17 02:07 rnpy

Thank you for verifying this.

I'll create a new issue on databind, linking to this one -- I hope this is reproducible on pure Java version.

cowtowncoder avatar Jul 13 '17 03:07 cowtowncoder

Ok, I can not reproduce this with Java, but I do have one guess: could it be that Kotlin module might be recognizing factory method as property-based creator (one that maps by name), and not delegating one? Easiest fix here would be to add explicit mode property for @JsonCreator:

@JsonCreator(mode=JsonCreator.Mode.DELEGATING)

which would ensure correct mode is used.

Question of why mode is detected is more complex and it's something that may or may not be fixable here -- but let's first figure out if this is the problem.

cowtowncoder avatar Jul 21 '17 20:07 cowtowncoder

Fwtw forcing mode PROPERTIES on Java side does produce exception that sounds similar to what was reported. So I suspect my guess is correct.

cowtowncoder avatar Jul 21 '17 20:07 cowtowncoder

@cowtowncoder I can confirm your guess. I ran into a similar issue when upgrading from Jackson 2.7.7 to 2.9.3. I didn't set any specific visibility configurations on the ObjectMapper. I did some debugging into Jackson's internals and it was, indeed, inferring PROPERTIES for the JsonCreator in the following test class:

enum class Test(val id: Int) {
    A(10), B(20), C(30);

    companion object {
        @JsonCreator
        @JvmStatic
        fun findById(id: Int) = values().find { it.id == id }
    }
}

It appears that Jackson infers this mode when it matches the parameter name from findById (id) against the class properties (which has an id field). Renaming the method parameter to anything else (in my test I literally renamed it to abc) makes Jackson infer DELEGATING instead, and the deserialization works as expected. I also tested the mode override via annotation (@JsonCreator(mode=JsonCreator.Mode.DELEGATING)), which also solved the problem.

From my perspective, it doesn't make sense at all to use PROPERTIES binding on enums, since its entries are predefined and if an entry has any properties, they should be final anyway (at least for me, it doesn't make sense for Jackson to invoke setters at enum classes). That said, I'm not suggesting that Jackson should use a different approach when inferring the type of binding to use on creators, but it would be really useful if a global mode override could be set for enums instead of overriding mode at all of our Kotlin enums =)

jose-cieni-afterverse avatar Mar 22 '18 21:03 jose-cieni-afterverse

@jose-cieni-movile It does sound like heuristics to figure out mode may have backfired here, but I think the solution really is to define mode explicitly for @JsonCreator. As to type being enum: while I agree that with full context likely guess should be different, unfortunately such information is not really available for code that makes determination.

This wrt core Jackson databind: perhaps Kotlin module could try more elaborate handling. That'd be for @apatrida to comment.

If it was me, I would add mode selection to @JsonCreator here to ensure correct operation.

cowtowncoder avatar Mar 22 '18 21:03 cowtowncoder

Is there any improvement on this issue? Both of the workarounds work in latest version but would be nice get this fixed.

buremba avatar May 05 '18 22:05 buremba

I seem to be having this issue with a pure Java application (no Kotlin). The Mode.DELEGATING workaround solves the problem for me, too. Using Jackson v2.9.8.

ianfp avatar Feb 13 '19 19:02 ianfp

Still very complicated to just parse a value as a string. Instead of just using the provided constructor (Kotlin) you still have to

  • create a companion object
  • create a method like fromInt(number: Int) = values().first{it.number == value}
  • annotate with @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
  • annotate with @JvmStatic

Im mean, seriously, WTF?!

Why not simply like this?

enum class Quarters @JsonCreator constructor(val number: Number) {
   ONE(1),
   TWO(2),
   THREE(3),
   FOUR(4);
}

spyro2000 avatar Feb 25 '21 10:02 spyro2000

@spyro2000 Since you know how to do that and how simple it is, maybe you can submit a fix?

cowtowncoder avatar Feb 25 '21 18:02 cowtowncoder

@spyro2000 That'd be a great contribution, make a PR and I'll give it a look.

dinomite avatar Mar 17 '21 10:03 dinomite