Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revisit DataAccessException translation API #24634

Closed
ttddyy opened this issue Mar 3, 2020 · 3 comments
Closed

Revisit DataAccessException translation API #24634

ttddyy opened this issue Mar 3, 2020 · 3 comments
Assignees
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: documentation A documentation task type: enhancement A general enhancement
Milestone

Comments

@ttddyy
Copy link
Contributor

ttddyy commented Mar 3, 2020

I am implementing transaction retry mechanism to our applications and using DataAccessException mapping facility to translate some db failures to transient exception.
Since DataAccessException mapping is one of the early days feature of spring-framework, I felt some of the APIs are bit outdated.

Here is list of things I encountered:

SQLErrorCodeSQLExceptionTranslator requires subclassing to perform custom mapping logic.

To add custom mappings to existing logic, SQLErrorCodeSQLExceptionTranslator#customTranslate need to be overridden by subclass.

I made a delegation subclass, then injected custom translators.

public class CustomDelegatingSQLErrorCodeSQLExceptionTranslator extends SQLErrorCodeSQLExceptionTranslator {

	@Nullable
	private SQLExceptionTranslator delegate;

	// constructors

	@Override
	protected DataAccessException customTranslate(String task, String sql, SQLException sqlEx) {
		if (this.delegate == null) {
			return null;
		}
		return this.delegate.translate(task, sql, sqlEx);
	}
}

I think composition style API is nicer to have than sub-classing.

Does not have a way to append/update some of the error codes on SQLErrorCodes (not just entire override)

Error code provided by org/springframework/jdbc/support/sql-error-codes.xml(SQL_ERROR_CODE_DEFAULT_PATH) can be overridden by sql-error-codes.xml(SQL_ERROR_CODE_OVERRIDE_PATH).
However, the override is done by bean(DB type). So, it cannot partially update error codes on a specific bean. For example, adding some error codes to transient exceptions to PostgreSQL.

To workaround repeating entire DB error code mapping, I created a SQLErrorCodesUpdater which uses the nature that the SQLErrorCodes is a mutable java bean.

@AllArgsConstructor
@Getter
public class CustomSQLErrorCodes {

	/**
	 * name has to match with db vendor name from
	 * "DataSourceMetaData#getDatabaseProductName" which also matches to the bean id in
	 * "org/springframework/jdbc/support/sql-error-codes.xml"
	 */
	private String databaseName;

	/**
	 * Which exception to map
	 */
	private DataAccessExceptionType dataAccessExceptionType;

	/**
	 * Whether replace or add to the default sql error code
	 */
	private boolean add;

	/**
	 * SQL error code to add or replace
	 */
	private String[] codes;

}
public enum DataAccessExceptionType {
	BAD_SQL_GRAMMAR("badSqlGrammarCodes"),
	INVALID_RESULT_SET_ACCESS("invalidResultSetAccessCodes"),
	DUPLICATE_KEY("duplicateKeyCodes"),
  ... 

	private String propertyName;
  ...
}
public class SQLErrorCodesUpdater {

	public void update(CustomSQLErrorCodes customSQLErrorCodes) {
		update(customSQLErrorCodes.getDatabaseName(), customSQLErrorCodes.getDataAccessExceptionType(),
				customSQLErrorCodes.isAdd(), customSQLErrorCodes.getCodes());
	}

