From ae70e3c81c83b7420e54d53d85904d95d33abfd5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:09:17 +0200 Subject: [PATCH 1/6] Apply read-only enforcement after R2DBC transaction begin Includes prepareTransactionalConnection variant aligned with JDBC DataSourceTransactionManager. Closes gh-28610 --- .../connection/R2dbcTransactionManager.java | 96 ++++++++++--------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/R2dbcTransactionManager.java b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/R2dbcTransactionManager.java index e22c7a1dac9..2bebbd4c402 100644 --- a/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/R2dbcTransactionManager.java +++ b/spring-r2dbc/src/main/java/org/springframework/r2dbc/connection/R2dbcTransactionManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -47,7 +47,7 @@ import org.springframework.util.Assert; *

Note: The {@code ConnectionFactory} that this transaction manager * operates on needs to return independent {@code Connection}s. * The {@code Connection}s may come from a pool (the typical case), but the - * {@code ConnectionFactory} must not return scoped scoped {@code Connection}s + * {@code ConnectionFactory} must not return scoped {@code Connection}s * or the like. This transaction manager will associate {@code Connection} * with context-bound transactions itself, according to the specified propagation * behavior. It assumes that a separate, independent {@code Connection} can @@ -142,8 +142,8 @@ public class R2dbcTransactionManager extends AbstractReactiveTransactionManager * transactional connection: "SET TRANSACTION READ ONLY" as understood by Oracle, * MySQL and Postgres. *

The exact treatment, including any SQL statement executed on the connection, - * can be customized through through {@link #prepareTransactionalConnection}. - * @see #prepareTransactionalConnection + * can be customized through {@link #prepareTransactionalConnection(Connection, TransactionDefinition)}. + * @see #prepareTransactionalConnection(Connection, TransactionDefinition) */ public void setEnforceReadOnly(boolean enforceReadOnly) { this.enforceReadOnly = enforceReadOnly; @@ -179,6 +179,7 @@ public class R2dbcTransactionManager extends AbstractReactiveTransactionManager return (txObject.hasConnectionHolder() && txObject.getConnectionHolder().isTransactionActive()); } + @SuppressWarnings("deprecation") @Override protected Mono doBegin(TransactionSynchronizationManager synchronizationManager, Object transaction, TransactionDefinition definition) throws TransactionException { @@ -202,27 +203,27 @@ public class R2dbcTransactionManager extends AbstractReactiveTransactionManager connectionMono = Mono.just(txObject.getConnectionHolder().getConnection()); } - return connectionMono.flatMap(con -> { - return prepareTransactionalConnection(con, definition, transaction).then(Mono.from(con.beginTransaction())) - .doOnSuccess(v -> { - txObject.getConnectionHolder().setTransactionActive(true); - Duration timeout = determineTimeout(definition); - if (!timeout.isNegative() && !timeout.isZero()) { - txObject.getConnectionHolder().setTimeoutInMillis(timeout.toMillis()); - } - // Bind the connection holder to the thread. - if (txObject.isNewConnectionHolder()) { - synchronizationManager.bindResource(obtainConnectionFactory(), txObject.getConnectionHolder()); - } - }).thenReturn(con).onErrorResume(e -> { - if (txObject.isNewConnectionHolder()) { - return ConnectionFactoryUtils.releaseConnection(con, obtainConnectionFactory()) - .doOnTerminate(() -> txObject.setConnectionHolder(null, false)) - .then(Mono.error(e)); - } - return Mono.error(e); - }); - }).onErrorResume(e -> { + return connectionMono.flatMap(con -> prepareTransactionalConnection(con, definition, transaction) + .then(Mono.from(con.beginTransaction())) + .then(prepareTransactionalConnection(con, definition)) + .doOnSuccess(v -> { + txObject.getConnectionHolder().setTransactionActive(true); + Duration timeout = determineTimeout(definition); + if (!timeout.isNegative() && !timeout.isZero()) { + txObject.getConnectionHolder().setTimeoutInMillis(timeout.toMillis()); + } + // Bind the connection holder to the thread. + if (txObject.isNewConnectionHolder()) { + synchronizationManager.bindResource(obtainConnectionFactory(), txObject.getConnectionHolder()); + } + }).thenReturn(con).onErrorResume(e -> { + if (txObject.isNewConnectionHolder()) { + return ConnectionFactoryUtils.releaseConnection(con, obtainConnectionFactory()) + .doOnTerminate(() -> txObject.setConnectionHolder(null, false)) + .then(Mono.error(e)); + } + return Mono.error(e); + })).onErrorResume(e -> { CannotCreateTransactionException ex = new CannotCreateTransactionException( "Could not open R2DBC Connection for transaction", e); return Mono.error(ex); @@ -350,31 +351,17 @@ public class R2dbcTransactionManager extends AbstractReactiveTransactionManager } /** - * Prepare the transactional {@link Connection} right after transaction begin. - *

The default implementation executes a "SET TRANSACTION READ ONLY" statement if the - * {@link #setEnforceReadOnly "enforceReadOnly"} flag is set to {@code true} and the - * transaction definition indicates a read-only transaction. - *

The "SET TRANSACTION READ ONLY" is understood by Oracle, MySQL and Postgres - * and may work with other databases as well. If you'd like to adapt this treatment, - * override this method accordingly. - * @param con the transactional R2DBC Connection - * @param definition the current transaction definition - * @param transaction the transaction object - * @see #setEnforceReadOnly + * Prepare the transactional {@link Connection} right before transaction begin. + * @deprecated in favor of {@link #prepareTransactionalConnection(Connection, TransactionDefinition)} + * since this variant gets called too early (before transaction begin) for read-only customization */ + @Deprecated protected Mono prepareTransactionalConnection( Connection con, TransactionDefinition definition, Object transaction) { ConnectionFactoryTransactionObject txObject = (ConnectionFactoryTransactionObject) transaction; - Mono prepare = Mono.empty(); - if (isEnforceReadOnly() && definition.isReadOnly()) { - prepare = Mono.from(con.createStatement("SET TRANSACTION READ ONLY").execute()) - .flatMapMany(Result::getRowsUpdated) - .then(); - } - // Apply specific isolation level, if any. IsolationLevel isolationLevelToUse = resolveIsolationLevel(definition.getIsolationLevel()); if (isolationLevelToUse != null && definition.getIsolationLevel() != TransactionDefinition.ISOLATION_DEFAULT) { @@ -404,6 +391,29 @@ public class R2dbcTransactionManager extends AbstractReactiveTransactionManager return prepare; } + /** + * Prepare the transactional {@link Connection} right after transaction begin. + *

The default implementation executes a "SET TRANSACTION READ ONLY" statement if the + * {@link #setEnforceReadOnly "enforceReadOnly"} flag is set to {@code true} and the + * transaction definition indicates a read-only transaction. + *

The "SET TRANSACTION READ ONLY" is understood by Oracle, MySQL and Postgres + * and may work with other databases as well. If you'd like to adapt this treatment, + * override this method accordingly. + * @param con the transactional R2DBC Connection + * @param definition the current transaction definition + * @since 5.3.22 + * @see #setEnforceReadOnly + */ + protected Mono prepareTransactionalConnection(Connection con, TransactionDefinition definition) { + Mono prepare = Mono.empty(); + if (isEnforceReadOnly() && definition.isReadOnly()) { + prepare = Mono.from(con.createStatement("SET TRANSACTION READ ONLY").execute()) + .flatMapMany(Result::getRowsUpdated) + .then(); + } + return prepare; + } + /** * Resolve the {@linkplain TransactionDefinition#getIsolationLevel() isolation level constant} to a R2DBC * {@link IsolationLevel}. If you'd like to extend isolation level translation for vendor-specific From 5247eeba84a879fe48e18de4f38490dd8d5afba5 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:09:30 +0200 Subject: [PATCH 2/6] Support LocalDate/Time for SQL type mappings Closes gh-28778 --- .../springframework/jdbc/core/StatementCreatorUtils.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java index 5c6c1d859d2..5d60846c54b 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/core/StatementCreatorUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -26,6 +26,9 @@ import java.sql.DatabaseMetaData; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Types; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; import java.util.Arrays; import java.util.Calendar; import java.util.Collection; @@ -98,6 +101,9 @@ public abstract class StatementCreatorUtils { javaTypeToSqlTypeMap.put(double.class, Types.DOUBLE); javaTypeToSqlTypeMap.put(Double.class, Types.DOUBLE); javaTypeToSqlTypeMap.put(BigDecimal.class, Types.DECIMAL); + javaTypeToSqlTypeMap.put(LocalDate.class, Types.DATE); + javaTypeToSqlTypeMap.put(LocalTime.class, Types.TIME); + javaTypeToSqlTypeMap.put(LocalDateTime.class, Types.TIMESTAMP); javaTypeToSqlTypeMap.put(java.sql.Date.class, Types.DATE); javaTypeToSqlTypeMap.put(java.sql.Time.class, Types.TIME); javaTypeToSqlTypeMap.put(java.sql.Timestamp.class, Types.TIMESTAMP); From d72aeac319e1f98018acf41f2c1c185e9e0652b8 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:09:43 +0200 Subject: [PATCH 3/6] Create well-known non-interface types without using reflection Closes gh-28718 --- .../core/CollectionFactory.java | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java index 4f90a3bff18..15e3ee77538 100644 --- a/spring-core/src/main/java/org/springframework/core/CollectionFactory.java +++ b/spring-core/src/main/java/org/springframework/core/CollectionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -181,19 +181,18 @@ public final class CollectionFactory { @SuppressWarnings({"unchecked", "cast"}) public static Collection createCollection(Class collectionType, @Nullable Class elementType, int capacity) { Assert.notNull(collectionType, "Collection type must not be null"); - if (collectionType.isInterface()) { - if (Set.class == collectionType || Collection.class == collectionType) { - return new LinkedHashSet<>(capacity); - } - else if (List.class == collectionType) { - return new ArrayList<>(capacity); - } - else if (SortedSet.class == collectionType || NavigableSet.class == collectionType) { - return new TreeSet<>(); - } - else { - throw new IllegalArgumentException("Unsupported Collection interface: " + collectionType.getName()); - } + if (LinkedHashSet.class == collectionType || HashSet.class == collectionType || + Set.class == collectionType || Collection.class == collectionType) { + return new LinkedHashSet<>(capacity); + } + else if (ArrayList.class == collectionType || List.class == collectionType) { + return new ArrayList<>(capacity); + } + else if (LinkedList.class == collectionType) { + return new LinkedList<>(); + } + else if (SortedSet.class == collectionType || NavigableSet.class == collectionType) { + return new TreeSet<>(); } else if (EnumSet.class.isAssignableFrom(collectionType)) { Assert.notNull(elementType, "Cannot create EnumSet for unknown element type"); @@ -201,7 +200,7 @@ public final class CollectionFactory { return (Collection) EnumSet.noneOf(asEnumType(elementType)); } else { - if (!Collection.class.isAssignableFrom(collectionType)) { + if (collectionType.isInterface() || !Collection.class.isAssignableFrom(collectionType)) { throw new IllegalArgumentException("Unsupported Collection type: " + collectionType.getName()); } try { From de1b938e2ed95104c532afc708e4accca7461e76 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:10:00 +0200 Subject: [PATCH 4/6] Improve diagnostics for CGLIB ClassLoader mismatch with --add-opens hint Closes gh-28747 --- .../org/springframework/cglib/core/ReflectUtils.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java b/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java index b765ce12c0f..9467b935507 100644 --- a/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java +++ b/spring-core/src/main/java/org/springframework/cglib/core/ReflectUtils.java @@ -490,7 +490,7 @@ public class ReflectUtils { return defineClass(className, b, loader, protectionDomain, null); } - @SuppressWarnings("deprecation") // on JDK 9 + @SuppressWarnings({"deprecation", "serial"}) public static Class defineClass(String className, byte[] b, ClassLoader loader, ProtectionDomain protectionDomain, Class contextClass) throws Exception { @@ -578,6 +578,16 @@ public class ReflectUtils { catch (InvocationTargetException ex) { throw new CodeGenerationException(ex.getTargetException()); } + catch (IllegalAccessException ex) { + throw new CodeGenerationException(ex) { + @Override + public String getMessage() { + return "ClassLoader mismatch for [" + contextClass.getName() + + "]: JVM should be started with --add-opens=java.base/java.lang=ALL-UNNAMED " + + "for ClassLoader.defineClass to be accessible on " + loader.getClass().getName(); + } + }; + } catch (Throwable ex) { throw new CodeGenerationException(ex); } From a3e46a2db7473f8e2f0fb6a29fd9e7bf4f631b5b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:10:35 +0200 Subject: [PATCH 5/6] ResolvableType.forInstance returns NONE for null instance Closes gh-28776 --- .../org/springframework/core/ResolvableType.java | 15 +++++++-------- .../springframework/core/ResolvableTypeTests.java | 8 +++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/ResolvableType.java b/spring-core/src/main/java/org/springframework/core/ResolvableType.java index 6ba088cfd1e..c1cc47261b8 100644 --- a/spring-core/src/main/java/org/springframework/core/ResolvableType.java +++ b/spring-core/src/main/java/org/springframework/core/ResolvableType.java @@ -221,9 +221,8 @@ public class ResolvableType implements Serializable { /** * Return the underlying source of the resolvable type. Will return a {@link Field}, * {@link MethodParameter} or {@link Type} depending on how the {@link ResolvableType} - * was constructed. With the exception of the {@link #NONE} constant, this method will - * never return {@code null}. This method is primarily to provide access to additional - * type information or meta-data that alternative JVM languages may provide. + * was constructed. This method is primarily to provide access to additional type + * information or meta-data that alternative JVM languages may provide. */ public Object getSource() { Object source = (this.typeProvider != null ? this.typeProvider.getSource() : null); @@ -1103,20 +1102,20 @@ public class ResolvableType implements Serializable { * convey generic information but if it implements {@link ResolvableTypeProvider} a * more precise {@link ResolvableType} can be used than the simple one based on * the {@link #forClass(Class) Class instance}. - * @param instance the instance - * @return a {@link ResolvableType} for the specified instance + * @param instance the instance (possibly {@code null}) + * @return a {@link ResolvableType} for the specified instance, + * or {@code NONE} for {@code null} * @since 4.2 * @see ResolvableTypeProvider */ - public static ResolvableType forInstance(Object instance) { - Assert.notNull(instance, "Instance must not be null"); + public static ResolvableType forInstance(@Nullable Object instance) { if (instance instanceof ResolvableTypeProvider) { ResolvableType type = ((ResolvableTypeProvider) instance).getResolvableType(); if (type != null) { return type; } } - return ResolvableType.forClass(instance.getClass()); + return (instance != null ? forClass(instance.getClass()) : NONE); } /** diff --git a/spring-core/src/test/java/org/springframework/core/ResolvableTypeTests.java b/spring-core/src/test/java/org/springframework/core/ResolvableTypeTests.java index 71b17e0da02..48641a7c70c 100644 --- a/spring-core/src/test/java/org/springframework/core/ResolvableTypeTests.java +++ b/spring-core/src/test/java/org/springframework/core/ResolvableTypeTests.java @@ -146,11 +146,9 @@ class ResolvableTypeTests { assertThat(typeVariable.isAssignableFrom(raw)).isTrue(); } - @Test - void forInstanceMustNotBeNull() throws Exception { - assertThatIllegalArgumentException() - .isThrownBy(() -> ResolvableType.forInstance(null)) - .withMessage("Instance must not be null"); + @Test // gh-28776 + void forInstanceNull() throws Exception { + assertThat(ResolvableType.forInstance(null)).isEqualTo(ResolvableType.NONE); } @Test From 3c3ae32f0793931ce32614c8ff3508238f07632c Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 13 Jul 2022 11:11:17 +0200 Subject: [PATCH 6/6] Upgrade to Netty 4.1.79, Jetty 9.4.48, Undertow 2.2.18, Checkstyle 10.3.1 --- build.gradle | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/build.gradle b/build.gradle index 22552391927..b6037c2c184 100644 --- a/build.gradle +++ b/build.gradle @@ -28,11 +28,11 @@ configure(allprojects) { project -> dependencyManagement { imports { mavenBom "com.fasterxml.jackson:jackson-bom:2.12.7" - mavenBom "io.netty:netty-bom:4.1.77.Final" + mavenBom "io.netty:netty-bom:4.1.79.Final" mavenBom "io.projectreactor:reactor-bom:2020.0.21" mavenBom "io.r2dbc:r2dbc-bom:Arabba-SR13" mavenBom "io.rsocket:rsocket-bom:1.1.2" - mavenBom "org.eclipse.jetty:jetty-bom:9.4.46.v20220331" + mavenBom "org.eclipse.jetty:jetty-bom:9.4.48.v20220622" mavenBom "org.jetbrains.kotlin:kotlin-bom:1.5.32" mavenBom "org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.5.2" mavenBom "org.jetbrains.kotlinx:kotlinx-serialization-bom:1.2.2" @@ -139,7 +139,7 @@ configure(allprojects) { project -> entry 'tomcat-embed-core' entry 'tomcat-embed-websocket' } - dependencySet(group: 'io.undertow', version: '2.2.17.Final') { + dependencySet(group: 'io.undertow', version: '2.2.18.Final') { entry 'undertow-core' entry('undertow-servlet') { exclude group: "org.jboss.spec.javax.servlet", name: "jboss-servlet-api_4.0_spec" @@ -340,7 +340,7 @@ configure([rootProject] + javaProjects) { project -> } checkstyle { - toolVersion = "10.3" + toolVersion = "10.3.1" configDirectory.set(rootProject.file("src/checkstyle")) }