diff --git a/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs b/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs index 5e8b6872..200910ca 100644 --- a/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs +++ b/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs @@ -347,7 +347,7 @@ private static int CalculateWeeklyDayOffset(DayOfWeek day1, DayOfWeek day2) /// private static List SortDaysOfWeek(IEnumerable daysOfWeek, DayOfWeek firstDayOfWeek) { - List result = daysOfWeek.ToList(); + List result = daysOfWeek.Distinct().ToList(); // dedup result.Sort((x, y) => CalculateWeeklyDayOffset(x, firstDayOfWeek) diff --git a/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceValidator.cs b/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceValidator.cs index 230ae674..54b31e16 100644 --- a/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceValidator.cs +++ b/src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceValidator.cs @@ -341,49 +341,40 @@ private static bool TryValidateNumberOfOccurrences(TimeWindowFilterSettings sett private static bool IsDurationCompliantWithDaysOfWeek(TimeSpan duration, int interval, IEnumerable daysOfWeek, DayOfWeek firstDayOfWeek) { Debug.Assert(interval > 0); + Debug.Assert(daysOfWeek.Count() > 0); if (daysOfWeek.Count() == 1) { return true; } - DateTime firstDayOfThisWeek = DateTime.Today.AddDays( - DaysPerWeek - CalculateWeeklyDayOffset(DateTime.Today.DayOfWeek, firstDayOfWeek)); - List sortedDaysOfWeek = SortDaysOfWeek(daysOfWeek, firstDayOfWeek); - DateTime prev = DateTime.MinValue; + DayOfWeek firstDay = sortedDaysOfWeek.First(); // the closest occurrence day to the first day of week + + DayOfWeek prev = firstDay; TimeSpan minGap = TimeSpan.FromDays(DaysPerWeek); - foreach (DayOfWeek dayOfWeek in sortedDaysOfWeek) + for (int i = 1; i < sortedDaysOfWeek.Count(); i++) // start from the second day to calculate the gap { - DateTime date = firstDayOfThisWeek.AddDays( - CalculateWeeklyDayOffset(dayOfWeek, firstDayOfWeek)); + DayOfWeek dayOfWeek = sortedDaysOfWeek[i]; - if (prev != DateTime.MinValue) - { - TimeSpan gap = date - prev; + TimeSpan gap = TimeSpan.FromDays(CalculateWeeklyDayOffset(dayOfWeek, prev)); - if (gap < minGap) - { - minGap = gap; - } + if (gap < minGap) + { + minGap = gap; } - prev = date; + prev = dayOfWeek; } // // It may across weeks. Check the next week if the interval is one week. if (interval == 1) { - DateTime firstDayOfNextWeek = firstDayOfThisWeek.AddDays(DaysPerWeek); - - DateTime firstOccurrenceInNextWeek = firstDayOfNextWeek.AddDays( - CalculateWeeklyDayOffset(sortedDaysOfWeek.First(), firstDayOfWeek)); - - TimeSpan gap = firstOccurrenceInNextWeek - prev; + TimeSpan gap = TimeSpan.FromDays(CalculateWeeklyDayOffset(firstDay, prev)); if (gap < minGap) { @@ -413,7 +404,7 @@ private static int CalculateWeeklyDayOffset(DayOfWeek day1, DayOfWeek day2) /// private static List SortDaysOfWeek(IEnumerable daysOfWeek, DayOfWeek firstDayOfWeek) { - List result = daysOfWeek.ToList(); + List result = daysOfWeek.Distinct().ToList(); // dedup result.Sort((x, y) => CalculateWeeklyDayOffset(x, firstDayOfWeek) diff --git a/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs b/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs index 4519456e..498abf8a 100644 --- a/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs +++ b/tests/Tests.FeatureManagement/RecurrenceEvaluation.cs @@ -291,6 +291,7 @@ public void InvalidTimeWindowAcrossWeeksTest() { Type = RecurrencePatternType.Weekly, Interval = 1, + FirstDayOfWeek = DayOfWeek.Sunday, DaysOfWeek = new List() { DayOfWeek.Tuesday, DayOfWeek.Saturday } // The time window duration should be shorter than 3 days because the gap between Saturday in the previous week and Tuesday in this week is 3 days. }, Range = new RecurrenceRange() @@ -299,7 +300,7 @@ public void InvalidTimeWindowAcrossWeeksTest() // // The settings is valid. No exception should be thrown. - RecurrenceEvaluator.IsMatch(DateTimeOffset.UtcNow, settings); + Assert.True(RecurrenceValidator.TryValidateSettings(settings, out string paramName, out string errorMessage)); settings = new TimeWindowFilterSettings() { @@ -320,7 +321,49 @@ public void InvalidTimeWindowAcrossWeeksTest() // // The settings is valid. No exception should be thrown. - RecurrenceEvaluator.IsMatch(DateTimeOffset.UtcNow, settings); + Assert.True(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage)); + + settings = new TimeWindowFilterSettings() + { + Start = DateTimeOffset.Parse("2024-1-15T00:00:00+08:00"), // Monday + End = DateTimeOffset.Parse("2024-1-17T00:00:00+08:00"), // Time window duration is 2 days. + Recurrence = new Recurrence() + { + Pattern = new RecurrencePattern() + { + Type = RecurrencePatternType.Weekly, + Interval = 1, + FirstDayOfWeek = DayOfWeek.Sunday, + DaysOfWeek = new List() { DayOfWeek.Monday, DayOfWeek.Saturday } + }, + Range = new RecurrenceRange() + } + }; + + // + // The settings is valid. No exception should be thrown. + Assert.True(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage)); + + settings = new TimeWindowFilterSettings() + { + Start = DateTimeOffset.Parse("2024-1-15T00:00:00+08:00"), // Monday + End = DateTimeOffset.Parse("2024-1-17T00:00:01+08:00"), // Time window duration is more than 2 days. + Recurrence = new Recurrence() + { + Pattern = new RecurrencePattern() + { + Type = RecurrencePatternType.Weekly, + Interval = 1, + FirstDayOfWeek = DayOfWeek.Sunday, + DaysOfWeek = new List() { DayOfWeek.Monday, DayOfWeek.Saturday } + }, + Range = new RecurrenceRange() + } + }; + + Assert.False(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage)); + Assert.Equal(ParamName.End, paramName); + Assert.Equal(ErrorMessage.TimeWindowDurationOutOfRange, errorMessage); settings = new TimeWindowFilterSettings() { @@ -339,7 +382,7 @@ public void InvalidTimeWindowAcrossWeeksTest() } }; - Assert.False(RecurrenceValidator.TryValidateSettings(settings, out string paramName, out string errorMessage)); + Assert.False(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage)); Assert.Equal(ParamName.End, paramName); Assert.Equal(ErrorMessage.TimeWindowDurationOutOfRange, errorMessage); }