From a5783d0943c4fa072f634b0060fbfc55d6881816 Mon Sep 17 00:00:00 2001 From: Samrat Dhillon Date: Sun, 22 Sep 2019 21:17:16 -0400 Subject: [PATCH] Fix incorrect exception throw by spring-jdbc closes gh-23675 --- .../SQLErrorCodeSQLExceptionTranslator.java | 9 ++++ .../jdbc/support/SQLErrorCodesFactory.java | 6 +++ ...LErrorCodeSQLExceptionTranslatorTests.java | 49 +++++++++++++++++++ .../support/SQLErrorCodesFactoryTests.java | 16 +++++- 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslator.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslator.java index 2493f0b30949..467b88a16cf1 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslator.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslator.java @@ -78,6 +78,10 @@ public class SQLErrorCodeSQLExceptionTranslator extends AbstractFallbackSQLExcep @Nullable private SQLErrorCodes sqlErrorCodes; + /** Reference to the dataSource if SQLErrorCodeSQLExceptionTranslator is initialized via new SQLErrorCodeSQLExceptionTranslator(DataSource dataSource).**/ + @Nullable + private DataSource dataSource; + /** * Constructor for use as a JavaBean. @@ -97,6 +101,7 @@ public SQLErrorCodeSQLExceptionTranslator() { */ public SQLErrorCodeSQLExceptionTranslator(DataSource dataSource) { this(); + this.dataSource = dataSource; setDataSource(dataSource); } @@ -186,6 +191,10 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL return dae; } + if(this.sqlErrorCodes == null && this.dataSource != null) { + this.sqlErrorCodes = SQLErrorCodesFactory.getInstance().getErrorCodes(this.dataSource); + } + // Next, try the custom SQLException translator, if available. if (this.sqlErrorCodes != null) { SQLExceptionTranslator customTranslator = this.sqlErrorCodes.getCustomSqlExceptionTranslator(); diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodesFactory.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodesFactory.java index 74a9f1e6f851..a945f515fc5c 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodesFactory.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/support/SQLErrorCodesFactory.java @@ -29,6 +29,7 @@ import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; +import org.springframework.jdbc.CannotGetJdbcConnectionException; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ConcurrentReferenceHashMap; @@ -219,6 +220,11 @@ public SQLErrorCodes getErrorCodes(DataSource dataSource) { } catch (MetaDataAccessException ex) { logger.warn("Error while extracting database name - falling back to empty error codes", ex); + //returning null error codes will make sure that next time error codes are looked up, + //Spring will try to load the error code again + if(ex.getCause() != null && ex.getCause() instanceof CannotGetJdbcConnectionException) { + return null; + } } // Fallback is to return an empty SQLErrorCodes instance. return new SQLErrorCodes(); diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslatorTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslatorTests.java index cef07433d312..5c8385109137 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslatorTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodeSQLExceptionTranslatorTests.java @@ -17,9 +17,13 @@ package org.springframework.jdbc.support; import java.sql.BatchUpdateException; +import java.sql.Connection; import java.sql.DataTruncation; +import java.sql.DatabaseMetaData; import java.sql.SQLException; +import javax.sql.DataSource; + import org.junit.jupiter.api.Test; import org.springframework.dao.CannotAcquireLockException; @@ -30,11 +34,15 @@ import org.springframework.dao.DeadlockLoserDataAccessException; import org.springframework.dao.DuplicateKeyException; import org.springframework.jdbc.BadSqlGrammarException; +import org.springframework.jdbc.CannotGetJdbcConnectionException; import org.springframework.jdbc.InvalidResultSetAccessException; import org.springframework.lang.Nullable; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.BDDMockito.mock; +import static org.mockito.BDDMockito.when; + /** * @author Rod Johnson @@ -176,4 +184,45 @@ public void customExceptionTranslation() { customTranslation.setExceptionClass(String.class)); } + @Test + public void testDataSourceMethodTranslation() throws SQLException { + DataSource ds = mock(DataSource.class); + Connection conn = mock(Connection.class); + DatabaseMetaData metaData = mock(DatabaseMetaData.class); + when(ds.getConnection()).thenReturn(conn); + when(conn.getMetaData()).thenReturn(metaData); + when(metaData.getDatabaseProductName()).thenReturn("H2"); + SQLErrorCodeSQLExceptionTranslator translator = new SQLErrorCodeSQLExceptionTranslator(ds); + SQLErrorCodes codes = translator.getSqlErrorCodes(); + assertThat(codes).isNotNull(); + } + + @Test + public void testDataSourceMethodTranslationConnectionFailureRetried() throws SQLException { + DataSource ds = mock(DataSource.class); + Connection conn = mock(Connection.class); + DatabaseMetaData metaData = mock(DatabaseMetaData.class); + when(ds.getConnection()).thenThrow(CannotGetJdbcConnectionException.class).thenReturn(conn); + when(conn.getMetaData()).thenReturn(metaData); + when(metaData.getDatabaseProductName()).thenReturn("H2"); + SQLErrorCodeSQLExceptionTranslator translator = new SQLErrorCodeSQLExceptionTranslator(ds); + DataAccessException ex = translator.translate("task", "SQL", new SQLException("reason1", "01504", 23001)); + assertThat(ex).isInstanceOf(DuplicateKeyException.class); + } + + @Test + public void testDataSourceMethodTranslationConnectionFailureRetriedAndFallback() throws SQLException { + DataSource ds = mock(DataSource.class); + Connection conn = mock(Connection.class); + DatabaseMetaData metaData = mock(DatabaseMetaData.class); + when(ds.getConnection()).thenThrow(CannotGetJdbcConnectionException.class).thenThrow(CannotGetJdbcConnectionException.class).thenReturn(conn); + when(conn.getMetaData()).thenReturn(metaData); + when(metaData.getDatabaseProductName()).thenReturn("H2"); + SQLErrorCodeSQLExceptionTranslator translator = new SQLErrorCodeSQLExceptionTranslator(ds); + DataAccessException ex = translator.translate("task", "SQL", new SQLException("reason1", "01504", 23001)); + assertThat(ex).isNotInstanceOf(DuplicateKeyException.class); + ex = translator.translate("task", "SQL", new SQLException("reason1", "01504", 23001)); + assertThat(ex).isInstanceOf(DuplicateKeyException.class); + } + } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodesFactoryTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodesFactoryTests.java index f2d69fa14930..4b4e649591e3 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodesFactoryTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/support/SQLErrorCodesFactoryTests.java @@ -27,6 +27,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; +import org.springframework.jdbc.CannotGetJdbcConnectionException; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.BDDMockito.given; @@ -245,15 +246,28 @@ public void testDataSourceWithNullMetadata() throws Exception { @Test public void testGetFromDataSourceWithSQLException() throws Exception { + Connection connection = mock(Connection.class); SQLException expectedSQLException = new SQLException(); DataSource dataSource = mock(DataSource.class); - given(dataSource.getConnection()).willThrow(expectedSQLException); + given(dataSource.getConnection()).willReturn(connection); + given(connection.getMetaData()).willThrow(expectedSQLException); SQLErrorCodes sec = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource); assertIsEmpty(sec); } + @Test + public void testGetFromDataSourceWithCannotGetJdbcConnectionException() throws Exception { + CannotGetJdbcConnectionException expectedException = new CannotGetJdbcConnectionException("Could not get jdbc connection", mock(SQLException.class)); + + DataSource dataSource = mock(DataSource.class); + given(dataSource.getConnection()).willThrow(expectedException); + + SQLErrorCodes sec = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource); + assertThat(sec).isNull(); + } + private void assertIsEmpty(SQLErrorCodes sec) { // Codes should be empty assertThat(sec.getBadSqlGrammarCodes().length).isEqualTo(0);