From 80e82cd43f1c2ec85c405bff4b8de6f36f6d6e59 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Oct 2023 15:43:02 +0200 Subject: [PATCH 1/3] Do not close transactional Connection in doReleaseConnection Closes gh-28133 --- .../r2dbc/connection/ConnectionFactoryUtils.java | 1 + .../TransactionAwareConnectionFactoryProxyUnitTests.java | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java index c128a16790a..346c5d915ce 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java @@ -178,6 +178,7 @@ public abstract class ConnectionFactoryUtils { if (conHolder != null && connectionEquals(conHolder, connection)) { // It's the transactional Connection: Don't close it. conHolder.released(); + return Mono.empty(); } return Mono.from(connection.close()); }).onErrorResume(NoTransactionException.class, ex -> Mono.from(connection.close())); diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java index 8dd4794d39b..4d6bdc7b0ac 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java @@ -146,14 +146,15 @@ class TransactionAwareConnectionFactoryProxyUnitTests { ConnectionFactoryUtils.getConnection(connectionFactoryMock) .doOnNext(transactionalConnection::set).flatMap(connection -> proxyCf.create() .doOnNext(wrappedConnection -> assertThat(((Wrapped) wrappedConnection).unwrap()).isSameAs(connection))) - .as(rxtx::transactional) .flatMapMany(Connection::close) + .as(rxtx::transactional) .as(StepVerifier::create) .verifyComplete(); + verify(connectionFactoryMock, times(1)).create(); + verify(connectionMock1, times(1)).close(); verifyNoInteractions(connectionMock2); verifyNoInteractions(connectionMock3); - verify(connectionFactoryMock, times(1)).create(); } } From 87424cd6057049a03c9e35074c744a97469df16c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Oct 2023 15:44:07 +0200 Subject: [PATCH 2/3] Upgrade to SLF4J 2.0.9 and AspectJ 1.9.20.1 --- framework-platform/framework-platform.gradle | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/framework-platform/framework-platform.gradle b/framework-platform/framework-platform.gradle index ea531e51c38..6561c5718a6 100644 --- a/framework-platform/framework-platform.gradle +++ b/framework-platform/framework-platform.gradle @@ -103,9 +103,9 @@ dependencies { api("org.apache.tomcat.embed:tomcat-embed-websocket:10.1.14") api("org.apache.tomcat:tomcat-util:10.1.14") api("org.apache.tomcat:tomcat-websocket:10.1.14") - api("org.aspectj:aspectjrt:1.9.20") - api("org.aspectj:aspectjtools:1.9.20") - api("org.aspectj:aspectjweaver:1.9.20") + api("org.aspectj:aspectjrt:1.9.20.1") + api("org.aspectj:aspectjtools:1.9.20.1") + api("org.aspectj:aspectjweaver:1.9.20.1") api("org.assertj:assertj-core:3.24.2") api("org.awaitility:awaitility:4.2.0") api("org.bouncycastle:bcpkix-jdk18on:1.72") @@ -137,7 +137,7 @@ dependencies { api("org.seleniumhq.selenium:htmlunit-driver:2.70.0") api("org.seleniumhq.selenium:selenium-java:3.141.59") api("org.skyscreamer:jsonassert:1.5.1") - api("org.slf4j:slf4j-api:2.0.7") + api("org.slf4j:slf4j-api:2.0.9") api("org.testng:testng:7.8.0") api("org.webjars:underscorejs:1.8.3") api("org.webjars:webjars-locator-core:0.53") From 86650d1a39f585b22e33425c59e9d07c9db139a1 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 11 Oct 2023 16:10:53 +0200 Subject: [PATCH 3/3] Polishing --- .../connection/ConnectionFactoryUtils.java | 10 +++++----- .../connection/SingleConnectionFactory.java | 4 ++-- .../r2dbc/core/DefaultDatabaseClient.java | 2 +- .../R2dbcTransactionManagerUnitTests.java | 2 +- ...nAwareConnectionFactoryProxyUnitTests.java | 2 +- .../MapConnectionFactoryLookupUnitTests.java | 20 ++++++++++--------- .../core/NamedParameterUtilsUnitTests.java | 10 ++++++---- .../binding/IndexedBindMarkersUnitTests.java | 8 +------- .../binding/NamedBindMarkersUnitTests.java | 2 +- 9 files changed, 29 insertions(+), 31 deletions(-) diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java index 346c5d915ce..4c7e07683d3 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/ConnectionFactoryUtils.java @@ -302,13 +302,13 @@ public abstract class ConnectionFactoryUtils { * @return the innermost target Connection, or the passed-in one if not wrapped * @see Wrapped#unwrap() */ - @SuppressWarnings({"rawtypes", "unchecked"}) + @SuppressWarnings("unchecked") public static Connection getTargetConnection(Connection con) { - Object conToUse = con; - while (conToUse instanceof Wrapped wrapped) { - conToUse = wrapped.unwrap(); + Connection conToUse = con; + while (conToUse instanceof Wrapped) { + conToUse = ((Wrapped) conToUse).unwrap(); } - return (Connection) conToUse; + return conToUse; } /** diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/SingleConnectionFactory.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/SingleConnectionFactory.java index f40d34ea99b..527da9d9b00 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/SingleConnectionFactory.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/SingleConnectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -231,7 +231,7 @@ public class SingleConnectionFactory extends DelegatingConnectionFactory */ protected Connection getCloseSuppressingConnectionProxy(Connection target) { return (Connection) Proxy.newProxyInstance(SingleConnectionFactory.class.getClassLoader(), - new Class[] { Connection.class, Wrapped.class }, new CloseSuppressingInvocationHandler(target)); + new Class[] {Connection.class, Wrapped.class}, new CloseSuppressingInvocationHandler(target)); } diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java index dcfdbd33ee6..b9ff92fa7e8 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java @@ -186,7 +186,7 @@ final class DefaultDatabaseClient implements DatabaseClient { */ private static Connection createConnectionProxy(Connection con) { return (Connection) Proxy.newProxyInstance(DatabaseClient.class.getClassLoader(), - new Class[] { Connection.class, Wrapped.class }, + new Class[] {Connection.class, Wrapped.class}, new CloseSuppressingInvocationHandler(con)); } diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerUnitTests.java index 2dd2341cc71..d4041daeb78 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/R2dbcTransactionManagerUnitTests.java @@ -70,7 +70,7 @@ class R2dbcTransactionManagerUnitTests { @BeforeEach - @SuppressWarnings({"unchecked", "rawtypes"}) + @SuppressWarnings({"rawtypes", "unchecked"}) void before() { when(connectionFactoryMock.create()).thenReturn((Mono) Mono.just(connectionMock)); when(connectionMock.beginTransaction(any(io.r2dbc.spi.TransactionDefinition.class))).thenReturn(Mono.empty()); diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java index 4d6bdc7b0ac..de978346337 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/TransactionAwareConnectionFactoryProxyUnitTests.java @@ -57,7 +57,7 @@ class TransactionAwareConnectionFactoryProxyUnitTests { @BeforeEach - @SuppressWarnings({ "rawtypes", "unchecked" }) + @SuppressWarnings({"rawtypes", "unchecked"}) void before() { when(connectionFactoryMock.create()).thenReturn((Mono) Mono.just(connectionMock1), (Mono) Mono.just(connectionMock2), (Mono) Mono.just(connectionMock3)); diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/lookup/MapConnectionFactoryLookupUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/lookup/MapConnectionFactoryLookupUnitTests.java index 89815b71c2f..40ac7940094 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/lookup/MapConnectionFactoryLookupUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/connection/lookup/MapConnectionFactoryLookupUnitTests.java @@ -34,13 +34,14 @@ public class MapConnectionFactoryLookupUnitTests { private static final String CONNECTION_FACTORY_NAME = "connectionFactory"; + @Test public void getConnectionFactoriesReturnsUnmodifiableMap() { MapConnectionFactoryLookup lookup = new MapConnectionFactoryLookup(); Map connectionFactories = lookup.getConnectionFactories(); - assertThatThrownBy(() -> connectionFactories.put("", - new DummyConnectionFactory())).isInstanceOf(UnsupportedOperationException.class); + assertThatThrownBy(() -> connectionFactories.put("", new DummyConnectionFactory())) + .isInstanceOf(UnsupportedOperationException.class); } @Test @@ -52,8 +53,8 @@ public class MapConnectionFactoryLookupUnitTests { MapConnectionFactoryLookup lookup = new MapConnectionFactoryLookup(); lookup.setConnectionFactories(connectionFactories); - ConnectionFactory connectionFactory = lookup.getConnectionFactory(CONNECTION_FACTORY_NAME); - assertThat(connectionFactory).isNotNull().isSameAs(expectedConnectionFactory); + assertThat(lookup.getConnectionFactory(CONNECTION_FACTORY_NAME)) + .isNotNull().isSameAs(expectedConnectionFactory); } @Test @@ -67,12 +68,12 @@ public class MapConnectionFactoryLookupUnitTests { lookup.setConnectionFactories(connectionFactories); lookup.addConnectionFactory(CONNECTION_FACTORY_NAME, expectedConnectionFactory); - ConnectionFactory connectionFactory = lookup.getConnectionFactory(CONNECTION_FACTORY_NAME); - assertThat(connectionFactory).isNotNull().isSameAs(expectedConnectionFactory); + assertThat(lookup.getConnectionFactory(CONNECTION_FACTORY_NAME)) + .isNotNull().isSameAs(expectedConnectionFactory); } @Test - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({"rawtypes", "unchecked"}) public void getConnectionFactoryWhereSuppliedMapHasNonConnectionFactoryTypeUnderSpecifiedKey() { Map connectionFactories = new HashMap<>(); connectionFactories.put(CONNECTION_FACTORY_NAME, new Object()); @@ -86,8 +87,9 @@ public class MapConnectionFactoryLookupUnitTests { public void getConnectionFactoryWhereSuppliedMapHasNoEntryForSpecifiedKey() { MapConnectionFactoryLookup lookup = new MapConnectionFactoryLookup(); - assertThatThrownBy(() -> lookup.getConnectionFactory(CONNECTION_FACTORY_NAME)) - .isInstanceOf(ConnectionFactoryLookupFailureException.class); + assertThatThrownBy( + () -> lookup.getConnectionFactory(CONNECTION_FACTORY_NAME)).isInstanceOf( + ConnectionFactoryLookupFailureException.class); } } diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/NamedParameterUtilsUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/NamedParameterUtilsUnitTests.java index 021d721d976..8bb5d5eb537 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/NamedParameterUtilsUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/NamedParameterUtilsUnitTests.java @@ -84,8 +84,9 @@ public class NamedParameterUtilsUnitTests { @Test public void substituteObjectArray() { MapBindParameterSource namedParams = new MapBindParameterSource(new HashMap<>()); - namedParams.addValue("a", Arrays.asList(new Object[] { "Walter", "Heisenberg" }, - new Object[] { "Walt Jr.", "Flynn" })); + namedParams.addValue("a", + Arrays.asList(new Object[] {"Walter", "Heisenberg"}, + new Object[] {"Walt Jr.", "Flynn"})); PreparedOperation operation = NamedParameterUtils.substituteNamedParameters( "xxx :a", BIND_MARKERS, namedParams); @@ -96,8 +97,9 @@ public class NamedParameterUtilsUnitTests { @Test public void shouldBindObjectArray() { MapBindParameterSource namedParams = new MapBindParameterSource(new HashMap<>()); - namedParams.addValue("a", Arrays.asList(new Object[] { "Walter", "Heisenberg" }, - new Object[] { "Walt Jr.", "Flynn" })); + namedParams.addValue("a", + Arrays.asList(new Object[] {"Walter", "Heisenberg"}, + new Object[] {"Walt Jr.", "Flynn"})); BindTarget bindTarget = mock(); diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/IndexedBindMarkersUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/IndexedBindMarkersUnitTests.java index 3a9292db36f..21c4a3bb046 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/IndexedBindMarkersUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/IndexedBindMarkersUnitTests.java @@ -32,7 +32,6 @@ class IndexedBindMarkersUnitTests { @Test void shouldCreateNewBindMarkers() { BindMarkersFactory factory = BindMarkersFactory.indexed("$", 0); - BindMarkers bindMarkers1 = factory.create(); BindMarkers bindMarkers2 = factory.create(); @@ -43,7 +42,6 @@ class IndexedBindMarkersUnitTests { @Test void shouldCreateNewBindMarkersWithOffset() { BindTarget bindTarget = mock(); - BindMarkers bindMarkers = BindMarkersFactory.indexed("$", 1).create(); BindMarker first = bindMarkers.next(); @@ -60,10 +58,9 @@ class IndexedBindMarkersUnitTests { @Test void nextShouldIncrementBindMarker() { - String[] prefixes = { "$", "?" }; + String[] prefixes = {"$", "?"}; for (String prefix : prefixes) { - BindMarkers bindMarkers = BindMarkersFactory.indexed(prefix, 0).create(); BindMarker marker1 = bindMarkers.next(); @@ -76,9 +73,7 @@ class IndexedBindMarkersUnitTests { @Test void bindValueShouldBindByIndex() { - BindTarget bindTarget = mock(); - BindMarkers bindMarkers = BindMarkersFactory.indexed("$", 0).create(); bindMarkers.next().bind(bindTarget, "foo"); @@ -91,7 +86,6 @@ class IndexedBindMarkersUnitTests { @Test void bindNullShouldBindByIndex() { BindTarget bindTarget = mock(); - BindMarkers bindMarkers = BindMarkersFactory.indexed("$", 0).create(); bindMarkers.next(); // ignore diff --git a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/NamedBindMarkersUnitTests.java b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/NamedBindMarkersUnitTests.java index 5b38d2c0478..1cc4b7580f1 100644 --- a/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/NamedBindMarkersUnitTests.java +++ b/spring-r2dbc/src/test/java/org/springframework/r2dbc/core/binding/NamedBindMarkersUnitTests.java @@ -43,7 +43,7 @@ class NamedBindMarkersUnitTests { } @ParameterizedTest - @ValueSource(strings = { "$", "?" }) + @ValueSource(strings = {"$", "?"}) void nextShouldIncrementBindMarker(String prefix) { BindMarkers bindMarkers = BindMarkersFactory.named(prefix, "p", 32).create();