mirror of
https://github.com/jpawlowski/hass.tibber_prices.git
synced 2026-05-28 18:43:40 +00:00
fix(interval_pool): rebuild index after dead interval cleanup without empty groups
Cherry-pick of v0.30.1 hotfix (ec3bc9f). When _cleanup_dead_intervals
compacted group lists but no groups became fully empty, the index retained
stale interval_index values pointing past compacted list ends, causing
IndexError in _get_cached_intervals.
Now rebuilds the index whenever dead intervals are removed, even if no
groups are deleted.
Includes regression test for Issue #118 and updated touch operation tests
to reflect that GC now runs immediately after touch-only paths.
Closes #118
Impact: Eliminates IndexError crash for users with brand-new Tibber accounts
that have limited price history, where partial group compaction was most likely.
This commit is contained in:
parent
6b4c46a305
commit
ebcb9cfe77
2 changed files with 94 additions and 14 deletions
|
|
@ -93,6 +93,19 @@ class TibberPricesIntervalPoolGarbageCollector:
|
||||||
empty_removed,
|
empty_removed,
|
||||||
self._home_id,
|
self._home_id,
|
||||||
)
|
)
|
||||||
|
elif dead_count > 0:
|
||||||
|
# _cleanup_dead_intervals compacted group["intervals"] lists in-place,
|
||||||
|
# shifting the positions of surviving intervals. _remove_empty_groups
|
||||||
|
# only rebuilds the index when it removes completely-empty groups.
|
||||||
|
# If no groups became empty, the index still holds stale interval_index
|
||||||
|
# values that now point past the end of the compacted lists, causing
|
||||||
|
# an IndexError in _get_cached_intervals. Rebuild the index here to
|
||||||
|
# keep it consistent with the compacted groups.
|
||||||
|
self._index.rebuild(fetch_groups)
|
||||||
|
_LOGGER_DETAILS.debug(
|
||||||
|
"GC rebuilt index after dead interval cleanup for home %s",
|
||||||
|
self._home_id,
|
||||||
|
)
|
||||||
|
|
||||||
# Phase 2: Count total intervals after cleanup
|
# Phase 2: Count total intervals after cleanup
|
||||||
total_intervals = self._cache.count_total_intervals()
|
total_intervals = self._cache.count_total_intervals()
|
||||||
|
|
|
||||||
|
|
@ -104,16 +104,17 @@ class TestTouchOperations:
|
||||||
original_id = id(first_interval_original)
|
original_id = id(first_interval_original)
|
||||||
|
|
||||||
# Second fetch: Touch same intervals
|
# Second fetch: Touch same intervals
|
||||||
|
# GC now runs after touch-only paths, removing the old (dead) group
|
||||||
pool._add_intervals(sample_intervals, fetch_time_2) # noqa: SLF001
|
pool._add_intervals(sample_intervals, fetch_time_2) # noqa: SLF001
|
||||||
|
|
||||||
# Re-fetch groups (list may have changed)
|
# Re-fetch groups (GC compacted: old group removed, touch group remains)
|
||||||
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
||||||
|
|
||||||
# Verify: Now we have 2 fetch groups
|
# After touch + GC: only the touch group survives (old group fully dead → removed)
|
||||||
assert len(fetch_groups) == 2
|
assert len(fetch_groups) == 1
|
||||||
|
|
||||||
# Get reference to first interval from TOUCH group
|
# Get reference to first interval from the surviving touch group
|
||||||
first_interval_touched = fetch_groups[1]["intervals"][0]
|
first_interval_touched = fetch_groups[0]["intervals"][0]
|
||||||
touched_id = id(first_interval_touched)
|
touched_id = id(first_interval_touched)
|
||||||
|
|
||||||
# CRITICAL: Should be SAME object (same memory address)
|
# CRITICAL: Should be SAME object (same memory address)
|
||||||
|
|
@ -146,13 +147,13 @@ class TestTouchOperations:
|
||||||
assert index_entry is not None
|
assert index_entry is not None
|
||||||
assert index_entry["fetch_group_index"] == 0
|
assert index_entry["fetch_group_index"] == 0
|
||||||
|
|
||||||
# Second fetch (touch)
|
# Second fetch (touch) — GC runs after, compacting old group away
|
||||||
pool._add_intervals(sample_intervals, fetch_time_2) # noqa: SLF001
|
pool._add_intervals(sample_intervals, fetch_time_2) # noqa: SLF001
|
||||||
|
|
||||||
# Verify index now points to group 1 (touch group)
|
# After touch + GC: old group removed, touch group is now group 0
|
||||||
index_entry = pool._index.get(first_key) # noqa: SLF001
|
index_entry = pool._index.get(first_key) # noqa: SLF001
|
||||||
assert index_entry is not None
|
assert index_entry is not None
|
||||||
assert index_entry["fetch_group_index"] == 1, "Index should point to touch group"
|
assert index_entry["fetch_group_index"] == 0, "Index should point to surviving touch group"
|
||||||
|
|
||||||
|
|
||||||
class TestGarbageCollection:
|
class TestGarbageCollection:
|
||||||
|
|
@ -243,6 +244,70 @@ class TestGarbageCollection:
|
||||||
assert len(fetch_groups) == 1, "Empty fetch group should be removed"
|
assert len(fetch_groups) == 1, "Empty fetch group should be removed"
|
||||||
assert len(fetch_groups[0]["intervals"]) == 4
|
assert len(fetch_groups[0]["intervals"]) == 4
|
||||||
|
|
||||||
|
def test_gc_rebuilds_index_after_dead_interval_cleanup_no_empty_groups(
|
||||||
|
self,
|
||||||
|
pool: TibberPricesIntervalPool,
|
||||||
|
) -> None:
|
||||||
|
"""
|
||||||
|
Regression test for Issue #118 (IndexError for brand-new Tibber users).
|
||||||
|
|
||||||
|
Scenario: GC compacts a fetch group in-place (removes dead intervals at the
|
||||||
|
BEGINNING of the list), shifting surviving intervals to lower positions.
|
||||||
|
If no groups become completely empty, _remove_empty_groups does NOT rebuild
|
||||||
|
the index, leaving stale interval_index values that point past the end of
|
||||||
|
the compacted list → IndexError in _get_cached_intervals.
|
||||||
|
|
||||||
|
The fix: after dead interval cleanup without full group removal, explicitly
|
||||||
|
rebuild the index so surviving interval positions match the compacted list.
|
||||||
|
"""
|
||||||
|
# Step 1: Add 5 intervals (hours 0-4) → group 0
|
||||||
|
# Index: h0→(0,0), h1→(0,1), h2→(0,2), h3→(0,3), h4→(0,4)
|
||||||
|
initial_intervals = [
|
||||||
|
{
|
||||||
|
"startsAt": datetime(2025, 11, 25, h, 0, 0, tzinfo=UTC).isoformat(),
|
||||||
|
"total": 10.0 + h,
|
||||||
|
}
|
||||||
|
for h in range(5)
|
||||||
|
]
|
||||||
|
pool._add_intervals(initial_intervals, "2025-11-25T09:00:00+00:00") # noqa: SLF001
|
||||||
|
|
||||||
|
assert pool._cache.count_total_intervals() == 5 # noqa: SLF001
|
||||||
|
assert pool._index.count() == 5 # noqa: SLF001
|
||||||
|
|
||||||
|
# Step 2: Re-fetch h0, h1 (touch) + add new h5 → group 1
|
||||||
|
# - h0, h1 become dead in group 0 (index moves them to group 1)
|
||||||
|
# - h5 is new → added to group 1
|
||||||
|
# - h2, h3, h4 survive in group 0 at positions 2, 3, 4 (stale after GC)
|
||||||
|
second_fetch = [
|
||||||
|
{
|
||||||
|
"startsAt": datetime(2025, 11, 25, h, 0, 0, tzinfo=UTC).isoformat(),
|
||||||
|
"total": 10.0 + h,
|
||||||
|
}
|
||||||
|
for h in [0, 1, 5] # touch h0, h1; add new h5
|
||||||
|
]
|
||||||
|
pool._add_intervals(second_fetch, "2025-11-25T09:15:00+00:00") # noqa: SLF001
|
||||||
|
|
||||||
|
# GC ran (h5 was new): group 0 compacted from 5 → 3 intervals [h2, h3, h4]
|
||||||
|
# WITHOUT FIX: index has h2→(0,2), h3→(0,3), h4→(0,4) but group 0 only has 3 items
|
||||||
|
# WITH FIX: index rebuilt → h2→(0,0), h3→(0,1), h4→(0,2)
|
||||||
|
assert pool._cache.count_total_intervals() == 6 # h0,h1,h5 in group1 + h2,h3,h4 in group0 # noqa: SLF001
|
||||||
|
assert pool._index.count() == 6 # noqa: SLF001
|
||||||
|
|
||||||
|
# Step 3: Read all 6 intervals via the index — must NOT raise IndexError
|
||||||
|
start_iso = datetime(2025, 11, 25, 0, 0, 0, tzinfo=UTC).isoformat()
|
||||||
|
end_iso = datetime(2025, 11, 25, 6, 0, 0, tzinfo=UTC).isoformat()
|
||||||
|
|
||||||
|
# This calls _get_cached_intervals which looks up each timestamp in the index
|
||||||
|
# and accesses the interval by fetch_group_index + interval_index.
|
||||||
|
# Stale interval_index values (pointing past end of compacted list) → IndexError.
|
||||||
|
result = pool._get_cached_intervals(start_iso, end_iso) # noqa: SLF001
|
||||||
|
|
||||||
|
assert len(result) == 6, f"Expected 6 intervals, got {len(result)}"
|
||||||
|
|
||||||
|
# Verify correct values (spot-check h2, h3, h4 which were in the compacted group)
|
||||||
|
totals = {r["total"] for r in result}
|
||||||
|
assert totals == {10.0, 11.0, 12.0, 13.0, 14.0, 15.0}, f"Unexpected totals: {totals}"
|
||||||
|
|
||||||
|
|
||||||
class TestSerialization:
|
class TestSerialization:
|
||||||
"""Test serialization excludes dead intervals."""
|
"""Test serialization excludes dead intervals."""
|
||||||
|
|
@ -404,14 +469,15 @@ class TestIntervalIdentityPreservation:
|
||||||
# Collect memory addresses of intervals in original group
|
# Collect memory addresses of intervals in original group
|
||||||
original_ids = [id(interval) for interval in fetch_groups[0]["intervals"]]
|
original_ids = [id(interval) for interval in fetch_groups[0]["intervals"]]
|
||||||
|
|
||||||
# Second fetch (touch)
|
# Second fetch (touch) — GC runs after, compacting old group away
|
||||||
pool._add_intervals(sample_intervals, "2025-11-25T10:15:00+01:00") # noqa: SLF001
|
pool._add_intervals(sample_intervals, "2025-11-25T10:15:00+01:00") # noqa: SLF001
|
||||||
|
|
||||||
# Re-fetch groups
|
# Re-fetch groups (after GC: only touch group survives at index 0)
|
||||||
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
||||||
|
assert len(fetch_groups) == 1
|
||||||
|
|
||||||
# Collect memory addresses of intervals in touch group
|
# Collect memory addresses of intervals in surviving touch group
|
||||||
touched_ids = [id(interval) for interval in fetch_groups[1]["intervals"]]
|
touched_ids = [id(interval) for interval in fetch_groups[0]["intervals"]]
|
||||||
|
|
||||||
# CRITICAL: All memory addresses should be identical (same objects)
|
# CRITICAL: All memory addresses should be identical (same objects)
|
||||||
assert original_ids == touched_ids, "Touch should preserve interval identity (memory addresses)"
|
assert original_ids == touched_ids, "Touch should preserve interval identity (memory addresses)"
|
||||||
|
|
@ -419,9 +485,10 @@ class TestIntervalIdentityPreservation:
|
||||||
# Third fetch (touch again)
|
# Third fetch (touch again)
|
||||||
pool._add_intervals(sample_intervals, "2025-11-25T10:30:00+01:00") # noqa: SLF001
|
pool._add_intervals(sample_intervals, "2025-11-25T10:30:00+01:00") # noqa: SLF001
|
||||||
|
|
||||||
# Re-fetch groups
|
# Re-fetch groups (again only 1 group after GC)
|
||||||
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
fetch_groups = pool._cache.get_fetch_groups() # noqa: SLF001
|
||||||
|
assert len(fetch_groups) == 1
|
||||||
|
|
||||||
# New touch group should also reference the SAME original objects
|
# New touch group should also reference the SAME original objects
|
||||||
touched_ids_2 = [id(interval) for interval in fetch_groups[2]["intervals"]]
|
touched_ids_2 = [id(interval) for interval in fetch_groups[0]["intervals"]]
|
||||||
assert original_ids == touched_ids_2, "Multiple touches should preserve original identity"
|
assert original_ids == touched_ids_2, "Multiple touches should preserve original identity"
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue