GH-2324 - Improve handling of 0-valued IsoDuration.

Introduce a dedicated converter in parallel to the already existing
adapter, that is able to augment the returned type in case a zero based
`IsoDuration` has been received with the actual target type.

Closes #2324.

Author: michael-simons

src/main/java/org/springframework/data/neo4j/core/convert/AdditionalTypes.java

@@ -25,7 +25,6 @@
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
-import java.time.temporal.TemporalAmount;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -93,8 +92,7 @@ final class AdditionalTypes {
 		hlp.add(ConverterBuilder.reading(Value.class, String[].class, AdditionalTypes::asStringArray).andWriting(Values::value));
 		hlp.add(ConverterBuilder.reading(Value.class, BigDecimal.class, AdditionalTypes::asBigDecimal).andWriting(AdditionalTypes::value));
 		hlp.add(ConverterBuilder.reading(Value.class, BigInteger.class, AdditionalTypes::asBigInteger).andWriting(AdditionalTypes::value));
-		hlp.add(ConverterBuilder.reading(Value.class, TemporalAmount.class, AdditionalTypes::asTemporalAmount)
-				.andWriting(AdditionalTypes::value));
+		hlp.add(new TemporalAmountConverter());
 		hlp.add(ConverterBuilder.reading(Value.class, Instant.class, AdditionalTypes::asInstant).andWriting(AdditionalTypes::value));
 		hlp.add(ConverterBuilder.reading(Value.class, UUID.class, AdditionalTypes::asUUID).andWriting(AdditionalTypes::value));
 		hlp.add(ConverterBuilder.reading(Value.class, URL.class, AdditionalTypes::asURL).andWriting(AdditionalTypes::value));
@@ -179,14 +177,6 @@ static Value value(Instant instant) {
 		return Values.value(instant.atOffset(ZoneOffset.UTC));
 	}
 
-	static TemporalAmount asTemporalAmount(Value value) {
-		return new TemporalAmountAdapter().apply(value.asIsoDuration());
-	}
-
-	static Value value(TemporalAmount temporalAmount) {
-		return Values.value(temporalAmount);
-	}
-
 	static BigDecimal asBigDecimal(Value value) {
 		return new BigDecimal(value.asString());
 	}

src/main/java/org/springframework/data/neo4j/core/convert/TemporalAmountConverter.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright 2011-2021 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.data.neo4j.core.convert;
+
+import java.time.Duration;
+import java.time.Period;
+import java.time.temporal.TemporalAmount;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.neo4j.driver.Value;
+import org.neo4j.driver.Values;
+import org.neo4j.driver.types.IsoDuration;
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.core.convert.converter.GenericConverter;
+
+/**
+ * This generic converter has been introduce to augment the {@link TemporalAmountAdapter} with the type information passed
+ * to a generic converter to make some educated guesses whether an {@link org.neo4j.driver.types.IsoDuration} of {@literal 0}
+ * should be possibly treated as {@link java.time.Period} or {@link java.time.Duration}.
+ *
+ * @author Michael J. Simons
+ * @soundtrack Motörhead - Bomber
+ */
+final class TemporalAmountConverter implements GenericConverter {
+
+	private final TemporalAmountAdapter adapter = new TemporalAmountAdapter();
+	private final Set<ConvertiblePair> convertibleTypes = Collections.unmodifiableSet(
+			new HashSet<>(Arrays.asList(
+					new ConvertiblePair(Value.class, TemporalAmount.class),
+					new ConvertiblePair(TemporalAmount.class, Value.class)
+			)));
+
+	@Override
+	public Set<ConvertiblePair> getConvertibleTypes() {
+		return convertibleTypes;
+	}
+
+	@Override
+	public Object convert(Object value, TypeDescriptor sourceType, TypeDescriptor targetType) {
+
+		if (TemporalAmount.class.isAssignableFrom(sourceType.getType())) {
+			return Values.value(value);
+		}
+
+		boolean valueIsLiteralNullOrNullValue = value == null || value == Values.NULL;
+		Object convertedValue = valueIsLiteralNullOrNullValue ? null : adapter.apply(((Value) value).asIsoDuration());
+
+		if (convertedValue instanceof IsoDuration && isZero((IsoDuration) convertedValue)) {
+			if (Period.class.isAssignableFrom(targetType.getType())) {
+				return Period.of(0, 0, 0);
+			} else if (Duration.class.isAssignableFrom(targetType.getType())) {
+				return Duration.ZERO;
+			}
+		}
+		return convertedValue;
+	}
+
+	/**
+	 * @param isoDuration The duration to check whether it's {@literal 0} or not.
+	 * @return True if there are only temporal units in that duration with a value of {@literal 0}.
+	 */
+	private static boolean isZero(IsoDuration isoDuration) {
+
+		return isoDuration.months() == 0L && isoDuration.days() == 0L &&
+			   isoDuration.seconds() == 0L && isoDuration.nanoseconds() == 0L;
+	}
+}

src/test/java/org/springframework/data/neo4j/core/convert/TemporalAmountAdapterTest.java

@@ -24,39 +24,47 @@
 
 import org.junit.jupiter.api.Test;
 import org.neo4j.driver.Values;
+import org.neo4j.driver.types.IsoDuration;
 
 /**
  * @author Michael J. Simons
  */
 class TemporalAmountAdapterTest {
+
+	private final TemporalAmountAdapter underTest = new TemporalAmountAdapter();
+
 	@Test
 	public void internallyCreatedTypesShouldBeConvertedCorrect() {
-		final TemporalAmountAdapter adapter = new TemporalAmountAdapter();
 
-		assertThat(adapter.apply(Values.isoDuration(1, 0, 0, 0).asIsoDuration())).isEqualTo(Period.ofMonths(1));
-		assertThat(adapter.apply(Values.isoDuration(1, 1, 0, 0).asIsoDuration())).isEqualTo(Period.ofMonths(1).plusDays(1));
-		assertThat(adapter.apply(Values.isoDuration(1, 1, 1, 0).asIsoDuration()))
+		assertThat(underTest.apply(Values.isoDuration(1, 0, 0, 0).asIsoDuration())).isEqualTo(Period.ofMonths(1));
+		assertThat(underTest.apply(Values.isoDuration(1, 1, 0, 0).asIsoDuration())).isEqualTo(Period.ofMonths(1).plusDays(1));
+		assertThat(underTest.apply(Values.isoDuration(1, 1, 1, 0).asIsoDuration()))
 				.isEqualTo(Values.isoDuration(1, 1, 1, 0).asIsoDuration());
-		assertThat(adapter.apply(Values.isoDuration(0, 0, 120, 1).asIsoDuration()))
+		assertThat(underTest.apply(Values.isoDuration(0, 0, 120, 1).asIsoDuration()))
 				.isEqualTo(Duration.ofMinutes(2).plusNanos(1));
 	}
 
 	@Test
 	public void durationsShouldStayDurations() {
-		final TemporalAmountAdapter adapter = new TemporalAmountAdapter();
 
 		Duration duration = ChronoUnit.MONTHS.getDuration().multipliedBy(13).plus(ChronoUnit.DAYS.getDuration().multipliedBy(32)).plusHours(25)
 				.plusMinutes(120);
 
-		assertThat(adapter.apply(Values.value(duration).asIsoDuration())).isEqualTo(duration);
+		assertThat(underTest.apply(Values.value(duration).asIsoDuration())).isEqualTo(duration);
 	}
 
 	@Test
 	public void periodsShouldStayPeriods() {
-		final TemporalAmountAdapter adapter = new TemporalAmountAdapter();
 
 		Period period = Period.between(LocalDate.of(2018, 11, 15), LocalDate.of(2020, 12, 24));
 
-		assertThat(adapter.apply(Values.value(period).asIsoDuration())).isEqualTo(period.normalized());
+		assertThat(underTest.apply(Values.value(period).asIsoDuration())).isEqualTo(period.normalized());
+	}
+
+	@Test // GH-2324
+	public void zeroDurationShouldReturnTheIsoDuration() {
+
+		IsoDuration zeroDuration = Values.isoDuration(0, 0, 0, 0).asIsoDuration();
+		assertThat(underTest.apply(zeroDuration)).isSameAs(zeroDuration);
 	}
 }

src/test/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConversionServiceTest.java

@@ -15,10 +15,14 @@
  */
 package org.springframework.data.neo4j.core.mapping;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 
+import java.time.Duration;
 import java.time.LocalDate;
+import java.time.Period;
 import java.time.format.DateTimeParseException;
+import java.time.temporal.TemporalAmount;
 import java.util.Date;
 
 import org.junit.jupiter.api.Nested;
@@ -43,6 +47,35 @@ class DefaultNeo4jConversionServiceTest {
 
 	@Nested
 	class Reads {
+
+		@Test // GH-2324
+		void shouldDealWith0IsoDurationsAsPeriods() {
+			Value zeroDuration = Values.isoDuration(0, 0, 0, 0);
+
+			Period period = (Period) defaultNeo4jEntityAccessor.readValue(zeroDuration, ClassTypeInformation.from(Period.class), null);
+			assertThat(period.isZero()).isTrue();
+		}
+
+		@Test // GH-2324
+		void shouldDealWith0IsoDurationsAsDurations() {
+			Value zeroDuration = Values.isoDuration(0, 0, 0, 0);
+
+			Duration duration = (Duration) defaultNeo4jEntityAccessor.readValue(zeroDuration, ClassTypeInformation.from(Duration.class), null);
+			assertThat(duration).isZero();
+		}
+
+		@Test // GH-2324
+		void shouldDealWithNullTemporalValueOnRead() {
+			Duration duration = (Duration) defaultNeo4jEntityAccessor.readValue(null, ClassTypeInformation.from(Duration.class), null);
+			assertThat(duration).isNull();
+		}
+
+		@Test // GH-2324
+		void shouldDealWithNullTemporalValueOnWrite() {
+			Value value = defaultNeo4jEntityAccessor.writeValue(null, ClassTypeInformation.from(TemporalAmount.class), null);
+			assertThat(value).isNull();
+		}
+
 		@Test
 		void shouldCatchConversionErrors() {
 			Value value = Values.value("Das funktioniert nicht.");
@@ -65,7 +98,7 @@ void shouldCatchUncoercibleErrors() {
 		}
 
 		@Test
-		void shouldCatchUncoerfcibleErrors() {
+		void shouldCatchCoercibleErrors() {
 			Value value = Values.value("Das funktioniert nicht.");
 
 			assertThatExceptionOfType(TypeMismatchDataAccessException.class).isThrownBy(

src/test/java/org/springframework/data/neo4j/integration/shared/common/ThingWithAllCypherTypes.java

@@ -21,10 +21,12 @@
 import lombok.Data;
 import lombok.With;
 
+import java.time.Duration;
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.LocalTime;
 import java.time.OffsetTime;
+import java.time.Period;
 import java.time.ZonedDateTime;
 
 import org.neo4j.driver.types.IsoDuration;
@@ -69,4 +71,8 @@ public class ThingWithAllCypherTypes {
 	private IsoDuration anIsoDuration;
 
 	private Point aPoint;
+
+	private Period aZeroPeriod;
+
+	private Duration aZeroDuration;
 }

src/test/java/org/springframework/data/neo4j/integration/shared/conversion/Neo4jConversionsITBase.java

@@ -123,6 +123,8 @@ public abstract class Neo4jConversionsITBase {
 		hlp.put("listOfDoubles", Arrays.asList(1.0));
 		hlp.put("aTimeZone", TimeZone.getTimeZone("America/Los_Angeles"));
 		hlp.put("aZoneId", ZoneId.of("America/New_York"));
+		hlp.put("aZeroPeriod", Period.of(0, 0, 0));
+		hlp.put("aZeroDuration", Duration.ZERO);
 		ADDITIONAL_TYPES = Collections.unmodifiableMap(hlp);
 	}
 
@@ -202,37 +204,36 @@ static void prepareData() {
 
 				parameters = new HashMap<>();
 				parameters.put("aByteArray", "A thing".getBytes());
-				ID_OF_CYPHER_TYPES_NODE = w.run("CREATE (n:CypherTypes) SET " + " n.aBoolean = true,"
-						+ " n.aLong = 9223372036854775807," + " n.aDouble = 1.7976931348," + " n.aString = 'Hallo, Cypher',"
-						+ " n.aByteArray = $aByteArray," + " n.aLocalDate = date('2015-07-21'),"
-						+ " n.anOffsetTime  = time({ hour:12, minute:31, timezone: '+01:00' }),"
-						+ " n.aLocalTime = localtime({ hour:12, minute:31, second:14 }),"
-						+ " n.aZoneDateTime = datetime('2015-07-21T21:40:32-04[America/New_York]'),"
-						+ " n.aLocalDateTime = localdatetime('2015202T21')," + " n.anIsoDuration = duration('P14DT16H12M'),"
-						+ " n.aPoint = point({x:47, y:11})" + " RETURN id(n) AS id", parameters).single().get("id").asLong();
+				ID_OF_CYPHER_TYPES_NODE = w.run("CREATE (n:CypherTypes) SET  n.aBoolean = true,\n"
+						+ " n.aLong = 9223372036854775807, n.aDouble = 1.7976931348, n.aString = 'Hallo, Cypher',\n"
+						+ " n.aByteArray = $aByteArray, n.aLocalDate = date('2015-07-21'),\n"
+						+ " n.anOffsetTime  = time({ hour:12, minute:31, timezone: '+01:00' }),\n"
+						+ " n.aLocalTime = localtime({ hour:12, minute:31, second:14 }),\n"
+						+ " n.aZoneDateTime = datetime('2015-07-21T21:40:32-04[America/New_York]'),\n"
+						+ " n.aLocalDateTime = localdatetime('2015202T21'), n.anIsoDuration = duration('P14DT16H12M'),\n"
+						+ " n.aPoint = point({x:47, y:11})\n"
+						+ " RETURN id(n) AS id", parameters).single().get("id").asLong();
 
 				parameters = new HashMap<>();
 				parameters.put("aByte", Values.value(new byte[] { 6 }));
-				ID_OF_ADDITIONAL_TYPES_NODE = w
-						.run("CREATE (n:AdditionalTypes) SET " + " n.booleanArray = [true, true, false]," + " n.aByte = $aByte,"
-								+ " n.aChar = 'x'," + " n.charArray = ['x', 'y', 'z']," + " n.aDate = '2019-09-21T13:23:11Z',"
-								+ " n.doubleArray = [1.1, 2.2, 3.3]," + " n.aFloat = '23.42'," + " n.floatArray = ['4.4', '5.5'],"
-								+ " n.anInt = 42," + " n.intArray = [21, 9]," + " n.aLocale = 'de_DE',"
-								+ " n.longArray = [-9223372036854775808, 9223372036854775807]," + " n.aShort = 127,"
-								+ " n.shortArray = [-10, 10]," + " n.aBigDecimal = '1.79769313486231570E+309',"
-								+ " n.aBigInteger = '92233720368547758070'," + " n.aPeriod = duration('P23Y4M7D'),"
-								+ " n.aDuration = duration('PT26H4M5S')," + " n.stringArray = ['Hallo', 'Welt'],"
-								+ " n.listOfStrings = ['Hello', 'World']," + " n.setOfStrings = ['Hallo', 'Welt'],"
-								+ " n.anInstant = datetime('2019-09-26T20:34:23Z'),"
-								+ " n.aUUID = 'd4ec9208-4b17-4ec7-a709-19a5e53865a8'," + " n.listOfDoubles = [1.0],"
-								+ " n.aURL = 'https://www.test.com',"
-								+ " n.aURI = 'urn:isbn:9783864905254',"
-								+ " n.anEnum = 'TheUsualMisfit'," + " n.anArrayOfEnums = ['ValueA', 'ValueB'],"
-								+ " n.aCollectionOfEnums = ['ValueC', 'TheUsualMisfit'],"
-							 	+ " n.aTimeZone = 'America/Los_Angeles', "
-							 	+ " n.aZoneId = 'America/New_York'"
-							 	+ " RETURN id(n) AS id", parameters)
-						.single().get("id").asLong();
+				ID_OF_ADDITIONAL_TYPES_NODE = w.run("CREATE (n:AdditionalTypes) SET  n.booleanArray = [true, true, false], n.aByte = $aByte,\n"
+						+ " n.aChar = 'x', n.charArray = ['x', 'y', 'z'], n.aDate = '2019-09-21T13:23:11Z',\n"
+						+ " n.doubleArray = [1.1, 2.2, 3.3], n.aFloat = '23.42', n.floatArray = ['4.4', '5.5'],\n"
+						+ " n.anInt = 42, n.intArray = [21, 9], n.aLocale = 'de_DE',\n"
+						+ " n.longArray = [-9223372036854775808, 9223372036854775807], n.aShort = 127,\n"
+						+ " n.shortArray = [-10, 10], n.aBigDecimal = '1.79769313486231570E+309',\n"
+						+ " n.aBigInteger = '92233720368547758070', n.aPeriod = duration('P23Y4M7D'),\n"
+						+ " n.aDuration = duration('PT26H4M5S'), n.stringArray = ['Hallo', 'Welt'],\n"
+						+ " n.listOfStrings = ['Hello', 'World'], n.setOfStrings = ['Hallo', 'Welt'],\n"
+						+ " n.anInstant = datetime('2019-09-26T20:34:23Z'),\n"
+						+ " n.aUUID = 'd4ec9208-4b17-4ec7-a709-19a5e53865a8', n.listOfDoubles = [1.0],\n"
+						+ " n.aURL = 'https://www.test.com',\n"
+						+ " n.aURI = 'urn:isbn:9783864905254',\n"
+						+ " n.anEnum = 'TheUsualMisfit', n.anArrayOfEnums = ['ValueA', 'ValueB'],\n"
+						+ " n.aCollectionOfEnums = ['ValueC', 'TheUsualMisfit'],\n"
+						+ " n.aTimeZone = 'America/Los_Angeles', \n"
+						+ " n.aZoneId = 'America/New_York', n.aZeroPeriod = duration('PT0S'), n.aZeroDuration = duration('PT0S')\n"
+						+ " RETURN id(n) AS id", parameters).single().get("id").asLong();
 
 				parameters = new HashMap<>();
 				parameters.put("neo4j", NEO_HQ.toParameterMap());

src/test/java/org/springframework/data/neo4j/integration/shared/conversion/ThingWithAllAdditionalTypes.java

@@ -118,4 +118,8 @@ enum SomeEnum {
 	private TimeZone aTimeZone;
 
 	private ZoneId aZoneId;
+
+	private Period aZeroPeriod;
+
+	private Duration aZeroDuration;
 }