Fix find_next_time_expression_time (home-assistant#58894)

* Better tests

* Fix find_next_time_expression_time

* Add tests for Nov 7th 2021, Chicago transtion

* Update event tests

* Update test_event.py

* small performance improvement

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: Erik Montnemery <erik@montnemery.com>

homeassistant/util/dt.py

@@ -272,7 +272,8 @@ def _lower_bound(arr: list[int], cmp: int) -> int | None:
             return None
         return arr[left]
 
-    result = now.replace(microsecond=0)
+    # Reset microseconds and fold; fold (for ambiguous DST times) will be handled later
+    result = now.replace(microsecond=0, fold=0)
 
     # Match next second
     if (next_second := _lower_bound(seconds, result.second)) is None:
@@ -309,40 +310,58 @@ def _lower_bound(arr: list[int], cmp: int) -> int | None:
     result = result.replace(hour=next_hour)
 
     if result.tzinfo in (None, UTC):
+        # Using UTC, no DST checking needed
         return result
 
-    if _datetime_ambiguous(result):
-        # This happens when we're leaving daylight saving time and local
-        # clocks are rolled back. In this case, we want to trigger
-        # on both the DST and non-DST time. So when "now" is in the DST
-        # use the DST-on time, and if not, use the DST-off time.
-        fold = 1 if now.dst() else 0
-        if result.fold != fold:
-            result = result.replace(fold=fold)
-
     if not _datetime_exists(result):
-        # This happens when we're entering daylight saving time and local
-        # clocks are rolled forward, thus there are local times that do
-        # not exist. In this case, we want to trigger on the next time
-        # that *does* exist.
-        # In the worst case, this will run through all the seconds in the
-        # time shift, but that's max 3600 operations for once per year
+        # When entering DST and clocks are turned forward.
+        # There are wall clock times that don't "exist" (an hour is skipped).
+
+        # -> trigger on the next time that 1. matches the pattern and 2. does exist
+        # for example:
+        #   on 2021.03.28 02:00:00 in CET timezone clocks are turned forward an hour
+        #   with pattern "02:30", don't run on 28 mar (such a wall time does not exist on this day)
+        #   instead run at 02:30 the next day
+
+        # We solve this edge case by just iterating one second until the result exists
+        # (max. 3600 operations, which should be fine for an edge case that happens once a year)
         return find_next_time_expression_time(
             result + dt.timedelta(seconds=1), seconds, minutes, hours
         )
 
-    # Another edge-case when leaving DST:
-    # When now is in DST and ambiguous *and* the next trigger time we *should*
-    # trigger is ambiguous and outside DST, the excepts above won't catch it.
-    # For example: if triggering on 2:30 and now is 28.10.2018 2:30 (in DST)
-    # we should trigger next on 28.10.2018 2:30 (out of DST), but our
-    # algorithm above would produce 29.10.2018 2:30 (out of DST)
-    if _datetime_ambiguous(now):
+    now_is_ambiguous = _datetime_ambiguous(now)
+    result_is_ambiguous = _datetime_ambiguous(result)
+
+    # When leaving DST and clocks are turned backward.
+    # Then there are wall clock times that are ambiguous i.e. exist with DST and without DST
+    # The logic above does not take into account if a given pattern matches _twice_
+    # in a day.
+    # Example: on 2021.10.31 02:00:00 in CET timezone clocks are turned backward an hour
+
+    if now_is_ambiguous and result_is_ambiguous:
+        # `now` and `result` are both ambiguous, so the next match happens
+        # _within_ the current fold.
+
+        # Examples:
+        #  1. 2021.10.31 02:00:00+02:00 with pattern 02:30 -> 2021.10.31 02:30:00+02:00
+        #  2. 2021.10.31 02:00:00+01:00 with pattern 02:30 -> 2021.10.31 02:30:00+01:00
+        return result.replace(fold=now.fold)
+
+    if now_is_ambiguous and now.fold == 0 and not result_is_ambiguous:
+        # `now` is in the first fold, but result is not ambiguous (meaning it no longer matches
+        # within the fold).
+        # -> Check if result matches in the next fold. If so, emit that match
+
+        # Turn back the time by the DST offset, effectively run the algorithm on the first fold
+        # If it matches on the first fold, that means it will also match on the second one.
+
+        # Example: 2021.10.31 02:45:00+02:00 with pattern 02:30 -> 2021.10.31 02:30:00+01:00
+
         check_result = find_next_time_expression_time(
             now + _dst_offset_diff(now), seconds, minutes, hours
         )
         if _datetime_ambiguous(check_result):
-            return check_result
+            return check_result.replace(fold=1)
 
     return result
 

tests/helpers/test_event.py

@@ -3399,9 +3399,19 @@ async def test_periodic_task_entering_dst(hass):
     dt_util.set_default_time_zone(timezone)
     specific_runs = []
 
-    now = dt_util.utcnow()
+    # DST starts early morning March 27th 2022
+    yy = 2022
+    mm = 3
+    dd = 27
+
+    # There's no 2022-03-27 02:30, the event should not fire until 2022-03-28 02:30
     time_that_will_not_match_right_away = datetime(
-        now.year + 1, 3, 25, 2, 31, 0, tzinfo=timezone
+        yy, mm, dd, 1, 28, 0, tzinfo=timezone, fold=0
+    )
+    # Make sure we enter DST during the test
+    assert (
+        time_that_will_not_match_right_away.utcoffset()
+        != (time_that_will_not_match_right_away + timedelta(hours=2)).utcoffset()
     )
 
     with patch(
@@ -3416,25 +3426,25 @@ async def test_periodic_task_entering_dst(hass):
         )
 
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 3, 25, 1, 50, 0, 999999, tzinfo=timezone)
+        hass, datetime(yy, mm, dd, 1, 50, 0, 999999, tzinfo=timezone)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 0
 
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 3, 25, 3, 50, 0, 999999, tzinfo=timezone)
+        hass, datetime(yy, mm, dd, 3, 50, 0, 999999, tzinfo=timezone)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 0
 
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 3, 26, 1, 50, 0, 999999, tzinfo=timezone)
+        hass, datetime(yy, mm, dd + 1, 1, 50, 0, 999999, tzinfo=timezone)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 0
 
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 3, 26, 2, 50, 0, 999999, tzinfo=timezone)
+        hass, datetime(yy, mm, dd + 1, 2, 50, 0, 999999, tzinfo=timezone)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 1
@@ -3448,10 +3458,19 @@ async def test_periodic_task_leaving_dst(hass):
     dt_util.set_default_time_zone(timezone)
     specific_runs = []
 
