Kotlin boolean property setters are skipped if isX prefixed

[IceBlizz6]

Hello

Another interesting Kotlin problem here. I'm not sure if this issue should be reported to graphql-java or here. I will try here first, let me know if it belongs elsewhere.

Look at the following code:

class MyTest {
    var isAlive: Boolean? = null
        set(value) {
            field = value
            println("Hello world")
        }
}

@GraphQLQuery
fun makeTest(input: MyTest): Int {
    return 42
}

This is comparable to a backing field with a setter and a getter. javap displays it like this:

public final class project.MyTest {
  private java.lang.Boolean isAlive;
  public project.MyTest();
  public final java.lang.Boolean isAlive();
  public final void setAlive(java.lang.Boolean);
}

Making a call to makeTest like this query { makeTest(input: { isAlive: true }) }

I expect it to print "Hello world", but it does not. I believe this is because it may assign the value through the backing field directly instead.

Now i rename the field to 'alive'

class MyTest {
    var alive: Boolean? = null
    set(value) {
        field = value
        println("Hello world")
    }
}

javap displays it like this:

public final class project.MyTest {
  private java.lang.Boolean alive;
  public project.MyTest();
  public final java.lang.Boolean getAlive();
  public final void setAlive(java.lang.Boolean);
}

If i call it from GraphQL now then it will print as expected. So it seems like boolean fields prefixed with isX will be set through backing field directly. But boolean fields without the prefix will be set through the setter method.

I expected the setter to be used regardless of property name.

[kaqqao]

EDIT: I got things mixed up. The issue here is with Jackson, not SPQR. See my comment below.

~In a way, this is expected... Because SPQR expects the JavaBean naming convention. At least in Java, there is no relationship between the field and the setter beyond the naming convention and a matching type. So if the naming scheme is violated, there's no clear way of knowing the relationship exists. In Kotlin, the relationship is likely maintained by different means (and I expect Kotlin reflection to be aware of it), but SPQR isn't really aware of Kotlin, so the issue described above occurs.~

~What I had always been advocating for is a Kotlin aware module, (that would ideally be written in Kotlin, although Java works fine too) that takes care of all Kotlin specific behaviors. It would likely be a small module, without much complications. But. It would ideally be community maintained, as I'm not all that familiar with Kotlin. If you'd care to contribute such a thing, just with this one fix, I'd happily take it as a start.~

[IceBlizz6]

Would such a module for SPQR solve this specific case? or is this a problem with graphql-java?

[kaqqao]

Ignore (almost) everything I said. I got confused 🤦‍♂️ This is about Jackson, nothing else. You just have to add jackson-module-kotlin. That makes Jackson understand Kotlin properties correctly. The root issue is the naming convention, as I described above, but in Jackson, not in SPQR.

[IceBlizz6]

jackson-module-kotlin has already been added to my project. How do i apply it to graphql-spqr?

[kaqqao]

Huh. SPQR normally calls objectMapper.findAndRegisterModules() so it should find it automatically. But I don't know how or what is customized etc. Try:

generator.withValueMapperFactory(JacksonValueMapperFactory.builder()
             .withPrototype(new ObjectMapper()) //customize ObjectMapper as you please, e.g. add a module
             .build())

If you're using SPQR with Spring, you might want to use the Spring-provided ObjectMapper instance (I don't actually advise this, but it might be a good test).

[kaqqao]

If that doesn't help... then Jackson is weird about Kotlin? The thing to note here is that SPQR delegates (almost) all input deserialization to Jackson. So Jackson decides how to instantiate the object and populate its fields. No clue why it would decide against using the setter though 🤷‍♂️ Can you try messing with your class and Jackson directly, and see how it behaves?

[kaqqao]

I tired this quickly* and I indeed do not get "Hello world" printed:

class MyTest {
    var isAlive: Boolean? = null
        set(value) {
            field = value
            println("Hello world")
        }
}

fun main(args:Array<String>) {
    val clazz: Class<MyTest> = MyTest::class.java
    ObjectMapper().registerModules(KotlinModule.Builder()
                   .enable(KotlinFeature.KotlinPropertyNameAsImplicitName).build())
        .readValue("{ \"isAlive\": true }", clazz)
}

So either I did something wrong (did I?) or Jackson did...

*not actually quickly 😶

[kaqqao]

Actually... the more I look at this the more sense it makes. Jackson isn't doing anything wrong. Without KotlinPropertyNameAsImplicitName, it sees isAlive and decides the property is called alive, as per JavaBeans convention. With KotlinPropertyNameAsImplicitName the property is called isAlive but such a property doesn't have a setter. Jackson is simply following the spec, just like SPQR. So you'd have to do some further customizations if to achieve what you want. I can help you do that, but... I first have to ask: why go against the JavaBeans spec in the first place? It exists for a reason and tools and libraries are expecting it to be respected. Why go against the grain here?

[kaqqao]

The easiest you can do is:

class MyTest {

    var isAlive: Boolean? = null
        @JsonSetter("isAlive")
        set(value) {
            field = value
            println("Hello world")
        }
}

fun main(args:Array<String>) {
    val clazz: Class<MyTest> = MyTest::class.java
    ObjectMapper() //No KotlinPropertyNameAsImplicitName
        .readValue("{ \"isAlive\": true }", clazz)
}

It prints "Hello world" as expected. You could probably achieve the same with @GraphQLInputField(name = "isAlive"). There are also a couple of ways to ensure this is applied everywhere. I can whip something up if you're sure that's what you need.

[IceBlizz6]

I actually tried this myself some time ago and reached the same conclusion. You will get a "Hello world" if you rename it to 'alive' by dropping the isX prefix. Which seems to confirm that the bug is indeed in jackson. Thank you for your assistance. I will bring the issue to them.

[IceBlizz6]

Actually didn't see your last 2 messages.

So i think the JavaBean convention probably makes sense for Java. But it doesn't seem to play well with Kotlin here. It makes more sense for me to write a property in Kotlin named 'isAlive' rather than 'alive'. Kotlin doesn't need to specify explicit getter and setter methods as that is handled by Kotlin compiler. I normally don't think about or care about how this is translated into java's getters and setters. If anything maybe it is the Kotlin language designers who should have tried to make Kotlin compatible with JavaBeans convention.

Your @JsonSetter solution is interesting, i may use that as a fallback. But before that i will try to contact jackson maintainers to see what they have to say.

[AnneMayor]

Wow, I encountered the same issue on my project, too. I am gonna apply the way that you guys suggested. Thanks a lot 👍

[AnneMayor]

@IceBlizz6 Thanks for this issue. However, I don't think that this issue should be handled in this library. As @kaqqao said, this issue is related to Java Bean model Convention and Kotlin language convert rules. I think we can resolve by customizing each repos like the way that @kaqqao guided. He also introduced @GraphQLInputField(name = "isAlive") way. Please, consider it🙂

[IceBlizz6]

I agree that the solution for this probably does not belong in this library. We can close this for now, but i'm hoping jackson-module-kotlin will be able to find a better solution in the future.

For reference here is the issue that i created in the jackson-module-kotlin repo. https://github.com/FasterXML/jackson-module-kotlin/issues/798