adding tests and fixes

Author: mrm9084

featuremanagement/_time_window_filter/_models.py

@@ -70,15 +70,19 @@ class RecurrencePattern:  # pylint: disable=too-few-public-methods
     """
 
     days: List[str] = ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday", "Saturday", "Sunday"]
-    days_of_week: List[int] = []
 
     def __init__(self, pattern_data: Dict[str, Any]):
         self.type = RecurrencePatternType.from_str(pattern_data.get("Type", "Daily"))
         self.interval = pattern_data.get("Interval", 1)
         if self.interval <= 0:
             raise ValueError("The interval must be greater than 0.")
-        days_of_week = pattern_data.get("DaysOfWeek", [])
-        for day in days_of_week:
+        # Days of the week are represented as a list of strings of there names.
+        days_of_week_str = pattern_data.get("DaysOfWeek", [])
+
+        # Days of the week are represented as a list of integers from 0 to 6.
+        # 0 = Sunday, 1 = Monday, ..., 6 = Saturday
+        self.days_of_week = []
+        for day in days_of_week_str:
             self.days_of_week.append(self.days.index(day))
         self.first_day_of_week = self.days.index(pattern_data.get("FirstDayOfWeek", "Sunday"))
 

featuremanagement/_time_window_filter/_recurrence_validator.py

@@ -5,7 +5,7 @@
 # -------------------------------------------------------------------------
 from datetime import datetime, timedelta
 from typing import List
-from ._models import RecurrencePatternType, RecurrenceRangeType, Recurrence
+from ._models import RecurrencePatternType, RecurrenceRangeType, Recurrence, RecurrencePattern, RecurrenceRange
 
 
 DAYS_PER_WEEK = 7
@@ -27,102 +27,90 @@ def validate_settings(recurrence: Recurrence, start: datetime, end: datetime) ->
     :param TimeWindowFilterSettings settings: The settings for the time window filter.
     :raises ValueError: If the settings are invalid.
     """
-    _validate_recurrence_required_parameter(recurrence, start, end)
-    _validate_recurrence_pattern(recurrence, start, end)
-    _validate_recurrence_range(recurrence, start)
-
-
-def _validate_recurrence_required_parameter(recurrence: Recurrence, start: datetime, end: datetime) -> None:
-    param_name = ""
-    reason = ""
-    if recurrence.pattern is None:
-        param_name = f"{RECURRENCE_PATTERN}"
-        reason = REQUIRED_PARAMETER
-    if recurrence.range is None:
-        param_name = f"{RECURRENCE_RANGE}"
-        reason = REQUIRED_PARAMETER
-    if not end > start:
-        param_name = "end"
-        reason = OUT_OF_RANGE
-    if end > start + timedelta(days=TEN_YEARS):
-        param_name = "end"
-        reason = TIME_WINDOW_DURATION_TEN_YEARS
+    if not recurrence:
+        raise ValueError("Recurrence is required")
+    
+    _validate_start_end_parameter(start, end)
+    _validate_recurrence_pattern(recurrence.pattern, start, end)
+    _validate_recurrence_range(recurrence.range, start)
+
 
-    if param_name:
-        raise ValueError(reason % param_name)
+def _validate_start_end_parameter(start: datetime, end: datetime) -> None:
+    param_name = "end"
+    if end > start + timedelta(days=TEN_YEARS):
+        raise ValueError(TIME_WINDOW_DURATION_TEN_YEARS % param_name)
 
 
-def _validate_recurrence_pattern(recurrence: Recurrence, start: datetime, end: datetime) -> None:
-    if recurrence is None:
-        raise ValueError(REQUIRED_PARAMETER % "Recurrence")
-    pattern_type = recurrence.pattern.type
+def _validate_recurrence_pattern(pattern: RecurrencePattern, start: datetime, end: datetime) -> None:
+    pattern_type = pattern.type
 
     if pattern_type == RecurrencePatternType.DAILY:
-        _validate_daily_recurrence_pattern(recurrence, start, end)
+        _validate_daily_recurrence_pattern(pattern, start, end)
     else:
-        _validate_weekly_recurrence_pattern(recurrence, start, end)
+        _validate_weekly_recurrence_pattern(pattern, start, end)
 
 
-def _validate_recurrence_range(recurrence: Recurrence, start: datetime) -> None:
-    range_type = recurrence.range.type
+def _validate_recurrence_range(recurrence_range: RecurrenceRange, start: datetime) -> None:
+    range_type = recurrence_range.type
     if range_type == RecurrenceRangeType.END_DATE:
-        _validate_end_date(recurrence, start)
+        _validate_end_date(recurrence_range, start)
 
 
-def _validate_daily_recurrence_pattern(recurrence: Recurrence, start: datetime, end: datetime) -> None:
+def _validate_daily_recurrence_pattern(pattern: RecurrencePattern, start: datetime, end: datetime) -> None:
     # "Start" is always a valid first occurrence for "Daily" pattern.
     # Only need to check if time window validated
-    _validate_time_window_duration(recurrence, start, end)
+    _validate_time_window_duration(pattern, start, end)
 
 
-def _validate_weekly_recurrence_pattern(recurrence: Recurrence, start: datetime, end: datetime) -> None:
-    _validate_days_of_week(recurrence)
+def _validate_weekly_recurrence_pattern(pattern: RecurrencePattern, start: datetime, end: datetime) -> None:
+    _validate_days_of_week(pattern)
 
     # Check whether "Start" is a valid first occurrence
-    pattern = recurrence.pattern
     if start.weekday() not in pattern.days_of_week:
         raise ValueError(NOT_MATCHED % "start")
 
     # Time window duration must be shorter than how frequently it occurs
-    _validate_time_window_duration(recurrence, start, end)
+    _validate_time_window_duration(pattern, start, end)
 
     # Check whether the time window duration is shorter than the minimum gap between days of week
-    if not _is_duration_compliant_with_days_of_week(recurrence, start, end):
+    if not _is_duration_compliant_with_days_of_week(pattern, start, end):
         raise ValueError(TIME_WINDOW_DURATION_OUT_OF_RANGE % "Recurrence.Pattern.DaysOfWeek")
 
 
-def _validate_time_window_duration(recurrence: Recurrence, start: datetime, end: datetime) -> None:
-    pattern = recurrence.pattern
+def _validate_time_window_duration(pattern: RecurrencePattern, start: datetime, end: datetime) -> None:
     interval_duration = (
         timedelta(days=pattern.interval)
         if pattern.type == RecurrencePatternType.DAILY
         else timedelta(days=pattern.interval * DAYS_PER_WEEK)
     )
     time_window_duration = end - start
+    if start > end:
+        raise ValueError(OUT_OF_RANGE % "The filter start date Start needs to before the End date.")
+
     if time_window_duration > interval_duration:
         raise ValueError(TIME_WINDOW_DURATION_OUT_OF_RANGE % "Recurrence.Pattern.Interval")
 
 
-def _validate_days_of_week(recurrence: Recurrence) -> None:
-    days_of_week = recurrence.pattern.days_of_week
+def _validate_days_of_week(pattern: RecurrencePattern) -> None:
+    days_of_week = pattern.days_of_week
     if not days_of_week:
         raise ValueError(REQUIRED_PARAMETER % "Recurrence.Pattern.DaysOfWeek")
 
 
-def _validate_end_date(recurrence: Recurrence, start: datetime) -> None:
-    end_date = recurrence.range.end_date
+def _validate_end_date(recurrence_range: RecurrenceRange, start: datetime) -> None:
+    end_date = recurrence_range.end_date
     if end_date and end_date < start:
         raise ValueError("The Recurrence.Range.EndDate should be after the Start")
 
 
-def _is_duration_compliant_with_days_of_week(recurrence: Recurrence, start: datetime, end: datetime) -> bool:
-    days_of_week = recurrence.pattern.days_of_week
+def _is_duration_compliant_with_days_of_week(pattern: RecurrencePattern, start: datetime, end: datetime) -> bool:
+    days_of_week = pattern.days_of_week
     if len(days_of_week) == 1:
         return True
 
     # Get the date of first day of the week
     today = datetime.now()
