Refactor handling of FREQ in recurrence pattern (#789)

* Resolves #788

* Update xmldoc for RecurrencePattern.Interval

* Add range check for the setter of RecurrencePattern.Frequency

Author: axunonb

Ical.Net.Tests/RecurrenceTests.cs

@@ -3010,20 +3010,18 @@ public void GetOccurrences1()
     }
 
     [Test, Category("Recurrence")]
-    public void Test1()
+    public void TryingToSetInvalidFrequency_ShouldThrow()
     {
-        var cal = new Calendar();
-        var evt = cal.Create<CalendarEvent>();
-        evt.Summary = "Event summary";
-        evt.Start = new CalDateTime(DateTime.SpecifyKind(DateTime.Today, DateTimeKind.Utc));
-
-        var recur = new RecurrencePattern();
-        evt.RecurrenceRules.Add(recur);
-
-        Assert.That(() =>
+        Assert.Multiple(() =>
         {
-            _ = evt.GetOccurrences(CalDateTime.Today.AddDays(1)).TakeUntil(CalDateTime.Today.AddDays(2));
-        }, Throws.Exception, "An exception should be thrown when evaluating a recurrence with no specified FREQUENCY");
+            // Using the constructor
+            Assert.That(() => _ = new RecurrencePattern((FrequencyType) int.MaxValue, 1),
+                Throws.TypeOf<ArgumentOutOfRangeException>());
+
+            // Using the property
+            Assert.That(() => _ = new RecurrencePattern {Frequency = (FrequencyType) 9876543 },
+                Throws.TypeOf<ArgumentOutOfRangeException>());
+        });
     }
 
     [Test, Category("Recurrence")]
@@ -4070,11 +4068,11 @@ public void Recurrence_RRULE_Without_Freq_Should_Throw()
     }
 
     [Test]
-    public void Recurrence_RRULE_With_Freq_None_Should_Throw()
+    public void Recurrence_RRULE_With_Freq_Undefined_Should_Throw()
     {
         var serializer = new RecurrencePatternSerializer();
 
-        Assert.That(() => serializer.Deserialize(new StringReader("FREQ=NONE;INTERVAL=2;UNTIL=20250430T000000Z")), Throws.TypeOf<ArgumentOutOfRangeException>());
+        Assert.That(() => serializer.Deserialize(new StringReader("FREQ=UNDEFINED;INTERVAL=2;UNTIL=20250430T000000Z")), Throws.TypeOf<ArgumentOutOfRangeException>());
     }
 
     [Test]

Ical.Net/Constants.cs

