Jooq: Optionally generate @Nullable annotation for column getters

[rwiggins]

IntelliJ IDEA static analysis understands javax.annotation.Nullable and can warn about possible null pointer dereferences if that annotation is present. It'd be really nice if I could configure jOOQ's code generator to add @Nullable to the column getters for records: maybe under the validation annotation configuration option?

I don't see any _easy_ way for end-users to do this themselves without overriding/re-implementing large portions of JavaGenerator, since JavaGenerator.printValidationAnnotation is a private method.

[randombk]

Hi, as an addendum, can you generate @Nullable annotations on setters and constructor parameters? This will let IDEs know that nulls can be safely passed into those methods.

[lukaseder]

@randombk: Thanks for the reminder. If we do proceed with implementing this, the annotation will be present on whatever item it makes sense to put it.

[stangls]

Please implement this feature!
It would be a great way of improving null-safe coding for all the Kotlin users out there (which are steadily increasing)
There is already this post on the jooq website about jooq & kotlin, which generates hope for all the Kotlin users to have a nice typesafe db-layer!

[okkero]

Is this still being implemented? I would greatly benefit from this if it were implemented. I am a Kotlin user, and having to rely on platform types where all bets on nullability are off, especially seeing as JOOQ is such a central part in our backend, is inconvenient at best.

[lukaseder]

Thank you for your feedback, @stangls / @okkero.

Well, we're already generating nullability information through JPA annotations and bean validation annotations, so these ones here wouldn't strictly be against what we already have.

But unlike JPA annotations, for instance, they make a wrong promise. The JPA nullable flag indicates what is permitted inside of the database once a column is stored. The JSR-305 annotation, for instance, indicates what is allowed in the domain object (the POJO) or the TableRecord. When we initialise such an object in Java, the value is indeed null (what else could it be), so the promise is a false one, practically. There are other situations (e.g. when using OUTER JOIN) where the nonnull promise won't be correct.

I'd like to better understand the use-case for Kotlin though, as it seems to be a recurring theme in combination with this annotation. How would you profit from this in Kotlin? Can you show some examples?

[okkero]

Thank you for the prompt response.

So, Kotlin takes the issue of nullability to the compiler. To do this, it treats nullable (kind of like the Java Optional<T>, but as a language construct) and non-null types as two distinct types at the language level. The compiler will not allow any unsafe operations to be done on a reference of a nullable type. That is, in order to dereference a nullable reference you have to handle the null case (check for null, or use safe operators like ?. and ?:), after which you can treat the reference as a non-null reference.

This is in contrast to Java, which doesn't have any sort of null validation at compile time. So when dealing with Java methods in Kotlin, there is no way of knowing whether it should be nullable or not. Consequently Kotlin deals with these values specially, in that it does not do any compile time validation on them (but may still generate runtime checks). Kotlin calls these "platform types".

So as a Kotlin user, when dealing with these platform types, I cannot rely on the nullsafe type system to prevent me from doing any unsafe operations.

So, how do we benefit from nullability annotations? Well, plain Java code is inherently not nullsafe in the way described above, but Java code annotated with special nullability annotations, can be treated specially by the Kotlin compiler. If Kotlin encounters a Java member or parameter annotated with a 'nullable'-annotation, it will treat member or parameter as nullable. Likewise, if a Java member or parameter has a 'non-null'-annotation, it will be treated as definitely not null. No platform types! Great! Here is a list of all the supported nullability annotations (and other specially treated annotations): https://github.com/JetBrains/kotlin/blob/master/core/descriptor.loader.java/src/org/jetbrains/kotlin/load/java/JvmAnnotationNames.kt

Examples:
Java declarations:

public class Test {

    public static String m1() {
        return null;
    }

    @Nullable
    public static String m2() {
        return null;
    }

    @NotNull
    public static String m3() {
        return "m3";
    }

    public static int m4(String input) {
        //We actually expect input to be non null
        return input.length(); //may throw NPE
    }

