You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by sr...@apache.org on 2018/10/27 12:15:48 UTC
[incubator-plc4x] branch master updated: [plc4j-pool] invalidate
connections on PlcConnectionException
This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-plc4x.git
The following commit(s) were added to refs/heads/master by this push:
new 0654828 [plc4j-pool] invalidate connections on PlcConnectionException
0654828 is described below
commit 0654828e3625bc4841dbc5a1d00df8b809603d95
Author: Sebastian Rühl <sr...@apache.org>
AuthorDate: Sat Oct 27 14:13:05 2018 +0200
[plc4j-pool] invalidate connections on PlcConnectionException
---
.../connectionpool/PooledPlcDriverManager.java | 57 +++++++++++++---------
.../connectionpool/PooledPlcDriverManagerTest.java | 47 ++++++++++++++++--
2 files changed, 77 insertions(+), 27 deletions(-)
diff --git a/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java b/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
index 2f917d8..b0f9701 100644
--- a/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
+++ b/plc4j/utils/connection-pool/src/main/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManager.java
@@ -26,6 +26,7 @@ import org.apache.plc4x.java.api.exceptions.PlcConnectionException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Proxy;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
@@ -75,34 +76,42 @@ public class PooledPlcDriverManager extends PlcDriverManager {
public PlcConnection getConnection(String url, PlcAuthentication authentication) throws PlcConnectionException {
PoolKey poolKey = PoolKey.of(url, authentication);
ObjectPool<PlcConnection> pool = retrieveFromPool(poolKey);
- try {
- if (LOGGER.isDebugEnabled()) {
- if (authentication != noPlcAuthentication) {
- LOGGER.debug("Try to borrow an object for url {} and authentication {}", url, authentication);
- } else {
- LOGGER.debug("Try to borrow an object for url {}", url);
- }
+ if (LOGGER.isDebugEnabled()) {
+ if (authentication != noPlcAuthentication) {
+ LOGGER.debug("Try to borrow an object for url {} and authentication {}", url, authentication);
+ } else {
+ LOGGER.debug("Try to borrow an object for url {}", url);
}
- PlcConnection plcConnection = pool.borrowObject();
- // Used to invalidate a proxy
- AtomicBoolean proxyInvalidated = new AtomicBoolean(false);
- return (PlcConnection) Proxy.newProxyInstance(classLoader, new Class[]{PlcConnection.class}, (proxy, method, args) -> {
- if (proxyInvalidated.get()) {
- throw new IllegalStateException("Proxy not valid anymore");
- }
- if ("close".equals(method.getName())) {
- LOGGER.debug("close called on {}. Returning to {}", plcConnection, pool);
- proxyInvalidated.set(true);
- pool.returnObject(plcConnection);
- return null;
- } else {
- // TODO: add exception handler which catches exceptions like plcConnectionException and then invalidates them
- return method.invoke(plcConnection, args);
- }
- });
+ }
+ PlcConnection plcConnection;
+ try {
+ plcConnection = pool.borrowObject();
} catch (Exception e) {
throw new PlcConnectionException(e);
}
+ // Used to invalidate a proxy
+ AtomicBoolean proxyInvalidated = new AtomicBoolean(false);
+ return (PlcConnection) Proxy.newProxyInstance(classLoader, new Class[]{PlcConnection.class}, (proxy, method, args) -> {
+ if (proxyInvalidated.get()) {
+ throw new IllegalStateException("Proxy not valid anymore");
+ }
+ if ("close".equals(method.getName())) {
+ LOGGER.debug("close called on {}. Returning to {}", plcConnection, pool);
+ proxyInvalidated.set(true);
+ pool.returnObject(plcConnection);
+ return null;
+ } else {
+ try {
+ return method.invoke(plcConnection, args);
+ } catch (InvocationTargetException e) {
+ if (e.getCause().getClass() == PlcConnectionException.class) {
+ pool.invalidateObject(plcConnection);
+ proxyInvalidated.set(true);
+ }
+ throw e;
+ }
+ }
+ });
}
private ObjectPool<PlcConnection> retrieveFromPool(PoolKey poolKey) {
diff --git a/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java b/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
index 3c79b46..9ab3972 100644
--- a/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
+++ b/plc4j/utils/connection-pool/src/test/java/org/apache/plc4x/java/utils/connectionpool/PooledPlcDriverManagerTest.java
@@ -47,6 +47,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.IntStream;
@@ -224,6 +225,47 @@ class PooledPlcDriverManagerTest implements WithAssertions {
assertThatThrownBy(connection::unsubscriptionRequestBuilder).isInstanceOf(IllegalStateException.class).hasMessage("Proxy not valid anymore");
}
+ @Test
+ void cleanupOfBrokenConnections() throws Exception {
+ AtomicBoolean failNow = new AtomicBoolean(false);
+ when(plcDriver.connect(anyString())).then(invocationOnMock -> {
+ DummyPlcConnection dummyPlcConnection = spy(new DummyPlcConnection(invocationOnMock.getArgument(0)));
+ // we fake an connection which breaks at this call
+ doAnswer(invocation -> {
+ if (failNow.get()) {
+ throw new PlcConnectionException("blub");
+ }
+ return invocation.callRealMethod();
+ }).when(dummyPlcConnection).connect();
+ return dummyPlcConnection;
+ });
+
+ assertThat(SUT.getStatistics()).containsOnly(
+ entry("pools.count", 0)
+ );
+ PlcConnection connection = SUT.getConnection("dummydummy:breakIt");
+ assertThat(SUT.getStatistics()).containsOnly(
+ entry("pools.count", 1),
+ entry("dummydummy:breakIt.numActive", 1),
+ entry("dummydummy:breakIt.numIdle", 0)
+ );
+ failNow.set(true);
+ try {
+ connection.connect();
+ fail("This should throw an exception");
+ } catch (Exception e) {
+ // TODO: currently UndeclaredThrowableException is the top one which should be InvocationTargetException
+ //assertThat(e).isInstanceOf(InvocationTargetException.class);
+ assertThat(e).hasRootCauseInstanceOf(PlcConnectionException.class);
+ }
+ // Faulty connection should have been discarded
+ assertThat(SUT.getStatistics()).containsOnly(
+ entry("pools.count", 1),
+ entry("dummydummy:breakIt.numActive", 0),
+ entry("dummydummy:breakIt.numIdle", 0)
+ );
+ }
+
@Nested
class PoolCleanup {
@Test
@@ -283,7 +325,6 @@ class PooledPlcDriverManagerTest implements WithAssertions {
}
}
-
class DummyPlcConnection implements PlcConnection, PlcConnectionMetadata {
private final String url;
@@ -332,8 +373,8 @@ class PooledPlcDriverManagerTest implements WithAssertions {
}
@Override
- public void close() throws Exception {
- throw new UnsupportedOperationException("this should never be called due to pool");
+ public void close() {
+ connected = false;
}
@Override