Skip to content

Commit 54a98aa

Browse files
michael-simonsmichael-simons
authored
GH-2430 - Introduce converterRef on @ConvertWith and @CompositeProperty.
Closes #2430.
1 parent c11ec06 commit 54a98aa

File tree

10 files changed

+259
-26
lines changed

10 files changed

+259
-26
lines changed

‎src/main/asciidoc/appendix/conversions.adoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ This is an annotation that can be put on attributes of both entities (`@Node`) a
212212
It defines a `Neo4jPersistentPropertyConverter` via the `converter` attribute
213213
and an optional `Neo4jPersistentPropertyConverterFactory` to construct the former.
214214
With an implementation of `Neo4jPersistentPropertyConverter` all specific conversions for a given type can be addressed.
215+
In addition, `@ConvertWith` also provides `converterRef` for referencing any Spring bean in the application context implementing
216+
`Neo4jPersistentPropertyConverter`. The referenced bean will be preferred over constructing a new converter.
215217

216218
We provide `@DateLong` and `@DateString` as meta-annotated annotations for backward compatibility with Neo4j-OGM schemes not using native types.
217219
Those are meta annotated annotations building on the concept above.

‎src/main/java/org/springframework/data/neo4j/core/convert/ConvertWith.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838
* either this annotation and its values or with the meta annotated annotation, including all configuration
3939
* available.
4040
*
41+
* <p>In case {@link ConvertWith#converterRef()} is set to a non {@literal null} and non-empty value, the mapping context
42+
* will try to lookup a bean under the given name of type {@link Neo4jPersistentPropertyConverter} in the application context.
43+
* If no such bean is found an exception will be thrown. This attribute has precedence over {@link ConvertWith#converter()}.
44+
*
4145
* @author Michael J. Simons
4246
* @soundtrack Antilopen Gang - Abwasser
4347
* @since 6.0
@@ -59,6 +63,11 @@
5963
*/
6064
Class<? extends Neo4jPersistentPropertyConverterFactory> converterFactory() default DefaultNeo4jPersistentPropertyConverterFactory.class;
6165

66+
/**
67+
* @return An optional reference to a bean to be used as converter, must implement {@link Neo4jPersistentPropertyConverter}.
68+
*/
69+
String converterRef() default "";
70+
6271
/**
6372
* Indicates an unset converter.
6473
*/

‎src/main/java/org/springframework/data/neo4j/core/convert/DefaultNeo4jPersistentPropertyConverterFactory.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616
package org.springframework.data.neo4j.core.convert;
1717

1818
import org.springframework.beans.BeanUtils;
19+
import org.springframework.beans.factory.BeanFactory;
1920
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
21+
import org.springframework.lang.Nullable;
22+
import org.springframework.util.StringUtils;
2023

2124
/**
2225
* @author Michael J. Simons
@@ -25,11 +28,29 @@
2528
*/
2629
final class DefaultNeo4jPersistentPropertyConverterFactory implements Neo4jPersistentPropertyConverterFactory {
2730

31+
@Nullable
32+
private final BeanFactory beanFactory;
33+
34+
DefaultNeo4jPersistentPropertyConverterFactory(@Nullable BeanFactory beanFactory) {
35+
36+
this.beanFactory = beanFactory;
37+
}
38+
2839
@Override
2940
public Neo4jPersistentPropertyConverter<?> getPropertyConverterFor(Neo4jPersistentProperty persistentProperty) {
3041

3142
// At this point we already checked for the annotation.
3243
ConvertWith config = persistentProperty.getRequiredAnnotation(ConvertWith.class);
44+
45+
if (StringUtils.hasText(config.converterRef())) {
46+
if (beanFactory == null) {
47+
throw new IllegalStateException(
48+
"The default converter factory has been configured without a bean factory and cannot use a converter from the application context.");
49+
}
50+
51+
return beanFactory.getBean(config.converterRef(), Neo4jPersistentPropertyConverter.class);
52+
}
53+
3354
if (config.converter() == ConvertWith.UnsetConverter.class) {
3455
throw new IllegalArgumentException(
3556
"The default custom conversion factory cannot be used with a placeholder");

‎src/main/java/org/springframework/data/neo4j/core/convert/Neo4jPersistentPropertyConverterFactory.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@
2626
* an instance of {@link Neo4jConversionService}, such service is provided. This allows for conversions delegating part
2727
* of the conversion.
2828
*
29+
* <p>In same cases a factory might be interested in having access to a {@link org.springframework.beans.factory.BeanFactory}.
30+
* In case SDN can provide it, it will prefer such a constructor to the default one or the one taken a {@link Neo4jConversionService}.
31+
*
2932
* @author Michael J. Simons
3033
* @soundtrack Antilopen Gang - Abwasser
3134
* @since 6.0

‎src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package org.springframework.data.neo4j.core.mapping;
1717

1818
import java.lang.reflect.Constructor;
19+
import java.lang.reflect.Method;
1920
import java.lang.reflect.Modifier;
2021
import java.lang.reflect.ParameterizedType;
2122
import java.lang.reflect.Type;
@@ -34,6 +35,7 @@
3435
import org.neo4j.driver.types.TypeSystem;
3536
import org.springframework.beans.BeanUtils;
3637
import org.springframework.beans.BeansException;
38+
import org.springframework.beans.factory.BeanFactory;
3739
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
3840
import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
3941
import org.springframework.context.ApplicationContext;
@@ -52,9 +54,9 @@
5254
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverterFactory;
5355
import org.springframework.data.neo4j.core.schema.IdGenerator;
5456
import org.springframework.data.neo4j.core.schema.Node;
55-
import org.springframework.data.util.ReflectionUtils;
5657
import org.springframework.data.util.TypeInformation;
5758
import org.springframework.lang.Nullable;
59+
import org.springframework.util.ReflectionUtils;
5860

5961
/**
6062
* An implementation of both a {@link Schema} as well as a Neo4j version of Spring Data's
@@ -328,12 +330,37 @@ public <T extends IdGenerator<?>> Optional<T> getIdGenerator(String reference) {
328330
}
329331
}
330332

333+
@Nullable
334+
Constructor<?> findConstructor(Class<?> clazz, Class<?>... parameterTypes) {
335+
try {
336+
return ReflectionUtils.accessibleConstructor(clazz, parameterTypes);
337+
} catch (NoSuchMethodException e) {
338+
return null;
339+
}
340+
}
341+
331342
private <T extends Neo4jPersistentPropertyConverterFactory> T getOrCreateConverterFactoryOfType(Class<T> converterFactoryType) {
332343

333344
return converterFactoryType.cast(this.converterFactories.computeIfAbsent(converterFactoryType, t -> {
334-
Optional<Constructor<?>> optionalConstructor = ReflectionUtils.findConstructor(t, this.conversionService);
335-
if (optionalConstructor.isPresent()) {
336-
return t.cast(BeanUtils.instantiateClass(optionalConstructor.get(), this.conversionService));
345+
Constructor<?> optionalConstructor;
346+
optionalConstructor = findConstructor(t, BeanFactory.class, Neo4jConversionService.class);
347+
if (optionalConstructor != null) {
348+
return t.cast(BeanUtils.instantiateClass(optionalConstructor, this.beanFactory, this.conversionService));
349+
}
350+
351+
optionalConstructor = findConstructor(t, Neo4jConversionService.class, BeanFactory.class);
352+
if (optionalConstructor != null) {
353+
return t.cast(BeanUtils.instantiateClass(optionalConstructor, this.beanFactory, this.conversionService));
354+
}
355+
356+
optionalConstructor = findConstructor(t, BeanFactory.class);
357+
if (optionalConstructor != null) {
358+
return t.cast(BeanUtils.instantiateClass(optionalConstructor, this.beanFactory));
359+
}
360+
361+
optionalConstructor = findConstructor(t, Neo4jConversionService.class);
362+
if (optionalConstructor != null) {
363+
return t.cast(BeanUtils.instantiateClass(optionalConstructor, this.conversionService));
337364
}
338365
return BeanUtils.instantiateClass(t);
339366
}));
@@ -353,11 +380,19 @@ Neo4jPersistentPropertyConverter<?> getOptionalCustomConversionsFor(Neo4jPersist
353380

354381
ConvertWith convertWith = persistentProperty.getRequiredAnnotation(ConvertWith.class);
355382
Neo4jPersistentPropertyConverterFactory persistentPropertyConverterFactory = this.getOrCreateConverterFactoryOfType(convertWith.converterFactory());
356-
Neo4jPersistentPropertyConverter<?> customConversions = persistentPropertyConverterFactory.getPropertyConverterFor(persistentProperty);
383+
Neo4jPersistentPropertyConverter<?> customConverter = persistentPropertyConverterFactory.getPropertyConverterFor(persistentProperty);
357384

358385
boolean forCollection = false;
359-
if (persistentProperty.isCollectionLike() && convertWith.converter() != ConvertWith.UnsetConverter.class) {
360-
Map<String, Type> typeVariableMap = GenericTypeResolver.getTypeVariableMap(convertWith.converter())
386+
if (persistentProperty.isCollectionLike()) {
387+
Class<?> converterClass;
388+
Method getClassOfDelegate = ReflectionUtils.findMethod(customConverter.getClass(), "getClassOfDelegate");
389+
if (getClassOfDelegate != null) {
390+
ReflectionUtils.makeAccessible(getClassOfDelegate);
391+
converterClass = (Class<?>) ReflectionUtils.invokeMethod(getClassOfDelegate, customConverter);
392+
} else {
393+
converterClass = customConverter.getClass();
394+
}
395+
Map<String, Type> typeVariableMap = GenericTypeResolver.getTypeVariableMap(converterClass)
361396
.entrySet()
362397
.stream()
363398
.collect(Collectors.toMap(e -> e.getKey().getName(), Map.Entry::getValue));
@@ -367,12 +402,11 @@ Neo4jPersistentPropertyConverter<?> getOptionalCustomConversionsFor(Neo4jPersist
367402
} else if (typeVariableMap.containsKey("P")) {
368403
propertyType = typeVariableMap.get("P");
369404
}
370-
forCollection =
371-
propertyType != null && propertyType instanceof ParameterizedType && persistentProperty.getType()
372-
.equals(((ParameterizedType) propertyType).getRawType());
405+
forCollection = propertyType instanceof ParameterizedType &&
406+
persistentProperty.getType().equals(((ParameterizedType) propertyType).getRawType());
373407
}
374408

375-
return new NullSafeNeo4jPersistentPropertyConverter<>(customConversions, persistentProperty.isComposite(), forCollection);
409+
return new NullSafeNeo4jPersistentPropertyConverter<>(customConverter, persistentProperty.isComposite(), forCollection);
376410
}
377411

378412
@Override

‎src/main/java/org/springframework/data/neo4j/core/schema/CompositeProperty.java

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.neo4j.driver.Value;
3535
import org.neo4j.driver.Values;
3636
import org.springframework.beans.BeanUtils;
37+
import org.springframework.beans.factory.BeanFactory;
3738
import org.springframework.core.GenericTypeResolver;
3839
import org.springframework.core.annotation.AliasFor;
3940
import org.springframework.data.neo4j.core.convert.ConvertWith;
@@ -47,6 +48,7 @@
4748
import org.springframework.data.util.TypeInformation;
4849
import org.springframework.lang.Nullable;
4950
import org.springframework.util.Assert;
51+
import org.springframework.util.StringUtils;
5052

5153
/**
5254
* This annotation indicates a {@link org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty persistent property}
@@ -74,6 +76,9 @@
7476
@AliasFor(annotation = ConvertWith.class, value = "converter")
7577
Class<? extends Neo4jPersistentPropertyToMapConverter> converter() default CompositeProperty.DefaultToMapConverter.class;
7678

79+
@AliasFor(annotation = ConvertWith.class, value = "converterRef")
80+
String converterRef() default "";
81+
7782
/**
7883
* Allows to specify the prefix for the map properties. The default empty value instructs SDN to use the
7984
* field name of the annotated property.
@@ -223,6 +228,15 @@ public P read(Value source) {
223228
});
224229
return this.delegate.compose(temp, neo4jConversionService);
225230
}
231+
232+
/**
233+
* Internally used via reflection.
234+
* @return The type of the underlying delegate.
235+
*/
236+
@SuppressWarnings("unused")
237+
Class<?> getClassOfDelegate() {
238+
return this.delegate.getClass();
239+
}
226240
}
227241

228242
/**
@@ -233,9 +247,11 @@ final class CompositePropertyConverterFactory implements Neo4jPersistentProperty
233247
private static final String KEY_TYPE_KEY = "K";
234248
private static final String PROPERTY_TYPE_KEY = "P";
235249

250+
private final BeanFactory beanFactory;
236251
private final Neo4jConversionService conversionServiceDelegate;
237252

238-
CompositePropertyConverterFactory(@Nullable Neo4jConversionService conversionServiceDelegate) {
253+
CompositePropertyConverterFactory(@Nullable BeanFactory beanFactory, @Nullable Neo4jConversionService conversionServiceDelegate) {
254+
this.beanFactory = beanFactory;
239255
this.conversionServiceDelegate = conversionServiceDelegate;
240256
}
241257

@@ -245,6 +261,17 @@ public Neo4jPersistentPropertyConverter<?> getPropertyConverterFor(Neo4jPersiste
245261

246262
CompositeProperty config = persistentProperty.getRequiredAnnotation(CompositeProperty.class);
247263
Class<? extends Neo4jPersistentPropertyToMapConverter> delegateClass = config.converter();
264+
Neo4jPersistentPropertyToMapConverter<?, Map<?, Object>> delegate = null;
265+
266+
if (StringUtils.hasText(config.converterRef())) {
267+
if (beanFactory == null) {
268+
throw new IllegalStateException(
269+
"The default composite converter factory has been configured without a bean factory and cannot use a converter from the application context.");
270+
}
271+
272+
delegate = beanFactory.getBean(config.converterRef(), Neo4jPersistentPropertyToMapConverter.class);
273+
delegateClass = delegate.getClass();
274+
}
248275

249276
Class<?> componentType;
250277

@@ -307,11 +334,12 @@ public Neo4jPersistentPropertyConverter<?> getPropertyConverterFor(Neo4jPersiste
307334
keyWriter = (String key) -> keyTransformation.apply(Phase.WRITE, key);
308335
}
309336

310-
Neo4jPersistentPropertyToMapConverter<?, Map<?, Object>> delegate;
311-
if (delegateClass == CompositeProperty.DefaultToMapConverter.class) {
312-
delegate = new CompositeProperty.DefaultToMapConverter(ClassTypeInformation.from(persistentProperty.getActualType()));
313-
} else {
314-
delegate = BeanUtils.instantiateClass(delegateClass);
337+
if (delegate == null) {
338+
if (delegateClass == CompositeProperty.DefaultToMapConverter.class) {
339+
delegate = new CompositeProperty.DefaultToMapConverter(ClassTypeInformation.from(persistentProperty.getActualType()));
340+
} else {
341+
delegate = BeanUtils.instantiateClass(delegateClass);
342+
}
315343
}
316344

317345
String prefixWithDelimiter = persistentProperty.computePrefixWithDelimiter();

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/DomainObject.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,7 @@ public class DomainObject {
4343

4444
@ConvertWith(converter = UnrelatedObjectPropertyConverter.class)
4545
private UnrelatedObject storedAsSingleProperty = new UnrelatedObject();
46+
47+
@ConvertWith(converterRef = "converterBean")
48+
private UnrelatedObject storedAsAnotherSingleProperty = new UnrelatedObject();
4649
}

‎src/test/java/org/springframework/data/neo4j/integration/issues/gh2168/Gh2168IT.java

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,12 @@
2929
import org.springframework.beans.factory.annotation.Autowired;
3030
import org.springframework.context.annotation.Bean;
3131
import org.springframework.context.annotation.Configuration;
32+
import org.springframework.data.mapping.PersistentPropertyAccessor;
3233
import org.springframework.data.neo4j.config.AbstractNeo4jConfig;
3334
import org.springframework.data.neo4j.core.DatabaseSelectionProvider;
35+
import org.springframework.data.neo4j.core.mapping.Neo4jMappingContext;
36+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentEntity;
37+
import org.springframework.data.neo4j.core.mapping.Neo4jPersistentProperty;
3438
import org.springframework.data.neo4j.core.transaction.Neo4jBookmarkManager;
3539
import org.springframework.data.neo4j.core.transaction.Neo4jTransactionManager;
3640
import org.springframework.data.neo4j.repository.Neo4jRepository;
@@ -97,30 +101,54 @@ void compositePropertyCustomConverterDefaultPrefixShouldWork(
97101
});
98102
}
99103

100-
101-
102104
// That test and the underlying mapping cause the original issue to fail, as `@ConvertWith` was missing for non-simple
103105
// types in the lookup that checked whether something is an association or not
104106
@Test // GH-2168
105107
void propertyCustomConverterDefaultPrefixShouldWork(
108+
@Autowired Neo4jMappingContext ctx,
106109
@Autowired DomainObjectRepository repository,
107110
@Autowired Driver driver,
108111
@Autowired BookmarkCapture bookmarkCapture
109112
) {
113+
Neo4jPersistentEntity<?> entity = ctx.getRequiredPersistentEntity(DomainObject.class);
114+
assertWriteAndReadConversionForProperty(entity, "storedAsSingleProperty", repository, driver, bookmarkCapture);
115+
}
110116

111-
DomainObject domainObject = new DomainObject();
112-
domainObject.setStoredAsSingleProperty(new UnrelatedObject(true, 4711L));
113-
domainObject = repository.save(domainObject);
117+
@Test // GH-2430
118+
void propertyConversionsWithBeansShouldWork(
119+
@Autowired Neo4jMappingContext ctx,
120+
@Autowired DomainObjectRepository repository,
121+
@Autowired Driver driver,
122+
@Autowired BookmarkCapture bookmarkCapture
123+
) {
124+
Neo4jPersistentEntity<?> entity = ctx.getRequiredPersistentEntity(DomainObject.class);
125+
assertWriteAndReadConversionForProperty(entity, "storedAsAnotherSingleProperty", repository, driver, bookmarkCapture);
126+
}
127+
128+
private void assertWriteAndReadConversionForProperty(
129+
Neo4jPersistentEntity<?> entity,
130+
String propertyName,
131+
DomainObjectRepository repository,
132+
Driver driver,
133+
BookmarkCapture bookmarkCapture
134+
) {
135+
Neo4jPersistentProperty property = entity.getPersistentProperty(propertyName);
136+
PersistentPropertyAccessor<DomainObject> propertyAccessor = entity.getPropertyAccessor(new DomainObject());
137+
138+
propertyAccessor.setProperty(property, new UnrelatedObject(true, 4711L));
139+
DomainObject domainObject = repository.save(propertyAccessor.getBean());
114140

115141
try (Session session = driver.session(bookmarkCapture.createSessionConfig())) {
116142
Node node = session
117-
.run("MATCH (n:DomainObject {id: $id}) RETURN n", Collections.singletonMap("id", domainObject.getId()))
143+
.run("MATCH (n:DomainObject {id: $id}) RETURN n",
144+
Collections.singletonMap("id", domainObject.getId()))
118145
.single().get(0).asNode();
119-
assertThat(node.get("storedAsSingleProperty").asString()).isEqualTo("true;4711");
146+
assertThat(node.get(propertyName).asString()).isEqualTo("true;4711");
120147
}
121148

122149
domainObject = repository.findById(domainObject.getId()).get();
123-
assertThat(domainObject.getStoredAsSingleProperty())
150+
UnrelatedObject unrelatedObject = (UnrelatedObject) entity.getPropertyAccessor(domainObject).getProperty(property);
151+
assertThat(unrelatedObject)
124152
.satisfies(t -> {
125153
assertThat(t.isABooleanValue()).isTrue();
126154
assertThat(t.getALongValue()).isEqualTo(4711L);
@@ -146,6 +174,11 @@ public BookmarkCapture bookmarkCapture() {
146174
return new BookmarkCapture();
147175
}
148176

177+
@Bean
178+
public UnrelatedObjectPropertyConverterAsBean converterBean() {
179+
return new UnrelatedObjectPropertyConverterAsBean();
180+
}
181+
149182
@Override
150183
public PlatformTransactionManager transactionManager(Driver driver, DatabaseSelectionProvider databaseNameProvider) {
151184

0 commit comments

Comments
 (0)