Skip to content

Make JooqExceptionTranslator conditional #38736

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

Closed
MelleD opened this issue Dec 11, 2023 · 3 comments
Closed

Make JooqExceptionTranslator conditional #38736

MelleD opened this issue Dec 11, 2023 · 3 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@MelleD
Copy link
Contributor

MelleD commented Dec 11, 2023

I would like to set my own Exception Translator or ExecuteListener to translate exception. Currently the exception is always translated twice.
Can't the bean be set conditionally? What is the use case for the Order to have 2 beans?

See JooqAutoConfiguration

@Bean
@Order(0)
public DefaultExecuteListenerProvider jooqExceptionTranslatorExecuteListenerProvider() {
	return new DefaultExecuteListenerProvider(new JooqExceptionTranslator());
}

Proposal if only 1 provider is necessary

@Bean
@ConditionalOnMissingBean
public DefaultExecuteListenerProvider jooqExceptionTranslatorExecuteListenerProvider() {
	return new DefaultExecuteListenerProvider(new JooqExceptionTranslator());
}

Quick Fix with order

@Bean
@Order(0)
@CondationalOnProperty("spring.xyz.jooq.translator.enabled")
public DefaultExecuteListenerProvider jooqExceptionTranslatorExecuteListenerProvider() {
	return new DefaultExecuteListenerProvider(new JooqExceptionTranslator());
}

Or own interface for jOOQ Bean Translator

public interface JooqExceptionTranslator extends ExecuteListener

@Bean
@Order(0)
public DefaultExecuteListenerProvider jooqExceptionTranslatorExecuteListenerProvider(JooqExceptionTranslator translator) {
	return new DefaultExecuteListenerProvider(translator);
}

@Bean
@ConditionalOnMissingBean
public JooqExceptionTranslator jooqExceptionTranslator() {
	return new DefaultJooqExceptionTranslator();
}

What do you think?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 11, 2023
@philwebb
Copy link
Member

All ExecuteListenerProvider beans currently get passed to org.jooq.impl.DefaultConfiguration.set(...) so I don't think we can make DefaultExecuteListenerProvider conditional. That would help in your case, but not for folks that want a ExecuteListenerProvider bean as well as our JooqExceptionTranslator.

We'd probably also want to avoid @CondationalOnProperty("spring.xyz.jooq.translator.enabled") because as a general rule we try not to add too many *.enabled properties.

I think the third option isn't too bad and perhaps we could even change the existing JooqExceptionTranslator to make it easier to subclass.

I'm curious if you're finding the translation happening twice an actual problem? Is it a performance issue or is it somehow doing a translation that you don't want?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Dec 11, 2023
@MelleD
Copy link
Contributor Author

MelleD commented Dec 13, 2023

It's not a real problem. But very confusing when debugging that it is translated 2 times. It is also difficult to understand why the exception is translated twice. It's not intuitive. You also have to pay attention to the order, if it's not correct it won't work.
And the third option is not a big change and it's not breaking. So for me would be a good improvement.

Maybe something like this
#38762

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Dec 13, 2023
@mhalbritter
Copy link
Contributor

Superseded by #38762.

@mhalbritter mhalbritter closed this as not planned Won't fix, can't repro, duplicate, stale Jan 10, 2024
@mhalbritter mhalbritter added status: superseded An issue that has been superseded by another and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

4 participants