-    first_day_of_week = recurrence.pattern.first_day_of_week
+    first_day_of_week = pattern.first_day_of_week
     offset = _get_passed_week_days(today.weekday(), first_day_of_week)
     first_date_of_week = today - timedelta(days=offset)
     sorted_days_of_week = _sort_days_of_week(days_of_week, first_day_of_week)
@@ -138,7 +126,7 @@ def _is_duration_compliant_with_days_of_week(recurrence: Recurrence, start: date
             min_gap = min(min_gap, current_gap)
         prev_occurrence = date
 
-    if recurrence.pattern.interval == 1:
+    if pattern.interval == 1:
         # It may cross weeks. Check the adjacent week
         date = first_date_of_week + timedelta(
             days=DAYS_PER_WEEK + _get_passed_week_days(sorted_days_of_week[0], first_day_of_week)

tests/time_window_filter/test_recurrence_validator.py

@@ -0,0 +1,88 @@
+import pytest
+from datetime import datetime, timedelta
+from featuremanagement._time_window_filter._models import Recurrence
+from featuremanagement._time_window_filter._recurrence_validator import validate_settings
+
+def valid_daily_pattern():
+    return {
+        "Type": "Daily",
+        "Interval": 1,
+        "DaysOfWeek": ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday"],
+        "FirstDayOfWeek": "Sunday",
+    }
+
+def valid_daily_recurrence():
+    return Recurrence(
+        {
+            "Pattern": valid_daily_pattern(),
+            "Range": {"Type": "NoEnd", "EndDate": None, "NumberOfOccurrences": 10},
+        }
+    )
+
+def valid_daily_end_date_recurrence():
+    return Recurrence(
+        {
+            "Pattern": valid_daily_pattern(),
+            "Range": {"Type": "EndDate", "EndDate": datetime.now() + timedelta(days=10), "NumberOfOccurrences": 10},
+        }
+    )
+
+def valid_weekly_recurrence():
+    return Recurrence(
+        {
+            "Pattern": {
+                "Type": "Weekly",
+                "Interval": 1,
+                "DaysOfWeek": ["Monday", "Tuesday", "Wednesday", "Thursday", "Friday"],
+                "FirstDayOfWeek": "Monday",
+            },
+            "Range": {"Type": "NoEnd", "EndDate": None, "NumberOfOccurrences": 10},
+        }
+    )
+
+
+def test_validate_settings_valid_daily():
+    start = datetime.now()
+    end = start + timedelta(days=1)
+    validate_settings(valid_daily_recurrence(), start, end)
+
+def test_validate_settings_valid_daily_end_date():
+    start = datetime.now()
+    end = start + timedelta(days=1)
+    validate_settings(valid_daily_end_date_recurrence(), start, end)
+
+
+def test_validate_settings_valid_weekly():
+    start = datetime.now()
+    end = start + timedelta(days=1)
+    validate_settings(valid_weekly_recurrence(), start, end)
+
+def test_validate_settings_duration_exceeds_ten_years():
+    start = datetime.now()
+    end = start + timedelta(days=3651)
+    with pytest.raises(ValueError, match="Time window duration exceeds ten years: end"):
+        validate_settings(valid_daily_recurrence(), start, end)
+
+def test_validate_settings_invalid_start_end():
+    start = datetime.now() + timedelta(days=1)
+    end = datetime.now()
+    with pytest.raises(ValueError, match="The filter start date Start needs to before the End date."):
+        validate_settings(valid_daily_recurrence(), start, end)
+
+def test_validate_settings_end_date_in_past():
+    start = datetime.now()
+    end = datetime.now() - timedelta(days=1)
+    with pytest.raises(ValueError, match="The filter start date Start needs to before the End date."):
+        validate_settings(valid_daily_recurrence(), start, end)
+
+def test_validate_settings_missing_recurrence():
+    start = datetime.now()
+    end = start + timedelta(days=1)
+    with pytest.raises(ValueError, match="Recurrence is required"):
+        validate_settings(None, start, end)
+
+def test_validate_settings_invalid_recurrence_pattern():
+    with pytest.raises(ValueError, match="Invalid value: InvalidType"):
+         Recurrence({"Pattern": {"Type": "InvalidType"}, "Range": {"Type": "NoEnd"}})
+
+