From 86e581c869ae37be9f2e0b1db1e25354cabcf950 Mon Sep 17 00:00:00 2001 From: Lukas Eder Date: Mon, 4 Feb 2013 20:39:43 +0100 Subject: [PATCH] [#1578] Change configuration of ExecuteListeners in Configuration. Listeners instances should be provided, not classes --- jOOQ-test/src/org/jooq/test/BaseTest.java | 2 +- .../jooq/test/_/TestStatisticsListener.java | 2 +- .../jooq/test/_/testcases/BenchmarkTests.java | 3 ++- .../_/testcases/ExecuteListenerTests.java | 20 ++++++++-------- .../jooq/test/_/testcases/GeneralTests.java | 4 +++- .../jooq/test/_/testcases/StatementTests.java | 7 +++--- .../src/org/jooq/test/jOOQAbstractTest.java | 10 ++++---- .../src/main/java/org/jooq/Configuration.java | 5 ++++ .../main/java/org/jooq/ExecuteListener.java | 4 ++-- .../org/jooq/impl/AbstractConfiguration.java | 14 +++++++++++ .../main/java/org/jooq/impl/BatchCRUD.java | 23 ++++++++----------- .../org/jooq/impl/DefaultConfiguration.java | 21 +++++++++++++++++ .../src/main/java/org/jooq/impl/Executor.java | 10 ++++++++ jOOQ/src/main/java/org/jooq/impl/Utils.java | 18 ++------------- .../main/resources/xsd/jooq-runtime-3.0.0.xsd | 11 --------- .../test/java/org/jooq/test/SettingsTest.java | 5 ---- 16 files changed, 89 insertions(+), 70 deletions(-) diff --git a/jOOQ-test/src/org/jooq/test/BaseTest.java b/jOOQ-test/src/org/jooq/test/BaseTest.java index efd78b6346..292d768f8b 100644 --- a/jOOQ-test/src/org/jooq/test/BaseTest.java +++ b/jOOQ-test/src/org/jooq/test/BaseTest.java @@ -706,7 +706,7 @@ public abstract class BaseTest< protected Executor create(Settings settings) { Executor create = delegate.create(settings); - create.getSettings().getExecuteListeners().add(TestStatisticsListener.class.getName()); + create.getExecuteListeners().add(new TestStatisticsListener()); return create; } diff --git a/jOOQ-test/src/org/jooq/test/_/TestStatisticsListener.java b/jOOQ-test/src/org/jooq/test/_/TestStatisticsListener.java index 981fee0206..ed8b3ab28f 100644 --- a/jOOQ-test/src/org/jooq/test/_/TestStatisticsListener.java +++ b/jOOQ-test/src/org/jooq/test/_/TestStatisticsListener.java @@ -47,7 +47,7 @@ public class TestStatisticsListener extends DefaultExecuteListener { public static Map STATISTICS = new HashMap(); @Override - public void start(ExecuteContext ctx) { + public synchronized void start(ExecuteContext ctx) { Integer count = STATISTICS.get(ctx.type()); if (count == null) { diff --git a/jOOQ-test/src/org/jooq/test/_/testcases/BenchmarkTests.java b/jOOQ-test/src/org/jooq/test/_/testcases/BenchmarkTests.java index d045b45007..f545dde49c 100644 --- a/jOOQ-test/src/org/jooq/test/_/testcases/BenchmarkTests.java +++ b/jOOQ-test/src/org/jooq/test/_/testcases/BenchmarkTests.java @@ -41,6 +41,7 @@ import java.sql.ResultSet; import java.util.Collections; import java.util.Random; +import org.jooq.ExecuteListener; import org.jooq.Record1; import org.jooq.Record2; import org.jooq.Record3; @@ -119,7 +120,7 @@ extends BaseTestemptyList()); + create.setExecuteListeners(Collections.emptyList()); // Dry-run to avoid side-effects testBenchmarkFullExecution(create, 1); diff --git a/jOOQ-test/src/org/jooq/test/_/testcases/ExecuteListenerTests.java b/jOOQ-test/src/org/jooq/test/_/testcases/ExecuteListenerTests.java index bb365e5321..b583ed6e0c 100644 --- a/jOOQ-test/src/org/jooq/test/_/testcases/ExecuteListenerTests.java +++ b/jOOQ-test/src/org/jooq/test/_/testcases/ExecuteListenerTests.java @@ -63,7 +63,6 @@ import org.jooq.Record6; import org.jooq.Result; import org.jooq.TableRecord; import org.jooq.UpdatableRecord; -import org.jooq.conf.Settings; import org.jooq.conf.SettingsTools; import org.jooq.impl.DefaultExecuteListener; import org.jooq.impl.Executor; @@ -100,8 +99,8 @@ extends BaseTest> query = create(settings).select(val(1)); + ResultQuery> query = create.select(val(1)); assertEquals(1, query.fetchOne(0)); assertEquals(2, query.bind(1, 2).fetchOne(0)); diff --git a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java index 4704073675..b579e36980 100644 --- a/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java +++ b/jOOQ-test/src/org/jooq/test/jOOQAbstractTest.java @@ -774,12 +774,12 @@ public abstract class jOOQAbstractTest< Settings settings = SettingsTools.defaultSettings() .withRenderSchema(renderSchema) .withRenderMapping(new RenderMapping() - .withDefaultSchema(defaultSchema)) - .withExecuteListeners( - TestStatisticsListener.class.getName(), - PrettyPrinter.class.getName()); + .withDefaultSchema(defaultSchema)); - return create(settings); + Executor create = create(settings); + create.getExecuteListeners().add(new TestStatisticsListener()); + create.getExecuteListeners().add(new PrettyPrinter()); + return create; } protected final SQLDialect getDialect() { diff --git a/jOOQ/src/main/java/org/jooq/Configuration.java b/jOOQ/src/main/java/org/jooq/Configuration.java index eec3abb7bc..83fae813c7 100644 --- a/jOOQ/src/main/java/org/jooq/Configuration.java +++ b/jOOQ/src/main/java/org/jooq/Configuration.java @@ -36,6 +36,7 @@ package org.jooq; import java.io.Serializable; +import java.util.List; import java.util.Map; import org.jooq.conf.Settings; @@ -138,4 +139,8 @@ public interface Configuration extends Serializable { */ Object setData(String key, Object value); + List getExecuteListeners(); + + void setExecuteListeners(List listeners); + } diff --git a/jOOQ/src/main/java/org/jooq/ExecuteListener.java b/jOOQ/src/main/java/org/jooq/ExecuteListener.java index 19954358ac..3b17e90d31 100644 --- a/jOOQ/src/main/java/org/jooq/ExecuteListener.java +++ b/jOOQ/src/main/java/org/jooq/ExecuteListener.java @@ -54,10 +54,10 @@ import org.jooq.tools.StopWatchListener; *

* ExecuteListener is a base type for loggers, debuggers, * profilers, data collectors that can be hooked into a jOOQ {@link Executor} - * using the {@link Settings#getExecuteListeners()} property, passing + * using the {@link Configuration#getExecuteListeners()} property, passing * Settings to * {@link Executor#Executor(java.sql.Connection, SQLDialect, Settings)}. jOOQ - * will use those settings at the beginning of a query execution event to + * will use that configuration at the beginning of a query execution event to * instanciate all the provided listeners. In other words, listeners have the * same lifetime as a single query execution, and can thus be used to store * state between the moment when a query execution starts, and the moment when a diff --git a/jOOQ/src/main/java/org/jooq/impl/AbstractConfiguration.java b/jOOQ/src/main/java/org/jooq/impl/AbstractConfiguration.java index 9e00f5c7df..e105130b9f 100644 --- a/jOOQ/src/main/java/org/jooq/impl/AbstractConfiguration.java +++ b/jOOQ/src/main/java/org/jooq/impl/AbstractConfiguration.java @@ -35,10 +35,12 @@ */ package org.jooq.impl; +import java.util.List; import java.util.Map; import org.jooq.Configuration; import org.jooq.ConnectionProvider; +import org.jooq.ExecuteListener; import org.jooq.SQLDialect; import org.jooq.conf.Settings; @@ -96,4 +98,16 @@ abstract class AbstractConfiguration implements Configuration { public final Object setData(String key, Object value) { return configuration.setData(key, value); } + + @Override + public final List getExecuteListeners() { + return configuration.getExecuteListeners(); + } + + @Override + public final void setExecuteListeners(List listeners) { + configuration.setExecuteListeners(listeners); + } + + } diff --git a/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java b/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java index dd328948c3..dfec7bbd4e 100644 --- a/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java +++ b/jOOQ/src/main/java/org/jooq/impl/BatchCRUD.java @@ -38,7 +38,6 @@ package org.jooq.impl; import static org.jooq.conf.SettingsTools.executeStaticStatements; import java.util.ArrayList; -import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -51,8 +50,6 @@ import org.jooq.Configuration; import org.jooq.ExecuteContext; import org.jooq.Query; import org.jooq.UpdatableRecord; -import org.jooq.conf.Settings; -import org.jooq.conf.SettingsTools; import org.jooq.exception.DataAccessException; /** @@ -96,18 +93,18 @@ class BatchCRUD implements Batch { private final int[] executePrepared() { Map> queries = new LinkedHashMap>(); - Settings work = create.getSettings(); - Settings orig = SettingsTools.clone(work); + Boolean executeLogging = create.getSettings().isExecuteLogging(); + QueryCollector collector = new QueryCollector(); try { // [#1537] Communicate with UpdatableRecordImpl create.setData(Utils.DATA_OMIT_RETURNING_CLAUSE, true); // Add the QueryCollector to intercept query execution after rendering - work.setExecuteListeners(Arrays.asList(QueryCollector.class.getName())); + create.getExecuteListeners().add(collector); // [#1529] Avoid DEBUG logging of single INSERT / UPDATE statements - work.setExecuteLogging(false); + create.getSettings().setExecuteLogging(false); for (int i = 0; i < records.length; i++) { Configuration previous = ((AttachableInternal) records[i]).getConfiguration(); @@ -142,8 +139,8 @@ class BatchCRUD implements Batch { finally { create.getData().remove(Utils.DATA_OMIT_RETURNING_CLAUSE); - work.setExecuteListeners(orig.getExecuteListeners()); - work.setExecuteLogging(orig.isExecuteLogging()); + create.getExecuteListeners().remove(collector); + create.getSettings().setExecuteLogging(executeLogging); } // Execute one batch statement for each identical SQL statement. Every @@ -174,12 +171,10 @@ class BatchCRUD implements Batch { private final int[] executeStatic() { List queries = new ArrayList(); - - Settings work = create.getSettings(); - Settings orig = SettingsTools.clone(work); + QueryCollector collector = new QueryCollector(); try { - work.setExecuteListeners(Arrays.asList(QueryCollector.class.getName())); + create.getExecuteListeners().add(collector); for (int i = 0; i < records.length; i++) { Configuration previous = ((AttachableInternal) records[i]).getConfiguration(); @@ -203,7 +198,7 @@ class BatchCRUD implements Batch { // Restore the original factory finally { - work.setExecuteListeners(orig.getExecuteListeners()); + create.getExecuteListeners().remove(collector); } // Resulting statements can be batch executed in their requested order diff --git a/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java b/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java index e58f7f8293..fd2ae13f7f 100644 --- a/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java +++ b/jOOQ/src/main/java/org/jooq/impl/DefaultConfiguration.java @@ -38,13 +38,16 @@ package org.jooq.impl; import static org.jooq.SQLDialect.SQL99; import java.io.StringWriter; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import javax.xml.bind.JAXB; import org.jooq.Configuration; import org.jooq.ConnectionProvider; +import org.jooq.ExecuteListener; import org.jooq.SQLDialect; import org.jooq.conf.Settings; import org.jooq.conf.SettingsTools; @@ -67,6 +70,7 @@ class DefaultConfiguration implements Configuration { private final org.jooq.SchemaMapping mapping; private final Settings settings; private final Map data; + private List listeners; @SuppressWarnings("deprecation") DefaultConfiguration() { @@ -80,6 +84,7 @@ class DefaultConfiguration implements Configuration { this.settings = settings != null ? settings : SettingsTools.defaultSettings(); this.mapping = new org.jooq.SchemaMapping(this.settings); this.data = data != null ? data : new HashMap(); + this.listeners = new ArrayList(); } /** @@ -139,6 +144,22 @@ class DefaultConfiguration implements Configuration { return data.put(key, value); } + /** + * {@inheritDoc} + */ + @Override + public final List getExecuteListeners() { + return listeners; + } + + /** + * {@inheritDoc} + */ + @Override + public final void setExecuteListeners(List listeners) { + this.listeners = listeners != null ? listeners : new ArrayList(); + } + @Override public String toString() { StringWriter writer = new StringWriter(); diff --git a/jOOQ/src/main/java/org/jooq/impl/Executor.java b/jOOQ/src/main/java/org/jooq/impl/Executor.java index a205f8047d..875f8b5e97 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Executor.java +++ b/jOOQ/src/main/java/org/jooq/impl/Executor.java @@ -383,6 +383,16 @@ public class Executor implements Configuration { return configuration.getConnectionProvider(); } + @Override + public final List getExecuteListeners() { + return configuration.getExecuteListeners(); + } + + @Override + public final void setExecuteListeners(List listeners) { + configuration.setExecuteListeners(listeners); + } + // ------------------------------------------------------------------------- // XXX Convenience methods accessing the underlying Connection // ------------------------------------------------------------------------- diff --git a/jOOQ/src/main/java/org/jooq/impl/Utils.java b/jOOQ/src/main/java/org/jooq/impl/Utils.java index 2422184928..e10ed589fe 100644 --- a/jOOQ/src/main/java/org/jooq/impl/Utils.java +++ b/jOOQ/src/main/java/org/jooq/impl/Utils.java @@ -834,27 +834,13 @@ final class Utils { result.add(new LoggerListener()); } - for (String listener : configuration.getSettings().getExecuteListeners()) { - result.add(getListener(listener)); + for (ExecuteListener listener : configuration.getExecuteListeners()) { + result.add(listener); } return result; } - private static final ExecuteListener getListener(String name) { - try { - - // [#1572] Loading classes like this is needed for class loading to - // work with OSGi. [#1578] The current implementation of loading - // ExecuteListeners will be reworked in jOOQ 3.0, though - Class type = Thread.currentThread().getContextClassLoader().loadClass(name); - return (ExecuteListener) Reflect.accessible(type.getDeclaredConstructor()).newInstance(); - } - catch (Exception e) { - throw new RuntimeException(e); - } - } - /** * Wrap a piece of SQL code in parentheses, if not wrapped already */ diff --git a/jOOQ/src/main/resources/xsd/jooq-runtime-3.0.0.xsd b/jOOQ/src/main/resources/xsd/jooq-runtime-3.0.0.xsd index cd3310d94e..64c136e5eb 100644 --- a/jOOQ/src/main/resources/xsd/jooq-runtime-3.0.0.xsd +++ b/jOOQ/src/main/resources/xsd/jooq-runtime-3.0.0.xsd @@ -35,9 +35,6 @@ - - - @@ -150,12 +147,4 @@ - - - - - - - - \ No newline at end of file diff --git a/jOOQ/src/test/java/org/jooq/test/SettingsTest.java b/jOOQ/src/test/java/org/jooq/test/SettingsTest.java index d1d3847793..9bab5dcbe8 100644 --- a/jOOQ/src/test/java/org/jooq/test/SettingsTest.java +++ b/jOOQ/src/test/java/org/jooq/test/SettingsTest.java @@ -77,10 +77,5 @@ public class SettingsTest { Settings settings2 = SettingsTools.clone(settings1); assertEquals(settings1.isAttachRecords(), settings2.isAttachRecords()); - assertEquals(settings1.getExecuteListeners(), settings2.getExecuteListeners()); - - // Check if clone makes a deep-copy - settings1.getExecuteListeners().add("asdf"); - assertEquals(settings1.getExecuteListeners().size(), settings2.getExecuteListeners().size() + 1); } }