diff options
28 files changed, 752 insertions, 70 deletions
diff --git a/doc/index.rst b/doc/index.rst index 9ce95b9..0b69ef1 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -12,6 +12,7 @@ Welcome to Fietsboek's documentation! administration developer + user .. toctree:: :maxdepth: 1 diff --git a/doc/user.rst b/doc/user.rst new file mode 100644 index 0000000..ab210dc --- /dev/null +++ b/doc/user.rst @@ -0,0 +1,12 @@ +User Guide +========== + +.. toctree:: + :maxdepth: 2 + :caption: Contents + + user/transformers + +This is the user guide for Fietsboek! In here, you can find information that +might be interesting if you plan on sharing your tracks on a Fietsboek +instance. diff --git a/doc/user/images/fixed_elevation.png b/doc/user/images/fixed_elevation.png Binary files differnew file mode 100644 index 0000000..10d34c7 --- /dev/null +++ b/doc/user/images/fixed_elevation.png diff --git a/doc/user/images/wrong_elevation.png b/doc/user/images/wrong_elevation.png Binary files differnew file mode 100644 index 0000000..2e8557b --- /dev/null +++ b/doc/user/images/wrong_elevation.png diff --git a/doc/user/transformers.rst b/doc/user/transformers.rst new file mode 100644 index 0000000..c8ddf01 --- /dev/null +++ b/doc/user/transformers.rst @@ -0,0 +1,45 @@ +Transformers +============ + +Transformers are small (or big) transformations that are applied to your GPX +track after it has been uploaded. This allows Fietsboek to provide some common, +simple editing options, without users having to do that themselves. + +All transformers are optional and disabled by default. You can enable +transformers when uploading a track on the bottom of the page. You can also +enable transformers for already uploaded tracks in the editing view. + +.. note:: + + When enabling transformers for already existing tracks, your browser might + not show the changed track. Try force-refreshing (Crtl + F5) the page so + that it reloads the GPX from the server. + +In other applications, transformers are sometimes called "filters". That term +however has many different meanings (like the filters on the "Browse" page), +and as such, Fietsboek calls them transformers. + +Fix Null Elevation +------------------ + +The *fix null elevation* transformer removes points at the start and end of a +track that have a "wrong" elevation. This helps to avoid issues when GPX +trackers don't have elevation data yet and fill in 0, leading to wrong uphill +calculations and wrong diagrams: + +.. image:: images/wrong_elevation.png + :width: 200 + :alt: An elevation graph that starts at 0 and makes a jump to 165. + +Activating the transformer will produce the following track: + +.. image:: images/fixed_elevation.png + :width: 200 + :alt: The same track, but with a fixed elevation graph that starts at 165. + +The transformer considers "wrong" elevation to be either points that have an +elevation of 0, or points that have a slope of more than 100% to the next +point. + +To fix those points, the transformer will find the first correct point, and +copy its elevation to the wrong points. diff --git a/fietsboek/__init__.py b/fietsboek/__init__.py index 98e7b99..41e8a04 100644 --- a/fietsboek/__init__.py +++ b/fietsboek/__init__.py @@ -29,6 +29,7 @@ from pyramid.session import SignedCookieSessionFactory from . import config as mod_config from . import jinja2 as mod_jinja2 +from . import transformers from .data import DataManager from .pages import Pages from .security import SecurityPolicy @@ -145,5 +146,6 @@ def main(_global_config, **settings): jinja2_env.filters["format_datetime"] = mod_jinja2.filter_format_datetime jinja2_env.filters["local_datetime"] = mod_jinja2.filter_local_datetime jinja2_env.globals["embed_tile_layers"] = mod_jinja2.global_embed_tile_layers + jinja2_env.globals["list_transformers"] = transformers.list_transformers return config.make_wsgi_app() diff --git a/fietsboek/actions.py b/fietsboek/actions.py index bba4185..e49e27d 100644 --- a/fietsboek/actions.py +++ b/fietsboek/actions.py @@ -10,13 +10,17 @@ import logging import re from typing import List, Optional +import brotli +import gpxpy from pyramid.request import Request from sqlalchemy import select from sqlalchemy.orm.session import Session -from fietsboek import models, util -from fietsboek.data import DataManager, TrackDataDir -from fietsboek.models.track import TrackType, Visibility +from . import models, util +from . import transformers as mod_transformers +from . import util +from .data import DataManager, TrackDataDir +from .models.track import TrackType, Visibility LOGGER = logging.getLogger(__name__) @@ -33,6 +37,7 @@ def add_track( badges: List[models.Badge], tagged_people: List[models.User], tags: List[str], + transformers: list[mod_transformers.Transformer], gpx_data: bytes, ) -> models.Track: """Adds a track to the database. @@ -54,11 +59,12 @@ def add_track( :param badges: Badges to attach to the track. :param tagged_people: List of people to tag. :param tags: List of text tags for the track. + :param transformers: List of :class:`~fietsboek.transformers.Transformer` to apply. :param gpx_data: Actual GPX data (uncompressed, straight from the source). :return: The track object that has been inserted into the database. Useful for its ``id`` attribute. """ - # pylint: disable=too-many-arguments + # pylint: disable=too-many-arguments,too-many-locals LOGGER.debug("Inserting new track...") track = models.Track( owner=owner, @@ -75,10 +81,6 @@ def add_track( dbsession.add(track) dbsession.flush() - # Best time to build the cache is right after the upload - track.ensure_cache(gpx_data) - dbsession.add(track.cache) - # Save the GPX data LOGGER.debug("Creating a new data folder for %d", track.id) with data_manager.initialize(track.id) as manager: @@ -86,11 +88,23 @@ def add_track( manager.compress_gpx(gpx_data) manager.backup() + gpx = gpxpy.parse(gpx_data) + for transformer in transformers: + LOGGER.debug("Running %s with %r", transformer, transformer.parameters) + transformer.execute(gpx) + track.transformers = [[tfm.identifier(), tfm.parameters.dict()] for tfm in transformers] + + # Best time to build the cache is right after the upload, but *after* the + # transformers have been applied! + track.ensure_cache(gpx) + dbsession.add(track.cache) + manager.engrave_metadata( title=track.title, description=track.description, author_name=track.owner.name, time=track.date, + gpx=gpx, ) return track @@ -156,3 +170,52 @@ def edit_images(request: Request, track: models.Track, *, manager: Optional[Trac image_meta = models.ImageMetadata.get_or_create(request.dbsession, track, image_id) image_meta.description = description request.dbsession.add(image_meta) + + +def execute_transformers(request: Request, track: models.Track) -> Optional[gpxpy.gpx.GPX]: + """Execute the transformers for the given track. + + Note that this function "short circuits" if the saved transformer settings + already match the settings given in the request. + + This function saves the modified data, but does also return it in case you + need to do further processing (unless no transformations have taken place). + + :param request: The request. + :param track: The track. + :return: The transformed track. + """ + # pylint: disable=too-many-locals + LOGGER.debug("Executing transformers for %d", track.id) + + settings = mod_transformers.extract_from_request(request) + + serialized = [[tfm.identifier(), tfm.parameters.dict()] for tfm in settings] + if serialized == track.transformers: + LOGGER.debug("Applied transformations match on %d, skipping", track.id) + return None + + # We always start with the backup, that way we don't get "deepfried GPX" + # files by having the same filters run multiple times on the same input. + # They are not idempotent after all. + manager = request.data_manager.open(track.id) + gpx_bytes = manager.backup_path().read_bytes() + gpx_bytes = brotli.decompress(gpx_bytes) + gpx = gpxpy.parse(gpx_bytes) + + for transformer in settings: + LOGGER.debug("Running %s with %r", transformer, transformer.parameters) + transformer.execute(gpx) + + LOGGER.debug("Saving transformed file for %d", track.id) + manager.compress_gpx(gpx.to_xml().encode("utf-8")) + + LOGGER.debug("Saving new transformers on %d", track.id) + track.transformers = serialized + + LOGGER.debug("Rebuilding cache for %d", track.id) + request.dbsession.delete(track.cache) + track.cache = None + track.ensure_cache(gpx) + request.dbsession.add(track.cache) + return gpx diff --git a/fietsboek/alembic/versions/20230203_3149aa2d0114.py b/fietsboek/alembic/versions/20230203_3149aa2d0114.py new file mode 100644 index 0000000..eb9ef78 --- /dev/null +++ b/fietsboek/alembic/versions/20230203_3149aa2d0114.py @@ -0,0 +1,22 @@ +"""add transformer column + +Revision ID: 3149aa2d0114 +Revises: c939800af428 +Create Date: 2023-02-03 21:44:39.429564 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = '3149aa2d0114' +down_revision = 'c939800af428' +branch_labels = None +depends_on = None + +def upgrade(): + op.add_column('tracks', sa.Column('transformers', sa.JSON(), nullable=True)) + op.execute('UPDATE tracks SET transformers="[]";') + +def downgrade(): + op.drop_column('tracks', 'transformers') diff --git a/fietsboek/data.py b/fietsboek/data.py index cbe20e9..9f49c49 100644 --- a/fietsboek/data.py +++ b/fietsboek/data.py @@ -256,7 +256,13 @@ class TrackDataDir: return brotli.decompress(self.gpx_path().read_bytes()) def engrave_metadata( - self, title: str, description: str, author_name: str, time: datetime.datetime + self, + title: str, + description: str, + author_name: str, + time: datetime.datetime, + *, + gpx: Optional[gpxpy.gpx.GPX] = None, ): """Engrave the given metadata into the GPX file. @@ -266,8 +272,10 @@ class TrackDataDir: :param description: The description of the track. :param creator: Name of the track's creator. :param time: Time of the track. + :param gpx: The pre-parsed GPX track, to save time if it is already parsed. """ - gpx = gpxpy.parse(self.decompress_gpx()) + if gpx is None: + gpx = gpxpy.parse(self.decompress_gpx()) # First we delete the existing metadata for track in gpx.tracks: track.name = None diff --git a/fietsboek/locale/de/LC_MESSAGES/messages.mo b/fietsboek/locale/de/LC_MESSAGES/messages.mo Binary files differindex 75b0abe..a79349d 100644 --- a/fietsboek/locale/de/LC_MESSAGES/messages.mo +++ b/fietsboek/locale/de/LC_MESSAGES/messages.mo diff --git a/fietsboek/locale/de/LC_MESSAGES/messages.po b/fietsboek/locale/de/LC_MESSAGES/messages.po index 8f6d9f2..8b59ecb 100644 --- a/fietsboek/locale/de/LC_MESSAGES/messages.po +++ b/fietsboek/locale/de/LC_MESSAGES/messages.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2023-02-06 19:52+0100\n" +"POT-Creation-Date: 2023-03-07 20:11+0100\n" "PO-Revision-Date: 2022-07-02 17:35+0200\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language: de\n" @@ -18,39 +18,39 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Generated-By: Babel 2.11.0\n" -#: fietsboek/util.py:273 +#: fietsboek/util.py:276 msgid "password_constraint.mismatch" msgstr "Passwörter stimmen nicht überein" -#: fietsboek/util.py:275 +#: fietsboek/util.py:278 msgid "password_constraint.length" msgstr "Passwort zu kurz" -#: fietsboek/models/track.py:545 +#: fietsboek/models/track.py:566 msgid "tooltip.table.length" msgstr "Länge" -#: fietsboek/models/track.py:546 +#: fietsboek/models/track.py:567 msgid "tooltip.table.uphill" msgstr "Bergauf" -#: fietsboek/models/track.py:547 +#: fietsboek/models/track.py:568 msgid "tooltip.table.downhill" msgstr "Bergab" -#: fietsboek/models/track.py:548 +#: fietsboek/models/track.py:569 msgid "tooltip.table.moving_time" msgstr "Fahrzeit" -#: fietsboek/models/track.py:549 +#: fietsboek/models/track.py:570 msgid "tooltip.table.stopped_time" msgstr "Haltezeit" -#: fietsboek/models/track.py:551 +#: fietsboek/models/track.py:572 msgid "tooltip.table.max_speed" msgstr "Maximalgeschwindigkeit" -#: fietsboek/models/track.py:555 +#: fietsboek/models/track.py:576 msgid "tooltip.table.avg_speed" msgstr "Durchschnittsgeschwindigkeit" @@ -438,6 +438,10 @@ msgstr "Bildbeschreibung" msgid "page.track.form.image_description_modal.save" msgstr "Übernehmen" +#: fietsboek/templates/edit_form.jinja2:166 +msgid "page.track.form.transformer.enable" +msgstr "Transformation anwenden" + #: fietsboek/templates/finish_upload.jinja2:8 #: fietsboek/templates/upload.jinja2:6 msgid "page.upload.title" @@ -456,7 +460,8 @@ msgstr "Abbrechen" msgid "page.home.title" msgstr "Startseite" -#: fietsboek/templates/home.jinja2:12 +#: fietsboek/templates/home.jinja2:12 fietsboek/templates/home.jinja2:19 +#: fietsboek/templates/home.jinja2:37 msgid "page.home.summary.track" msgid_plural "page.home.summary.tracks" msgstr[0] "%(num)d Strecke" @@ -632,6 +637,16 @@ msgstr "Anfrage senden" msgid "page.upload.form.gpx" msgstr "GPX Datei" +#: fietsboek/transformers/__init__.py:140 +msgid "transformers.fix-null-elevation.title" +msgstr "Nullhöhen beheben" + +#: fietsboek/transformers/__init__.py:144 +msgid "transformers.fix-null-elevation.description" +msgstr "" +"Diese Transformation passt die Höhenangabe für Punkte an, bei denen die " +"Höhenangabe fehlt." + #: fietsboek/views/account.py:54 msgid "flash.invalid_name" msgstr "Ungültiger Name" @@ -743,11 +758,11 @@ msgstr "Keine Datei ausgewählt" msgid "flash.invalid_file" msgstr "Ungültige GPX-Datei gesendet" -#: fietsboek/views/upload.py:181 +#: fietsboek/views/upload.py:188 msgid "flash.upload_success" msgstr "Hochladen erfolgreich" -#: fietsboek/views/upload.py:197 +#: fietsboek/views/upload.py:204 msgid "flash.upload_cancelled" msgstr "Hochladen abgebrochen" diff --git a/fietsboek/locale/en/LC_MESSAGES/messages.mo b/fietsboek/locale/en/LC_MESSAGES/messages.mo Binary files differindex 7a7d215..13debb5 100644 --- a/fietsboek/locale/en/LC_MESSAGES/messages.mo +++ b/fietsboek/locale/en/LC_MESSAGES/messages.mo diff --git a/fietsboek/locale/en/LC_MESSAGES/messages.po b/fietsboek/locale/en/LC_MESSAGES/messages.po index c1f17a8..632e0b7 100644 --- a/fietsboek/locale/en/LC_MESSAGES/messages.po +++ b/fietsboek/locale/en/LC_MESSAGES/messages.po @@ -7,7 +7,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2023-02-06 19:52+0100\n" +"POT-Creation-Date: 2023-03-07 20:11+0100\n" "PO-Revision-Date: 2022-06-28 13:11+0200\n" "Last-Translator: \n" "Language: en\n" @@ -18,39 +18,39 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Generated-By: Babel 2.11.0\n" -#: fietsboek/util.py:273 +#: fietsboek/util.py:276 msgid "password_constraint.mismatch" msgstr "Passwords don't match" -#: fietsboek/util.py:275 +#: fietsboek/util.py:278 msgid "password_constraint.length" msgstr "Password not long enough" -#: fietsboek/models/track.py:545 +#: fietsboek/models/track.py:566 msgid "tooltip.table.length" msgstr "Length" -#: fietsboek/models/track.py:546 +#: fietsboek/models/track.py:567 msgid "tooltip.table.uphill" msgstr "Uphill" -#: fietsboek/models/track.py:547 +#: fietsboek/models/track.py:568 msgid "tooltip.table.downhill" msgstr "Downhill" -#: fietsboek/models/track.py:548 +#: fietsboek/models/track.py:569 msgid "tooltip.table.moving_time" msgstr "Moving Time" -#: fietsboek/models/track.py:549 +#: fietsboek/models/track.py:570 msgid "tooltip.table.stopped_time" msgstr "Stopped Time" -#: fietsboek/models/track.py:551 +#: fietsboek/models/track.py:572 msgid "tooltip.table.max_speed" msgstr "Max Speed" -#: fietsboek/models/track.py:555 +#: fietsboek/models/track.py:576 msgid "tooltip.table.avg_speed" msgstr "Average Speed" @@ -434,6 +434,10 @@ msgstr "Image description" msgid "page.track.form.image_description_modal.save" msgstr "Apply" +#: fietsboek/templates/edit_form.jinja2:166 +msgid "page.track.form.transformer.enable" +msgstr "Apply transformation" + #: fietsboek/templates/finish_upload.jinja2:8 #: fietsboek/templates/upload.jinja2:6 msgid "page.upload.title" @@ -452,7 +456,8 @@ msgstr "Cancel" msgid "page.home.title" msgstr "Home" -#: fietsboek/templates/home.jinja2:12 +#: fietsboek/templates/home.jinja2:12 fietsboek/templates/home.jinja2:19 +#: fietsboek/templates/home.jinja2:37 msgid "page.home.summary.track" msgid_plural "page.home.summary.tracks" msgstr[0] "%(num)d track" @@ -628,6 +633,14 @@ msgstr "Send request" msgid "page.upload.form.gpx" msgstr "GPX file" +#: fietsboek/transformers/__init__.py:140 +msgid "transformers.fix-null-elevation.title" +msgstr "Fix null elevation" + +#: fietsboek/transformers/__init__.py:144 +msgid "transformers.fix-null-elevation.description" +msgstr "This transformer fixes the elevation of points whose elevation is unset." + #: fietsboek/views/account.py:54 msgid "flash.invalid_name" msgstr "Invalid name" @@ -738,11 +751,11 @@ msgstr "No file selected" msgid "flash.invalid_file" msgstr "Invalid GPX file selected" -#: fietsboek/views/upload.py:181 +#: fietsboek/views/upload.py:188 msgid "flash.upload_success" msgstr "Upload successful" -#: fietsboek/views/upload.py:197 +#: fietsboek/views/upload.py:204 msgid "flash.upload_cancelled" msgstr "Upload cancelled" diff --git a/fietsboek/locale/fietslog.pot b/fietsboek/locale/fietslog.pot index f4e3d01..70f010a 100644 --- a/fietsboek/locale/fietslog.pot +++ b/fietsboek/locale/fietslog.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PROJECT VERSION\n" "Report-Msgid-Bugs-To: EMAIL@ADDRESS\n" -"POT-Creation-Date: 2023-02-06 19:52+0100\n" +"POT-Creation-Date: 2023-03-07 20:11+0100\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" "Language-Team: LANGUAGE <LL@li.org>\n" @@ -17,39 +17,39 @@ msgstr "" "Content-Transfer-Encoding: 8bit\n" "Generated-By: Babel 2.11.0\n" -#: fietsboek/util.py:273 +#: fietsboek/util.py:276 msgid "password_constraint.mismatch" msgstr "" -#: fietsboek/util.py:275 +#: fietsboek/util.py:278 msgid "password_constraint.length" msgstr "" -#: fietsboek/models/track.py:545 +#: fietsboek/models/track.py:566 msgid "tooltip.table.length" msgstr "" -#: fietsboek/models/track.py:546 +#: fietsboek/models/track.py:567 msgid "tooltip.table.uphill" msgstr "" -#: fietsboek/models/track.py:547 +#: fietsboek/models/track.py:568 msgid "tooltip.table.downhill" msgstr "" -#: fietsboek/models/track.py:548 +#: fietsboek/models/track.py:569 msgid "tooltip.table.moving_time" msgstr "" -#: fietsboek/models/track.py:549 +#: fietsboek/models/track.py:570 msgid "tooltip.table.stopped_time" msgstr "" -#: fietsboek/models/track.py:551 +#: fietsboek/models/track.py:572 msgid "tooltip.table.max_speed" msgstr "" -#: fietsboek/models/track.py:555 +#: fietsboek/models/track.py:576 msgid "tooltip.table.avg_speed" msgstr "" @@ -431,6 +431,10 @@ msgstr "" msgid "page.track.form.image_description_modal.save" msgstr "" +#: fietsboek/templates/edit_form.jinja2:166 +msgid "page.track.form.transformer.enable" +msgstr "" + #: fietsboek/templates/finish_upload.jinja2:8 #: fietsboek/templates/upload.jinja2:6 msgid "page.upload.title" @@ -449,7 +453,8 @@ msgstr "" msgid "page.home.title" msgstr "" -#: fietsboek/templates/home.jinja2:12 +#: fietsboek/templates/home.jinja2:12 fietsboek/templates/home.jinja2:19 +#: fietsboek/templates/home.jinja2:37 msgid "page.home.summary.track" msgid_plural "page.home.summary.tracks" msgstr[0] "" @@ -623,6 +628,14 @@ msgstr "" msgid "page.upload.form.gpx" msgstr "" +#: fietsboek/transformers/__init__.py:140 +msgid "transformers.fix-null-elevation.title" +msgstr "" + +#: fietsboek/transformers/__init__.py:144 +msgid "transformers.fix-null-elevation.description" +msgstr "" + #: fietsboek/views/account.py:54 msgid "flash.invalid_name" msgstr "" @@ -727,11 +740,11 @@ msgstr "" msgid "flash.invalid_file" msgstr "" -#: fietsboek/views/upload.py:181 +#: fietsboek/views/upload.py:188 msgid "flash.upload_success" msgstr "" -#: fietsboek/views/upload.py:197 +#: fietsboek/views/upload.py:204 msgid "flash.upload_cancelled" msgstr "" diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py index c5e4c48..e0d2820 100644 --- a/fietsboek/models/track.py +++ b/fietsboek/models/track.py @@ -18,6 +18,7 @@ import logging from itertools import chain from typing import TYPE_CHECKING, List, Optional, Set, Union +import gpxpy from babel.numbers import format_decimal from markupsafe import Markup from pyramid.authorization import ( @@ -32,6 +33,7 @@ from pyramid.httpexceptions import HTTPNotFound from pyramid.i18n import Localizer from pyramid.i18n import TranslationString as _ from sqlalchemy import ( + JSON, Column, DateTime, Enum, @@ -174,6 +176,8 @@ class Track(Base): :vartype link_secret: str :ivar type: Type of the track :vartype type: TrackType + :ivar transformers: The enabled transformers together with their parameters. + :vartype transformers: list[tuple[str, dict]] :ivar owner: Owner of the track. :vartype owner: fietsboek.models.user.User :ivar cache: Cache for the computed track metadata. @@ -198,6 +202,7 @@ class Track(Base): visibility = Column(Enum(Visibility)) link_secret = Column(Text) type = Column(Enum(TrackType)) + transformers = Column(JSON) owner = relationship("User", back_populates="tracks") cache = relationship( @@ -326,7 +331,7 @@ class Track(Base): result = ACLHelper().permits(self, principals, "track.view") return isinstance(result, ACLAllowed) - def ensure_cache(self, gpx_data: Union[str, bytes]): + def ensure_cache(self, gpx_data: Union[str, bytes, gpxpy.gpx.GPX]): """Ensure that a cached version of this track's metadata exists. :param gpx_data: GPX data (uncompressed) from which to build the cache. @@ -403,6 +408,22 @@ class Track(Base): for i in to_delete[::-1]: del self.tags[i] + def transformer_params_for(self, transformer_id: str) -> Optional[dict]: + """Returns the transformer parameters for the given transformer. + + If the transformer is not active, returns ``None``. + + :param transformer_id: The string ID of the transformer. + :return: The settings as a dictionary. + """ + if not self.transformers: + return None + + for t_id, settings in self.transformers: + if t_id == transformer_id: + return settings + return None + class TrackWithMetadata: """A class to add metadata to a :class:`Track`. diff --git a/fietsboek/templates/details.jinja2 b/fietsboek/templates/details.jinja2 index 2ff492c..7c1ba8c 100644 --- a/fietsboek/templates/details.jinja2 +++ b/fietsboek/templates/details.jinja2 @@ -88,46 +88,46 @@ <tbody> <tr> <th scope="row">{{ _("page.details.date") }}</th> - <td>{{ track.date | format_datetime }}</td> + <td id="detailsDate">{{ track.date | format_datetime }}</td> </tr> {% if show_organic %} <tr> <th scope="row">{{ _("page.details.start_time") }}</th> - <td>{{ track.start_time | format_datetime }}</td> + <td id="detailsStartTime">{{ track.start_time | format_datetime }}</td> </tr> <tr> <th scope="row">{{ _("page.details.end_time") }}</th> - <td>{{ track.end_time | format_datetime }}</td> + <td id="detailsEndTime">{{ track.end_time | format_datetime }}</td> </tr> {% endif %} <tr> <th scope="row">{{ _("page.details.length") }}</th> - <td>{{ (track.length / 1000) | round(2) | format_decimal }} km</td> + <td id="detailsLength">{{ (track.length / 1000) | round(2) | format_decimal }} km</td> </tr> <tr> <th scope="row">{{ _("page.details.uphill") }}</th> - <td>{{ track.uphill | round(2) | format_decimal }} m</td> + <td id="detailsUphill">{{ track.uphill | round(2) | format_decimal }} m</td> </tr> <tr> <th scope="row">{{ _("page.details.downhill") }}</th> - <td>{{ track.downhill | round(2) | format_decimal }} m</td> + <td id="detailsDownhill">{{ track.downhill | round(2) | format_decimal }} m</td> </tr> {% if show_organic %} <tr> <th scope="row">{{ _("page.details.moving_time") }}</th> - <td>{{ track.moving_time }}</td> + <td id="detailsMovingTime">{{ track.moving_time }}</td> </tr> <tr> <th scope="row">{{ _("page.details.stopped_time") }}</th> - <td>{{ track.stopped_time }}</td> + <td id="detailsStoppedTime">{{ track.stopped_time }}</td> </tr> <tr> <th scope="row">{{ _("page.details.max_speed") }}</th> - <td>{{ mps_to_kph(track.max_speed) | round(2) | format_decimal }} km/h</td> + <td id="detailsMaxSpeed">{{ mps_to_kph(track.max_speed) | round(2) | format_decimal }} km/h</td> </tr> <tr> <th scope="row">{{ _("page.details.avg_speed") }}</th> - <td>{{ mps_to_kph(track.avg_speed) | round(2) | format_decimal }} km/h</td> + <td id="detailsAvgSpeed">{{ mps_to_kph(track.avg_speed) | round(2) | format_decimal }} km/h</td> </tr> {% endif %} </tbody> diff --git a/fietsboek/templates/edit_form.jinja2 b/fietsboek/templates/edit_form.jinja2 index bfd45a1..1abe076 100644 --- a/fietsboek/templates/edit_form.jinja2 +++ b/fietsboek/templates/edit_form.jinja2 @@ -142,4 +142,52 @@ </div> </div> </div> + +<div class="mb-3"> + <div class="accordion accordion-flush"> + {% for transformer in list_transformers() %} + <div class="accordion-item"> + <h2 class="accordion-header" id="transformer-heading-{{ loop.index }}"> + <button class="accordion-button collapsed" type="button" data-bs-toggle="collapse" data-bs-target="#transformer-{{ loop.index }}" aria-expanded="true" aria-controls="transformer-{{ loop.index }}"> + {{ _(transformer.name()) }} + </button> + </h2> + </div> + <div id="transformer-{{ loop.index}}" class="accordion-collapse collapse" aria-labelledby="transformer-heading-{{ loop.index }}"> + <div class="accordion-body"> + {% set params = track.transformer_params_for(transformer.identifier()) if track else None %} + <!-- Short transformer description --> + <p>{{ _(transformer.description()) }}</p> + + <!-- Checkbox to enable the transformer --> + <div class="form-check"> + <input class="form-check-input" type="checkbox" value="on" id="transformer-enabled-{{ loop.index }}" name="transformer[{{ transformer.identifier() }}]"{% if params is not none %} checked{% endif %}> + <label class="form-check-label" for="transformer-enabled-{{ loop.index }}"> + {{ _("page.track.form.transformer.enable") }} + </label> + </div> + + <!-- Parameters as defined by the transformer --> + {% if params is not none %} + {{ render_parameters(transformer.identifier(), transformer.parameter_model().parse_obj(params).html_ui()) }} + {% else %} + {{ render_parameters(transformer.identifier(), transformer().parameters.html_ui()) }} + {% endif %} + </div> + </div> + {% endfor %} + </div> +</div> +{% endmacro %} + +{% macro render_parameters(identifier, params) %} +{% for param in params %} +{% if param.type == "int" %} +<label for="transformer[{{ identifier }}][{{ param.name }}" class="form-label">{{ _(param.label) }}</label> +<input id="transformer[{{ identifier }}][{{ param.name }}]" name="transformer[{{ identifier }}][{{ param.name }}]" type="number" value="{{ param.value }}" class="form-control" /> +{% elif param.type == "str" %} +<label for="transformer[{{ identifier }}][{{ param.name }}" class="form-label">{{ _(param.label) }}</label> +<input id="transformer[{{ identifier }}][{{ param.name }}]" name="transformer[{{ identifier }}][{{ param.name }}]" type="text" value="{{ param.value }}" class="form-control" /> +{% endif %} +{% endfor %} {% endmacro %} diff --git a/fietsboek/transformers/__init__.py b/fietsboek/transformers/__init__.py new file mode 100644 index 0000000..f6318d7 --- /dev/null +++ b/fietsboek/transformers/__init__.py @@ -0,0 +1,264 @@ +"""Fietsboek GPX transformers. + +A "transformer" is something like a filter - it takes in a GPX file and applies +some small corrections, such as smoothing out the elevation. In order to avoid +confusion with the naming (and the "filters" you have when searching for +tracks), we call those "GPX filters" *transformers*. + +This module defines the abstract interface for transformers, as well as +function to load and apply transformers. +""" + +from abc import ABC, abstractmethod +from collections.abc import Callable, Iterable, Mapping +from itertools import islice +from typing import Literal, NamedTuple, TypeVar + +from gpxpy.gpx import GPX, GPXTrackPoint +from pydantic import BaseModel +from pyramid.i18n import TranslationString +from pyramid.request import Request + +_ = TranslationString + +T = TypeVar("T", bound="Transformer") + + +class ParameterDefinition(NamedTuple): + """A parameter definition for the UI to render.""" + + type: Literal["int", "str"] + """Type of the parameter.""" + + name: str + """Name of the parameter. + + This is the machine-readable identifier, not the human readable name. + """ + + label: TranslationString + """Human-readable label of the parameter.""" + + value: str + """The serialized value of the parameter.""" + + +class Parameters(BaseModel): + """Parameters for a transformer. + + This is basically a wrapper around pydantic models that allows the + parameters to be serialized from and to POST request parameters. + """ + + def html_ui(self) -> list[ParameterDefinition]: + """Renders a HTML UI for this parameter set. + + :return: The rendered UI, ready for inclusion. + """ + return [] + + def read_from_request(self, data: Mapping[str, str]): + """Parses the parameters from the given request data. + + :param data: The request data, e.g. from the POST values. + """ + + +class Transformer(ABC): + """A :class:`Transformer` is the main interface for track manipulation.""" + + @classmethod + @abstractmethod + def identifier(cls: type[T]) -> str: + """Returns a string identifier for this transformer. + + This identifier is used when serializing/deserializing the filters. + + :return: A machine-readable identifier for this transformer. + """ + + @classmethod + @abstractmethod + def name(cls: type[T]) -> TranslationString: + """The human-readable name of this transformer, as a translateable string. + + :return: The transformer's name. + """ + + @classmethod + @abstractmethod + def description(cls: type[T]) -> TranslationString: + """A short description of what this transformer does. + + :return: The transformer's description. + """ + + @classmethod + @abstractmethod + def parameter_model(cls: type[T]) -> type[Parameters]: + """Returns the parameter model that this transformer expects. + + :return: The parameter model class. + """ + + @property + @abstractmethod + def parameters(self) -> Parameters: + """Returns the parameters of this transformer. + + Note that the caller may modify the parameters, which should be + reflected in future applications of the transformer. + + :return: The parameters. + """ + + @parameters.setter + @abstractmethod + def parameters(self, value: Parameters): + pass + + @abstractmethod + def execute(self, gpx: GPX): + """Run the transformation on the input gpx. + + This is expected to modify the GPX object to represent the new state. + + :param gpx: The GPX object to transform. Note that this object will be + mutated! + """ + + +class FixNullElevation(Transformer): + """A transformer that fixes points with zero elevation.""" + + @classmethod + def identifier(cls) -> str: + return "fix-null-elevation" + + @classmethod + def name(cls) -> TranslationString: + return _("transformers.fix-null-elevation.title") + + @classmethod + def description(cls) -> TranslationString: + return _("transformers.fix-null-elevation.description") + + @classmethod + def parameter_model(cls) -> type[Parameters]: + return Parameters + + @property + def parameters(self) -> Parameters: + return Parameters() + + @parameters.setter + def parameters(self, value): + pass + + def execute(self, gpx: GPX): + def all_points(): + return ( + point + for track in gpx.tracks + for segment in track.segments + for point in segment.points + ) + + def rev_points(): + return ( + point + for track in reversed(gpx.tracks) + for segment in reversed(track.segments) + for point in reversed(segment.points) + ) + + # First, from the front + self.fixup(all_points) + # Then, from the back + self.fixup(rev_points) + + @classmethod + def fixup(cls, points: Callable[[], Iterable[GPXTrackPoint]]): + """Fixes the given GPX points. + + This iterates over the points and checks for the first point that has a + non-zero elevation, and a slope that doesn't exceed 100%. All previous + points will have their elevation adjusted to match this first "good + point". + + :param points: A function that generates the iterable of points. + """ + max_slope = 1.0 + + bad_until = 0 + final_elevation = 0.0 + for i, (point, next_point) in enumerate(zip(points(), islice(points(), 1, None))): + if ( + point.elevation is not None + and point.elevation != 0.0 + and cls.slope(point, next_point) < max_slope + ): + bad_until = i + final_elevation = point.elevation + break + + for point in islice(points(), None, bad_until): + point.elevation = final_elevation + + @staticmethod + def slope(point_a: GPXTrackPoint, point_b: GPXTrackPoint) -> float: + """Returns the slope between two GPX points. + + This is defined as delta_h / euclid_distance. + + :param point_a: First point. + :param point_b: Second point. + :return: The slope, as percentage. + """ + if point_a.elevation is None or point_b.elevation is None: + return 0.0 + delta_h = abs(point_a.elevation - point_b.elevation) + dist = point_a.distance_2d(point_b) + if dist == 0.0 or dist is None: + return 0.0 + return delta_h / dist + + +def list_transformers() -> list[type[Transformer]]: + """Returns a list of all available transformers. + + :return: A list of transformers. + """ + return [ + FixNullElevation, + ] + + +def extract_from_request(request: Request) -> list[Transformer]: + """Extracts the list of transformers to execute from the given request. + + Note that this sets up the transformers with the right parameters. + + :param request: The pyramid request. + :return: The list of prepared transformers. + """ + transformers = [] + for tfm in list_transformers(): + ident = tfm.identifier() + prefix = f"transformer[{ident}]" + req_params = {} + for name, val in request.params.items(): + if name.startswith(prefix): + name = name[len(prefix) :] + name = name.strip("[]") + req_params[name] = val + + if req_params.get("") == "on": + instance = tfm() + instance.parameters.read_from_request(req_params) + transformers.append(instance) + + return transformers + + +__all__ = ["Parameters", "Transformer", "list_transformers"] diff --git a/fietsboek/util.py b/fietsboek/util.py index c741550..68ba769 100644 --- a/fietsboek/util.py +++ b/fietsboek/util.py @@ -152,19 +152,22 @@ def guess_gpx_timezone(gpx: gpxpy.gpx.GPX) -> datetime.tzinfo: return datetime.timezone.utc -def tour_metadata(gpx_data: Union[str, bytes]) -> dict: +def tour_metadata(gpx_data: Union[str, bytes, gpxpy.gpx.GPX]) -> dict: """Calculate the metadata of the tour. Returns a dict with ``length``, ``uphill``, ``downhill``, ``moving_time``, ``stopped_time``, ``max_speed``, ``avg_speed``, ``start_time`` and ``end_time``. - :param gpx_data: The GPX data of the tour. + :param gpx_data: The GPX data of the tour. Can be pre-parsed to save time. :return: A dictionary with the computed values. """ if isinstance(gpx_data, bytes): gpx_data = gpx_data.decode("utf-8") - gpx = gpxpy.parse(gpx_data) + if isinstance(gpx_data, gpxpy.gpx.GPX): + gpx = gpx_data + else: + gpx = gpxpy.parse(gpx_data) timezone = guess_gpx_timezone(gpx) uphill, downhill = gpx.get_uphill_downhill() moving_data = gpx.get_moving_data() diff --git a/fietsboek/views/edit.py b/fietsboek/views/edit.py index 61741e5..881f404 100644 --- a/fietsboek/views/edit.py +++ b/fietsboek/views/edit.py @@ -95,11 +95,13 @@ def do_edit(request): track.sync_tags(tags) actions.edit_images(request, request.context, manager=data) + gpx = actions.execute_transformers(request, request.context) data.engrave_metadata( title=track.title, description=track.description, author_name=track.owner.name, time=track.date, + gpx=gpx, ) return HTTPFound(request.route_url("details", track_id=track.id)) diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py index ed8f285..fd93034 100644 --- a/fietsboek/views/upload.py +++ b/fietsboek/views/upload.py @@ -9,7 +9,7 @@ from pyramid.response import Response from pyramid.view import view_config from sqlalchemy import select -from .. import actions, models, util +from .. import actions, models, transformers, util from ..models.track import TrackType, Visibility LOGGER = logging.getLogger(__name__) @@ -170,6 +170,7 @@ def do_finish_upload(request): tagged_people=tagged_people, date=date, tags=request.params.getall("tag[]"), + transformers=transformers.extract_from_request(request), gpx_data=upload.gpx_data, ) request.dbsession.delete(upload) diff --git a/pylint.toml b/pylint.toml index 9e3a63c..f1e4270 100644 --- a/pylint.toml +++ b/pylint.toml @@ -17,7 +17,7 @@ # be loaded. Extensions are loading into the active Python interpreter and may # run arbitrary code. (This is an alternative name to extension-pkg-allow-list # for backward compatibility.) -# extension-pkg-whitelist = +extension-pkg-whitelist = ["pydantic"] # Return non-zero exit code if any of these messages/categories are detected, # even if score is above --fail-under value. Syntax same as enable. Messages diff --git a/tests/assets/Synthetic_Steep_Slope.gpx.gz b/tests/assets/Synthetic_Steep_Slope.gpx.gz Binary files differnew file mode 100644 index 0000000..244f78a --- /dev/null +++ b/tests/assets/Synthetic_Steep_Slope.gpx.gz diff --git a/tests/assets/Synthetic_Zero_Elevation.gpx.gz b/tests/assets/Synthetic_Zero_Elevation.gpx.gz Binary files differnew file mode 100644 index 0000000..94abb0a --- /dev/null +++ b/tests/assets/Synthetic_Zero_Elevation.gpx.gz diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py index 18b7ad0..f57aca7 100644 --- a/tests/playwright/conftest.py +++ b/tests/playwright/conftest.py @@ -126,6 +126,7 @@ class Helper: tags=[], badges=[], tagged_people=[], + transformers=[], gpx_data=load_gpx_asset(track_name), ) self.dbaccess.commit() diff --git a/tests/playwright/test_basic.py b/tests/playwright/test_basic.py index 3b3329a..a98e52d 100644 --- a/tests/playwright/test_basic.py +++ b/tests/playwright/test_basic.py @@ -51,7 +51,7 @@ def test_upload(page: Page, playwright_helper, tmp_path, dbaccess): page.locator(".bi-upload").click() # We now fill in most of the data - page.get_by_label("Title").fill("An awesome track!") + page.get_by_label("Title", exact=True).fill("An awesome track!") page.get_by_label("Date").type("07302022") page.get_by_label("Date").press("Tab") page.get_by_label("Date").type("12:41") @@ -82,7 +82,7 @@ def test_edit(page: Page, playwright_helper, dbaccess): page.locator(".btn", has_text="Edit").click() # We now fill in most of the data - page.get_by_label("Title").fill("Not so awesome anymore!") + page.get_by_label("Title", exact=True).fill("Not so awesome anymore!") page.get_by_label("Date").type("09232019") page.get_by_label("Date").press("Tab") page.get_by_label("Date").type("15:28") diff --git a/tests/playwright/test_transformers.py b/tests/playwright/test_transformers.py new file mode 100644 index 0000000..0b2e4de --- /dev/null +++ b/tests/playwright/test_transformers.py @@ -0,0 +1,130 @@ +from playwright.sync_api import Page, expect +from sqlalchemy import select + +from testutils import extract_and_upload +from fietsboek import models + + +def test_transformer_zero_elevation_disabled(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Zero_Elevation.gpx.gz", tmp_path) + + page.locator(".btn", has_text="Upload").click() + + # Expect early (here and in the other tests) to ensure that the backend has + # caught up with executing the transformer. Otherwise it might happen that + # we read the database while the request is not finished yet. + expect(page.locator("#detailsUphill")).to_contain_text("167.7 m") + new_track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=new_track_id)).scalar_one() + + assert track.cache.uphill > 160 + + +def test_transformer_zero_elevation_enabled(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Zero_Elevation.gpx.gz", tmp_path) + + page.locator("#transformer-heading-1 .accordion-button").click() + page.locator("#transformer-enabled-1").click() + + page.locator(".btn", has_text="Upload").click() + + expect(page.locator("#detailsUphill")).to_contain_text("0 m") + new_track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=new_track_id)).scalar_one() + + assert track.cache.uphill < 0.1 + + +def test_transformer_zero_elevation_edited(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Zero_Elevation.gpx.gz", tmp_path) + + page.locator(".btn", has_text="Upload").click() + + page.locator(".btn", has_text="Edit").click() + + page.locator("#transformer-heading-1 .accordion-button").click() + page.locator("#transformer-enabled-1").click() + + page.locator(".btn", has_text="Save").click() + + expect(page.locator("#detailsUphill")).to_contain_text("0 m") + track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=track_id)).scalar_one() + + assert track.cache.uphill < 0.1 + + +def test_transformer_steep_slope_disabled(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Steep_Slope.gpx.gz", tmp_path) + + page.locator(".btn", has_text="Upload").click() + + expect(page.locator("#detailsUphill")).to_contain_text("61.54 m") + new_track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=new_track_id)).scalar_one() + + assert track.cache.uphill > 60 + + +def test_transformer_steep_slope_enabled(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Steep_Slope.gpx.gz", tmp_path) + + page.locator("#transformer-heading-1 .accordion-button").click() + page.locator("#transformer-enabled-1").click() + + page.locator(".btn", has_text="Upload").click() + + expect(page.locator("#detailsUphill")).to_contain_text("1.2 m") + new_track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=new_track_id)).scalar_one() + + assert track.cache.uphill < 2 + + +def test_transformer_steep_slope_edited(page: Page, playwright_helper, tmp_path, dbaccess): + playwright_helper.login() + + page.goto("/") + page.get_by_text("Upload").click() + + extract_and_upload(page, "Synthetic_Steep_Slope.gpx.gz", tmp_path) + + page.locator(".btn", has_text="Upload").click() + + page.locator(".btn", has_text="Edit").click() + + page.locator("#transformer-heading-1 .accordion-button").click() + page.locator("#transformer-enabled-1").click() + + page.locator(".btn", has_text="Save").click() + + expect(page.locator("#detailsUphill")).to_contain_text("1.2 m") + track_id = int(page.url.rsplit("/", 1)[1]) + track = dbaccess.execute(select(models.Track).filter_by(id=track_id)).scalar_one() + + assert track.cache.uphill < 2 diff --git a/tests/testutils.py b/tests/testutils.py index 3ddbdbe..810bdf7 100644 --- a/tests/testutils.py +++ b/tests/testutils.py @@ -2,19 +2,37 @@ import gzip from pathlib import Path +from playwright.sync_api import Page -def load_gpx_asset(filename): + +def load_gpx_asset(filename: str) -> bytes: """Load a GPX test asset. This looks in the tests/assets/ folder, reads and unzips the file and returns its contents. :param filename: Name of the asset to load. - :type filename: str :return: The content of the asset as bytes. - :rtype: bytes """ asset_dir = Path(__file__).parent / 'assets' test_file = asset_dir / filename with gzip.open(test_file, 'rb') as fobj: return fobj.read() + + +def extract_and_upload(page: Page, filename: str, tmp_path: Path): + """Extracts the given test asset, fills in the upload form and presses + upload. + + :param page: The playwright page on which to execute the actions. + :param filename: The filename. + :param tmp_path: The temporary path (as given by pytest). + """ + gpx_data = load_gpx_asset(filename) + gpx_path = tmp_path / "Upload.gpx" + with open(gpx_path, "wb") as gpx_fobj: + gpx_fobj.write(gpx_data) + + page.get_by_label("GPX file").set_input_files(gpx_path) + + page.locator(".bi-upload").click() |
