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

Sort detected PersistenceExceptionTranslator beans #24644

Conversation

ttddyy
Copy link
Contributor

@ttddyy ttddyy commented Mar 5, 2020

Apply sorting on detected PersistenceExceptionTranslators.

Relates to #24634

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 5, 2020
@ttddyy
Copy link
Contributor Author

ttddyy commented Mar 5, 2020

The default PersistenceExceptionTranslators, such as AbstractEntityManagerFactoryBean, are not ordered, so they always come last.
For my usecase, I need my custom one to be first, so it is fine. But, if custom translators need to come after defaults, then, default ones need to have some ordering numbers assigned.

}
if (comparatorToUse == null) {
comparatorToUse = OrderComparator.INSTANCE;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the above code can be more compact and elegant as

Comparator<Object> comparatorToUse = OrderComparator.INSTANCE;
if (beanFactory instanceof DefaultListableBeanFactory) {
	comparatorToUse = ((DefaultListableBeanFactory) beanFactory).getDependencyComparator();
}

or simply

Comparator<Object> comparatorToUse;
if (beanFactory instanceof DefaultListableBeanFactory) {
	comparatorToUse = ((DefaultListableBeanFactory) beanFactory).getDependencyComparator();
}
else  {
	comparatorToUse = OrderComparator.INSTANCE;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultListableBeanFactory#getDependencyComparator() is @Nullable, so it might return null.

Apply sorting on detected PersistenceExceptionTranslators.

Relates to spring-projects#24634
@ttddyy ttddyy force-pushed the 24634_order-detected-ex-translator branch from 2bf1975 to 4b09631 Compare March 26, 2020 18:26
@jhoeller jhoeller self-assigned this Jul 22, 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 Jul 22, 2020
@jhoeller jhoeller added this to the 5.3 M2 milestone Jul 22, 2020
@jhoeller jhoeller changed the title Sort detected PersistenceExceptionTranslator Sort detected PersistenceExceptionTranslator beans Aug 7, 2020
@jhoeller
Copy link
Contributor

jhoeller commented Aug 7, 2020

As indicated on #25559, I've resolved this in a different way, calling getBeanProvider with its orderedStream method (which takes the factory-level dependency comparator into account even for @Order definitions on @Bean methods). In order to preserve non-eager init semantics, we're specifically using a new getBeanProvider variant with an explicit allowEagerInit flag there now.

Thanks for the pull request, in any case!

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: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants