From e15bd3162c5e1e151ab80ce6af6bd252fea68e38 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 15 Jul 2019 14:00:26 +0200 Subject: [PATCH] #124 - Polishing. Tighten nullability constraints. Consistent Javadoc. --- .../ConnectionFactoryUtils.java | 101 ++++++++---------- .../connectionfactory/ConnectionHolder.java | 5 +- .../DelegatingConnectionFactory.java | 2 +- .../R2dbcTransactionManager.java | 2 +- .../lookup/MapConnectionFactoryLookup.java | 2 +- .../data/r2dbc/core/DatabaseClient.java | 3 +- .../data/r2dbc/core/SqlProvider.java | 3 +- .../data/r2dbc/dialect/BindTarget.java | 4 +- .../data/r2dbc/dialect/Bindings.java | 4 +- .../data/r2dbc/dialect/MutableBindings.java | 6 +- 10 files changed, 61 insertions(+), 71 deletions(-) diff --git a/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionFactoryUtils.java b/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionFactoryUtils.java index 9533b523..ee1a6df0 100644 --- a/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionFactoryUtils.java +++ b/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionFactoryUtils.java @@ -18,8 +18,6 @@ package org.springframework.data.r2dbc.connectionfactory; import io.r2dbc.spi.Connection; import io.r2dbc.spi.ConnectionFactory; import reactor.core.publisher.Mono; -import reactor.util.function.Tuple2; -import reactor.util.function.Tuples; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -61,9 +59,10 @@ public abstract class ConnectionFactoryUtils { * Is aware of a corresponding Connection bound to the current {@link reactor.util.context.Context}. Will bind a * Connection to the {@link reactor.util.context.Context} if transaction synchronization is active. * - * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to obtain Connections from + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to obtain {@link io.r2dbc.spi.Connection + * Connections} from. * @return a R2DBC Connection from the given {@link io.r2dbc.spi.ConnectionFactory}. - * @throws DataAccessResourceFailureException if the attempt to get a {@link io.r2dbc.spi.Connection} failed + * @throws DataAccessResourceFailureException if the attempt to get a {@link io.r2dbc.spi.Connection} failed. * @see #releaseConnection */ public static Mono getConnection(ConnectionFactory connectionFactory) { @@ -72,14 +71,14 @@ public abstract class ConnectionFactoryUtils { } /** - * Actually obtain a R2DBC Connection from the given {@link ConnectionFactory}. Same as {@link #getConnection}, but - * preserving the original exceptions. + * Actually obtain a R2DBC Connection from the given {@link io.r2dbc.spi.ConnectionFactory}. Same as + * {@link #getConnection}, but preserving the original exceptions. *

