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.

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

Disallow initializing CalDateTime from DateTimeKind.Local and some cleanup #704

Merged
merged 7 commits into from
Jan 23, 2025
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);
Copy link
Collaborator

@axunonb axunonb Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nullable arguments should have #nullable enable. The effort shouldn't be too big? Otherwise I could just do that in another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. It actually wasn't my intention to introduce nullable, but now that it's already there, I tried to enable #nullable for the affected files :-D I'm not using it too much, so please have a close look!

}
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
Loading