Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make BclDateTimeZone behave more reasonably for midnight transitions #1527

Merged
merged 1 commit into from May 2, 2020
Merged

Make BclDateTimeZone behave more reasonably for midnight transitions #1527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 76 additions & 5 deletions src/NodaTime.Test/TimeZones/BclDateTimeZoneTest.cs
View file
Open in desktop
Expand Up @@ -2,11 +2,12 @@
// Use of this source code is governed by the Apache License 2.0,
// as found in the LICENSE.txt file.

using NodaTime.TimeZones;
using NUnit.Framework;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using NUnit.Framework;
using NodaTime.TimeZones;

namespace NodaTime.Test.TimeZones
{
Expand Down Expand Up @@ -267,6 +268,51 @@ public void FakeDaylightSavingTime()
Assert.AreEqual(expectedSummer, summerInterval);
}

// See https://github.com/nodatime/nodatime/issues/1524 for the background
// It would be nice to test earlier dates (e.g. 1967 and 1998) where transitions
// actually occurred on the first of the "next month", but the Windows database has very different
// data from TZDB in those cases.
[Test]
public void TransitionAtMidnight()
{
var bclZone = GetBclZoneOrIgnore("E. South America Standard Time");
var nodaTzdbZone = DateTimeZoneProviders.Tzdb["America/Sao_Paulo"];
var nodaBclZone = BclDateTimeZone.FromTimeZoneInfo(bclZone);

// Brazil in 2012:
// Fall back from -02:00 to -03:00 at midnight on February 26th
var expectedFallBack = Instant.FromUtc(2012, 2, 26, 2, 0, 0);
// Spring forward from -03:00 to -02:00 at midnight on October 21st
var expectedSpringForward = Instant.FromUtc(2012, 10, 21, 3, 0, 0);
// This is an arbitrary instant between the fall back and spring forward.
var betweenTransitions = Instant.FromUtc(2012, 6, 1, 0, 0, 0);

// Check that these transitions are as expected when we use TZDB.
var nodaTzdbInterval = nodaTzdbZone.GetZoneInterval(betweenTransitions);
Assert.AreEqual(expectedFallBack, nodaTzdbInterval.Start);
Assert.AreEqual(expectedSpringForward, nodaTzdbInterval.End);

// Check that the real BCL time zone behaves as reported in the issue: the transitions occur one millisecond early
var expectedFallBackBclTransition = expectedFallBack - Duration.FromMilliseconds(1);
Assert.AreEqual(TimeSpan.FromHours(-2), bclZone.GetUtcOffset(expectedFallBackBclTransition.ToDateTimeUtc() - TimeSpan.FromTicks(1)));
Assert.AreEqual(TimeSpan.FromHours(-3), bclZone.GetUtcOffset(expectedFallBackBclTransition.ToDateTimeUtc()));

var expectedSpringForwardBclTransition = expectedSpringForward - Duration.FromMilliseconds(1);
Assert.AreEqual(TimeSpan.FromHours(-3), bclZone.GetUtcOffset(expectedSpringForwardBclTransition.ToDateTimeUtc() - TimeSpan.FromTicks(1)));
Assert.AreEqual(TimeSpan.FromHours(-2), bclZone.GetUtcOffset(expectedSpringForwardBclTransition.ToDateTimeUtc()));

// Assert that Noda Time accounts for the Windows time zone data weirdness, and corrects it to
// a transition at midnight.
var nodaBclInterval = nodaBclZone.GetZoneInterval(betweenTransitions);
Assert.AreEqual(nodaTzdbInterval.Start, nodaBclInterval.Start);
Assert.AreEqual(nodaTzdbInterval.End, nodaBclInterval.End);

// Finally check the use case that was actually reported
var actualStartOfDayAfterSpringForward = nodaBclZone.AtStartOfDay(new LocalDate(2012, 10, 21));
var expectedStartOfDayAfterSpringForward = new LocalDateTime(2012, 10, 21, 1, 0, 0).InZoneStrictly(nodaBclZone);
Assert.AreEqual(expectedStartOfDayAfterSpringForward, actualStartOfDayAfterSpringForward);
}

private void ValidateZoneEquality(Instant instant, DateTimeZone nodaZone, TimeZoneInfo windowsZone)
{
// The BCL is basically broken (up to and including .NET 4.5.1 at least) around its interpretation
Expand All @@ -292,12 +338,37 @@ private void ValidateZoneEquality(Instant instant, DateTimeZone nodaZone, TimeZo
"Non-transition from {0} to {1}", previousInterval, interval);
}
var nodaOffset = interval.WallOffset;
var windowsOffset = windowsZone.GetUtcOffset(instant.ToDateTimeUtc());
Assert.AreEqual(windowsOffset, nodaOffset.ToTimeSpan(), "Incorrect offset at {0} in interval {1}", instant, interval);
var bclDaylight = windowsZone.IsDaylightSavingTime(instant.ToDateTimeUtc());

