Skip to content

Commit 3d08c10

Browse files
committed
Avoid warn logging for custom converters using Collection.
If the collection type is not a store-native type, we no longer issue a warn log. Converters can be registered for List or Collection types and convert into a different, non-collection type while being a support converter instead of forcing a specific write type. This is e.g. the case for MongoDB where a List of numbers is converted to the Vector type. Closes #3306
1 parent d114269 commit 3d08c10

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@
7070
*/
7171
public class CustomConversions {
7272

73-
private static final Log logger = LogFactory.getLog(CustomConversions.class);
73+
private static Log logger = LogFactory.getLog(CustomConversions.class);
7474
private static final String READ_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as reading converter although it doesn't convert from a store-supported type; You might want to check your annotation setup at the converter implementation";
7575
private static final String WRITE_CONVERTER_NOT_SIMPLE = "Registering converter from %s to %s as writing converter although it doesn't convert to a store-supported type; You might want to check your annotation setup at the converter implementation";
7676
private static final String NOT_A_CONVERTER = "Converter %s is neither a Spring Converter, GenericConverter or ConverterFactory";
@@ -324,7 +324,8 @@ private Object register(ConverterRegistration converterRegistration) {
324324

325325
readingPairs.add(pair);
326326

327-
if (logger.isWarnEnabled() && !converterRegistration.isSimpleSourceType()) {
327+
if (logger.isWarnEnabled() && !converterRegistration.isSimpleSourceType()
328+
&& !Collection.class.isAssignableFrom(pair.getSourceType())) {
328329
logger.warn(String.format(READ_CONVERTER_NOT_SIMPLE, pair.getSourceType(), pair.getTargetType()));
329330
}
330331
}
@@ -334,7 +335,8 @@ private Object register(ConverterRegistration converterRegistration) {
334335
writingPairs.add(pair);
335336
customSimpleTypes.add(pair.getSourceType());
336337

337-
if (logger.isWarnEnabled() && !converterRegistration.isSimpleTargetType()) {
338+
if (logger.isWarnEnabled() && !converterRegistration.isSimpleTargetType()
339+
&& !Collection.class.isAssignableFrom(pair.getTargetType())) {
338340
logger.warn(String.format(WRITE_CONVERTER_NOT_SIMPLE, pair.getSourceType(), pair.getTargetType()));
339341
}
340342
}

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

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@
2727
import java.util.Map;
2828
import java.util.function.Predicate;
2929

30+
import org.apache.commons.logging.Log;
3031
import org.jmolecules.ddd.types.Association;
3132
import org.jmolecules.ddd.types.Identifier;
3233
import org.junit.jupiter.api.Test;
34+
3335
import org.springframework.aop.framework.ProxyFactory;
3436
import org.springframework.core.convert.converter.Converter;
3537
import org.springframework.core.convert.converter.ConverterFactory;
@@ -44,6 +46,7 @@
4446
import org.springframework.data.geo.Point;
4547
import org.springframework.data.mapping.PersistentProperty;
4648
import org.springframework.data.mapping.model.SimpleTypeHolder;
49+
import org.springframework.test.util.ReflectionTestUtils;
4750

4851
/**
4952
* Unit tests for {@link CustomConversions}.
@@ -156,8 +159,7 @@ void registersConverterFactoryCorrectly() {
156159

157160
var conversions = StoreConversions.of(new SimpleTypeHolder(Collections.singleton(Format.class), true));
158161

159-
var customConversions = new CustomConversions(conversions,
160-
Collections.singletonList(new FormatConverterFactory()));
162+
var customConversions = new CustomConversions(conversions, Collections.singletonList(new FormatConverterFactory()));
161163

162164
assertThat(customConversions.getCustomWriteTarget(String.class, SimpleDateFormat.class)).isPresent();
163165
}
@@ -214,7 +216,7 @@ void doesNotSkipUnsupportedUserConverter() {
214216

215217
new CustomConversions(StoreConversions.of(DATE_EXCLUDING_SIMPLE_TYPE_HOLDER),
216218
Collections.singletonList(Jsr310Converters.LocalDateTimeToInstantConverter.INSTANCE))
217-
.registerConvertersIn(registry);
219+
.registerConvertersIn(registry);
218220

219221
verify(registry).addConverter(any(Jsr310Converters.LocalDateTimeToInstantConverter.class));
220222
}
@@ -293,8 +295,8 @@ void doesNotFailIfPropertiesConversionIsNull() {
293295
@Test
294296
void hasValueConverterReturnsFalseWhenNoPropertyValueConversionsAreConfigured() {
295297

296-
ConverterConfiguration configuration = new ConverterConfiguration(StoreConversions.NONE,
297-
Collections.emptyList(), it -> true, null);
298+
ConverterConfiguration configuration = new ConverterConfiguration(StoreConversions.NONE, Collections.emptyList(),
299+
it -> true, null);
298300

299301
CustomConversions conversions = new CustomConversions(configuration);
300302

@@ -315,8 +317,8 @@ public void hasValueConverterReturnsTrueWhenConverterRegisteredForProperty() {
315317

316318
doReturn(true).when(mockPropertyValueConversions).hasValueConverter(eq(mockProperty));
317319

318-
ConverterConfiguration configuration = new ConverterConfiguration(StoreConversions.NONE,
319-
Collections.emptyList(), it -> true, mockPropertyValueConversions);
320+
ConverterConfiguration configuration = new ConverterConfiguration(StoreConversions.NONE, Collections.emptyList(),
321+
it -> true, mockPropertyValueConversions);
320322

321323
CustomConversions conversions = new CustomConversions(configuration);
322324

@@ -328,6 +330,19 @@ public void hasValueConverterReturnsTrueWhenConverterRegisteredForProperty() {
328330
verifyNoInteractions(mockProperty);
329331
}
330332

333+
@Test // GH-3306
334+
void doesNotWarnForAsymmetricListConverter() {
335+
336+
Log actualLogger = (Log) ReflectionTestUtils.getField(CustomConversions.class, "logger");
337+
Log actualLoggerSpy = spy(actualLogger);
338+
ReflectionTestUtils.setField(CustomConversions.class, "logger", actualLoggerSpy, Log.class);
339+
340+
new CustomConversions(StoreConversions.NONE, List.of(ListOfNumberToStringConverter.INSTANCE));
341+
342+
verify(actualLoggerSpy, never()).warn(anyString());
343+
verify(actualLoggerSpy, never()).warn(anyString(), any());
344+
}
345+
331346
private static Class<?> createProxyTypeFor(Class<?> type) {
332347

333348
var factory = new ProxyFactory();
@@ -337,6 +352,16 @@ private static Class<?> createProxyTypeFor(Class<?> type) {
337352
return factory.getProxy().getClass();
338353
}
339354

355+
@ReadingConverter
356+
enum ListOfNumberToStringConverter implements Converter<List<Number>, String> {
357+
358+
INSTANCE;
359+
360+
public String convert(List<Number> source) {
361+
return source.toString();
362+
}
363+
}
364+
340365
enum FormatToStringConverter implements Converter<Format, String> {
341366

342367
INSTANCE;

0 commit comments

Comments
 (0)