Skip to content

ConversionFailedException on generic Entity Id and custom converter #1842

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
lukas-krecan opened this issue Jul 26, 2024 · 3 comments
Closed
Assignees
Labels
type: bug A general bug

Comments

@lukas-krecan
Copy link

lukas-krecan commented Jul 26, 2024

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();
        }
    }
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 26, 2024
@mp911de mp911de self-assigned this Jul 29, 2024
@mp911de
Copy link
Member

mp911de commented Jul 30, 2024

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.

@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 30, 2024
@mp911de mp911de assigned schauder and unassigned mp911de Jul 30, 2024
@lukas-krecan
Copy link
Author

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
Copy link
Author

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)

@schauder schauder linked a pull request Sep 16, 2024 that will close this issue
schauder pushed a commit that referenced this issue Sep 16, 2024
schauder added a commit that referenced this issue Sep 16, 2024
Restructure test.
Remove redundant code.
Formatting.

Original pull request #1876
See #1842
schauder pushed a commit that referenced this issue Sep 16, 2024
schauder added a commit that referenced this issue Sep 16, 2024
Restructure test.
Remove redundant code.
Formatting.

Original pull request #1876
See #1842
schauder added a commit that referenced this issue Sep 16, 2024
Restructure test.
Remove redundant code.
Formatting.

Original pull request #1876
See #1842
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants