Repository: tomee Updated Branches: refs/heads/master e3411b761 -> 5fdf755bf
TOMEE-1625 ResetOnError datasource flag + fixing datasource proxy unwrapping - was working for only 1 level Project: http://git-wip-us.apache.org/repos/asf/tomee/repo Commit: http://git-wip-us.apache.org/repos/asf/tomee/commit/5fdf755b Tree: http://git-wip-us.apache.org/repos/asf/tomee/tree/5fdf755b Diff: http://git-wip-us.apache.org/repos/asf/tomee/diff/5fdf755b Branch: refs/heads/master Commit: 5fdf755bfa31c037caaa353b0e916e428b803436 Parents: e3411b7 Author: Romain Manni-Bucau <[email protected]> Authored: Sun Aug 9 11:51:02 2015 -0700 Committer: Romain Manni-Bucau <[email protected]> Committed: Sun Aug 9 11:51:02 2015 -0700 ---------------------------------------------------------------------- container/openejb-core/pom.xml | 1 + .../resource/jdbc/DataSourceFactory.java | 128 +++++++++++---- .../resource/jdbc/DelegatableHandler.java | 24 +++ .../jdbc/FlushableDataSourceHandler.java | 86 ++++++---- .../jdbc/ResettableDataSourceHandler.java | 156 ++++++++++++++++++ .../jdbc/dbcp/DbcpDataSourceCreator.java | 2 +- .../resource/jdbc/driver/AlternativeDriver.java | 5 +- .../jdbc/logging/LoggingSqlDataSource.java | 10 +- .../jdbc/pool/DefaultDataSourceCreator.java | 11 -- .../jdbc/pool/PoolDataSourceCreator.java | 8 +- .../openejb/core/singleton/DependsOnTest.java | 3 + .../jdbc/ResettableDataSourceHandlerTest.java | 159 +++++++++++++++++++ pom.xml | 2 +- .../openejb/bonecp/BoneCPDataSourceCreator.java | 2 + tck/cdi-embedded/src/test/resources/failing.xml | 6 +- .../tomee/jdbc/TomEEDataSourceCreator.java | 10 ++ 16 files changed, 530 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/pom.xml ---------------------------------------------------------------------- diff --git a/container/openejb-core/pom.xml b/container/openejb-core/pom.xml index d29a0bc..df13f18 100644 --- a/container/openejb-core/pom.xml +++ b/container/openejb-core/pom.xml @@ -315,6 +315,7 @@ <version>2.17</version> <configuration> <reuseForks>false</reuseForks> + <forkCount>2</forkCount> <testNGArtifactName>none:none</testNGArtifactName> <argLine> -javaagent:${project.basedir}/target/openejb-javaagent-${project.version}.jar http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DataSourceFactory.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DataSourceFactory.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DataSourceFactory.java index 6f4f113..49b1fb1 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DataSourceFactory.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DataSourceFactory.java @@ -32,10 +32,11 @@ import org.apache.openejb.resource.jdbc.pool.DefaultDataSourceCreator; import org.apache.openejb.util.Duration; import org.apache.openejb.util.LogCategory; import org.apache.openejb.util.Logger; -import org.apache.openejb.util.PropertiesHelper; import org.apache.openejb.util.SuperProperties; +import org.apache.xbean.recipe.ExecutionContext; import org.apache.xbean.recipe.ObjectRecipe; import org.apache.xbean.recipe.Option; +import org.apache.xbean.recipe.Recipe; import javax.sql.CommonDataSource; import javax.sql.DataSource; @@ -48,8 +49,10 @@ import java.lang.reflect.Proxy; import java.sql.Driver; import java.sql.SQLException; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.TreeMap; import java.util.concurrent.TimeUnit; @@ -65,15 +68,17 @@ public class DataSourceFactory { public static final String LOG_SQL_PROPERTY = "LogSql"; public static final String LOG_SQL_PACKAGE_PROPERTY = "LogSqlPackages"; public static final String FLUSHABLE_PROPERTY = "Flushable"; + public static final String RESET_PROPERTY = "ResetOnError"; + public static final String RESET_METHODS_PROPERTY = "ResetOnErrorMethods"; public static final String GLOBAL_LOG_SQL_PROPERTY = "openejb.jdbc.log"; public static final String GLOBAL_LOG_SQL_PACKAGE_PROPERTY = "openejb.jdbc.log.packages"; public static final String GLOBAL_FLUSH_PROPERTY = "openejb.jdbc.flushable"; public static final String POOL_PROPERTY = "openejb.datasource.pool"; public static final String DATA_SOURCE_CREATOR_PROP = "DataSourceCreator"; - private static final Map<CommonDataSource, AlternativeDriver> driverByDataSource = new HashMap<CommonDataSource, AlternativeDriver>(); + private static final Map<CommonDataSource, AlternativeDriver> driverByDataSource = new HashMap<>(); - private static final Map<CommonDataSource, DataSourceCreator> creatorByDataSource = new HashMap<CommonDataSource, DataSourceCreator>(); + private static final Map<CommonDataSource, DataSourceCreator> creatorByDataSource = new HashMap<>(); private static final Map<String, String> KNOWN_CREATORS = new TreeMap<String, String>(String.CASE_INSENSITIVE_ORDER) {{ put("simple", "org.apache.openejb.resource.jdbc.SimpleDataSourceCreator"); // use user provided DS, pooling not supported put("dbcp", "org.apache.openejb.resource.jdbc.pool.DefaultDataSourceCreator"); // the original one @@ -90,20 +95,10 @@ public class DataSourceFactory { final Duration timeBetweenEvictionRuns, final Duration minEvictableIdleTime) throws IllegalAccessException, InstantiationException, IOException { final Properties properties = asProperties(definition); + final Set<String> originalKeys = properties.stringPropertyNames(); - final boolean flushable = SystemInstance.get().getOptions().get(GLOBAL_FLUSH_PROPERTY, + boolean flushable = SystemInstance.get().getOptions().get(GLOBAL_FLUSH_PROPERTY, "true".equalsIgnoreCase((String) properties.remove(FLUSHABLE_PROPERTY))); - final FlushableDataSourceHandler.FlushConfig flushConfig; - if (flushable) { - properties.remove("flushable"); // don't let it wrap the delegate again - - flushConfig = new FlushableDataSourceHandler.FlushConfig( - name, configuredManaged, - impl, PropertiesHelper.propertiesToString(properties), - maxWaitTime, timeBetweenEvictionRuns, minEvictableIdleTime); - } else { - flushConfig = null; - } convert(properties, maxWaitTime, "maxWaitTime", "maxWait"); convert(properties, timeBetweenEvictionRuns, "timeBetweenEvictionRuns", "timeBetweenEvictionRunsMillis"); @@ -139,8 +134,10 @@ public class DataSourceFactory { "true".equalsIgnoreCase((String) properties.remove(LOG_SQL_PROPERTY))); final String logPackages = SystemInstance.get().getProperty(GLOBAL_LOG_SQL_PACKAGE_PROPERTY, (String) properties.remove(LOG_SQL_PACKAGE_PROPERTY)); final DataSourceCreator creator = creator(properties.remove(DATA_SOURCE_CREATOR_PROP), logSql); + final String resetOnError = (String) properties.remove(RESET_PROPERTY); + final String resetMethods = (String) properties.remove(RESET_METHODS_PROPERTY); // before setProperties() - boolean useContainerLoader = "true".equalsIgnoreCase(SystemInstance.get().getProperty("openejb.resources.use-container-loader", "true")) && (impl == null || impl.getClassLoader() == DataSourceFactory.class.getClassLoader()); + boolean useContainerLoader = "true".equalsIgnoreCase(SystemInstance.get().getProperty("openejb.resources.use-container-loader", "true")) && impl.getClassLoader() == DataSourceFactory.class.getClassLoader(); final ClassLoader oldLoader = Thread.currentThread().getContextClassLoader(); if (useContainerLoader) { final ClassLoader containerLoader = DataSourceFactory.class.getClassLoader(); @@ -206,11 +203,76 @@ public class DataSourceFactory { driverByDataSource.put(ds, driver); } - if (logSql) { - ds = makeItLogging(ds, logPackages); - } - if (flushable) { - ds = makeFlushable(ds, flushConfig); + final boolean doResetOnError = resetOnError != null && !"false".equals(resetOnError); + if (doResetOnError || logSql || flushable) { // will get proxied + ObjectRecipe objectRecipe = null; + ResettableDataSourceHandler existingResettableHandler = null; + FlushableDataSourceHandler flushableDataSourceHandler = null; + if (ExecutionContext.isContextSet()) { + final ExecutionContext context = ExecutionContext.getContext(); + final List<Recipe> stack = context.getStack(); + if (stack.size() > 0) { + objectRecipe = ObjectRecipe.class.cast(stack.get(0)); + existingResettableHandler = ResettableDataSourceHandler.class.cast(objectRecipe.getProperty("resettableHandler")); + flushableDataSourceHandler = FlushableDataSourceHandler.class.cast(objectRecipe.getProperty("flushableHandler")); + + final Map<String, Object> props = objectRecipe.getProperties(); + for (final String key : originalKeys) { + props.remove(key); + } + + // meta properties, not needed here so gain few cycles removing them + props.remove("properties"); + props.remove("Definition"); + props.remove("ServiceId"); + props.remove("resettableHandler"); + props.remove("flushableHandler"); + + //we create a proxy so we cant get txmgr etc in another manner or we cant extend (= break) this method + new ObjectRecipe(ds.getClass()) {{ + allow(Option.CASE_INSENSITIVE_PROPERTIES); + allow(Option.IGNORE_MISSING_PROPERTIES); + allow(Option.NAMED_PARAMETERS); + allow(Option.PRIVATE_PROPERTIES); + setAllProperties(props); + }}.setProperties(ds); + } + } + + if (logSql) { + ds = makeItLogging(ds, logPackages); + } + + final ResettableDataSourceHandler resettableDataSourceHandler; + if (doResetOnError) { // needs to be done after flushable + // ensure we reuse the same handle instance otherwise we loose state + resettableDataSourceHandler = existingResettableHandler != null ? + existingResettableHandler : + new ResettableDataSourceHandler(ds, resetOnError, resetMethods); + } else { + resettableDataSourceHandler = null; + } + + if (flushable || doResetOnError) { + if (flushableDataSourceHandler == null) { + final FlushableDataSourceHandler.FlushConfig flushConfig; + properties.remove("flushable"); // don't let it wrap the delegate again + + final Map<String, Object> recipeProps = new HashMap<>(objectRecipe == null ? new HashMap<String, Object>() : objectRecipe.getProperties()); + recipeProps.remove("properties"); + + flushConfig = new FlushableDataSourceHandler.FlushConfig(recipeProps); + flushableDataSourceHandler = new FlushableDataSourceHandler(ds, flushConfig, resettableDataSourceHandler); + } else { + flushableDataSourceHandler.updateDataSource(ds); + } + ds = makeSerializableFlushableDataSourceProxy(ds, flushableDataSourceHandler); + } + if (doResetOnError) { // needs to be done after flushable + // ensure we reuse the same handle instance otherwise we loose state + resettableDataSourceHandler.updateDelegate(ds); + ds = makeSerializableFlushableDataSourceProxy(ds, resettableDataSourceHandler); + } } return ds; @@ -221,6 +283,13 @@ public class DataSourceFactory { } } + public static CommonDataSource makeSerializableFlushableDataSourceProxy(final CommonDataSource ds, final InvocationHandler handler) { + return (CommonDataSource) Proxy.newProxyInstance( + Thread.currentThread().getContextClassLoader(), + new Class<?>[]{DataSource.class.isInstance(ds) ? DataSource.class : XADataSource.class, Serializable.class, Flushable.class}, + handler); + } + private static boolean basicChecksThatDataSourceCanBeCreatedFromContainerLoader(final Properties properties, final ClassLoader containerLoader) { // check basic some classes can be loaded from container otherwise don't force it try { @@ -249,13 +318,6 @@ public class DataSourceFactory { return true; } - private static CommonDataSource makeFlushable(final CommonDataSource ds, final FlushableDataSourceHandler.FlushConfig flushConfig) { - return (CommonDataSource) Proxy.newProxyInstance( - Thread.currentThread().getContextClassLoader(), - new Class<?>[]{DataSource.class.isInstance(ds) ? DataSource.class : XADataSource.class, Flushable.class, Serializable.class}, - new FlushableDataSourceHandler(ds, flushConfig)); - } - public static void setCreatedWith(final DataSourceCreator creator, final CommonDataSource ds) { creatorByDataSource.put(ds, creator); } @@ -364,7 +426,7 @@ public class DataSourceFactory { } public static boolean knows(final Object object) { - return object instanceof CommonDataSource && creatorByDataSource.containsKey(realInstance(object)); + return object instanceof CommonDataSource && creatorByDataSource.containsKey(CommonDataSource.class.cast(realInstance(object))); } // TODO: should we get a get and a clear method instead of a single one? @@ -405,11 +467,9 @@ public class DataSourceFactory { Object ds = o; while (Proxy.isProxyClass(ds.getClass())) { - final InvocationHandler handler = Proxy.getInvocationHandler(o); - if (LoggingSqlDataSource.class.isInstance(handler)) { - ds = LoggingSqlDataSource.class.cast(handler).getDelegate(); - } else if (FlushableDataSourceHandler.class.isInstance(handler)) { - ds = FlushableDataSourceHandler.class.cast(handler).getDelegate(); + final InvocationHandler handler = Proxy.getInvocationHandler(ds); + if (DelegatableHandler.class.isInstance(handler)) { + ds = DelegatableHandler.class.cast(handler).getDelegate(); } else { break; } http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DelegatableHandler.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DelegatableHandler.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DelegatableHandler.java new file mode 100644 index 0000000..d2f9a9d --- /dev/null +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/DelegatableHandler.java @@ -0,0 +1,24 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.resource.jdbc; + +import javax.sql.CommonDataSource; +import java.lang.reflect.InvocationHandler; + +public interface DelegatableHandler extends InvocationHandler { + CommonDataSource getDelegate(); +} http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/FlushableDataSourceHandler.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/FlushableDataSourceHandler.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/FlushableDataSourceHandler.java index 316565b..811180e 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/FlushableDataSourceHandler.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/FlushableDataSourceHandler.java @@ -16,38 +16,56 @@ */ package org.apache.openejb.resource.jdbc; -import org.apache.openejb.util.Duration; import org.apache.openejb.util.LogCategory; import org.apache.openejb.util.Logger; +import org.apache.xbean.recipe.ObjectRecipe; +import org.apache.xbean.recipe.Option; +import javax.sql.CommonDataSource; import java.io.Flushable; import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import javax.sql.CommonDataSource; -public class FlushableDataSourceHandler implements InvocationHandler { +public class FlushableDataSourceHandler implements DelegatableHandler { private static final Logger LOGGER = Logger.getInstance(LogCategory.OPENEJB, FlushableDataSourceHandler.class); + public static final String[] FACTORY_ARGS = new String[]{ + "ServiceId", "JtaManaged", "JdbcDriver", "Definition", "MaxWaitTime", "TimeBetweenEvictionRuns", "MinEvictableIdleTime" + }; private final FlushConfig config; private final ReadWriteLock lock = new ReentrantReadWriteLock(); - private volatile CommonDataSource delegate; + private AtomicReference<CommonDataSource> delegate = new AtomicReference<>(); + private final ResettableDataSourceHandler resettableHandler; - public FlushableDataSourceHandler(final CommonDataSource original, final FlushConfig config) { + public FlushableDataSourceHandler(final CommonDataSource original, final FlushConfig config, final ResettableDataSourceHandler resettableHandler) { this.config = config; - this.delegate = original; + this.delegate.set(original); + this.resettableHandler = resettableHandler; } private void createANewDelegate() { - final CommonDataSource old = delegate; + final CommonDataSource old = delegate.get(); try { - this.delegate = DataSourceFactory.create(config.name, config.configuredManaged, config.impl, config.definition, config.maxWaitTime, config.timeBetweenEvictionRuns, config.minEvictableIdleTime); + final ObjectRecipe recipe = new ObjectRecipe(DataSourceFactory.class.getName(), "create", FACTORY_ARGS); + recipe.allow(Option.CASE_INSENSITIVE_PROPERTIES); + recipe.allow(Option.IGNORE_MISSING_PROPERTIES); + recipe.allow(Option.NAMED_PARAMETERS); + recipe.allow(Option.PRIVATE_PROPERTIES); + recipe.setAllProperties(config.properties); + + recipe.setProperty("resettableHandler", resettableHandler); + recipe.setProperty("flushableHandler", this); + + updateDataSource(CommonDataSource.class.cast(recipe.create())); } catch (final Exception e) { LOGGER.error("Can't recreate the datasource, keeping old one", e); - this.delegate = old; return; } @@ -62,18 +80,27 @@ public class FlushableDataSourceHandler implements InvocationHandler { @Override public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + final CommonDataSource actualDelegate = delegate.get(); if (Object.class == method.getDeclaringClass()) { if ("hashCode".equals(method.getName())) { return hashCode(); } + if ("toString".equals(method.getName())) { + return "Flushable[" + actualDelegate.toString() + "]"; + } } if (Flushable.class == method.getDeclaringClass()) { final Lock l = lock.writeLock(); l.lock(); try { createANewDelegate(); - if (Flushable.class.isInstance(delegate)) { - Flushable.class.cast(delegate).flush(); + if (Flushable.class.isInstance(actualDelegate)) { // these sanity could be enhanced + if (!Proxy.isProxyClass(actualDelegate.getClass()) || + // reset implies flush so we need to check both + (!FlushableDataSourceHandler.class.isInstance(Proxy.getInvocationHandler(actualDelegate)) && + !ResettableDataSourceHandler.class.isInstance(Proxy.getInvocationHandler(actualDelegate)))) { + Flushable.class.cast(actualDelegate).flush(); + } } } finally { l.unlock(); @@ -84,7 +111,7 @@ public class FlushableDataSourceHandler implements InvocationHandler { final Lock l = lock.readLock(); l.lock(); try { - return method.invoke(delegate, args); + return method.invoke(getDelegate(), args); } catch (final InvocationTargetException ite) { throw ite.getCause(); } finally { @@ -92,27 +119,30 @@ public class FlushableDataSourceHandler implements InvocationHandler { } } + @Override public CommonDataSource getDelegate() { - return delegate; + return delegate.get(); + } + + public void updateDataSource(final CommonDataSource ds) { // order is important, check DataSourceFactory + CommonDataSource current = ds; + while (Proxy.isProxyClass(current.getClass())) { + final InvocationHandler handler = Proxy.getInvocationHandler(current); + if (FlushableDataSourceHandler.class.isInstance(handler) || + ResettableDataSourceHandler.class.isInstance(handler)) { + current = DelegatableHandler.class.cast(handler).getDelegate(); + } else { + break; + } + } + delegate.set(current); } public static class FlushConfig { - public final String name; - public final boolean configuredManaged; - public final Class impl; - public final String definition; - public final Duration maxWaitTime; - public final Duration timeBetweenEvictionRuns; - public final Duration minEvictableIdleTime; + public final Map<String, Object> properties; - public FlushConfig(final String name, final boolean configuredManaged, final Class impl, final String definition, final Duration maxWaitTime, final Duration timeBetweenEvictionRuns, final Duration minEvictableIdleTime) { - this.name = name; - this.impl = impl; - this.configuredManaged = configuredManaged; - this.definition = definition; - this.maxWaitTime = maxWaitTime; - this.timeBetweenEvictionRuns = timeBetweenEvictionRuns; - this.minEvictableIdleTime = minEvictableIdleTime; + public FlushConfig(final Map<String, Object> properties) { + this.properties = properties; } } } http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandler.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandler.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandler.java new file mode 100644 index 0000000..d69c508 --- /dev/null +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandler.java @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.openejb.resource.jdbc; + +import org.apache.openejb.util.LogCategory; +import org.apache.openejb.util.Logger; + +import javax.sql.CommonDataSource; +import java.io.Flushable; +import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.sql.SQLException; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; + +import static java.util.Arrays.asList; + +public class ResettableDataSourceHandler implements DelegatableHandler { + private static final Logger LOGGER = Logger.getInstance(LogCategory.OPENEJB, ResettableDataSourceHandler.class.getName()); + + private final AtomicReference<CommonDataSource> delegate = new AtomicReference<>(); + private final RetryStrategy strategy; // TODO: add pause/exp backoff strategy + private Set<String> retryMethods = new HashSet<>(); + + public ResettableDataSourceHandler(final CommonDataSource ds, final String value, final String methods) { + this.delegate.set(ds); + + if (!"*".equals(methods)) { + this.retryMethods.addAll(asList(methods == null ? new String[]{"getConnection", "getXAConnection"} : methods.split(" *, *"))); + } + + final Runnable recreate = new Runnable() { + @Override + public void run() { + try { + Flushable.class.cast(delegate.get()).flush(); + } catch (final IOException ioe) { + LOGGER.error("Can't flush connection pool: " + ioe.getMessage()); + } + } + }; + + RetryStrategy tmp; + if (value.equals("true")) { + tmp = new CountRetryStrategy(recreate, 1); + } else if (value.startsWith("retry(") && value.endsWith(")")) { + tmp = new CountRetryStrategy(recreate, Integer.parseInt(value.substring("retry(".length(), value.length() - 1))); + } else { + try { + tmp = new CountRetryStrategy(recreate, Integer.parseInt(value.trim())); + } catch (final NumberFormatException nfe) { + try { + tmp = RetryStrategy.class.cast(Thread.currentThread().getContextClassLoader().loadClass(value) + .getConstructor(Runnable.class, String.class).newInstance(recreate, value)); + } catch (final InstantiationException | IllegalAccessException | ClassNotFoundException | NoSuchMethodException | InvocationTargetException e) { + throw new IllegalArgumentException("Unknown retry strategy: " + value, e); + } + } + } + strategy = tmp; + } + + @Override + public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + if (Object.class == method.getDeclaringClass() && "toString".equals(method.getName())) { + return "Resettable[" + getDelegate().toString() + "]"; + } + + Result retry = null; + do { + try { + return method.invoke(getDelegate(), args); + } catch (final InvocationTargetException ite) { + final Throwable cause = ite.getCause(); + if (SQLException.class.isInstance(cause) && isRetryMethod(method)) { + retry = strategy.shouldRetry(cause, retry); + if (!retry.status) { + throw cause; + } else { + continue; + } + } + throw cause; + } + } while (true); + } + + private boolean isRetryMethod(final Method method) { + return retryMethods.isEmpty() /* wildcard */ || retryMethods.contains(method.getName()); + } + + @Override + public CommonDataSource getDelegate() { + return delegate.get(); + } + + public void updateDelegate(final CommonDataSource ds) { + delegate.set(ds); + } + + public interface RetryStrategy { + Result shouldRetry(Throwable cause, Result previous); + } + + public static class Result { + private final boolean status; + + public Result(final boolean status) { + this.status = status; + } + } + + private static class CountRetryStrategy implements RetryStrategy { + private final Runnable task; + private final int max; + + public CountRetryStrategy(final Runnable recreate, final int max) { + this.task = recreate; + this.max = max; + } + + @Override + public Result shouldRetry(final Throwable cause, final Result previous) { + LOGGER.error("SQLException, resetting the connection pool.", cause); + + final Integer count = previous == null ? 1 : CountResult.class.cast(previous).count + 1; + task.run(); + return new CountResult(count <= max, count); + } + } + + private static class CountResult extends Result { + private int count; + + public CountResult(final boolean status, final int count) { + super(status); + this.count = count; + } + } +} http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/DbcpDataSourceCreator.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/DbcpDataSourceCreator.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/DbcpDataSourceCreator.java index 10be9a8..4e07d84 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/DbcpDataSourceCreator.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/dbcp/DbcpDataSourceCreator.java @@ -24,12 +24,12 @@ import org.apache.openejb.resource.jdbc.managed.xa.ManagedXADataSource; import org.apache.openejb.resource.jdbc.pool.PoolDataSourceCreator; import org.apache.openejb.resource.jdbc.pool.XADataSourceResource; -import java.util.Properties; import javax.sql.CommonDataSource; import javax.sql.DataSource; import javax.sql.XADataSource; import javax.transaction.TransactionManager; import javax.transaction.TransactionSynchronizationRegistry; +import java.util.Properties; // just a sample showing how to implement a datasourcecreator // this one will probably not be used since dbcp has already the integration we need http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/driver/AlternativeDriver.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/driver/AlternativeDriver.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/driver/AlternativeDriver.java index 3c171f7..f2028cd 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/driver/AlternativeDriver.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/driver/AlternativeDriver.java @@ -115,7 +115,10 @@ public class AlternativeDriver implements Driver { @Override public Connection connect(final String url, final Properties info) throws SQLException { - return getDelegate().connect(url, info); + if (acceptsURL(url)) { + return getDelegate().connect(url, info); + } + return null; } @Override http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/logging/LoggingSqlDataSource.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/logging/LoggingSqlDataSource.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/logging/LoggingSqlDataSource.java index 8dbc3d7..96fc62e 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/logging/LoggingSqlDataSource.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/logging/LoggingSqlDataSource.java @@ -17,14 +17,15 @@ package org.apache.openejb.resource.jdbc.logging; +import org.apache.openejb.resource.jdbc.DelegatableHandler; + import javax.sql.CommonDataSource; -import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.sql.Connection; -public class LoggingSqlDataSource implements InvocationHandler { +public class LoggingSqlDataSource implements DelegatableHandler { private static final Class<?>[] INTERFACES = new Class<?>[]{Connection.class}; private final CommonDataSource delegate; @@ -37,6 +38,10 @@ public class LoggingSqlDataSource implements InvocationHandler { @Override public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + if (Object.class == method.getDeclaringClass() && "toString".equals(method.getName())) { + return "Logging[" + delegate.toString() + "]"; + } + final Object result; try { result = method.invoke(delegate, args); @@ -51,6 +56,7 @@ public class LoggingSqlDataSource implements InvocationHandler { return result; } + @Override public CommonDataSource getDelegate() { return delegate; } http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/DefaultDataSourceCreator.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/DefaultDataSourceCreator.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/DefaultDataSourceCreator.java index 9f5be66..c4dba88 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/DefaultDataSourceCreator.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/DefaultDataSourceCreator.java @@ -29,7 +29,6 @@ import javax.sql.CommonDataSource; import javax.sql.DataSource; import java.util.Properties; -// TODO: remove it and replace it with org.apache.openejb.resource.jdbc.dbcp.DbcpDataSourceCreator public class DefaultDataSourceCreator extends DbcpDataSourceCreator { @Override public DataSource managed(final String name, final CommonDataSource ds) { @@ -69,14 +68,4 @@ public class DefaultDataSourceCreator extends DbcpDataSourceCreator { build(BasicManagedDataSource.class, ds, properties); return ds; } - - @Override - public void destroy(final Object object) throws Throwable { - ((org.apache.commons.dbcp2.BasicDataSource) object).close(); - } - - @Override - protected void doDestroy(final CommonDataSource dataSource) throws Throwable { - ((org.apache.commons.dbcp2.BasicDataSource) dataSource).close(); - } } http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/PoolDataSourceCreator.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/PoolDataSourceCreator.java b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/PoolDataSourceCreator.java index 31360a7..2929117 100644 --- a/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/PoolDataSourceCreator.java +++ b/container/openejb-core/src/main/java/org/apache/openejb/resource/jdbc/pool/PoolDataSourceCreator.java @@ -27,15 +27,15 @@ import org.apache.openejb.util.PassthroughFactory; import org.apache.xbean.recipe.ObjectRecipe; import org.apache.xbean.recipe.Option; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; -import java.util.Properties; import javax.sql.CommonDataSource; import javax.sql.DataSource; import javax.sql.XADataSource; import javax.transaction.TransactionManager; import javax.transaction.TransactionSynchronizationRegistry; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import java.util.Properties; public abstract class PoolDataSourceCreator implements DataSourceCreator { protected final Map<Object, ObjectRecipe> recipes = new HashMap<Object, ObjectRecipe>(); http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/test/java/org/apache/openejb/core/singleton/DependsOnTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/core/singleton/DependsOnTest.java b/container/openejb-core/src/test/java/org/apache/openejb/core/singleton/DependsOnTest.java index 1522337..9005ea8 100644 --- a/container/openejb-core/src/test/java/org/apache/openejb/core/singleton/DependsOnTest.java +++ b/container/openejb-core/src/test/java/org/apache/openejb/core/singleton/DependsOnTest.java @@ -33,6 +33,8 @@ import org.apache.openejb.jee.EjbJar; import org.apache.openejb.jee.SingletonBean; import org.apache.openejb.jee.StatelessBean; import org.junit.AfterClass; +import org.junit.FixMethodOrder; +import org.junit.runners.MethodSorters; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -78,6 +80,7 @@ public class DependsOnTest extends TestCase { final StatelessSessionContainerInfo statelessContainer = config.configureService(StatelessSessionContainerInfo.class); statelessContainer.properties.setProperty("MinSize", "1"); + statelessContainer.properties.setProperty("MaxSize", "1"); assembler.createContainer(statelessContainer); actual.clear(); http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandlerTest.java ---------------------------------------------------------------------- diff --git a/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandlerTest.java b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandlerTest.java new file mode 100644 index 0000000..8eeb5c5 --- /dev/null +++ b/container/openejb-core/src/test/java/org/apache/openejb/resource/jdbc/ResettableDataSourceHandlerTest.java @@ -0,0 +1,159 @@ +package org.apache.openejb.resource.jdbc; + +import org.apache.openejb.junit.ApplicationComposer; +import org.apache.openejb.testing.Classes; +import org.apache.openejb.testing.ContainerProperties; +import org.apache.openejb.testing.SimpleLog; +import org.hsqldb.jdbcDriver; +import org.junit.Test; +import org.junit.runner.RunWith; + +import javax.annotation.Resource; +import javax.ejb.EJB; +import javax.ejb.Singleton; +import javax.sql.DataSource; +import java.io.Flushable; +import java.io.IOException; +import java.sql.Connection; +import java.sql.Driver; +import java.sql.DriverManager; +import java.sql.DriverPropertyInfo; +import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; +import java.sql.Statement; +import java.util.Properties; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +@SimpleLog +@ContainerProperties({ + @ContainerProperties.Property(name = "db", value = "new://Resource?type=DataSource"), + @ContainerProperties.Property(name = "db.ResetOnError", value = "true"), // = retry(1) + // @ContainerProperties.Property(name = "db.ResetOnErrorMethods", value = "*"), + @ContainerProperties.Property(name = "db.JdbcDriver", value = "org.apache.openejb.resource.jdbc.ResettableDataSourceHandlerTest$DsTestDriver"), +}) +@Classes(innerClassesAsBean = true) +@RunWith(ApplicationComposer.class) +public class ResettableDataSourceHandlerTest { + @EJB + private DsAccessor accessor; + + @Resource + private DataSource ds; + + @Test + public void run() throws IOException { + actualTest(ds); // no tx + Flushable.class.cast(ds).flush(); // ensure we dont reuse previous checks cached connection + accessor.doTest(); // tx + } + + public static void actualTest(final DataSource ds) { + DsTestDriver.getConnectionCount.set(0); + + DsTestDriver.fail = true; // fail first otherwise we can get a cached connection with our test driver - ie hsqldb + try { + useConnection(ds); + } catch (final SQLException e) { + // no-op + } + assertEquals(0, DsTestDriver.getConnectionCount.get()); + + DsTestDriver.fail = false; + DsTestDriver.resilient = true; // will work since we retry by default + try { + useConnection(ds); + } catch (final SQLException e) { + // no-op + } + assertEquals(1 /* our conn */ + 1 /* the one created to validate the pool */, DsTestDriver.getConnectionCount.get()); + + DsTestDriver.resilient = false; + try { + useConnection(ds); + assertEquals(2 /* reuse existing */, DsTestDriver.getConnectionCount.get()); + } catch (final SQLException e) { + e.printStackTrace(); + fail(e.getMessage()); + } + } + + private static void useConnection(final DataSource ds) throws SQLException { + try (final Connection connection = ds.getConnection()) { + try (final Statement unused = connection.createStatement()) { + // just touched a method to force connection init + } + } + } + + @Singleton + public static class DsAccessor { + @Resource + private DataSource ds; + + public void doTest() { + actualTest(ds); + } + } + + public static class DsTestDriver implements Driver { + private static volatile boolean fail; + private static boolean resilient; + private static AtomicInteger getConnectionCount = new AtomicInteger(); + + public DsTestDriver() { + jdbcDriver.class.getName(); + } + + @Override + public Connection connect(final String url, final Properties info) throws SQLException { + if (fail) { + throw new SQLException(); + } + if (resilient) { + resilient = false; + throw new SQLException(); + } + + getConnectionCount.incrementAndGet(); + try { + return DriverManager.getConnection("jdbc:hsqldb:mem:resettabletest"); + } catch (final SQLException e) { + throw new IllegalStateException(e); + } + } + + @Override + public boolean acceptsURL(final String url) throws SQLException { + return true; + } + + @Override + public DriverPropertyInfo[] getPropertyInfo(String url, Properties info) throws SQLException { + return new DriverPropertyInfo[0]; + } + + @Override + public int getMajorVersion() { + return 0; + } + + @Override + public int getMinorVersion() { + return 0; + } + + @Override + public boolean jdbcCompliant() { + return false; + } + + @Override + public Logger getParentLogger() throws SQLFeatureNotSupportedException { + return null; + } + } +} http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index c83caa9..6e540b1 100644 --- a/pom.xml +++ b/pom.xml @@ -95,7 +95,7 @@ <tomee.version>7.0.0-SNAPSHOT</tomee.version> <openjpa.version>2.4.0</openjpa.version> - <org.apache.openwebbeans.version>1.6.2-SNAPSHOT</org.apache.openwebbeans.version> + <org.apache.openwebbeans.version>1.6.2</org.apache.openwebbeans.version> <jcs.version>2.0-SNAPSHOT</jcs.version> <!-- Maven module versions --> http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/server/openejb-bonecp/src/main/java/org/apache/openejb/bonecp/BoneCPDataSourceCreator.java ---------------------------------------------------------------------- diff --git a/server/openejb-bonecp/src/main/java/org/apache/openejb/bonecp/BoneCPDataSourceCreator.java b/server/openejb-bonecp/src/main/java/org/apache/openejb/bonecp/BoneCPDataSourceCreator.java index 2deed33..11c30db 100644 --- a/server/openejb-bonecp/src/main/java/org/apache/openejb/bonecp/BoneCPDataSourceCreator.java +++ b/server/openejb-bonecp/src/main/java/org/apache/openejb/bonecp/BoneCPDataSourceCreator.java @@ -69,6 +69,8 @@ public class BoneCPDataSourceCreator extends PoolDataSourceCreator { return pool; } + + private BoneCPDataSource createPool(final Properties properties) { final BoneCPConfig config; try { http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/tck/cdi-embedded/src/test/resources/failing.xml ---------------------------------------------------------------------- diff --git a/tck/cdi-embedded/src/test/resources/failing.xml b/tck/cdi-embedded/src/test/resources/failing.xml index ea96a64..11df52f 100644 --- a/tck/cdi-embedded/src/test/resources/failing.xml +++ b/tck/cdi-embedded/src/test/resources/failing.xml @@ -16,6 +16,9 @@ limitations under the License. --> <suite name="CDI TCK" verbose="0"> + <listeners> + <listener class-name="org.jboss.cdi.tck.impl.testng.SingleTestClassMethodInterceptor"/> + </listeners> <test name="CDI TCK"> <!-- runner helping properties -ea @@ -32,7 +35,8 @@ -Dopenejb.cdi.conversation.http.use-get-parameter=true --> <classes> - <class name="org.jboss.cdi.tck.tests.implementation.simple.lifecycle.SimpleBeanLifecycleTest" /> + <class name="org.jboss.cdi.tck.tests.context.NormalContextTest" /> + <class name="org.jboss.cdi.tck.tests.context.ContextDestroysBeansTest" /> </classes> </test> </suite> http://git-wip-us.apache.org/repos/asf/tomee/blob/5fdf755b/tomee/tomee-jdbc/src/main/java/org/apache/tomee/jdbc/TomEEDataSourceCreator.java ---------------------------------------------------------------------- diff --git a/tomee/tomee-jdbc/src/main/java/org/apache/tomee/jdbc/TomEEDataSourceCreator.java b/tomee/tomee-jdbc/src/main/java/org/apache/tomee/jdbc/TomEEDataSourceCreator.java index cb813ee..388487f 100644 --- a/tomee/tomee-jdbc/src/main/java/org/apache/tomee/jdbc/TomEEDataSourceCreator.java +++ b/tomee/tomee-jdbc/src/main/java/org/apache/tomee/jdbc/TomEEDataSourceCreator.java @@ -44,9 +44,19 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.sql.SQLException; import java.util.Properties; +import java.util.logging.Logger; public class TomEEDataSourceCreator extends PoolDataSourceCreator { @Override + public void resetConnections(final CommonDataSource ds) { + if (TomEEDataSource.class.isInstance(ds)) { + TomEEDataSource.class.cast(ds).purge(); + } else { + super.resetConnections(ds); + } + } + + @Override public DataSource pool(final String name, final DataSource ds, final Properties properties) { final PoolConfiguration config = build(TomEEPoolProperties.class, createProperties(name, properties)); config.setDataSource(ds);
