Re: jOOQ - Suggestions for improvement

[Lukas Eder]

Hello Johannes,

Thanks for your feedback. I'm replying directly to the user group as
these questions are usually of general interest.

> We are using jOOQ in a project @ work mainly for generating SQL statements, which we execute using Spring's JdbcTemplate.

Yes, this has become a very common use-case now. This Stack Overflow
question here has more and more traction:
http://stackoverflow.com/questions/4474365/jooq-and-spring

> 1) All factory implementations require a Connection and NOT a DataSource. [...]

The upcoming version 2.3.0 will feature Sergey Epik's contribution of
the FactoryProxy. It is already available on GitHub. The naming is not
yet clear, but this has been discussed recently on the user group and
been followed with a lot of interest:
https://groups.google.com/d/topic/jooq-user/tIwobFOR2iM/discussion

> 2) Maybe it would be a good idea to split the FactoryOperstions interface into two concerns.

That was precisely the conclusion to all of these discussions. For
historical reasons, the current Factory is both the query builder and
query executor object, when in fact the execution part should be
refactored out of the Factory. That would allow for many overall
improvements, one of which being that a dedicated Executor could
operate on a DataSource without relying on Spring's magic, as
suggested by Sergey.

It's nice to have your feedback on this topic as well. It shows that
we're going to be on a good track for jOOQ 3.0 (due in late 2012),
when the overall API will be re-worked into what end users really need
from jOOQ.

Any further input is very welcome!

Cheers
Lukas

2012/5/3 Johannes Scharf:
> Hallo Lukas!
>
> We are using jOOQ in a project @ work mainly for generating SQL statements, which we execute using Spring's JdbcTemplate.
>
> During development we came across the following suggestions for future improvements:
>
> 1) All factory implementations require a Connection and NOT a DataSource. Passing in a DataSource into the constructor would allow for jOOQ to participate more seamlessly in an enterprise environment. More over it would be possible to use jOOQ directly with Spring managed transactions and to reuse the factory instead of creating a new one for each connection at the beginning of a transaction.
>
> 2) Maybe it would be a good idea to split the FactoryOperstions interface into two concerns. One for creating SQL statements and one for executing them. This would come in handy in situations like our, when someone want's to use jOOQ mainly as a SQL builder.
>
>
> What do you think?
>
>
> Thank you for your efforts and creating jOOQ!
>
> Kind regards,
> Johannes

[Johannes Scharf]

I've taken a look at the "FactoryProxy". IMHO it's a good approach but
during a request it seems that it's very likely that the internal
factory delegate gets created many many times. This may not be a
dramatic performance penality but should be avoided. The client of the
"FactoryProxy" even has no chance to avoid the permanent creation of
the underlying FactoryOperations.
Hopefully the existing factories get refactored soon to accept a
DataSource, hence the factories can be created once and be used
accross application code.

I didn't know about the "FactoryProxy" before - in our Spring based
project I've developed the following interface to integrate jOOQ with
Spring and its transactional infrastructure:
import org.jooq.FactoryOperations;

/**
* Instances of this interface are used to create independent jOOQ
factories and
* usually encapsulate mechanisms like e.g. obtaining a new
(transactional)
* connection from a data source and passing the connection to the
created jOOQ
* factory.
* <p/>
* This abstraction is required as jOOQ's factories are <b>not</b>
thread-safe
* and therefore cannot be shared by multiple clients in an
application context.
*
* @author J. Scharf
* @since 1.0
*/
public interface JooqFactoryCreator {
/**
* Get a new jOOQ factory.
*
* @return the jOOQ factory (never null)
*/
public FactoryOperations getJooqFactory();
}

and its implementation "ConfigurableJooqFactoryCreator":
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.sql.Connection;

import javax.sql.DataSource;

import org.jooq.FactoryOperations;
import org.jooq.conf.Settings;
import org.jooq.util.oracle.OracleFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.jdbc.datasource.DataSourceUtils;
import org.springframework.util.Assert;
import org.springframework.util.ReflectionUtils;