* Is aware of a corresponding Connection bound to the current {@link reactor.util.context.Context}. Will bind a * Connection to the {@link reactor.util.context.Context} if transaction synchronization is active. * - * @param connectionFactory the {@link ConnectionFactory} to obtain Connections from. - * @return a R2DBC {@link io.r2dbc.spi.Connection} from the given {@link ConnectionFactory}. + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to obtain Connections from. + * @return a R2DBC {@link io.r2dbc.spi.Connection} from the given {@link io.r2dbc.spi.ConnectionFactory}. */ public static Mono doGetConnection(ConnectionFactory connectionFactory) { @@ -143,12 +142,14 @@ public abstract class ConnectionFactoryUtils { } /** - * Actually fetch a {@link Connection} from the given {@link ConnectionFactory}, defensively turning an unexpected - * {@code null} return value from {@link ConnectionFactory#create()} into an {@link IllegalStateException}. + * Actually fetch a {@link io.r2dbc.spi.Connection} from the given {@link io.r2dbc.spi.ConnectionFactory}, defensively + * turning an unexpected {@literal null} return value from {@link io.r2dbc.spi.ConnectionFactory#create()} into an + * {@link IllegalStateException}. * - * @param connectionFactory the {@link ConnectionFactory} to obtain {@link Connection}s from - * @return a R2DBC {@link Connection} from the given {@link ConnectionFactory} (never {@code null}) - * @throws IllegalStateException if the {@link ConnectionFactory} returned a {@literal null} value. + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to obtain {@link io.r2dbc.spi.Connection}s from + * @return a R2DBC {@link io.r2dbc.spi.Connection} from the given {@link io.r2dbc.spi.ConnectionFactory} (never + * {@literal null}). + * @throws IllegalStateException if the {@link io.r2dbc.spi.ConnectionFactory} returned a {@literal null} value. * @see ConnectionFactory#create() */ private static Mono fetchConnection(ConnectionFactory connectionFactory) { @@ -156,32 +157,29 @@ public abstract class ConnectionFactoryUtils { } /** - * Close the given {@link io.r2dbc.spi.Connection}, obtained from the given {@link ConnectionFactory}, if it is not - * managed externally (that is, not bound to the thread). + * Close the given {@link io.r2dbc.spi.Connection}, obtained from the given {@link io.r2dbc.spi.ConnectionFactory}, if + * it is not managed externally (that is, not bound to the thread). * * @param con the {@link io.r2dbc.spi.Connection} to close if necessary. - * @param connectionFactory the {@link ConnectionFactory} that the Connection was obtained from (may be - * {@literal null}). + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} that the Connection was obtained from. * @see #getConnection */ - public static Mono releaseConnection(@Nullable io.r2dbc.spi.Connection con, - @Nullable ConnectionFactory connectionFactory) { + public static Mono releaseConnection(io.r2dbc.spi.Connection con, ConnectionFactory connectionFactory) { return doReleaseConnection(con, connectionFactory) .onErrorMap(e -> new DataAccessResourceFailureException("Failed to close R2DBC Connection", e)); } /** - * Actually close the given {@link io.r2dbc.spi.Connection}, obtained from the given {@link ConnectionFactory}. Same - * as {@link #releaseConnection}, but preserving the original exception. + * Actually close the given {@link io.r2dbc.spi.Connection}, obtained from the given + * {@link io.r2dbc.spi.ConnectionFactory}. Same as {@link #releaseConnection}, but preserving the original exception. * * @param connection the {@link io.r2dbc.spi.Connection} to close if necessary. - * @param connectionFactory the {@link ConnectionFactory} that the Connection was obtained from (may be - * {@literal null}). + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} that the Connection was obtained from. * @see #doGetConnection */ - public static Mono doReleaseConnection(@Nullable io.r2dbc.spi.Connection connection, - @Nullable ConnectionFactory connectionFactory) { + public static Mono doReleaseConnection(io.r2dbc.spi.Connection connection, + ConnectionFactory connectionFactory) { return TransactionSynchronizationManager.forCurrentTransaction().flatMap(it -> { @@ -200,12 +198,17 @@ public abstract class ConnectionFactoryUtils { * Close the {@link io.r2dbc.spi.Connection}. Translates exceptions into the Spring hierarchy of unchecked generic * data access exceptions, simplifying calling code and making any exception that is thrown more meaningful. * - * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to obtain Connections from + * @param connection the {@link io.r2dbc.spi.Connection} to close. + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} that the {@link io.r2dbc.spi.Connection} was + * obtained from. * @return a R2DBC Connection from the given {@link io.r2dbc.spi.ConnectionFactory}. * @throws DataAccessResourceFailureException if the attempt to get a {@link io.r2dbc.spi.Connection} failed */ public static Mono closeConnection(Connection connection, ConnectionFactory connectionFactory) { + Assert.notNull(connection, "Connection must not be null!"); + Assert.notNull(connectionFactory, "ConnectionFactory must not be null!"); + return doCloseConnection(connection, connectionFactory) .onErrorMap(e -> new DataAccessResourceFailureException("Failed to obtain R2DBC Connection", e)); } @@ -214,7 +217,7 @@ public abstract class ConnectionFactoryUtils { * Close the {@link io.r2dbc.spi.Connection}, unless a {@link SmartConnectionFactory} doesn't want us to. * * @param connection the {@link io.r2dbc.spi.Connection} to close if necessary. - * @param connectionFactory the {@link ConnectionFactory} that the Connection was obtained from. + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} that the Connection was obtained from. * @see Connection#close() * @see SmartConnectionFactory#shouldClose(Connection) */ @@ -236,6 +239,7 @@ public abstract class ConnectionFactoryUtils { /** * Obtain the {@link io.r2dbc.spi.ConnectionFactory} from the current subscriber {@link reactor.util.context.Context}. * + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} that the Connection was obtained from. * @see TransactionSynchronizationManager */ public static Mono currentConnectionFactory(ConnectionFactory connectionFactory) { @@ -252,12 +256,13 @@ public abstract class ConnectionFactoryUtils { } /** - * Determine whether the given two {@link Connection}s are equal, asking the target {@link Connection} in case of a - * proxy. Used to detect equality even if the user passed in a raw target Connection while the held one is a proxy. + * Determine whether the given two {@link io.r2dbc.spi.Connection}s are equal, asking the target + * {@link io.r2dbc.spi.Connection} in case of a proxy. Used to detect equality even if the user passed in a raw target + * Connection while the held one is a proxy. * - * @param conHolder the {@link ConnectionHolder} for the held Connection (potentially a proxy) - * @param passedInCon the {@link Connection} passed-in by the user (potentially a target {@link Connection} without - * proxy) + * @param conHolder the {@link .ConnectionHolder} for the held {@link io.r2dbc.spi.Connection} (potentially a proxy). + * @param passedInCon the {@link io.r2dbc.spi.Connection} passed-in by the user (potentially a target + * {@link io.r2dbc.spi.Connection} without proxy). * @return whether the given Connections are equal * @see #getTargetConnection */ @@ -273,11 +278,11 @@ public abstract class ConnectionFactoryUtils { } /** - * Return the innermost target {@link Connection} of the given {@link Connection}. If the given {@link Connection} is - * a proxy, it will be unwrapped until a non-proxy {@link Connection} is found. Otherwise, the passed-in Connection - * will be returned as-is. + * Return the innermost target {@link io.r2dbc.spi.Connection} of the given {@link io.r2dbc.spi.Connection}. If the + * given {@link io.r2dbc.spi.Connection} is a proxy, it will be unwrapped until a non-proxy + * {@link io.r2dbc.spi.Connection} is found. Otherwise, the passed-in Connection will be returned as-is. * - * @param con the {@link Connection} proxy to unwrap + * @param con the {@link io.r2dbc.spi.Connection} proxy to unwrap * @return the innermost target Connection, or the passed-in one if no proxy * @see ConnectionProxy#getTargetConnection() */ @@ -291,11 +296,11 @@ public abstract class ConnectionFactoryUtils { } /** - * Determine the connection synchronization order to use for the given {@link ConnectionFactory}. Decreased for every - * level of nesting that a {@link ConnectionFactory} has, checked through the level of - * {@link DelegatingConnectionFactory} nesting. + * Determine the connection synchronization order to use for the given {@link io.r2dbc.spi.ConnectionFactory}. + * Decreased for every level of nesting that a {@link io.r2dbc.spi.ConnectionFactory} has, checked through the level + * of {@link DelegatingConnectionFactory} nesting. * - * @param connectionFactory the {@link ConnectionFactory} to check. + * @param connectionFactory the {@link io.r2dbc.spi.ConnectionFactory} to check. * @return the connection synchronization order to use. * @see #CONNECTION_SYNCHRONIZATION_ORDER */ @@ -310,22 +315,6 @@ public abstract class ConnectionFactoryUtils { return order; } - /** - * Create a {@link Connection} via the given {@link ConnectionFactory#create() factory} and return a {@link Tuple2} - * associating the {@link Connection} with its creating {@link ConnectionFactory}. - * - * @param factory must not be {@literal null}. - * @return never {@literal null} - */ - private static Mono> createConnection(ConnectionFactory factory) { - - if (logger.isDebugEnabled()) { - logger.debug("Fetching resumed R2DBC Connection from ConnectionFactory"); - } - - return Mono.from(factory.create()).map(connection -> Tuples.of(connection, factory)); - } - /** * Callback for resource cleanup at the end of a non-native R2DBC transaction. */ diff --git a/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionHolder.java b/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionHolder.java index 1db09c4a..1f82ea27 100644 --- a/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionHolder.java +++ b/src/main/java/org/springframework/data/r2dbc/connectionfactory/ConnectionHolder.java @@ -100,9 +100,10 @@ public class ConnectionHolder extends ResourceHolderSupport { } /** - * Override the existing Connection handle with the given {@link Connection}. Reset the handle if given {@code null}. + * Override the existing Connection handle with the given {@link Connection}. Reset the handle if given + * {@literal null}. *

- * Used for releasing the {@link Connection} on suspend (with a {@code null} argument) and setting a fresh + * Used for releasing the {@link Connection} on suspend (with a {@literal null} argument) and setting a fresh * {@link Connection} on resume. */ protected void setConnection(@Nullable Connection connection) { diff --git a/src/main/java/org/springframework/data/r2dbc/connectionfactory/DelegatingConnectionFactory.java b/src/main/java/org/springframework/data/r2dbc/connectionfactory/DelegatingConnectionFactory.java index 692ce10f..eba926c5 100644 --- a/src/main/java/org/springframework/data/r2dbc/connectionfactory/DelegatingConnectionFactory.java +++ b/src/main/java/org/springframework/data/r2dbc/connectionfactory/DelegatingConnectionFactory.java @@ -78,7 +78,7 @@ public class DelegatingConnectionFactory implements ConnectionFactory, Wrapped - * If the supplied {@link Map} is {@code null}, then this method call effectively has no effect. + * If the supplied {@link Map} is {@literal null}, then this method call effectively has no effect. * * @param connectionFactories said {@link Map} of {@link ConnectionFactory connectionFactories} */ diff --git a/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java b/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java index ff17135a..1ae343fb 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java +++ b/src/main/java/org/springframework/data/r2dbc/core/DatabaseClient.java @@ -53,7 +53,8 @@ public interface DatabaseClient { * * @param sql must not be {@literal null} or empty. * @return a new {@link GenericExecuteSpec}. - * @see NamedParameterExpander * @see DatabaseClient.Builder#namedParameters(boolean) + * @see NamedParameterExpander + * @see DatabaseClient.Builder#namedParameters(boolean) */ GenericExecuteSpec execute(String sql); diff --git a/src/main/java/org/springframework/data/r2dbc/core/SqlProvider.java b/src/main/java/org/springframework/data/r2dbc/core/SqlProvider.java index 6a04e381..7bd47898 100644 --- a/src/main/java/org/springframework/data/r2dbc/core/SqlProvider.java +++ b/src/main/java/org/springframework/data/r2dbc/core/SqlProvider.java @@ -31,9 +31,8 @@ public interface SqlProvider { /** * Return the SQL string for this object, i.e. typically the SQL used for creating statements. * - * @return the SQL string, or {@code null}. + * @return the SQL string, or {@literal null}. */ @Nullable String getSql(); - } diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/BindTarget.java b/src/main/java/org/springframework/data/r2dbc/dialect/BindTarget.java index 07ea885b..b8aeb708 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/BindTarget.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/BindTarget.java @@ -44,7 +44,7 @@ public interface BindTarget { void bind(int index, Object value); /** - * Bind a {@code null} value. + * Bind a {@literal null} value. * * @param identifier the identifier to bind to. * @param type the type of {@literal null} value. @@ -52,7 +52,7 @@ public interface BindTarget { void bindNull(Object identifier, Class type); /** - * Bind a {@code null} value. + * Bind a {@literal null} value. * * @param index the index to bind to. * @param type the type of {@literal null} value. diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/Bindings.java b/src/main/java/org/springframework/data/r2dbc/dialect/Bindings.java index fce5fb76..3e1d0f79 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/Bindings.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/Bindings.java @@ -32,8 +32,8 @@ import org.springframework.lang.Nullable; import org.springframework.util.Assert; /** - * Value object representing value and {@code null} bindings for a {@link Statement} using {@link BindMarkers}. Bindings - * are typically immutable. + * Value object representing value and {@literal null} bindings for a {@link Statement} using {@link BindMarkers}. + * Bindings are typically immutable. * * @author Mark Paluch */ diff --git a/src/main/java/org/springframework/data/r2dbc/dialect/MutableBindings.java b/src/main/java/org/springframework/data/r2dbc/dialect/MutableBindings.java index dd2abda5..303810e5 100644 --- a/src/main/java/org/springframework/data/r2dbc/dialect/MutableBindings.java +++ b/src/main/java/org/springframework/data/r2dbc/dialect/MutableBindings.java @@ -22,7 +22,7 @@ import java.util.LinkedHashMap; import org.springframework.util.Assert; /** - * Mutable extension to {@link Bindings} for Value and {@code null} bindings for a {@link Statement} using + * Mutable extension to {@link Bindings} for Value and {@literal null} bindings for a {@link Statement} using * {@link BindMarkers}. * * @author Mark Paluch @@ -51,7 +51,7 @@ public class MutableBindings extends Bindings { * @return the next {@link BindMarker}. */ public BindMarker nextMarker() { - return markers.next(); + return this.markers.next(); } /** @@ -61,7 +61,7 @@ public class MutableBindings extends Bindings { * @return the next {@link BindMarker}. */ public BindMarker nextMarker(String hint) { - return markers.next(hint); + return this.markers.next(hint); } /**