spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.67k stars 38.15k forks source link

Enhancement proposal: give jdbcclient access to conversion services for converting custom object types from db. #33467

Open alexanderankin opened 2 months ago

alexanderankin commented 2 months ago

Enhancement Description: allow JdbcClient to convert custom object types in records. for example, in postgres you may have a jsonb and you'd want to be able to convert that into something so that you can parse it onto your record class while fetching:

record CustomRecord(JsonNode jsonbColumn) {}
jdbcClient.sql("select jsonb_column from some_table").query(CustomRecord.class).list();

code references:

possible solution:

class DefaultJdbcClient {

    private final JdbcOperations classicOps;

    private final NamedParameterJdbcOperations namedParamOps;

    private final Map<Class<?>, RowMapper<?>> rowMapperCache = new ConcurrentHashMap<>();

    public DefaultJdbcClient(DataSource dataSource) {
        this.classicOps = new JdbcTemplate(dataSource);
        this.namedParamOps = new NamedParameterJdbcTemplate(this.classicOps);
    }

    public DefaultJdbcClient(JdbcOperations jdbcTemplate) {
        Assert.notNull(jdbcTemplate, "JdbcTemplate must not be null");
        this.classicOps = jdbcTemplate;
        this.namedParamOps = new NamedParameterJdbcTemplate(jdbcTemplate);
    }

    public DefaultJdbcClient(NamedParameterJdbcOperations jdbcTemplate) {
        Assert.notNull(jdbcTemplate, "JdbcTemplate must not be null");
        this.classicOps = jdbcTemplate.getJdbcOperations();
        this.namedParamOps = jdbcTemplate;
    }
}

to this pattern: ```java class DefaultJdbcClient { private final JdbcOperations classicOps; private final NamedParameterJdbcOperations namedParamOps; private final ConversionService conversionService; private final Map, RowMapper> rowMapperCache = new ConcurrentHashMap<>(); public DefaultJdbcClient(DataSource dataSource) { this(new JdbcTemplate(dataSource)); } public DefaultJdbcClient(JdbcOperations jdbcTemplate) { this(new NamedParameterJdbcTemplate(jdbcTemplate)); } public DefaultJdbcClient(NamedParameterJdbcOperations jdbcTemplate) { this(jdbcTemplate.getJdbcOperations(), jdbcTemplate, DefaultConversionService.getSharedInstance()); } public DefaultJdbcClient( JdbcOperations jdbcOperations, NamedParameterJdbcOperations namedParameterJdbcOperations, ConversionService conversionService ) { Assert.notNull(jdbcOperations, "jdbcOperations must not be null"); Assert.notNull(namedParameterJdbcOperations, "namedParameterJdbcOperations must not be null"); Assert.notNull(conversionService, "conversionService must not be null"); this.classicOps = jdbcOperations; this.namedParamOps = namedParameterJdbcOperations; this.conversionService = conversionService; } } ```
    @Bean
    JdbcClient jdbcClient(NamedParameterJdbcTemplate jdbcTemplate) {
        return JdbcClient.create(jdbcTemplate);
    }

to

    @Bean
    JdbcClient jdbcClient(NamedParameterJdbcTemplate jdbcTemplate, /*kosher?*/ @Nullable ConversionService conversionService) {
        return JdbcClient.create(jdbcTemplate, java.util.Objects.requireNonNullElseGet(conversionService, DefaultConversionService::getSharedInstance));
    }

benefits

you can now register converters as beans and they will be able to be used for jdbcClient queries.

considerations

alexanderankin commented 2 months ago

for anyone else struggling with this the solution was to add the jdbc driver for your database to the compilation scope (e.g. from runtimeClasspath to implementation in gradle terms) and simply refer the types in the jdbc driver in your record/classes for fetching. a conversion service for the jdbcclient would still be an improvement though, im not sure using types from a jdbc driver is a great choice.

quaff commented 2 months ago

It requires changes both in Spring Framework (construct DefaultJdbcClient by accepting ConversionService) and Spring Boot (autoconfigure DefaultJdbcClient by injecting ConversionService), it's not appropriate to add ConversionService as property of DefaultJdbcClient because conversion is not the responsibility of it, why not passing rowMapper instead of mappedClass ?

jdbcClient.sql("select jsonb_column from some_table").query(new SimplePropertyRowMapper<>(CustomRecord.class, conversionService)).list();
alexanderankin commented 2 months ago

If this issue is waiting for feedback from me, I would just say that it appears part of the plumbing is already there, so I am just trying to improve the wiring. clearly the responsibility of the client is to return pojos, which includes conversion. why not support conversion from the spring framework? this preserves the typical responsibility of framework to do, and boot to configure.

bclozel commented 2 months ago

We have discussed it as a team and here is our assessment:

From a Spring Framework perspective, this would indeed mean adding constructor variants for DefaultJdbcClient accepting a custom ConversionService (to be used in place of the default one, if present). This would then be used for rowmappers calls internally.

From a Spring Boot perspective, we don't think that we should change the auto-configuration there, as there is a default conversion service used for binding properties, another one for the web parts of the application, and possibly more... Instead, we think that the users can create a custom JdbcClient bean in their configuration. The JdbcClientAutoConfiguration is really straightforward so the effort would be minimal and the opinion always right.

We are declining this enhancement from a Spring Boot perspective, and transferring this issue to the Framework team so they can consider this request.

Thanks!