@@ -176,7 +176,6 @@ public enum FreeBusyStatus
 
 public enum FrequencyType
 {
-    None,
     Secondly,
     Minutely,
     Hourly,

Ical.Net/DataTypes/RecurrencePattern.cs

@@ -7,7 +7,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using Ical.Net.Evaluation;
 using Ical.Net.Serialization.DataTypes;
 using Ical.Net.Utility;
 
@@ -20,10 +19,30 @@ namespace Ical.Net.DataTypes;
 public class RecurrencePattern : EncodableDataType
 {
     private int? _interval;
+    private FrequencyType _frequency;
     private CalDateTime? _until;
 
-    public FrequencyType Frequency { get; set; }
+    /// <summary>
+    /// Specifies the frequency <i>FREQ</i> of the recurrence.
+    /// The default value is <see cref="FrequencyType.Yearly"/>.
+    /// </summary>
+    public FrequencyType Frequency
+    {
+        get => _frequency;
+        set
+        {
+            if (!Enum.IsDefined(typeof(FrequencyType), value))
+            {
+                throw new ArgumentOutOfRangeException(nameof(Frequency), $"Invalid FrequencyType '{value}'.");
+            }
+            _frequency = value;
+        }
+    }
 
+    /// <summary>
+    /// Specifies the end date of the recurrence (optional).
+    /// This property <b>must be null</b> if the <see cref="Count"/> property is set.
+    /// </summary>
     public CalDateTime? Until
     {
         get => _until;
@@ -37,13 +56,21 @@ public CalDateTime? Until
         }
     }
 
+    /// <summary>
+    /// Specifies the number of occurrences of the recurrence (optional).
+    /// This property <b>must be null</b> if the <see cref="Until"/> property is set.
+    /// </summary>
     public int? Count { get; set; }
 
+
     /// <summary>
-    /// Specifies how often the recurrence should repeat.
-    /// - 1 = every
-    /// - 2 = every second
-    /// - 3 = every third
+    /// The INTERVAL rule part contains a positive integer representing at
+    /// which intervals the recurrence rule repeats. The default value is
+    /// 1, meaning every second for a SECONDLY rule, every minute for a
+    /// MINUTELY rule, every hour for an HOURLY rule, every day for a
+    /// DAILY rule, every week for a WEEKLY rule, every month for a
+    /// MONTHLY rule, and every year for a YEARLY rule. For example,
+    /// within a DAILY rule, a value of 8 means every eight days.
     /// </summary>
     public int Interval
     {
@@ -88,14 +115,21 @@ public int Interval
 
     public DayOfWeek FirstDayOfWeek { get; set; } = DayOfWeek.Monday;
 
+    /// <summary>
+    /// Default constructor. Sets the <see cref="Frequency"/> to <see cref="FrequencyType.Yearly"/>
+    /// and <see cref="Interval"/> to 1.
+    /// </summary>
     public RecurrencePattern()
-    { }
+    {
+        Frequency = FrequencyType.Yearly;
+        Interval = 1;
+    }
 
     public RecurrencePattern(FrequencyType frequency) : this(frequency, 1) { }
 
     public RecurrencePattern(FrequencyType frequency, int interval) : this()
     {
-        Frequency = frequency;
+        Frequency = frequency; // for proper validation don't use the backing field
         Interval = interval;
     }
 

Ical.Net/Evaluation/Evaluator.cs

@@ -43,9 +43,10 @@ protected void IncrementDate(ref CalDateTime dt, RecurrencePattern pattern, int
                 case FrequencyType.Yearly:
                     dt = old.AddDays(-old.DayOfYear + 1).AddYears(interval);
                     break;
-                // FIXME: use a more specific exception.
                 default:
-                    throw new Exception("FrequencyType.NONE cannot be evaluated. Please specify a FrequencyType before evaluating the recurrence.");
+                    // Frequency should always be valid at this stage.
+                    System.Diagnostics.Debug.Fail($"'{pattern.Frequency}' as RecurrencePattern.Frequency is not implemented.");
+                    break;
             }
         }
         catch (ArgumentOutOfRangeException)

Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

@@ -759,7 +759,7 @@ private static Period CreatePeriod(CalDateTime dateTime, CalDateTime referenceDa
     /// <returns></returns>
     public override IEnumerable<Period> Evaluate(CalDateTime referenceDate, CalDateTime? periodStart, EvaluationOptions? options)
     {
-        if (Pattern.Frequency != FrequencyType.None && Pattern.Frequency < FrequencyType.Daily && !referenceDate.HasTime)
+        if (Pattern.Frequency < FrequencyType.Daily && !referenceDate.HasTime)
         {
             // This case is not defined by RFC 5545. We handle it by evaluating the rule
             // as if referenceDate had a time (i.e. set to midnight).

Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs

@@ -112,12 +112,11 @@ private static void SerializeByValue(List<string> aggregate, IList<int> byValue,
 
         // Push the recurrence pattern onto the serialization stack
         SerializationContext.Push(recur);
-        var values = new List<string>()
+        var values = new List<string>
         {
-            $"FREQ={Enum.GetName(typeof(FrequencyType), recur.Frequency)?.ToUpper()}"
+            $"FREQ={recur.Frequency.ToString().ToUpper()}"
         };
 
-
         //-- FROM RFC2445 --
         //The INTERVAL rule part contains a positive integer representing how
         //often the recurrence rule repeats. The default value is "1", meaning
@@ -243,26 +242,21 @@ private void DeserializePattern(string value, RecurrencePattern r, ISerializerFa
             ProcessKeyValuePair(keyValues[0].ToLower(), keyValues[1], r, factory);
         }
 
-        if (!freqPartExists || r.Frequency == FrequencyType.None)
+        if (!freqPartExists)
         {
             throw new ArgumentOutOfRangeException(nameof(value),
-                "The recurrence rule must specify a FREQ part that is not NONE.");
+                "The recurrence rule must specify a valid FREQ part.");
         }
         CheckMutuallyExclusive("COUNT", "UNTIL", r.Count, r.Until);
         CheckRanges(r);
     }
 
     private void ProcessKeyValuePair(string key, string value, RecurrencePattern r, ISerializerFactory factory)
     {
-        if (SerializationContext == null)
-        {
-            throw new InvalidOperationException("SerializationContext is not set.");
-        }
-
         switch (key)
         {
-            case "freq":
-                r.Frequency = (FrequencyType) Enum.Parse(typeof(FrequencyType), value, true);
+            case "freq" when Enum.TryParse(value, true, out FrequencyType freq):
+                r.Frequency = freq;
                 break;
 
             case "until":
@@ -320,7 +314,7 @@ private void ProcessKeyValuePair(string key, string value, RecurrencePattern r,
 
             default:
                 throw new ArgumentOutOfRangeException(nameof(key),
-                    $"The recurrence rule part '{key}' is not supported.");
+                    $"The recurrence rule part '{key}' or its value {value} is not supported.");
         }
     }