From d07b733dae7b7284e12e66a08695e691cde5c191 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 3 Sep 2020 23:15:06 +0200 Subject: [PATCH] Retry SQLErrorCodesFactory retrieval if DatabaseMetaData access failed Closes gh-25681 --- .../jdbc/support/JdbcUtils.java | 36 +++++++++--- .../SQLErrorCodeSQLExceptionTranslator.java | 58 ++++++++++--------- .../jdbc/support/SQLErrorCodesFactory.java | 25 ++++++-- ...LErrorCodeSQLExceptionTranslatorTests.java | 34 ++++++++++- .../support/SQLErrorCodesFactoryTests.java | 29 +++++----- 5 files changed, 126 insertions(+), 56 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/support/JdbcUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/support/JdbcUtils.java index 3a64a579dc14..4385927b9da2 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/support/JdbcUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/support/JdbcUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -299,18 +299,19 @@ else if (obj instanceof java.sql.Date) { /** * Extract database meta-data via the given DatabaseMetaDataCallback. - *

This method will open a connection to the database and retrieve the database meta-data. - * Since this method is called before the exception translation feature is configured for - * a datasource, this method can not rely on the SQLException translation functionality. - *

Any exceptions will be wrapped in a MetaDataAccessException. This is a checked exception - * and any calling code should catch and handle this exception. You can just log the - * error and hope for the best, but there is probably a more serious error that will - * reappear when you try to access the database again. + *

This method will open a connection to the database and retrieve its meta-data. + * Since this method is called before the exception translation feature is configured + * for a DataSource, this method can not rely on SQLException translation itself. + *

Any exceptions will be wrapped in a MetaDataAccessException. This is a checked + * exception and any calling code should catch and handle this exception. You can just + * log the error and hope for the best, but there is probably a more serious error that + * will reappear when you try to access the database again. * @param dataSource the DataSource to extract meta-data for * @param action callback that will do the actual work * @return object containing the extracted information, as returned by * the DatabaseMetaDataCallback's {@code processMetaData} method * @throws MetaDataAccessException if meta-data access failed + * @see java.sql.DatabaseMetaData */ public static Object extractDatabaseMetaData(DataSource dataSource, DatabaseMetaDataCallback action) throws MetaDataAccessException { @@ -318,7 +319,24 @@ public static Object extractDatabaseMetaData(DataSource dataSource, DatabaseMeta Connection con = null; try { con = DataSourceUtils.getConnection(dataSource); - DatabaseMetaData metaData = con.getMetaData(); + DatabaseMetaData metaData; + try { + metaData = con.getMetaData(); + } + catch (SQLException ex) { + if (DataSourceUtils.isConnectionTransactional(con, dataSource)) { + // Probably a closed thread-bound Connection - retry against fresh Connection + DataSourceUtils.releaseConnection(con, dataSource); + con = null; + logger.debug("Failed to obtain DatabaseMetaData from transactional Connection - " + + "retrying against fresh Connection", ex); + con = dataSource.getConnection(); + metaData = con.getMetaData(); + } + else { + throw ex; + } + } if (metaData == null) { // should only happen in test environments throw new MetaDataAccessException("DatabaseMetaData returned by Connection [" + con + "] was null"); 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 3df3f7a85b7e..f50380fcdec0 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 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. @@ -35,6 +35,8 @@ import org.springframework.jdbc.BadSqlGrammarException; import org.springframework.jdbc.InvalidResultSetAccessException; import org.springframework.lang.Nullable; +import org.springframework.util.function.SingletonSupplier; +import org.springframework.util.function.SupplierUtils; /** * Implementation of {@link SQLExceptionTranslator} that analyzes vendor-specific error codes. @@ -76,7 +78,7 @@ public class SQLErrorCodeSQLExceptionTranslator extends AbstractFallbackSQLExcep /** Error codes used by this translator. */ @Nullable - private SQLErrorCodes sqlErrorCodes; + private SingletonSupplier sqlErrorCodes; /** @@ -88,7 +90,7 @@ public SQLErrorCodeSQLExceptionTranslator() { } /** - * Create a SQL error code translator for the given DataSource. + * Create an SQL error code translator for the given DataSource. * Invoking this constructor will cause a Connection to be obtained * from the DataSource to get the meta-data. * @param dataSource the DataSource to use to find meta-data and establish @@ -101,7 +103,7 @@ public SQLErrorCodeSQLExceptionTranslator(DataSource dataSource) { } /** - * Create a SQL error code translator for the given database product name. + * Create an SQL error code translator for the given database product name. * Invoking this constructor will avoid obtaining a Connection from the * DataSource to get the meta-data. * @param dbName the database product name that identifies the error codes entry @@ -114,13 +116,13 @@ public SQLErrorCodeSQLExceptionTranslator(String dbName) { } /** - * Create a SQLErrorCode translator given these error codes. + * Create an SQLErrorCode translator given these error codes. * Does not require a database meta-data lookup to be performed using a connection. * @param sec error codes */ public SQLErrorCodeSQLExceptionTranslator(SQLErrorCodes sec) { this(); - this.sqlErrorCodes = sec; + this.sqlErrorCodes = SingletonSupplier.of(sec); } @@ -134,7 +136,9 @@ public SQLErrorCodeSQLExceptionTranslator(SQLErrorCodes sec) { * @see java.sql.DatabaseMetaData#getDatabaseProductName() */ public void setDataSource(DataSource dataSource) { - this.sqlErrorCodes = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource); + this.sqlErrorCodes = + SingletonSupplier.of(() -> SQLErrorCodesFactory.getInstance().resolveErrorCodes(dataSource)); + this.sqlErrorCodes.get(); // try early initialization - otherwise the supplier will retry later } /** @@ -146,7 +150,7 @@ public void setDataSource(DataSource dataSource) { * @see java.sql.DatabaseMetaData#getDatabaseProductName() */ public void setDatabaseProductName(String dbName) { - this.sqlErrorCodes = SQLErrorCodesFactory.getInstance().getErrorCodes(dbName); + this.sqlErrorCodes = SingletonSupplier.of(SQLErrorCodesFactory.getInstance().getErrorCodes(dbName)); } /** @@ -154,7 +158,7 @@ public void setDatabaseProductName(String dbName) { * @param sec custom error codes to use */ public void setSqlErrorCodes(@Nullable SQLErrorCodes sec) { - this.sqlErrorCodes = sec; + this.sqlErrorCodes = SingletonSupplier.ofNullable(sec); } /** @@ -164,7 +168,7 @@ public void setSqlErrorCodes(@Nullable SQLErrorCodes sec) { */ @Nullable public SQLErrorCodes getSqlErrorCodes() { - return this.sqlErrorCodes; + return SupplierUtils.resolve(this.sqlErrorCodes); } @@ -175,7 +179,6 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL if (sqlEx instanceof BatchUpdateException && sqlEx.getNextException() != null) { SQLException nestedSqlEx = sqlEx.getNextException(); if (nestedSqlEx.getErrorCode() > 0 || nestedSqlEx.getSQLState() != null) { - logger.debug("Using nested SQLException from the BatchUpdateException"); sqlEx = nestedSqlEx; } } @@ -187,8 +190,9 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL } // Next, try the custom SQLException translator, if available. - if (this.sqlErrorCodes != null) { - SQLExceptionTranslator customTranslator = this.sqlErrorCodes.getCustomSqlExceptionTranslator(); + SQLErrorCodes sqlErrorCodes = getSqlErrorCodes(); + if (sqlErrorCodes != null) { + SQLExceptionTranslator customTranslator = sqlErrorCodes.getCustomSqlExceptionTranslator(); if (customTranslator != null) { DataAccessException customDex = customTranslator.translate(task, sql, sqlEx); if (customDex != null) { @@ -198,9 +202,9 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL } // Check SQLErrorCodes with corresponding error code, if available. - if (this.sqlErrorCodes != null) { + if (sqlErrorCodes != null) { String errorCode; - if (this.sqlErrorCodes.isUseSqlStateForTranslation()) { + if (sqlErrorCodes.isUseSqlStateForTranslation()) { errorCode = sqlEx.getSQLState(); } else { @@ -215,7 +219,7 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL if (errorCode != null) { // Look for defined custom translations first. - CustomSQLErrorCodesTranslation[] customTranslations = this.sqlErrorCodes.getCustomTranslations(); + CustomSQLErrorCodesTranslation[] customTranslations = sqlErrorCodes.getCustomTranslations(); if (customTranslations != null) { for (CustomSQLErrorCodesTranslation customTranslation : customTranslations) { if (Arrays.binarySearch(customTranslation.getErrorCodes(), errorCode) >= 0 && @@ -230,43 +234,43 @@ protected DataAccessException doTranslate(String task, @Nullable String sql, SQL } } // Next, look for grouped error codes. - if (Arrays.binarySearch(this.sqlErrorCodes.getBadSqlGrammarCodes(), errorCode) >= 0) { + if (Arrays.binarySearch(sqlErrorCodes.getBadSqlGrammarCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new BadSqlGrammarException(task, (sql != null ? sql : ""), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getInvalidResultSetAccessCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getInvalidResultSetAccessCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new InvalidResultSetAccessException(task, (sql != null ? sql : ""), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getDuplicateKeyCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getDuplicateKeyCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new DuplicateKeyException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getDataIntegrityViolationCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getDataIntegrityViolationCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new DataIntegrityViolationException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getPermissionDeniedCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getPermissionDeniedCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new PermissionDeniedDataAccessException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getDataAccessResourceFailureCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getDataAccessResourceFailureCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new DataAccessResourceFailureException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getTransientDataAccessResourceCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getTransientDataAccessResourceCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new TransientDataAccessResourceException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getCannotAcquireLockCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getCannotAcquireLockCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new CannotAcquireLockException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getDeadlockLoserCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getDeadlockLoserCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new DeadlockLoserDataAccessException(buildMessage(task, sql, sqlEx), sqlEx); } - else if (Arrays.binarySearch(this.sqlErrorCodes.getCannotSerializeTransactionCodes(), errorCode) >= 0) { + else if (Arrays.binarySearch(sqlErrorCodes.getCannotSerializeTransactionCodes(), errorCode) >= 0) { logTranslation(task, sql, sqlEx, false); return new CannotSerializeTransactionException(buildMessage(task, sql, sqlEx), sqlEx); } @@ -276,7 +280,7 @@ else if (Arrays.binarySearch(this.sqlErrorCodes.getCannotSerializeTransactionCod // We couldn't identify it more precisely - let's hand it over to the SQLState fallback translator. if (logger.isDebugEnabled()) { String codes; - if (this.sqlErrorCodes != null && this.sqlErrorCodes.isUseSqlStateForTranslation()) { + if (sqlErrorCodes != null && sqlErrorCodes.isUseSqlStateForTranslation()) { codes = "SQL state '" + sqlEx.getSQLState() + "', error code '" + sqlEx.getErrorCode(); } else { 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..33fc1a40a386 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 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. @@ -159,6 +159,7 @@ protected Resource loadResource(String path) { *

No need for a database meta-data lookup. * @param databaseName the database name (must not be {@code null}) * @return the {@code SQLErrorCodes} instance for the given database + * (never {@code null}; potentially empty) * @throws IllegalArgumentException if the supplied database name is {@code null} */ public SQLErrorCodes getErrorCodes(String databaseName) { @@ -195,9 +196,26 @@ public SQLErrorCodes getErrorCodes(String databaseName) { * instance if no {@code SQLErrorCodes} were found. * @param dataSource the {@code DataSource} identifying the database * @return the corresponding {@code SQLErrorCodes} object + * (never {@code null}; potentially empty) * @see java.sql.DatabaseMetaData#getDatabaseProductName() */ public SQLErrorCodes getErrorCodes(DataSource dataSource) { + SQLErrorCodes sec = resolveErrorCodes(dataSource); + return (sec != null ? sec : new SQLErrorCodes()); + } + + /** + * Return {@link SQLErrorCodes} for the given {@link DataSource}, + * evaluating "databaseProductName" from the + * {@link java.sql.DatabaseMetaData}, or {@code null} if case + * of a JDBC meta-data access problem. + * @param dataSource the {@code DataSource} identifying the database + * @return the corresponding {@code SQLErrorCodes} object, + * or {@code null} in case of a JDBC meta-data access problem + * @see java.sql.DatabaseMetaData#getDatabaseProductName() + */ + @Nullable + SQLErrorCodes resolveErrorCodes(DataSource dataSource) { Assert.notNull(dataSource, "DataSource must not be null"); if (logger.isDebugEnabled()) { logger.debug("Looking up default SQLErrorCodes for DataSource [" + identify(dataSource) + "]"); @@ -218,10 +236,9 @@ public SQLErrorCodes getErrorCodes(DataSource dataSource) { } } catch (MetaDataAccessException ex) { - logger.warn("Error while extracting database name - falling back to empty error codes", ex); + logger.warn("Error while extracting database name", ex); } - // Fallback is to return an empty SQLErrorCodes instance. - return new SQLErrorCodes(); + return null; } } } 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 59cf7a34e60f..22cc7979f33f 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2020 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. @@ -17,12 +17,17 @@ 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.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.mockito.Mockito; import org.springframework.dao.CannotAcquireLockException; import org.springframework.dao.CannotSerializeTransactionException; @@ -36,6 +41,9 @@ import org.springframework.lang.Nullable; import static org.junit.Assert.*; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; /** * @author Rod Johnson @@ -181,4 +189,28 @@ public void customExceptionTranslation() { customTranslation.setExceptionClass(String.class); } + @Test + public void dataSourceInitialization() throws Exception { + SQLException connectionException = new SQLException(); + SQLException duplicateKeyException = new SQLException("test", "", 1); + + DataSource dataSource = mock(DataSource.class); + given(dataSource.getConnection()).willThrow(connectionException); + + SQLErrorCodeSQLExceptionTranslator sext = new SQLErrorCodeSQLExceptionTranslator(dataSource); + assertFalse(sext.translate("test", null, duplicateKeyException) instanceof DuplicateKeyException); + + DatabaseMetaData databaseMetaData = mock(DatabaseMetaData.class); + given(databaseMetaData.getDatabaseProductName()).willReturn("Oracle"); + + Connection connection = mock(Connection.class); + given(connection.getMetaData()).willReturn(databaseMetaData); + + Mockito.reset(dataSource); + given(dataSource.getConnection()).willReturn(connection); + assertTrue(sext.translate("test", null, duplicateKeyException) instanceof DuplicateKeyException); + + verify(connection).close(); + } + } 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 824ae289debc..e2a7e7036e97 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2020 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. @@ -38,6 +38,7 @@ * @author Rod Johnson * @author Thomas Risberg * @author Stephane Nicoll + * @author Juergen Hoeller */ public class SQLErrorCodesFactoryTests { @@ -238,7 +239,11 @@ public void testDataSourceWithNullMetadata() throws Exception { SQLErrorCodes sec = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource); assertIsEmpty(sec); + verify(connection).close(); + reset(connection); + sec = SQLErrorCodesFactory.getInstance().resolveErrorCodes(dataSource); + assertNull(sec); verify(connection).close(); } @@ -251,12 +256,9 @@ public void testGetFromDataSourceWithSQLException() throws Exception { SQLErrorCodes sec = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource); assertIsEmpty(sec); - } - private void assertIsEmpty(SQLErrorCodes sec) { - // Codes should be empty - assertEquals(0, sec.getBadSqlGrammarCodes().length); - assertEquals(0, sec.getDataIntegrityViolationCodes().length); + sec = SQLErrorCodesFactory.getInstance().resolveErrorCodes(dataSource); + assertNull(sec); } private SQLErrorCodes getErrorCodesFromDataSource(String productName, SQLErrorCodesFactory factory) throws Exception { @@ -269,17 +271,9 @@ private SQLErrorCodes getErrorCodesFromDataSource(String productName, SQLErrorCo DataSource dataSource = mock(DataSource.class); given(dataSource.getConnection()).willReturn(connection); - SQLErrorCodesFactory secf = null; - if (factory != null) { - secf = factory; - } - else { - secf = SQLErrorCodesFactory.getInstance(); - } - + SQLErrorCodesFactory secf = (factory != null ? factory : SQLErrorCodesFactory.getInstance()); SQLErrorCodes sec = secf.getErrorCodes(dataSource); - SQLErrorCodes sec2 = secf.getErrorCodes(dataSource); assertSame("Cached per DataSource", sec2, sec); @@ -374,4 +368,9 @@ protected Resource loadResource(String path) { assertIsEmpty(sec); } + private void assertIsEmpty(SQLErrorCodes sec) { + assertEquals(0, sec.getBadSqlGrammarCodes().length); + assertEquals(0, sec.getDataIntegrityViolationCodes().length); + } + }