Skip to content

Enable NRT for AlarmOccurrence,Attachment, Attendee, CalendarDataType, Duration, ICalendarDataType, ICalendarParameterCollectionContainer #762

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

Merged
axunonb merged 10 commits into from
Apr 7, 2025
Merged

Enable NRT for AlarmOccurrence,Attachment, Attendee, CalendarDataType, Duration, ICalendarDataType, ICalendarParameterCollectionContainer #762

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7735978
Enable NRT for `AlarmOccurrence`
axunonb Apr 7, 2025
c659a72
Enable NRT for `Attachment`
axunonb Apr 7, 2025
5592cc9
Enable NRT for `Attendee`
axunonb Apr 7, 2025
d10d886
Enable NRT for `CalendarDataType`
axunonb Apr 7, 2025
d482732
Enable NRT for `Duration`
axunonb Apr 7, 2025
639b47f
Enable NRT for `ICalendarDataType`, `ICalendarParameterCollectionCont…
axunonb Apr 7, 2025
78f6218
Enable NRT for `IEncodableDataType`
axunonb Apr 7, 2025
80c8287
PeriodList.CopyFrom: Remove null check
axunonb Apr 7, 2025
4e0333c
Modifications from review
axunonb Apr 7, 2025
030f32f
Merge branch 'main' into axunonb/wip/pr/datatypes-nrt
axunonb Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Ical.Net.Tests/RecurrenceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1232,8 +1232,8 @@ public void WeekNoOrderingShouldNotMatter()
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"));

var recurringPeriods1 = rpe1.Evaluate(new CalDateTime(start), start, end, default).ToList();
var recurringPeriods2 = rpe2.Evaluate(new CalDateTime(start), start, end, default).ToList();
var recurringPeriods1 = rpe1.Evaluate(new CalDateTime(start), start, end, null).ToList();
Copy link
Collaborator

@minichma minichma Apr 7, 2025

Choose a reason for hiding this comment

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

Yes, better. I started with default because the options should have been a value type at first.

var recurringPeriods2 = rpe2.Evaluate(new CalDateTime(start), start, end, null).ToList();

Assert.That(recurringPeriods2, Has.Count.EqualTo(recurringPeriods1.Count));
}
Expand Down Expand Up @@ -3901,7 +3901,7 @@ public void TestDtStartTimezone(string? tzId)
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), default);
var occurrences = ev.Evaluate(evt.DtStart, evt.DtStart.ToTimeZone(tzId), evt.DtStart.AddMinutes(61).ToTimeZone(tzId), null);
var occurrencesStartTimes = occurrences.Select(x => x.StartTime).Take(2).ToList();

var expectedStartTimes = new[]
Expand Down
19 changes: 14 additions & 5 deletions Ical.Net/CalendarComponents/Alarm.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
/// Gets a list of alarm occurrences for the given recurring component, <paramref name="rc"/>
/// that occur between <paramref name="fromDate"/> and <paramref name="toDate"/>.
/// </summary>
public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, CalDateTime? fromDate, CalDateTime? toDate, EvaluationOptions options)
public virtual IList<AlarmOccurrence> GetOccurrences(IRecurringComponent rc, CalDateTime? fromDate, CalDateTime? toDate, EvaluationOptions? options)
{
if (Trigger == null)
{
Expand Down Expand Up @@ -141,10 +141,11 @@
/// since the provided <paramref name="start"/> date/time. If <paramref name="start"/>
/// is null, all triggered alarms will be returned.
/// </summary>
/// <param name="start">The earliest date/time to poll trigered alarms for.</param>
/// <param name="start">The earliest date/time to poll triggered alarms for.</param>
/// <param name="end"></param>
/// <param name="options"></param>
/// <returns>A list of <see cref="AlarmOccurrence"/> objects, each containing a triggered alarm.</returns>
public virtual IList<AlarmOccurrence> Poll(CalDateTime start, CalDateTime end, EvaluationOptions options = default)
public virtual IList<AlarmOccurrence> Poll(CalDateTime start, CalDateTime end, EvaluationOptions? options = null)
{
var results = new List<AlarmOccurrence>();

Expand All @@ -170,12 +171,20 @@
for (var i = 0; i < len; i++)
{
var ao = occurrences[i];
if (ao?.DateTime == null || ao.Component == null)
{
continue;
}

var alarmTime = ao.DateTime.Copy();

for (var j = 0; j < Repeat; j++)
{
alarmTime = alarmTime.Add(Duration);
occurrences.Add(new AlarmOccurrence(this, alarmTime.Copy(), ao.Component));
alarmTime = alarmTime?.Add(Duration);

Check warning on line 183 in Ical.Net/CalendarComponents/Alarm.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/CalendarComponents/Alarm.cs#L183 

Added line #L183 was not covered by tests
if (alarmTime != null)
{
occurrences.Add(new AlarmOccurrence(this, alarmTime.Copy(), ao.Component));

Check warning on line 186 in Ical.Net/CalendarComponents/Alarm.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/CalendarComponents/Alarm.cs#L186 

Added line #L186 was not covered by tests
}
}
}
}
Expand Down
24 changes: 16 additions & 8 deletions Ical.Net/DataTypes/AlarmOccurrence.cs
View file
Open in desktop
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 Ical.Net.CalendarComponents;

Expand All @@ -18,16 +19,16 @@
/// </remarks>
public class AlarmOccurrence : IComparable<AlarmOccurrence>
{
public Period Period { get; set; }
public Period? Period { get; set; }

public IRecurringComponent Component { get; set; }
public IRecurringComponent? Component { get; set; }

public Alarm Alarm { get; set; }
public Alarm? Alarm { get; set; }

public CalDateTime DateTime
public CalDateTime? DateTime
{
get => Period.StartTime;
set => Period = new Period(value);
get => Period?.StartTime;
set => Period = value != null ? new Period(value) : null;

Check warning on line 31 in Ical.Net/DataTypes/AlarmOccurrence.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/DataTypes/AlarmOccurrence.cs#L31 

Added line #L31 was not covered by tests
}

public AlarmOccurrence(AlarmOccurrence ao)
Expand All @@ -44,14 +45,21 @@
Component = rc;
}

public int CompareTo(AlarmOccurrence other) => Period.CompareTo(other.Period);
public int CompareTo(AlarmOccurrence? other)
Copy link

Copilot AI Apr 7, 2025

Choose a reason for hiding this comment

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

[nitpick] In the CompareTo method, returning 1 when other or its Period is null could result in inconsistent ordering; consider defining explicit ordering when both Period properties are null.

Copilot uses AI. Check for mistakes.
{
if (other == null || other.Period == null)
Copy link
Collaborator

@minichma minichma Apr 7, 2025

Choose a reason for hiding this comment

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

What if other.Period == null and this.Period == null?

{
return 1;

Check warning on line 52 in Ical.Net/DataTypes/AlarmOccurrence.cs

View check run for this annotation

Codecov / codecov/patch

Ical.Net/DataTypes/AlarmOccurrence.cs#L52 

Added line #L52 was not covered by tests
}
return Period?.CompareTo(other.Period) ?? 1;
}

protected bool Equals(AlarmOccurrence other)
=> Equals(Period, other.Period)
&& Equals(Component, other.Component)
&& Equals(Alarm, other.Alarm);

public override bool Equals(object obj)
public override bool Equals(object? obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
Expand Down
17 changes: 9 additions & 8 deletions Ical.Net/DataTypes/Attachment.cs
View file
Open in desktop
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.Linq;
using System.Text;
Expand All @@ -18,8 +19,8 @@ namespace Ical.Net.DataTypes;
/// </summary>
public class Attachment : EncodableDataType
{
public virtual Uri Uri { get; set; }
public virtual byte[] Data { get; private set; } // private set for CopyFrom
public virtual Uri? Uri { get; set; }
public virtual byte[]? Data { get; private set; } // private set for CopyFrom

private Encoding _valueEncoding = System.Text.Encoding.UTF8;
public virtual Encoding ValueEncoding
Expand All @@ -43,7 +44,7 @@ public virtual string FormatType

public Attachment() { }

public Attachment(byte[] value) : this()
public Attachment(byte[]? value) : this()
{
if (value != null)
{
Expand Down Expand Up @@ -93,15 +94,15 @@ public override void CopyFrom(ICopyable obj)
FormatType = att.FormatType;
}

protected bool Equals(Attachment other)
protected bool Equals(Attachment? other)
{
var firstPart = Equals(Uri, other.Uri) && ValueEncoding.Equals(other.ValueEncoding);
var firstPart = Equals(Uri, other?.Uri) && ValueEncoding.Equals(other?.ValueEncoding);
return Data == null
? firstPart
: firstPart && Data.SequenceEqual(other.Data);
: firstPart && other?.Data != null && Data.SequenceEqual(other.Data);
Copy link
Collaborator

@minichma minichma Apr 7, 2025

Choose a reason for hiding this comment

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

This comparison is not correct. If other?.Data == null, then this.Data is ignored, even if it contains data.

}

public override bool Equals(object obj)
public override bool Equals(object? obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
Expand All @@ -118,4 +119,4 @@ public override int GetHashCode()
return hashCode;
}
}
}
}
38 changes: 19 additions & 19 deletions Ical.Net/DataTypes/Attendee.cs
View file
Open in desktop
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.Linq;
Expand All @@ -12,9 +13,9 @@

public class Attendee : EncodableDataType
{
private Uri _sentBy;
private Uri? _sentBy;
/// <summary> SENT-BY, to indicate who is acting on behalf of the ATTENDEE </summary>
public virtual Uri SentBy
public virtual Uri? SentBy
{
get
{
Expand All @@ -38,7 +39,7 @@
}
}

private string _commonName;
private string? _commonName;
/// <summary> CN: to show the common or displayable name associated with the calendar address </summary>
public virtual string CommonName
{
Expand All @@ -48,7 +49,7 @@
{
_commonName = Parameters.Get("CN");
}
return _commonName;

Check warning on line 52 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference return.

Check warning on line 52 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference return.
}
set
{
Expand All @@ -61,9 +62,9 @@
}
}

private Uri _directoryEntry;
private Uri? _directoryEntry;
/// <summary> DIR, to indicate the URI that points to the directory information corresponding to the attendee </summary>
public virtual Uri DirectoryEntry
public virtual Uri? DirectoryEntry
{
get
{
Expand All @@ -87,7 +88,7 @@
}
}

private string _type;
private string? _type;
/// <summary> CUTYPE: the type of calendar user </summary>
public virtual string Type
{
Expand All @@ -97,7 +98,7 @@
{
_type = Parameters.Get("CUTYPE");
}
return _type;

Check warning on line 101 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference return.

Check warning on line 101 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference return.
}
set
{
Expand All @@ -111,7 +112,7 @@
}
}

private List<string> _members;
private List<string>? _members;
/// <summary> MEMBER: the groups the user belongs to </summary>
public virtual IList<string> Members
{
Expand All @@ -123,7 +124,7 @@
}
}

private string _role;
private string? _role;
/// <summary> ROLE: the intended role the attendee will have </summary>
public virtual string Role
{
Expand All @@ -133,7 +134,7 @@
{
_role = Parameters.Get("ROLE");
}
return _role;

Check warning on line 137 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference return.

Check warning on line 137 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference return.
}
set
{
Expand All @@ -146,7 +147,7 @@
}
}

private string _participationStatus;
private string? _participationStatus;
public virtual string ParticipationStatus
{
get
Expand All @@ -155,7 +156,7 @@
{
_participationStatus = Parameters.Get(EventParticipationStatus.Key);
}
return _participationStatus;

Check warning on line 159 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / coverage 

Possible null reference return.

Check warning on line 159 in Ical.Net/DataTypes/Attendee.cs

View workflow job for this annotation

GitHub Actions / tests 

Possible null reference return.
}
set
{
Expand All @@ -179,9 +180,8 @@
return _rsvp.Value;
}

bool val;
var rsvp = Parameters.Get("RSVP");
if (rsvp != null && bool.TryParse(rsvp, out val))
if (rsvp != null && bool.TryParse(rsvp, out var val))
{
_rsvp = val;
return _rsvp.Value;
Expand All @@ -196,9 +196,9 @@
}
}

private List<string> _delegatedTo;
private List<string>? _delegatedTo;
/// <summary> DELEGATED-TO, to indicate the calendar users that the original request was delegated to </summary>
public virtual IList<string> DelegatedTo
public virtual IList<string>? DelegatedTo
{
get => _delegatedTo ?? (_delegatedTo = new List<string>(Parameters.GetMany("DELEGATED-TO")));
set
Expand All @@ -212,7 +212,7 @@
}
}

private List<string> _delegatedFrom;
private List<string>? _delegatedFrom;
/// <summary> DELEGATED-FROM, to indicate whom the request was delegated from </summary>
public virtual IList<string> DelegatedFrom
{
Expand All @@ -229,7 +229,7 @@
}

/// <summary> Uri associated with the attendee, typically an email address </summary>
public virtual Uri Value { get; set; }
public virtual Uri? Value { get; set; }

public Attendee() { }

Expand Down Expand Up @@ -276,10 +276,10 @@
&& Rsvp == other.Rsvp
&& Equals(Value, other.Value)
&& Members.SequenceEqual(other.Members)
&& DelegatedTo.SequenceEqual(other.DelegatedTo)
&& DelegatedFrom.SequenceEqual(other.DelegatedFrom);
&& (DelegatedTo?.SequenceEqual(other.DelegatedTo ?? Enumerable.Empty<string>()) ?? other.DelegatedTo == null)
Copy link
Collaborator

@minichma minichma Apr 7, 2025

Choose a reason for hiding this comment

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

This comparison is not commutative this way. I.e. if one is null and the other is empty, then the comparison is not symmetrical. Not sure though, whether an empty sequence should equal null. Usually they're considered different.

&& (DelegatedFrom?.SequenceEqual(other.DelegatedFrom ?? Enumerable.Empty<string>()) ?? other.DelegatedFrom == null);

public override bool Equals(object obj)
public override bool Equals(object? obj)
{
if (ReferenceEquals(null, obj)) return false;
if (ReferenceEquals(this, obj)) return true;
Expand All @@ -305,4 +305,4 @@
return hashCode;
}
}
}
}
24 changes: 13 additions & 11 deletions Ical.Net/DataTypes/CalendarDataType.cs
View file
Open in desktop
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.Runtime.Serialization;
using Ical.Net.Proxies;
Expand All @@ -14,10 +15,11 @@ namespace Ical.Net.DataTypes;
/// </summary>
public abstract class CalendarDataType : ICalendarDataType
{
private IParameterCollection _parameters;
private ParameterCollectionProxy _proxy;
// Well be set with Initialize()
private IParameterCollection _parameters = null!;
private ParameterCollectionProxy _proxy = null!;

private ICalendarObject _associatedObject;
private ICalendarObject? _associatedObject;

protected CalendarDataType()
{
Expand Down Expand Up @@ -49,10 +51,10 @@ protected virtual void OnDeserializing(StreamingContext context)

protected virtual void OnDeserialized(StreamingContext context) { }

public virtual Type GetValueType()
public virtual Type? GetValueType()
{
// See RFC 5545 Section 3.2.20.
if (_proxy != null && _proxy.ContainsKey("VALUE"))
if (_proxy.ContainsKey("VALUE"))
{
switch (_proxy.Get("VALUE"))
{
Expand Down Expand Up @@ -92,12 +94,12 @@ public virtual Type GetValueType()
return null;
}

public virtual void SetValueType(string type)
public virtual void SetValueType(string? type)
{
_proxy?.Set("VALUE", type?.ToUpper());
_proxy.Set("VALUE", type?.ToUpper());
}

public virtual ICalendarObject AssociatedObject
public virtual ICalendarObject? AssociatedObject
{
get => _associatedObject;
set
Expand All @@ -124,9 +126,9 @@ public virtual ICalendarObject AssociatedObject
}
}

public virtual Calendar Calendar => _associatedObject?.Calendar;
public virtual Calendar? Calendar => _associatedObject?.Calendar;

public virtual string Language
public virtual string? Language
{
get => Parameters.Get("LANGUAGE");
set => Parameters.Set("LANGUAGE", value);
Expand All @@ -149,7 +151,7 @@ public virtual void CopyFrom(ICopyable obj)
/// Creates a deep copy of the <see cref="T"/> object.
/// </summary>
/// <returns>The copy of the <see cref="T"/> object.</returns>
public virtual T Copy<T>()
public virtual T? Copy<T>()
{
var type = GetType();
var obj = Activator.CreateInstance(type, true) as ICopyable;
Expand Down
Loading
Loading