    public static int m5(@NotNull String input) {
        return input.length(); //Fine. By contract, input shouldn't be null.
    }

}

Kotlin callsite:

    val s1 = Test.m1() //platform type. no compile time null safety
    println(s1.length) //this will throw an NPE

    val s2 = Test.m2() //nullable type. compile time null safety applies
    println(s2.length) //Compile time error: only safe or non-null asserted calls allowed

    val s3 = Test.m3()  //non null type. we can be confident that this value is not null,
                        //per the contract established by the @NotNull annotation
    println(s3.length)  //fine

    val i1 = Test.m4(null)  //Throws an NPE in the Java code.
                            //Kotlin allows null to be passed because the parameter is of a platform type.

    val i2 = Test.m5(null) //Compile time error: Null can not be a value of a non-null type String

Hope this clarifies a bit :)

[lukaseder]

Thank you very much for the clarifications. I'm sorry, I should have been more clear. I know how Kotlin models these things (investigated the language some time ago). I meant how you could profit from these features specifically in the context of using the jOOQ API. Because any @NotNull guarantee on a generated POJO or TableRecord is a "lie" in my opinion. Example:

class Book {
  @NotNull Integer id;
}

class BookRecord extends UpdatableRecordImpl<...> {
  @NotNull Integer getId() { ... }
}

And then:

// Sorry if syntax isn't entirely correct. My Kotlin isn't very fluent:
val book = ctx.newRecord(BOOK);

// This is actually null right now, because the record wasn't fetched from the DB
val id1 = book.id

// This is also null right now
val id2 = book.into(Book::class.java).id

val result = 
ctx.select()
   .from(AUTHOR).leftJoin(BOOK).on(AUTHOR.ID.eq(BOOK.AUTHOR_ID))
   .limit(1)
   .fetchOne();

val book = result.into(BOOK)

// This could be null or not, depending if LEFT JOIN found a book for the author
val id3 = book.id

I hope this clarifies my concerns... I'd like to know what the expectation of a Kotlin developer is with respect to the fact that a @NonNull annotation's semantics cannot be guaranteed by the jOOQ API.

[okkero]

I'm sorry, I've been under the impression that values found in record objects are perfect representations of values found in the database. But I see now that this is not necessarily the case for nullability, and may even be unfeasible if JOOQ allows for partially initialised record objects. Which is probably even unavoidable in the case of outer joins (I hadn't thought about that.)

Your argument makes sense, and I haven't used JOOQ that much, so my knowledge of its API and internal workings is somewhat limited. I will see if I can suggest a solution after doing some more research and testing.

[lukaseder]

Which is probably even unavoidable in the case of outer joins (I hadn't thought about that.)

No worries :)

I've studied the approach of the Slick library in Scala time and again. They make use of primitive type Int and Option[Int] for nullables, which completely destroys the entire query type system, because generics/monads are really the wrong tool to model SQL NULL.

I've come to the conclusion that ultimately, in a SQL library, we just have to accept NULL as something unavoidable. Everything is nullable. A NOT NULL constraint only has a very limited scope, and it is really just a constraint on data that is stored in a physical column. Not a type modifier. In fact, some databases even allow for NOT NULL (and other constraints) to be deferred, which means you're allowed to insert NULL as long as you rectify this by the end of the transaction. Nice feature, eh? :)

[okkero]

Actually, thinking about it. Nullable fields in the database will surely always be nullable on the JVM as well. It should be perfectly reasonable to expect such fields to potentially contain null.

So, what if, for database fields that are not marked as NOT NULL, their getter methods and setter parameters are marked as @Nullable, and the rest can remain not annotated.

That should be good enough for most cases, I think.
Thoughts about this?

[lukaseder]

I see, so in the case of Kotlin, you'd get no warning (and live with the risk and guilt) of running into NPE for NOT NULL fields in rare occasions (left join, union, incomplete initialisation, etc.), but at least when a field is explicitly nullable, you are forced to deal with it explicitly as well?

