From 7a9dc37f808886a521f9ec63a34fb092bdc80454 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 10 Dec 2021 09:21:16 +0100 Subject: [PATCH 1/9] Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8c94b24960..6ddf2a9fc3 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.7.0-SNAPSHOT + 2.7.0-GH-1484-SNAPSHOT Spring Data Core From 9f732c1b5c83a1d74f5ed5535a35dad462e437b1 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 10 Dec 2021 12:18:54 +0100 Subject: [PATCH 2/9] first draft --- .../data/convert/PropertyConverter.java | 42 +++++++++++++++++++ .../data/convert/PropertyValueConverter.java | 40 ++++++++++++++++++ .../data/mapping/PersistentProperty.java | 12 ++++++ .../AnnotationBasedPersistentProperty.java | 34 +++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 src/main/java/org/springframework/data/convert/PropertyConverter.java create mode 100644 src/main/java/org/springframework/data/convert/PropertyValueConverter.java diff --git a/src/main/java/org/springframework/data/convert/PropertyConverter.java b/src/main/java/org/springframework/data/convert/PropertyConverter.java new file mode 100644 index 0000000000..dfde48a678 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyConverter.java @@ -0,0 +1,42 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import static java.lang.annotation.ElementType.*; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +import org.springframework.data.convert.PropertyValueConverter.ObjectToObjectPropertyValueConverter; + +/** + * Annotation to define usage of a {@link PropertyValueConverter} to read/write the property. + *

+ * Consult the store specific documentation for details and support notes. + * + * @author Christoph Strobl + * @since 2.7 + */ +@Target(FIELD) +@Documented +@Retention(RetentionPolicy.RUNTIME) +public @interface PropertyConverter { + + Class> value() default ObjectToObjectPropertyValueConverter.class; + +} diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java new file mode 100644 index 0000000000..973fe06ff2 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -0,0 +1,40 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +/** + * @author Christoph Strobl + * @since 2.7 + */ +public interface PropertyValueConverter { + + T read(S value); + + S write(T value); + + class ObjectToObjectPropertyValueConverter implements PropertyValueConverter { + + @Override + public Object read(Object value) { + return value; + } + + @Override + public Object write(Object value) { + return value; + } + } +} diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 69ac12a9eb..78aec58a33 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -23,6 +23,8 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; +import org.springframework.data.convert.PropertyConverter; +import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -433,4 +435,14 @@ default PersistentPropertyAccessor getAccessorForOwner(T owner) { return getOwner().getPropertyAccessor(owner); } + + @Nullable + default Class> getValueConverterType() { + PropertyConverter annotation = findAnnotation(PropertyConverter.class); + return annotation == null ? null : annotation.value(); + } + + default boolean hasValueConverter() { + return isAnnotationPresent(PropertyConverter.class); + } } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index f44fd5d48e..e48132eac3 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -23,8 +23,11 @@ import java.util.stream.Collectors; import java.util.stream.Stream; +import org.springframework.beans.BeanUtils; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.data.annotation.AccessType; import org.springframework.data.annotation.AccessType.Type; @@ -33,6 +36,8 @@ import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; import org.springframework.data.annotation.Version; +import org.springframework.data.convert.PropertyConverter; +import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; @@ -308,6 +313,35 @@ public TypeInformation getAssociationTargetTypeInformation() { return associationTargetType.getNullable(); } + @Nullable + @Override + public Class> getValueConverterType() { + + return doFindAnnotation(PropertyConverter.class) // + .map(PropertyConverter::value) // + .orElse(null); + } + + protected PropertyValueConverter resolveConverter(@Nullable BeanFactory beanFactory) { + + // TODO: caching + + if(!hasValueConverter()) { + return null; + } + + Class> target = getValueConverterType(); + if(beanFactory == null) { + return BeanUtils.instantiateClass(target); + } + + if(beanFactory instanceof AutowireCapableBeanFactory) { + return (PropertyValueConverter) ((AutowireCapableBeanFactory)beanFactory).createBean(target, AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); + } + + return beanFactory.getBean(target); + } + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.AbstractPersistentProperty#toString() From 817e3eaf17ec2449844b94495ccff7c36efe8cc1 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 10 Dec 2021 15:23:09 +0100 Subject: [PATCH 3/9] hacking - get the bean fractory from the spel context maybe? --- .../data/mapping/PersistentProperty.java | 3 + .../AnnotationBasedPersistentProperty.java | 44 +++++++++---- .../AbstractPersistentPropertyUnitTests.java | 10 +++ ...ationBasedPersistentPropertyUnitTests.java | 65 +++++++++++++++++++ 4 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 78aec58a33..4e91000415 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -442,6 +442,9 @@ default Class> getValueConverterType() { return annotation == null ? null : annotation.value(); } + @Nullable + PropertyValueConverter getValueConverter(); + default boolean hasValueConverter() { return isAnnotationPresent(PropertyConverter.class); } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index e48132eac3..4ffc183cbc 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -17,6 +17,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Field; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -28,6 +29,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.context.expression.BeanFactoryResolver; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.data.annotation.AccessType; import org.springframework.data.annotation.AccessType.Type; @@ -48,6 +50,7 @@ import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.StreamUtils; import org.springframework.data.util.TypeInformation; +import org.springframework.expression.BeanResolver; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -97,6 +100,22 @@ public abstract class AnnotationBasedPersistentProperty

super.getAssociationTargetTypeInformation()); }); + private final Lazy> propertyValueConverter = Lazy.of(() -> { + + PersistentEntity owner = getOwner(); + if (owner instanceof BasicPersistentEntity) { + + BeanResolver beanResolver = ((BasicPersistentEntity) owner).getEvaluationContext(null).getBeanResolver(); + if (beanResolver != null) { + Field beanFactory = org.springframework.util.ReflectionUtils.findField(BeanFactoryResolver.class, "beanFactory"); + org.springframework.util.ReflectionUtils.makeAccessible(beanFactory); + BeanFactory field = (BeanFactory) org.springframework.util.ReflectionUtils.getField(beanFactory, beanResolver); + return resolveConverter(field); + } + } + return resolveConverter(null); + }); + /** * Creates a new {@link AnnotationBasedPersistentProperty}. * @@ -137,8 +156,7 @@ private void populateAnnotationCache(Property property) { + "multiple times on accessor methods of property %s in class %s!", annotationType.getSimpleName(), getName(), getOwner().getType().getSimpleName()); - annotationCache.put(annotationType, - Optional.of(mergedAnnotation)); + annotationCache.put(annotationType, Optional.of(mergedAnnotation)); } }); @@ -153,8 +171,7 @@ private void populateAnnotationCache(Property property) { "Ambiguous mapping! Annotation %s configured " + "on field %s and one of its accessor methods in class %s!", annotationType.getSimpleName(), it.getName(), getOwner().getType().getSimpleName()); - annotationCache.put(annotationType, - Optional.of(mergedAnnotation)); + annotationCache.put(annotationType, Optional.of(mergedAnnotation)); } }); } @@ -322,21 +339,25 @@ public TypeInformation getAssociationTargetTypeInformation() { .orElse(null); } - protected PropertyValueConverter resolveConverter(@Nullable BeanFactory beanFactory) { + public PropertyValueConverter getValueConverter() { + return propertyValueConverter.get(); + } + + protected PropertyValueConverter resolveConverter(@Nullable BeanFactory beanFactory) { // TODO: caching - - if(!hasValueConverter()) { + if (!hasValueConverter()) { return null; } Class> target = getValueConverterType(); - if(beanFactory == null) { + if (beanFactory == null) { return BeanUtils.instantiateClass(target); } - if(beanFactory instanceof AutowireCapableBeanFactory) { - return (PropertyValueConverter) ((AutowireCapableBeanFactory)beanFactory).createBean(target, AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); + if (beanFactory instanceof AutowireCapableBeanFactory) { + return (PropertyValueConverter) ((AutowireCapableBeanFactory) beanFactory).createBean(target, + AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); } return beanFactory.getBean(target); @@ -377,8 +398,7 @@ private Stream getAccessors() { @SuppressWarnings("unchecked") private static Class loadIdentityType() { - return (Class) ReflectionUtils.loadIfPresent( - "org.jmolecules.ddd.annotation.Identity", + return (Class) ReflectionUtils.loadIfPresent("org.jmolecules.ddd.annotation.Identity", AbstractPersistentProperty.class.getClassLoader()); } } diff --git a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java index 67dfb19065..4266561ec5 100755 --- a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java @@ -36,6 +36,7 @@ import org.jmolecules.ddd.types.Identifier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -43,6 +44,7 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.Optionals; import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; /** @@ -381,10 +383,18 @@ public boolean isAnnotationPresent(Class annotationType) { return false; } + @Nullable + @Override + public PropertyValueConverter getValueConverter() { + return null; + } + @Override public A findPropertyOrOwnerAnnotation(Class annotationType) { return null; } + + } static class Sample { diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 0185b2780b..e848f7bf6d 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -36,6 +36,8 @@ import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType; @@ -44,6 +46,8 @@ import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; +import org.springframework.data.convert.PropertyConverter; +import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SampleMappingContext; @@ -338,6 +342,22 @@ void detectsJMoleculesIdentity() { assertThat(property.isIdProperty()).isTrue(); } + @Test // GH-1484 + void xxx() { + + GenericApplicationContext applicationContext = new GenericApplicationContext(); + applicationContext.refresh(); +// applicationContext.registerBean(); + + context.setApplicationContext(applicationContext); + + SamplePersistentProperty property = getProperty(WithPropertyConverter.class, "value"); + PropertyValueConverter valueConverter = property.getValueConverter(); + + assertThat(valueConverter).isInstanceOf(MyPropertyConverter.class); + } + + @SuppressWarnings("unchecked") private Map, Annotation> getAnnotationCache(SamplePersistentProperty property) { return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); @@ -529,4 +549,49 @@ static class JMolecules { } interface JMoleculesAggregate extends AggregateRoot {} + + static class WithPropertyConverter { + + @PropertyConverter(MyPropertyConverter.class) + String value; + + @PropertyConverter(MyPropertyConverterThatRequiresComponents.class) + String value2; + } + + static class MyPropertyConverter implements PropertyValueConverter { + + @Override + public Object read(Object value) { + return null; + } + + @Override + public Object write(Object value) { + return null; + } + } + + static class MyPropertyConverterThatRequiresComponents implements PropertyValueConverter { + + private final SomeDependency someDependency; + + public MyPropertyConverterThatRequiresComponents(@Autowired SomeDependency someDependency) { + this.someDependency = someDependency; + } + + @Override + public Object read(Object value) { + return null; + } + + @Override + public Object write(Object value) { + return null; + } + } + + static class SomeDependency { + + } } From db2f1515ae2a54c5bc72b320bcbccecdef3980f1 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 13 Dec 2021 19:16:52 +0100 Subject: [PATCH 4/9] move the conversion out of the mapping package --- ...oryAwarePropertyValueConverterFactory.java | 52 +++++++++++ .../CachingPropertyValueConverterFactory.java | 84 ++++++++++++++++++ .../data/convert/CustomConversions.java | 24 +++++ .../data/convert/PropertyValueConverter.java | 38 ++++++-- .../PropertyValueConverterFactory.java | 88 +++++++++++++++++++ .../SimplePropertyConverterFactory.java | 40 +++++++++ .../data/mapping/PersistentProperty.java | 3 - .../AnnotationBasedPersistentProperty.java | 40 --------- .../AbstractPersistentPropertyUnitTests.java | 6 -- ...ationBasedPersistentPropertyUnitTests.java | 24 +---- 10 files changed, 324 insertions(+), 75 deletions(-) create mode 100644 src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java create mode 100644 src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java create mode 100644 src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java create mode 100644 src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java diff --git a/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java new file mode 100644 index 0000000000..4892b63ab3 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java @@ -0,0 +1,52 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.util.Assert; + +/** + * {@link PropertyValueConverterFactory} implementation that leverages the {@link BeanFactory} to create the desired + * {@link PropertyValueConverter}. This allows the {@link PropertyValueConverter} to make use of DI. + * + * @author Christoph Strobl + */ +public class BeanFactoryAwarePropertyValueConverterFactory implements PropertyValueConverterFactory, BeanFactoryAware { + + private BeanFactory beanFactory; + + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + this.beanFactory = beanFactory; + } + + @Override + public PropertyValueConverter getConverter(Class> converterType) { + + Assert.state(beanFactory != null, "BeanFactory must not be null. Did you forget to set it!"); + Assert.notNull(converterType, "ConverterType must not be null!"); + + if (beanFactory instanceof AutowireCapableBeanFactory) { + return (PropertyValueConverter) ((AutowireCapableBeanFactory) beanFactory).createBean(converterType, + AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); + } + + return beanFactory.getBean(converterType); + } +} diff --git a/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java new file mode 100644 index 0000000000..728136c319 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java @@ -0,0 +1,84 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.HashMap; +import java.util.Map; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * TODO: This would be easier if we'd get rid of {@link PropertyValueConverterFactory#getConverter(Class)}. + * + * @author Christoph Strobl + * @since 2021/12 + */ +class CachingPropertyValueConverterFactory implements PropertyValueConverterFactory { + + private final PropertyValueConverterFactory delegate; + private Cache cache = new Cache(); + + public CachingPropertyValueConverterFactory(PropertyValueConverterFactory delegate) { + this.delegate = delegate; + } + + @Nullable + @Override + public PropertyValueConverter getConverter(PersistentProperty property) { + + PropertyValueConverter converter = cache.get(property); + if (converter != null) { + return converter; + } + return cache.cache(property, delegate.getConverter(property)); + } + + @Override + public PropertyValueConverter getConverter(Class> converterType) { + + PropertyValueConverter converter = cache.get(converterType); + if (converter != null) { + return converter; + } + return cache.cache(converterType, delegate.getConverter(converterType)); + } + + static class Cache { + + Map, PropertyValueConverter> perPropertyCache = new HashMap<>(); + Map, PropertyValueConverter> typeCache = new HashMap<>(); + + PropertyValueConverter get(PersistentProperty property) { + return perPropertyCache.get(property); + } + + PropertyValueConverter get(Class type) { + return typeCache.get(type); + } + + PropertyValueConverter cache(PersistentProperty property, PropertyValueConverter converter) { + perPropertyCache.putIfAbsent(property, converter); + cache(property.getValueConverterType(), converter); + return converter; + } + + PropertyValueConverter cache(Class type, PropertyValueConverter converter) { + typeCache.putIfAbsent(type, converter); + return converter; + } + } +} diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index 6a97eb0cb9..f68723c6f9 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -98,6 +98,8 @@ public class CustomConversions { private final Function> getRawWriteTarget = convertiblePair -> getCustomTarget( convertiblePair.getSourceType(), null, writingPairs); + private final PropertyValueConverterFactory propertyValueConverterFactory; + /** * @param converterConfiguration the {@link ConverterConfiguration} to apply. * @since 2.3 @@ -120,6 +122,11 @@ public CustomConversions(ConverterConfiguration converterConfiguration) { this.converters = Collections.unmodifiableList(registeredConverters); this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, converterConfiguration.getStoreConversions().getStoreTypeHolder()); + if(converterConfiguration.getPropertyConverterFactory() != null) { + this.propertyValueConverterFactory = converterConfiguration.getPropertyConverterFactory(); + } else { + this.propertyValueConverterFactory = new CachingPropertyValueConverterFactory(new SimplePropertyConverterFactory()); + } } /** @@ -440,6 +447,10 @@ private Class getCustomTarget(Class sourceType, @Nullable Class targetT return null; } + public PropertyValueConverterFactory getPropertyValueConverterFactory() { + return propertyValueConverterFactory; + } + private static boolean hasAssignableSourceType(ConvertiblePair pair, Class sourceType) { return pair.getSourceType().isAssignableFrom(sourceType); } @@ -877,6 +888,7 @@ protected static class ConverterConfiguration { private final StoreConversions storeConversions; private final List userConverters; private final Predicate converterRegistrationFilter; + private final PropertyValueConverterFactory propertyConverterFactory; /** * Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters. @@ -902,9 +914,16 @@ public ConverterConfiguration(StoreConversions storeConversions, List userCon public ConverterConfiguration(StoreConversions storeConversions, List userConverters, Predicate converterRegistrationFilter) { + this(storeConversions, userConverters, converterRegistrationFilter, null); + } + + public ConverterConfiguration(StoreConversions storeConversions, List userConverters, + Predicate converterRegistrationFilter, @Nullable PropertyValueConverterFactory propertyConverterFactory) { + this.storeConversions = storeConversions; this.userConverters = new ArrayList<>(userConverters); this.converterRegistrationFilter = converterRegistrationFilter; + this.propertyConverterFactory = propertyConverterFactory; } /** @@ -927,5 +946,10 @@ List getUserConverters() { boolean shouldRegister(ConvertiblePair candidate) { return this.converterRegistrationFilter.test(candidate); } + + @Nullable + public PropertyValueConverterFactory getPropertyConverterFactory() { + return propertyConverterFactory; + } } } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java index 973fe06ff2..edd8e017a5 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -15,25 +15,51 @@ */ package org.springframework.data.convert; +import org.springframework.lang.Nullable; + /** + * {@link PropertyValueConverter} provides a symmetric way of converting certain properties from domain to store + * specific values. + *

