Merge pull request #356 from spoo-me/fix/link-scheduling-datetimes

fix(links): normalise scheduling datetimes before comparing

Author: Zingzy

infrastructure/cache/url_cache.py

@@ -14,7 +14,7 @@
 
 from infrastructure.crypto import verify_password as verify_password_hash
 from infrastructure.logging import get_logger
-from shared.datetime_utils import to_unix_timestamp
+from shared.datetime_utils import to_unix_timestamp, to_unix_timestamp_ceil
 
 if TYPE_CHECKING:
     from schemas.models.url import UrlV2Doc
@@ -88,7 +88,8 @@ def from_v2_doc(cls, doc: UrlV2Doc) -> UrlCacheData:
             meta_color=doc.meta_tags.color if doc.meta_tags else None,
             meta_image_width=im.width if im else None,
             meta_image_height=im.height if im else None,
-            start_time=to_unix_timestamp(doc.starts_at),
+            # Ceil: a fractional start must never read as live a moment early.
+            start_time=to_unix_timestamp_ceil(doc.starts_at),
             pre_start_url=doc.pre_start_url,
         )
 

services/url_service.py

@@ -89,7 +89,7 @@
     resolution_order,
     v2_lookup_code,
 )
-from shared.datetime_utils import parse_datetime, to_unix_timestamp
+from shared.datetime_utils import as_aware_utc, parse_datetime, to_unix_timestamp
 from shared.emoji_policy import (
     canonicalize_emoji_alias,
     check_emoji_alias,
@@ -321,6 +321,9 @@ def _check_start_before_expiry(
     starts_at: datetime | None, expire_after: datetime | None
 ) -> None:
     """A link that expires before it starts would never be live."""
+    # Stored values come back naive-UTC; the request side is aware.
+    starts_at = as_aware_utc(starts_at)
+    expire_after = as_aware_utc(expire_after)
     if starts_at is not None and expire_after is not None and starts_at >= expire_after:
         raise ValidationError(
             "starts_at must be before expire_after", field="starts_at"

shared/datetime_utils.py

@@ -8,6 +8,7 @@
 
 from __future__ import annotations
 
+import math
 from datetime import datetime, timezone
 from typing import Any
 
@@ -62,6 +63,19 @@ def to_unix_timestamp(dt: datetime | None, default: int | None = None) -> int |
     return int(dt.timestamp())
 
 
+def to_unix_timestamp_ceil(dt: datetime | None) -> int | None:
+    """Like ``to_unix_timestamp`` but rounds a fractional second UP.
+
+    For a moment something starts: truncating would let it read as begun
+    up to 999 ms early; rounding up errs on the side of not yet.
+    """
+    if dt is None:
+        return None
+    if dt.tzinfo is None:
+        dt = dt.replace(tzinfo=timezone.utc)
+    return math.ceil(dt.timestamp())
+
+
 def as_aware_utc(dt: Any) -> datetime | None:
     """Normalize a stored datetime for comparison — Mongo returns naive
     UTC datetimes by default (the client is not ``tz_aware``)."""

tests/unit/infrastructure/test_cache.py

@@ -412,3 +412,28 @@ def test_from_v2_doc_carries_start_and_fallback(self):
         data = UrlCacheData.from_v2_doc(doc)
         assert data.start_time == int(starts.replace(tzinfo=timezone.utc).timestamp())
         assert data.pre_start_url == "https://example.org/soon"
+
+
+class TestFromV2DocFractionalStart:
+    def test_fractional_start_rounds_up_so_the_link_is_never_live_early(self):
+        from datetime import datetime, timezone
+
+        from schemas.models.url import UrlV2Doc
+
+        starts = datetime(2030, 6, 1, 12, 0, 0, 900_000, tzinfo=timezone.utc)
+        doc = UrlV2Doc.from_mongo(
+            {
+                "_id": ObjectId(),
+                "alias": "abc1234",
+                "domain": "spoo.me",
+                "created_at": starts,
+                "long_url": "https://example.com",
+                "starts_at": starts,
+            }
+        )
+        data = UrlCacheData.from_v2_doc(doc)
+        whole = int(starts.replace(microsecond=0).timestamp())
+        assert data.start_time == whole + 1
+        # At the floor second the link is still not live; one second on it is.
+        assert data.is_not_yet_live(whole) is True
+        assert data.is_not_yet_live(whole + 1) is False

tests/unit/services/test_url_service.py

@@ -3872,7 +3872,9 @@ async def test_cache_hit_past_start_resolves(self):
     async def test_db_path_future_start_raises_after_recache(self):
         svc, url_repo, url_cache = self._svc()
         url_cache.get.return_value = None
-        future = datetime.now(timezone.utc) + timedelta(hours=1)
+        future = (datetime.now(timezone.utc) + timedelta(hours=1)).replace(
+            microsecond=0
+        )
         url_repo.find_by_alias.return_value = make_url_v2_doc(starts_at=future)
 
         with pytest.raises(NotYetLiveError) as exc:
@@ -4002,6 +4004,36 @@ async def test_update_start_after_existing_expiry_rejected(self):
                 URL_OID, UpdateUrlRequest(starts_at=self.FUTURE_TS + 60), USER_OID
             )
 
+    @pytest.mark.asyncio
+    async def test_update_start_against_naive_stored_expiry_is_a_400_not_a_500(self):
+        """Mongo hands back naive UTC; the request side is aware. The order
+        check must normalise both or the comparison raises TypeError."""
+        svc, url_repo, _url_cache = self._svc()
+        from schemas.dto.requests.url import UpdateUrlRequest
+
+        naive_expiry = datetime.fromtimestamp(self.FUTURE_TS, tz=timezone.utc).replace(
+            tzinfo=None
+        )
+        url_repo.find_by_id.return_value = make_url_v2_doc(expire_after=naive_expiry)
+        with pytest.raises(ValidationError, match="before expire_after"):
+            await svc.update(
+                URL_OID, UpdateUrlRequest(starts_at=self.FUTURE_TS + 60), USER_OID
+            )
+
+    @pytest.mark.asyncio
+    async def test_update_expiry_against_naive_stored_start_is_a_400_not_a_500(self):
+        svc, url_repo, _url_cache = self._svc()
+        from schemas.dto.requests.url import UpdateUrlRequest
+
+        naive_start = datetime.fromtimestamp(self.FUTURE_TS, tz=timezone.utc).replace(
+            tzinfo=None
+        )
+        url_repo.find_by_id.return_value = make_url_v2_doc(starts_at=naive_start)
+        with pytest.raises(ValidationError, match="before expire_after"):
+            await svc.update(
+                URL_OID, UpdateUrlRequest(expire_after=self.FUTURE_TS - 60), USER_OID
+            )
+
     @pytest.mark.asyncio
     async def test_update_unchanged_start_is_a_noop(self):
         svc, url_repo, _url_cache = self._svc()

tests/unit/shared/test_datetime_utils.py

@@ -4,7 +4,12 @@
 
 import pytest
 
-from shared.datetime_utils import convert_to_gmt, parse_datetime
+from shared.datetime_utils import (
+    convert_to_gmt,
+    parse_datetime,
+    to_unix_timestamp,
+    to_unix_timestamp_ceil,
+)
 
 # ---------------------------------------------------------------------------
 # shared.datetime_utils — parse_datetime
@@ -69,3 +74,32 @@ def test_parse_datetime_naive_assumed_utc():
 )
 def test_convert_to_gmt(value, expected):
     assert convert_to_gmt(value) == expected
+
+
+# ---------------------------------------------------------------------------
+# shared.datetime_utils — to_unix_timestamp_ceil
+# ---------------------------------------------------------------------------
+
+
+_WHOLE = datetime(2030, 6, 1, 12, 0, 0, tzinfo=timezone.utc)
+_WHOLE_TS = int(_WHOLE.timestamp())
+
+
+@pytest.mark.parametrize(
+    "value, expected",
+    [
+        (None, None),
+        (_WHOLE, _WHOLE_TS),
+        (_WHOLE.replace(microsecond=900_000), _WHOLE_TS + 1),
+        (_WHOLE.replace(microsecond=1), _WHOLE_TS + 1),
+        # Naive is UTC, the same rule to_unix_timestamp applies.
+        (_WHOLE.replace(microsecond=900_000, tzinfo=None), _WHOLE_TS + 1),
+    ],
+)
+def test_to_unix_timestamp_ceil(value, expected):
+    assert to_unix_timestamp_ceil(value) == expected
+
+
+def test_ceil_agrees_with_floor_on_whole_seconds():
+    whole = datetime(2030, 6, 1, 12, 0, 0, tzinfo=timezone.utc)
+    assert to_unix_timestamp_ceil(whole) == to_unix_timestamp(whole)