That certainly seems to make sense. I'll revisit this again later but I think that's feasible.

[okkero]

but at least when a field is explicitly nullable, you are forced to deal with it explicitly as well?

Yes, that is correct.

Thanks for taking your time looking into this :)

[lukaseder]

I've changed my opinion about the utility of these annotations for tooling and language interoperability. I'm even considering adding JSR-305 annotations throughout the jOOQ API itself, which can make a lot more guarantees than generated classes, e.g. the return type of Condition.and(Condition) can certainly be annotated @Nonnull as jOOQ can guarantee this, much like the argument of Field.eq(Field) can be annotated @Nullable as jOOQ can handle this, too. This is will be done in https://github.com/jOOQ/jOOQ/issues/6244 (perhaps in 3.10, certainly in 3.11). The only open question is how to handle the dependency.

With the above in mind, we'll definitely implement opting into generating @Nullable annotations on generated types - probably already in jOOQ 3.10.

[okkero]

This is good news. It will definitely improve my experience using JOOQ with Kotlin :)

[stangls]

Great news!
This will be a major improvement not only for Kotlin users.

[lukaseder]

Gah, what a stupid mess...

While licensing doesn't matter for generated code (at least not for jOOQ, only for end users), it does for the implementation of https://github.com/jOOQ/jOOQ/issues/6244, as we will probably have to ship the dependency with jOOQ for users to be able to build jOOQ. And the JSR-305 annotations are of @Retention(RUNTIME), so they would be part of our deliverables.

And with JSR-305 being dormant and not officially released yet, we (and our users) might get into trouble by adding a dependency:

• https://github.com/findbugsproject/findbugs/issues/88
• https://github.com/findbugsproject/findbugs/issues/128

The only alternative I see right now for Kotlin would be to depend on the Jetbrains annotations (which are ASL 2.0 licensed, thus fine), but I don't really fancy that option too much.

I guess this means postponing the feature once more.

[okkero]

At any rate I'm glad this is on the table :)

[lukaseder]

Yeah, sure! We'll find a solution. But it is a mess, so this certainly deserves more thought :)

Let's hope Kotlin will pick up more momentum. That'll put some pressure on a variety of people, including Google (who are backing JSR-305 and now support Kotlin on Android)

[rwiggins]

If adding support directly to jOOQ is a nonoption because of licensing issues, that's cool... A possible workaround was hinted at in my original post:

I don't see any easy way for end-users to do this themselves without overriding/re-implementing large portions of JavaGenerator, since JavaGenerator.printValidationAnnotation is a private method.

If JavaGenerator could be altered such that subclasses can print arbitrary validation annotations (or something like that), then end-users could implement this functionality themselves. Less than ideal, I realize, but it's a path forward at least.

Edit: If JavaGenerator is altered to accommodate subclasses that print custom validation annotations, it also shouldn't be too difficult to provide an optional jar that contains a Jsr305JavaGenerator (name t.b.d. obviously) that uses one of the JSR-305 implementations. That could be nice.

For what it's worth, I originally opened this issue because our product ended up having a NullPointerException that would've been caught in dev if @Nullable was used for nullable DB column getters in the TableRecord classes. I think implementing it is a clear win for my use-case at least, not to mention the Kotlin stuff.

As far as @NotNull goes, I understand that has correctness issues with .newRecord() et al., so I don't expect @NotNull to ever be generated by jOOQ without a significant rework of how new records work. That said, I don't think @Nullable has that problem.

[stangls]

Java has become a mess... :-/
I had to go back from Kotlin to Java for a project and boy is it a mess!

[lukaseder]

If JavaGenerator could be altered such that subclasses can print arbitrary validation annotations (or something like that), then end-users could implement this functionality themselves. Less than ideal, I realize, but it's a path forward at least.

There are generatePojoGetter() and generateRecordGetter() which can be overridden. Is there anything missing for you?

