spring-projects / spring-data-relational

Spring Data Relational. Home of Spring Data JDBC and Spring Data R2DBC.
https://spring.io/projects/spring-data-jdbc
Apache License 2.0
753 stars 345 forks source link

ConversionFailedException on generic Entity Id and custom converter #1842

Open lukas-krecan opened 1 month ago

lukas-krecan commented 1 month ago

We are using Spring Data JDBC and a generic type for entity ID.

class Entity1 {
  @Id
  Id<Long> id;
  ...
}

class Entity2 {
   @Id
   Id<UUID> id;
   ...
}

For it to work we have a custom converter (see the example below).

Our code works in Spring 3.2.5 but does not in 3.2.8 (and most likely some previous versions too). It throws

Failed to execute InsertRoot{entity=AssetEntity(id=715776570, externalId=1ae46f5b6f5f4509cbdfaed64546fdfc169be3a8, category=persequeris, subcategory=tempor, type=Best assets, name=Test Asset, mgmtId=111, groupId=1247545726705402873, agentUuid=7b721483-f5b2-48cc-bf93-a3e5dfd34f7c, osType=IOS, privileged=false, issuerType=null, internalCreatedAt=2024-07-26T18:51:59.476985Z, internalUpdatedAt=2024-07-26T18:51:59.476985Z), idValueSource=PROVIDED}
org.springframework.data.relational.core.conversion.DbActionExecutionException: Failed to execute InsertRoot{entity=AssetEntity(id=715776570, externalId=1ae46f5b6f5f4509cbdfaed64546fdfc169be3a8, category=persequeris, subcategory=tempor, type=Best assets, name=Test Asset, mgmtId=111, groupId=1247545726705402873, agentUuid=7b721483-f5b2-48cc-bf93-a3e5dfd34f7c, osType=IOS, privileged=false, issuerType=null, internalCreatedAt=2024-07-26T18:51:59.476985Z, internalUpdatedAt=2024-07-26T18:51:59.476985Z), idValueSource=PROVIDED}
at org.springframework.data.jdbc.core.AggregateChangeExecutor.execute(AggregateChangeExecutor.java:118)
at org.springframework.data.jdbc.core.AggregateChangeExecutor.lambda$executeSave$0(AggregateChangeExecutor.java:61)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at org.springframework.data.relational.core.conversion.SaveBatchingAggregateChange.forEachAction(SaveBatchingAggregateChange.java:74)
at org.springframework.data.jdbc.core.AggregateChangeExecutor.executeSave(AggregateChangeExecutor.java:61)
at org.springframework.data.jdbc.core.JdbcAggregateTemplate.performSave(JdbcAggregateTemplate.java:491)
at org.springframework.data.jdbc.core.JdbcAggregateTemplate.save(JdbcAggregateTemplate.java:168)
at org.springframework.data.jdbc.repository.support.SimpleJdbcRepository.save(SimpleJdbcRepository.java:68)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:354)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker$RepositoryFragmentMethodInvoker.lambda$new$0(RepositoryMethodInvoker.java:277)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.doInvoke(RepositoryMethodInvoker.java:170)
at org.springframework.data.repository.core.support.RepositoryMethodInvoker.invoke(RepositoryMethodInvoker.java:158)
at org.springframework.data.repository.core.support.RepositoryComposition$RepositoryFragments.invoke(RepositoryComposition.java:516)
at org.springframework.data.repository.core.support.RepositoryComposition.invoke(RepositoryComposition.java:285)
at org.springframework.data.repository.core.support.RepositoryFactorySupport$ImplementationMethodExecutionInterceptor.invoke(RepositoryFactorySupport.java:628)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.doInvoke(QueryExecutorMethodInterceptor.java:168)
at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.invoke(QueryExecutorMethodInterceptor.java:143)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.data.projection.DefaultMethodInvokingMethodInterceptor.invoke(DefaultMethodInvokingMethodInterceptor.java:70)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:379)
at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:119)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.dao.support.PersistenceExceptionTranslationInterceptor.invoke(PersistenceExceptionTranslationInterceptor.java:138)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:97)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.data.repository.core.support.MethodInvocationValidator.invoke(MethodInvocationValidator.java:95)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:184)
at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:223)
at jdk.proxy3/jdk.proxy3.$Proxy95.save(Unknown Source)
...
Caused by: org.springframework.core.convert.ConversionFailedException: Failed to convert from type [java.lang.String] to type [java.util.UUID] for value [715776570]
at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:47)
at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:182)
at org.springframework.core.convert.support.GenericConversionService.convert(GenericConversionService.java:165)
at org.springframework.data.relational.core.conversion.MappingRelationalConverter.writeValue(MappingRelationalConverter.java:682)
at org.springframework.data.jdbc.core.convert.MappingJdbcConverter.writeValue(MappingJdbcConverter.java:214)
at org.springframework.data.jdbc.core.convert.MappingJdbcConverter.writeJdbcValue(MappingJdbcConverter.java:251)
at org.springframework.data.jdbc.core.convert.JdbcConverter.writeJdbcValue(JdbcConverter.java:53)
at org.springframework.data.jdbc.core.convert.SqlParametersFactory.addConvertedValue(SqlParametersFactory.java:199)
at org.springframework.data.jdbc.core.convert.SqlParametersFactory.addConvertedPropertyValue(SqlParametersFactory.java:186)
at org.springframework.data.jdbc.core.convert.SqlParametersFactory.forInsert(SqlParametersFactory.java:92)
at org.springframework.data.jdbc.core.convert.DefaultDataAccessStrategy.insert(DefaultDataAccessStrategy.java:105)
at org.springframework.data.jdbc.core.JdbcAggregateChangeExecutionContext.executeInsertRoot(JdbcAggregateChangeExecutionContext.java:83)
at org.springframework.data.jdbc.core.AggregateChangeExecutor.execute(AggregateChangeExecutor.java:85)
... 155 more
Caused by: java.lang.IllegalArgumentException: Invalid UUID string: 715776570
at java.base/java.util.UUID.fromString1(UUID.java:280)
at java.base/java.util.UUID.fromString(UUID.java:258)
at org.springframework.core.convert.support.StringToUUIDConverter.convert(StringToUUIDConverter.java:37)
at org.springframework.core.convert.support.StringToUUIDConverter.convert(StringToUUIDConverter.java:32)
at org.springframework.core.convert.support.GenericConversionService$ConverterAdapter.convert(GenericConversionService.java:358)
at org.springframework.core.convert.support.ConversionUtils.invokeConverter(ConversionUtils.java:41)
... 167 more

