Auto-add and naive datetimes not offsetting timezone correctly

[jacobg]

Here's some unit tests that check retrieved datetime values from the DateTimeProperty:

class TestDateTimeTimezone:

    @staticmethod
    def _now():
        return datetime.now(timezone.utc)

    @classmethod
    def _assert_datetime_is_current(cls, dt):
        print(f'_assert_datetime_is_current dt = {dt}')
        now = cls._now()
        assert abs(dt - cls._now()) < timedelta(seconds=3), \
            f"Provided datetime {dt} is not close to current datetime {now}"

    def test_ndb_gets_auto_add_datetime_in_same_timezone(self):
        class ModelWithDateTime(ndb.Model):
            dt = ndb.DateTimeProperty(auto_now_add=True, tzinfo=timezone.utc)
        entity = ModelWithDateTime()
        with ndb.Client().context():
            entity.put()
            self._assert_datetime_is_current(entity.dt)
            entity.key.get()
        self._assert_datetime_is_current(entity.dt)

    def test_ndb_gets_manual_naive_datetime_field_in_same_timezone(self):
        class ModelWithDateTime(ndb.Model):
            dt = ndb.DateTimeProperty(tzinfo=timezone.utc)
        entity = ModelWithDateTime(dt=datetime.utcnow())
        with ndb.Client().context():
            entity.put()
            self._assert_datetime_is_current(entity.dt)
            entity.key.get()
        self._assert_datetime_is_current(entity.dt)

    def test_ndb_gets_manual_aware_datetime_field_in_same_timezone(self):
        class ModelWithDateTime(ndb.Model):
            dt = ndb.DateTimeProperty(tzinfo=timezone.utc)
        entity = ModelWithDateTime(dt=datetime.now(timezone.utc))
        self._assert_datetime_is_current(entity.dt)
        with ndb.Client().context():
            entity.put()
            self._assert_datetime_is_current(entity.dt)
            entity.key.get()
        self._assert_datetime_is_current(entity.dt)

Here is the test output when running in timezone America/New_York during DST (i.e., UTC -4):

FAILED tests/test_ndb.py::TestDateTimeTimezone::test_ndb_gets_auto_add_datetime_in_same_timezone - AssertionError: Provided datetime 2020-08-21 22:12:53.442799+00:00 is not close to current datetime 2020-08-21 18:12:53.646155+...

FAILED tests/test_ndb.py::TestDateTimeTimezone::test_ndb_gets_manual_naive_datetime_field_in_same_timezone - AssertionError: Provided datetime 2020-08-21 22:13:10.314805+00:00 is not close to current datetime 2020-08-21 18:13:...

The first two tests, auto-add and manually set naive datetime, fail. They both seem to do a double offset of 4 hours from the local system timezone. The last test is a manually set aware datetime, and it passes.

[jacobg]

Perhaps a hint to the bug here:

>>> datetime.datetime.utcnow()
datetime.datetime(2020, 8, 21, 17, 57, 23, 479409)
>>> datetime.datetime.utcnow().astimezone(datetime.timezone.utc)
datetime.datetime(2020, 8, 21, 21, 57, 24, 355568, tzinfo=datetime.timezone.utc)

Similar logic is in both _from_base_type and _to_base_type.

[jacobg]

Here's another implementation of the test class above that parametrizes the inputs, and also makes it easier to subclass it in order to test a fixed version of the DateTimeProperty:

from datetime import datetime, timedelta, timezone

import pytest
from google.cloud import ndb

class TestDateTimeTimezone:
    DATE_TIME_PROPERTY = ndb.DateTimeProperty

    @classmethod
    def _model(cls):
        class ModelWithDateTime(ndb.Model):
            dt = cls.DATE_TIME_PROPERTY(auto_now_add=True, tzinfo=timezone.utc)
        return ModelWithDateTime

    @staticmethod
    def _now():
        return datetime.now(timezone.utc)

    @classmethod
    def _assert_datetime_is_current(cls, dt):
        now = cls._now()
        assert abs(dt - cls._now()) < timedelta(seconds=3), \
            f"Provided datetime {dt} is not close to current datetime {now}"

    @pytest.mark.parametrize("kwarg_factory", [
        lambda: dict(),
        lambda: dict(dt=datetime.utcnow()),
        lambda: dict(dt=datetime.now(timezone.utc)),
    ])
    def test_ndb_datetime_correct(self, kwarg_factory):
        entity = self._model()(**kwarg_factory())
        with ndb.Client().context():
            entity.put()
            self._assert_datetime_is_current(entity.dt)
            entity.key.get()
        self._assert_datetime_is_current(entity.dt)

This is its test output:

====================================================================================================== short test summary info ======================================================================================================
FAILED tests/test_ndb.py::TestDateTimeTimezone::test_ndb_datetime_correct[<lambda>0] - AssertionError: Provided datetime 2020-08-21 22:24:35.435881+00:00 is not close to current datetime 2020-08-21 18:24:35.607341+00:00
FAILED tests/test_ndb.py::TestDateTimeTimezone::test_ndb_datetime_correct[<lambda>1] - AssertionError: Provided datetime 2020-08-21 22:24:36.151383+00:00 is not close to current datetime 2020-08-21 18:24:36.497620+00:00
============================================================================================= 2 failed, 1 passed, 48 warnings in 3.76s ==============================================================================================
E

[jacobg]

Here is the fix using a subclassed DateTimeProperty:

class DateTimeProperty(ndb.DateTimeProperty):
    """This class fixes a bug with timezone offsets when using the auto_add
       kwarg feature, or when setting a naive datetime.
       https://github.com/googleapis/python-ndb/issues/517
       """

    def _to_base_type(self, value):
        if self._tzinfo is not None:
            import pytz
            if value.tzinfo is not None:
                return value.astimezone(pytz.utc)
            else:
                return value.replace(tzinfo=pytz.utc)

Here is the test class subclasses from the test class in my last comment:

class TestDateTimePropertyFix(test_ndb.TestDateTimeTimezone):
    DATE_TIME_PROPERTY = properties.DateTimeProperty