From 3dbbb15672d4f46184b351a5837ac5fb2b756c30 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 3 Jul 2017 15:38:54 +0200 Subject: [PATCH] DATACMNS-1101 - Use dedicated type to cache custom conversion targets without requested target type. We now use a marker interface to store cache hits for conversion targets without requesting a conversion target in the conversion query. The dedicated marker type has an individual cache view and does not interfere with other types. Previously we used Object as cache key which caused false positives due to assignability checks. If an earlier conversion query with a requested target type yielded a hit (e.g. negative hit), subsequent queries without a target type received the same answer. This might happen although the non-cached answer would return a different result. --- .../data/convert/CustomConversions.java | 7 ++++++- .../data/convert/CustomConversionsUnitTests.java | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java index fdf7ced74..652dce516 100644 --- a/src/main/java/org/springframework/data/convert/CustomConversions.java +++ b/src/main/java/org/springframework/data/convert/CustomConversions.java @@ -372,7 +372,7 @@ public class CustomConversions { */ public Optional> computeIfAbsent(Class sourceType, Function>> mappingFunction) { - return computeIfAbsent(sourceType, Object.class, mappingFunction); + return computeIfAbsent(sourceType, AbsentTargetTypeMarker.class, mappingFunction); } /** @@ -391,6 +391,11 @@ public class CustomConversions { TargetTypes targetTypes = customReadTargetTypes.computeIfAbsent(sourceType, TargetTypes::new); return targetTypes.computeIfAbsent(targetType, mappingFunction); } + + /** + * Marker type for absent target type caching. + */ + interface AbsentTargetTypeMarker {} } /** diff --git a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java index 6bb6788c0..f5dcf244d 100644 --- a/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java +++ b/src/test/java/org/springframework/data/convert/CustomConversionsUnitTests.java @@ -44,6 +44,7 @@ import org.threeten.bp.LocalDateTime; * * @author Oliver Gierke * @author Christoph Strobl + * @author Mark Paluch * @since 2.0 */ public class CustomConversionsUnitTests { @@ -71,6 +72,17 @@ public class CustomConversionsUnitTests { assertThat(conversions.hasCustomReadTarget(String.class, Long.class)).isTrue(); } + @Test // DATACMNS-1101 + public void considersSubtypeCachingCorrectly() { + + CustomConversions conversions = new CustomConversions(StoreConversions.NONE, + Arrays.asList(NumberToStringConverter.INSTANCE, StringToNumberConverter.INSTANCE)); + + assertThat(conversions.getCustomWriteTarget(Long.class, Object.class)).isEmpty(); + assertThat(conversions.getCustomWriteTarget(Long.class)).hasValue(String.class); + assertThat(conversions.getCustomWriteTarget(Long.class, Object.class)).isEmpty(); + } + @Test // DATACMNS-1035 public void populatesConversionServiceCorrectly() {