Fix _Timestamp edge cases (danielgtaylor#534)

lukasbindreiter · Lukas Bindreiter · Gobot1234 · web-flow · commit 02aa4e88b795 · 2023-10-18T00:41:09.000+01:00
* Add failing test cases for timestamp conversion

* Fix timestamp to datetime conversion

* Fix formatting

* Move timestamp tests outside of inputs folder

---------

Co-authored-by: Lukas Bindreiter &lt;lukas.bindreiter@tilebox.io&gt;
Co-authored-by: James Hilton-Balfe &lt;gobot1234yt@gmail.com&gt;
diff --git a/src/betterproto/__init__.py b/src/betterproto/__init__.py
@@ -1908,15 +1908,24 @@ def delta_to_json(delta: timedelta) -> str:
 class _Timestamp(Timestamp):
     @classmethod
     def from_datetime(cls, dt: datetime) -> "_Timestamp":
-        # apparently 0 isn't a year in [0, 9999]??
-        seconds = int((dt - DATETIME_ZERO).total_seconds())
-        nanos = int(dt.microsecond * 1e3)
-        return cls(seconds, nanos)
+        # manual epoch offset calulation to avoid rounding errors,
+        # to support negative timestamps (before 1970) and skirt
+        # around datetime bugs (apparently 0 isn't a year in [0, 9999]??)
+        offset = dt - DATETIME_ZERO
+        # below is the same as timedelta.total_seconds() but without dividing by 1e6
+        # so we end up with microseconds as integers instead of seconds as float
+        offset_us = (
+            offset.days * 24 * 60 * 60 + offset.seconds
+        ) * 10**6 + offset.microseconds
+        seconds, us = divmod(offset_us, 10**6)
+        return cls(seconds, us * 1000)
 
     def to_datetime(self) -> datetime:
-        ts = self.seconds + (self.nanos / 1e9)
-        # if datetime.fromtimestamp ever supports -62135596800 use that instead see #407
-        return DATETIME_ZERO + timedelta(seconds=ts)
+        # datetime.fromtimestamp() expects a timestamp in seconds, not microseconds
+        # if we pass it as a floating point number, we will run into rounding errors
+        # see also #407
+        offset = timedelta(seconds=self.seconds, microseconds=self.nanos // 1000)
+        return DATETIME_ZERO + offset
 
     @staticmethod
     def timestamp_to_json(dt: datetime) -> str:
diff --git a/tests/test_timestamp.py b/tests/test_timestamp.py
@@ -0,0 +1,27 @@
+from datetime import (
+    datetime,
+    timezone,
+)
+
+import pytest
+
+from betterproto import _Timestamp
+
+
+@pytest.mark.parametrize(
+    "dt",
+    [
+        datetime(2023, 10, 11, 9, 41, 12, tzinfo=timezone.utc),
+        datetime.now(timezone.utc),
+        # potential issue with floating point precision:
+        datetime(2242, 12, 31, 23, 0, 0, 1, tzinfo=timezone.utc),
+        # potential issue with negative timestamps:
+        datetime(1969, 12, 31, 23, 0, 0, 1, tzinfo=timezone.utc),
+    ],
+)
+def test_timestamp_to_datetime_and_back(dt: datetime):
+    """
+    Make sure converting a datetime to a protobuf timestamp message
+    and then back again ends up with the same datetime.
+    """
+    assert _Timestamp.from_datetime(dt).to_datetime() == dt