// Some midnight transitions in the Noda Time representation are actually corrections for the
// BCL data indicating 23:59:59.999 on the previous day. If we're testing around midnight,
// allow the Windows data to be correct for either of those instants.
var acceptableInstants = new List<Instant> { instant };
var localTimeOfDay = instant.InZone(nodaZone).TimeOfDay;
if ((localTimeOfDay == LocalTime.Midnight || localTimeOfDay == LocalTime.MaxValue) && instant > NodaConstants.BclEpoch)
{
acceptableInstants.Add(instant - Duration.FromMilliseconds(1));
}

var expectedOffsetAsTimeSpan = nodaOffset.ToTimeSpan();

// Find an instant that at least has the right offset (so will pass the first assertion).
var instantToTest = acceptableInstants.FirstOrDefault(candidate => windowsZone.GetUtcOffset(candidate.ToDateTimeUtc()) == expectedOffsetAsTimeSpan);
// If the test is definitely going to fail, just use the original instant that was passed in.
if (instantToTest == default)
{
instantToTest = instant;
}

var windowsOffset = windowsZone.GetUtcOffset(instantToTest.ToDateTimeUtc());
Assert.AreEqual(windowsOffset, expectedOffsetAsTimeSpan, "Incorrect offset at {0} in interval {1}", instantToTest, interval);
var bclDaylight = windowsZone.IsDaylightSavingTime(instantToTest.ToDateTimeUtc());
Assert.AreEqual(bclDaylight, interval.Savings != Offset.Zero,
"At {0}, BCL IsDaylightSavingTime={1}; Noda savings={2}",
instant, bclDaylight, interval.Savings);
}

private TimeZoneInfo GetBclZoneOrIgnore(string systemTimeZoneId) =>
TimeZoneInfo.GetSystemTimeZones().FirstOrDefault(z => z.Id == systemTimeZoneId)
?? Ignore.Throw<TimeZoneInfo>($"Time zone {systemTimeZoneId} not found");
}
}
23 changes: 21 additions & 2 deletions src/NodaTime/TimeZones/BclDateTimeZone.cs
View file
Open in desktop
Expand Up @@ -27,6 +27,12 @@ namespace NodaTime.TimeZones
/// if the BCL were ever to be fixed. As far as we are aware, there are only discrepancies around new year where the zone
/// changes from observing one rule to observing another.
/// </para>
/// <para>
/// As of version 3.0, a new "incompatible but doing the right thing" category of differences has been implemented,
/// for time zones which have a transition at 24:00. The Windows time zone data represents this as a transition at 23:59:59.999,
/// and that's faithfully represented by TimeZoneInfo (and BclDateTimeZone in version 2.x). As of 3.0, this is spotted
/// and converted to a midnight-on-the-following-day transition.
/// </para>
/// </remarks>
/// <threadsafety>This type is immutable reference type. See the thread safety section of the user guide for more information.</threadsafety>
[Immutable]
Expand Down Expand Up @@ -336,16 +342,29 @@ private static bool IsStandardOffsetOnlyRule(TimeZoneInfo.AdjustmentRule rule)
standard.TimeOfDay.TimeOfDay < TimeSpan.FromMinutes(1);
}

private static readonly LocalTime OneMillisecondBeforeMidnight = new LocalTime(23, 59, 59, 999);

// Converts a TimeZoneInfo "TransitionTime" to a "ZoneYearOffset" - the two correspond pretty closely.
private static ZoneYearOffset ConvertTransition(TimeZoneInfo.TransitionTime transitionTime)
{
// Used for both fixed and non-fixed transitions.
LocalTime timeOfDay = LocalDateTime.FromDateTime(transitionTime.TimeOfDay).TimeOfDay;

// Transitions at midnight are represented in the Windows database by a transition one millisecond early.
// See BclDateTimeZoneTest.TransitionAtMidnight for a concrete example.
// We adjust to midnight to represent the correct data - it's clear this is just a data fudge.
// It's probably done like this to allow the rule to represent "Saturday 24:00" instead of "Sunday 00:00".
bool addDay = false;
if (timeOfDay == OneMillisecondBeforeMidnight)
{
timeOfDay = LocalTime.Midnight;
addDay = true;
}

// Easy case - fixed day of the month.
if (transitionTime.IsFixedDateRule)
{
return new ZoneYearOffset(TransitionMode.Wall, transitionTime.Month, transitionTime.Day, 0, false, timeOfDay);
return new ZoneYearOffset(TransitionMode.Wall, transitionTime.Month, transitionTime.Day, 0, false, timeOfDay, addDay);
}

// Floating: 1st Sunday in March etc.
Expand All @@ -365,7 +384,7 @@ private static ZoneYearOffset ConvertTransition(TimeZoneInfo.TransitionTime tran
// Week 2 corresponds to ">=8" etc
dayOfMonth = (transitionTime.Week * 7) - 6;
}
return new ZoneYearOffset(TransitionMode.Wall, transitionTime.Month, dayOfMonth, dayOfWeek, advance, timeOfDay);
return new ZoneYearOffset(TransitionMode.Wall, transitionTime.Month, dayOfMonth, dayOfWeek, advance, timeOfDay, addDay);
}
}

Expand Down