/**
* Implementation of {@link JooqFactoryCreator} which can be
configured to
* create instances of {@link FactoryOperations} from the specified
class.
* <p/>
* The class is aware of Spring's transaction infrastructure and
obtains new
* connections from the {@code DataSource} be calling
* {@link DataSourceUtils#getConnection(DataSource)}.
*
* @author J. Scharf
* @since 1.0
*/
public class ConfigurableJooqFactoryCreator implements
JooqFactoryCreator {
private final Logger logger =
LoggerFactory.getLogger(this.getClass());

private Class<?> jooqFactoryClazz = OracleFactory.class;
private Constructor<FactoryOperations> factoryConstructor;

private final DataSource dataSource;
private final Settings settings;

/**
* Creates a new factory creator.
*
* @param dataSource
* the data source to obtain a {@link Connection} from
* @param settings
* the settings to configure the jOOQ factory with
*/
public ConfigurableJooqFactoryCreator(DataSource dataSource,
Settings settings) {
Assert.notNull(dataSource, "The dataSource must not be null.");
Assert.notNull(settings, "The settings must not be null.");

this.dataSource = dataSource;
this.settings = settings;

initFactoryConstructor();
}

/**
* Used to cache the looked up {@link Constructor}.
*/
@SuppressWarnings("unchecked")
private void initFactoryConstructor() {
Class[] args = new Class[] { Connection.class, Settings.class };

logger.debug(
"Looking up constructor with arguments {} on class '{}'...",
args, jooqFactoryClazz.getName());

Constructor<FactoryOperations> constructor;

try {
constructor = (Constructor<FactoryOperations>) jooqFactoryClazz
.getConstructor(args);

this.factoryConstructor = constructor;
} catch (NoSuchMethodException ex) {
ReflectionUtils.handleReflectionException(ex);
}
}

/**
* Create a new jOOQ factory. Each method call returns a new
instance.
*
* @return the jOOQ factory (never null)
*/
@Override
public FactoryOperations getJooqFactory() {
logger.debug("Creating new jooq factory of type '{}'...",
jooqFactoryClazz.getName());

Connection connection = DataSourceUtils.getConnection(dataSource);
logger.debug("Fetched connection [{}] from DataSourceUtils.",
connection);

FactoryOperations factory;
try {
factory = factoryConstructor.newInstance(connection, settings);

return factory;
}
/*
* Unfortunately we have to catch each reflection exception
separately,
* because we don't want to catch any other exception not caused by
* newInstance
*/
catch (IllegalArgumentException ex) {
handleReflectionException(ex);
} catch (InstantiationException ex) {
handleReflectionException(ex);
} catch (IllegalAccessException ex) {
handleReflectionException(ex);
} catch (InvocationTargetException ex) {
handleReflectionException(ex);
}

// Unreachable
return null;
}

private void handleReflectionException(Exception ex) {
logger.error("Error creating jooq factory '{}': {}",
jooqFactoryClazz
.getName(), ex.toString());

ReflectionUtils.handleReflectionException(ex);
}

/**
* Set the jOOQ factory clazz to use. Defaults to {@link
OracleFactory}.
*
* @param jooqFactoryClazz
* the factory class (never null)
*/
public void setJooqFactoryClazz(Class<?> jooqFactoryClazz) {
Assert.notNull(jooqFactoryClazz,
"The jooqFactoryClazz must not be null.");

this.jooqFactoryClazz = jooqFactoryClazz;
initFactoryConstructor();
}
}

Maybe this could be helpful for others too. At the moment a new
factory usually will be created by client code each time
"getJooqFactory" gets called.
When I've got some time left I will try to synchronize (like Spring's
DataSourceUtils does it for a Connection) the created factory with the
current Spring-managed transaction (if any) so it only needs to be
created once during a transaction.

Kind regards,
Johannes

[Lukas Eder]

Hello Johannes

Thanks for your feedback

> I've taken a look at the "FactoryProxy". IMHO it's a good approach but
> during a request it seems that it's very likely that the internal
> factory delegate gets created many many times. This may not be a
> dramatic performance penality but should be avoided. The client of the
> "FactoryProxy" even has no chance to avoid the permanent creation of
> the underlying FactoryOperations.

Agreed, it is not yet optimal, and thus marked as "EXPERIMENTAL"

> Hopefully the existing factories get refactored soon to accept a
> DataSource, hence the factories can be created once and be used
> accross application code.

I hope so, too.

> [... code ...]

> Maybe this could be helpful for others too.

Thanks for sharing this. I'm curious about feedback by others!

> When I've got some time left I will try to synchronize (like Spring's
> DataSourceUtils does it for a Connection) the created factory with the
> current Spring-managed transaction (if any) so it only needs to be
> created once during a transaction.

Thanks for this contribution. Feel free to provide a pull request on
GitHub, potentially modifying the current FactoryProxy
https://github.com/jOOQ/jOOQ

Cheers
Lukas

[Sergey Epik]

Hello,

The Factory object is very light, so it's not a problem instantiate it for each request in FactoryProxy.
Solution with FactoryProxy makes possible to use JooQ in Spring environmen, it's simple but not optimal.
The major drawback of this solution is a necessity to declare all service methods as "Transactional", because TransactionInterceptor cares about closing of connections in this case.