Note that allowing annotations at arbitrary locations is out of scope for the current JavaGenerator. We're planning on re-designing it in jOOQ 4.0 for much better customisation.

I don't expect @NotNull to ever be generated by jOOQ without a significant rework of how new records work

It simply doesn't make sense in a SQL context where nullability is a constraint put on some references of a type that is a priori always nullable.

That said, I don't think @Nullable has that problem.

I agree, but Java has the problem of not finding concensus here :)

[alamothe]

Just stumbled upon this issue.

I'm using immutable POJOs in Kotlin. I understand that @NotNull and @Nullable are problematic for Records. That's fine with me - I think conceptually, Record should be treated as a builder.

However, I don't see a reason not to generate these annotations for immutable POJOs (apart from joins that were already mentioned?).

Currently, JOOQ only generates @NotNull and not @Nullable. Also, it generates them both for records and pojos.

At least JOOQ could generate @Nullable for pojos as an improvement. Thoughts?

[rwiggins]

Ah, yes, I never came back to this. I did end up taking Lukas's suggestion of using generateRecordGetter to make this a reality:

public class NullableJavaGenerator extends JavaGenerator {
    @Override
    protected void generateRecordGetter(TypedElementDefinition<?> column, int index, JavaWriter out) {
        if (generateValidationAnnotations()) {
            if (column.getType().isNullable() && !column.getType().isDefaulted() && !column.getType().isIdentity()) {
                out.println();
                out.tab(1).println("@%s", out.ref("javax.annotation.Nullable"));
            }
        }

        super.generateRecordGetter(column, index, out);
    }
}

It's not the prettiest output (the Nullable goes in sort of a weird spot, above a comment), but it works.

We don't use jOOQ POJOs, but the same thing could probably be done with generatePojoGetter() pretty easily?

[lukaseder]

Currently, JOOQ only generates @NotNull

Which has probably been a mistake. I'm more in favour of dropping this feature.

At least JOOQ could generate @Nullable for pojos as an improvement. Thoughts?

As far as I'm concerned, any out-of-the-box support of this stuff is the opposite of an improvement, because it will lead to more discussions that distract me from doing more interesting stuff :-) (given that there's not going to be a clean and correct solution in this space)

[lukaseder]

We don't use jOOQ POJOs, but the same thing could probably be done with generatePojoGetter() pretty easily?

Yes

[alamothe]

Which has probably been a mistake. I'm more in favour of dropping this feature.

Completely understand the reasoning @lukaseder! And I agree with you.

My point was that, in the case of immutable POJOs, the annotations could make sense (and only in this case). What I'm trying to do is: use Records without annotations, and treat them conceptually as builders, but use immutable POJOs for concrete records that come from DB. Not sure if I'll get far 🙂

@rwiggins Thanks for sharing your solution, will try to implement something like it.

[lukaseder]

My point was that, in the case of immutable POJOs, the annotations could make sense (and only in this case).

How do you even document such a thing? :-)

[alamothe]

For now, I ended up making a tiny change to generate @Nullable and to not generate @NotNull for records.

@Nullable is super clear, and telling Kotlin compiler that it can be null instead of treating it as ambiguous.

@NotNull is much harder one to crack, if at all possible given the semantics of SQL.

Weirdly, JOOQ went in the opposite direction but I understand this was by popular demand.

[lukaseder]

Thanks again everyone for the discussion. I'm tackling this now (finally) for jOOQ 3.13. There are two relevant issues in whose context this will be done:

• https://github.com/jOOQ/jOOQ/issues/9733 Add and (to allow for specifying which type of annotation should be generated, defaulting to JSR-305)
• https://github.com/jOOQ/jOOQ/issues/9734 Add and (to enable / disable the generation of these annotations, defaulting to false)

I'm tracking this under separate issues as this one has turned a bit verbose. Closing this as a duplicate of the other ones.

[lukaseder]

Alrighty. This is ready for 3.13, which is due soon, in Q1 2020