fix: unify naive datetime treatment to assume UTC

(cherry-picked from c6dd3d7.
I've included the refactoring mentioned in python-telegram-bot#1497 to facilitate the
change.)

There was inconsistent use of UTC vs local times. For instance, in the
former `_timestamp` helper (now `_datetime_to_float_timestamp`), assumed
that naive `datetime.datetime` objects were in the local timezone, while
the `from_timestamp` helper —which I would have thought was the
corresponding inverse function— returned naïve objects in UTC.

This meant that, for instance, `telegram.Message` objects' `date` field
was constructed as a naïve `datetime.datetime` (from the timestamp sent
by Telegram's server) in *UTC*, but when it was stored in `JSON` format
through the `to_json` method, the naïve `date` would be assumed to be in
*local time*, thus generating a different timestamp from the one it was
built from.

See python-telegram-bot#1505 for extended discussion.

telegram/ext/jobqueue.py

@@ -29,6 +29,7 @@
 
 from telegram.ext.callbackcontext import CallbackContext
 from telegram.utils.deprecate import TelegramDeprecationWarning
+from telegram.utils.helpers import to_float_timestamp
 
 
 class Days(object):
@@ -70,30 +71,25 @@ def __init__(self):
     def set_dispatcher(self, dispatcher):
         self._dispatcher = dispatcher
 
-    def _put(self, job, next_t=None, last_t=None):
-        if next_t is None:
-            next_t = job.interval
-            if next_t is None:
-                raise ValueError('next_t is None')
-
-        if isinstance(next_t, datetime.datetime):
-            next_t = (next_t - datetime.datetime.now()).total_seconds()
-
-        elif isinstance(next_t, datetime.time):
-            next_datetime = datetime.datetime.combine(datetime.date.today(), next_t)
-
-            if datetime.datetime.now().time() > next_t:
-                next_datetime += datetime.timedelta(days=1)
-
-            next_t = (next_datetime - datetime.datetime.now()).total_seconds()
+    def _put(self, job, next_t=None, previous_t=None):
+        """
+        Enqueues the job, scheduling its next run at the correct time.
 
-        elif isinstance(next_t, datetime.timedelta):
-            next_t = next_t.total_seconds()
+        Args:
+            job (telegram.ext.Job): job to enqueue
+            next_t (optional):
+                Time for which the job should be scheduled. The precise semantics of this parameter
+                depend on its type (see :func:`telegram.ext.JobQueue.run_repeating` for details).
+                Defaults to now + ``job.interval``.
+            previous_t (optional):
+                Time at which the job last ran (``None`` if it hasn't run yet).
+        """
 
-        next_t += last_t or time.time()
+        # get time at which to run:
+        next_t = to_float_timestamp(next_t or job.interval, reference_timestamp=previous_t)
 
+        # enqueue:
         self.logger.debug('Putting job %s with t=%f', job.name, next_t)
-
         self._queue.put((next_t, job))
 
         # Wake up the loop if this job should be executed next
@@ -276,7 +272,7 @@ def tick(self):
                 self.logger.debug('Skipping disabled job %s', job.name)
 
             if job.repeat and not job.removed:
-                self._put(job, last_t=t)
+                self._put(job, previous_t=t)
             else:
                 self.logger.debug('Dropping non-repeating or removed job %s', job.name)
 
@@ -379,7 +375,7 @@ def __init__(self,
         self.context = context
         self.name = name or callback.__name__
 
-        self._repeat = repeat
+        self._repeat = None
         self._interval = None
         self.interval = interval
         self.repeat = repeat

telegram/utils/helpers.py

@@ -17,7 +17,12 @@
 # You should have received a copy of the GNU Lesser Public License
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
 """This module contains helper functions."""
+
+import datetime as dtm  # dtm = "DateTime Module"
+import time
+
 from collections import defaultdict
+from numbers import Number
 
 try:
     import ujson as json
@@ -27,7 +32,6 @@
 
 import re
 import signal
-from datetime import datetime
 
 # From https://stackoverflow.com/questions/2549939/get-signal-names-from-numbers-in-python
 _signames = {v: k
@@ -40,54 +44,148 @@ def get_signal_name(signum):
     return _signames[signum]
 
 
+def escape_markdown(text):
+    """Helper function to escape telegram markup symbols."""
+    escape_chars = '\*_`\['
+    return re.sub(r'([%s])' % escape_chars, r'\\\1', text)
+
+
+# -------- date/time related helpers --------
+
+# hardcoded UTC timezone object (`datetime.timezone` isn't available in py2)
+def _utc():
+    # writing as internal class to "enforce" singleton
+    class UTCClass(dtm.tzinfo):
+        def tzname(self, dt):
+            return 'UTC'
+
+        def utcoffset(self, dt):
+            return dtm.timedelta(0)
+
+        def dst(self, dt):
+            return dtm.timedelta(0)
+
+    return UTCClass()
+
+
+# select UTC datetime.tzinfo object based on python version
+UTC = dtm.timezone.utc if hasattr(dtm, 'timezone') else _utc()
+
+# _datetime_to_float_timestamp
 # Not using future.backports.datetime here as datetime value might be an input from the user,
 # making every isinstace() call more delicate. So we just use our own compat layer.
-if hasattr(datetime, 'timestamp'):
+if hasattr(dtm.datetime, 'timestamp'):
     # Python 3.3+
-    def _timestamp(dt_obj):
+    def _datetime_to_float_timestamp(dt_obj):
+        if dt_obj.tzinfo is None:
+            dt_obj = dt_obj.replace(tzinfo=UTC)
         return dt_obj.timestamp()
 else:
     # Python < 3.3 (incl 2.7)
-    from time import mktime
+    EPOCH_DT = dtm.datetime.fromtimestamp(0, tz=UTC)
+    NAIVE_EPOCH_DT = EPOCH_DT.replace(tzinfo=None)
 
-    def _timestamp(dt_obj):
-        return mktime(dt_obj.timetuple())
+    def _datetime_to_float_timestamp(dt_obj):
+        epoch_dt = EPOCH_DT if dt_obj.tzinfo is not None else NAIVE_EPOCH_DT
+        return (dt_obj - epoch_dt).total_seconds()
 
 
-def escape_markdown(text):
-    """Helper function to escape telegram markup symbols."""
-    escape_chars = '\*_`\['
-    return re.sub(r'([%s])' % escape_chars, r'\\\1', text)
+_datetime_to_float_timestamp.__doc__ = \
+    """Converts a datetime object to a float timestamp (with sub-second precision).
+If the datetime object is timezone-naive, it is assumed to be in UTC."""
 
 
-def to_timestamp(dt_obj):
+def to_float_timestamp(t, reference_timestamp=None):
     """
+    Converts a given time object to a float POSIX timestamp.
+    Used to convert different time specifications to a common format. The time object
+    can be relative (i.e. indicate a time increment, or a time of day) or absolute.
+    Any objects from the :module:`datetime` module that are timezone-naive will be assumed
+    to be in UTC.
+
+    ``None`` s are left alone (i.e. ``to_float_timestamp(None)`` is ``None``).
+
     Args:
-        dt_obj (:class:`datetime.datetime`):
+        t (int | float | datetime.timedelta | datetime.datetime | datetime.time | None):
+            Time value to convert. The semantics of this parameter will depend on its type:
+
+            * :obj:`int` or :obj:`float` will be interpreted as "seconds from ``reference_t``"
+            * :obj:`datetime.timedelta` will be interpreted as
+              "time increment from ``reference_t``"
+            * :obj:`datetime.datetime` will be interpreted as an absolute date/time value
+            * :obj:`datetime.time` will be interpreted as a specific time of day
+
+        reference_timestamp (float, optional): POSIX timestamp that indicates the absolute time
+            from which relative calculations are to be performed (e.g. when ``t`` is given as an
+            :obj:`int`, indicating "seconds from ``reference_t``"). Defaults to now (the time at
+            which this function is called).
 
     Returns:
-        int:
+        (float | None) The return value depends on the type of argument ``t``. If ``t`` is
+            given as a time increment (i.e. as a obj:`int`, :obj:`float` or
+            :obj:`datetime.timedelta`), then the return value will be ``reference_t`` + ``t``.
 
+            Else if it is given as an absolute date/time value (i.e. a :obj:`datetime.datetime`
+            object), the equivalent value as a POSIX timestamp will be returned.
+
+            Finally, if it is a time of the day without date (i.e. a :obj:`datetime.time`
+            object), the return value is the nearest future occurrence of that time of day.
+
+            If ``t`` is ``None``, ``None`` is returned (to facilitate formulating HTTP requests
+            when the object to be serialized has a ``date`` which is ``None`` without having to
+            check explicitly).
     """
-    if not dt_obj:
+
+    if reference_timestamp is None:
+        reference_timestamp = time.time()
+
+    if t is None:
         return None
+    elif isinstance(t, dtm.timedelta):
+        return reference_timestamp + t.total_seconds()
+    elif isinstance(t, Number):
+        return reference_timestamp + t
+    elif isinstance(t, dtm.time):
+        reference_dt = dtm.datetime.fromtimestamp(reference_timestamp, tz=t.tzinfo) if t.tzinfo \
+            else dtm.datetime.utcfromtimestamp(reference_timestamp)  # assume UTC for naive
+        reference_date, reference_time = reference_dt.date(), reference_dt.timetz()
+        if reference_time > t:  # if the time of day has passed today, use tomorrow
+            reference_date += dtm.timedelta(days=1)
+        t = dtm.datetime.combine(reference_date, t)
 
-    return int(_timestamp(dt_obj))
+    if isinstance(t, dtm.datetime):
+        return _datetime_to_float_timestamp(t)
 
+    raise TypeError('Unable to convert {} object to timestamp'.format(type(t).__name__))
 
-def from_timestamp(unixtime):
+
+def to_timestamp(t, *args, **kwargs):
+    """
+    Converts a time object to an integer UNIX timestamp.
+    Returns the corresponding float timestamp truncated down to the nearest integer.
+    See the documentation for :func:`to_float_timestamp` for more details.
+    """
+    return int(to_float_timestamp(t, *args, **kwargs)) if t is not None else None
+
+
+def from_timestamp(timestamp):
     """
+    Converts an (integer) unix timestamp to a naive datetime object in UTC.
+    ``None`` s are left alone (i.e. ``from_timestamp(None)`` is ``None``).
+
     Args:
-        unixtime (int):
+        timestamp (int): integer POSIX timestamp
 
     Returns:
-        datetime.datetime:
-
+        equivalent :obj:`datetime.datetime` value in naive UTC if ``timestamp`` is not
+        ``None``; else ``None``
     """
-    if not unixtime:
+    if timestamp is None:
         return None
 
-    return datetime.utcfromtimestamp(unixtime)
+    return dtm.datetime.utcfromtimestamp(timestamp)
+
+# -------- end --------
 
 
 def mention_html(user_id, name):

tests/test_jobqueue.py

@@ -16,7 +16,7 @@
 #
 # You should have received a copy of the GNU Lesser Public License
 # along with this program.  If not, see [http://www.gnu.org/licenses/].
-import datetime
+import datetime as dtm
 import os
 import sys
 import time
@@ -181,7 +181,7 @@ def test_time_unit_int(self, job_queue):
     def test_time_unit_dt_timedelta(self, job_queue):
         # Testing seconds, minutes and hours as datetime.timedelta object
         # This is sufficient to test that it actually works.
-        interval = datetime.timedelta(seconds=0.05)
+        interval = dtm.timedelta(seconds=0.05)
         expected_time = time.time() + interval.total_seconds()
 
         job_queue.run_once(self.job_datetime_tests, interval)
@@ -190,43 +190,43 @@ def test_time_unit_dt_timedelta(self, job_queue):
 
     def test_time_unit_dt_datetime(self, job_queue):
         # Testing running at a specific datetime
-        delta = datetime.timedelta(seconds=0.05)
-        when = datetime.datetime.now() + delta
-        expected_time = time.time() + delta.total_seconds()
+        delta, now = dtm.timedelta(seconds=0.05), time.time()
+        when = dtm.datetime.utcfromtimestamp(now) + delta
+        expected_time = now + delta.total_seconds()
 
         job_queue.run_once(self.job_datetime_tests, when)
         sleep(0.06)
-        assert pytest.approx(self.job_time) == expected_time
+        assert self.job_time == pytest.approx(expected_time)
 
     def test_time_unit_dt_time_today(self, job_queue):
         # Testing running at a specific time today
-        delta = 0.05
-        when = (datetime.datetime.now() + datetime.timedelta(seconds=delta)).time()
-        expected_time = time.time() + delta
+        delta, now = 0.05, time.time()
+        when = (dtm.datetime.utcfromtimestamp(now) + dtm.timedelta(seconds=delta)).time()
+        expected_time = now + delta
 
         job_queue.run_once(self.job_datetime_tests, when)
         sleep(0.06)
-        assert pytest.approx(self.job_time) == expected_time
+        assert self.job_time == pytest.approx(expected_time)
 
     def test_time_unit_dt_time_tomorrow(self, job_queue):
         # Testing running at a specific time that has passed today. Since we can't wait a day, we
         # test if the jobs next_t has been calculated correctly
-        delta = -2
-        when = (datetime.datetime.now() + datetime.timedelta(seconds=delta)).time()
-        expected_time = time.time() + delta + 60 * 60 * 24
+        delta, now = -2, time.time()
+        when = (dtm.datetime.utcfromtimestamp(now) + dtm.timedelta(seconds=delta)).time()
+        expected_time = now + delta + 60 * 60 * 24
 
         job_queue.run_once(self.job_datetime_tests, when)
-        assert pytest.approx(job_queue._queue.get(False)[0]) == expected_time
+        assert job_queue._queue.get(False)[0] == pytest.approx(expected_time)
 
     def test_run_daily(self, job_queue):
-        delta = 0.5
-        time_of_day = (datetime.datetime.now() + datetime.timedelta(seconds=delta)).time()
-        expected_time = time.time() + 60 * 60 * 24 + delta
+        delta, now = 0.5, time.time()
+        time_of_day = (dtm.datetime.utcfromtimestamp(now) + dtm.timedelta(seconds=delta)).time()
+        expected_time = now + 60 * 60 * 24 + delta
 
         job_queue.run_daily(self.job_run_once, time_of_day)
         sleep(0.6)
         assert self.result == 1
-        assert pytest.approx(job_queue._queue.get(False)[0]) == expected_time
+        assert job_queue._queue.get(False)[0] == pytest.approx(expected_time)
 
     def test_warnings(self, job_queue):
         j = Job(self.job_run_once, repeat=False)