Skip to content

Commit 0640d05

Browse files
christophstroblchristophstrobl
committed
DATACMNS-1615 - Allow deferred converter configuration.
Add a Supplier variant for constructing new CustomConversions. This allows store implementations to hook into the actual configuration based on user code. Required for messing with store specific simple type registration based on user decisions.
1 parent 580e55a commit 0640d05

File tree

2 files changed

+209
-67
lines changed

2 files changed

+209
-67
lines changed

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

+160-54
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,19 @@
2222
import lombok.Value;
2323
import lombok.extern.slf4j.Slf4j;
2424

25-
import java.util.*;
25+
import java.util.ArrayList;
26+
import java.util.Arrays;
27+
import java.util.Collection;
28+
import java.util.Collections;
29+
import java.util.HashSet;
30+
import java.util.LinkedHashSet;
31+
import java.util.List;
32+
import java.util.Map;
33+
import java.util.Optional;
34+
import java.util.Set;
2635
import java.util.concurrent.ConcurrentHashMap;
2736
import java.util.function.Function;
37+
import java.util.function.Supplier;
2838
import java.util.stream.Collectors;
2939

3040
import org.springframework.core.GenericTypeResolver;
@@ -61,7 +71,7 @@ public class CustomConversions {
6171
private static final String CONVERTER_FILTER = "converter from %s to %s as %s converter.";
6272
private static final String ADD_CONVERTER = "Adding %s" + CONVERTER_FILTER;
6373
private static final String SKIP_CONVERTER = "Skipping " + CONVERTER_FILTER
64-
+ "%s is not a store supported simple type!";
74+
+ " %s is not a store supported simple type!";
6575
private static final List<Object> DEFAULT_CONVERTERS;
6676

6777
static {
@@ -84,6 +94,8 @@ public class CustomConversions {
8494
private final ConversionTargetsCache customReadTargetTypes = new ConversionTargetsCache();
8595
private final ConversionTargetsCache customWriteTargetTypes = new ConversionTargetsCache();
8696

97+
private final ConverterConfiguration converterConfiguration;
98+
8799
private final Function<ConvertiblePair, Class<?>> getReadTarget = convertiblePair -> getCustomTarget(
88100
convertiblePair.getSourceType(), convertiblePair.getTargetType(), readingPairs);
89101

@@ -94,60 +106,51 @@ public class CustomConversions {
94106
convertiblePair.getSourceType(), null, writingPairs);
95107

96108
/**
97-
* Creates a new {@link CustomConversions} instance registering all given user defined converters and selecting
98-
* {@link Converter converters} from {@link StoreConversions} depending on
99-
* {@link StoreConversions#getSimpleTypeHolder() store simple types} only considering those that either convert
100-
* to/from a store supported type.
109+
* Deferred initialization allowing store implementations use a callback/consumer driven configuration of the actual
110+
* {@link CustomConversions}.
101111
*
102-
* @param storeConversions must not be {@literal null}.
103-
* @param converters must not be {@literal null}.
112+
* @param converterConfiguration the {@link Supplier} providing a {@link ConverterConfiguration}.
113+
* @since 2.3
104114
*/
105-
public CustomConversions(StoreConversions storeConversions, Collection<?> converters) {
115+
protected CustomConversions(Supplier<ConverterConfiguration> converterConfiguration) {
116+
this(converterConfiguration.get());
117+
}
106118

107-
Assert.notNull(storeConversions, "StoreConversions must not be null!");
108-
Assert.notNull(converters, "List of converters must not be null!");
119+
/**
120+
* @param converterConfiguration the {@link ConverterConfiguration} to apply.
121+
* @since 2.3
122+
*/
123+
protected CustomConversions(ConverterConfiguration converterConfiguration) {
109124

110-
List<Object> registeredConverters = collectPotentialConverterRegistrations(storeConversions, converters).stream() //
111-
.filter(this::isSupportedConverter) //
112-
.map(ConverterRegistrationIntent::getConverterRegistration) //
113-
.map(this::register) //
114-
.distinct() //
115-
.collect(Collectors.toList());
125+
this.converterConfiguration = converterConfiguration;
126+
127+
List<Object> registeredConverters = collectPotentialConverterRegistrations(
128+
converterConfiguration.getStoreConversions(), converterConfiguration.getUserConverters()).stream() //
129+
.filter(this::isSupportedConverter) //
130+
.filter(it -> !skip(it)) //
131+
.map(ConverterRegistrationIntent::getConverterRegistration) //
132+
.map(this::register) //
133+
.distinct() //
134+
.collect(Collectors.toList());
116135

117136
Collections.reverse(registeredConverters);
118137

119138
this.converters = Collections.unmodifiableList(registeredConverters);
120-
this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes, storeConversions.getStoreTypeHolder());
139+
this.simpleTypeHolder = new SimpleTypeHolder(customSimpleTypes,
140+
converterConfiguration.getStoreConversions().getStoreTypeHolder());
121141
}
122142

123143
/**
124-
* Validate a given {@link ConverterRegistration} in a specific setup. <br />
125-
* Non {@link ReadingConverter reading} and user defined {@link Converter converters} are only considered supported if
126-
* the {@link ConverterRegistrationIntent#isSimpleTargetType() target type} is considered to be a store simple type.
127-
*
128-
* @param registration
129-
* @return
130-
* @since 2.3
144+
* Creates a new {@link CustomConversions} instance registering all given user defined converters and selecting
145+
* {@link Converter converters} from {@link StoreConversions} depending on
146+
* {@link StoreConversions#getSimpleTypeHolder() store simple types} only considering those that either convert
147+
* to/from a store supported type.
148+
*
149+
* @param storeConversions must not be {@literal null}.
150+
* @param converters must not be {@literal null}.
131151
*/
132-
protected boolean isSupportedConverter(ConverterRegistrationIntent registration) {
133-
134-
boolean register = registration.isUserConverter() || (registration.isReading() && registration.isSimpleSourceType())
135-
|| (registration.isWriting() && registration.isSimpleTargetType());
136-
137-
if (LOG.isDebugEnabled()) {
138-
139-
if (register) {
140-
LOG.debug(String.format(ADD_CONVERTER, registration.isUserConverter() ? "user defined " : "",
141-
registration.getSourceType(), registration.getTargetType(),
142-
registration.isReading() ? "reading" : "writing"));
143-
} else {
144-
LOG.debug(String.format(SKIP_CONVERTER, registration.getSourceType(), registration.getTargetType(),
145-
registration.isReading() ? "reading" : "writing",
146-
registration.isReading() ? registration.getSourceType() : registration.getTargetType()));
147-
}
148-
}
149-
150-
return register;
152+
public CustomConversions(StoreConversions storeConversions, Collection<?> converters) {
153+
this(new ConverterConfiguration(storeConversions, new ArrayList<>(converters)));
151154
}
152155

153156
/**
@@ -187,11 +190,12 @@ public void registerConvertersIn(ConverterRegistry conversionService) {
187190
}
188191

189192
/**
190-
* Get all converters and add some origin information
193+
* Get all converters and add origin information
191194
*
192195
* @param storeConversions
193196
* @param converters
194197
* @return
198+
* @since 2.3
195199
*/
196200
private List<ConverterRegistrationIntent> collectPotentialConverterRegistrations(StoreConversions storeConversions,
197201
Collection<?> converters) {
@@ -224,31 +228,27 @@ private List<ConverterRegistrationIntent> collectPotentialConverterRegistrations
224228
*/
225229
private void registerConverterIn(Object candidate, ConverterRegistry conversionService) {
226230

227-
boolean added = false;
228-
229231
if (candidate instanceof Converter) {
230232
conversionService.addConverter(Converter.class.cast(candidate));
231-
added = true;
233+
return;
232234
}
233235

234236
if (candidate instanceof ConverterFactory) {
235237
conversionService.addConverterFactory(ConverterFactory.class.cast(candidate));
236-
added = true;
238+
return;
237239
}
238240

239241
if (candidate instanceof GenericConverter) {
240242
conversionService.addConverter(GenericConverter.class.cast(candidate));
241-
added = true;
243+
return;
242244
}
243245

244246
if (candidate instanceof ConverterAware) {
245247
ConverterAware.class.cast(candidate).getConverters().forEach(it -> registerConverterIn(it, conversionService));
246-
added = true;
248+
return;
247249
}
248250

249-
if (!added) {
250-
throw new IllegalArgumentException(String.format(NOT_A_CONVERTER, candidate));
251-
}
251+
throw new IllegalArgumentException(String.format(NOT_A_CONVERTER, candidate));
252252
}
253253

254254
/**
@@ -285,6 +285,48 @@ private Object register(ConverterRegistration converterRegistration) {
285285
return converterRegistration.getConverter();
286286
}
287287

288+
/**
289+
* Validate a given {@link ConverterRegistration} in a specific setup. <br />
290+
* Non {@link ReadingConverter reading} and user defined {@link Converter converters} are only considered supported if
291+
* the {@link ConverterRegistrationIntent#isSimpleTargetType() target type} is considered to be a store simple type.
292+
*
293+
* @param registrationIntent
294+
* @return {@literal true} if supported.
295+
* @since 2.3
296+
*/
297+
private boolean isSupportedConverter(ConverterRegistrationIntent registrationIntent) {
298+
299+
boolean register = registrationIntent.isUserConverter()
300+
|| (registrationIntent.isReading() && registrationIntent.isSimpleSourceType())
301+
|| (registrationIntent.isWriting() && registrationIntent.isSimpleTargetType());
302+
303+
if (LOG.isDebugEnabled()) {
304+
305+
if (register) {
306+
LOG.debug(String.format(ADD_CONVERTER, registrationIntent.isUserConverter() ? "user defined " : "",
307+
registrationIntent.getSourceType(), registrationIntent.getTargetType(),
308+
registrationIntent.isReading() ? "reading" : "writing"));
309+
} else {
310+
LOG.debug(String.format(SKIP_CONVERTER, registrationIntent.getSourceType(), registrationIntent.getTargetType(),
311+
registrationIntent.isReading() ? "reading" : "writing",
312+
registrationIntent.isReading() ? registrationIntent.getSourceType() : registrationIntent.getTargetType()));
313+
}
314+
}
315+
316+
return register;
317+
}
318+
319+
/**
320+
* @param intent must not be {@literal null}.
321+
* @return {@literal true} if the given {@link ConverterRegistration} shall be skipped.
322+
* @since 2.3
323+
*/
324+
private boolean skip(ConverterRegistrationIntent intent) {
325+
326+
return intent.isDefaultConveter()
327+
&& converterConfiguration.getSkipFor().contains(intent.getConverterRegistration().getConvertiblePair());
328+
}
329+
288330
/**
289331
* Returns the target type to convert to in case we have a custom conversion registered to convert the given source
290332
* type into a Mongo native one.
@@ -765,4 +807,68 @@ private boolean isStoreSimpleType(Class<?> type) {
765807
return storeTypeHolder.isSimpleType(type);
766808
}
767809
}
810+
811+
/**
812+
* Value object holding the actual {@link StoreConversions} and custom {@link Converter converters} configured for
813+
* registration.
814+
*
815+
* @author Christoph Strobl
816+
* @since 2.3
817+
*/
818+
protected static class ConverterConfiguration {
819+
820+
private final StoreConversions storeConversions;
821+
private final List<?> userConverters;
822+
private final Set<ConvertiblePair> skipFor;
823+
824+
/**
825+
* Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters.
826+
*
827+
* @param storeConversions must not be {@literal null}.
828+
* @param userConverters must not be {@literal null} use {@link Collections#emptyList()} instead.
829+
*/
830+
public ConverterConfiguration(StoreConversions storeConversions, List<?> userConverters) {
831+
this(storeConversions, userConverters, Collections.emptySet());
832+
}
833+
834+
/**
835+
* Create a new ConverterConfiguration holding the given {@link StoreConversions} and user defined converters as
836+
* well as a {@link Collection} of {@link ConvertiblePair} for which to skip the registration of default converters.
837+
* <br />
838+
* This allows store implementations to modify default converter registration based on specific needs and
839+
* configurations. User defined converters will are never subject of filtering.
840+
*
841+
* @param storeConversions must not be {@literal null}.
842+
* @param userConverters must not be {@literal null} use {@link Collections#emptyList()} instead.
843+
* @param skipFor must not be {@literal null} use {@link Collections#emptyList()} instead.
844+
*/
845+
public ConverterConfiguration(StoreConversions storeConversions, List<?> userConverters,
846+
Collection<ConvertiblePair> skipFor) {
847+
848+
this.storeConversions = storeConversions;
849+
this.userConverters = new ArrayList<>(userConverters);
850+
this.skipFor = new HashSet<>(skipFor);
851+
}
852+
853+
/**
854+
* @return never {@literal null}
855+
*/
856+
StoreConversions getStoreConversions() {
857+
return storeConversions;
858+
}
859+
860+
/**
861+
* @return never {@literal null}.
862+
*/
863+
List<?> getUserConverters() {
864+
return userConverters;
865+
}
866+
867+
/**
868+
* @return never {@literal null}.
869+
*/
870+
Set<ConvertiblePair> getSkipFor() {
871+
return skipFor;
872+
}
873+
}
768874
}

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

+49-13
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,19 @@
2727
import java.util.Locale;
2828

2929
import org.joda.time.DateTime;
30-
import org.junit.Test;
30+
import org.junit.jupiter.api.Test;
3131
import org.springframework.aop.framework.ProxyFactory;
3232
import org.springframework.core.convert.converter.Converter;
3333
import org.springframework.core.convert.converter.ConverterFactory;
3434
import org.springframework.core.convert.converter.ConverterRegistry;
35+
import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
3536
import org.springframework.core.convert.support.ConfigurableConversionService;
3637
import org.springframework.core.convert.support.DefaultConversionService;
3738
import org.springframework.core.convert.support.GenericConversionService;
3839
import org.springframework.data.convert.ConverterBuilder.ConverterAware;
40+
import org.springframework.data.convert.CustomConversions.ConverterConfiguration;
3941
import org.springframework.data.convert.CustomConversions.StoreConversions;
42+
import org.springframework.data.convert.Jsr310Converters.LocalDateTimeToDateConverter;
4043
import org.springframework.data.convert.ThreeTenBackPortConverters.LocalDateTimeToJavaTimeInstantConverter;
4144
import org.springframework.data.mapping.model.SimpleTypeHolder;
4245
import org.threeten.bp.LocalDateTime;
@@ -51,6 +54,15 @@
5154
*/
5255
public class CustomConversionsUnitTests {
5356

57+
static final SimpleTypeHolder DATE_EXCLUDING_SIMPLE_TYPE_HOLDER = new SimpleTypeHolder(
58+
Collections.singleton(Date.class), true) {
59+
60+
@Override
61+
public boolean isSimpleType(Class<?> type) {
62+
return type.getName().startsWith("java.time") ? false : super.isSimpleType(type);
63+
}
64+
};
65+
5466
@Test // DATACMNS-1035
5567
public void findsBasicReadAndWriteConversions() {
5668

@@ -199,23 +211,47 @@ public void registersConverterFromConverterAware() {
199211
public void skipsUnsupportedDefaultWritingConverter() {
200212

201213
ConverterRegistry registry = mock(ConverterRegistry.class);
202-
StoreConversions storeConversions = StoreConversions
203-
.of(new SimpleTypeHolder(Collections.singleton(Date.class), true) {
204214

205-
@Override
206-
public boolean isSimpleType(Class<?> type) {
215+
new CustomConversions(StoreConversions.of(DATE_EXCLUDING_SIMPLE_TYPE_HOLDER), Collections.emptyList())
216+
.registerConvertersIn(registry);
217+
218+
verify(registry, never()).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class));
219+
}
220+
221+
@Test // DATACMNS-1615
222+
public void doesNotSkipUnsupportedUserConverter() {
207223

208-
if (type.getName().startsWith("java.time")) {
209-
return false;
210-
}
224+
ConverterRegistry registry = mock(ConverterRegistry.class);
211225

212-
return super.isSimpleType(type);
213-
}
214-
});
226+
new CustomConversions(StoreConversions.of(DATE_EXCLUDING_SIMPLE_TYPE_HOLDER),
227+
Collections.singletonList(LocalDateTimeToJavaTimeInstantConverter.INSTANCE)).registerConvertersIn(registry);
215228

216-
new CustomConversions(storeConversions, Collections.emptyList()).registerConvertersIn(registry);
229+
verify(registry).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class));
230+
}
217231

218-
verify(registry, never()).addConverter(any(LocalDateTimeToJavaTimeInstantConverter.class));
232+
@Test // DATACMNS-1615
233+
public void skipsConverterBasedOnConfiguration() {
234+
235+
ConverterRegistry registry = mock(ConverterRegistry.class);
236+
237+
ConverterConfiguration config = new ConverterConfiguration(StoreConversions.NONE, Collections.emptyList(),
238+
Collections.singleton(new ConvertiblePair(java.time.LocalDateTime.class, Date.class)));
239+
new CustomConversions(config).registerConvertersIn(registry);
240+
241+
verify(registry, never()).addConverter(any(LocalDateTimeToDateConverter.class));
242+
}
243+
244+
@Test // DATACMNS-1615
245+
public void doesNotSkipUserConverterConverterEvenWhenConfigurationWouldNotAllowIt() {
246+
247+
ConverterRegistry registry = mock(ConverterRegistry.class);
248+
249+
ConverterConfiguration config = new ConverterConfiguration(StoreConversions.NONE,
250+
Collections.singletonList(LocalDateTimeToDateConverter.INSTANCE),
251+
Collections.singleton(new ConvertiblePair(java.time.LocalDateTime.class, Date.class)));
252+
new CustomConversions(config).registerConvertersIn(registry);
253+
254+
verify(registry).addConverter(any(LocalDateTimeToDateConverter.class));
219255
}
220256

221257
private static Class<?> createProxyTypeFor(Class<?> type) {

0 commit comments

Comments
 (0)