+ * A {@link PropertyValueConverter} is, other than a {@link ReadingConverter} or {@link WritingConverter}, only applied + * to special annotated fields which allows a fine grained conversion of certain values within a specific context. + * * @author Christoph Strobl + * @param domain specific type + * @param store native type * @since 2.7 */ -public interface PropertyValueConverter { +public interface PropertyValueConverter { + + /** + * Convert the given store specific value into it's domain value representation. + * + * @param nativeValue can be {@literal null}. + * @return the converted value. Can be {@literal null}. + */ + @Nullable + A /*read*/nativeToDomain(@Nullable B nativeValue); - T read(S value); + /** + * Convert the given domain specific value into it's native store representation. + * + * @param domainValue can be {@literal null}. + * @return the converted value. Can be {@literal null}. + */ + @Nullable + B /*write*/domainToNative(@Nullable A domainValue); - S write(T value); + enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { - class ObjectToObjectPropertyValueConverter implements PropertyValueConverter { + INSTANCE; @Override - public Object read(Object value) { + public Object nativeToDomain(Object value) { return value; } @Override - public Object write(Object value) { + public Object domainToNative(Object value) { return value; } } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java new file mode 100644 index 0000000000..d13faadd7c --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java @@ -0,0 +1,88 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.function.Supplier; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + */ +public interface PropertyValueConverterFactory { + + /** + * Get the {@link PropertyValueConverter} applicable for the given {@link PersistentProperty}. + * + * @param property must not be {@literal null}. + * @param domain specific type + * @param store native type + * @return can be {@literal null}. + */ + @Nullable + default PropertyValueConverter getConverter(PersistentProperty property) { + if (!property.hasValueConverter()) { + return null; + } + return getConverter((Class>) property.getValueConverterType()); + } + + /** + * Get the {@link PropertyValueConverter} applicable for the given {@link PersistentProperty} or return a fallback + * converter via the given {@link Supplier}. + * + * @param property must not be {@literal null}. + * @param domain specific type + * @param store native type + * @return can be {@literal null}. + */ + default PropertyValueConverter getConverter(PersistentProperty property, + Supplier> fallback) { + PropertyValueConverter resolved = getConverter(property); + return resolved != null ? resolved : fallback.get(); + } + + // TODO: do we actually need this one or should we only rely on 'resolve(PersistentProperty property)' + PropertyValueConverter getConverter(Class> converterType); + + // TODO: do we actually need this one or should we only rely on 'resolve(PersistentProperty property)' + default PropertyValueConverter getConverter(Class> converterType, + Supplier> fallback) { + PropertyValueConverter resolved = getConverter(converterType); + return resolved != null ? resolved : fallback.get(); + } + + /** + * Obtain a {@link PropertyValueConverterFactory} that creates {@link PropertyValueConverter converters} via their + * default constructor. + * + * @return new instance of {@link PropertyValueConverterFactory}. + */ + static PropertyValueConverterFactory simple() { + return new SimplePropertyConverterFactory(); + } + + /** + * Obtain a {@link PropertyValueConverterFactory} that caches {@link PropertyValueConverter converters} created by the + * given {@link PropertyValueConverterFactory factory}. + * + * @return new instance of {@link PropertyValueConverterFactory}. + */ + static PropertyValueConverterFactory caching(PropertyValueConverterFactory factory) { + return new CachingPropertyValueConverterFactory(factory); + } +} diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java b/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java new file mode 100644 index 0000000000..f9f5b849a8 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java @@ -0,0 +1,40 @@ +/* + * Copyright 2021 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.EnumSet; + +import org.springframework.beans.BeanUtils; +import org.springframework.util.Assert; + +/** + * Trivial implementation of {@link PropertyValueConverter}. + * + * @author Christoph Strobl + */ +class SimplePropertyConverterFactory implements PropertyValueConverterFactory { + + @Override + public PropertyValueConverter getConverter(Class> converterType) { + + Assert.notNull(converterType, "ConverterType must not be null!"); + + if (converterType.isEnum()) { + return (PropertyValueConverter) EnumSet.allOf((Class) converterType).iterator().next(); + } + return BeanUtils.instantiateClass(converterType); + } +} diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 4e91000415..78aec58a33 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -442,9 +442,6 @@ default Class> getValueConverterType() { return annotation == null ? null : annotation.value(); } - @Nullable - PropertyValueConverter getValueConverter(); - default boolean hasValueConverter() { return isAnnotationPresent(PropertyConverter.class); } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 4ffc183cbc..a6483cf0bd 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -100,22 +100,6 @@ public abstract class AnnotationBasedPersistentProperty

super.getAssociationTargetTypeInformation()); }); - private final Lazy> propertyValueConverter = Lazy.of(() -> { - - PersistentEntity owner = getOwner(); - if (owner instanceof BasicPersistentEntity) { - - BeanResolver beanResolver = ((BasicPersistentEntity) owner).getEvaluationContext(null).getBeanResolver(); - if (beanResolver != null) { - Field beanFactory = org.springframework.util.ReflectionUtils.findField(BeanFactoryResolver.class, "beanFactory"); - org.springframework.util.ReflectionUtils.makeAccessible(beanFactory); - BeanFactory field = (BeanFactory) org.springframework.util.ReflectionUtils.getField(beanFactory, beanResolver); - return resolveConverter(field); - } - } - return resolveConverter(null); - }); - /** * Creates a new {@link AnnotationBasedPersistentProperty}. * @@ -339,30 +323,6 @@ public TypeInformation getAssociationTargetTypeInformation() { .orElse(null); } - public PropertyValueConverter getValueConverter() { - return propertyValueConverter.get(); - } - - protected PropertyValueConverter resolveConverter(@Nullable BeanFactory beanFactory) { - - // TODO: caching - if (!hasValueConverter()) { - return null; - } - - Class> target = getValueConverterType(); - if (beanFactory == null) { - return BeanUtils.instantiateClass(target); - } - - if (beanFactory instanceof AutowireCapableBeanFactory) { - return (PropertyValueConverter) ((AutowireCapableBeanFactory) beanFactory).createBean(target, - AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); - } - - return beanFactory.getBean(target); - } - /* * (non-Javadoc) * @see org.springframework.data.mapping.model.AbstractPersistentProperty#toString() diff --git a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java index 4266561ec5..d15a5ccb14 100755 --- a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java @@ -383,12 +383,6 @@ public boolean isAnnotationPresent(Class annotationType) { return false; } - @Nullable - @Override - public PropertyValueConverter getValueConverter() { - return null; - } - @Override public A findPropertyOrOwnerAnnotation(Class annotationType) { return null; diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index e848f7bf6d..53c373bc57 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -342,22 +342,6 @@ void detectsJMoleculesIdentity() { assertThat(property.isIdProperty()).isTrue(); } - @Test // GH-1484 - void xxx() { - - GenericApplicationContext applicationContext = new GenericApplicationContext(); - applicationContext.refresh(); -// applicationContext.registerBean(); - - context.setApplicationContext(applicationContext); - - SamplePersistentProperty property = getProperty(WithPropertyConverter.class, "value"); - PropertyValueConverter valueConverter = property.getValueConverter(); - - assertThat(valueConverter).isInstanceOf(MyPropertyConverter.class); - } - - @SuppressWarnings("unchecked") private Map, Annotation> getAnnotationCache(SamplePersistentProperty property) { return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); @@ -562,12 +546,12 @@ static class WithPropertyConverter { static class MyPropertyConverter implements PropertyValueConverter { @Override - public Object read(Object value) { + public Object nativeToDomain(Object value) { return null; } @Override - public Object write(Object value) { + public Object domainToNative(Object value) { return null; } } @@ -581,12 +565,12 @@ public MyPropertyConverterThatRequiresComponents(@Autowired SomeDependency someD } @Override - public Object read(Object value) { + public Object nativeToDomain(Object value) { return null; } @Override - public Object write(Object value) { + public Object domainToNative(Object value) { return null; } } From c9f07863e4d2a91eaef867d84e4ce8744e80b385 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 19 Jan 2022 13:17:01 +0100 Subject: [PATCH 5/9] Introduce ValueConversionContext and PropertyValueConversions --- ...oryAwarePropertyValueConverterFactory.java | 52 ---- .../CachingPropertyValueConverterFactory.java | 84 ------ .../data/convert/CustomConversions.java | 59 +++-- .../data/convert/PropertyConverter.java | 11 +- .../convert/PropertyValueConversions.java | 54 ++++ .../data/convert/PropertyValueConverter.java | 35 ++- .../PropertyValueConverterFactories.java | 228 +++++++++++++++++ .../PropertyValueConverterFactory.java | 103 ++++++-- .../PropertyValueConverterRegistrar.java | 99 ++++++++ .../SimplePropertyConverterFactory.java | 40 --- .../SimplePropertyValueConversions.java | 89 +++++++ .../data/mapping/PersistentProperty.java | 3 +- .../AnnotationBasedPersistentProperty.java | 9 +- .../convert/CustomConversionsUnitTests.java | 20 ++ ...ropertyValueConverterFactoryUnitTests.java | 239 ++++++++++++++++++ .../data/convert/WhatWeWant.java | 90 +++++++ ...ationBasedPersistentPropertyUnitTests.java | 14 +- 17 files changed, 984 insertions(+), 245 deletions(-) delete mode 100644 src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java delete mode 100644 src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java create mode 100644 src/main/java/org/springframework/data/convert/PropertyValueConversions.java create mode 100644 src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java create mode 100644 src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java delete mode 100644 src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java create mode 100644 src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java create mode 100644 src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java create mode 100644 src/test/java/org/springframework/data/convert/WhatWeWant.java diff --git a/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java deleted file mode 100644 index 4892b63ab3..0000000000 --- a/src/main/java/org/springframework/data/convert/BeanFactoryAwarePropertyValueConverterFactory.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright 2021 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.convert; - -import org.springframework.beans.BeansException; -import org.springframework.beans.factory.BeanFactory; -import org.springframework.beans.factory.BeanFactoryAware; -import org.springframework.beans.factory.config.AutowireCapableBeanFactory; -import org.springframework.util.Assert; - -/** - * {@link PropertyValueConverterFactory} implementation that leverages the {@link BeanFactory} to create the desired - * {@link PropertyValueConverter}. This allows the {@link PropertyValueConverter} to make use of DI. - * - * @author Christoph Strobl - */ -public class BeanFactoryAwarePropertyValueConverterFactory implements PropertyValueConverterFactory, BeanFactoryAware { - - private BeanFactory beanFactory; - - @Override - public void setBeanFactory(BeanFactory beanFactory) throws BeansException { - this.beanFactory = beanFactory; - } - - @Override - public PropertyValueConverter getConverter(Class> converterType) { - - Assert.state(beanFactory != null, "BeanFactory must not be null. Did you forget to set it!"); - Assert.notNull(converterType, "ConverterType must not be null!"); - - if (beanFactory instanceof AutowireCapableBeanFactory) { - return (PropertyValueConverter) ((AutowireCapableBeanFactory) beanFactory).createBean(converterType, - AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); - } - - return beanFactory.getBean(converterType); - } -} diff --git a/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java deleted file mode 100644 index 728136c319..0000000000 --- a/src/main/java/org/springframework/data/convert/CachingPropertyValueConverterFactory.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * Copyright 2021 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.convert; - -import java.util.HashMap; -import java.util.Map; - -import org.springframework.data.mapping.PersistentProperty; -import org.springframework.lang.Nullable; - -/** - * TODO: This would be easier if we'd get rid of {@link PropertyValueConverterFactory#getConverter(Class)}. - * - * @author Christoph Strobl - * @since 2021/12 - */ -class CachingPropertyValueConverterFactory implements PropertyValueConverterFactory { - - private final PropertyValueConverterFactory delegate; - private Cache cache = new Cache(); - - public CachingPropertyValueConverterFactory(PropertyValueConverterFactory delegate) { - this.delegate = delegate; - } - - @Nullable - @Override - public PropertyValueConverter getConverter(PersistentProperty property) { - - PropertyValueConverter converter = cache.get(property); - if (converter != null) { - return converter; - } - return cache.cache(property, delegate.getConverter(property)); - } - - @Override - public PropertyValueConverter getConverter(Class> converterType) { - - PropertyValueConverter converter = cache.get(converterType); - if (converter != null) { - return converter; - } - return cache.cache(converterType, delegate.getConverter(converterType)); - } - - static class Cache { - - Map, PropertyValueConverter> perPropertyCache = new HashMap<>(); - Map, PropertyValueConverter> typeCache = new HashMap<>(); - - PropertyValueConverter get(PersistentProperty property) { - return perPropertyCache.get(property); - } - - PropertyValueConverter get(Class type) { - return typeCache.get(type); - } - - PropertyValueConverter cache(PersistentProperty property, PropertyValueConverter converter) { - perPropertyCache.putIfAbsent(property, converter); - cache(property.getValueConverterType(), converter); - return converter; - } - - PropertyValueConverter cache(Class type, PropertyValueConverter converter) { - typeCache.putIfAbsent(type, converter); - return converter; - } - } -} diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index f68723c6f9..54f8d12c3b 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -33,6 +33,7 @@ import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.ConverterBuilder.ConverterAware; +import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.Predicates; import org.springframework.data.util.Streamable; @@ -98,7 +99,7 @@ public class CustomConversions { private final Function> getRawWriteTarget = convertiblePair -> getCustomTarget( convertiblePair.getSourceType(), null, writingPairs); - private final PropertyValueConverterFactory propertyValueConverterFactory; + private PropertyValueConversions propertyValueConversions; /** * @param converterConfiguration the {@link ConverterConfiguration} to apply. @@ -122,11 +123,7 @@ public CustomConversions(ConverterConfiguration converterConfiguration) { this.converters = Collections.unmodifiableList(registeredConverters); this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, converterConfiguration.getStoreConversions().getStoreTypeHolder()); - if(converterConfiguration.getPropertyConverterFactory() != null) { - this.propertyValueConverterFactory = converterConfiguration.getPropertyConverterFactory(); - } else { - this.propertyValueConverterFactory = new CachingPropertyValueConverterFactory(new SimplePropertyConverterFactory()); - } + this.propertyValueConversions = converterConfiguration.getPropertyValueConversions(); } /** @@ -179,6 +176,36 @@ public void registerConvertersIn(ConverterRegistry conversionService) { VavrCollectionConverters.getConvertersToRegister().forEach(it -> registerConverterIn(it, conversionService)); } + /** + * Delegate check if a {@link PropertyValueConverter} for the given {@literal property} is present via + * {@link PropertyValueConversions}. + * + * @param property must not be {@literal null}. + * @return {@literal true} if a specific {@link PropertyValueConverter} is available. + * @see PropertyValueConversions#hasValueConverter(PersistentProperty) + * @since ? + */ + public boolean hasPropertyValueConverter(PersistentProperty property) { + return propertyValueConversions != null ? propertyValueConversions.hasValueConverter(property) : false; + } + + /** + * Delegate to obtain the {@link PropertyValueConverter} for the given {@literal property} from + * {@link PropertyValueConversions}. + * + * @param property must not be {@literal null}. param domain specific type + * @param store native type + * @param conversion context type + * @return the suitable {@link PropertyValueConverter} or {@literal null} if none available. + * @see PropertyValueConversions#getValueConverter(PersistentProperty) + * @since ? + */ + @Nullable + public PropertyValueConverter getPropertyValueConverter( + PersistentProperty property) { + return propertyValueConversions != null ? propertyValueConversions.getValueConverter(property) : null; + } + /** * Get all converters and add origin information * @@ -447,10 +474,6 @@ private Class getCustomTarget(Class sourceType, @Nullable Class targetT return null; } - public PropertyValueConverterFactory getPropertyValueConverterFactory() { - return propertyValueConverterFactory; - } - private static boolean hasAssignableSourceType(ConvertiblePair pair, Class sourceType) { return pair.getSourceType().isAssignableFrom(sourceType); } @@ -888,7 +911,7 @@ protected static class ConverterConfiguration { private final StoreConversions storeConversions; private final List userConverters; private final Predicate converterRegistrationFilter; - private final PropertyValueConverterFactory propertyConverterFactory; + private final PropertyValueConversions propertyValueConversions; /** * Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters. @@ -914,16 +937,16 @@ public ConverterConfiguration(StoreConversions storeConversions, List userCon public ConverterConfiguration(StoreConversions storeConversions, List userConverters, Predicate converterRegistrationFilter) { - this(storeConversions, userConverters, converterRegistrationFilter, null); + this(storeConversions, userConverters, converterRegistrationFilter, new SimplePropertyValueConversions()); } public ConverterConfiguration(StoreConversions storeConversions, List userConverters, - Predicate converterRegistrationFilter, @Nullable PropertyValueConverterFactory propertyConverterFactory) { + Predicate converterRegistrationFilter, @Nullable PropertyValueConversions propertyValueConversions) { this.storeConversions = storeConversions; this.userConverters = new ArrayList<>(userConverters); this.converterRegistrationFilter = converterRegistrationFilter; - this.propertyConverterFactory = propertyConverterFactory; + this.propertyValueConversions = propertyValueConversions; } /** @@ -947,9 +970,13 @@ boolean shouldRegister(ConvertiblePair candidate) { return this.converterRegistrationFilter.test(candidate); } + /** + * @return the configured {@link PropertyValueConversions} if set, {@literal null} otherwise. + * @since ? + */ @Nullable - public PropertyValueConverterFactory getPropertyConverterFactory() { - return propertyConverterFactory; + public PropertyValueConversions getPropertyValueConversions() { + return this.propertyValueConversions; } } } diff --git a/src/main/java/org/springframework/data/convert/PropertyConverter.java b/src/main/java/org/springframework/data/convert/PropertyConverter.java index dfde48a678..43fc76d35a 100644 --- a/src/main/java/org/springframework/data/convert/PropertyConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -30,13 +30,18 @@ * Consult the store specific documentation for details and support notes. * * @author Christoph Strobl - * @since 2.7 + * @since ? */ @Target(FIELD) @Documented @Retention(RetentionPolicy.RUNTIME) public @interface PropertyConverter { - Class> value() default ObjectToObjectPropertyValueConverter.class; + /** + * The {@link PropertyValueConverter} type handling the value conversion of the annotated property. + * + * @return the configured {@link PropertyValueConverter}. {@link ObjectToObjectPropertyValueConverter} by default. + */ + Class> value() default ObjectToObjectPropertyValueConverter.class; } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConversions.java b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java new file mode 100644 index 0000000000..8a8a42056d --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java @@ -0,0 +1,54 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * {@link PropertyValueConversions} provides access to {@link PropertyValueConverter converters} that may only be + * applied to a specific property. Other than {@link org.springframework.core.convert.converter.Converter converters} + * registered in {@link CustomConversions} the property based variants accept and allow returning {@literal null} values + * and provide access to a store specific {@link PropertyValueConverter.ValueConversionContext conversion context}. + * + * @author Christoph Strobl + * @since ? + * @currentBook The Desert Prince - Peter V. Brett + */ +public interface PropertyValueConversions { + + /** + * Check if a {@link PropertyValueConverter} is present for the given {@literal property}. + * + * @param property must not be {@literal null}. + * @return {@literal true} if a specific {@link PropertyValueConverter} is available. + */ + default boolean hasValueConverter(PersistentProperty property) { + return getValueConverter(property) != null; + } + + /** + * Get the {@link PropertyValueConverter} for the given {@literal property} if present. + * + * @param property must not be {@literal null}. param domain specific type + * @param store native type + * @param conversion context type + * @return the suitable {@link PropertyValueConverter} or {@literal null} if none available. + */ + @Nullable + PropertyValueConverter getValueConverter( + PersistentProperty property); +} diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java index edd8e017a5..ce9d8140fa 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -15,6 +15,7 @@ */ package org.springframework.data.convert; +import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; /** @@ -29,38 +30,56 @@ * @param store native type * @since 2.7 */ -public interface PropertyValueConverter { +public interface PropertyValueConverter { /** - * Convert the given store specific value into it's domain value representation. + * Convert the given store specific value into it's domain value representation. Typically a {@literal read} + * operation. * * @param nativeValue can be {@literal null}. + * @param context never {@literal null}. * @return the converted value. Can be {@literal null}. */ @Nullable - A /*read*/nativeToDomain(@Nullable B nativeValue); + A /*read*/ nativeToDomain(@Nullable B nativeValue, C context); /** - * Convert the given domain specific value into it's native store representation. + * Convert the given domain specific value into it's native store representation. Typically a {@literal write} + * operation. * * @param domainValue can be {@literal null}. + * @param context never {@literal null}. * @return the converted value. Can be {@literal null}. */ @Nullable - B /*write*/domainToNative(@Nullable A domainValue); + B /*write*/ domainToNative(@Nullable A domainValue, C context); - enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { + /** + * @author Christoph Strobl + */ + interface ValueConversionContext { + + PersistentProperty getProperty(); + } + + /** + * NoOp {@link PropertyValueConverter} implementation. + * + * @author Christoph Strobl + */ + enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { INSTANCE; @Override - public Object nativeToDomain(Object value) { + public Object nativeToDomain(Object value, ValueConversionContext context) { return value; } @Override - public Object domainToNative(Object value) { + public Object domainToNative(Object value, ValueConversionContext context) { return value; } } + } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java new file mode 100644 index 0000000000..1843e263c8 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java @@ -0,0 +1,228 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.Arrays; +import java.util.EnumSet; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import org.springframework.beans.BeanUtils; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; + +/** + * {@link PropertyValueConverterFactories} provides a collection of predefined {@link PropertyValueConverterFactory} + * implementations. Depending on the applications need {@link PropertyValueConverterFactory factories} can be + * {@link CompositePropertyValueConverterFactory chained} and the created {@link PropertyValueConverter converter} + * {@link CachingPropertyValueConverterFactory cached}. + * + * @author Christoph Strobl + * @since ? + */ +final class PropertyValueConverterFactories { + + /** + * @author Christoph Strobl + * @since ? + */ + static class CompositePropertyValueConverterFactory implements PropertyValueConverterFactory { + + private List delegates; + + CompositePropertyValueConverterFactory(PropertyValueConverterFactory... delegates) { + this(Arrays.asList(delegates)); + } + + CompositePropertyValueConverterFactory(List delegates) { + this.delegates = delegates; + } + + @Nullable + @Override + public PropertyValueConverter getConverter( + PersistentProperty property) { + return delegates.stream().map(it -> (PropertyValueConverter) it.getConverter(property)) + .filter(Objects::nonNull).findFirst().orElse(null); + } + + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + return delegates.stream().filter(it -> it.getConverter(converterType) != null).findFirst() + .map(it -> it.getConverter(converterType)).orElse(null); + } + } + + /** + * Trivial implementation of {@link PropertyValueConverter}. + * + * @author Christoph Strobl + * @since ? + */ + static class SimplePropertyConverterFactory implements PropertyValueConverterFactory { + + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + + Assert.notNull(converterType, "ConverterType must not be null!"); + + if (converterType.isEnum()) { + return (PropertyValueConverter) EnumSet.allOf((Class) converterType).iterator().next(); + } + return BeanUtils.instantiateClass(converterType); + } + } + + /** + * {@link PropertyValueConverterFactory} implementation that leverages the {@link BeanFactory} to create the desired + * {@link PropertyValueConverter}. This allows the {@link PropertyValueConverter} to make use of DI. + * + * @author Christoph Strobl + * @since ? + */ + static class BeanFactoryAwarePropertyValueConverterFactory implements PropertyValueConverterFactory { + + private final BeanFactory beanFactory; + + public BeanFactoryAwarePropertyValueConverterFactory(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + + Assert.state(beanFactory != null, "BeanFactory must not be null. Did you forget to set it!"); + Assert.notNull(converterType, "ConverterType must not be null!"); + + try { + return beanFactory.getBean(converterType); + } catch (NoSuchBeanDefinitionException exception) { + + if (beanFactory instanceof AutowireCapableBeanFactory) { + return (PropertyValueConverter) ((AutowireCapableBeanFactory) beanFactory).createBean(converterType, + AutowireCapableBeanFactory.AUTOWIRE_CONSTRUCTOR, false); + } + } + return null; + } + } + + /** + * @author Christoph Strobl + * @since ? + */ + static class ConfiguredInstanceServingValueConverterFactory implements PropertyValueConverterFactory { + + private final PropertyValueConverterRegistrar conversionsRegistrar; + + public ConfiguredInstanceServingValueConverterFactory(PropertyValueConverterRegistrar conversionsRegistrar) { + + Assert.notNull(conversionsRegistrar, "ConversionsRegistrar must not be null!"); + this.conversionsRegistrar = conversionsRegistrar; + } + + @Nullable + @Override + public PropertyValueConverter getConverter( + PersistentProperty property) { + return (PropertyValueConverter) conversionsRegistrar.getConverter(property.getOwner().getType(), + property.getName()); + } + + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + return null; + } + } + + /** + * TODO: This would be easier if we'd get rid of {@link PropertyValueConverterFactory#getConverter(Class)}. + * + * @author Christoph Strobl + * @since ? + */ + static class CachingPropertyValueConverterFactory implements PropertyValueConverterFactory { + + private final PropertyValueConverterFactory delegate; + private final Cache cache = new Cache(); + + public CachingPropertyValueConverterFactory(PropertyValueConverterFactory delegate) { + + Assert.notNull(delegate, "Delegate must not be null!"); + this.delegate = delegate; + } + + @Nullable + @Override + public PropertyValueConverter getConverter( + PersistentProperty property) { + + PropertyValueConverter converter = cache.get(property); + if (converter != null) { + return converter; + } + return cache.cache(property, delegate.getConverter(property)); + } + + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + + PropertyValueConverter converter = cache.get(converterType); + if (converter != null) { + return converter; + } + return cache.cache(converterType, delegate.getConverter(converterType)); + } + + static class Cache { + + Map, PropertyValueConverter> perPropertyCache = new HashMap<>(); + Map, PropertyValueConverter> typeCache = new HashMap<>(); + + PropertyValueConverter get(PersistentProperty property) { + return perPropertyCache.get(property); + } + + PropertyValueConverter get(Class type) { + return typeCache.get(type); + } + + PropertyValueConverter cache(PersistentProperty property, + PropertyValueConverter converter) { + perPropertyCache.putIfAbsent(property, converter); + cache(property.getValueConverterType(), converter); + return converter; + } + + PropertyValueConverter cache(Class type, + PropertyValueConverter converter) { + typeCache.putIfAbsent(type, converter); + return converter; + } + } + } +} diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java index d13faadd7c..bb5cf47a8b 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2021 the original author or authors. + * Copyright 2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,13 +15,28 @@ */ package org.springframework.data.convert; -import java.util.function.Supplier; +import java.util.Arrays; +import java.util.List; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.BeanFactoryAwarePropertyValueConverterFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.CachingPropertyValueConverterFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.CompositePropertyValueConverterFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.ConfiguredInstanceServingValueConverterFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.SimplePropertyConverterFactory; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; +import org.springframework.util.Assert; /** + * A factory that provides {@link PropertyValueConverter value converters}. + *

+ * Depending on the applications need {@link PropertyValueConverterFactory factories} can be {@link #chained(List) + * chained} and the resulting {@link PropertyValueConverter converter} may be + * {@link #caching(PropertyValueConverterFactory) cached}. + * * @author Christoph Strobl + * @since ? */ public interface PropertyValueConverterFactory { @@ -34,54 +49,88 @@ public interface PropertyValueConverterFactory { * @return can be {@literal null}. */ @Nullable - default PropertyValueConverter getConverter(PersistentProperty property) { + default PropertyValueConverter getConverter( + PersistentProperty property) { + if (!property.hasValueConverter()) { return null; } - return getConverter((Class>) property.getValueConverterType()); + return getConverter((Class>) property.getValueConverterType()); } + @Nullable + PropertyValueConverter getConverter( + Class> converterType); + /** - * Get the {@link PropertyValueConverter} applicable for the given {@link PersistentProperty} or return a fallback - * converter via the given {@link Supplier}. + * Obtain a simple {@link PropertyValueConverterFactory} capable of instantiating {@link PropertyValueConverter} + * implementations via their default {@link java.lang.reflect.Constructor} or in case of an {@link Enum} accessing the + * first enum value. * - * @param property must not be {@literal null}. - * @param domain specific type - * @param store native type - * @return can be {@literal null}. + * @return new instance of {@link PropertyValueConverterFactory}. */ - default PropertyValueConverter getConverter(PersistentProperty property, - Supplier> fallback) { - PropertyValueConverter resolved = getConverter(property); - return resolved != null ? resolved : fallback.get(); + static PropertyValueConverterFactory simple() { + return new SimplePropertyConverterFactory(); } - // TODO: do we actually need this one or should we only rely on 'resolve(PersistentProperty property)' - PropertyValueConverter getConverter(Class> converterType); + /** + * Obtain a {@link PropertyValueConverterFactory} capable of looking up/creating the {@link PropertyValueConverter} + * via the given {@link BeanFactory}. + * + * @param beanFactory must not be {@literal null}. + * @return new instance of {@link PropertyValueConverterFactory}. + */ + static PropertyValueConverterFactory beanFactoryAware(BeanFactory beanFactory) { + return new BeanFactoryAwarePropertyValueConverterFactory(beanFactory); + } - // TODO: do we actually need this one or should we only rely on 'resolve(PersistentProperty property)' - default PropertyValueConverter getConverter(Class> converterType, - Supplier> fallback) { - PropertyValueConverter resolved = getConverter(converterType); - return resolved != null ? resolved : fallback.get(); + /** + * Obtain a {@link PropertyValueConverterFactory} capable of looking up the {@link PropertyValueConverter} in the + * given {@link PropertyValueConverterRegistrar}. + * + * @param registrar must not be {@literal null}. + * @return new instance of {@link PropertyValueConverterFactory}. + */ + static PropertyValueConverterFactory configuredInstance(PropertyValueConverterRegistrar registrar) { + return new ConfiguredInstanceServingValueConverterFactory(registrar); } /** - * Obtain a {@link PropertyValueConverterFactory} that creates {@link PropertyValueConverter converters} via their - * default constructor. + * Obtain a {@link PropertyValueConverterFactory} that will try to obtain a {@link PropertyValueConverter} from the + * given array of {@link PropertyValueConverterFactory factories} by returning the first non {@literal null} one. * + * @param factories must not be {@literal null} nor contain {@literal null} values. * @return new instance of {@link PropertyValueConverterFactory}. */ - static PropertyValueConverterFactory simple() { - return new SimplePropertyConverterFactory(); + static PropertyValueConverterFactory chained(PropertyValueConverterFactory... factories) { + return chained(Arrays.asList(factories)); } /** - * Obtain a {@link PropertyValueConverterFactory} that caches {@link PropertyValueConverter converters} created by the - * given {@link PropertyValueConverterFactory factory}. + * Obtain a {@link PropertyValueConverterFactory} that will try to obtain a {@link PropertyValueConverter} from the + * given list of {@link PropertyValueConverterFactory factories} by returning the first non {@literal null} one. * + * @param factoryList must not be {@literal null} nor contain {@literal null} values. * @return new instance of {@link PropertyValueConverterFactory}. */ + static PropertyValueConverterFactory chained(List factoryList) { + + Assert.noNullElements(factoryList, "FactoryList must not contain null elements."); + + if (factoryList.size() == 1) { + return factoryList.iterator().next(); + } + + return new CompositePropertyValueConverterFactory(factoryList); + } + + /** + * Obtain a {@link PropertyValueConverterFactory} that will cache {@link PropertyValueConverter} instances per + * {@link PersistentProperty}. + * + * @param factory + * @return + */ static PropertyValueConverterFactory caching(PropertyValueConverterFactory factory) { return new CachingPropertyValueConverterFactory(factory); } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java new file mode 100644 index 0000000000..73d5888365 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java @@ -0,0 +1,99 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; + +/** + * @author Christoph Strobl + * @since ? + */ +public class PropertyValueConverterRegistrar { + + private final Map> converterRegistrationMap = new LinkedHashMap<>(); + + boolean hasConverterFor(Class type, String path) { + return converterRegistrationMap.containsKey(new Key(type, path)); + } + + @Nullable + public PropertyValueConverter getConverter(Class type, String path) { + return converterRegistrationMap.get(new Key(type, path)); + } + + public int size() { + return converterRegistrationMap.size(); + } + + public boolean isEmpty() { + return converterRegistrationMap.isEmpty(); + } + + public PropertyValueConverterRegistrar register(Class type, String path, + PropertyValueConverter converter) { + + converterRegistrationMap.put(new Key(type, path), converter); + return this; + } + + public Collection> converters() { + return converterRegistrationMap.values(); + } + + public PropertyValueConverterRegistrar registerIfAbsent(Class type, String path, + PropertyValueConverter converter) { + converterRegistrationMap.putIfAbsent(new Key(type, path), converter); + return this; + } + + static class Key { + + Class type; + String path; + + public Key(Class type, String path) { + this.type = type; + this.path = path; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + + Key key = (Key) o; + + if (!ObjectUtils.nullSafeEquals(type, key.type)) { + return false; + } + return ObjectUtils.nullSafeEquals(path, key.path); + } + + @Override + public int hashCode() { + int result = ObjectUtils.nullSafeHashCode(type); + result = 31 * result + ObjectUtils.nullSafeHashCode(path); + return result; + } + } +} diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java b/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java deleted file mode 100644 index f9f5b849a8..0000000000 --- a/src/main/java/org/springframework/data/convert/SimplePropertyConverterFactory.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright 2021 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.convert; - -import java.util.EnumSet; - -import org.springframework.beans.BeanUtils; -import org.springframework.util.Assert; - -/** - * Trivial implementation of {@link PropertyValueConverter}. - * - * @author Christoph Strobl - */ -class SimplePropertyConverterFactory implements PropertyValueConverterFactory { - - @Override - public PropertyValueConverter getConverter(Class> converterType) { - - Assert.notNull(converterType, "ConverterType must not be null!"); - - if (converterType.isEnum()) { - return (PropertyValueConverter) EnumSet.allOf((Class) converterType).iterator().next(); - } - return BeanUtils.instantiateClass(converterType); - } -} diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java new file mode 100644 index 0000000000..2e13dd7d09 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java @@ -0,0 +1,89 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.springframework.beans.factory.InitializingBean; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + * @since ? + */ +public class SimplePropertyValueConversions implements PropertyValueConversions, InitializingBean { + + private @Nullable PropertyValueConverterFactory converterFactory; + private @Nullable PropertyValueConverterRegistrar converterRegistrar; + private boolean converterCacheEnabled = true; + private AtomicBoolean initialized = new AtomicBoolean(false); + + public void setConverterFactory(PropertyValueConverterFactory converterFactory) { + this.converterFactory = converterFactory; + } + + public void setConverterRegistrar(PropertyValueConverterRegistrar converterRegistrar) { + this.converterRegistrar = converterRegistrar; + } + + public void setConverterCacheEnabled(boolean converterCacheEnabled) { + this.converterCacheEnabled = converterCacheEnabled; + } + + @Override + public PropertyValueConverter getValueConverter( + PersistentProperty property) { + + if (!initialized.get()) { + init(); + } + + return this.converterFactory.getConverter(property); + } + + public void init() { + + if (initialized.compareAndSet(false, true)) { + List factoryList = new ArrayList<>(3); + + if (converterFactory != null) { + factoryList.add(converterFactory); + } else { + factoryList.add(PropertyValueConverterFactory.simple()); + } + + if (converterRegistrar != null && !converterRegistrar.isEmpty()) { + factoryList.add(PropertyValueConverterFactory.configuredInstance(converterRegistrar)); + } + + PropertyValueConverterFactory targetFactory = factoryList.size() > 1 + ? PropertyValueConverterFactory.chained(factoryList) + : factoryList.iterator().next(); + + this.converterFactory = converterCacheEnabled ? PropertyValueConverterFactory.caching(targetFactory) + : targetFactory; + } + } + + @Override + public void afterPropertiesSet() throws Exception { + + init(); + } +} diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 78aec58a33..e06251d9a3 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -25,6 +25,7 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.convert.PropertyConverter; import org.springframework.data.convert.PropertyValueConverter; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -437,7 +438,7 @@ default PersistentPropertyAccessor getAccessorForOwner(T owner) { } @Nullable - default Class> getValueConverterType() { + default Class> getValueConverterType() { PropertyConverter annotation = findAnnotation(PropertyConverter.class); return annotation == null ? null : annotation.value(); } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index a6483cf0bd..976384e1ab 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -17,19 +17,14 @@ import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; -import java.lang.reflect.Field; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.springframework.beans.BeanUtils; -import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; -import org.springframework.beans.factory.config.AutowireCapableBeanFactory; -import org.springframework.context.expression.BeanFactoryResolver; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.data.annotation.AccessType; import org.springframework.data.annotation.AccessType.Type; @@ -40,6 +35,7 @@ import org.springframework.data.annotation.Version; import org.springframework.data.convert.PropertyConverter; import org.springframework.data.convert.PropertyValueConverter; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; @@ -50,7 +46,6 @@ import org.springframework.data.util.ReflectionUtils; import org.springframework.data.util.StreamUtils; import org.springframework.data.util.TypeInformation; -import org.springframework.expression.BeanResolver; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -316,7 +311,7 @@ public TypeInformation getAssociationTargetTypeInformation() { @Nullable @Override - public Class> getValueConverterType() { + public Class> getValueConverterType() { return doFindAnnotation(PropertyConverter.class) // .map(PropertyConverter::value) // diff --git a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java index c034094497..8722594f09 100644 --- a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java +++ b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java @@ -45,6 +45,7 @@ import org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConverter; import org.springframework.data.convert.ThreeTenBackPortConverters.LocalDateTimeToJavaTimeInstantConverter; import org.springframework.data.geo.Point; +import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.threeten.bp.LocalDateTime; @@ -303,6 +304,25 @@ void registersVavrConverters() { assertThat(conversionService.canConvert(List.class, io.vavr.collection.List.class)).isTrue(); } + @Test // GH-1484 + void allowsToRegisterPropertyConversions() { + + PropertyValueConversions propertyValueConversions = mock(PropertyValueConversions.class); + when(propertyValueConversions.getValueConverter(any())).thenReturn(mock(PropertyValueConverter.class)); + + CustomConversions conversions = new CustomConversions(new ConverterConfiguration(StoreConversions.NONE, + Collections.emptyList(), (it) -> true, propertyValueConversions)); + assertThat(conversions.getPropertyValueConverter(mock(PersistentProperty.class))).isNotNull(); + } + + @Test // GH-1484 + void doesNotFailIfPropertiesConversionIsNull() { + + CustomConversions conversions = new CustomConversions(new ConverterConfiguration(StoreConversions.NONE, + Collections.emptyList(), (it) -> true, null)); + assertThat(conversions.getPropertyValueConverter(mock(PersistentProperty.class))).isNull(); + } + private static Class createProxyTypeFor(Class type) { ProxyFactory factory = new ProxyFactory(); diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java new file mode 100644 index 0000000000..15426b8660 --- /dev/null +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java @@ -0,0 +1,239 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import java.util.UUID; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + */ +public class PropertyValueConverterFactoryUnitTests { + + @Test // GH-1484 + void simpleConverterFactoryCanInstantiateFactoryWithDefaultCtor() { + + assertThat(PropertyValueConverterFactory.simple().getConverter(ConverterWithDefaultCtor.class)) + .isInstanceOf(ConverterWithDefaultCtor.class); + } + + @Test // GH-1484 + void simpleConverterFactoryReadsConverterFromAnnotation() { + + PersistentProperty property = mock(PersistentProperty.class); + when(property.hasValueConverter()).thenReturn(true); + when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class); + + assertThat(PropertyValueConverterFactory.simple().getConverter(property)) + .isInstanceOf(ConverterWithDefaultCtor.class); + } + + @Test // GH-1484 + void simpleConverterFactoryErrorsOnNullType() { + + assertThatIllegalArgumentException() + .isThrownBy(() -> PropertyValueConverterFactory.simple().getConverter((Class) null)); + } + + @Test // GH-1484 + void simpleConverterFactoryCanExtractFactoryEnumInstance() { + + assertThat(PropertyValueConverterFactory.simple().getConverter(ConverterEnum.class)) + .isInstanceOf(ConverterEnum.class); + } + + @Test // GH-1484 + void simpleConverterFactoryCannotInstantiateFactoryWithDependency() { + + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> PropertyValueConverterFactory.simple().getConverter(ConverterWithDependency.class)); + } + + @Test // GH-1484 + void beanFactoryAwareConverterFactoryCanInstantiateFactoryWithDefaultCtor() { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + + assertThat(PropertyValueConverterFactory.beanFactoryAware(beanFactory).getConverter(ConverterWithDefaultCtor.class)) + .isInstanceOf(ConverterWithDefaultCtor.class); + } + + @Test // GH-1484 + void beanFactoryAwareConverterFactoryCanInstantiateFactoryWithBeanReference() { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("someDependency", + BeanDefinitionBuilder.rootBeanDefinition(SomeDependency.class).getBeanDefinition()); + + assertThat(PropertyValueConverterFactory.beanFactoryAware(beanFactory).getConverter(ConverterWithDependency.class)) + .isInstanceOf(ConverterWithDependency.class); + } + + @Test // GH-1484 + void beanFactoryAwareConverterFactoryCanLookupExistingBean() { + + DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory(); + beanFactory.registerBeanDefinition("someDependency", + BeanDefinitionBuilder.rootBeanDefinition(SomeDependency.class).getBeanDefinition()); + beanFactory.registerBeanDefinition("theMightyConverter", + BeanDefinitionBuilder.rootBeanDefinition(ConverterWithDependency.class) + .addConstructorArgReference("someDependency").getBeanDefinition()); + + assertThat(PropertyValueConverterFactory.beanFactoryAware(beanFactory).getConverter(ConverterWithDependency.class)) + .isSameAs(beanFactory.getBean("theMightyConverter")); + } + + @Test // GH-1484 + void compositeConverterFactoryIteratesFactories() { + + PropertyValueConverter expected = mock(PropertyValueConverter.class); + + PropertyValueConverterFactory factory = PropertyValueConverterFactory.chained(new PropertyValueConverterFactory() { + @Nullable + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + return null; + } + }, new PropertyValueConverterFactory() { + @Nullable + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + return expected; + } + }); + + assertThat(factory.getConverter(ConverterWithDefaultCtor.class)).isSameAs(expected); + } + + @Test // GH-1484 + void compositeConverterFactoryFailsOnException() { + + PropertyValueConverterFactory factory = PropertyValueConverterFactory.chained(new PropertyValueConverterFactory() { + @Nullable + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + return null; + } + }, new PropertyValueConverterFactory() { + @Nullable + @Override + public PropertyValueConverter getConverter( + Class> converterType) { + throw new RuntimeException("can't touch this!"); + } + }); + + assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(() -> factory.getConverter(ConverterWithDefaultCtor.class)); + } + + @Test // GH-1484 + void cachingConverterFactoryServesCachedInstance() { + + PropertyValueConverterFactory factory = PropertyValueConverterFactory + .caching(PropertyValueConverterFactory.simple()); + assertThat(factory.getConverter(ConverterWithDefaultCtor.class)) + .isSameAs(factory.getConverter(ConverterWithDefaultCtor.class)); + } + + @Test // GH-1484 + void cachingConverterFactoryServesCachedInstanceForProperty() { + + PersistentProperty property = mock(PersistentProperty.class); + when(property.hasValueConverter()).thenReturn(true); + when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class); + + PropertyValueConverterFactory factory = PropertyValueConverterFactory + .caching(PropertyValueConverterFactory.simple()); + assertThat(factory.getConverter(property)) // + .isSameAs(factory.getConverter(property)) // + .isSameAs(factory.getConverter(ConverterWithDefaultCtor.class)); // TODO: is this a valid assumption? + } + + static class ConverterWithDefaultCtor implements PropertyValueConverter { + + @Nullable + @Override + public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { + return nativeValue.toString(); + } + + @Nullable + @Override + public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + return UUID.fromString(domainValue); + } + } + + enum ConverterEnum implements PropertyValueConverter { + + INSTANCE; + + @Nullable + @Override + public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { + return nativeValue.toString(); + } + + @Nullable + @Override + public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + return UUID.fromString(domainValue); + } + } + + static class ConverterWithDependency implements PropertyValueConverter { + + private final SomeDependency someDependency; + + public ConverterWithDependency(@Autowired SomeDependency someDependency) { + this.someDependency = someDependency; + } + + @Nullable + @Override + public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { + + assertThat(someDependency).isNotNull(); + return nativeValue.toString(); + } + + @Nullable + @Override + public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + + assertThat(someDependency).isNotNull(); + return UUID.fromString(domainValue); + } + } + + static class SomeDependency { + + } +} diff --git a/src/test/java/org/springframework/data/convert/WhatWeWant.java b/src/test/java/org/springframework/data/convert/WhatWeWant.java new file mode 100644 index 0000000000..9092b40e14 --- /dev/null +++ b/src/test/java/org/springframework/data/convert/WhatWeWant.java @@ -0,0 +1,90 @@ +/* + * Copyright 2022. the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.function.Predicate; + +import org.junit.jupiter.api.Test; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.mapping.PropertyPath; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + * @since 2022/01 + */ +public class WhatWeWant { + + @Test + void converterConfig() { + + ConverterConfig converterConfig = null; + + converterConfig.registerConverter(Foo.class, "value", new PropertyValueConverter() { + + @Nullable + @Override + public Object nativeToDomain(@Nullable Object nativeValue, ValueConversionContext context) { + return null; + } + + @Nullable + @Override + public Object domainToNative(@Nullable Object domainValue, ValueConversionContext context) { + return null; + } + }); + + } + + static class ConverterConfig { + + ConverterConfig registerConverter(Predicate> filter, PropertyValueConverter converter) { + return this; + } + + ConverterConfig registerConverter(Class type, String property, PropertyValueConverter converter) { + PropertyPath.from(property, type); + return this; + } + } + + static class Foo { + String value; + } + + interface SpecificValueConversionContext extends PropertyValueConverter.ValueConversionContext { + + } + + interface SpecificPropertyValueConverter extends PropertyValueConverter {} +} diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 53c373bc57..68611affeb 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -37,7 +37,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType; @@ -48,6 +47,7 @@ import org.springframework.data.annotation.Transient; import org.springframework.data.convert.PropertyConverter; import org.springframework.data.convert.PropertyValueConverter; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SampleMappingContext; @@ -543,20 +543,20 @@ static class WithPropertyConverter { String value2; } - static class MyPropertyConverter implements PropertyValueConverter { + static class MyPropertyConverter implements PropertyValueConverter { @Override - public Object nativeToDomain(Object value) { + public Object nativeToDomain(Object value, ValueConversionContext context) { return null; } @Override - public Object domainToNative(Object value) { + public Object domainToNative(Object value, ValueConversionContext context) { return null; } } - static class MyPropertyConverterThatRequiresComponents implements PropertyValueConverter { + static class MyPropertyConverterThatRequiresComponents implements PropertyValueConverter { private final SomeDependency someDependency; @@ -565,12 +565,12 @@ public MyPropertyConverterThatRequiresComponents(@Autowired SomeDependency someD } @Override - public Object nativeToDomain(Object value) { + public Object nativeToDomain(Object value, ValueConversionContext context) { return null; } @Override - public Object domainToNative(Object value) { + public Object domainToNative(Object value, ValueConversionContext context) { return null; } } From d6b8629165cfdbbc02bda8852208aa0c9455c8e2 Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Wed, 16 Feb 2022 15:40:12 +0100 Subject: [PATCH 6/9] Fluent registration API for PropertyValueConverters. Introduce a builder API to register PropertyValueConverters using simple (Bi)Functions. Additional methods on ValueConversionContext to enable advanced use cases in converter implementation. Tweak generics of VCC to be able to expose store-specific PersistentProperty implementations via the context. --- .../data/convert/CustomConversions.java | 3 +- .../data/convert/PropertyConverter.java | 2 +- .../convert/PropertyValueConversions.java | 5 +- .../data/convert/PropertyValueConverter.java | 89 +++++++- .../PropertyValueConverterFactories.java | 47 ++-- .../PropertyValueConverterFactory.java | 6 +- .../SimplePropertyValueConversions.java | 5 +- .../data/mapping/PersistentProperty.java | 7 +- .../AnnotationBasedPersistentProperty.java | 4 +- ...ropertyValueConverterFactoryUnitTests.java | 25 ++- .../data/convert/WhatWeWant.java | 203 +++++++++++++++++- ...ationBasedPersistentPropertyUnitTests.java | 6 +- 12 files changed, 344 insertions(+), 58 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index 54f8d12c3b..cd0c36ab26 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -33,6 +33,7 @@ import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.ConverterBuilder.ConverterAware; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.Predicates; @@ -201,7 +202,7 @@ public boolean hasPropertyValueConverter(PersistentProperty property) { * @since ? */ @Nullable - public PropertyValueConverter getPropertyValueConverter( + public > PropertyValueConverter getPropertyValueConverter( PersistentProperty property) { return propertyValueConversions != null ? propertyValueConversions.getValueConverter(property) : null; } diff --git a/src/main/java/org/springframework/data/convert/PropertyConverter.java b/src/main/java/org/springframework/data/convert/PropertyConverter.java index 43fc76d35a..482e4f318a 100644 --- a/src/main/java/org/springframework/data/convert/PropertyConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyConverter.java @@ -42,6 +42,6 @@ * * @return the configured {@link PropertyValueConverter}. {@link ObjectToObjectPropertyValueConverter} by default. */ - Class> value() default ObjectToObjectPropertyValueConverter.class; + Class value() default ObjectToObjectPropertyValueConverter.class; } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConversions.java b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java index 8a8a42056d..e69b14164d 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConversions.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java @@ -15,6 +15,7 @@ */ package org.springframework.data.convert; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; @@ -23,7 +24,7 @@ * applied to a specific property. Other than {@link org.springframework.core.convert.converter.Converter converters} * registered in {@link CustomConversions} the property based variants accept and allow returning {@literal null} values * and provide access to a store specific {@link PropertyValueConverter.ValueConversionContext conversion context}. - * + * * @author Christoph Strobl * @since ? * @currentBook The Desert Prince - Peter V. Brett @@ -49,6 +50,6 @@ default boolean hasValueConverter(PersistentProperty property) { * @return the suitable {@link PropertyValueConverter} or {@literal null} if none available. */ @Nullable - PropertyValueConverter getValueConverter( + > PropertyValueConverter getValueConverter( PersistentProperty property); } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java index ce9d8140fa..e249931c8c 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -15,7 +15,10 @@ */ package org.springframework.data.convert; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; /** @@ -30,7 +33,7 @@ * @param store native type * @since 2.7 */ -public interface PropertyValueConverter { +public interface PropertyValueConverter>> { /** * Convert the given store specific value into it's domain value representation. Typically a {@literal read} @@ -56,10 +59,86 @@ public interface PropertyValueConverter> { - PersistentProperty getProperty(); + /** + * Return the {@link PersistentProperty} to be handled. + * + * @return will never be {@literal null}. + */ + P getProperty(); + + /** + * Write to whatever type is considered best for the given source. + * + * @param value + * @return + */ + @Nullable + default Object write(@Nullable Object value) { + return null; + } + + /** + * Write as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T write(@Nullable Object value, Class target) { + return write(value, ClassTypeInformation.from(target)); + } + + /** + * Write as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T write(@Nullable Object value, TypeInformation target) { + return null; + } + + /** + * Reads the value into the type of the current property. + * + * @param value can be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default Object read(@Nullable Object value) { + return read(value, getProperty().getTypeInformation()); + } + + /** + * Reads the value as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T read(@Nullable Object value, Class target) { + return null; + } + + /** + * Reads the value as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T read(@Nullable Object value, TypeInformation target) { + return null; + } } /** @@ -67,7 +146,8 @@ interface ValueConversionContext { * * @author Christoph Strobl */ - enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { + @SuppressWarnings({ "rawtypes", "null" }) + enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { INSTANCE; @@ -81,5 +161,4 @@ public Object domainToNative(Object value, ValueConversionContext context) { return value; } } - } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java index 1843e263c8..540521824e 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java @@ -26,6 +26,7 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.AutowireCapableBeanFactory; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -59,14 +60,14 @@ static class CompositePropertyValueConverterFactory implements PropertyValueConv @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( PersistentProperty property) { return delegates.stream().map(it -> (PropertyValueConverter) it.getConverter(property)) .filter(Objects::nonNull).findFirst().orElse(null); } @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { return delegates.stream().filter(it -> it.getConverter(converterType) != null).findFirst() .map(it -> it.getConverter(converterType)).orElse(null); @@ -82,7 +83,7 @@ public PropertyV static class SimplePropertyConverterFactory implements PropertyValueConverterFactory { @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { Assert.notNull(converterType, "ConverterType must not be null!"); @@ -110,7 +111,7 @@ public BeanFactoryAwarePropertyValueConverterFactory(BeanFactory beanFactory) { } @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { Assert.state(beanFactory != null, "BeanFactory must not be null. Did you forget to set it!"); @@ -145,14 +146,14 @@ public ConfiguredInstanceServingValueConverterFactory(PropertyValueConverterRegi @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( PersistentProperty property) { return (PropertyValueConverter) conversionsRegistrar.getConverter(property.getOwner().getType(), property.getName()); } @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { return null; } @@ -170,55 +171,55 @@ static class CachingPropertyValueConverterFactory implements PropertyValueConver private final Cache cache = new Cache(); public CachingPropertyValueConverterFactory(PropertyValueConverterFactory delegate) { - + Assert.notNull(delegate, "Delegate must not be null!"); this.delegate = delegate; } @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( PersistentProperty property) { PropertyValueConverter converter = cache.get(property); - if (converter != null) { - return converter; - } - return cache.cache(property, delegate.getConverter(property)); + + return converter != null + ? converter + : cache.cache(property, delegate.getConverter(property)); } @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { PropertyValueConverter converter = cache.get(converterType); - if (converter != null) { - return converter; - } - return cache.cache(converterType, delegate.getConverter(converterType)); + + return converter != null + ? converter + : cache.cache(converterType, delegate.getConverter(converterType)); } static class Cache { - Map, PropertyValueConverter> perPropertyCache = new HashMap<>(); - Map, PropertyValueConverter> typeCache = new HashMap<>(); + Map, PropertyValueConverter>> perPropertyCache = new HashMap<>(); + Map, PropertyValueConverter>> typeCache = new HashMap<>(); - PropertyValueConverter get(PersistentProperty property) { + PropertyValueConverter> get(PersistentProperty property) { return perPropertyCache.get(property); } - PropertyValueConverter get(Class type) { + PropertyValueConverter> get(Class type) { return typeCache.get(type); } - PropertyValueConverter cache(PersistentProperty property, + > PropertyValueConverter cache(PersistentProperty property, PropertyValueConverter converter) { perPropertyCache.putIfAbsent(property, converter); cache(property.getValueConverterType(), converter); return converter; } - PropertyValueConverter cache(Class type, + > PropertyValueConverter cache(Class type, PropertyValueConverter converter) { typeCache.putIfAbsent(type, converter); return converter; diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java index bb5cf47a8b..f3df7a1827 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java @@ -19,6 +19,7 @@ import java.util.List; import org.springframework.beans.factory.BeanFactory; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.convert.PropertyValueConverterFactories.BeanFactoryAwarePropertyValueConverterFactory; import org.springframework.data.convert.PropertyValueConverterFactories.CachingPropertyValueConverterFactory; import org.springframework.data.convert.PropertyValueConverterFactories.CompositePropertyValueConverterFactory; @@ -49,7 +50,8 @@ public interface PropertyValueConverterFactory { * @return can be {@literal null}. */ @Nullable - default PropertyValueConverter getConverter( + @SuppressWarnings("unchecked") + default > PropertyValueConverter getConverter( PersistentProperty property) { if (!property.hasValueConverter()) { @@ -59,7 +61,7 @@ default Property } @Nullable - PropertyValueConverter getConverter( + > PropertyValueConverter getConverter( Class> converterType); /** diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java index 2e13dd7d09..0740fb82f9 100644 --- a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java +++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java @@ -20,6 +20,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.springframework.beans.factory.InitializingBean; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; @@ -47,7 +48,7 @@ public void setConverterCacheEnabled(boolean converterCacheEnabled) { } @Override - public PropertyValueConverter getValueConverter( + public > PropertyValueConverter getValueConverter( PersistentProperty property) { if (!initialized.get()) { @@ -68,7 +69,7 @@ public void init() { factoryList.add(PropertyValueConverterFactory.simple()); } - if (converterRegistrar != null && !converterRegistrar.isEmpty()) { + if ((converterRegistrar != null) && !converterRegistrar.isEmpty()) { factoryList.add(PropertyValueConverterFactory.configuredInstance(converterRegistrar)); } diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index e06251d9a3..82a1b3e520 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -438,9 +438,12 @@ default PersistentPropertyAccessor getAccessorForOwner(T owner) { } @Nullable - default Class> getValueConverterType() { + default Class>> getValueConverterType() { + PropertyConverter annotation = findAnnotation(PropertyConverter.class); - return annotation == null ? null : annotation.value(); + + return annotation == null ? null + : (Class>>) annotation.value(); } default boolean hasValueConverter() { diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 976384e1ab..1e8e060137 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -311,10 +311,12 @@ public TypeInformation getAssociationTargetTypeInformation() { @Nullable @Override - public Class> getValueConverterType() { + @SuppressWarnings("unchecked") + public Class>> getValueConverterType() { return doFindAnnotation(PropertyConverter.class) // .map(PropertyConverter::value) // + .map(Class.class::cast) // .orElse(null); } diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java index 15426b8660..b502145b18 100644 --- a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java @@ -26,6 +26,7 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.lang.Nullable; /** @@ -114,14 +115,14 @@ void compositeConverterFactoryIteratesFactories() { PropertyValueConverterFactory factory = PropertyValueConverterFactory.chained(new PropertyValueConverterFactory() { @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { return null; } }, new PropertyValueConverterFactory() { @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { return expected; } @@ -136,14 +137,14 @@ void compositeConverterFactoryFailsOnException() { PropertyValueConverterFactory factory = PropertyValueConverterFactory.chained(new PropertyValueConverterFactory() { @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { return null; } }, new PropertyValueConverterFactory() { @Nullable @Override - public PropertyValueConverter getConverter( + public > PropertyValueConverter getConverter( Class> converterType) { throw new RuntimeException("can't touch this!"); } @@ -176,22 +177,23 @@ void cachingConverterFactoryServesCachedInstanceForProperty() { .isSameAs(factory.getConverter(ConverterWithDefaultCtor.class)); // TODO: is this a valid assumption? } - static class ConverterWithDefaultCtor implements PropertyValueConverter { + static class ConverterWithDefaultCtor + implements PropertyValueConverter> { @Nullable @Override - public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { + public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { return nativeValue.toString(); } @Nullable @Override - public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { return UUID.fromString(domainValue); } } - enum ConverterEnum implements PropertyValueConverter { + enum ConverterEnum implements PropertyValueConverter> { INSTANCE; @@ -208,7 +210,8 @@ public UUID domainToNative(@Nullable String domainValue, ValueConversionContext } } - static class ConverterWithDependency implements PropertyValueConverter { + static class ConverterWithDependency + implements PropertyValueConverter> { private final SomeDependency someDependency; @@ -218,7 +221,7 @@ public ConverterWithDependency(@Autowired SomeDependency someDependency) { @Nullable @Override - public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { + public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext context) { assertThat(someDependency).isNotNull(); return nativeValue.toString(); @@ -226,7 +229,7 @@ public String nativeToDomain(@Nullable UUID nativeValue, ValueConversionContext @Nullable @Override - public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { assertThat(someDependency).isNotNull(); return UUID.fromString(domainValue); diff --git a/src/test/java/org/springframework/data/convert/WhatWeWant.java b/src/test/java/org/springframework/data/convert/WhatWeWant.java index 9092b40e14..7ca616af49 100644 --- a/src/test/java/org/springframework/data/convert/WhatWeWant.java +++ b/src/test/java/org/springframework/data/convert/WhatWeWant.java @@ -31,11 +31,19 @@ */ package org.springframework.data.convert; +import java.util.HashMap; +import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Predicate; import org.junit.jupiter.api.Test; +import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PropertyPath; +import org.springframework.data.mapping.context.SamplePersistentProperty; +import org.springframework.data.util.MethodInvocationRecorder; import org.springframework.lang.Nullable; /** @@ -47,7 +55,33 @@ public class WhatWeWant { @Test void converterConfig() { - ConverterConfig converterConfig = null; + ConverterConfig converterConfig = new ConverterConfig(); + + // Arbitrary translations + converterConfig.registerConverter(Foo.class, Foo::getValue) + .writing((source, context) -> WhatWeWant.reverse(source)) // store reversed + .reading((source, context) -> WhatWeWant.reverse(source)); // restore order + + converterConfig.registerConverter(it -> it.findAnnotation(Deprecated.class) != null) + .writing((source, context) -> WhatWeWant.reverse(source.toString())) + .reading((source, context) -> { + return context.read(WhatWeWant.reverse(source)); + }); + + // Clean up on read + converterConfig.registerConverter(Foo.class, Foo::getValue) + .writingAsIs() + .reading((source, context) -> source.trim()); + + converterConfig.registerConverter(Foo.class, Foo::getAddress) + .writing((source, context) -> { + + Map map = new HashMap<>(); + map.put("STREET", source.street.toUpperCase()); + map.put("ZIP", context.write(source.zipCode)); + + return map; + }); converterConfig.registerConverter(Foo.class, "value", new PropertyValueConverter() { @@ -63,28 +97,185 @@ public Object domainToNative(@Nullable Object domainValue, ValueConversionContex return null; } }); + } + static String reverse(String source) { + return new StringBuilder(source).reverse().toString(); } - static class ConverterConfig { + static class ConverterConfig

> { - ConverterConfig registerConverter(Predicate> filter, PropertyValueConverter converter) { + ConverterConfig registerConverter(Predicate

filter, + PropertyValueConverter> converter) { return this; } - ConverterConfig registerConverter(Class type, String property, PropertyValueConverter converter) { + ConverterConfig registerConverter(Class type, String property, + PropertyValueConverter> converter) { PropertyPath.from(property, type); return this; } + + /** + * Starts a converter registration by pointing to a property of a domain type. + * + * @param the domain type + * @param the property type + * @param type the domain type to obtain the property from + * @param property a function to describe the property to be referenced. Usually a method handle to a getter. + * @return will never be {@literal null}. + */ + WritingConverterRegistrationBuilder registerConverter( + Class type, Function property) { + + String propertyName = MethodInvocationRecorder.forProxyOf(type) + .record(property) + .getPropertyPath() + .orElseThrow(() -> new IllegalArgumentException("Cannot obtain property name!")); + + return new WritingConverterRegistrationBuilder(type, propertyName, this); + } + + WritingConverterRegistrationBuilder registerConverter(Predicate

predicate) { + return new WritingConverterRegistrationBuilder(predicate, this); + } + + /** + * Helper to build up a fluent registration API starting on + * {@link ConverterConfig#registerConverter(Class, Function)}. + * + * @author Oliver Drotbohm + */ + static class WritingConverterRegistrationBuilder> { + + private final Consumer registration; + private final ConverterConfig config; + + public WritingConverterRegistrationBuilder(Class type, String property, ConverterConfig config) { + + this.config = config; + this.registration = (converter) -> config.registerConverter(type, property, converter); + } + + public WritingConverterRegistrationBuilder(Predicate

predicate, ConverterConfig config) { + + this.config = config; + this.registration = (converter) -> config.registerConverter(predicate, converter); + } + + ReadingConverterRegistrationBuilder writingAsIs() { + return writing((source, context) -> source); + } + + ReadingConverterRegistrationBuilder writing(Function writer) { + return writing((source, context) -> writer.apply(source)); + } + + /** + * Describes how to convert the domain property value into the database native property. + * + * @param the type to be written to the database + * @param writer the property conversion to extract a value written to the database + * @return will never be {@literal null}. + */ + ReadingConverterRegistrationBuilder writing(BiFunction, R> writer) { + return new ReadingConverterRegistrationBuilder<>(this, writer); + } + } + + /** + * A helper to build a fluent API to register how to read a database value into a domain object property. + * + * @author Oliver Drotbohm + */ + static class ReadingConverterRegistrationBuilder> { + + private WritingConverterRegistrationBuilder origin; + private BiFunction, R> writer; + + public ReadingConverterRegistrationBuilder(WritingConverterRegistrationBuilder origin, + BiFunction, R> writer) { + this.origin = origin; + this.writer = writer; + } + + ConverterConfig

readingAsIs() { + return reading((source, context) -> (S) source); + } + + ConverterConfig

reading(Function reader) { + return reading((source, context) -> reader.apply(source)); + } + + /** + * Describes how to read a database value into a domain object's property value. + * + * @param reader must not be {@literal null}. + * @return + */ + ConverterConfig

reading(BiFunction, S> reader) { + + origin.registration.accept(new FunctionPropertyValueConverter(writer, reader)); + + return origin.config; + } + + /** + * A {@link PropertyValueConverter} that delegates conversion to the given {@link BiFunction}s. + * + * @author Oliver Drotbohm + */ + static class FunctionPropertyValueConverter> + implements PropertyValueConverter> { + + private final BiFunction, B> writer; + private final BiFunction, A> reader; + + public FunctionPropertyValueConverter(BiFunction, B> writer, + BiFunction, A> reader) { + this.writer = writer; + this.reader = reader; + } + + @Override + public B domainToNative(A domainValue, ValueConversionContext

context) { + return writer.apply(domainValue, context); + } + + @Override + public A nativeToDomain(B nativeValue, ValueConversionContext

context) { + return reader.apply(nativeValue, context); + } + } + } } static class Foo { String value; + Address address; + + public String getValue() { + return value; + } + + public Address getAddress() { + return address; + } + } + + static class Address { + String street; + ZipCode zipCode; + } + + static class ZipCode { + } - interface SpecificValueConversionContext extends PropertyValueConverter.ValueConversionContext { + interface SpecificValueConversionContext

> extends ValueConversionContext

{ } - interface SpecificPropertyValueConverter extends PropertyValueConverter {} + interface SpecificPropertyValueConverter + extends PropertyValueConverter> {} } diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 68611affeb..702e1e4798 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -543,7 +543,8 @@ static class WithPropertyConverter { String value2; } - static class MyPropertyConverter implements PropertyValueConverter { + static class MyPropertyConverter + implements PropertyValueConverter> { @Override public Object nativeToDomain(Object value, ValueConversionContext context) { @@ -556,7 +557,8 @@ public Object domainToNative(Object value, ValueConversionContext context) { } } - static class MyPropertyConverterThatRequiresComponents implements PropertyValueConverter { + static class MyPropertyConverterThatRequiresComponents + implements PropertyValueConverter> { private final SomeDependency someDependency; From ea252f0ccb499215c8a8374086d7036606fb1683 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 24 Feb 2022 14:59:17 +0100 Subject: [PATCH 7/9] Rename/move/update types & add javadoc --- .../data/convert/CustomConversions.java | 44 +++- .../convert/PropertyValueConversions.java | 29 ++- .../data/convert/PropertyValueConverter.java | 129 ++++-------- .../PropertyValueConverterFactories.java | 65 +++--- .../PropertyValueConverterFactory.java | 32 ++- .../PropertyValueConverterRegistrar.java | 194 +++++++++++++----- .../SimplePropertyValueConversions.java | 56 ++++- .../SimplePropertyValueConverterRegistry.java | 126 ++++++++++++ .../data/convert/ValueConversionContext.java | 113 ++++++++++ ...ertyConverter.java => ValueConverter.java} | 16 +- .../data/convert/ValueConverterRegistry.java | 77 +++++++ .../data/mapping/PersistentProperty.java | 35 +++- .../AnnotationBasedPersistentProperty.java | 10 +- ...ropertyValueConverterFactoryUnitTests.java | 51 +++-- ...pertyValueConverterRegistrarUnitTests.java | 107 ++++++++++ ...mplePropertyValueConversionsUnitTests.java | 69 +++++++ ...opertyValueConverterRegistryUnitTests.java | 88 ++++++++ .../data/convert/WhatWeWant.java | 31 +-- .../AbstractPersistentPropertyUnitTests.java | 4 - ...ationBasedPersistentPropertyUnitTests.java | 47 ++--- 20 files changed, 1018 insertions(+), 305 deletions(-) create mode 100644 src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java create mode 100644 src/main/java/org/springframework/data/convert/ValueConversionContext.java rename src/main/java/org/springframework/data/convert/{PropertyConverter.java => ValueConverter.java} (78%) create mode 100644 src/main/java/org/springframework/data/convert/ValueConverterRegistry.java create mode 100644 src/test/java/org/springframework/data/convert/PropertyValueConverterRegistrarUnitTests.java create mode 100644 src/test/java/org/springframework/data/convert/SimplePropertyValueConversionsUnitTests.java create mode 100644 src/test/java/org/springframework/data/convert/SimplePropertyValueConverterRegistryUnitTests.java diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index cd0c36ab26..271b1b0601 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -16,7 +16,16 @@ package org.springframework.data.convert; import java.lang.annotation.Annotation; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Predicate; @@ -33,7 +42,6 @@ import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.convert.ConverterBuilder.ConverterAware; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.model.SimpleTypeHolder; import org.springframework.data.util.Predicates; @@ -100,7 +108,7 @@ public class CustomConversions { private final Function> getRawWriteTarget = convertiblePair -> getCustomTarget( convertiblePair.getSourceType(), null, writingPairs); - private PropertyValueConversions propertyValueConversions; + private @Nullable PropertyValueConversions propertyValueConversions; /** * @param converterConfiguration the {@link ConverterConfiguration} to apply. @@ -184,7 +192,7 @@ public void registerConvertersIn(ConverterRegistry conversionService) { * @param property must not be {@literal null}. * @return {@literal true} if a specific {@link PropertyValueConverter} is available. * @see PropertyValueConversions#hasValueConverter(PersistentProperty) - * @since ? + * @since 2.7 */ public boolean hasPropertyValueConverter(PersistentProperty property) { return propertyValueConversions != null ? propertyValueConversions.hasValueConverter(property) : false; @@ -199,11 +207,11 @@ public boolean hasPropertyValueConverter(PersistentProperty property) { * @param conversion context type * @return the suitable {@link PropertyValueConverter} or {@literal null} if none available. * @see PropertyValueConversions#getValueConverter(PersistentProperty) - * @since ? + * @since 2.7 */ @Nullable - public > PropertyValueConverter getPropertyValueConverter( - PersistentProperty property) { + public , D extends ValueConversionContext> PropertyValueConverter getPropertyValueConverter( + C property) { return propertyValueConversions != null ? propertyValueConversions.getValueConverter(property) : null; } @@ -328,8 +336,8 @@ private boolean isSupportedConverter(ConverterRegistrationIntent registrationInt registrationIntent.getSourceType(), registrationIntent.getTargetType(), registrationIntent.isReading() ? "reading" : "writing")); } else { - logger.debug(String.format(SKIP_CONVERTER, registrationIntent.getSourceType(), registrationIntent.getTargetType(), - registrationIntent.isReading() ? "reading" : "writing", + logger.debug(String.format(SKIP_CONVERTER, registrationIntent.getSourceType(), + registrationIntent.getTargetType(), registrationIntent.isReading() ? "reading" : "writing", registrationIntent.isReading() ? registrationIntent.getSourceType() : registrationIntent.getTargetType())); } } @@ -941,8 +949,22 @@ public ConverterConfiguration(StoreConversions storeConversions, List userCon this(storeConversions, userConverters, converterRegistrationFilter, new SimplePropertyValueConversions()); } + /** + * Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters as + * well as a {@link Collection} of {@link ConvertiblePair} for which to skip the registration of default converters. + *
+ * This allows store implementations to modify default converter registration based on specific needs and + * configurations. User defined converters will are never subject of filtering. + * + * @param storeConversions must not be {@literal null}. + * @param userConverters must not be {@literal null} use {@link Collections#emptyList()} instead. + * @param converterRegistrationFilter must not be {@literal null}. + * @param propertyValueConversions can be {@literal null}. + * @since 2.7 + */ public ConverterConfiguration(StoreConversions storeConversions, List userConverters, - Predicate converterRegistrationFilter, @Nullable PropertyValueConversions propertyValueConversions) { + Predicate converterRegistrationFilter, + @Nullable PropertyValueConversions propertyValueConversions) { this.storeConversions = storeConversions; this.userConverters = new ArrayList<>(userConverters); @@ -973,7 +995,7 @@ boolean shouldRegister(ConvertiblePair candidate) { /** * @return the configured {@link PropertyValueConversions} if set, {@literal null} otherwise. - * @since ? + * @since 2.7 */ @Nullable public PropertyValueConversions getPropertyValueConversions() { diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConversions.java b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java index e69b14164d..1c1b09fcf2 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConversions.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConversions.java @@ -15,18 +15,19 @@ */ package org.springframework.data.convert; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import java.util.function.Consumer; + import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; /** * {@link PropertyValueConversions} provides access to {@link PropertyValueConverter converters} that may only be * applied to a specific property. Other than {@link org.springframework.core.convert.converter.Converter converters} - * registered in {@link CustomConversions} the property based variants accept and allow returning {@literal null} values - * and provide access to a store specific {@link PropertyValueConverter.ValueConversionContext conversion context}. + * registered in {@link CustomConversions}, the property based variants accept and allow returning {@literal null} + * values and provide access to a store specific {@link ValueConversionContext conversion context}. * * @author Christoph Strobl - * @since ? + * @since 2.7 * @currentBook The Desert Prince - Peter V. Brett */ public interface PropertyValueConversions { @@ -38,7 +39,7 @@ public interface PropertyValueConversions { * @return {@literal true} if a specific {@link PropertyValueConverter} is available. */ default boolean hasValueConverter(PersistentProperty property) { - return getValueConverter(property) != null; + return getValueConverter((PersistentProperty) property) != null; } /** @@ -50,6 +51,20 @@ default boolean hasValueConverter(PersistentProperty property) { * @return the suitable {@link PropertyValueConverter} or {@literal null} if none available. */ @Nullable - > PropertyValueConverter getValueConverter( - PersistentProperty property); + , D extends ValueConversionContext> PropertyValueConverter getValueConverter( + C property); + + /** + * Helper that allows to create {@link PropertyValueConversions} instance with the configured + * {@link PropertyValueConverter converters} provided via the given callback. + */ + static

> PropertyValueConversions simple( + Consumer> config) { + + SimplePropertyValueConversions conversions = new SimplePropertyValueConversions(); + PropertyValueConverterRegistrar registrar = new PropertyValueConverterRegistrar(); + config.accept(registrar); + conversions.setValueConverterRegistry(registrar.buildRegistry()); + return conversions; + } } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java index e249931c8c..7c0ad8834e 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -15,10 +15,9 @@ */ package org.springframework.data.convert; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import java.util.function.BiFunction; + import org.springframework.data.mapping.PersistentProperty; -import org.springframework.data.util.ClassTypeInformation; -import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; /** @@ -29,8 +28,9 @@ * to special annotated fields which allows a fine grained conversion of certain values within a specific context. * * @author Christoph Strobl - * @param domain specific type - * @param store native type + * @param domain specific type. + * @param store native type. + * @param the store specific {@link ValueConversionContext conversion context}. * @since 2.7 */ public interface PropertyValueConverter>> { @@ -39,126 +39,71 @@ public interface PropertyValueConverter> { - - /** - * Return the {@link PersistentProperty} to be handled. - * - * @return will never be {@literal null}. - */ - P getProperty(); - - /** - * Write to whatever type is considered best for the given source. - * - * @param value - * @return - */ - @Nullable - default Object write(@Nullable Object value) { - return null; - } - - /** - * Write as the given type. - * - * @param value can be {@literal null}. - * @param target must not be {@literal null}. - * @return can be {@literal null}. - */ - @Nullable - default T write(@Nullable Object value, Class target) { - return write(value, ClassTypeInformation.from(target)); - } - - /** - * Write as the given type. - * - * @param value can be {@literal null}. - * @param target must not be {@literal null}. - * @return can be {@literal null}. - */ - @Nullable - default T write(@Nullable Object value, TypeInformation target) { - return null; - } + @SuppressWarnings({ "rawtypes", "null" }) + enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { - /** - * Reads the value into the type of the current property. - * - * @param value can be {@literal null}. - * @return can be {@literal null}. - */ - @Nullable - default Object read(@Nullable Object value) { - return read(value, getProperty().getTypeInformation()); - } + INSTANCE; - /** - * Reads the value as the given type. - * - * @param value can be {@literal null}. - * @param target must not be {@literal null}. - * @return can be {@literal null}. - */ - @Nullable - default T read(@Nullable Object value, Class target) { - return null; + @Override + public Object read(Object value, ValueConversionContext context) { + return value; } - /** - * Reads the value as the given type. - * - * @param value can be {@literal null}. - * @param target must not be {@literal null}. - * @return can be {@literal null}. - */ - @Nullable - default T read(@Nullable Object value, TypeInformation target) { - return null; + @Override + public Object write(Object value, ValueConversionContext context) { + return value; } } /** - * NoOp {@link PropertyValueConverter} implementation. + * A {@link PropertyValueConverter} that delegates conversion to the given {@link BiFunction}s. * - * @author Christoph Strobl + * @author Oliver Drotbohm */ - @SuppressWarnings({ "rawtypes", "null" }) - enum ObjectToObjectPropertyValueConverter implements PropertyValueConverter { + class FunctionPropertyValueConverter> + implements PropertyValueConverter> { - INSTANCE; + private final BiFunction, B> writer; + private final BiFunction, A> reader; + + public FunctionPropertyValueConverter(BiFunction, B> writer, + BiFunction, A> reader) { + + this.writer = writer; + this.reader = reader; + } @Override - public Object nativeToDomain(Object value, ValueConversionContext context) { - return value; + public B write(A value, ValueConversionContext

context) { + return writer.apply(value, context); } @Override - public Object domainToNative(Object value, ValueConversionContext context) { - return value; + public A read(B value, ValueConversionContext

context) { + return reader.apply(value, context); } } } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java index 540521824e..dde48e7d80 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java @@ -15,7 +15,9 @@ */ package org.springframework.data.convert; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; import java.util.List; @@ -26,7 +28,6 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.AutowireCapableBeanFactory; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -34,28 +35,31 @@ /** * {@link PropertyValueConverterFactories} provides a collection of predefined {@link PropertyValueConverterFactory} * implementations. Depending on the applications need {@link PropertyValueConverterFactory factories} can be - * {@link CompositePropertyValueConverterFactory chained} and the created {@link PropertyValueConverter converter} + * {@link ChainedPropertyValueConverterFactory chained} and the created {@link PropertyValueConverter converter} * {@link CachingPropertyValueConverterFactory cached}. * * @author Christoph Strobl - * @since ? + * @since 2.7 */ final class PropertyValueConverterFactories { /** + * {@link PropertyValueConverterFactory} implementation that returns the first no null {@link PropertyValueConverter} + * by asking given {@link PropertyValueConverterFactory factories} one by one. + * * @author Christoph Strobl - * @since ? + * @since 2.7 */ - static class CompositePropertyValueConverterFactory implements PropertyValueConverterFactory { + static class ChainedPropertyValueConverterFactory implements PropertyValueConverterFactory { private List delegates; - CompositePropertyValueConverterFactory(PropertyValueConverterFactory... delegates) { + ChainedPropertyValueConverterFactory(PropertyValueConverterFactory... delegates) { this(Arrays.asList(delegates)); } - CompositePropertyValueConverterFactory(List delegates) { - this.delegates = delegates; + ChainedPropertyValueConverterFactory(List delegates) { + this.delegates = Collections.unmodifiableList(delegates); } @Nullable @@ -72,13 +76,18 @@ public > PropertyValueConverter it.getConverter(converterType) != null).findFirst() .map(it -> it.getConverter(converterType)).orElse(null); } + + public List converterFactories() { + return delegates; + } } /** - * Trivial implementation of {@link PropertyValueConverter}. + * Trivial implementation of {@link PropertyValueConverterFactory} capable of instantiating a + * {@link PropertyValueConverter} via default constructor or in case of {@link Enum} returning the first value. * * @author Christoph Strobl - * @since ? + * @since 2.7 */ static class SimplePropertyConverterFactory implements PropertyValueConverterFactory { @@ -100,13 +109,13 @@ public > PropertyValueConverter> PropertyValueConverter converterRegistry; - public ConfiguredInstanceServingValueConverterFactory(PropertyValueConverterRegistrar conversionsRegistrar) { + ConfiguredInstanceServingValueConverterFactory(ValueConverterRegistry converterRegistry) { - Assert.notNull(conversionsRegistrar, "ConversionsRegistrar must not be null!"); - this.conversionsRegistrar = conversionsRegistrar; + Assert.notNull(converterRegistry, "ConversionsRegistrar must not be null!"); + this.converterRegistry = converterRegistry; } @Nullable @Override public > PropertyValueConverter getConverter( PersistentProperty property) { - return (PropertyValueConverter) conversionsRegistrar.getConverter(property.getOwner().getType(), + return (PropertyValueConverter) converterRegistry.getConverter(property.getOwner().getType(), property.getName()); } @@ -160,17 +172,18 @@ public > PropertyValueConverter> PropertyValueConverter> PropertyValueConverter domain specific type - * @param store native type + * @param domain specific type. + * @param store native type. + * @param value conversion context to use. * @return can be {@literal null}. */ @Nullable @@ -57,12 +57,22 @@ default > PropertyValueConverter>) property.getValueConverterType()); } + /** + * Get the converter by its type. + * + * @param converterType must not be {@literal null}. + * @param domain specific type. + * @param store native type. + * @param value conversion context to use. + * @return + */ @Nullable - > PropertyValueConverter getConverter( - Class> converterType); + > PropertyValueConverter getConverter( + Class> converterType); /** * Obtain a simple {@link PropertyValueConverterFactory} capable of instantiating {@link PropertyValueConverter} @@ -93,7 +103,7 @@ static PropertyValueConverterFactory beanFactoryAware(BeanFactory beanFactory) { * @param registrar must not be {@literal null}. * @return new instance of {@link PropertyValueConverterFactory}. */ - static PropertyValueConverterFactory configuredInstance(PropertyValueConverterRegistrar registrar) { + static PropertyValueConverterFactory configuredInstance(ValueConverterRegistry registrar) { return new ConfiguredInstanceServingValueConverterFactory(registrar); } @@ -123,15 +133,15 @@ static PropertyValueConverterFactory chained(List return factoryList.iterator().next(); } - return new CompositePropertyValueConverterFactory(factoryList); + return new ChainedPropertyValueConverterFactory(factoryList); } /** * Obtain a {@link PropertyValueConverterFactory} that will cache {@link PropertyValueConverter} instances per * {@link PersistentProperty}. * - * @param factory - * @return + * @param factory must not be {@literal null}. + * @return new instance of {@link PropertyValueConverterFactory}. */ static PropertyValueConverterFactory caching(PropertyValueConverterFactory factory) { return new CachingPropertyValueConverterFactory(factory); diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java index 73d5888365..b7fb46364a 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java @@ -15,85 +15,177 @@ */ package org.springframework.data.convert; -import java.util.Collection; -import java.util.LinkedHashMap; -import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Consumer; +import java.util.function.Function; -import org.springframework.lang.Nullable; -import org.springframework.util.ObjectUtils; +import org.springframework.data.convert.PropertyValueConverter.FunctionPropertyValueConverter; +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.util.MethodInvocationRecorder; +import org.springframework.util.Assert; /** + * Configuration class to register a {@link PropertyValueConverter} with a {@link SimplePropertyValueConverterRegistry} + * that can be used with {@link PropertyValueConversions}. + *

+ * It is possible to register type safe converters via {@link #registerConverter(Class, Function)} + * + * 

+ * registrar.registerConverter(Person.class, Person::getName) //
+ * 		.writing(StringConverter::encrypt) //
+ * 		.reading(StringConverter::decrypt);
+ *
+ * * @author Christoph Strobl - * @since ? + * @author Oliver Drotbohm + * @since 2.7 */ -public class PropertyValueConverterRegistrar { +public class PropertyValueConverterRegistrar

> { - private final Map> converterRegistrationMap = new LinkedHashMap<>(); + private final SimplePropertyValueConverterRegistry

registry = new SimplePropertyValueConverterRegistry<>(); - boolean hasConverterFor(Class type, String path) { - return converterRegistrationMap.containsKey(new Key(type, path)); - } + /** + * Starts a converter registration by pointing to a property of a domain type. + * + * @param the domain type + * @param the property type + * @param type the domain type to obtain the property from + * @param property a function to describe the property to be referenced. Usually a method handle to a getter. + * @return will never be {@literal null}. + */ + public WritingConverterRegistrationBuilder registerConverter(Class type, Function property) { - @Nullable - public PropertyValueConverter getConverter(Class type, String path) { - return converterRegistrationMap.get(new Key(type, path)); - } + String propertyName = MethodInvocationRecorder.forProxyOf(type).record(property).getPropertyPath() + .orElseThrow(() -> new IllegalArgumentException("Cannot obtain property name!")); - public int size() { - return converterRegistrationMap.size(); + return new WritingConverterRegistrationBuilder(type, propertyName, this); } - public boolean isEmpty() { - return converterRegistrationMap.isEmpty(); + /** + * Starts a converter registration by pointing to a property of a domain type. + * + * @param the domain type + * @param the property type + * @param type the domain type to obtain the property from + * @param propertyName a function to describe the property to be referenced. Usually a method handle to a getter. + * @return will never be {@literal null}. + */ + public WritingConverterRegistrationBuilder registerConverter(Class type, String propertyName, + Class propertyType) { + return new WritingConverterRegistrationBuilder(type, propertyName, this); } - public PropertyValueConverterRegistrar register(Class type, String path, - PropertyValueConverter converter) { - - converterRegistrationMap.put(new Key(type, path), converter); + /** + * Register the given converter for the types property identified via its name. + * + * @param type the domain type to obtain the property from + * @param path the property name. + * @param converter the converter to apply. + * @return this. + */ + public PropertyValueConverterRegistrar registerConverter(Class type, String path, + PropertyValueConverter> converter) { + + registry.registerConverter(type, path, + (PropertyValueConverter>) converter); return this; } - public Collection> converters() { - return converterRegistrationMap.values(); + /** + * Obtain the {@link SimplePropertyValueConverterRegistry}. + * + * @return new instance of {@link SimplePropertyValueConverterRegistry}. + */ + public ValueConverterRegistry

buildRegistry() { + return new SimplePropertyValueConverterRegistry<>(registry); } - public PropertyValueConverterRegistrar registerIfAbsent(Class type, String path, - PropertyValueConverter converter) { - converterRegistrationMap.putIfAbsent(new Key(type, path), converter); - return this; + /** + * Register collected {@link PropertyValueConverter converters} within the given {@link ValueConverterRegistry + * registry}. + * + * @return new instance of {@link SimplePropertyValueConverterRegistry}. + */ + public void registerConvertersIn(ValueConverterRegistry

target) { + + Assert.notNull(target, "Target registry must not be null!"); + + registry.getConverterRegistrationMap().entrySet().forEach(entry -> { + target.registerConverter(entry.getKey().type, entry.getKey().path, entry.getValue()); + }); } - static class Key { + /** + * Helper to build up a fluent registration API starting on + * + * @author Oliver Drotbohm + */ + static class WritingConverterRegistrationBuilder> { + + private final Consumer registration; + private final PropertyValueConverterRegistrar config; - Class type; - String path; + public WritingConverterRegistrationBuilder(Class type, String property, PropertyValueConverterRegistrar config) { - public Key(Class type, String path) { - this.type = type; - this.path = path; + this.config = config; + this.registration = (converter) -> config.registerConverter(type, property, converter); } - @Override - public boolean equals(Object o) { - if (this == o) - return true; - if (o == null || getClass() != o.getClass()) - return false; + ReadingConverterRegistrationBuilder writingAsIs() { + return writing((source, context) -> source); + } - Key key = (Key) o; + ReadingConverterRegistrationBuilder writing(Function writer) { + return writing((source, context) -> writer.apply(source)); + } - if (!ObjectUtils.nullSafeEquals(type, key.type)) { - return false; - } - return ObjectUtils.nullSafeEquals(path, key.path); + /** + * Describes how to convert the domain property value into the database native property. + * + * @param the type to be written to the database + * @param writer the property conversion to extract a value written to the database + * @return will never be {@literal null}. + */ + ReadingConverterRegistrationBuilder writing(BiFunction, R> writer) { + return new ReadingConverterRegistrationBuilder<>(this, writer); } + } + + /** + * A helper to build a fluent API to register how to read a database value into a domain object property. + * + * @author Oliver Drotbohm + */ + static class ReadingConverterRegistrationBuilder> { + + private WritingConverterRegistrationBuilder origin; + private BiFunction, R> writer; + + public ReadingConverterRegistrationBuilder(WritingConverterRegistrationBuilder origin, + BiFunction, R> writer) { + this.origin = origin; + this.writer = writer; + } + + PropertyValueConverterRegistrar

readingAsIs() { + return reading((source, context) -> (S) source); + } + + PropertyValueConverterRegistrar

reading(Function reader) { + return reading((source, context) -> reader.apply(source)); + } + + /** + * Describes how to read a database value into a domain object's property value. + * + * @param reader must not be {@literal null}. + * @return + */ + PropertyValueConverterRegistrar

reading(BiFunction, S> reader) { + + origin.registration.accept(new FunctionPropertyValueConverter(writer, reader)); - @Override - public int hashCode() { - int result = ObjectUtils.nullSafeHashCode(type); - result = 31 * result + ObjectUtils.nullSafeHashCode(path); - return result; + return origin.config; } } } diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java index 0740fb82f9..a8b1539def 100644 --- a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java +++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java @@ -20,36 +20,69 @@ import java.util.concurrent.atomic.AtomicBoolean; import org.springframework.beans.factory.InitializingBean; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import org.springframework.data.convert.PropertyValueConverterFactories.ChainedPropertyValueConverterFactory; import org.springframework.data.mapping.PersistentProperty; import org.springframework.lang.Nullable; /** + * {@link PropertyValueConversions} implementation that allows to pick a {@link PropertyValueConverterFactory} serving + * {@link PropertyValueConverter converters}. Activating {@link #setConverterCacheEnabled(boolean) cahing} allows to + * reuse converters. Providing a {@link SimplePropertyValueConverterRegistry} adds path configured converter instances. + *

+ * Should be {@link #afterPropertiesSet() initialized}. If not, {@link #init()} will be called of fist attempt of + * {@link PropertyValueConverter converter} retrieval. + * * @author Christoph Strobl - * @since ? + * @since 2.7 */ public class SimplePropertyValueConversions implements PropertyValueConversions, InitializingBean { private @Nullable PropertyValueConverterFactory converterFactory; - private @Nullable PropertyValueConverterRegistrar converterRegistrar; + private @Nullable ValueConverterRegistry valueConverterRegistry; private boolean converterCacheEnabled = true; private AtomicBoolean initialized = new AtomicBoolean(false); + /** + * Set the {@link PropertyValueConverterFactory factory} responsible for creating the actual + * {@link PropertyValueConverter converter}. + * + * @param converterFactory must not be {@literal null}. + */ public void setConverterFactory(PropertyValueConverterFactory converterFactory) { this.converterFactory = converterFactory; } - public void setConverterRegistrar(PropertyValueConverterRegistrar converterRegistrar) { - this.converterRegistrar = converterRegistrar; + @Nullable + PropertyValueConverterFactory getConverterFactory() { + return converterFactory; + } + + /** + * Set the {@link ValueConverterRegistry converter registry} for path configured converters. This is short for adding + * a + * {@link org.springframework.data.convert.PropertyValueConverterFactories.ConfiguredInstanceServingValueConverterFactory} + * at the end of a {@link ChainedPropertyValueConverterFactory}. + * + * @param valueConverterRegistry must not be {@literal null}. + */ + public void setValueConverterRegistry(ValueConverterRegistry valueConverterRegistry) { + this.valueConverterRegistry = valueConverterRegistry; } + /** + * Dis-/Enable caching. Enabled by default. + * + * @param converterCacheEnabled set to {@literal true} to enable caching of {@link PropertyValueConverter converter} + * instances. + */ public void setConverterCacheEnabled(boolean converterCacheEnabled) { this.converterCacheEnabled = converterCacheEnabled; } + @Nullable @Override - public > PropertyValueConverter getValueConverter( - PersistentProperty property) { + public , D extends ValueConversionContext> PropertyValueConverter getValueConverter( + C property) { if (!initialized.get()) { init(); @@ -58,9 +91,13 @@ public > PropertyValueConverter factoryList = new ArrayList<>(3); if (converterFactory != null) { @@ -69,8 +106,8 @@ public void init() { factoryList.add(PropertyValueConverterFactory.simple()); } - if ((converterRegistrar != null) && !converterRegistrar.isEmpty()) { - factoryList.add(PropertyValueConverterFactory.configuredInstance(converterRegistrar)); + if ((valueConverterRegistry != null) && !valueConverterRegistry.isEmpty()) { + factoryList.add(PropertyValueConverterFactory.configuredInstance(valueConverterRegistry)); } PropertyValueConverterFactory targetFactory = factoryList.size() > 1 @@ -84,7 +121,6 @@ public void init() { @Override public void afterPropertiesSet() throws Exception { - init(); } } diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java new file mode 100644 index 0000000000..6b336eb455 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java @@ -0,0 +1,126 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.util.ObjectUtils; + +/** + * A registry of property specific {@link PropertyValueConverter value convertes} that may be used to convert only + * specific properties/values of an object. + * + * @author Christoph Strobl + * @since 2.7 + */ +public class SimplePropertyValueConverterRegistry

> implements ValueConverterRegistry

{ + + private final Map>> converterRegistrationMap = new LinkedHashMap<>(); + + public SimplePropertyValueConverterRegistry() {} + + SimplePropertyValueConverterRegistry(SimplePropertyValueConverterRegistry

source) { + this.converterRegistrationMap.putAll(source.converterRegistrationMap); + } + + @Override + public void registerConverter(Class type, String path, + PropertyValueConverter> converter) { + + converterRegistrationMap.put(new Key(type, path), converter); + } + + /** + * Register the {@link PropertyValueConverter} for the property of the given type if none had been registered before. + * + * @param type the target type. + * @param path the property name. + * @param converter the converter to register. + */ + public void registerConverterIfAbsent(Class type, String path, + PropertyValueConverter> converter) { + converterRegistrationMap.putIfAbsent(new Key(type, path), converter); + } + + @Override + public boolean containsConverterFor(Class type, String path) { + return converterRegistrationMap.containsKey(new Key(type, path)); + } + + @Override + public PropertyValueConverter> getConverter(Class type, + String path) { + + return (PropertyValueConverter>) converterRegistrationMap + .get(new Key(type, path)); + } + + /** + * @return the number of registered converters. + */ + public int size() { + return converterRegistrationMap.size(); + } + + public boolean isEmpty() { + return converterRegistrationMap.isEmpty(); + } + + /** + * Obtain the underlying (mutable) map of converters. + * + * @return never {@literal null}. + */ + Map>> getConverterRegistrationMap() { + return converterRegistrationMap; + } + + static class Key { + + Class type; + String path; + + public Key(Class type, String path) { + this.type = type; + this.path = path; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + + Key key = (Key) o; + + if (!ObjectUtils.nullSafeEquals(type, key.type)) { + return false; + } + return ObjectUtils.nullSafeEquals(path, key.path); + } + + @Override + public int hashCode() { + int result = ObjectUtils.nullSafeHashCode(type); + result = 31 * result + ObjectUtils.nullSafeHashCode(path); + return result; + } + } + +} diff --git a/src/main/java/org/springframework/data/convert/ValueConversionContext.java b/src/main/java/org/springframework/data/convert/ValueConversionContext.java new file mode 100644 index 0000000000..cf5fec26b9 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/ValueConversionContext.java @@ -0,0 +1,113 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.data.util.ClassTypeInformation; +import org.springframework.data.util.TypeInformation; +import org.springframework.lang.Nullable; + +/** + * The {@link ValueConversionContext} provides access to the store specific {@link PersistentProperty} and allows to + * call the store default conversion via the {@literal read}/{@literal write} methods. + *

+ * Store implementations should provide their own flavor of {@link ValueConversionContext} enhancing the existing API, + * implementing delegates for {@link #read(Object, TypeInformation)}, {@link #write(Object, TypeInformation)}. + * + * @author Christoph Strobl + * @author Oliver Drotbohm + */ +public interface ValueConversionContext

> { + + /** + * Return the {@link PersistentProperty} to be handled. + * + * @return will never be {@literal null}. + */ + P getProperty(); + + /** + * Write to whatever type is considered best for the given source. + * + * @param value + * @return + */ + @Nullable + default Object write(@Nullable Object value) { + return write(value, getProperty().getTypeInformation()); + } + + /** + * Write as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T write(@Nullable Object value, Class target) { + return write(value, ClassTypeInformation.from(target)); + } + + /** + * Write as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T write(@Nullable Object value, TypeInformation target) { + throw new IllegalStateException(String.format( + "%s does not provide write function that allows value conversion to target type (%s).", getClass(), target)); + } + + /** + * Reads the value into the type of the current property. + * + * @param value can be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default Object read(@Nullable Object value) { + return read(value, getProperty().getTypeInformation()); + } + + /** + * Reads the value as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T read(@Nullable Object value, Class target) { + return read(value, ClassTypeInformation.from(target)); + } + + /** + * Reads the value as the given type. + * + * @param value can be {@literal null}. + * @param target must not be {@literal null}. + * @return can be {@literal null}. + */ + @Nullable + default T read(@Nullable Object value, TypeInformation target) { + throw new IllegalStateException(String.format( + "%s does not provide write function that allows value conversion to target type (%s).", getClass(), target)); + } +} diff --git a/src/main/java/org/springframework/data/convert/PropertyConverter.java b/src/main/java/org/springframework/data/convert/ValueConverter.java similarity index 78% rename from src/main/java/org/springframework/data/convert/PropertyConverter.java rename to src/main/java/org/springframework/data/convert/ValueConverter.java index 482e4f318a..bdcf7933ae 100644 --- a/src/main/java/org/springframework/data/convert/PropertyConverter.java +++ b/src/main/java/org/springframework/data/convert/ValueConverter.java @@ -25,17 +25,23 @@ import org.springframework.data.convert.PropertyValueConverter.ObjectToObjectPropertyValueConverter; /** - * Annotation to define usage of a {@link PropertyValueConverter} to read/write the property. + * Annotation to define usage of a {@link PropertyValueConverter} to read/write the property.
+ * May be used as meta annotation utilizing {@link org.springframework.core.annotation.AliasFor}. + *

+ * The target {@link PropertyValueConverter} is typically provided via a {@link PropertyValueConverterFactory converter + * factory}. *

* Consult the store specific documentation for details and support notes. - * + * * @author Christoph Strobl - * @since ? + * @since 2.7 + * @see PropertyValueConverter + * @see PropertyValueConverterFactory */ -@Target(FIELD) +@Target({ FIELD, ANNOTATION_TYPE }) @Documented @Retention(RetentionPolicy.RUNTIME) -public @interface PropertyConverter { +public @interface ValueConverter { /** * The {@link PropertyValueConverter} type handling the value conversion of the annotated property. diff --git a/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java b/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java new file mode 100644 index 0000000000..2631a57c21 --- /dev/null +++ b/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java @@ -0,0 +1,77 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https//www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import org.springframework.data.mapping.PersistentProperty; +import org.springframework.lang.Nullable; + +/** + * A registry of property specific {@link PropertyValueConverter value convertes} that may be used to convert only + * specific properties/values of an object. + * + * @author Christoph Strobl + * @since 2.7 + */ +public interface ValueConverterRegistry

> { + + /** + * Register the {@link PropertyValueConverter} for the property of the given type. + * + * @param type the target type. Must not be {@literal null}. + * @param path the property name. Must not be {@literal null}. + * @param converter the converter to register. Must not be {@literal null}. + */ + void registerConverter(Class type, String path, + PropertyValueConverter> converter); + + /** + * Obtain the converter registered for the given type, path combination or {@literal null} if none defined. + * + * @param type the target type. Must not be {@literal null}. + * @param path the property name. Must not be {@literal null}. + * @param + * @param + * @return {@literal null} if no converter present for the given type/path combination. + */ + @Nullable + PropertyValueConverter> getConverter(Class type, String path); + + /** + * Check if a converter is registered for the given type, path combination. + * + * @param type the target type. Must not be {@literal null}. + * @param path the property name. Must not be {@literal null}. + * @return {@literal false} if no converter present for the given type/path combination. + */ + default boolean containsConverterFor(Class type, String path) { + return getConverter(type, path) != null; + } + + /** + * Check if there a converters registered. + */ + boolean isEmpty(); + + /** + * Obtain a simple {@link ValueConverterRegistry}. + * + * @param + * @return new instance of {@link ValueConverterRegistry}. + */ + static > ValueConverterRegistry simple() { + return new SimplePropertyValueConverterRegistry<>(); + } +} diff --git a/src/main/java/org/springframework/data/mapping/PersistentProperty.java b/src/main/java/org/springframework/data/mapping/PersistentProperty.java index 82a1b3e520..5ce731e21a 100644 --- a/src/main/java/org/springframework/data/mapping/PersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/PersistentProperty.java @@ -23,9 +23,9 @@ import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationUtils; -import org.springframework.data.convert.PropertyConverter; import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import org.springframework.data.convert.ValueConversionContext; +import org.springframework.data.convert.ValueConverter; import org.springframework.data.util.TypeInformation; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -36,6 +36,7 @@ * @author Oliver Gierke * @author Mark Paluch * @author Jens Schauder + * @author Christoph Strobl */ public interface PersistentProperty

> { @@ -79,9 +80,9 @@ public interface PersistentProperty

> { Iterable> getPersistentEntityTypes(); /** - * Returns the detected {@link TypeInformation TypeInformations} if the property references a {@link PersistentEntity}. - * Will return an {@literal empty} {@link Iterable} in case it refers to a simple type. Will return the {@link Collection}'s - * component types or the {@link Map}'s value type transparently. + * Returns the detected {@link TypeInformation TypeInformations} if the property references a + * {@link PersistentEntity}. Will return an {@literal empty} {@link Iterable} in case it refers to a simple type. Will + * return the {@link Collection}'s component types or the {@link Map}'s value type transparently. * * @return never {@literal null}. * @since 2.6 @@ -437,16 +438,32 @@ default PersistentPropertyAccessor getAccessorForOwner(T owner) { return getOwner().getPropertyAccessor(owner); } + /** + * Obtain the the {@link PropertyValueConverter converter type} to be used for read-/writing the properties value. By + * default looks for the {@link ValueConverter} annotation and extracts its {@link ValueConverter#value() value} + * attribute. + *

+ * Store implementations may override the default and provide a more specific type. + * + * @return {@literal null} if none defined. Check {@link #hasValueConverter()} to check if the annotation is present + * at all. + * @since 2.7 + */ @Nullable - default Class>> getValueConverterType() { + default Class>>> getValueConverterType() { - PropertyConverter annotation = findAnnotation(PropertyConverter.class); + ValueConverter annotation = findAnnotation(ValueConverter.class); return annotation == null ? null - : (Class>>) annotation.value(); + : (Class>>>) annotation + .value(); } + /** + * @return by default return {@literal true} if {@link ValueConverter} annotation is present. + * @since 2.7 + */ default boolean hasValueConverter() { - return isAnnotationPresent(PropertyConverter.class); + return isAnnotationPresent(ValueConverter.class); } } diff --git a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java index 1e8e060137..1db387a152 100644 --- a/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java +++ b/src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java @@ -33,9 +33,9 @@ import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; import org.springframework.data.annotation.Version; -import org.springframework.data.convert.PropertyConverter; +import org.springframework.data.convert.ValueConverter; import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import org.springframework.data.convert.ValueConversionContext; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentEntity; @@ -312,10 +312,10 @@ public TypeInformation getAssociationTargetTypeInformation() { @Nullable @Override @SuppressWarnings("unchecked") - public Class>> getValueConverterType() { + public Class>>> getValueConverterType() { - return doFindAnnotation(PropertyConverter.class) // - .map(PropertyConverter::value) // + return doFindAnnotation(ValueConverter.class) // + .map(ValueConverter::value) // .map(Class.class::cast) // .orElse(null); } diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java index b502145b18..c539f57ff9 100644 --- a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java @@ -24,7 +24,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.lang.Nullable; @@ -108,7 +107,7 @@ void beanFactoryAwareConverterFactoryCanLookupExistingBean() { } @Test // GH-1484 - void compositeConverterFactoryIteratesFactories() { + void chainedConverterFactoryIteratesFactories() { PropertyValueConverter expected = mock(PropertyValueConverter.class); @@ -132,7 +131,7 @@ public > PropertyValueConverter context) { - return nativeValue.toString(); + public String read(@Nullable UUID value, ValueConversionContext context) { + return value.toString(); } @Nullable @Override - public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { - return UUID.fromString(domainValue); + public UUID write(@Nullable String value, ValueConversionContext context) { + return UUID.fromString(value); } } @@ -199,14 +198,14 @@ enum ConverterEnum implements PropertyValueConverter context) { + public String read(@Nullable UUID value, ValueConversionContext context) { assertThat(someDependency).isNotNull(); - return nativeValue.toString(); + return value.toString(); } @Nullable @Override - public UUID domainToNative(@Nullable String domainValue, ValueConversionContext context) { + public UUID write(@Nullable String value, ValueConversionContext context) { assertThat(someDependency).isNotNull(); - return UUID.fromString(domainValue); + return UUID.fromString(value); } } static class SomeDependency { } + + static class Person { + String name; + Address address; + + public String getName() { + return name; + } + + public Address getAddress() { + return address; + } + } + + static class Address { + String street; + ZipCode zipCode; + } + + static class ZipCode { + + } } diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterRegistrarUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterRegistrarUnitTests.java new file mode 100644 index 0000000000..b15cb5fdb0 --- /dev/null +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterRegistrarUnitTests.java @@ -0,0 +1,107 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import static org.assertj.core.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.springframework.data.convert.PropertyValueConverterFactoryUnitTests.Person; +import org.springframework.data.mapping.context.SamplePersistentProperty; +import org.springframework.lang.Nullable; + +/** + * @author Christoph Strobl + */ +public class PropertyValueConverterRegistrarUnitTests { + + @Test // GH-1484 + void buildsRegistryCorrectly() { + + ValueConverterRegistry registry = new PropertyValueConverterRegistrar<>() // + .registerConverter(Person.class, "name", new ReversingPropertyValueConverter()) // + .buildRegistry(); // + + assertThat(registry.containsConverterFor(Person.class, "name")).isTrue(); + assertThat(registry.containsConverterFor(Person.class, "not-a-property")).isFalse(); + } + + @Test // GH-1484 + void registersConvertersInRegistryCorrectly() { + + ValueConverterRegistry registry = ValueConverterRegistry.simple(); + + new PropertyValueConverterRegistrar<>() // + .registerConverter(Person.class, "name", new ReversingPropertyValueConverter()) // + .registerConvertersIn(registry); // + + assertThat(registry.containsConverterFor(Person.class, "name")).isTrue(); + assertThat(registry.containsConverterFor(Person.class, "not-a-property")).isFalse(); + } + + @Test // GH-1484 + void allowsTypeSafeConverterRegistration() { + + PropertyValueConverterRegistrar registrar = new PropertyValueConverterRegistrar<>(); + registrar.registerConverter(Person.class, "name", String.class) // + .writing(PropertyValueConverterRegistrarUnitTests::reverse) // + .readingAsIs(); // + + PropertyValueConverter> name = registrar + .buildRegistry().getConverter(Person.class, "name"); + assertThat(name.write("foo", null)).isEqualTo("oof"); + assertThat(name.read("off", null)).isEqualTo("off"); + } + + @Test // GH-1484 + void allowsTypeSafeConverterRegistrationViaRecordedProperty() { + + PropertyValueConverterRegistrar registrar = new PropertyValueConverterRegistrar<>(); + registrar.registerConverter(Person.class, Person::getName) // + .writing(PropertyValueConverterRegistrarUnitTests::reverse) // + .readingAsIs(); + + PropertyValueConverter> name = registrar + .buildRegistry().getConverter(Person.class, "name"); + assertThat(name.write("foo", null)).isEqualTo("oof"); + assertThat(name.read("мир", null)).isEqualTo("мир"); + } + + static class ReversingPropertyValueConverter + implements PropertyValueConverter> { + + @Nullable + @Override + public String read(@Nullable String value, ValueConversionContext context) { + return PropertyValueConverterRegistrarUnitTests.reverse(value); + } + + @Nullable + @Override + public String write(@Nullable String value, ValueConversionContext context) { + return PropertyValueConverterRegistrarUnitTests.reverse(value); + } + } + + @Nullable + static String reverse(@Nullable String source) { + + if (source == null) { + return null; + } + + return new StringBuilder(source).reverse().toString(); + } +} diff --git a/src/test/java/org/springframework/data/convert/SimplePropertyValueConversionsUnitTests.java b/src/test/java/org/springframework/data/convert/SimplePropertyValueConversionsUnitTests.java new file mode 100644 index 0000000000..047d2dd76a --- /dev/null +++ b/src/test/java/org/springframework/data/convert/SimplePropertyValueConversionsUnitTests.java @@ -0,0 +1,69 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.CachingPropertyValueConverterFactory; +import org.springframework.data.convert.PropertyValueConverterFactories.ChainedPropertyValueConverterFactory; + +/** + * @author Christoph Strobl + */ +class SimplePropertyValueConversionsUnitTests { + + @Test // GH-1484 + void decoratesTargetFactoryWithCacheWhenCachingIsEnabled() { + + SimplePropertyValueConversions conversions = new SimplePropertyValueConversions(); + conversions.setConverterFactory(PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class))); + conversions.setConverterCacheEnabled(true); + + conversions.init(); + assertThat(conversions.getConverterFactory()).isInstanceOf(CachingPropertyValueConverterFactory.class); + } + + @Test // GH-1484 + void doesNotDecorateTargetFactoryWithCacheWhenCachingIsDisabled() { + + PropertyValueConverterFactory factory = PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class)); + + SimplePropertyValueConversions conversions = new SimplePropertyValueConversions(); + conversions.setConverterFactory(factory); + conversions.setConverterCacheEnabled(false); + + conversions.init(); + assertThat(conversions.getConverterFactory()).isSameAs(factory); + } + + @Test // GH-1484 + void chainsFactoriesIfConverterRegistryPresent() { + + ValueConverterRegistry registry = mock(ValueConverterRegistry.class); + PropertyValueConverterFactory factory = PropertyValueConverterFactory.beanFactoryAware(mock(BeanFactory.class)); + + SimplePropertyValueConversions conversions = new SimplePropertyValueConversions(); + conversions.setConverterFactory(factory); + conversions.setValueConverterRegistry(registry); + conversions.setConverterCacheEnabled(false); + + conversions.init(); + assertThat(conversions.getConverterFactory()).isInstanceOf(ChainedPropertyValueConverterFactory.class); + } +} diff --git a/src/test/java/org/springframework/data/convert/SimplePropertyValueConverterRegistryUnitTests.java b/src/test/java/org/springframework/data/convert/SimplePropertyValueConverterRegistryUnitTests.java new file mode 100644 index 0000000000..5eea00e75d --- /dev/null +++ b/src/test/java/org/springframework/data/convert/SimplePropertyValueConverterRegistryUnitTests.java @@ -0,0 +1,88 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.convert; + +import static org.assertj.core.api.Assertions.*; +import static org.mockito.Mockito.*; + +import org.junit.jupiter.api.Test; +import org.springframework.data.mapping.Person; + +/** + * @author Christoph Strobl + */ +class SimplePropertyValueConverterRegistryUnitTests { + + @Test // GH-1484 + void emptyRegistryDoesNotServeConverters() { + + SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry(); + + assertThat(registry.isEmpty()).isTrue(); + assertThat(registry.size()).isZero(); + assertThat(registry.containsConverterFor(Person.class, "name")).isFalse(); + assertThat(registry.getConverter(Person.class, "name")).isNull(); + } + + @Test // GH-1484 + void registryCopiesOverConverters() { + + SimplePropertyValueConverterRegistry sourceRegistry = new SimplePropertyValueConverterRegistry(); + sourceRegistry.registerConverter(Person.class, "name", mock(PropertyValueConverter.class)); + + SimplePropertyValueConverterRegistry targetRegistry = new SimplePropertyValueConverterRegistry(sourceRegistry); + assertThat(targetRegistry.size()).isOne(); + + sourceRegistry.registerConverter(Address.class, "street", mock(PropertyValueConverter.class)); + assertThat(sourceRegistry.size()).isEqualTo(2); + assertThat(targetRegistry.size()).isOne(); + } + + @Test // GH-1484 + void registryServesMatchingConverter() { + + SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry(); + registry.registerConverter(Person.class, "name", mock(PropertyValueConverter.class)); + + assertThat(registry.isEmpty()).isFalse(); + assertThat(registry.size()).isOne(); + + assertThat(registry.containsConverterFor(Person.class, "name")).isTrue(); + assertThat(registry.getConverter(Person.class, "name")).isNotNull(); + + assertThat(registry.containsConverterFor(Person.class, "age")).isFalse(); + assertThat(registry.getConverter(Person.class, "age")).isNull(); + + assertThat(registry.getConverter(Address.class, "name")).isNull(); + } + + @Test // GH-1484 + void registryMayHoldConvertersForDifferentPropertiesOfSameType() { + + PropertyValueConverter nameConverter = mock(PropertyValueConverter.class); + PropertyValueConverter ageConverter = mock(PropertyValueConverter.class); + + SimplePropertyValueConverterRegistry registry = new SimplePropertyValueConverterRegistry(); + registry.registerConverter(Person.class, "name", nameConverter); + registry.registerConverter(Person.class, "age", ageConverter); + + assertThat(registry.getConverter(Person.class, "name")).isSameAs(nameConverter); + assertThat(registry.getConverter(Person.class, "age")).isSameAs(ageConverter); + } + + static class Address {} + +} diff --git a/src/test/java/org/springframework/data/convert/WhatWeWant.java b/src/test/java/org/springframework/data/convert/WhatWeWant.java index 7ca616af49..2f4a14a1e3 100644 --- a/src/test/java/org/springframework/data/convert/WhatWeWant.java +++ b/src/test/java/org/springframework/data/convert/WhatWeWant.java @@ -1,19 +1,3 @@ -/* - * Copyright 2022. the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - /* * Copyright 2022 the original author or authors. * @@ -21,7 +5,7 @@ * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * https://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -39,7 +23,6 @@ import java.util.function.Predicate; import org.junit.jupiter.api.Test; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.PropertyPath; import org.springframework.data.mapping.context.SamplePersistentProperty; @@ -87,13 +70,13 @@ void converterConfig() { @Nullable @Override - public Object nativeToDomain(@Nullable Object nativeValue, ValueConversionContext context) { + public Object read(@Nullable Object value, ValueConversionContext context) { return null; } @Nullable @Override - public Object domainToNative(@Nullable Object domainValue, ValueConversionContext context) { + public Object write(@Nullable Object value, ValueConversionContext context) { return null; } }); @@ -238,13 +221,13 @@ public FunctionPropertyValueConverter(BiFunction, B } @Override - public B domainToNative(A domainValue, ValueConversionContext

context) { - return writer.apply(domainValue, context); + public B write(A value, ValueConversionContext

context) { + return writer.apply(value, context); } @Override - public A nativeToDomain(B nativeValue, ValueConversionContext

context) { - return reader.apply(nativeValue, context); + public A read(B value, ValueConversionContext

context) { + return reader.apply(value, context); } } } diff --git a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java index d15a5ccb14..67dfb19065 100755 --- a/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AbstractPersistentPropertyUnitTests.java @@ -36,7 +36,6 @@ import org.jmolecules.ddd.types.Identifier; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; @@ -44,7 +43,6 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.Optionals; import org.springframework.data.util.TypeInformation; -import org.springframework.lang.Nullable; import org.springframework.util.ReflectionUtils; /** @@ -387,8 +385,6 @@ public boolean isAnnotationPresent(Class annotationType) { public A findPropertyOrOwnerAnnotation(Class annotationType) { return null; } - - } static class Sample { diff --git a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java index 702e1e4798..c2acacfb1f 100755 --- a/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java @@ -36,7 +36,6 @@ import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.annotation.AliasFor; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.annotation.AccessType; @@ -45,9 +44,9 @@ import org.springframework.data.annotation.ReadOnlyProperty; import org.springframework.data.annotation.Reference; import org.springframework.data.annotation.Transient; -import org.springframework.data.convert.PropertyConverter; import org.springframework.data.convert.PropertyValueConverter; -import org.springframework.data.convert.PropertyValueConverter.ValueConversionContext; +import org.springframework.data.convert.ValueConversionContext; +import org.springframework.data.convert.ValueConverter; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SampleMappingContext; @@ -342,6 +341,15 @@ void detectsJMoleculesIdentity() { assertThat(property.isIdProperty()).isTrue(); } + @Test // GH-1484 + void detectsValueConverter() { + + SamplePersistentProperty property = getProperty(WithPropertyConverter.class, "value"); + + assertThat(property.hasValueConverter()).isTrue(); + assertThat(property.getValueConverterType()).isEqualTo(MyPropertyConverter.class); + } + @SuppressWarnings("unchecked") private Map, Annotation> getAnnotationCache(SamplePersistentProperty property) { return (Map, Annotation>) ReflectionTestUtils.getField(property, "annotationCache"); @@ -536,48 +544,21 @@ interface JMoleculesAggregate extends AggregateRoot> { @Override - public Object nativeToDomain(Object value, ValueConversionContext context) { - return null; - } - - @Override - public Object domainToNative(Object value, ValueConversionContext context) { - return null; - } - } - - static class MyPropertyConverterThatRequiresComponents - implements PropertyValueConverter> { - - private final SomeDependency someDependency; - - public MyPropertyConverterThatRequiresComponents(@Autowired SomeDependency someDependency) { - this.someDependency = someDependency; - } - - @Override - public Object nativeToDomain(Object value, ValueConversionContext context) { + public Object read(Object value, ValueConversionContext context) { return null; } @Override - public Object domainToNative(Object value, ValueConversionContext context) { + public Object write(Object value, ValueConversionContext context) { return null; } } - - static class SomeDependency { - - } } From ee5811486a2a4517a4208b1b4f06878fde99f21a Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 2 Mar 2022 11:36:55 +0100 Subject: [PATCH 8/9] Open API a bit and allow cache absent converters as well. --- .../PropertyValueConverterFactories.java | 28 ++++++++++--------- .../SimplePropertyValueConversions.java | 12 +++++++- ...ropertyValueConverterFactoryUnitTests.java | 23 +++++++++++++++ 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java index dde48e7d80..dac34508d5 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java @@ -15,7 +15,6 @@ */ package org.springframework.data.convert; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; @@ -23,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import org.springframework.beans.BeanUtils; import org.springframework.beans.factory.BeanFactory; @@ -194,43 +194,45 @@ static class CachingPropertyValueConverterFactory implements PropertyValueConver public > PropertyValueConverter getConverter( PersistentProperty property) { - PropertyValueConverter converter = cache.get(property); + Optional>> converter = cache.get(property); - return converter != null ? converter : cache.cache(property, delegate.getConverter(property)); + return converter != null ? (PropertyValueConverter) converter.orElse(null) + : cache.cache(property, delegate.getConverter(property)); } @Override public > PropertyValueConverter getConverter( Class> converterType) { - PropertyValueConverter converter = cache.get(converterType); + Optional>> converter = cache.get(converterType); - return converter != null ? converter : cache.cache(converterType, delegate.getConverter(converterType)); + return converter != null ? (PropertyValueConverter) converter.orElse(null) + : cache.cache(converterType, delegate.getConverter(converterType)); } static class Cache { - Map, PropertyValueConverter>> perPropertyCache = new HashMap<>(); - Map, PropertyValueConverter>> typeCache = new HashMap<>(); + Map, Optional>>> perPropertyCache = new HashMap<>(); + Map, Optional>>> typeCache = new HashMap<>(); - PropertyValueConverter> get(PersistentProperty property) { + Optional>> get(PersistentProperty property) { return perPropertyCache.get(property); } - PropertyValueConverter> get(Class type) { + Optional>> get(Class type) { return typeCache.get(type); } > PropertyValueConverter cache(PersistentProperty property, - PropertyValueConverter converter) { - perPropertyCache.putIfAbsent(property, converter); + @Nullable PropertyValueConverter converter) { + perPropertyCache.putIfAbsent(property, Optional.ofNullable(converter)); cache(property.getValueConverterType(), converter); return converter; } > PropertyValueConverter cache(Class type, - PropertyValueConverter converter) { - typeCache.putIfAbsent(type, converter); + @Nullable PropertyValueConverter converter) { + typeCache.putIfAbsent(type, Optional.ofNullable(converter)); return converter; } } diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java index a8b1539def..b93f3e660d 100644 --- a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java +++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java @@ -53,7 +53,7 @@ public void setConverterFactory(PropertyValueConverterFactory converterFactory) } @Nullable - PropertyValueConverterFactory getConverterFactory() { + public PropertyValueConverterFactory getConverterFactory() { return converterFactory; } @@ -69,6 +69,16 @@ public void setValueConverterRegistry(ValueConverterRegistry valueConverterRe this.valueConverterRegistry = valueConverterRegistry; } + /** + * Get the {@link ValueConverterRegistry} used for path configured converters. + * + * @return can be {@literal null}. + */ + @Nullable + public ValueConverterRegistry getValueConverterRegistry() { + return valueConverterRegistry; + } + /** * Dis-/Enable caching. Enabled by default. * diff --git a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java index c539f57ff9..7a84aed72c 100644 --- a/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java +++ b/src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java @@ -21,9 +21,11 @@ import java.util.UUID; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.data.mapping.PersistentEntity; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.mapping.context.SamplePersistentProperty; import org.springframework.lang.Nullable; @@ -162,6 +164,27 @@ void cachingConverterFactoryServesCachedInstance() { .isSameAs(factory.getConverter(ConverterWithDefaultCtor.class)); } + @Test // GH-1484 + void cachingConverterFactoryAlsoCachesAbsenceOfConverter() { + + PropertyValueConverterFactory source = Mockito.spy(PropertyValueConverterFactory.simple()); + PropertyValueConverterFactory factory = PropertyValueConverterFactory.caching(source); + + PersistentEntity entity = mock(PersistentEntity.class); + PersistentProperty property = mock(PersistentProperty.class); + when(property.getOwner()).thenReturn(entity); + when(entity.getType()).thenReturn(Person.class); + when(property.getName()).thenReturn("firstname"); + + // fill the cache + assertThat(factory.getConverter(property)).isNull(); + verify(source).getConverter(any(PersistentProperty.class)); + + // now get the cached null value + assertThat(factory.getConverter(property)).isNull(); + verify(source).getConverter(any(PersistentProperty.class)); + } + @Test // GH-1484 void cachingConverterFactoryServesCachedInstanceForProperty() { From f23115e205b550c156b5ccd9a2afa45e0553fc80 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 4 Mar 2022 09:13:37 +0100 Subject: [PATCH 9/9] Polishing. Tweak Javadoc. Increase visibility of converter builders. --- .../data/convert/PropertyValueConverter.java | 22 ++++++----- .../PropertyValueConverterRegistrar.java | 39 ++++++++++--------- .../SimplePropertyValueConverterRegistry.java | 8 ++-- .../data/convert/ValueConversionContext.java | 14 ++++++- .../data/convert/ValueConverter.java | 6 +-- .../data/convert/ValueConverterRegistry.java | 6 +-- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java index 7c0ad8834e..dd8d5d610d 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverter.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverter.java @@ -21,11 +21,11 @@ import org.springframework.lang.Nullable; /** - * {@link PropertyValueConverter} provides a symmetric way of converting certain properties from domain to store - * specific values. + * {@link PropertyValueConverter} provides a symmetric way of converting certain properties from domain to + * store-specific values. *

* A {@link PropertyValueConverter} is, other than a {@link ReadingConverter} or {@link WritingConverter}, only applied - * to special annotated fields which allows a fine grained conversion of certain values within a specific context. + * to special annotated fields which allows a fine-grained conversion of certain values within a specific context. * * @author Christoph Strobl * @param domain specific type. @@ -36,7 +36,7 @@ public interface PropertyValueConverter>> { /** - * Convert the given store specific value into it's domain value representation. Typically a {@literal read} + * Convert the given store specific value into it's domain value representation. Typically, a {@literal read} * operation. * * @param value can be {@literal null}. @@ -47,7 +47,7 @@ public interface PropertyValueConverter, B this.reader = reader; } + @Nullable @Override - public B write(A value, ValueConversionContext

context) { + public B write(@Nullable A value, ValueConversionContext

context) { return writer.apply(value, context); } + @Nullable @Override - public A read(B value, ValueConversionContext

context) { + public A read(@Nullable B value, ValueConversionContext

context) { return reader.apply(value, context); } } diff --git a/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java index b7fb46364a..d4207d4131 100644 --- a/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java +++ b/src/main/java/org/springframework/data/convert/PropertyValueConverterRegistrar.java @@ -29,7 +29,7 @@ * that can be used with {@link PropertyValueConversions}. *

* It is possible to register type safe converters via {@link #registerConverter(Class, Function)} - * + * * 

  * registrar.registerConverter(Person.class, Person::getName) //
  * 		.writing(StringConverter::encrypt) //
@@ -58,7 +58,7 @@ public  WritingConverterRegistrationBuilder registerConverter(Cla
 		String propertyName = MethodInvocationRecorder.forProxyOf(type).record(property).getPropertyPath()
 				.orElseThrow(() -> new IllegalArgumentException("Cannot obtain property name!"));
 
-		return new WritingConverterRegistrationBuilder(type, propertyName, this);
+		return new WritingConverterRegistrationBuilder<>(type, propertyName, this);
 	}
 
 	/**
@@ -71,8 +71,8 @@ public  WritingConverterRegistrationBuilder registerConverter(Cla
 	 * @return will never be {@literal null}.
 	 */
 	public  WritingConverterRegistrationBuilder registerConverter(Class type, String propertyName,
-			Class propertyType) {
-		return new WritingConverterRegistrationBuilder(type, propertyName, this);
+			@SuppressWarnings("unused") Class propertyType) {
+		return new WritingConverterRegistrationBuilder<>(type, propertyName, this);
 	}
 
 	/**
@@ -83,7 +83,7 @@ public  WritingConverterRegistrationBuilder registerConverter(Cla
 	 * @param converter the converter to apply.
 	 * @return this.
 	 */
-	public PropertyValueConverterRegistrar registerConverter(Class type, String path,
+	public PropertyValueConverterRegistrar
 registerConverter(Class type, String path,
 			PropertyValueConverter> converter) {
 
 		registry.registerConverter(type, path,
@@ -120,22 +120,22 @@ public void registerConvertersIn(ValueConverterRegistry
 target) {
 	 *
 	 * @author Oliver Drotbohm
 	 */
-	static class WritingConverterRegistrationBuilder> {
+	public static class WritingConverterRegistrationBuilder> {
 
-		private final Consumer registration;
-		private final PropertyValueConverterRegistrar config;
+		private final Consumer>> registration;
+		private final PropertyValueConverterRegistrar
 config;
 
-		public WritingConverterRegistrationBuilder(Class type, String property, PropertyValueConverterRegistrar config) {
+		WritingConverterRegistrationBuilder(Class type, String property, PropertyValueConverterRegistrar
 config) {
 
 			this.config = config;
 			this.registration = (converter) -> config.registerConverter(type, property, converter);
 		}
 
-		 ReadingConverterRegistrationBuilder writingAsIs() {
+		public ReadingConverterRegistrationBuilder writingAsIs() {
 			return writing((source, context) -> source);
 		}
 
-		 ReadingConverterRegistrationBuilder writing(Function writer) {
+		public  ReadingConverterRegistrationBuilder writing(Function writer) {
 			return writing((source, context) -> writer.apply(source));
 		}
 
@@ -146,7 +146,8 @@  ReadingConverterRegistrationBuilder writing(Function write
 		 * @param writer the property conversion to extract a value written to the database
 		 * @return will never be {@literal null}.
 		 */
-		 ReadingConverterRegistrationBuilder writing(BiFunction, R> writer) {
+		public  ReadingConverterRegistrationBuilder writing(
+				BiFunction, R> writer) {
 			return new ReadingConverterRegistrationBuilder<>(this, writer);
 		}
 	}
@@ -156,22 +157,22 @@  ReadingConverterRegistrationBuilder writing(BiFunction> {
+	public static class ReadingConverterRegistrationBuilder> {
 
-		private WritingConverterRegistrationBuilder origin;
-		private BiFunction, R> writer;
+		private final WritingConverterRegistrationBuilder origin;
+		private final BiFunction, R> writer;
 
-		public ReadingConverterRegistrationBuilder(WritingConverterRegistrationBuilder origin,
+		ReadingConverterRegistrationBuilder(WritingConverterRegistrationBuilder origin,
 				BiFunction, R> writer) {
 			this.origin = origin;
 			this.writer = writer;
 		}
 
-		PropertyValueConverterRegistrar
 readingAsIs() {
+		public PropertyValueConverterRegistrar
 readingAsIs() {
 			return reading((source, context) -> (S) source);
 		}
 
-		PropertyValueConverterRegistrar
 reading(Function reader) {
+		public PropertyValueConverterRegistrar
 reading(Function reader) {
 			return reading((source, context) -> reader.apply(source));
 		}
 
@@ -181,7 +182,7 @@ PropertyValueConverterRegistrar
 reading(Function reader) {
 		 * @param reader must not be {@literal null}.
 		 * @return
 		 */
-		PropertyValueConverterRegistrar
 reading(BiFunction, S> reader) {
+		public PropertyValueConverterRegistrar
 reading(BiFunction, S> reader) {
 
 			origin.registration.accept(new FunctionPropertyValueConverter(writer, reader));
 
diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java
index 6b336eb455..9a26a195d4 100644
--- a/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java
+++ b/src/main/java/org/springframework/data/convert/SimplePropertyValueConverterRegistry.java
@@ -24,7 +24,7 @@
 /**
  * A registry of property specific {@link PropertyValueConverter value convertes} that may be used to convert only
  * specific properties/values of an object.
- * 
+ *
  * @author Christoph Strobl
  * @since 2.7
  */
@@ -83,7 +83,7 @@ public boolean isEmpty() {
 
 	/**
 	 * Obtain the underlying (mutable) map of converters.
-	 * 
+	 *
 	 * @return never {@literal null}.
 	 */
 	Map>> getConverterRegistrationMap() {
@@ -92,8 +92,8 @@ public boolean isEmpty() {
 
 	static class Key {
 
-		Class type;
-		String path;
+		final Class type;
+		final String path;
 
 		public Key(Class type, String path) {
 			this.type = type;
diff --git a/src/main/java/org/springframework/data/convert/ValueConversionContext.java b/src/main/java/org/springframework/data/convert/ValueConversionContext.java
index cf5fec26b9..049485af08 100644
--- a/src/main/java/org/springframework/data/convert/ValueConversionContext.java
+++ b/src/main/java/org/springframework/data/convert/ValueConversionContext.java
@@ -21,8 +21,8 @@
 import org.springframework.lang.Nullable;
 
 /**
- * The {@link ValueConversionContext} provides access to the store specific {@link PersistentProperty} and allows to
- * call the store default conversion via the {@literal read}/{@literal write} methods.
+ * The {@link ValueConversionContext} provides access to the store-specific {@link PersistentProperty} and allows to
+ * call the store-default conversion through the {@literal read}/{@literal write} methods.
  * 

  * Store implementations should provide their own flavor of {@link ValueConversionContext} enhancing the existing API,
  * implementing delegates for {@link #read(Object, TypeInformation)}, {@link #write(Object, TypeInformation)}.
@@ -71,6 +71,11 @@ default  T write(@Nullable Object value, Class target) {
 	 */
 	@Nullable
 	default  T write(@Nullable Object value, TypeInformation target) {
+
+		if (value == null || target.getType().isInstance(value)) {
+			return target.getType().cast(value);
+		}
+
 		throw new IllegalStateException(String.format(
 				"%s does not provide write function that allows value conversion to target type (%s).", getClass(), target));
 	}
@@ -107,6 +112,11 @@ default  T read(@Nullable Object value, Class target) {
 	 */
 	@Nullable
 	default  T read(@Nullable Object value, TypeInformation target) {
+
+		if (value == null || target.getType().isInstance(value)) {
+			return target.getType().cast(value);
+		}
+
 		throw new IllegalStateException(String.format(
 				"%s does not provide write function that allows value conversion to target type (%s).", getClass(), target));
 	}
diff --git a/src/main/java/org/springframework/data/convert/ValueConverter.java b/src/main/java/org/springframework/data/convert/ValueConverter.java
index bdcf7933ae..f345ea7912 100644
--- a/src/main/java/org/springframework/data/convert/ValueConverter.java
+++ b/src/main/java/org/springframework/data/convert/ValueConverter.java
@@ -26,13 +26,13 @@
 
 /**
  * Annotation to define usage of a {@link PropertyValueConverter} to read/write the property. 

- * May be used as meta annotation utilizing {@link org.springframework.core.annotation.AliasFor}.
+ * Can be used as meta annotation utilizing {@link org.springframework.core.annotation.AliasFor}.
  * 

  * The target {@link PropertyValueConverter} is typically provided via a {@link PropertyValueConverterFactory converter
  * factory}.
  * 

- * Consult the store specific documentation for details and support notes.
- * 
+ * Consult the store-specific documentation for details and support notes.
+ *
  * @author Christoph Strobl
  * @since 2.7
  * @see PropertyValueConverter
diff --git a/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java b/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java
index 2631a57c21..2ad9c7a2e7 100644
--- a/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java
+++ b/src/main/java/org/springframework/data/convert/ValueConverterRegistry.java
@@ -19,8 +19,8 @@
 import org.springframework.lang.Nullable;
 
 /**
- * A registry of property specific {@link PropertyValueConverter value convertes} that may be used to convert only
- * specific properties/values of an object.
+ * A registry of property-specific {@link PropertyValueConverter value converters} to convert only specific
+ * properties/values of an object.
  *
  * @author Christoph Strobl
  * @since 2.7
@@ -61,7 +61,7 @@ default boolean containsConverterFor(Class type, String path) {
 	}
 
 	/**
-	 * Check if there a converters registered.
+	 * Check if converters are registered.
 	 */
 	boolean isEmpty();