GH-2367 - Remove AutoCloseableQueryRunner proxy.

Closes #2367, delegating is the nicer choice for all driver versions.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/DefaultNeo4jClient.java

@@ -15,10 +15,6 @@
  */
 package org.springframework.data.neo4j.core;
 
-import java.lang.reflect.InvocationHandler;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Optional;
@@ -28,10 +24,12 @@
 import java.util.stream.Collectors;
 
 import org.neo4j.driver.Driver;
+import org.neo4j.driver.Query;
 import org.neo4j.driver.QueryRunner;
 import org.neo4j.driver.Record;
 import org.neo4j.driver.Result;
 import org.neo4j.driver.Session;
+import org.neo4j.driver.Value;
 import org.neo4j.driver.summary.ResultSummary;
 import org.neo4j.driver.types.TypeSystem;
 import org.springframework.core.convert.ConversionService;
@@ -71,50 +69,57 @@ class DefaultNeo4jClient implements Neo4jClient {
 		new Neo4jConversions().registerConvertersIn((ConverterRegistry) conversionService);
 	}
 
-	AutoCloseableQueryRunner getQueryRunner(@Nullable final String targetDatabase) {
+	QueryRunner getQueryRunner(@Nullable final String targetDatabase) {
 
 		QueryRunner queryRunner = Neo4jTransactionManager.retrieveTransaction(driver, targetDatabase);
 		if (queryRunner == null) {
 			queryRunner = driver.session(Neo4jTransactionUtils.defaultSessionConfig(targetDatabase));
 		}
 
-		return (AutoCloseableQueryRunner) Proxy.newProxyInstance(this.getClass().getClassLoader(),
-				new Class<?>[] { AutoCloseableQueryRunner.class }, new AutoCloseableQueryRunnerHandler(queryRunner));
+		return new DelegatingQueryRunner(queryRunner);
 	}
 
-	/**
-	 * Makes a query runner automatically closeable and aware whether it's session or a transaction
-	 */
-	interface AutoCloseableQueryRunner extends QueryRunner, AutoCloseable {
+	private static class DelegatingQueryRunner implements QueryRunner {
+
+		private final QueryRunner delegate;
+
+		private DelegatingQueryRunner(QueryRunner delegate) {
+			this.delegate = delegate;
+		}
 
 		@Override
-		void close();
-	}
+		public void close() throws Exception {
 
-	static class AutoCloseableQueryRunnerHandler implements InvocationHandler {
+			// We're only going to close sessions we have acquired inside the client, not something that
+			// has been retrieved from the tx manager.
+			if (this.delegate instanceof Session) {
+				this.delegate.close();
+			}
+		}
 
-		private final QueryRunner target;
+		@Override
+		public Result run(String s, Value value) {
+			return delegate.run(s, value);
+		}
 
-		AutoCloseableQueryRunnerHandler(QueryRunner target) {
-			this.target = target;
+		@Override
+		public Result run(String s, Map<String, Object> map) {
+			return delegate.run(s, map);
 		}
 
-		@Nullable
 		@Override
-		public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+		public Result run(String s, Record record) {
+			return delegate.run(s, record);
+		}
 
-			if ("close".equals(method.getName())) {
-				if (this.target instanceof Session) {
-					((Session) this.target).close();
-				}
-				return null;
-			} else {
-				try {
-					return method.invoke(target, args);
-				} catch (InvocationTargetException ite) {
-					throw ite.getCause();
-				}
-			}
+		@Override
+		public Result run(String s) {
+			return delegate.run(s);
+		}
+
+		@Override
+		public Result run(Query query) {
+			return delegate.run(query);
 		}
 	}
 
@@ -159,7 +164,7 @@ static class RunnableStatement {
 
 		private final NamedParameters parameters;
 
-		protected final Result runWith(AutoCloseableQueryRunner statementRunner) {
+		protected final Result runWith(QueryRunner statementRunner) {
 			String statementTemplate = cypherSupplier.get();
 
 			if (cypherLog.isDebugEnabled()) {
@@ -257,11 +262,13 @@ public RecordFetchSpec<Map<String, Object>> fetch() {
 		@Override
 		public ResultSummary run() {
 
-			try (AutoCloseableQueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
+			try (QueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
 				Result result = runnableStatement.runWith(statementRunner);
 				return ResultSummaries.process(result.consume());
 			} catch (RuntimeException e) {
 				throw potentiallyConvertRuntimeException(e, persistenceExceptionTranslator);
+			} catch (Exception e) {
+				throw new RuntimeException(e);
 			}
 		}
 
@@ -306,7 +313,7 @@ public RecordFetchSpec<T> mappedBy(
 		@Override
 		public Optional<T> one() {
 
-			try (AutoCloseableQueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
+			try (QueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
 				Result result = runnableStatement.runWith(statementRunner);
 				Optional<T> optionalValue = result.hasNext() ?
 						Optional.ofNullable(mappingFunction.apply(typeSystem, result.single())) :
@@ -315,32 +322,38 @@ public Optional<T> one() {
 				return optionalValue;
 			} catch (RuntimeException e) {
 				throw potentiallyConvertRuntimeException(e, persistenceExceptionTranslator);
+			} catch (Exception e) {
+				throw new RuntimeException(e);
 			}
 		}
 
 		@Override
 		public Optional<T> first() {
 
-			try (AutoCloseableQueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
+			try (QueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
 				Result result = runnableStatement.runWith(statementRunner);
 				Optional<T> optionalValue = result.stream().map(partialMappingFunction(typeSystem)).findFirst();
 				ResultSummaries.process(result.consume());
 				return optionalValue;
 			} catch (RuntimeException e) {
 				throw potentiallyConvertRuntimeException(e, persistenceExceptionTranslator);
+			} catch (Exception e) {
+				throw new RuntimeException(e);
 			}
 		}
 
 		@Override
 		public Collection<T> all() {
 
-			try (AutoCloseableQueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
+			try (QueryRunner statementRunner = getQueryRunner(this.targetDatabase)) {
 				Result result = runnableStatement.runWith(statementRunner);
 				Collection<T> values = result.stream().map(partialMappingFunction(typeSystem)).collect(Collectors.toList());
 				ResultSummaries.process(result.consume());
 				return values;
 			} catch (RuntimeException e) {
 				throw potentiallyConvertRuntimeException(e, persistenceExceptionTranslator);
+			} catch (Exception e) {
+				throw new RuntimeException(e);
 			}
 		}
 
@@ -377,8 +390,12 @@ public RunnableDelegation<T> in(@Nullable @SuppressWarnings("HiddenField") Strin
 
 		@Override
 		public Optional<T> run() {
-			try (AutoCloseableQueryRunner queryRunner = getQueryRunner(targetDatabase)) {
+			try (QueryRunner queryRunner = getQueryRunner(targetDatabase)) {
 				return callback.apply(queryRunner);
+			} catch (RuntimeException e) {
+				throw potentiallyConvertRuntimeException(e, persistenceExceptionTranslator);
+			} catch (Exception e) {
+				throw new RuntimeException(e);
 			}
 		}
 	}

src/test/java/org/springframework/data/neo4j/core/TransactionHandlingTest.java

@@ -39,6 +39,7 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.neo4j.driver.Driver;
+import org.neo4j.driver.QueryRunner;
 import org.neo4j.driver.Session;
 import org.neo4j.driver.SessionConfig;
 import org.neo4j.driver.Transaction;
@@ -92,8 +93,10 @@ void shouldCallCloseOnSession() {
 
 				// Make template acquire session
 				DefaultNeo4jClient neo4jClient = new DefaultNeo4jClient(driver, null);
-				try (DefaultNeo4jClient.AutoCloseableQueryRunner s = neo4jClient.getQueryRunner("aDatabase")) {
+				try (QueryRunner s = neo4jClient.getQueryRunner("aDatabase")) {
 					s.run("MATCH (n) RETURN n");
+				} catch (Exception e) {
+					throw new RuntimeException(e);
 				}
 
 				verify(driver).session(configArgumentCaptor.capture());
@@ -126,8 +129,10 @@ void shouldNotInvokeCloseOnTransaction() {
 
 				DefaultNeo4jClient neo4jClient = new DefaultNeo4jClient(driver, null);
 				txTemplate.execute(tx -> {
-					try (DefaultNeo4jClient.AutoCloseableQueryRunner s = neo4jClient.getQueryRunner(null)) {
+					try (QueryRunner s = neo4jClient.getQueryRunner(null)) {
 						s.run("MATCH (n) RETURN n");
+					} catch (Exception e) {
+						throw new RuntimeException(e);
 					}
 					return null;
 				});