From 03b08ca7454e8ff60b26c0bbe79d4bee89b45eaf Mon Sep 17 00:00:00 2001 From: Daniel Schadt Date: Tue, 5 Jul 2022 17:38:01 +0200 Subject: start to use pyramid context factories This puts the handling of retrieving the right database object at a single place, which makes it easier to use from the different routes, and unifies the error handling (some places for example lacked the check to see if the returned object is None). It is also the first step to a better permission system based on Pyramid's ACLs, as we can now implement those on the object. This will make the code even better and even more uniform, as we don't need to do the permission checks manually anymore. --- fietsboek/models/badge.py | 23 +++++++++++++++++ fietsboek/models/track.py | 43 ++++++++++++++++++++++++++++++++ fietsboek/routes.py | 28 ++++++++++++++------- fietsboek/templates/browse.jinja2 | 2 +- fietsboek/templates/details.jinja2 | 10 ++++---- fietsboek/templates/edit.jinja2 | 4 +-- fietsboek/templates/finish_upload.jinja2 | 4 +-- fietsboek/templates/home.jinja2 | 2 +- fietsboek/templates/util.jinja2 | 2 +- fietsboek/views/detail.py | 24 +++++------------- fietsboek/views/edit.py | 8 +++--- fietsboek/views/upload.py | 13 ++++------ 12 files changed, 111 insertions(+), 52 deletions(-) diff --git a/fietsboek/models/badge.py b/fietsboek/models/badge.py index 37fa140..3bbe714 100644 --- a/fietsboek/models/badge.py +++ b/fietsboek/models/badge.py @@ -4,9 +4,12 @@ from sqlalchemy import ( Integer, Text, LargeBinary, + select, ) from sqlalchemy.orm import relationship +from pyramid.httpexceptions import HTTPNotFound + from .meta import Base @@ -31,3 +34,23 @@ class Badge(Base): image = Column(LargeBinary) tracks = relationship('Track', secondary='track_badge_assoc', back_populates='badges') + + @classmethod + def factory(cls, request): + """Factory method to pass to a route definition. + + This factory retrieves the badge based on the ``badge_id`` matched + route parameter, and returns the badge. If the badge is not found, + ``HTTPNotFound`` is raised. + + :raises pyramid.httpexception.NotFound: If the badge is not found. + :param request: The pyramid request. + :type request: ~pyramid.request.Request + :return: The badge. + :type: Badge + """ + query = select(cls).filter_by(id=request.matchdict["badge_id"]) + badge_object = request.dbsession.execute(query).scalar_one_or_none() + if badge_object is None: + raise HTTPNotFound() + return badge_object diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py index 315f484..e34805f 100644 --- a/fietsboek/models/track.py +++ b/fietsboek/models/track.py @@ -27,9 +27,11 @@ from sqlalchemy import ( Float, Enum, Table, + select, ) from sqlalchemy.orm import relationship +from pyramid.httpexceptions import HTTPNotFound from pyramid.i18n import TranslationString as _ from markupsafe import Markup @@ -147,6 +149,27 @@ class Track(Base): tags = relationship('Tag', back_populates='track', cascade="all, delete-orphan") comments = relationship('Comment', back_populates='track', cascade="all, delete-orphan") + @classmethod + def factory(cls, request): + """Factory method to pass to a route definition. + + This factory retrieves the track based on the ``track_id`` matched + route parameter, and returns the track. If the track is not found, + ``HTTPNotFound`` is raised. + + :raises pyramid.httpexception.NotFound: If the track is not found. + :param request: The pyramid request. + :type request: ~pyramid.request.Request + :return: The track. + :type: Track + """ + track_id = request.matchdict['track_id'] + query = select(cls).filter_by(id=track_id) + track = request.dbsession.execute(query).scalar_one_or_none() + if track is None: + raise HTTPNotFound() + return track + # GPX files are XML files with a lot of repeated property names. Really, it # is quite inefficient to store a whole ton of GPS points in big XML # structs. Therefore, we add transparent gzip compression to reduce the @@ -479,6 +502,26 @@ class Upload(Base): owner = relationship('User', back_populates='uploads') + @classmethod + def factory(cls, request): + """Factory method to pass to a route definition. + + This factory retrieves the upload based on the ``upload_id`` matched + route parameter, and returns the upload. If the upload is not found, + ``HTTPNotFound`` is raised. + + :raises pyramid.httpexception.NotFound: If the upload is not found. + :param request: The pyramid request. + :type request: ~pyramid.request.Request + :return: The upload. + :type: Track + """ + query = select(cls).filter_by(id=request.matchdict['upload_id']) + upload = request.dbsession.execute(query).scalar_one_or_none() + if upload is None: + raise HTTPNotFound() + return upload + @property def gpx_data(self): """Access the decompressed gpx data.""" diff --git a/fietsboek/routes.py b/fietsboek/routes.py index e19b47a..27a8f90 100644 --- a/fietsboek/routes.py +++ b/fietsboek/routes.py @@ -12,16 +12,26 @@ def includeme(config): config.add_route('create-account', '/create-account') config.add_route('upload', '/upload') - config.add_route('preview', '/preview/{id}.gpx') - config.add_route('finish-upload', '/upload/{id}') - config.add_route('cancel-upload', '/cancel/{id}') + config.add_route('preview', '/preview/{upload_id}.gpx', + factory='fietsboek.models.Upload.factory') + config.add_route('finish-upload', '/upload/{upload_id}', + factory='fietsboek.models.Upload.factory') + config.add_route('cancel-upload', '/cancel/{upload_id}', + factory='fietsboek.models.Upload.factory') - config.add_route('details', '/track/{id}') - config.add_route('edit', '/track/{id}/edit') - config.add_route('gpx', '/gpx/{id}.gpx') - config.add_route('invalidate-share', '/track/{id}/invalidate-link') - config.add_route('badge', '/badge/{id}') - config.add_route('add-comment', '/track/{track_id}/comment') + config.add_route('details', '/track/{track_id}', + factory='fietsboek.models.Track.factory') + config.add_route('edit', '/track/{track_id}/edit', + factory='fietsboek.models.Track.factory') + config.add_route('gpx', '/gpx/{track_id}.gpx', + factory='fietsboek.models.Track.factory') + config.add_route('invalidate-share', '/track/{track_id}/invalidate-link', + factory='fietsboek.models.Track.factory') + config.add_route('add-comment', '/track/{track_id}/comment', + factory='fietsboek.models.Track.factory') + + config.add_route('badge', '/badge/{badge_id}', + factory='fietsboek.models.Badge.factory') config.add_route('admin', '/admin') config.add_route('admin-badge-add', '/admin/add-badge') diff --git a/fietsboek/templates/browse.jinja2 b/fietsboek/templates/browse.jinja2 index 6b70406..117315a 100644 --- a/fietsboek/templates/browse.jinja2 +++ b/fietsboek/templates/browse.jinja2 @@ -5,7 +5,7 @@ {% for track in tracks %}
- {{ track.title | default(track.date, true) }} + {{ track.title | default(track.date, true) }} {% if track.text_tags() %} {% for tag in track.tags %}{{ tag.tag }} {% endfor %} {% endif %} diff --git a/fietsboek/templates/details.jinja2 b/fietsboek/templates/details.jinja2 index 5ef64b9..741c488 100644 --- a/fietsboek/templates/details.jinja2 +++ b/fietsboek/templates/details.jinja2 @@ -6,7 +6,7 @@

{{ track.title | default(_("page.details.title"), true) }}

{% if show_edit_link %}
- {{ _("page.details.edit") }} + {{ _("page.details.edit") }}

{{ _("page.details.sharelink.info") }}

- {% set share_link = request.route_url('details', id=track.id, _query=[("secret", track.link_secret)]) %} + {% set share_link = request.route_url('details', track_id=track.id, _query=[("secret", track.link_secret)]) %} {{ share_link }}
-
+
@@ -45,9 +45,9 @@

{% if 'secret' in request.GET %} - {% set gpx_url = request.route_path('gpx', id=track.id, _query=[('secret', request.GET['secret'])]) %} + {% set gpx_url = request.route_path('gpx', track_id=track.id, _query=[('secret', request.GET['secret'])]) %} {% else %} - {% set gpx_url = request.route_path('gpx', id=track.id) %} + {% set gpx_url = request.route_path('gpx', track_id=track.id) %} {% endif %}