Fix the set property's overrides

[nikita2206]

I noticed while working on an unrelated feature proposal, and testing it with a production project, that the jooq-maven-plugin was failing with NPE because the set() method was overwriting the targetDirectory even though override flag was not set and no system properties were passed.

Hopefully I got the idea of method correctly.

The stack trace of the NPE:

Caused by: java.lang.NullPointerException
    at java.io.File.<init> (File.java:278)
    at org.jooq.codegen.GenerationTool.run0 (GenerationTool.java:724)
    at org.jooq.codegen.GenerationTool.run (GenerationTool.java:255)
    at org.jooq.codegen.GenerationTool.generate (GenerationTool.java:247)
    at org.jooq.codegen.maven.Plugin.execute (Plugin.java:253)

Reproducer:

• Go to the jooq-testcontainers-flyway-example locally
• Remove the line that specifies target directory, meaning that Jooq Maven Plugin will go with the default target/generated-sources/jooq
• mvn compile

What happens:

• generator.getTarget().getDirectory() points to the default directory at the moment before this line is executed
• on the following line we get null because we didn't specify the target directory using system property

String p = System.getProperty(property);

• next line finds out that the override isn't true either, which makes it then check if the current target directory equals to the default value, and it does

if (override ? p != null : checkDefault.test(get.apply(configurationObject)))

• because the directory happened to equal to the default, we set the value in the config to whatever comes from the system property jooq.codegen.target.directory, which happens to be null

set.accept(configurationObject, convert.apply(p));

Fix

So if I understand correctly, the logic you were going for is:

• if override is active, then take whatever is in the system properties, even if it's null (allowing us to unset some values, which otherwise would be impossible)
• if override is not active but the system property contains some value (not null) and the current value equals to default then use the value from the system property instead
• if override is not active but the system property contains some value (not null) and the current value is not the same as default then ignore the system property (not sure about this one)

[lukaseder]

Thanks for your report. Can you provide a reproducer and/or a stack trace?

[nikita2206]

Hey @lukaseder thanks for quick reply. To be honest I assumed that it was reproducible in a much more trivial way but I'm seeing now that this is not the case, will get back to you with a reproducer during the day.

[nikita2206]

@lukaseder done :) please, find the reproducer and description in the PR description

[lukaseder]

Ah yes, thanks a lot for the reproducer. I can see it now. I've created an issue for this:
#15168

It seems that only unpublished snapshot versions are affected, so far. I'll check your fix suggestion soon.

[nikita2206]

@lukaseder I couldn't reproduce it with the latest release (3.18.4 IIRC), so I think it's not in any of the releases indeed.

[lukaseder]

I don't think your suggested fix is correct. You suggested

if ((p != null && currentValueIsDefault) || override)

But it should be:

if (p != null && (currentValueIsDefault || override))

[lukaseder]

Fixed in versions:

• 3.19.0 (NullPointerException in code generator when omitting <target> directory #15168)
• 3.18.5 (NullPointerException in code generator when omitting <target> directory - Merge [#15168] #15169)
• 3.17.14 (NullPointerException in code generator when omitting <target> directory - Merge [#15168] #15170)

Thanks again for your report!