I am able to reproduce the behavior using the following code - it passes in 3.2.5 but fails in 3.2.8. In 3.2.5 it correctly calls the custom converter, in later releases it picks a wrong converter and fails.

import org.junit.jupiter.api.Test;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.convert.CustomConversions.StoreConversions;
import org.springframework.data.relational.core.conversion.MappingRelationalConverter;
import org.springframework.data.relational.core.mapping.RelationalMappingContext;
import org.springframework.data.util.TypeInformation;

import java.util.Collections;
import java.util.Set;
import java.util.UUID;

public class Converter2Test {

    @Test
    void testConverter() {
        CustomConversions customConversions = new CustomConversions(StoreConversions.NONE, Collections.singletonList(new IdConverter()));
        MappingRelationalConverter mappingRelationalConverter = new MappingRelationalConverter(new RelationalMappingContext(), customConversions);

        // persistentEntity.getIdentifierAccessor(instance).getRequiredIdentifier(); in SqlParametersFactory returns string/int not the original type
        mappingRelationalConverter.writeValue(UUID.randomUUID().toString(), TypeInformation.of(Id.class));
        mappingRelationalConverter.writeValue(1, TypeInformation.of(Id.class));
    }

    public static class IdConverter implements GenericConverter {
        @Override
        public Set<ConvertiblePair> getConvertibleTypes() {
            return Set.of(
                new ConvertiblePair(Id.class, UUID.class),
                new ConvertiblePair(Id.class, Long.class),
                new ConvertiblePair(UUID.class, Id.class),
                new ConvertiblePair(Long.class, Id.class)
            );
        }

        @Override
        public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
            if (source == null) {
                return null;
            }
            if (targetType.getType() == Id.class) {
                return new Id<>(source);
            } else {
                return ((Id<?>) source).value();
            }
        }
    }

    public record Id<ID>(ID value) {
        @Override
        public String toString() {
            return value.toString();
        }
    }
}
mp911de commented 1 month ago

This is indeed correct behavior. Spring Data determines the target type for a conversion. new ConvertiblePair(Id.class, Long.class) indicates that Id values should be converted into Long. new ConvertiblePair(Id.class, UUID.class) imposes a conflicting declaration and only one variant can win, therefore you see various incarnations of ConversionFailedException.

You could bypass the issue with annotating IdConverter with @ReadingConverter to just register converters.

The actual issue however is still present that Spring Data JDBC makes invalid assumptions regarding proper conversion in the code path that attempts to convert Id<?> into JdbcValue because it assumes Id is an entity type.

We need to fix that.

lukas-krecan commented 1 month ago

IMO the issue is not about confusion about variants, the core problem is that a different converter is used. I register Id<->UUID converter, but a standard String<->UUID converter is used (type argument of writeValue is basically ignored).

But if I understand you correctly, annotating the converter as @ReadingConverter would let us use the standard converters for the write path and our converter only for reading? Cool, I will try that. Thanks.

lukas-krecan commented 1 month ago

The workaround with ReadingConverter does not work due to

Caused by: java.lang.IllegalArgumentException: Cannot query by nested entity: userId
    at org.springframework.data.jdbc.repository.query.JdbcQueryCreator.validateProperty(JdbcQueryCreator.java:151)
    at java.base/java.lang.Iterable.forEach(Iterable.java:75)
    at org.springframework.data.jdbc.repository.query.JdbcQueryCreator.validate(JdbcQueryCreator.java:130)
    at org.springframework.data.jdbc.repository.query.PartTreeJdbcQuery.<init>(PartTreeJdbcQuery.java:114)
    at org.springframework.data.jdbc.repository.support.JdbcQueryLookupStrategy$CreateQueryLookupStrategy.resolveQuery(JdbcQueryLookupStrategy.java:124)
    at org.springframework.data.jdbc.repository.support.JdbcQueryLookupStrategy$CreateIfNotFoundQueryLookupStrategy.resolveQuery(JdbcQueryLookupStrategy.java:212)
    at org.springframework.data.repository.core.support.QueryExecutorMethodInterceptor.lookupQuery(QueryExecutorMethodInterceptor.java:111)