diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java index 24f3f5b9d..2a3b12c07 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/adaptor/impl/ResultSetAdaptor.java @@ -28,9 +28,9 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.util.Calendar; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; -import java.util.Objects; import java.util.function.Function; import org.hibernate.engine.jdbc.BlobProxy; @@ -42,6 +42,9 @@ import io.vertx.sqlclient.RowSet; import io.vertx.sqlclient.desc.ColumnDescriptor; +import static java.util.Collections.emptyList; +import static java.util.Objects.requireNonNull; + /** * An adaptor that allows Hibernate core code which expects a JDBC * {@code ResultSet} to read values from Vert.x's {@code RowSet}. @@ -54,6 +57,7 @@ public class ResultSetAdaptor implements ResultSet { private boolean wasNull; public ResultSetAdaptor(RowSet rows) { + requireNonNull( rows ); this.iterator = rows.iterator(); this.rows = rows; } @@ -199,15 +203,26 @@ public String getString(String columnLabel) { } private T caseInsensitiveGet(String columnLabel, Function produce) { - for ( String columnName : rows.columnsNames() ) { + for ( String columnName : getColumnsNames() ) { if ( columnName.equalsIgnoreCase( columnLabel ) ) { return produce.apply( columnName ); } } + // Same error thrown by io.vertx.sqlclient.Row when it doesn't find the label throw new NoSuchElementException( "Column " + columnLabel + " does not exist" ); } + /** + * rows.columnsNames() might return null for some databases. + * For example, when creating a stored procedure in PostgreSQL using a native query. + * + * @return A list of column names or an empty list. + */ + private List getColumnsNames() { + return rows.columnsNames() == null ? emptyList() : rows.columnsNames(); + } + @Override public boolean getBoolean(String columnLabel) { try { @@ -368,7 +383,7 @@ public ResultSetMetaData getMetaData() { return new ResultSetMetaData() { @Override public int getColumnCount() { - return rows.columnsNames().size(); + return getColumnsNames() == null ? 0 : getColumnsNames().size(); } @Override @@ -379,12 +394,12 @@ public int getColumnType(int column) { @Override public String getColumnLabel(int column) { - return rows.columnsNames().get( column - 1 ); + return getColumnsNames().get( column - 1 ); } @Override public String getColumnName(int column) { - return rows.columnsNames().get( column - 1 ); + return getColumnsNames().get( column - 1 ); } @Override @@ -502,7 +517,7 @@ public Object getObject(String columnLabel) { public int findColumn(String columnLabel) { // JDBC parameters index start from 1 int index = 1; - for ( String column : rows.columnsNames() ) { + for ( String column : getColumnsNames() ) { // Some dbs, like Oracle and Db2, return the column names always in uppercase if ( column.equalsIgnoreCase( columnLabel ) ) { return index; @@ -865,7 +880,7 @@ private static class RowIdAdaptor implements RowId { private final Buffer buffer; private RowIdAdaptor(Buffer buffer) { - Objects.requireNonNull( buffer ); + requireNonNull( buffer ); this.buffer = buffer; } diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java index 1d1324652..72760a26b 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java @@ -255,6 +255,9 @@ public interface Log extends BasicLogger { @Message(id = 79, value = "The configuration property '%1$s' was not provided, or is in invalid format. This is required when using the default DefaultSqlClientPool: either provide the configuration setting or integrate with a different SqlClientPool implementation") HibernateException blankConnectionString(String property); + @Message(id = 80, value = "No results were returned by the query (you can try running it with '.executeUpdate()'): %1$s") + HibernateException noResultException(String sql); + // Same method that exists in CoreMessageLogger @LogMessage(level = WARN) @Message(id = 104, value = "firstResult/maxResults specified with collection fetch; applying in memory!" ) diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java index 88fd2346e..0ae26c795 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/sql/results/internal/ReactiveDeferredResultSetAccess.java @@ -10,6 +10,7 @@ import java.sql.ResultSet; import java.sql.ResultSetMetaData; import java.sql.SQLException; +import java.util.concurrent.CompletionException; import java.util.concurrent.CompletionStage; import java.util.function.Function; @@ -24,6 +25,7 @@ import org.hibernate.reactive.pool.ReactiveConnection; import org.hibernate.reactive.pool.impl.Parameters; import org.hibernate.reactive.session.ReactiveConnectionSupplier; +import org.hibernate.reactive.util.impl.CompletionStages; import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcOperationQuerySelect; @@ -41,7 +43,6 @@ public class ReactiveDeferredResultSetAccess extends DeferredResultSetAccess imp private static final Log LOG = LoggerFactory.make( Log.class, MethodHandles.lookup() ); private final SqlStatementLogger sqlStatementLogger; - private final Function statementCreator; private final ExecutionContext executionContext; @@ -59,7 +60,6 @@ public ReactiveDeferredResultSetAccess( super( jdbcSelect, jdbcParameterBindings, executionContext, statementCreator ); this.executionContext = executionContext; this.sqlStatementLogger = executionContext.getSession().getJdbcServices().getSqlStatementLogger(); - this.statementCreator = statementCreator; } @Override @@ -159,17 +159,33 @@ private CompletionStage executeQuery() { eventListenerManager.jdbcExecuteStatementStart(); return connection() .selectJdbc( sql, parameters ) + .thenCompose( this::validateResultSet ) .whenComplete( (resultSet, throwable) -> { // FIXME: I don't know if this event makes sense for Vert.x eventListenerManager.jdbcExecuteStatementEnd(); sqlStatementLogger.logSlowQuery( getFinalSql(), executeStartNanos ); } ) .thenCompose( this::reactiveSkipRows ) - .handle( this::convertException ); + .handle( CompletionStages::handle ) + .thenCompose( handler -> handler.hasFailed() + ? convertException( resultSet, handler.getThrowable() ) + : handler.getResultAsCompletionStage() + ); } ) .whenComplete( (o, throwable) -> logicalConnection.afterStatement() ); } + private CompletionStage validateResultSet(ResultSet resultSet) { + try { + return resultSet.getMetaData().getColumnCount() == 0 + ? failedFuture( LOG.noResultException( getFinalSql() ) ) + : completedFuture( resultSet ); + } + catch (SQLException e) { + throw new RuntimeException( e ); + } + } + private ResultSet saveResultSet(ResultSet resultSet) { this.resultSet = resultSet; return saveColumnCount( resultSet ); @@ -185,18 +201,29 @@ private ReactiveConnection connection() { return ( (ReactiveConnectionSupplier) executionContext.getSession() ).getReactiveConnection(); } - private ResultSet convertException(ResultSet resultSet, Throwable throwable) { - // FIXME: Vert.x will probably throw another exception. Check this. - if ( throwable instanceof SQLException) { - throw executionContext.getSession().getJdbcServices() - .getSqlExceptionHelper() - // FIXME: Add this to the logger? - .convert( (SQLException) throwable, "Exception executing SQL [" + getFinalSql() + "]" ); - } + private CompletionStage convertException(T object, Throwable throwable) { if ( throwable != null ) { - throw new HibernateException( throwable ); + Throwable cause = throwable; + if ( throwable instanceof CompletionException ) { + cause = throwable.getCause(); + } + // I doubt this is ever going to happen because Vert.x is not going to throw an SQLException + if ( cause instanceof SQLException ) { + return failedFuture( executionContext + .getSession().getJdbcServices() + .getSqlExceptionHelper() + .convert( + (SQLException) cause, + "Exception executing SQL [" + getFinalSql() + "]" + ) + ); + } + if ( cause instanceof HibernateException ) { + return failedFuture( cause ); + } + return failedFuture( new HibernateException( cause ) ); } - return resultSet; + return completedFuture( object ); } private CompletionStage reactiveSkipRows(ResultSet resultSet) { diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomOneToOneStoredProcedureSqlTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomOneToOneStoredProcedureSqlTest.java index ef542e8a3..451c95fee 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomOneToOneStoredProcedureSqlTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomOneToOneStoredProcedureSqlTest.java @@ -9,9 +9,11 @@ import io.vertx.junit5.VertxTestContext; import jakarta.persistence.*; +import org.hibernate.HibernateException; import org.hibernate.annotations.SQLDelete; import org.hibernate.annotations.SQLInsert; import org.hibernate.reactive.testing.DBSelectionExtension; +import org.hibernate.reactive.testing.ReactiveAssertions; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -23,6 +25,7 @@ import java.util.List; import static java.util.concurrent.TimeUnit.*; +import static org.assertj.core.api.Assertions.assertThat; import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.*; import static org.hibernate.reactive.testing.DBSelectionExtension.*; import static org.junit.jupiter.api.Assertions.*; @@ -84,6 +87,22 @@ public void populateDb(VertxTestContext context) { ); } + @Test + public void testFailureWithGetSingleResultOrNull(VertxTestContext context) { + test( context, ReactiveAssertions.assertThrown( HibernateException.class, getMutinySessionFactory() + .withTransaction( s -> s.createNativeQuery( INSERT_DRIVER_LICENCE_SQL ).getSingleResultOrNull() ) ) + .invoke( e -> assertThat( e ).hasMessageContainingAll( "HR000080:", INSERT_DRIVER_LICENCE_SQL ) ) + ); + } + + @Test + public void testFailureWithGetSingleResult(VertxTestContext context) { + test( context, ReactiveAssertions.assertThrown( HibernateException.class, getSessionFactory() + .withTransaction( s -> s.createNativeQuery( INSERT_DRIVER_LICENCE_SQL ).getSingleResult() ) ) + .thenAccept( e -> assertThat( e ).hasMessageContainingAll( "HR000080:", INSERT_DRIVER_LICENCE_SQL ) ) + ); + } + @Test public void testInsertStoredProcedureDriverLicence(VertxTestContext context) { test( context, openSession().thenCompose( session -> session diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomStoredProcedureSqlTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomStoredProcedureSqlTest.java index ac002b9db..512b23dda 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomStoredProcedureSqlTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/CustomStoredProcedureSqlTest.java @@ -9,10 +9,12 @@ import java.util.Collection; import java.util.List; +import org.hibernate.HibernateException; import org.hibernate.annotations.SQLDelete; import org.hibernate.annotations.SQLInsert; import org.hibernate.annotations.SQLUpdate; import org.hibernate.reactive.testing.DBSelectionExtension; +import org.hibernate.reactive.testing.ReactiveAssertions; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; @@ -29,6 +31,7 @@ import jakarta.persistence.Table; import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.Assertions.assertThat; import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.POSTGRESQL; import static org.hibernate.reactive.testing.DBSelectionExtension.runOnlyFor; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -102,6 +105,22 @@ public void populateDb(VertxTestContext context) { ); } + @Test + public void testFailureWithGetSingleResultOrNull(VertxTestContext context) { + test( context, ReactiveAssertions.assertThrown( HibernateException.class, getMutinySessionFactory() + .withTransaction( s -> s.createNativeQuery( INSERT_SP_SQL ).getSingleResultOrNull() ) ) + .invoke( e -> assertThat( e ).hasMessageContainingAll( "HR000080:", INSERT_SP_SQL ) ) + ); + } + + @Test + public void testFailureWithGetSingleResult(VertxTestContext context) { + test( context, ReactiveAssertions.assertThrown( HibernateException.class, getSessionFactory() + .withTransaction( s -> s.createNativeQuery( INSERT_SP_SQL ).getSingleResult() ) ) + .thenAccept( e -> assertThat( e ).hasMessageContainingAll( "HR000080:", INSERT_SP_SQL ) ) + ); + } + @Test public void testInsertStoredProcedure(VertxTestContext context) { test( context, openSession().thenCompose( session -> session