	private void update(String databaseName, DataAccessExceptionType exceptionType, boolean add, String[] values) {
		SQLErrorCodesFactory factory = SQLErrorCodesFactory.getInstance();
		SQLErrorCodes sec = factory.getErrorCodes(databaseName);

		String propertyName = exceptionType.getPropertyName();

		BeanWrapper beanWrapper = new BeanWrapperImpl(sec);
		Set<String> newCodes = new HashSet<>(Arrays.asList(values));
		if (add) {
			// since codes fields in SQLErrorCodes have initially empty String[] assigned
			// codes will never be null
			String[] codes = (String[]) beanWrapper.getPropertyValue(propertyName);
			newCodes.addAll(Arrays.asList(codes));
		}
		beanWrapper.setPropertyValue(propertyName, newCodes.toArray(new String[0]));

	}

Then, run them at runtime

@Bean
public InitializingBean updateSQLErrorCodes(ObjectProvider<CustomSQLErrorCodes> customSQLErrorCodesProvider) {
  SQLErrorCodesUpdater updater = new SQLErrorCodesUpdater();
  return () -> {
    for (CustomSQLErrorCodes customSQLErrorCodes : customSQLErrorCodesProvider) {
      updater.update(customSQLErrorCodes);
    }
  };
}

Alternatively, CustomSQLErrorCodesTranslation can be used to provide custom mappings, but it is mapped to SQLErrorCodes instance which is database vendor specific. I can create a custom error code map, but assigning it requires to retrieve db vendor type.

CustomSQLErrorCodesTranslation translation = new CustomSQLErrorCodesTranslation();
translation.setExceptionClass(TransientDataAccessResourceException.class);
translation.setErrorCodes("99999");

// custom error map by DB vendor type
Map<String, List<CustomSQLErrorCodesTranslation>> map = new HashMap<>();
map.put("H2", Arrays.asList(translation));

// here needs to retrieve db vendor name
String dbName = null;
try {
  dbName = JdbcUtils.extractDatabaseMetaData(dataSource, "getDatabaseProductName");
}
catch (MetaDataAccessException ex) {
  // TODO: log or re-throw
}
List<CustomSQLErrorCodesTranslation> customTranslations = map.getOrDefault(dbName, Collections.emptyList());

SQLErrorCodes sqlErrorCodes = SQLErrorCodesFactory.getInstance().getErrorCodes(dataSource);
sqlErrorCodes.setCustomTranslations(customTranslations.toArray(new CustomSQLErrorCodesTranslation[0]));

Still it's a bit complicated to just apply custom error code mappings.

SQLErrorCodes does not know the db vendor name, I need to retrieve it from datasource. (SQLErrorCodes#getDatabaseProductName() returns one from bean property and different from bean id(db vendor name), which is a bit confusing as well.)

An instance of SQLErrorCodeSQLExceptionTranslator is specific to single DB type

SQLErrorCodeSQLExceptionTranslator when setDataSource, setDatabaseProductName, setSqlErrorCodes, or corresponding constructors are called, then sqlErrorCodes is populated for the specific DB vendor type.
This makes the bean dependent to specific vendor type. It is fine but not simple to find this dependency until digging the code.

Probably adding documentation would be helpful.

PersistenceExceptionTranslationInterceptor#detectPersistenceExceptionTranslators does not consider ordering

When multiple PersistenceExceptionTranslator are available, there is no way to specify ordering.
I don't find any workaround to guarantee the ordering of each translator unless spring code is updated to consider the ordering.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 3, 2020
@jhoeller jhoeller self-assigned this Mar 4, 2020
@jhoeller jhoeller added in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 4, 2020
@jhoeller jhoeller added this to the 5.3 M2 milestone Mar 4, 2020
ttddyy added a commit to ttddyy/spring-framework that referenced this issue Mar 26, 2020
Apply sorting on detected PersistenceExceptionTranslators.

Relates to spring-projects#24634
@jhoeller jhoeller modified the milestones: 5.3 M2, 5.x Backlog Jul 22, 2020
@jhoeller jhoeller changed the title Modernize DataAccessException translation API Revisit DataAccessException translation API Nov 11, 2020
@jhoeller jhoeller added the type: documentation A documentation task label Nov 11, 2020
@jhoeller jhoeller modified the milestones: 5.x Backlog, 5.3.2 Nov 11, 2020
@jhoeller jhoeller modified the milestones: 5.3.2, 5.x Backlog Dec 7, 2020
@jhoeller jhoeller modified the milestones: 6.x Backlog, 6.1.0-M3 Jul 5, 2023
@jhoeller
Copy link
Contributor

jhoeller commented Jul 5, 2023

I intend to revisit this for 6.1, in the context of some of the 6.0 baseline upgrades that we did, e.g. SQLExceptionSubclassTranslator being the default now.

@jhoeller
Copy link
Contributor

jhoeller commented Jul 8, 2023

Also, as of #24644, detectPersistenceExceptionTranslators operates with ordering in the meantime. So let's focus on the customizability options for SQLException translation in our 6.0 baseline arrangement, in order to ultimately close this issue.

@jhoeller
Copy link
Contributor

AbstractFallbackSQLExceptionTranslator allows for a configurable customTranslator now, overriding before the regular doTranslate step, whereas the existing fallbackTranslator kicks in afterwards. This works with SQLErrorCodeSQLExceptionTranslator (where the protected customTranslate method is deprecated now) but in particular also with the now-default SQLExceptionSubclassTranslator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: documentation A documentation task type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants