Unify duration string representation and add tests (#885)

Author: RichardIrons-neo4j

Neo4j.Driver/Neo4j.Driver.Tests/Types/DurationTests.cs

@@ -15,6 +15,7 @@
 
 using System;
 using System.Collections;
+using System.Globalization;
 using FluentAssertions;
 using Xunit;
 
@@ -67,18 +68,17 @@ public void ShouldCreateDurationWithDaysSecondsAndNanoseconds()
     }
 
     [Theory]
-    [InlineData(15, 32, 785, 789215800, "P15M32DT785.789215800S")]
-    [InlineData(0, 32, 785, 789215800, "P0M32DT785.789215800S")]
-    [InlineData(0, 0, 785, 789215800, "P0M0DT785.789215800S")]
-    [InlineData(0, 0, 0, 789215800, "P0M0DT0.789215800S")]
-    [InlineData(0, 0, -1, 0, "P0M0DT-1S")]
-    [InlineData(0, 0, 0, 999999999, "P0M0DT0.999999999S")]
-    [InlineData(0, 0, -1, 5, "P0M0DT-0.999999995S")]
-    [InlineData(0, 0, -1, 999999999, "P0M0DT-0.000000001S")]
-    [InlineData(500, 0, 0, 0, "P500M0DT0S")]
-    [InlineData(0, 0, 0, 5, "P0M0DT0.000000005S")]
-    [InlineData(0, 0, -500, 1, "P0M0DT-499.999999999S")]
-    [InlineData(0, 0, -500, 0, "P0M0DT-500S")]
+    [InlineData(15, 32, 785, 789215800, "P1Y3M32DT13M5.789215800S")] // 15M -> 1Y3M, 785S -> 13M5S
+    [InlineData(0, 32, 785, 789215800, "P32DT13M5.789215800S")]    // 785S -> 13M5S
+    [InlineData(0, 0, 785, 789215800, "PT13M5.789215800S")]      // 785S -> 13M5S
+    [InlineData(0, 0, 0, 789215800, "PT0.789215800S")]
+    [InlineData(0, 0, -1, 0, "PT-1S")]
+    [InlineData(0, 0, 0, 999999999, "PT0.999999999S")]
+    [InlineData(0, 0, -1, 5, "PT-0.999999995S")]
+    [InlineData(0, 0, -1, 999999999, "PT-0.000000001S")]
+    [InlineData(0, 0, 0, 5, "PT0.000000005S")]
+    [InlineData(0, 0, -500, 1, "PT-8M19.999999999S")]          // -500S -> -8M20S, then +1NS
+    [InlineData(0, 0, -500, 0, "PT-8M20S")]                    // -500S -> -8M20S
     [InlineData(-10, 5, -2, 500, "P-10M5DT-1.999999500S")]
     [InlineData(-10, -5, -2, 500, "P-10M-5DT-1.999999500S")]
     public void ShouldGenerateCorrectString(int months, int days, int seconds, int nanoseconds, string expected)
@@ -233,11 +233,13 @@ public void ShouldReportSmallerOnCompareToAbsolute()
     public void ShouldBeConvertableToString()
     {
         var duration = new Duration(12, 15, 59, 660000999);
-        var durationStr1 = Convert.ToString(duration);
+        var durationStr1 = Convert.ToString(duration, CultureInfo.InvariantCulture);
         var durationStr2 = Convert.ChangeType(duration, typeof(string));
 
-        durationStr1.Should().Be("P12M15DT59.660000999S");
-        durationStr2.Should().Be("P12M15DT59.660000999S");
+        const string expected = "P1Y15DT59.660000999S";
+
+        durationStr1.Should().Be(expected);
+        durationStr2.Should().Be(expected);
     }
 
     [Fact]

Neo4j.Driver/Neo4j.Driver/Internal/Helpers/TemporalHelpers.cs

@@ -42,12 +42,13 @@ internal static class TemporalHelpers
     public const long NanosPerSecond = 1_000_000_000;
     public const long NanosPerDay = NanosPerHour * HoursPerDay;
 
+    private const int MonthsPerYear = 12;
     private const int HoursPerDay = 24;
     private const int MinutesPerHour = 60;
     private const int SecondsPerMinute = 60;
     private const int SecondsPerHour = SecondsPerMinute * MinutesPerHour;
     private const int SecondsPerDay = SecondsPerHour * HoursPerDay;
-    private const long NanosPerMinute = NanosPerSecond * SecondsPerMinute;
+    private const long NanosPerMinute = (long)NanosPerSecond * SecondsPerMinute;
     private const long NanosPerHour = NanosPerMinute * MinutesPerHour;
 
     private const long Days0000To1970 = DaysPerCycle * 5L - (30L * 365L + 7L);
