From 5ff2bef550768f03ff5b627b94d28f5730559e7a Mon Sep 17 00:00:00 2001 From: imSp4rky Date: Tue, 13 Oct 2026 19:58:50 -0600 Subject: [PATCH] fix(cacher): resolve cache expiry by unix time --- tests/core/test_cacher.py | 100 ++++++++++++++++++++++++++++++++++++++ unshackle/core/cacher.py | 74 +++++++++++++--------------- 2 files changed, 133 insertions(+), 41 deletions(-) create mode 100644 tests/core/test_cacher.py diff --git a/tests/core/test_cacher.py b/tests/core/test_cacher.py new file mode 100644 index 00000000..0b9ebdfa --- /dev/null +++ b/tests/core/test_cacher.py @@ -0,0 +1,100 @@ +import time +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import jsonpickle +import jwt +import pytest + +from unshackle.core import cacher +from unshackle.core.cacher import Cacher + +CLOSE = timedelta(seconds=5) + + +def set_zone(monkeypatch: pytest.MonkeyPatch, zone: str) -> None: + monkeypatch.setenv("TZ", zone) + time.tzset() + + +@pytest.fixture +def zone(monkeypatch: pytest.MonkeyPatch): # type: ignore[no-untyped-def] + """Let a test change the process time zone, and put it back afterwards.""" + if not hasattr(time, "tzset"): + pytest.skip("needs time.tzset") + yield lambda name: set_zone(monkeypatch, name) + monkeypatch.undo() + time.tzset() + + +@pytest.fixture +def cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Cacher: + monkeypatch.setattr(cacher.config.directories, "cache", tmp_path) + return Cacher("TAG") + + +def test_seconds_until_expiration_ignore_the_1970_utc_offset(zone) -> None: # type: ignore[no-untyped-def] + zone("Asia/Kathmandu") + if time.localtime(0).tm_gmtoff == time.localtime().tm_gmtoff: + pytest.skip("zone data missing: the 1970 offset equals the current one") + assert abs(Cacher.resolve_datetime(283) - (datetime.now() + timedelta(seconds=283))) < CLOSE + + +@pytest.mark.parametrize("value", [283, 283.5, "283"]) +def test_small_number_is_seconds_until_expiration(value: object) -> None: + assert abs(Cacher.resolve_datetime(value) - (datetime.now() + timedelta(seconds=283))) < CLOSE # type: ignore[arg-type] + + +@pytest.mark.parametrize("offset", [-3600, 3600]) +def test_unix_timestamp_is_absolute_also_when_it_is_in_the_past(offset: int) -> None: + stamp = int(time.time()) + offset + assert Cacher.resolve_datetime(stamp) == datetime.fromtimestamp(stamp) + assert Cacher.resolve_datetime(stamp * 1000) == datetime.fromtimestamp(stamp) + + +def test_iso_string_with_utc_offset_becomes_naive_local_time() -> None: + moment = datetime.now(timezone.utc).replace(microsecond=0) + timedelta(hours=1) + expected = datetime.fromtimestamp(moment.timestamp()) + assert Cacher.resolve_datetime(moment.isoformat()) == expected + assert Cacher.resolve_datetime(moment.strftime("%Y-%m-%dT%H:%M:%SZ")) == expected + assert Cacher.resolve_datetime(moment) == expected + + +@pytest.mark.parametrize("value", [float("inf"), 10**17, "not a time"]) +def test_unusable_value_raises_value_error(value: object) -> None: + with pytest.raises(ValueError): + Cacher.resolve_datetime(value) # type: ignore[arg-type] + + +def test_zero_seconds_expires_now(cache: Cacher) -> None: + entry = cache.get("key") + entry.set({"a": 1}, expiration=0) + assert entry.expired + + +def test_expired_jwt_is_stored_as_expired(cache: Cacher) -> None: + entry = cache.get("key") + entry.set(jwt.encode({"exp": int(time.time()) - 60}, "k" * 32, algorithm="HS256")) + assert entry.expired + + +def test_expiration_survives_a_change_of_time_zone(cache: Cacher, zone) -> None: # type: ignore[no-untyped-def] + zone("UTC") + cache.get("key").set({"a": 1}, expiration=300) + zone("America/Denver") + entry = cache.get("key") + assert entry.data == {"a": 1} + assert abs(entry.expiration - (datetime.now() + timedelta(seconds=300))) < CLOSE + + +def test_entry_without_expires_at_reads_its_local_expiration(cache: Cacher) -> None: + entry = cache.get("key") + entry.set({"a": 1}, expiration=300) + payload = jsonpickle.loads(entry.path.read_text(encoding="utf8")) + del payload["expires_at"], payload["crc32"] + payload["crc32"] = cacher.zlib.crc32(jsonpickle.dumps(payload).encode("utf8")) + entry.path.write_text(jsonpickle.dumps(payload), encoding="utf8") + + legacy = cache.get("key") + assert not legacy.expired + assert abs(legacy.expiration - (datetime.now() + timedelta(seconds=300))) < CLOSE diff --git a/unshackle/core/cacher.py b/unshackle/core/cacher.py index 36e18268..291bd140 100644 --- a/unshackle/core/cacher.py +++ b/unshackle/core/cacher.py @@ -1,7 +1,8 @@ from __future__ import annotations +import time import zlib -from datetime import datetime, timedelta +from datetime import datetime from os import stat_result from pathlib import Path from typing import Any, Optional, Union @@ -13,6 +14,9 @@ from unshackle.core.config import config EXP_T = Union[datetime, str, int, float] +# A number below this is seconds until expiration; at or above it, a unix timestamp (2001-09-09 onwards). +ABSOLUTE_TIMESTAMP_MIN = 1_000_000_000 + class Cacher: """Cacher for Services to get and set arbitrary data with expiration dates.""" @@ -45,7 +49,8 @@ class Cacher: @property def expired(self) -> bool: - return self.expiration and self.expiration < datetime.now() + """Compare as unix time: wall-clock comparison is wrong for an hour at each DST change.""" + return bool(self.expiration and self.expiration.timestamp() < time.time()) def get(self, key: str, version: int = 1) -> Cacher: """ @@ -66,7 +71,9 @@ class Cacher: f"The checksum of the Cache payload mismatched. Checksum: {checksum} !== Calculated: {calculated}" ) cache.data = data["data"] - cache.expiration = data["expiration"] + # The pickled `expiration` is local wall-clock and only right in the zone that wrote it. + expires_at = data.get("expires_at") + cache.expiration = datetime.fromtimestamp(expires_at) if expires_at is not None else data["expiration"] cache.version = data["version"] if cache.version != version: raise ValueError( @@ -84,15 +91,20 @@ class Cacher: """ self.data = data - if not expiration: + if expiration is None: try: expiration = jwt.decode(self.data, options={"verify_signature": False})["exp"] except jwt.DecodeError: pass - self.expiration = self.resolve_datetime(expiration) if expiration else None + self.expiration = self.resolve_datetime(expiration) if expiration is not None else None - payload = {"data": self.data, "expiration": self.expiration, "version": self.version} + payload = { + "data": self.data, + "expiration": self.expiration, + "expires_at": self.expiration.timestamp() if self.expiration else None, + "version": self.version, + } payload["crc32"] = zlib.crc32(jsonpickle.dumps(payload).encode("utf8")) self.path.parent.mkdir(parents=True, exist_ok=True) @@ -110,46 +122,26 @@ class Cacher: @staticmethod def resolve_datetime(timestamp: EXP_T) -> datetime: """ - Change a Datetime or a Timestamp in one of several formats to an absolute Datetime. + Change an expiration in one of several formats to a naive local datetime. - Examples: - >>> now = datetime.now() - datetime.datetime(2022, 6, 27, 9, 49, 13, 657208) - >>> iso8601 = now.isoformat() - '2022-06-27T09:49:13.657208' - >>> Cacher.resolve_datetime(iso8601) - datetime.datetime(2022, 6, 27, 9, 49, 13, 657208) - >>> Cacher.resolve_datetime(iso8601 + "Z") - datetime.datetime(2022, 6, 27, 9, 49, 13, 657208) - >>> Cacher.resolve_datetime(3600) - datetime.datetime(2022, 6, 27, 10, 52, 50, 657208) - >>> Cacher.resolve_datetime('3600') - datetime.datetime(2022, 6, 27, 10, 52, 51, 657208) - >>> Cacher.resolve_datetime(7800.113) - datetime.datetime(2022, 6, 27, 11, 59, 13, 770208) - - In the int/float examples you may notice that it did not return now + 3600 seconds - but rather something a bit more than that. This is because the 3600 seconds start at - the moment the function operates, not at the `now` variable. + A datetime or an ISO 8601 string is taken as given, and one with a UTC offset + (or a final `Z`) is converted to local time. A number is a unix timestamp, in + seconds or 13-digit milliseconds, or the seconds until expiration when it is + smaller than `ABSOLUTE_TIMESTAMP_MIN`. A numeric string is read as a number. """ - if isinstance(timestamp, datetime): - return timestamp if isinstance(timestamp, str): - if timestamp.endswith("Z"): - # fromisoformat doesn't accept the final Z - timestamp = timestamp.split("Z")[0] try: - return datetime.fromisoformat(timestamp) + timestamp = datetime.fromisoformat(timestamp) except ValueError: - timestamp = float(timestamp) + timestamp = float(str(timestamp)) + if isinstance(timestamp, datetime): + # naive local time is the one form every consumer of `expiration` can do arithmetic with + return timestamp.astimezone().replace(tzinfo=None) if timestamp.tzinfo else timestamp try: - if len(str(int(timestamp))) == 13: # JS-style timestamp + if len(str(abs(int(timestamp)))) == 13: # JS-style timestamp timestamp /= 1000 - timestamp = datetime.fromtimestamp(timestamp) - except ValueError: + if timestamp < ABSOLUTE_TIMESTAMP_MIN: + timestamp += time.time() + return datetime.fromtimestamp(timestamp) + except (ValueError, OverflowError, OSError): raise ValueError(f"Unrecognized Timestamp value {timestamp!r}") - if timestamp < datetime.now(): - # timestamp is likely an amount of seconds til expiration - # or, it's an already expired timestamp which is unlikely - timestamp = timestamp + timedelta(seconds=datetime.now().timestamp()) - return timestamp