Flooding with warning messages

[apptio-msobala]

First of all, thanks for this great library!

When both Configuration and Settings are available then there's a warning logged:

if (configuration != null) {
      logger.debug("Creating factory from configuration having dialect {}", configuration.dialect());
      if (jooqSettings != null) {
        logger.warn("@Injected org.jooq.conf.Settings is being ignored since a full org.jooq.Configuration was supplied");
      }

Unfortunately Guice is always binding the Settings automatically using the default constructor and I can't get rid of this warning. I tried to declare the Settings as null but then I get NULL_INJECTED_INTO_NON_NULLABLE exception.

Could you please either change the log level or add @Nullable (https://github.com/google/guice/wiki/UseNullable) annotation to the fields?

Otherwise every transaction logs a warning.

WARN [JooqPersistService] @Injected org.jooq.conf.Settings is being ignored since a full org.jooq.Configuration was supplied

[supercargo]

It sounds like there are a couple of issues here.

1. Warning log spam
2. No way to proactively avoid the warning.

I think it would be nice to leave the warning, but obviously not log it so frequently. As for the null "unbinding" issue, it looks like OptionalBinder is the preferred way to go over the currently used @Inject(optional = true). OptionalBinder would allow the two configuration alternatives to be provided using constructor injection, and a warning could be logged at construction time instead. This may still be frequently if the service isn't scoped, but I think Optional will give you a way to suppress them.

If I open a branch with these changes, do you have the ability to build and try them out without a full maven release?

[apptio-msobala]

The OptionalBinder is the best option here.

And oh well I spotted a couple of issues later on. I'm integrating with Dropwizard.

• You can't inject DSLContext because it requires transaction to be opened and as of time of injection there's no trasaction opened obviously. I had to use the Provider<DSLContext> instead.
• JOOQ changed a little bit I believe and the rollback doesn't work with the latest JOOQ version. I tried to analyse what's wrong and it seems to me that a ThreadLocalTransactionProvider should be used instead of DefaultConnectionProvider.

I'll let you know if I figure something out.

[supercargo]

Typically DSLContext should be @Request scoped, implicitly. How to achieve that even depends on if your Resource classes are singletons or instantiated per request. Provider is appropriate if you have a component whose lifecycle extends beyond a request that also needs to process transactions (transaction boundaries need to be under your control then). Are you using a dropwizard integration library like guicey or similar?

TBH, I haven't kept up with JOOQ since I'm mostly not using SQL these days. The landscape around dropwizard + guice has changed a bit, too, since I wrote the initial documentation.

[apptio-msobala]

Disclaimer - the reason for this is that we use our internal util library which sets the connection provider for the configuration... So from your perspective it may be not needed.

Ok so what I had to do, to make it work with our Dropwizard's ManagedDataSource was to set the acquired connection in the configuration. Without this jOOQ used the original DataSourceConnectionProvider which returned a new connection from the pool for every query. Hence the rollback was operating on a different connection.

jooqFactory = DSL.using(conn, configuration.dialect(), configuration.settings());

[apptio-msobala]

Also:

  private final Settings jooqSettings;
  private final Configuration configuration;

  @Inject
  public JooqPersistService(final Provider<DataSource> jdbcSource, final SQLDialect sqlDialect,
      Optional<Settings> jooqSettings, Optional<Configuration> configuration) {
    this.jdbcSource = jdbcSource;
    this.sqlDialect = sqlDialect;
    this.jooqSettings = jooqSettings.orNull();
    this.configuration = configuration.orNull();
  }

Would be cool if you release a new version with these changes. I can test the unofficial version before publishing to maven.

[supercargo]

@apptio-msobala are you able to open a pull request with the suggested changes?