Skip to content

Commit 87d2500

Browse files
Small fix for recurring time window testcase (#522)
* add comments & correct testcase * update * update * update * update * update
1 parent 71aa5f2 commit 87d2500

File tree

3 files changed

+60
-26
lines changed

3 files changed

+60
-26
lines changed

src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceEvaluator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ private static int CalculateWeeklyDayOffset(DayOfWeek day1, DayOfWeek day2)
347347
/// </summary>
348348
private static List<DayOfWeek> SortDaysOfWeek(IEnumerable<DayOfWeek> daysOfWeek, DayOfWeek firstDayOfWeek)
349349
{
350-
List<DayOfWeek> result = daysOfWeek.ToList();
350+
List<DayOfWeek> result = daysOfWeek.Distinct().ToList(); // dedup
351351

352352
result.Sort((x, y) =>
353353
CalculateWeeklyDayOffset(x, firstDayOfWeek)

src/Microsoft.FeatureManagement/FeatureFilters/Recurrence/RecurrenceValidator.cs

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -341,49 +341,40 @@ private static bool TryValidateNumberOfOccurrences(TimeWindowFilterSettings sett
341341
private static bool IsDurationCompliantWithDaysOfWeek(TimeSpan duration, int interval, IEnumerable<DayOfWeek> daysOfWeek, DayOfWeek firstDayOfWeek)
342342
{
343343
Debug.Assert(interval > 0);
344+
Debug.Assert(daysOfWeek.Count() > 0);
344345

345346
if (daysOfWeek.Count() == 1)
346347
{
347348
return true;
348349
}
349350

350-
DateTime firstDayOfThisWeek = DateTime.Today.AddDays(
351-
DaysPerWeek - CalculateWeeklyDayOffset(DateTime.Today.DayOfWeek, firstDayOfWeek));
352-
353351
List<DayOfWeek> sortedDaysOfWeek = SortDaysOfWeek(daysOfWeek, firstDayOfWeek);
354352

355-
DateTime prev = DateTime.MinValue;
353+
DayOfWeek firstDay = sortedDaysOfWeek.First(); // the closest occurrence day to the first day of week
354+
355+
DayOfWeek prev = firstDay;
356356

357357
TimeSpan minGap = TimeSpan.FromDays(DaysPerWeek);
358358

359-
foreach (DayOfWeek dayOfWeek in sortedDaysOfWeek)
359+
for (int i = 1; i < sortedDaysOfWeek.Count(); i++) // start from the second day to calculate the gap
360360
{
361-
DateTime date = firstDayOfThisWeek.AddDays(
362-
CalculateWeeklyDayOffset(dayOfWeek, firstDayOfWeek));
361+
DayOfWeek dayOfWeek = sortedDaysOfWeek[i];
363362

364-
if (prev != DateTime.MinValue)
365-
{
366-
TimeSpan gap = date - prev;
363+
TimeSpan gap = TimeSpan.FromDays(CalculateWeeklyDayOffset(dayOfWeek, prev));
367364

368-
if (gap < minGap)
369-
{
370-
minGap = gap;
371-
}
365+
if (gap < minGap)
366+
{
367+
minGap = gap;
372368
}
373369

374-
prev = date;
370+
prev = dayOfWeek;
375371
}
376372

377373
//
378374
// It may across weeks. Check the next week if the interval is one week.
379375
if (interval == 1)
380376
{
381-
DateTime firstDayOfNextWeek = firstDayOfThisWeek.AddDays(DaysPerWeek);
382-
383-
DateTime firstOccurrenceInNextWeek = firstDayOfNextWeek.AddDays(
384-
CalculateWeeklyDayOffset(sortedDaysOfWeek.First(), firstDayOfWeek));
385-
386-
TimeSpan gap = firstOccurrenceInNextWeek - prev;
377+
TimeSpan gap = TimeSpan.FromDays(CalculateWeeklyDayOffset(firstDay, prev));
387378

388379
if (gap < minGap)
389380
{
@@ -413,7 +404,7 @@ private static int CalculateWeeklyDayOffset(DayOfWeek day1, DayOfWeek day2)
413404
/// </summary>
414405
private static List<DayOfWeek> SortDaysOfWeek(IEnumerable<DayOfWeek> daysOfWeek, DayOfWeek firstDayOfWeek)
415406
{
416-
List<DayOfWeek> result = daysOfWeek.ToList();
407+
List<DayOfWeek> result = daysOfWeek.Distinct().ToList(); // dedup
417408

418409
result.Sort((x, y) =>
419410
CalculateWeeklyDayOffset(x, firstDayOfWeek)

tests/Tests.FeatureManagement/RecurrenceEvaluation.cs

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ public void InvalidTimeWindowAcrossWeeksTest()
291291
{
292292
Type = RecurrencePatternType.Weekly,
293293
Interval = 1,
294+
FirstDayOfWeek = DayOfWeek.Sunday,
294295
DaysOfWeek = new List<DayOfWeek>() { 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.
295296
},
296297
Range = new RecurrenceRange()
@@ -299,7 +300,7 @@ public void InvalidTimeWindowAcrossWeeksTest()
299300

300301
//
301302
// The settings is valid. No exception should be thrown.
302-
RecurrenceEvaluator.IsMatch(DateTimeOffset.UtcNow, settings);
303+
Assert.True(RecurrenceValidator.TryValidateSettings(settings, out string paramName, out string errorMessage));
303304

304305
settings = new TimeWindowFilterSettings()
305306
{
@@ -320,7 +321,49 @@ public void InvalidTimeWindowAcrossWeeksTest()
320321

321322
//
322323
// The settings is valid. No exception should be thrown.
323-
RecurrenceEvaluator.IsMatch(DateTimeOffset.UtcNow, settings);
324+
Assert.True(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage));
325+
326+
settings = new TimeWindowFilterSettings()
327+
{
328+
Start = DateTimeOffset.Parse("2024-1-15T00:00:00+08:00"), // Monday
329+
End = DateTimeOffset.Parse("2024-1-17T00:00:00+08:00"), // Time window duration is 2 days.
330+
Recurrence = new Recurrence()
331+
{
332+
Pattern = new RecurrencePattern()
333+
{
334+
Type = RecurrencePatternType.Weekly,
335+
Interval = 1,
336+
FirstDayOfWeek = DayOfWeek.Sunday,
337+
DaysOfWeek = new List<DayOfWeek>() { DayOfWeek.Monday, DayOfWeek.Saturday }
338+
},
339+
Range = new RecurrenceRange()
340+
}
341+
};
342+
343+
//
344+
// The settings is valid. No exception should be thrown.
345+
Assert.True(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage));
346+
347+
settings = new TimeWindowFilterSettings()
348+
{
349+
Start = DateTimeOffset.Parse("2024-1-15T00:00:00+08:00"), // Monday
350+
End = DateTimeOffset.Parse("2024-1-17T00:00:01+08:00"), // Time window duration is more than 2 days.
351+
Recurrence = new Recurrence()
352+
{
353+
Pattern = new RecurrencePattern()
354+
{
355+
Type = RecurrencePatternType.Weekly,
356+
Interval = 1,
357+
FirstDayOfWeek = DayOfWeek.Sunday,
358+
DaysOfWeek = new List<DayOfWeek>() { DayOfWeek.Monday, DayOfWeek.Saturday }
359+
},
360+
Range = new RecurrenceRange()
361+
}
362+
};
363+
364+
Assert.False(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage));
365+
Assert.Equal(ParamName.End, paramName);
366+
Assert.Equal(ErrorMessage.TimeWindowDurationOutOfRange, errorMessage);
324367

325368
settings = new TimeWindowFilterSettings()
326369
{
@@ -339,7 +382,7 @@ public void InvalidTimeWindowAcrossWeeksTest()
339382
}
340383
};
341384

342-
Assert.False(RecurrenceValidator.TryValidateSettings(settings, out string paramName, out string errorMessage));
385+
Assert.False(RecurrenceValidator.TryValidateSettings(settings, out paramName, out errorMessage));
343386
Assert.Equal(ParamName.End, paramName);
344387
Assert.Equal(ErrorMessage.TimeWindowDurationOutOfRange, errorMessage);
345388
}

0 commit comments

Comments
 (0)