Skip to content

Commit e25fb0b

Browse files
mp911demp911de
committed
Remove package cycle between mapping and convert.
Remove accessor methods to obtain annotated converters on PersistentProperty. Closes #2576
1 parent 0792b74 commit e25fb0b

7 files changed

+77
-95
lines changed

‎src/main/java/org/springframework/data/convert/AnnotatedPropertyValueConverterAccessor.java

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright 2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.convert;
17+
18+
import org.springframework.data.mapping.PersistentProperty;
19+
import org.springframework.lang.Nullable;
20+
import org.springframework.util.Assert;
21+
22+
/**
23+
* Accessor to obtain metadata for {@link PropertyValueConverter} from an annotated {@link PersistentProperty}.
24+
*
25+
* @author Mark Paluch
26+
* @since 2.7
27+
*/
28+
class AnnotatedPropertyValueConverterAccessor {
29+
30+
private final ValueConverter annotation;
31+
32+
public AnnotatedPropertyValueConverterAccessor(PersistentProperty<?> property) {
33+
34+
Assert.notNull(property, "PersistentProperty must not be null");
35+
annotation = property.findAnnotation(ValueConverter.class);
36+
}
37+
38+
/**
39+
* Obtain the {@link PropertyValueConverter converter type} to be used for reading and writing property values. Uses
40+
* the {@link ValueConverter} annotation and extracts its {@link ValueConverter#value() value} attribute.
41+
*
42+
* @return {@literal null} if none defined. Check {@link #hasValueConverter()} to check if the annotation is present
43+
* at all.
44+
*/
45+
@Nullable
46+
@SuppressWarnings({ "unchecked", "rawtypes" })
47+
public Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> getValueConverterType() {
48+
return annotation != null ? (Class) annotation.value() : null;
49+
}
50+
51+
/**
52+
* Return whether a value converter is configured. Uses {@link ValueConverter} as annotation type.
53+
*
54+
* @return {@literal true} if a value converter is configured.
55+
*/
56+
public boolean hasValueConverter() {
57+
return annotation != null;
58+
}
59+
60+
}

‎src/main/java/org/springframework/data/convert/PropertyValueConverterFactories.java

+3-10
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package org.springframework.data.convert;
1717

18-
import java.util.Arrays;
1918
import java.util.Collections;
2019
import java.util.EnumSet;
2120
import java.util.List;
@@ -53,10 +52,6 @@ static class ChainedPropertyValueConverterFactory implements PropertyValueConver
5352

5453
private List<PropertyValueConverterFactory> delegates;
5554

56-
ChainedPropertyValueConverterFactory(PropertyValueConverterFactory... delegates) {
57-
this(Arrays.asList(delegates));
58-
}
59-
6055
ChainedPropertyValueConverterFactory(List<PropertyValueConverterFactory> delegates) {
6156
this.delegates = Collections.unmodifiableList(delegates);
6257
}
@@ -75,10 +70,6 @@ public <S, T, C extends ValueConversionContext<?>> PropertyValueConverter<S, T,
7570
return delegates.stream().filter(it -> it.getConverter(converterType) != null).findFirst()
7671
.map(it -> it.getConverter(converterType)).orElse(null);
7772
}
78-
79-
public List<PropertyValueConverterFactory> converterFactories() {
80-
return delegates;
81-
}
8273
}
8374