@@ -270,29 +271,61 @@ public static void AssertNoTruncation(TimeSpan offset, string target)
 
     public static string ToIsoDurationString(long months, long days, long seconds, int nanoseconds)
     {
-        var timePart = string.Empty;
-
+        // carry the excess up each level
+        seconds += nanoseconds / NanosPerSecond;
+        nanoseconds %= (int)NanosPerSecond;
+        var minutes = seconds / SecondsPerMinute;
+        seconds %= SecondsPerMinute;
+        var hours = minutes / MinutesPerHour;
+        minutes %= MinutesPerHour;
+        days += hours / HoursPerDay;
+        hours %= HoursPerDay;
+        var years = months / MonthsPerYear;
+        months %= MonthsPerYear;
+        
+        // do negative second/nanosecond handling
+        var negativeTime = hours < 0 || minutes < 0 || seconds < 0 || nanoseconds < 0;
+        var timeSign = negativeTime ? "-" : "";
         if (seconds < 0 && nanoseconds > 0)
         {
-            seconds = seconds + 1;
+            seconds++;
             nanoseconds = (int)NanosPerSecond - nanoseconds;
-
-            if (seconds == 0)
-            {
-                timePart = "-";
-            }
         }
-
-        if (nanoseconds == 0)
+        
+        hours = Math.Abs(hours);
+        minutes = Math.Abs(minutes);
+        seconds = Math.Abs(seconds);
+        nanoseconds = Math.Abs(nanoseconds);
+        
+        var dateComponent = years != 0 || months != 0 || days != 0
+            ? $"{IfNonZero(years, 'Y')}{IfNonZero(months, 'M')}{IfNonZero(days, 'D')}"
+            : "";
+        
+        string timeComponent = hours != 0 || minutes != 0 || seconds != 0 || nanoseconds != 0
+            ? $"T{timeSign}{IfNonZero(hours, 'H')}{IfNonZero(minutes, 'M')}{Seconds()}"
+            : "";
+
+        // if both empty, return P0D
+        if (dateComponent == "" && timeComponent == "")
         {
-            timePart = $"{timePart}{seconds}";
-        }
-        else
+            return "P0D";
+        }       
+        
+        return $"P{dateComponent}{timeComponent}";
+
+        string IfNonZero(long amount, char identifier) => amount != 0 ? $"{amount}{identifier}" : "";
+       
+        string Seconds()
         {
-            timePart = $"{timePart}{seconds}.{nanoseconds:D9}";
-        }
+            if (seconds == 0 && nanoseconds == 0)
+            {
+                return "";
+            }
 
-        return $"P{months}M{days}DT{timePart}S";
+            var secstr = $"{seconds}";
+            var nanosecstr = nanoseconds > 0 ? $".{nanoseconds:D9}" : "";
+            return secstr + nanosecstr + "S";
+        }
     }
 
     public static string ToIsoDateString(int year, int month, int day)

Neo4j.Driver/Neo4j.Driver/Public/Types/Duration.cs

@@ -215,18 +215,12 @@ public override bool Equals(object obj)
     /// <returns>A 32-bit signed integer hash code.</returns>
     public override int GetHashCode()
     {
-        unchecked
-        {
-            var hashCode = Months.GetHashCode();
-            hashCode = (hashCode * 397) ^ Days.GetHashCode();
-            hashCode = (hashCode * 397) ^ Seconds.GetHashCode();
-            hashCode = (hashCode * 397) ^ Nanos;
-            return hashCode;
-        }
+        return HashCode.Combine(Months, Days, Seconds, Nanos);
     }
 
-    /// <summary>Converts the value of the current <see cref="Duration"/> object to its equivalent string representation.</summary>
-    /// <returns>String representation of this Point.</returns>
+    /// <summary>Converts the value of the current <see cref="Duration"/> object to its equivalent string representation
+    /// as an ISO 8601 duration string.</summary>
+    /// <returns>ISO 8601 duration string representation of this Duration.</returns>
     public override string ToString()
     {
         return TemporalHelpers.ToIsoDurationString(Months, Days, Seconds, Nanos);

Neo4j.Driver/Neo4j.Driver/Public/Types/OffsetTime.cs

@@ -170,8 +170,8 @@ public int CompareTo(OffsetTime other)
             return 1;
         }
 
-        var thisNanoOfDay = this.ToNanoOfDay() - OffsetSeconds * TemporalHelpers.NanosPerSecond;
-        var otherNanoOfDay = other.ToNanoOfDay() - other.OffsetSeconds * TemporalHelpers.NanosPerSecond;
+        var thisNanoOfDay = this.ToNanoOfDay() - (long)OffsetSeconds * TemporalHelpers.NanosPerSecond;
+        var otherNanoOfDay = other.ToNanoOfDay() - (long)other.OffsetSeconds * TemporalHelpers.NanosPerSecond;
 
         if (thisNanoOfDay < 0)
         {