diff --git a/backend/app/services/calendar_service.py b/backend/app/services/calendar_service.py index 5de171f..38bdb37 100644 --- a/backend/app/services/calendar_service.py +++ b/backend/app/services/calendar_service.py @@ -8,8 +8,14 @@ UI can show what the app actually treats as free, not just the statutory calendar. No runtime external API calls (§6): everything here reads the pre-built -cache (or, outside its range, the local `holidays` library) via -`app.utils.holidays`. +cache via `app.utils.holidays`. The requested window is clamped to the +cache's [min_year, max_year] (see `get_calendar_window`) and every lookup +uses `strict=True`, so this endpoint can never silently fall back to the +local `holidays` library for a year the cache doesn't cover — that fallback +only knows formal röda dagar (+ every Sunday), not de facto days or +klämdagar, and serving it unlabelled here previously caused a real bug +(Julafton/Nyårsafton/Midsommarafton silently reading as working days past +the cache's end). """ from __future__ import annotations @@ -93,13 +99,27 @@ class CalendarService: ) end_year = start_year + years - 1 + if max_year is not None and end_year > max_year: + # Clamp rather than 422: the response already carries min_year/ + # max_year for the UI's paging bounds, so the last page is + # naturally shorter than `years` requested. `end_year` below + # reports the ACTUAL clamped range — never a range not returned. + end_year = max_year overrides = await self._day_override_service.get_effective_map(workspace_id) days: list[CalendarDay] = [] for year in range(start_year, end_year + 1): - holidays_map = get_swedish_holidays(year) - red_days = get_red_days(year) + # `strict=True`: after the clamp above, every `year` here is + # guaranteed inside [min_year, max_year], so this must never hit + # the live-`holidays` fallback. If it ever does, that's a bug in + # the clamping above, not bad input — fail loudly (see + # `app.utils.holidays.UncachedYearError`) rather than silently + # serving degraded data for this endpoint (the actual bug this + # guards against: years past the cache losing Julafton, + # Nyårsafton, Midsommarafton). + holidays_map = get_swedish_holidays(year, strict=True) + red_days = get_red_days(year, strict=True) day = date(year, 1, 1) one_day = timedelta(days=1) while day.year == year: diff --git a/backend/app/utils/holidays.py b/backend/app/utils/holidays.py index e8c25da..4a8c238 100644 --- a/backend/app/utils/holidays.py +++ b/backend/app/utils/holidays.py @@ -103,7 +103,22 @@ def _from_library(year: int) -> Dict[datetime.date, str]: return dict(sorted(se_holidays.items())) -def get_swedish_holidays(year: int) -> Dict[datetime.date, str]: +class UncachedYearError(RuntimeError): + """Raised by a `strict=True` caller when `year` falls outside + `swedish_calendar.json`'s range (or the cache file is missing). + + This should never fire in normal operation: callers that pass + `strict=True` (currently only `CalendarService`) are expected to clamp + their requested window to `cached_year_bounds()` first. Hitting this is + therefore a bug in that clamping, not bad user input — fail loudly + instead of silently falling back to the `holidays` library, which gives + strictly poorer answers for the same dates (missing de facto days like + Julafton/Nyårsafton/Midsommarafton) and would defeat the whole point of + the cache (see module docstring; §6 offline/deterministic guarantee). + """ + + +def get_swedish_holidays(year: int, *, strict: bool = False) -> Dict[datetime.date, str]: """Get all Swedish public holidays + de facto days + Sundays for a year. Returns a mapping of date -> Swedish display name. Cached per year; @@ -111,10 +126,24 @@ def get_swedish_holidays(year: int) -> Dict[datetime.date, str]: Args: year: Calendar year (e.g., 2026) + strict: if True, raise `UncachedYearError` instead of silently + falling back to the `holidays` library when `year` is outside + the cached range. Checked before consulting the per-year cache + so a prior non-strict call for the same (uncached) year can + never mask a later strict call's check. Returns: Dict mapping datetime.date to holiday name (e.g. "Juldagen", "Julafton") """ + if strict: + bounds = cached_year_bounds() + if bounds is None or not (bounds[0] <= year <= bounds[1]): + where = f"range {bounds[0]}-{bounds[1]}" if bounds is not None else "cache (missing)" + raise UncachedYearError( + f"Year {year} is outside the cached Swedish calendar {where}; " + "refusing to fall back to the live `holidays` library for a strict caller." + ) + if year in _holiday_cache: return _holiday_cache[year] @@ -126,7 +155,7 @@ def get_swedish_holidays(year: int) -> Dict[datetime.date, str]: return result -def get_red_days(year: int) -> Dict[datetime.date, str]: +def get_red_days(year: int, *, strict: bool = False) -> Dict[datetime.date, str]: """Real red days for `year`: public holidays and de facto days off, EXCLUDING ordinary Sundays (the bare "Söndag" marker). @@ -135,9 +164,13 @@ def get_red_days(year: int) -> Dict[datetime.date, str]: dropped. Use this (not `get_swedish_holidays`) for red-day cluster detection, where a bare weekend must not be conflated with a real holiday. + + `strict` is forwarded to `get_swedish_holidays` — see there. """ return { - d: name for d, name in get_swedish_holidays(year).items() if name != _SUNDAY_HOLIDAY_NAME + d: name + for d, name in get_swedish_holidays(year, strict=strict).items() + if name != _SUNDAY_HOLIDAY_NAME } diff --git a/backend/tests/unit/test_calendar_router.py b/backend/tests/unit/test_calendar_router.py index c987126..0f767a1 100644 --- a/backend/tests/unit/test_calendar_router.py +++ b/backend/tests/unit/test_calendar_router.py @@ -99,6 +99,40 @@ class TestGetCalendarDays: assert response.status_code == 422 + async def test_a_window_reaching_the_cache_end_is_clamped_not_overshot( + self, client: AsyncClient, connection: aiosqlite.Connection + ) -> None: + """Regression test for the real bug: `start_year=&years=4` + used to return 200 with `end_year = max_year + 3`, silently serving + years past the cache from the degraded `holidays`-library fallback + (missing Julafton/Nyårsafton/Midsommarafton). It must now clamp to + the cache and report the ACTUAL served range. + """ + workspace_repo = SQLiteWorkspaceRepository(connection) + ws = await workspace_repo.create( + Workspace(id=uuid4(), name="Familj", created_at=datetime.now()) + ) + + # Discover max_year from a normal response rather than hardcoding it. + bounds_response = await client.get( + "/api/calendar/days", + params={"workspace_id": str(ws.id), "start_year": 2026}, + ) + max_year = bounds_response.json()["max_year"] + assert max_year is not None + + response = await client.get( + "/api/calendar/days", + params={"workspace_id": str(ws.id), "start_year": max_year, "years": 4}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["start_year"] == max_year + assert body["end_year"] == max_year, "end_year must report the clamped range, not overshoot" + years_seen = {int(d["date"][:4]) for d in body["days"]} + assert years_seen == {max_year}, "no day may come from beyond the cache" + async def test_workspace_day_overrides_are_merged_into_the_response( self, client: AsyncClient, connection: aiosqlite.Connection ) -> None: diff --git a/backend/tests/unit/test_calendar_service.py b/backend/tests/unit/test_calendar_service.py index e7c0bcc..3138b3c 100644 --- a/backend/tests/unit/test_calendar_service.py +++ b/backend/tests/unit/test_calendar_service.py @@ -77,6 +77,115 @@ class TestYearWindowPagination: years_seen = {d.date.year for d in window.days} assert len(years_seen) == MAX_WINDOW_YEARS + async def test_window_reaching_max_year_is_clamped_not_rejected( + self, calendar_service: CalendarService + ) -> None: + """§ bug: `GET /days?start_year=&years=4` used to happily + report `end_year = max_year + 3` and silently serve the extra years + from the degraded `holidays`-library fallback. The window must be + clamped to the cache instead, and `end_year` must report the ACTUAL + range served — never a range that wasn't returned. + """ + bounds = cached_year_bounds() + assert bounds is not None + _, max_year = bounds + + window = await calendar_service.get_calendar_window( + workspace_id=uuid4(), start_year=max_year, years=MAX_WINDOW_YEARS + ) + + assert window.start_year == max_year + assert window.end_year == max_year, "must clamp end_year to the cache, not overshoot it" + years_seen = {d.date.year for d in window.days} + assert years_seen == {max_year}, "no day may come from a year beyond the cache" + + async def test_a_middle_page_that_would_overshoot_is_also_clamped( + self, calendar_service: CalendarService + ) -> None: + """Same clamp, exercised one page before the very last year (the + `start_year=2034, years=4` case from real paging: 2034+3=2037 would + overshoot the 2026-2035 cache by two years).""" + bounds = cached_year_bounds() + assert bounds is not None + _, max_year = bounds + start_year = max_year - 1 + + window = await calendar_service.get_calendar_window( + workspace_id=uuid4(), start_year=start_year, years=MAX_WINDOW_YEARS + ) + + assert window.end_year == max_year + years_seen = {d.date.year for d in window.days} + assert years_seen == {start_year, max_year} + assert max(years_seen) <= max_year + + +class TestNoDegradedDataAcrossTheServedRange: + """§4.2/§6: the fallback `holidays` library only knows formal röda dagar + and misses de facto days (Julafton, Nyårsafton, Midsommarafton). This is + exactly the regression that shipped: years past the cache silently lost + those days. Assert they're present for EVERY year the endpoint can ever + serve, and that no year is silently poorer than its neighbours. + """ + + @staticmethod + async def _all_cached_years_red_days( + calendar_service: CalendarService, + ) -> dict[int, dict[date, str]]: + bounds = cached_year_bounds() + assert bounds is not None + min_year, max_year = bounds + + by_year: dict[int, dict[date, str]] = {} + start = min_year + while start <= max_year: + window = await calendar_service.get_calendar_window( + workspace_id=uuid4(), start_year=start + ) + for day in window.days: + if day.day_type == "red_day": + by_year.setdefault(day.date.year, {})[day.date] = day.name or "" + start = window.end_year + 1 + return by_year + + async def test_julafton_nyarsafton_and_midsommarafton_are_red_days_every_year( + self, calendar_service: CalendarService + ) -> None: + by_year = await self._all_cached_years_red_days(calendar_service) + bounds = cached_year_bounds() + assert bounds is not None + min_year, max_year = bounds + assert set(by_year) == set(range(min_year, max_year + 1)), ( + "every cached year must be reachable via pagination" + ) + + for year, red_days in by_year.items(): + names_by_date = {d: name.lower() for d, name in red_days.items()} + + julafton = [d for d, n in names_by_date.items() if d.month == 12 and d.day == 24] + assert julafton and "julafton" in names_by_date[julafton[0]], ( + f"{year}: Julafton (12-24) must be a red day" + ) + + nyarsafton = [d for d, n in names_by_date.items() if d.month == 12 and d.day == 31] + assert nyarsafton and "nyårsafton" in names_by_date[nyarsafton[0]], ( + f"{year}: Nyårsafton (12-31) must be a red day" + ) + + midsommarafton = [d for d, n in names_by_date.items() if "midsommarafton" in n] + assert midsommarafton, f"{year}: Midsommarafton must be a red day" + + async def test_red_day_count_is_consistent_across_every_served_year( + self, calendar_service: CalendarService + ) -> None: + by_year = await self._all_cached_years_red_days(calendar_service) + counts = {year: len(red_days) for year, red_days in by_year.items()} + # No year may be silently poorer than its neighbours (the fallback's + # signature: 18 real red days in-cache vs. 13 once it kicks in). + assert len(set(counts.values())) == 1, ( + f"red-day counts differ across years, one may be degraded: {counts}" + ) + class TestOverrideMerging: async def test_worked_override_flips_a_statutory_red_day_to_not_free( diff --git a/backend/tests/unit/test_holidays.py b/backend/tests/unit/test_holidays.py index 3a91d8d..213cf7a 100644 --- a/backend/tests/unit/test_holidays.py +++ b/backend/tests/unit/test_holidays.py @@ -3,7 +3,11 @@ import datetime +import pytest + from app.utils.holidays import ( + UncachedYearError, + cached_year_bounds, get_holiday_name, get_red_days, get_swedish_holidays, @@ -255,3 +259,56 @@ class TestFallbackForUncachedYear: d for d, name in get_swedish_holidays(2040).items() if "Påskdagen" in name ) assert easter_sunday in red_days + + +# --------------------------------------------------------------------------- +# `strict=True` — defense in depth for callers that must never silently hit +# the fallback (currently `CalendarService`; see the calendar-endpoint bug +# where years past the cache silently degraded to this same fallback). +# --------------------------------------------------------------------------- + + +class TestStrictMode: + def test_strict_raises_for_a_year_past_the_cache(self) -> None: + bounds = cached_year_bounds() + assert bounds is not None + _, max_year = bounds + + with pytest.raises(UncachedYearError): + get_swedish_holidays(max_year + 1, strict=True) + + def test_strict_raises_for_a_year_before_the_cache(self) -> None: + bounds = cached_year_bounds() + assert bounds is not None + min_year, _ = bounds + + with pytest.raises(UncachedYearError): + get_swedish_holidays(min_year - 1, strict=True) + + def test_strict_succeeds_and_matches_non_strict_for_a_cached_year(self) -> None: + bounds = cached_year_bounds() + assert bounds is not None + min_year, _ = bounds + + assert get_swedish_holidays(min_year, strict=True) == get_swedish_holidays(min_year) + + def test_strict_still_raises_even_if_the_year_was_already_cached_non_strict(self) -> None: + """A prior non-strict call for an uncached year must not poison the + per-year memo cache so a later strict call is fooled into thinking + it's fine.""" + bounds = cached_year_bounds() + assert bounds is not None + _, max_year = bounds + uncached_year = max_year + 5 + + get_swedish_holidays(uncached_year) # warms the non-strict cache + with pytest.raises(UncachedYearError): + get_swedish_holidays(uncached_year, strict=True) + + def test_get_red_days_forwards_strict(self) -> None: + bounds = cached_year_bounds() + assert bounds is not None + _, max_year = bounds + + with pytest.raises(UncachedYearError): + get_red_days(max_year + 1, strict=True) diff --git a/frontend/src/components/calendar/RedDaysList.test.tsx b/frontend/src/components/calendar/RedDaysList.test.tsx index 08d2f96..36fd9fe 100644 --- a/frontend/src/components/calendar/RedDaysList.test.tsx +++ b/frontend/src/components/calendar/RedDaysList.test.tsx @@ -118,6 +118,27 @@ describe('RedDaysList', () => { expect(onNextWindow).toHaveBeenCalledTimes(1) }) + it('uses the actual endYear from the backend instead of startYear + years - 1 when the window was clamped', () => { + // Regression test: the backend clamps a window whose requested end + // exceeds the cached range (e.g. paging to the last page) and reports + // the ACTUAL end year via `endYear`. Without honoring it, this component + // would render extra year headers (2036, 2037) the backend never + // returned data for, showing a false "no red days" for years that are + // simply outside the cache rather than genuinely holiday-free. + render( + , + ) + expect(screen.getByTestId('red-days-range')).toHaveTextContent('2034–2035') + expect(screen.queryByTestId('red-days-year-2036')).not.toBeInTheDocument() + expect(screen.queryByTestId('red-days-year-2037')).not.toBeInTheDocument() + }) + it('shows a loading message and hides year groups while loading', () => { render() expect(screen.getByText(/Laddar röda dagar/)).toBeInTheDocument() diff --git a/frontend/src/components/calendar/RedDaysList.tsx b/frontend/src/components/calendar/RedDaysList.tsx index dcbb3c1..9500641 100644 --- a/frontend/src/components/calendar/RedDaysList.tsx +++ b/frontend/src/components/calendar/RedDaysList.tsx @@ -17,6 +17,17 @@ export interface RedDaysListProps { days: CalendarDayEntry[] startYear: number years?: number + /** + * The window's ACTUAL end year, as reported by `GET /api/calendar/days` + * (`CalendarWindow.endYear`). The backend clamps its window to the cached + * range, so the last page can span fewer than `years` years — pass this + * through so the header/year-groups never claim years the backend didn't + * return (which would otherwise render as false "no red days" for years + * past the cache, e.g. once paging reaches `maxYear`). Falls back to the + * naive `startYear + years - 1` when omitted, e.g. before the first fetch + * resolves and there's no response to read it from yet. + */ + endYear?: number | undefined minYear: number | null maxYear: number | null isLoading?: boolean @@ -29,6 +40,7 @@ export function RedDaysList({ days, startYear, years = 4, + endYear: endYearProp, minYear, maxYear, isLoading = false, @@ -37,7 +49,7 @@ export function RedDaysList({ onNextWindow, }: RedDaysListProps): ReactNode { const { t } = useTranslation() - const endYear = startYear + years - 1 + const endYear = endYearProp ?? startYear + years - 1 const byYear = useMemo(() => { const map = new Map() diff --git a/frontend/src/pages/Settings.tsx b/frontend/src/pages/Settings.tsx index 35256aa..8b0c164 100644 --- a/frontend/src/pages/Settings.tsx +++ b/frontend/src/pages/Settings.tsx @@ -383,6 +383,7 @@ export function Settings(): ReactNode { days={calendarWindow?.days ?? []} startYear={redDaysStartYear} years={RED_DAYS_WINDOW_YEARS} + endYear={calendarWindow?.endYear} minYear={calendarWindow?.minYear ?? null} maxYear={calendarWindow?.maxYear ?? null} isLoading={calendarLoading}