8475
/**
@@ -221,9 +212,11 @@ static class Cache {
221212

222213
<S, T, C extends ValueConversionContext<?>> PropertyValueConverter<S, T, C> cache(PersistentProperty<?> property,
223214
@Nullable PropertyValueConverter<S, T, C> converter) {
215+
224216
perPropertyCache.putIfAbsent(property, Optional.ofNullable(converter));
225217

226-
Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> valueConverterType = property
218+
AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property);
219+
Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> valueConverterType = accessor
227220
.getValueConverterType();
228221
if (valueConverterType != null) {
229222
cache(valueConverterType, converter);

‎src/main/java/org/springframework/data/convert/PropertyValueConverterFactory.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ public interface PropertyValueConverterFactory {
5454
default <DV, SV, C extends ValueConversionContext<?>> PropertyValueConverter<DV, SV, C> getConverter(
5555
PersistentProperty<?> property) {
5656

57-
if (!property.hasValueConverter()) {
57+
AnnotatedPropertyValueConverterAccessor accessor = new AnnotatedPropertyValueConverterAccessor(property);
58+
59+
if (!accessor.hasValueConverter()) {
5860
return null;
5961
}
6062

61-
return getConverter((Class<PropertyValueConverter<DV, SV, C>>) property.getValueConverterType());
63+
return getConverter((Class<PropertyValueConverter<DV, SV, C>>) accessor.getValueConverterType());
6264
}
6365

6466
/**

‎src/main/java/org/springframework/data/mapping/PersistentProperty.java

-33
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323

2424
import org.springframework.core.annotation.AnnotatedElementUtils;
2525
import org.springframework.core.annotation.AnnotationUtils;
26-
import org.springframework.data.convert.PropertyValueConverter;
27-
import org.springframework.data.convert.ValueConversionContext;
28-
import org.springframework.data.convert.ValueConverter;
2926
import org.springframework.data.util.TypeInformation;
3027
import org.springframework.lang.Nullable;
3128
import org.springframework.util.Assert;
@@ -427,34 +424,4 @@ default <T> PersistentPropertyAccessor<T> getAccessorForOwner(T owner) {
427424
return getOwner().getPropertyAccessor(owner);
428425
}
429426

430-
/**
431-
* Obtain the {@link PropertyValueConverter converter type} to be used for reading and writing property values. Uses
432-
* the {@link ValueConverter} annotation and extracts its {@link ValueConverter#value() value} attribute.
433-
* <p>
434-
* Store implementations may override the default and resort to a more specific annotation type.
435-
*
436-
* @return {@literal null} if none defined. Check {@link #hasValueConverter()} to check if the annotation is present
437-
* at all.
438-
* @since 2.7
439-
*/
440-
@Nullable
441-
@SuppressWarnings({ "unchecked", "rawtypes" })
442-
default Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> getValueConverterType() {
443-
444-
ValueConverter annotation = findAnnotation(ValueConverter.class);
445-
446-
return annotation == null ? null : (Class) annotation.value();
447-
}
448-
449-
/**
450-
* Return whether a value converter is configured. Uses {@link ValueConverter} as annotation type.
451-
* <p>
452-
* Store implementations may override the default and resort to a more specific annotation type.
453-
*
454-
* @return {@literal true} if a value converter is configured.
455-
* @since 2.7
456-
*/
457-
default boolean hasValueConverter() {
458-
return isAnnotationPresent(ValueConverter.class);
459-
}
460427
}

‎src/main/java/org/springframework/data/mapping/model/AnnotationBasedPersistentProperty.java

-14
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@
3333
import org.springframework.data.annotation.Reference;
3434
import org.springframework.data.annotation.Transient;
3535
import org.springframework.data.annotation.Version;
36-
import org.springframework.data.convert.PropertyValueConverter;
37-
import org.springframework.data.convert.ValueConversionContext;
38-
import org.springframework.data.convert.ValueConverter;
3936
import org.springframework.data.mapping.Association;
4037
import org.springframework.data.mapping.MappingException;
4138
import org.springframework.data.mapping.PersistentEntity;
@@ -285,17 +282,6 @@ public TypeInformation<?> getAssociationTargetTypeInformation() {
285282
return associationTargetType.getNullable();
286283
}
287284

