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

Add a new configuration that will format the SQL #89

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

rvullriede
Copy link
Contributor

@rvullriede rvullriede commented Aug 10, 2023

This PR introduces a new configuration property decorator.datasource.datasource-proxy.format-sql that leverages the new ProxyDataSourceBuilder.FormatQueryCallback() from datasource-proxy-1.9 to enable to log formatted output.
It uses Hibernate's formatter if present on the class path (which is the case for 99% of the user due to it being the default implementation used by Spring Boot).
If somebody opted in for a different JPA provider, https://github.com/vertical-blank/sql-formatter can be added as a runtime dependency to the app to enable this.

See jdbc-observations/datasource-proxy#94 for additional information.

@rvullriede
Copy link
Contributor Author

@gavlyukovskiy would you mind having a look?

Thanks :-)
Raphael

@gavlyukovskiy
Copy link
Owner

Hey @rvullriede, sorry for delayed response. I like the idea of formatter, but I'd like to implement it a bit differently - instead of using classExists I'd rather have autoconfigurations depending on the formatter class that would create appropriate FormatQueryCallback:

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass(BasicFormatterImpl.class)
    @ConditionalOnProperty(name = "decorator.datasource.datasource-proxy.format-sql", havingValue = "true", matchIfMissing = true)
    static class HibernateFormatterConfiguration {

        @Bean
        @ConditionalOnMissingBean // let users define their own
        public FormatQueryCallback hibernateFormatQueryCallback() {
            BasicFormatterImpl hibernateFormatter = new BasicFormatterImpl();
            return hibernateFormatter::format;
        }
    }

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass(SqlFormatter.class)
    @ConditionalOnProperty(name = "decorator.datasource.datasource-proxy.format-sql", havingValue = "true", matchIfMissing = true)
    static class SqlFormatterConfiguration {

        @Bean
        @ConditionalOnMissingBean // let users define their own
        public FormatQueryCallback sqlFormatterFormatQueryCallback() {
            return SqlFormatter::format;
        }
    }

then in the builder you can inject this optional bean and if exists - configure to use it.

It is a bit more complicated, but this way it would be more extensible in the future if there are other formatter dependencies we'd like to add, or, if users have their own preferred formatter they could always simply define a bean to override default formatter.

@rvullriede
Copy link
Contributor Author

Hi, @gavlyukovskiy

Thanks for your feedback. Your suggestion is certainly more the Spring way, I'll rework the PR accordingly.

@rvullriede
Copy link
Contributor Author

PR has been updated! Since the log output changes quite a bit I didn't use matchIfMissing = true to keep the default behaviour unchanged.

@gavlyukovskiy gavlyukovskiy merged commit 5a3f370 into gavlyukovskiy:master Dec 1, 2023
1 check passed
@gavlyukovskiy
Copy link
Owner

Thank you for the PR @rvullriede!

@gavlyukovskiy
Copy link
Owner

Since the default is now not changed, I have updated your changes with an exception instead of a warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants