Code review items

Author: mrm9084

featuremanagement/_defaultfilters.py

@@ -71,9 +71,6 @@ def evaluate(self, context: Mapping[Any, Any], **kwargs: Any) -> bool:
 
         current_time = datetime.now(timezone.utc)
 
-        if recurrence_data:
-            recurrence = Recurrence(recurrence_data)
-
         if not start and not end:
             logger.warning(
                 TIME_WINDOW_FILTER_INVALID,
@@ -87,24 +84,19 @@ def evaluate(self, context: Mapping[Any, Any], **kwargs: Any) -> bool:
         start_time: Optional[datetime] = parsedate_to_datetime(start) if start else None
         end_time: Optional[datetime] = parsedate_to_datetime(end) if end else None
 
-        if recurrence:
-            if start_time and end_time:
-                settings = TimeWindowFilterSettings(start_time, end_time, recurrence)
-                return is_match(settings, current_time)
-            logger.warning(
-                TIME_WINDOW_FILTER_INVALID_RECURRENCE,
-                TimeWindowFilter.__name__,
-                context.get(FEATURE_FLAG_NAME_KEY),
-                START_KEY,
-                END_KEY,
-            )
-            return False
-
         if not start and not end:
             logging.warning("%s: At least one of Start or End is required.", TimeWindowFilter.__name__)
             return False
 
-        return (start_time is None or start_time <= current_time) and (end_time is None or current_time < end_time)
+        if (start_time is None or start_time <= current_time) and (end_time is None or current_time < end_time):
+            return True
+
+        if recurrence_data:
+            recurrence = Recurrence(recurrence_data)
+            settings = TimeWindowFilterSettings(start_time, end_time, recurrence)
+            return is_match(settings, current_time)
+
+        return False
 
 
 @FeatureFilter.alias("Microsoft.Targeting")

featuremanagement/_time_window_filter/_models.py

@@ -3,6 +3,7 @@
 # Licensed under the MIT License. See License.txt in the project root for
 # license information.
 # -------------------------------------------------------------------------
+import math
 from enum import Enum
 from typing import Dict, Any, Optional
 from datetime import datetime
@@ -68,7 +69,8 @@ class RecurrencePattern:  # pylint: disable=too-few-public-methods
     The recurrence pattern settings.
     """
 
-    days: list = ["Sunday", "Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday"]
+    days: list = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"]
+    days_of_week: list = []
 
     def __init__(self, pattern_data: Dict[str, Any]):
         self.type = RecurrencePatternType.from_str(pattern_data.get("Type", "Daily"))
@@ -78,21 +80,23 @@ def __init__(self, pattern_data: Dict[str, Any]):
         days_of_week = pattern_data.get("DaysOfWeek", [])
         for day in days_of_week:
             self.days_of_week.append(self.days.index(day))
-        first_day_of_week = pattern_data.get("FirstDayOfWeek", "Sunday")
-        self.first_day_of_week = self.days.index(first_day_of_week) if first_day_of_week in self.days else 0
+        self.first_day_of_week = self.days.index(pattern_data.get("FirstDayOfWeek", "Sunday"))
 
 
 class RecurrenceRange:  # pylint: disable=too-few-public-methods
     """
     The recurrence range settings.
     """
 
+    type: RecurrenceRangeType
+    end_date: Optional[datetime] = None
+
     def __init__(self, range_data: Dict[str, Any]):
         self.type = RecurrenceRangeType.from_str(range_data.get("Type", "NoEnd"))
         if range_data.get("EndDate") and isinstance(range_data.get("EndDate"), str):
             end_date_str = range_data.get("EndDate", "")
             self.end_date = parsedate_to_datetime(end_date_str) if end_date_str else None
-        self.num_of_occurrences = range_data.get("NumberOfOccurrences", 0)
+        self.num_of_occurrences = range_data.get("NumberOfOccurrences", math.pow(2, 63) - 1)
         if self.num_of_occurrences < 0:
             raise ValueError("The number of occurrences must be greater than or equal to 0.")
 

featuremanagement/_time_window_filter/_recurrence_evaluator.py

@@ -8,6 +8,7 @@
 from ._models import RecurrencePatternType, RecurrenceRangeType, TimeWindowFilterSettings, OccurrenceInfo
 from ._recurrence_validator import validate_settings
 
+DAYS_PER_WEEK = 7
 
 def is_match(settings: TimeWindowFilterSettings, now: datetime) -> bool:
     """
@@ -36,8 +37,10 @@ def _get_previous_occurrence(settings: TimeWindowFilterSettings, now: datetime)
     pattern_type = settings.recurrence.pattern.type
     if pattern_type == RecurrencePatternType.DAILY:
         occurrence_info = _get_daily_previous_occurrence(settings, now)
-    else:
+    elif pattern_type == RecurrencePatternType.WEEKLY:
         occurrence_info = _get_weekly_previous_occurrence(settings, now)
+    else:
+        raise ValueError(f"Invalid recurrence pattern type: %s", pattern_type)
 
     recurrence_range = settings.recurrence.range
     range_type = recurrence_range.type
@@ -74,16 +77,16 @@ def _get_weekly_previous_occurrence(settings: TimeWindowFilterSettings, now: dat
     start = settings.start
     first_day_of_first_week = start - timedelta(days=_get_passed_week_days(start.weekday(), pattern.first_day_of_week))
 
-    number_of_interval = (now - first_day_of_first_week).days // (interval * 7)
+    number_of_interval = (now - first_day_of_first_week).days // (interval * DAYS_PER_WEEK)
     first_day_of_most_recent_occurring_week = first_day_of_first_week + timedelta(
-        days=number_of_interval * (interval * 7)
+        days=number_of_interval * (interval * DAYS_PER_WEEK)
     )
     sorted_days_of_week = _sort_days_of_week(pattern.days_of_week, pattern.first_day_of_week)
     max_day_offset = _get_passed_week_days(sorted_days_of_week[-1], pattern.first_day_of_week)
     min_day_offset = _get_passed_week_days(sorted_days_of_week[0], pattern.first_day_of_week)
     num_of_occurrences = number_of_interval * len(sorted_days_of_week) - sorted_days_of_week.index(start.weekday())
 
-    if now > first_day_of_most_recent_occurring_week + timedelta(days=7):
+    if now > first_day_of_most_recent_occurring_week + timedelta(days=DAYS_PER_WEEK):
         num_of_occurrences += len(sorted_days_of_week)
         most_recent_occurrence = first_day_of_most_recent_occurring_week + timedelta(days=max_day_offset)
         return OccurrenceInfo(most_recent_occurrence, num_of_occurrences)
@@ -94,7 +97,7 @@ def _get_weekly_previous_occurrence(settings: TimeWindowFilterSettings, now: dat
         day_with_min_offset = start
     if now < day_with_min_offset:
         most_recent_occurrence = (
-            first_day_of_most_recent_occurring_week - timedelta(days=interval * 7) + timedelta(days=max_day_offset)
+            first_day_of_most_recent_occurring_week - timedelta(days=interval * DAYS_PER_WEEK) + timedelta(days=max_day_offset)
         )
     else:
         most_recent_occurrence = day_with_min_offset
@@ -113,7 +116,14 @@ def _get_weekly_previous_occurrence(settings: TimeWindowFilterSettings, now: dat
 
 
 def _get_passed_week_days(current_day: int, first_day_of_week: int) -> int:
-    return (current_day - first_day_of_week + 7) % 7
+    """
+    Get the number of days passed since the first day of the week.
+    :param int current_day: The current day of the week (0-6).
+    :param int first_day_of_week: The first day of the week (0-6).
+    :return: The number of days passed since the first day of the week.
+    :rtype: int
+    """
+    return (current_day - first_day_of_week + DAYS_PER_WEEK) % DAYS_PER_WEEK
 
 
 def _sort_days_of_week(days_of_week: List[int], first_day_of_week: int) -> List[int]:

featuremanagement/_time_window_filter/_recurrence_validator.py

@@ -4,8 +4,8 @@
 # license information.
 # -------------------------------------------------------------------------
 from datetime import datetime, timedelta
-from typing import List
 from ._models import RecurrencePatternType, RecurrenceRangeType, TimeWindowFilterSettings
+from ._recurrence_evaluator import _get_passed_week_days, _sort_days_of_week
 
 
 DAYS_PER_WEEK = 7
@@ -151,12 +151,3 @@ def _is_duration_compliant_with_days_of_week(settings: TimeWindowFilterSettings)
 
     time_window_duration = settings.end - settings.start
     return min_gap >= time_window_duration
-
-
-def _get_passed_week_days(today: int, first_day_of_week: int) -> int:
-    return (today - first_day_of_week + DAYS_PER_WEEK) % DAYS_PER_WEEK
-
-
-def _sort_days_of_week(days_of_week: List[int], first_day_of_week: int) -> List[int]:
-    sorted_days = sorted(days_of_week, key=lambda day: _get_passed_week_days(day, first_day_of_week))
-    return sorted_days