spring-projects / spring-data-r2dbc

Provide support to increase developer productivity in Java when using Reactive Relational Database Connectivity. Uses familiar Spring concepts such as a DatabaseClient for core API usage and lightweight repository style data access.
Apache License 2.0
708 stars 132 forks source link

Enum is not properly mapped until custom converter created #843

Closed bskorka closed 10 months ago

bskorka commented 10 months ago

Hey! I observed an error in our logs thrown by the repository when the value for the enum in one of the columns is an empty string. Everything works properly when the value is null or one of the proper ones.

org.springframework.data.mapping.MappingException: Could not read property @org.springframework.data.relational.core.mapping.Column("debtSource")private sth.Debt$DebtSource sth.Debt.debtSource from column debtSource!
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.readFrom(MappingR2dbcConverter.java:197)
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.read(MappingR2dbcConverter.java:141)
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.read(MappingR2dbcConverter.java:124)
    at org.springframework.data.r2dbc.convert.EntityRowMapper.apply(EntityRowMapper.java:46)
    at org.springframework.data.r2dbc.convert.EntityRowMapper.apply(EntityRowMapper.java:29)
    at org.mariadb.r2dbc.MariadbResult.lambda$map$2(MariadbResult.java:144)
    at reactor.core.publisher.FluxHandle$HandleSubscriber.onNext(FluxHandle.java:110)
    at reactor.core.publisher.FluxTakeUntil$TakeUntilPredicateSubscriber.onNext(FluxTakeUntil.java:84)
    at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drainRegular(FluxWindowPredicate.java:668)
    at reactor.core.publisher.FluxWindowPredicate$WindowFlux.drain(FluxWindowPredicate.java:746)
    at reactor.core.publisher.FluxWindowPredicate$WindowFlux.onNext(FluxWindowPredicate.java:788)
    at reactor.core.publisher.FluxWindowPredicate$WindowPredicateMain.onNext(FluxWindowPredicate.java:266)
    at reactor.core.publisher.FluxCreate$BufferAsyncSink.drain(FluxCreate.java:878)
    at reactor.core.publisher.FluxCreate$BufferAsyncSink.next(FluxCreate.java:803)
    at reactor.core.publisher.FluxCreate$SerializedFluxSink.next(FluxCreate.java:161)
    at org.mariadb.r2dbc.client.MariadbPacketDecoder.handleBuffer(MariadbPacketDecoder.java:98)
    at org.mariadb.r2dbc.client.MariadbPacketDecoder.decode(MariadbPacketDecoder.java:79)
    at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:529)
    at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:468)
    at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
    at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
    at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
    at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
    at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
    at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:788)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:724)
    at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:650)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
    at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
    at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
    at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
    at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalArgumentException: No enum constant sth.Debt.DebtSource.
    at java.base/java.lang.Enum.valueOf(Enum.java:273)
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.getPotentiallyConvertedSimpleRead(MappingR2dbcConverter.java:286)
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.readValue(MappingR2dbcConverter.java:213)
    at org.springframework.data.r2dbc.convert.MappingR2dbcConverter.readFrom(MappingR2dbcConverter.java:194)
    ... 35 common frames omitted

I have created a converter and the configuration then:

@ReadingConverter
@Slf4j
public class EmptyStringToEnumReadingConverter<T extends Enum<T>> implements Converter<String, Enum<T>> {
    private final Class<T> enumClass;

    public EmptyStringToEnumReadingConverter(Class<T> enumClass) {
        this.enumClass = enumClass;
    }

    @Override
    public T convert(String from) {
        if (StringUtils.isBlank(from)) {
            return null;
        }

        return T.valueOf(enumClass, from);
    }

}
@Bean
    public R2dbcCustomConversions customConversions(ConnectionFactory connectionFactory) {
        R2dbcDialect r2dbcDialect = DialectResolver.getDialect(connectionFactory);
        return R2dbcCustomConversions.of(r2dbcDialect, List.of(
                new EmptyStringToEnumReadingConverter<>(Debt.DebtSource.class)
        ));
    }

It worked immediately, but I wanted to check something and changed the implementation of the convert method to just return null. The conversion was still working, and it was returning correct values when the value in DB was one of the enum ones. To check it I put some breakpoints and it became clear that the code is now using org.springframework.core.convert.support.StringToEnumConverterFactory.StringToEnum instead of my custom conversion.

The moment I passed just an empty list as converters to the R2dbcCustomConversions it stopped working.

It looks like I need to initialize the R2dbcCustomConversions with some Converter (I can even pass different enum classes than I need), and then the code is using the Spring StringToEnum converter which handles the empty string. Otherwise, it fails at any time.

Summary:

What can I do to use StringToEnum by default here, without the need to create some custom conversion which is not used at all?

Spring Boot version - 2.7.17

mp911de commented 10 months ago

Enum conversion works as designed. null values in the column map to null values in the domain model. Anything else is being looked up through Enum.valueOf(…). Translating empty string values to null is a rule specific to your domain and not something that applies broadly.

As this is a rather specific issue, please configure a custom conversions bean for your application.

bskorka commented 10 months ago

I don't know if you get my point here. When I create the custom conversion bean it is not used. The org.springframework.core.convert.support.StringToEnumConverterFactory.StringToEnum is used instead and it deals with empty strings. The question is why the Spring StringToEnum is used only if R2dbcCustomConversions are declared (not empty), and why my custom bean is not used.

bskorka commented 10 months ago

@mp911de - I still think it's a bug or a inconsistent behavior, do you disagree?

mp911de commented 10 months ago

R2dbcCustomConversions keeps track of read and write targets. When registering a converter, we determine the custom read or write target. If that is set, then we register the converter in Spring's ConversionService and use the conversion service to perform the actual conversion.

Spring's ConversionService comes already with a converter (StringToEnumConverterFactory) that matches Enum<T> as target type. Because of type erasure, the T from EmptyStringToEnumReadingConverter<T extends Enum<T>> is being flattened down to Enum and therefore, the conversion service now has two converters. Spring's ConversionService checks the raw type for your converter and since the raw type is Enum and not Debt.DebtSource, the custom converter isn't used.

Instead, StringToEnum is provided by the StringToEnumConverterFactory that is a ConverterFactory and it creates an instance for string to enum conversion with an exact type match.

Let me know whether this helps.

bskorka commented 10 months ago

After the explanation, I got the point of why my converter is not used.

What I didn't understand was why the Spring ConversionService and StringToEnumConverterFactory were not used without the registration of the R2dbcCustomConversions bean. I thought that spring and spring-data-r2dbc would treat the data equally. Your answer covers that a little.

It is also a little bit unclear that one day we will throw an error on an empty string, and on the next day if you register the custom converter for literally anything, your enums will be converted without any error.