-    now = dt_util.utcnow()
+    # DST ends early morning Ocotber 30th 2022
+    yy = 2022
+    mm = 10
+    dd = 30
 
     time_that_will_not_match_right_away = datetime(
-        now.year + 1, 10, 28, 2, 28, 0, tzinfo=timezone, fold=1
+        yy, mm, dd, 2, 28, 0, tzinfo=timezone, fold=0
+    )
+
+    # Make sure we leave DST during the test
+    assert (
+        time_that_will_not_match_right_away.utcoffset()
+        != time_that_will_not_match_right_away.replace(fold=1).utcoffset()
     )
 
     with patch(
@@ -3465,38 +3484,134 @@ async def test_periodic_task_leaving_dst(hass):
             second=0,
         )
 
+    # The task should not fire yet
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 10, 28, 2, 5, 0, 999999, tzinfo=timezone, fold=0)
+        hass, datetime(yy, mm, dd, 2, 28, 0, 999999, tzinfo=timezone, fold=0)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 0
 
+    # The task should fire
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 2, 30, 0, 999999, tzinfo=timezone, fold=0)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 1
+
+    # The task should not fire again
     async_fire_time_changed(
-        hass, datetime(now.year + 1, 10, 28, 2, 55, 0, 999999, tzinfo=timezone, fold=0)
+        hass, datetime(yy, mm, dd, 2, 55, 0, 999999, tzinfo=timezone, fold=0)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 1
 
+    # DST has ended, the task should not fire yet
     async_fire_time_changed(
         hass,
-        datetime(now.year + 2, 10, 28, 2, 45, 0, 999999, tzinfo=timezone, fold=1),
+        datetime(yy, mm, dd, 2, 15, 0, 999999, tzinfo=timezone, fold=1),
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 1
+
+    # The task should fire
+    async_fire_time_changed(
+        hass,
+        datetime(yy, mm, dd, 2, 45, 0, 999999, tzinfo=timezone, fold=1),
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 2
 
+    # The task should not fire again
     async_fire_time_changed(
         hass,
-        datetime(now.year + 2, 10, 28, 2, 55, 0, 999999, tzinfo=timezone, fold=1),
+        datetime(yy, mm, dd, 2, 55, 0, 999999, tzinfo=timezone, fold=1),
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 2
 
+    # The task should fire again the next day
     async_fire_time_changed(
-        hass, datetime(now.year + 2, 10, 28, 2, 55, 0, 999999, tzinfo=timezone, fold=1)
+        hass, datetime(yy, mm, dd + 1, 2, 55, 0, 999999, tzinfo=timezone, fold=1)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 3
+
+    unsub()
+
+
+async def test_periodic_task_leaving_dst_2(hass):
+    """Test periodic task behavior when leaving dst."""
+    timezone = dt_util.get_time_zone("Europe/Vienna")
+    dt_util.set_default_time_zone(timezone)
+    specific_runs = []
+
+    # DST ends early morning Ocotber 30th 2022
+    yy = 2022
+    mm = 10
+    dd = 30
+
+    time_that_will_not_match_right_away = datetime(
+        yy, mm, dd, 2, 28, 0, tzinfo=timezone, fold=0
+    )
+    # Make sure we leave DST during the test
+    assert (
+        time_that_will_not_match_right_away.utcoffset()
+        != time_that_will_not_match_right_away.replace(fold=1).utcoffset()
+    )
+
+    with patch(
+        "homeassistant.util.dt.utcnow", return_value=time_that_will_not_match_right_away
+    ):
+        unsub = async_track_time_change(
+            hass,
+            callback(lambda x: specific_runs.append(x)),
+            minute=30,
+            second=0,
+        )
+
+    # The task should not fire yet
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 2, 28, 0, 999999, tzinfo=timezone, fold=0)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 0
+
+    # The task should fire
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 2, 55, 0, 999999, tzinfo=timezone, fold=0)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 1
+
+    # DST has ended, the task should not fire yet
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 2, 15, 0, 999999, tzinfo=timezone, fold=1)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 1
+
+    # The task should fire
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 2, 45, 0, 999999, tzinfo=timezone, fold=1)
     )
     await hass.async_block_till_done()
     assert len(specific_runs) == 2
 
+    # The task should not fire again
+    async_fire_time_changed(
+        hass,
+        datetime(yy, mm, dd, 2, 55, 0, 999999, tzinfo=timezone, fold=1),
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 2
+
+    # The task should fire again the next hour
+    async_fire_time_changed(
+        hass, datetime(yy, mm, dd, 3, 55, 0, 999999, tzinfo=timezone, fold=0)
+    )
+    await hass.async_block_till_done()
+    assert len(specific_runs) == 3
+
     unsub()