Skip to content

Commit

Permalink
Disallow initializing CalDateTime from DateTimeKind.Local and som…
Browse files Browse the repository at this point in the history
…e cleanup (#704)

* Remove obsolete `IEvaluator.AssociatedObject`

* Remove obsolete `Evaluator.ConvertToIDateTime()`

* Enable `#nullable` for `Evaluator` classes.

* Change `IEvaluator.Evaluate()` start/end params from `DateTime` to `IDateTime` to avoid ambiguity.

* Add test `TestDtStartTimezone` from #406.

* CalDateTime: Disallow initializing from `DateTimeKind.Local`.

* Avoid using `DateTimeKind.Local` in test cases.
  • Loading branch information
minichma authored Jan 23, 2025
1 parent 9da55cc commit c8f791c
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 106 deletions.
18 changes: 18 additions & 0 deletions Ical.Net.Tests/CalDateTimeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Ical.Net.CalendarComponents;
using Ical.Net.DataTypes;
using NUnit.Framework;
using NUnit.Framework.Constraints;
using System;
using System.Collections;
using System.Collections.Generic;
Expand Down Expand Up @@ -343,4 +344,21 @@ public void AddAndSubtract_ShouldBeReversible(CalDateTime t, Duration d)
Assert.That(t.Add(d).SubtractExact(t), Is.EqualTo(d.ToTimeSpan()));
});
}

private static TestCaseData[] CalDateTime_FromDateTime_HandlesKindCorrectlyTestCases =>
[
new TestCaseData(DateTimeKind.Unspecified, Is.EqualTo(new CalDateTime(2024, 12, 30, 10, 44, 50, null))),
new TestCaseData(DateTimeKind.Utc, Is.EqualTo(new CalDateTime(2024, 12, 30, 10, 44, 50, "UTC"))),
new TestCaseData(DateTimeKind.Local, Throws.ArgumentException),
];

[Test, TestCaseSource(nameof(CalDateTime_FromDateTime_HandlesKindCorrectlyTestCases))]


public void CalDateTime_FromDateTime_HandlesKindCorrectly(DateTimeKind kind, IResolveConstraint constraint)
{
var dt = new DateTime(2024, 12, 30, 10, 44, 50, kind);

Assert.That(() => new CalDateTime(dt), constraint);
}
}
6 changes: 3 additions & 3 deletions Ical.Net.Tests/CopyComponentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public static IEnumerable CopyCalendarTest_TestCases()
yield return new TestCaseData(IcsFiles.XProperty2).SetName("XProperty2");
}

private static readonly DateTime _now = DateTime.Now;
private static readonly DateTime _now = DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified);
private static readonly DateTime _later = _now.AddHours(1);

