@UseFreemarkerSqlLocator is ignored when it comes before @UseFreemarkerEngine

[xak2000]

Currently the ordering of @UseFreemarkerSqlLocator and @UseFreemarkerEngine annotations on a type or a method matters, while I believe it should not.

When @UseFreemarkerEngine comes before @UseFreemarkerSqlLocator, then the result is expected: JDBI tries to find a file named {methodName}.sql.ftl and then tries to use FreemarkerEngine to build the SQL query.

So, this works as expected:

public interface Dao {

    @SqlQuery
    @UseFreemarkerEngine
    @UseFreemarkerSqlLocator
    long engineThenLocator(int limit);

}

When @UseFreemarkerEngine comes after @UseFreemarkerSqlLocator, then JDBI tries to interpret method name itself as an SQL query, i.e. it ignores @UseFreemarkerSqlLocator annotation on a method (or on an interface), like it was never present.

I believe this is a bug, as logically the order of these annotations should not change the result. Current behavior is very surprising and error-prone.

This code thows an exception:

public interface Dao {

    @SqlQuery
    @UseFreemarkerSqlLocator
    @UseFreemarkerEngine
    long locatorThenEngine(int limit);

}

org.jdbi.v3.core.statement.UnableToCreateStatementException: org.h2.jdbc.JdbcSQLSyntaxErrorException: Syntax error in SQL statement "[*]locatorThenEngine"; SQL statement:
locatorThenEngine [42000-224] [statement:"locatorThenEngine", arguments:{positional:{0:10}, named:{limit:10}, finder:[]}]

The only difference is the ordering of @UseFreemarkerSqlLocator and @UseFreemarkerEngine annotations.

I created a minimal example with 4 test methods each tests a specific ordering of the annotations on an interface or on a method. You can clone it and run tests. JDBI version used: 3.43.0.

[stevenschlansker]

Thanks for reporting this. I agree in general annotation order should not matter, but in practice I am not sure anyone checks this thoroughly. And, there definitely are some cases where order matters - for example order of registering mappers that can work with some of the same types. We'll see if we can make any improvements here.

[xak2000]

Thanks!

I agree about order of registering mappers. It is somewhat expected. Maybe unusual for annotations, but still makes sense to me.

But with @UseFreemarkerSqlLocator and @UseFreemarkerEngine it is very unexpected behavior and doesn't make any sense to me. I didn't check the implementation of handler of @UseFreemarkerEngine annotation, but logically it should not change the SQL source, only the engine that will be used to parse it. While @UseFreemarkerSqlLocator should only change the SQL source and not the engine. So, these 2 annotations should not be in the way of each other. At least, it is how I understand the docs.

Offtopic: I think that @UseFreemarkerSqlLocator could have been implemented in a way so it will change the engine too, because there is not a lot of sense to have an SQL query in the .ftl.sql file that will not use Freemarker template engine and will use some other template engine instead. :) But, of course, it is totally different story, unrelated to this issue. I just remembered my thoughts when I encountered these annotations for the first time: it was strange to always put both on a method. I thought the reason for such implementation was to make these 2 annotations totally unrelated (to not affect each other, even accidentally).

[stevenschlansker]

Agree with all of your points. Using the sql locator implying also using the engine seems reasonable to me.

[hgschmie]

However, it is not that easy. :-) The problem is not the locator. In fact, you should omit the @UseFreemarkerEngine annotation if you use the @UseFreemarkerSqlLocator annotation.

What happens is that the @UseFreemarkerEngine sets the template engine to the freemarker engine (which is reasonable), but the @UseFreemarkerSqlLocator sets the engine to the freemarker engine wrapped in a classpath loader here: https://github.com/jdbi/jdbi/blob/master/freemarker/src/main/java/org/jdbi/v3/freemarker/internal/UseFreemarkerSqlLocatorImpl.java#L57

If you put both annotations in the "wrong" order, then the locator annotation sets the engine correct, then the freemarker annotation (irony alert) overwrites that with the vanilla freemarker engine, which interprets the value of the sql statement annotation as a freemarker template.

This is the problem. It is not a bug but a documentation issue.

[xak2000]

Oh, interesting. Sorry for the confusion, then.

Based on what you described @hgschmie I assume that @UseFreemarkerSqlLocator and @UseFreemarkerEngine should never be used together. They are designed to use one or the other. Is that correct?

If yes, then probably it is possible to automatically check the presense of both and if both are found, throw an exception with a reasonable error message, that explains that these annotations should not be used together?

There is also @UseClasspathSqlLocator and @UseTemplateEngine(AnyEngine.class) (undocumented) and I honestly thought that I can combine locator and engine as I wish. E.g. @UseClasspathSqlLocator + @UseStringTemplateEngine or @UseClasspathSqlLocator + @UseFreemarkerEngine (to load template from .sql file as opposed to .sql.ftl used by @UseFreemarkerSqlLocator).

I even remember I previously used @UseStringTemplateEngine together with @UseClasspathSqlLocator and it worked fine (StringTemplate engine was used to parse SQL from .sql file instead of .sql.stg that is assumed by @UseStringTemplateSqlLocator). The configuration was like this:

@UseClasspathSqlLocator
interface Dao {
  @SqlQuery
  @UseStringTemplateEngine
  User findUser(@Define boolean param);
}

// Dao/findUser.sql
// SELECT * FROM users <if(param)> WHERE a = 1 <endif>

Probably that is why I assumed that locator and engine are independed.