It would be great to configure Factory with DataSource, instead of Connection.
There are several places in jOOQ (search usages of  method Util.translate, for example in AbstractQuery.execute() ), where we can receive connection from DataSource, execute statement and close connection (return it to connection pool).
For backward compatibility (in Connection-based Factory constructor) it's possible to wrap Connection to  SingleConnectionDataSource for internal usage.
Another advantage of using DataSource in internal jOOQ API - connection can be returned to pool as fast as possible.

Regarding separation executing and sql rendering concerns.
I like current jOOQ API and I suppose that much better to write
factory.select(..).fetch()
than something like
executor.fetch(factory.select(...))
This is one of the reasons why I do not like to use JdbcTemplate with jOOQ.

--
Best regards,

[Lukas Eder]

> For backward compatibility (in Connection-based Factory constructor) it's
> possible to wrap Connection to  SingleConnectionDataSource for internal
> usage.

That's actually a very good idea. Essentially, org.jooq.Configuration
(implemented by Factory) should have a close() method. If any
operation closes the ResultSet / Statement, it should also close the
Configuration. If the Configuration was initialised with a Connection,
then this is ignored (backwards-compatible). If the Configuration was
initialised with a DataSource, then this should close the jOOQ-managed
Connection ("Spring / DataSource- enabled"). Nice thinking!

> Another advantage of using DataSource in internal jOOQ API - connection can
> be returned to pool as fast as possible.

Yes

> Regarding separation executing and sql rendering concerns.
> I like current jOOQ API and I suppose that much better to write
> factory.select(..).fetch()
> than something like
> executor.fetch(factory.select(...))
> This is one of the reasons why I do not like to use JdbcTemplate with jOOQ.

I do like the current API as well. When adding Executor, I was
thinking of duplicating fetch methods. i.e.:

// this here
factory.select(...).fetch();

// will then internally call this here:
executor.fetch(factory.select(...));

From a backwards-compatibility perspective, this would be a must-have
requirement anyway. But the advantage of having Executors is the fact
that they can be subclassed and customised much more easily than
Factory / FactoryOperations today, as an Executor will not have to
implement all DSL-specific methods.

Cheers
Lukas

[Johannes Scharf]

I think there's a problem with the FactoryProxy (and also my code
above) if no transaction exists at all. In that case every method on
the FactoryProxy will result in obtaining a new connection from the
underlying DataSource which is never closed. That happens for example
if you use the FactoryProxy (or my code above) during startup of
Spring's aplication context, e.g. in a bean's "afterPropertiesSet"
method.
After a short time all available connections will be exhausted. Sure,
after a time the DataSource should close the abandoned connections,
but that really doesn't solve the problem. In our case the application
startup just hung forever after three calls (every call obtains a new
connection and creates a new factory behind the scenes) until timeout.

This lead me to the following conclusion:
- If the factories get refactored, they should take care about
obtaining and closing a connection. This allows for the factories to
operate in a non-transactional environment too and passing in a
"TransactionAwareDataSourceProxy" would perfectly integrate them with
Spring and it's transactional infrastructure.
- Therefore there should be no need that any interface or class of
jOOQ provides a "close" method - I even advise against this.

Are there any plans in which version the factories will accept a
DataSource?

[Lukas Eder]

Hello Johannes,

> I think there's a problem with the FactoryProxy (and also my code
> above) if no transaction exists at all. In that case every method on
> the FactoryProxy will result in obtaining a new connection from the

> underlying DataSource which is never closed. [...]

Yes, this is what I was afraid of. The current implementation relies
on a lot of magic. It is probably hard to get that magic right. As
mentioned below, the only thorough way to do this in jOOQ is to let
jOOQ handle the complete lifecycle of a Connection obtained from a
DataSource.

> This lead me to the following conclusion:
> - If the factories get refactored, they should take care about
> obtaining and closing a connection. This allows for the factories to
> operate in a non-transactional environment too and passing in a
> "TransactionAwareDataSourceProxy" would perfectly integrate them with
> Spring and it's transactional infrastructure.

Yes, Factory will have two operation modes:
1. Taking a Connection, leaving it open at the end of the execution
2. Taking a DataSource, fetching a Connection from it, and closing it
at the end of the execution

> - Therefore there should be no need that any interface or class of
> jOOQ provides a "close" method - I even advise against this.

It would be nice if close() doesn't need to be promoted to the public
API. But I cannot say that yet, as this is a non-trivial refactoring.

> Are there any plans in which version the factories will accept a
> DataSource?

I'll see if this is feasible for jOOQ 2.4.0. I filed ticket #1405 for this:
https://sourceforge.net/apps/trac/jooq/ticket/1405

Being a non-trivial change, I cannot make a guarantee about this, yet.

Cheers
Lukas