private static CalendarEvent GetSimpleEvent() => new CalendarEvent
Expand Down Expand Up @@ -154,7 +154,7 @@ public void CopyTodoTest()
{
Summary = "Test Todo",
Description = "This is a test todo",
Due = new CalDateTime(DateTime.Now.AddDays(10)),
Due = new CalDateTime(DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified).AddDays(10)),
Priority = 1,
Contacts = new[] { "John", "Paul" },
Status = "NeedsAction"
Expand All @@ -180,7 +180,7 @@ public void CopyJournalTest()
{
Summary = "Test Journal",
Description = "This is a test journal",
DtStart = new CalDateTime(DateTime.Now),
DtStart = new CalDateTime(DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified)),
Categories = new List<string> { "Category1", "Category2" },
Priority = 1,
Status = "Draft"
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net.Tests/EqualityAndHashingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Ical.Net.Tests;
public class EqualityAndHashingTests
{
private const string TzId = "America/Los_Angeles";
private static readonly DateTime _nowTime = DateTime.Parse("2016-07-16T16:47:02.9310521-04:00");
private static readonly DateTime _nowTime = DateTime.SpecifyKind(DateTime.Parse("2016-07-16T16:47:02.9310521-04:00"), DateTimeKind.Unspecified);
private static readonly DateTime _later = _nowTime.AddHours(1);

[Test, TestCaseSource(nameof(CalDateTime_TestCases))]
Expand Down
95 changes: 56 additions & 39 deletions Ical.Net.Tests/RecurrenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1189,8 +1189,8 @@ public void YearlyByDay1()
[Test, Category("Recurrence")]
public void WeekNoOrderingShouldNotMatter()
{
var start = new DateTime(2019, 1, 1);
var end = new DateTime(2019, 12, 31);
var start = new CalDateTime(2019, 1, 1);
var end = new CalDateTime(2019, 12, 31);
var rpe1 = new RecurrencePatternEvaluator(new RecurrencePattern("FREQ=YEARLY;WKST=MO;BYDAY=MO;BYWEEKNO=1,3,5,7,9,11,13,15,17,19,21,23,25,27,29,31,33,35,37,39,41,43,45,47,49,51,53"));
var rpe2 = new RecurrencePatternEvaluator(new RecurrencePattern("FREQ=YEARLY;WKST=MO;BYDAY=MO;BYWEEKNO=53,51,49,47,45,43,41,39,37,35,33,31,29,27,25,23,21,19,17,15,13,11,9,7,5,3,1"));

Expand Down Expand Up @@ -2585,11 +2585,11 @@ public void DurationOfRecurrencesOverDst(string dtStart, string dtEnd, string d1
[Test, Category("Recurrence")]
public void BugByWeekNoNotWorking()
{
var start = new DateTime(2019, 1, 1);
var end = new DateTime(2019, 12, 31);
var start = new CalDateTime(2019, 1, 1);
var end = new CalDateTime(2019, 12, 31);
var rpe = new RecurrencePatternEvaluator(new RecurrencePattern("FREQ=WEEKLY;BYDAY=MO;BYWEEKNO=2"));

var recurringPeriods = rpe.Evaluate(new CalDateTime(start, false), start, end, false).ToList();
var recurringPeriods = rpe.Evaluate(start, start, end, false).ToList();

Assert.That(recurringPeriods, Has.Count.EqualTo(1));
Assert.That(recurringPeriods.First().StartTime, Is.EqualTo(new CalDateTime(2019, 1, 7)));
Expand All @@ -2601,11 +2601,11 @@ public void BugByWeekNoNotWorking()
[Test, Category("Recurrence")]
public void BugByMonthWhileFreqIsWeekly()
{
var start = new DateTime(2020, 1, 1);
var end = new DateTime(2020, 12, 31);
var start = new CalDateTime(2020, 1, 1);
var end = new CalDateTime(2020, 12, 31);
var rpe = new RecurrencePatternEvaluator(new RecurrencePattern("FREQ=WEEKLY;BYDAY=MO;BYMONTH=1"));

var recurringPeriods = rpe.Evaluate(new CalDateTime(start, false), start, end, false).OrderBy(x => x).ToList();
var recurringPeriods = rpe.Evaluate(start, start, end, false).OrderBy(x => x).ToList();

Assert.That(recurringPeriods, Has.Count.EqualTo(4));
Assert.Multiple(() =>
Expand Down Expand Up @@ -2644,11 +2644,11 @@ public void ReccurencePattern_MaxDate_StopsOnCount()
[Test, Category("Recurrence")]
public void BugByMonthWhileFreqIsMonthly()
{
var start = new DateTime(2020, 1, 1);
var end = new DateTime(2020, 12, 31);
var start = new CalDateTime(2020, 1, 1);
var end = new CalDateTime(2020, 12, 31);
var rpe = new RecurrencePatternEvaluator(new RecurrencePattern("FREQ=MONTHLY;BYDAY=MO;BYMONTH=1"));

var recurringPeriods = rpe.Evaluate(new CalDateTime(start, false), start, end, false).OrderBy(x => x).ToList();
var recurringPeriods = rpe.Evaluate(start, start, end, false).OrderBy(x => x).ToList();

Assert.That(recurringPeriods, Has.Count.EqualTo(4));
Assert.Multiple(() =>
Expand All @@ -2669,11 +2669,11 @@ public void Bug3119920()
{
using (var sr = new StringReader("FREQ=WEEKLY;UNTIL=20251126T120000;INTERVAL=1;BYDAY=MO"))
{
var start = DateTime.Parse("2010-11-27 9:00:00");
var start = new CalDateTime(2010, 11, 27, 9, 0, 0);
var serializer = new RecurrencePatternSerializer();
var rp = (RecurrencePattern)serializer.Deserialize(sr);
var rpe = new RecurrencePatternEvaluator(rp);
var recurringPeriods = rpe.Evaluate(new CalDateTime(start), start, rp.Until, false).ToList();
var recurringPeriods = rpe.Evaluate(new CalDateTime(start), start, rp.Until?.AsCalDateTime(), false).ToList();

var period = recurringPeriods.ElementAt(recurringPeriods.Count - 1);

Expand Down Expand Up @@ -2740,15 +2740,15 @@ public void Issue432()
};
var vEvent = new CalendarEvent
{
Start = new CalDateTime(DateTime.Parse("2019-01-04T08:00Z")),
Start = new CalDateTime(DateTime.Parse("2019-01-04T08:00Z").ToUniversalTime()),
};

vEvent.RecurrenceRules.Add(rrule);

//Testing on both the first day and the next, results used to be different
for (var i = 0; i <= 1; i++)
{
var checkTime = DateTime.Parse("2019-01-04T08:00Z");
var checkTime = DateTime.Parse("2019-01-04T08:00Z").ToUniversalTime();
checkTime = checkTime.AddDays(i);
//Valid asking for the exact moment
var occurrences = vEvent.GetOccurrences(checkTime, checkTime).ToList();
Expand Down Expand Up @@ -2897,8 +2897,8 @@ public void RecurrencePattern1()

var occurrences = evaluator.Evaluate(
startDate,
SimpleDateTimeToMatch(fromDate, startDate),
SimpleDateTimeToMatch(toDate, startDate),
fromDate,
toDate,
false)
.OrderBy(o => o.StartTime)
.ToList();
Expand Down Expand Up @@ -2930,8 +2930,8 @@ public void RecurrencePattern2()

var occurrences = evaluator.Evaluate(
startDate,
SimpleDateTimeToMatch(fromDate, startDate),
SimpleDateTimeToMatch(toDate, startDate),
fromDate,
toDate,
false);
Assert.That(occurrences.Count, Is.Not.EqualTo(0));
}
Expand Down Expand Up @@ -3038,8 +3038,8 @@ public void Test4()
// Add the exception dates
var periods = evaluator.Evaluate(
evtStart,
DateUtil.GetSimpleDateTimeData(evtStart),
SimpleDateTimeToMatch(evtEnd, evtStart),
evtStart,
evtEnd,
false)
.OrderBy(p => p.StartTime)
.ToList();
Expand Down Expand Up @@ -3295,7 +3295,7 @@ public void AddExDateToEventAfterGetOccurrencesShouldRecomputeResult()
Assert.That(occurrences, Has.Count.EqualTo(3));
}

private static readonly DateTime _now = DateTime.Now;
private static readonly DateTime _now = DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified);
private static readonly DateTime _later = _now.AddHours(1);
private static CalendarEvent GetEventWithRecurrenceRules()
{
Expand Down Expand Up @@ -3830,23 +3830,6 @@ public void ShouldCreateARecurringYearlyEvent()
Assert.That(occurrences.Count, Is.EqualTo(27));
}

private static DateTime SimpleDateTimeToMatch(IDateTime dt, IDateTime toMatch)
{
if (toMatch.IsUtc && dt.IsUtc)
{
return dt.Value;
}
if (toMatch.IsUtc)
{
return dt.Value.ToUniversalTime();
}
if (dt.IsUtc)
{
return dt.Value.ToLocalTime();
}
return dt.Value;
}

[Test]
public void GetOccurrenceShouldExcludeDtEndFloating()
{
Expand Down Expand Up @@ -3897,4 +3880,38 @@ public void TestGetOccurrenceIndefinite()

Assert.That(instances.Count(), Is.EqualTo(100));
}

[Test, Category("Recurrence")]
[TestCase(null)]
[TestCase("UTC")]
[TestCase("Europe/Vienna")]
[TestCase("America/New_York")]
public void TestDtStartTimezone(string tzId)
{
var icalText = """
BEGIN:VCALENDAR
BEGIN:VEVENT
UID:ignore
DTSTAMP:20180613T154237Z
DTSTART;TZID=Europe/Vienna:20180612T180000
DTEND;TZID=Europe/Vienna:20180612T181000
RRULE:FREQ=HOURLY
END:VEVENT
END:VCALENDAR
""";

var cal = Calendar.Load(icalText);
var evt = cal.Events.First();
var ev = new EventEvaluator(evt);
var occurrences = ev.Evaluate(evt.DtStart, evt.DtStart.ToTimeZone(tzId), evt.DtStart.AddMinutes(61).ToTimeZone(tzId), false);
var occurrencesStartTimes = occurrences.Select(x => x.StartTime).Take(2).ToList();

var expectedStartTimes = new[]
{
new CalDateTime(2018, 06, 12, 18, 0, 0, "Europe/Vienna"),
new CalDateTime(2018, 06, 12, 19, 0, 0, "Europe/Vienna"),
};

Assert.That(expectedStartTimes.SequenceEqual(occurrencesStartTimes), Is.True);
}
}
2 changes: 1 addition & 1 deletion Ical.Net.Tests/SerializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Ical.Net.Tests;
[TestFixture]
public class SerializationTests
{
private static readonly DateTime _nowTime = DateTime.Now;
private static readonly DateTime _nowTime = DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified);
private static readonly DateTime _later = _nowTime.AddHours(1);
private static CalendarSerializer GetNewSerializer() => new CalendarSerializer();
private static string SerializeToString(Calendar c) => GetNewSerializer().SerializeToString(c);
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net.Tests/SymmetricSerializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class SymmetricSerializationTests
{
private const string _ldapUri = "ldap://example.com:6666/o=eDABC Industries,c=3DUS??(cn=3DBMary Accepted)";

private static readonly DateTime _nowTime = DateTime.Now;
private static readonly DateTime _nowTime = DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified);
private static readonly DateTime _later = _nowTime.AddHours(1);
private static CalendarSerializer GetNewSerializer() => new CalendarSerializer();
private static string SerializeToString(Calendar c) => GetNewSerializer().SerializeToString(c);
Expand Down
16 changes: 13 additions & 3 deletions Ical.Net/DataTypes/CalDateTime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,25 @@ public CalDateTime(IDateTime value)
/// It will represent an RFC 5545, Section 3.3.4, DATE value, if <see paramref="hasTime"/> is <see langword="false"/>.
/// <para/>
/// The <see cref="TzId"/> will be set to "UTC" if the <paramref name="value"/>
/// has <see cref="DateTimeKind.Utc"/> and <paramref name="hasTime"/> is <see langword="true"/>.
/// Otherwise <see cref="TzId"/> will be <see langword="null"/>.
/// has kind <see cref="DateTimeKind.Utc"/> and <paramref name="hasTime"/> is <see langword="true"/>.
/// It will be set to <see langword="null"/> if the kind is kind <see cref="DateTimeKind.Unspecified"/>
/// and will throw otherwise.
/// </summary>
/// <param name="value">The <see cref="DateTime"/> value. Its <see cref="DateTimeKind"/> will be ignored.</param>
/// <param name="hasTime">
/// The instance will represent an RFC 5545, Section 3.3.5, DATE-TIME value, if <see paramref="hasTime"/> is <see langword="true"/>.
/// It will represent an RFC 5545, Section 3.3.4, DATE value, if <see paramref="hasTime"/> is <see langword="false"/>.
/// </param>
public CalDateTime(DateTime value, bool hasTime = true) : this(value, value.Kind == DateTimeKind.Utc ? UtcTzId : null, hasTime)
/// <exception cref="System.ArgumentException">If the specified value's kind is <see cref="DateTimeKind.Local"/></exception>
public CalDateTime(DateTime value, bool hasTime = true) : this(
value,
value.Kind switch
{
DateTimeKind.Utc => UtcTzId,
DateTimeKind.Unspecified => null,
_ => throw new ArgumentException($"An instance of {nameof(CalDateTime)} can only be initializd from a {nameof(DateTime)} of kind {nameof(DateTimeKind.Utc)} or {nameof(DateTimeKind.Unspecified)}.")
},
hasTime)
{ }

/// <summary>
Expand Down
23 changes: 2 additions & 21 deletions Ical.Net/Evaluation/Evaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.Collections.Generic;
using System.Globalization;
Expand All @@ -13,25 +14,11 @@ namespace Ical.Net.Evaluation;

public abstract class Evaluator : IEvaluator
{
private ICalendarObject _mAssociatedObject;

protected Evaluator()
{
Initialize();
}

private void Initialize()
{
Calendar = CultureInfo.CurrentCulture.Calendar;
}

protected IDateTime ConvertToIDateTime(DateTime dt, IDateTime referenceDate)
{
IDateTime newDt = new CalDateTime(dt, referenceDate.TzId);
newDt.AssociateWith(referenceDate);
return newDt;
}

protected void IncrementDate(ref DateTime dt, RecurrencePattern pattern, int interval)
{
if (interval == 0)
Expand Down Expand Up @@ -69,11 +56,5 @@ protected void IncrementDate(ref DateTime dt, RecurrencePattern pattern, int int

public System.Globalization.Calendar Calendar { get; private set; }

public virtual ICalendarObject AssociatedObject
{
get => _mAssociatedObject;
protected set => _mAssociatedObject = value;
}

public abstract IEnumerable<Period> Evaluate(IDateTime referenceDate, DateTime? periodStart, DateTime? periodEnd, bool includeReferenceDateInResults);
public abstract IEnumerable<Period> Evaluate(IDateTime referenceDate, IDateTime? periodStart, IDateTime? periodEnd, bool includeReferenceDateInResults);
}
2 changes: 1 addition & 1 deletion Ical.Net/Evaluation/EventEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public EventEvaluator(CalendarEvent evt) : base(evt) { }
/// <param name="periodEnd">The end date of the range to evaluate.</param>
/// <param name="includeReferenceDateInResults"></param>
/// <returns></returns>
public override IEnumerable<Period> Evaluate(IDateTime referenceTime, DateTime? periodStart, DateTime? periodEnd, bool includeReferenceDateInResults)
public override IEnumerable<Period> Evaluate(IDateTime referenceTime, IDateTime? periodStart, IDateTime? periodEnd, bool includeReferenceDateInResults)
{
// Evaluate recurrences normally
var periods = base.Evaluate(referenceTime, periodStart, periodEnd, includeReferenceDateInResults)
Expand Down
8 changes: 2 additions & 6 deletions Ical.Net/Evaluation/IEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.Collections.Generic;
using Ical.Net.DataTypes;
Expand All @@ -16,11 +17,6 @@ public interface IEvaluator
/// </summary>
System.Globalization.Calendar Calendar { get; }

/// <summary>
/// Gets the object associated with this evaluator.
/// </summary>
ICalendarObject AssociatedObject { get; }

/// <summary>
/// Evaluates this item to determine the dates and times for which it occurs/recurs.
/// This method only evaluates items which occur/recur between <paramref name="periodStart"/>
Expand All @@ -46,5 +42,5 @@ public interface IEvaluator
/// A sequence of <see cref="Ical.Net.DataTypes.Period"/> objects for
/// each date/time when this item occurs/recurs.
/// </returns>
IEnumerable<Period> Evaluate(IDateTime referenceDate, DateTime? periodStart, DateTime? periodEnd, bool includeReferenceDateInResults);
IEnumerable<Period> Evaluate(IDateTime referenceDate, IDateTime? periodStart, IDateTime? periodEnd, bool includeReferenceDateInResults);
}
Loading

0 comments on commit c8f791c

Please sign in to comment.