288-
@Nullable
289-
@Override
290-
@SuppressWarnings("unchecked")
291-
public Class<? extends PropertyValueConverter<?, ?, ? extends ValueConversionContext<? extends PersistentProperty<?>>>> getValueConverterType() {
292-
293-
return doFindAnnotation(ValueConverter.class) //
294-
.map(ValueConverter::value) //
295-
.map(Class.class::cast) //
296-
.orElse(null);
297-
}
298-
299285
@Override
300286
public String toString() {
301287

‎src/test/java/org/springframework/data/convert/PropertyValueConverterFactoryUnitTests.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
* Unit tests for {@link PropertyValueConverterFactory}.
3535
*
3636
* @author Christoph Strobl
37+
* @author Mark Paluch
3738
*/
3839
public class PropertyValueConverterFactoryUnitTests {
3940

@@ -48,8 +49,9 @@ void simpleConverterFactoryCanInstantiateFactoryWithDefaultCtor() {
4849
void simpleConverterFactoryReadsConverterFromAnnotation() {
4950

5051
PersistentProperty property = mock(PersistentProperty.class);
51-
when(property.hasValueConverter()).thenReturn(true);
52-
when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class);
52+
ValueConverter annotation = mock(ValueConverter.class);
53+
when(annotation.value()).thenReturn((Class) ConverterWithDefaultCtor.class);
54+
when(property.findAnnotation(ValueConverter.class)).thenReturn(annotation);
5355

5456
assertThat(PropertyValueConverterFactory.simple().getConverter(property))
5557
.isInstanceOf(ConverterWithDefaultCtor.class);
@@ -191,8 +193,9 @@ void cachingConverterFactoryAlsoCachesAbsenceOfConverter() {
191193
void cachingConverterFactoryServesCachedInstanceForProperty() {
192194

193195
PersistentProperty property = mock(PersistentProperty.class);
194-
when(property.hasValueConverter()).thenReturn(true);
195-
when(property.getValueConverterType()).thenReturn(ConverterWithDefaultCtor.class);
196+
ValueConverter annotation = mock(ValueConverter.class);
197+
when(annotation.value()).thenReturn((Class) ConverterWithDefaultCtor.class);
198+
when(property.findAnnotation(ValueConverter.class)).thenReturn(annotation);
196199

197200
PropertyValueConverterFactory factory = PropertyValueConverterFactory
198201
.caching(PropertyValueConverterFactory.simple());

‎src/test/java/org/springframework/data/mapping/model/AnnotationBasedPersistentPropertyUnitTests.java

+3-32
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.junit.jupiter.api.DynamicTest;
3737
import org.junit.jupiter.api.Test;
3838
import org.junit.jupiter.api.TestFactory;
39+
3940
import org.springframework.core.annotation.AliasFor;
4041
import org.springframework.core.annotation.AnnotationUtils;
4142
import org.springframework.data.annotation.AccessType;
@@ -44,9 +45,6 @@
4445
import org.springframework.data.annotation.ReadOnlyProperty;
4546
import org.springframework.data.annotation.Reference;
4647
import org.springframework.data.annotation.Transient;
47-
import org.springframework.data.convert.PropertyValueConverter;
48-
import org.springframework.data.convert.ValueConversionContext;
49-
import org.springframework.data.convert.ValueConverter;
5048
import org.springframework.data.mapping.MappingException;
5149
import org.springframework.data.mapping.PersistentProperty;
5250
import org.springframework.data.mapping.context.SampleMappingContext;
@@ -341,15 +339,6 @@ void detectsJMoleculesIdentity() {
341339
assertThat(property.isIdProperty()).isTrue();
342340
}
343341

344-
@Test // GH-1484
345-
void detectsValueConverter() {
346-
347-
SamplePersistentProperty property = getProperty(WithPropertyConverter.class, "value");
348-
349-
assertThat(property.hasValueConverter()).isTrue();
350-
assertThat(property.getValueConverterType()).isEqualTo(MyPropertyConverter.class);
351-
}
352-
353342
@SuppressWarnings("unchecked")
354343
private Map<Class<? extends Annotation>, Annotation> getAnnotationCache(SamplePersistentProperty property) {
355344
return (Map<Class<? extends Annotation>, Annotation>) ReflectionTestUtils.getField(property, "annotationCache");
@@ -472,7 +461,8 @@ public String getProperty() {
472461
@Retention(RetentionPolicy.RUNTIME)
473462
@Target(value = { FIELD, METHOD, ANNOTATION_TYPE })
474463
@Id
475-
public @interface MyId {}
464+
public @interface MyId {
465+
}
476466

477467
static class FieldAccess {
478468
String name;
@@ -542,23 +532,4 @@ static class JMolecules {
542532

543533
interface JMoleculesAggregate extends AggregateRoot<JMoleculesAggregate, Identifier> {}
544534

545-
static class WithPropertyConverter {
546-
547-
@ValueConverter(MyPropertyConverter.class)
548-
String value;
549-
}
550-
551-
static class MyPropertyConverter
552-
implements PropertyValueConverter<Object, Object, ValueConversionContext<SamplePersistentProperty>> {
553-
554-
@Override
555-
public Object read(Object value, ValueConversionContext context) {
556-
return null;
557-
}
558-
559-
@Override
560-
public Object write(Object value, ValueConversionContext context) {
561-
return null;
562-
}
563-
}
564535
}

0 commit comments

Comments
 (0)