From 2cf5859a0bdb8771a73cd8fd65a45da5503cf184 Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Wed, 6 Jul 2022 15:37:01 +0200 Subject: better timezone handling for track dates --- .../alembic/versions/20220706_c89d9bdbfa68.py | 36 ++++++ fietsboek/models/track.py | 124 +++++++++++++++++++-- fietsboek/templates/edit.jinja2 | 2 +- fietsboek/templates/edit_form.jinja2 | 3 +- fietsboek/templates/finish_upload.jinja2 | 2 +- fietsboek/util.py | 104 ++++++++++++++++- fietsboek/views/edit.py | 4 + fietsboek/views/upload.py | 13 ++- 8 files changed, 272 insertions(+), 16 deletions(-) create mode 100644 fietsboek/alembic/versions/20220706_c89d9bdbfa68.py diff --git a/fietsboek/alembic/versions/20220706_c89d9bdbfa68.py b/fietsboek/alembic/versions/20220706_c89d9bdbfa68.py new file mode 100644 index 0000000..83b2978 --- /dev/null +++ b/fietsboek/alembic/versions/20220706_c89d9bdbfa68.py @@ -0,0 +1,36 @@ +"""Add timezone information to tracks + +Revision ID: c89d9bdbfa68 +Revises: 1b4b1c179e5a +Create Date: 2022-07-06 14:05:15.431716 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'c89d9bdbfa68' +down_revision = '1b4b1c179e5a' +branch_labels = None +depends_on = None + +def upgrade(): + op.alter_column('track_cache', 'start_time', new_column_name='start_time_raw') + op.alter_column('track_cache', 'end_time', new_column_name='end_time_raw') + op.add_column('track_cache', sa.Column('start_time_tz', sa.Integer(), nullable=True)) + op.add_column('track_cache', sa.Column('end_time_tz', sa.Integer(), nullable=True)) + op.alter_column('tracks', 'date', new_column_name='date_raw') + op.add_column('tracks', sa.Column('date_tz', sa.Integer(), nullable=True)) + + op.execute('UPDATE tracks SET date_tz=0;') + op.execute('UPDATE track_cache SET start_time_tz=0, end_time_tz=0;') + + +def downgrade(): + op.alter_column('tracks', 'date_raw', new_column_name='date') + op.drop_column('tracks', 'date_tz') + op.alter_column('track_cache', 'start_time_raw', new_column_name='start_time') + op.alter_column('track_cache', 'end_time_raw', new_column_name='end_time') + op.drop_column('track_cache', 'start_time_tz') + op.drop_column('track_cache', 'end_time_tz') diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py index 1b297fe..58e9745 100644 --- a/fietsboek/models/track.py +++ b/fietsboek/models/track.py @@ -14,6 +14,7 @@ meta information. import enum import gzip import datetime +import logging from itertools import chain @@ -44,6 +45,9 @@ from .meta import Base from .. import util +LOGGER = logging.getLogger(__name__) + + class Tag(Base): """A tag is a single keyword associated with a track. @@ -95,6 +99,27 @@ track_badge_assoc = Table( Column("badge_id", ForeignKey("badges.id"), primary_key=True), ) +# Some words about timezone handling in saved tracks: +# https://www.youtube.com/watch?v=-5wpm-gesOY +# +# We usually want to store the times and dates in UTC, and then convert them +# later to the user's display timezone. However, this is not quite right for +# tracks, as the time of tracks is what JSR-310 would call a "LocalDateTime" - +# that is, a date and time in the calendar without timezone information. This +# makes sense: Imagine you go on vacation and cycle at 9:00 AM, you don't want +# that to be displayed as a different time once you return. Similarly, you +# would tell "We went on a tour at 9 in the morning", not convert that time to +# the local time of where you are telling the story. +# +# We therefore save the dates of tracks as a local time, but still try to guess +# the UTC offset so that *if needed*, we can get the UTC timestamp. This is +# done by having two fields, one for the "naive" local datetime and one for the +# UTC offset in minutes. In most cases, we don't care too much about the +# offset, as long as the displayed time is alright for the user. +# +# The issue then mainly consists of getting the local time from the GPX file, +# as some of them store timestamps as UTC time. The problem of finding some +# local timestamp is delegated to util.guess_gpx_timestamp(). class Track(Base): """A :class:`Track` represents a single GPX track. @@ -112,8 +137,10 @@ class Track(Base): :vartype title: str :ivar description: Textual description of the track. :vartype description: str - :ivar date: Set date of the track. - :vartype date: datetime.datetime + :ivar date_raw: Set date of the track (naive :class:`~datetime.datetime`). + :vartype date_raw: datetime.datetime + :ivar date_tz: Timezone offset of the locale date in minutes. + :vartype date_tz: int :ivar gpx: Compressed GPX data. :vartype gpx: bytes :ivar visibility: Visibility of the track. @@ -138,7 +165,8 @@ class Track(Base): owner_id = Column(Integer, ForeignKey('users.id')) title = Column(Text) description = Column(Text) - date = Column(DateTime) + date_raw = Column(DateTime) + date_tz = Column(Integer) gpx = Column(LargeBinary) visibility = Column(Enum(Visibility)) link_secret = Column(Text) @@ -227,6 +255,32 @@ class Track(Base): def gpx_data(self, value): self.gpx = gzip.compress(value) + @property + def date(self): + """The time-zone-aware date this track has set. + + This combines the :attr:`date_raw` and :attr:`date_tz` values to + provide a timezone aware :class:`~datetime.datetime`. + + :return: The aware datetime. + :rtype: datetime.datetime + """ + if self.date_tz is None: + timezone = datetime.timezone.utc + else: + timezone = datetime.timezone(datetime.timedelta(minutes=self.date_tz)) + return self.date_raw.replace(tzinfo=timezone) + + @date.setter + def date(self, value): + if value.tzinfo is None: + LOGGER.debug('Non-aware datetime passed (track_id=%d, value=%s), assuming offset=0', + self.id, value) + self.date_tz = 0 + else: + self.date_tz = value.tzinfo.utcoffset(value).total_seconds() // 60 + self.date_raw = value.replace(tzinfo=None) + def is_visible_to(self, user): """Checks whether the track is visible to the given user. @@ -477,10 +531,14 @@ class TrackCache(Base): :vartype max_speed: float :ivar avg_speed: Average speed, in meters/second. :vartype avg_speed: float - :ivar start_time: Start time of the GPX recording. - :vartype start_time: datetime.datetime - :ivar end_time: End time of the GPX recording. - :vartype end_time: datetime.datetime + :ivar start_time_raw: Start time of the GPX recording. + :vartype start_time_raw: datetime.datetime + :ivar start_time_tz: Timezone offset of the start time in minutes. + :vartype start_time_tz: int + :ivar end_time_raw: End time of the GPX recording. + :vartype end_time_raw: datetime.datetime + :ivar end_time_tz: Timezone offset of the end time in minutes. + :vartype end_time_tz: int :ivar track: The track that belongs to this cache entry. :vartype track: Track """ @@ -494,11 +552,59 @@ class TrackCache(Base): stopped_time = Column(Float) max_speed = Column(Float) avg_speed = Column(Float) - start_time = Column(DateTime) - end_time = Column(DateTime) + start_time_raw = Column(DateTime) + start_time_tz = Column(Integer) + end_time_raw = Column(DateTime) + end_time_tz = Column(Integer) track = relationship('Track', back_populates='cache') + @property + def start_time(self): + """The time-zone-aware start time of this track. + + :return: The aware datetime. + :rtype: datetime.datetime + """ + if self.start_time_tz is None: + timezone = datetime.timezone.utc + else: + timezone = datetime.timezone(datetime.timedelta(minutes=self.start_time_tz)) + return self.start_time_raw.replace(tzinfo=timezone) + + @start_time.setter + def start_time(self, value): + if value.tzinfo is None: + LOGGER.debug('Non-aware datetime passed (cache_id=%d, value=%s), assuming offset=0', + self.id, value) + self.start_time_tz = 0 + else: + self.start_time_tz = value.tzinfo.utcoffset(value).total_seconds() // 60 + self.start_time_raw = value.replace(tzinfo=None) + + @property + def end_time(self): + """The time-zone-aware end time of this track. + + :return: The aware datetime. + :rtype: datetime.datetime + """ + if self.end_time_tz is None: + timezone = datetime.timezone.utc + else: + timezone = datetime.timezone(datetime.timedelta(minutes=self.end_time_tz)) + return self.end_time_raw.replace(tzinfo=timezone) + + @end_time.setter + def end_time(self, value): + if value.tzinfo is None: + LOGGER.debug('Non-aware datetime passed (cache_id=%d, value=%s), assuming offset=0', + self.id, value) + self.end_time_tz = 0 + else: + self.end_time_tz = value.tzinfo.utcoffset(value).total_seconds() // 60 + self.end_time_raw = value.replace(tzinfo=None) + class Upload(Base): """A track that is currently being uploaded. diff --git a/fietsboek/templates/edit.jinja2 b/fietsboek/templates/edit.jinja2 index fd26f6b..9d48cf1 100644 --- a/fietsboek/templates/edit.jinja2 +++ b/fietsboek/templates/edit.jinja2 @@ -9,7 +9,7 @@