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.

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 %}
   <div class="card mb-3">
     <h5 class="card-header">
-      <a href="{{ request.route_url('details', id=track.id) }}">{{ track.title | default(track.date, true) }}</a>
+      <a href="{{ request.route_url('details', track_id=track.id) }}">{{ track.title | default(track.date, true) }}</a>
       {% if track.text_tags() %}
       {% for tag in track.tags %}<span class="badge bg-info text-dark">{{ tag.tag }}</span> {% 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 @@
   <h1>{{ track.title | default(_("page.details.title"), true) }}</h1>
   {% if show_edit_link %}
   <div class="btn-group" role="group">
-    <a class="btn btn-success" href="{{ request.route_path('edit', id=track.id) }}"><i class="bi-pencil-square"></i> {{ _("page.details.edit") }}</a>
+    <a class="btn btn-success" href="{{ request.route_path('edit', track_id=track.id) }}"><i class="bi-pencil-square"></i> {{ _("page.details.edit") }}</a>
     <button type="button" class="btn btn-info" id="showShareLink" data-bs-toggle="modal" data-bs-target="#shareLinkModal"><i class="bi-share"></i> {{ _("page.details.share") }}</button>
   </div>
   <div class="modal fade" id="shareLinkModal" tabindex="-1" aria-hidden="true">
@@ -18,11 +18,11 @@
         </div>
         <div class="modal-body">
           <p>{{ _("page.details.sharelink.info") }}</p>
-          {% 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)]) %}
           <a href="{{ share_link }}">{{ share_link }}</a>
         </div>
         <div class="modal-footer">
-          <form method="POST" action="{{ request.route_url('invalidate-share', id=track.id) }}">
+          <form method="POST" action="{{ request.route_url('invalidate-share', track_id=track.id) }}">
             <button type="submit" class="btn btn-warning">{{ _("page.details.sharelink.invalidate") }}</button>
           </form>
           <button type="button" class="btn btn-secondary" data-bs-dismiss="modal">{{ _("page.details.sharelink.close") }}</button>
@@ -45,9 +45,9 @@
   </p>
 
   {% 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 %}
   <div id="mainmap" class="gpxview:{{ gpx_url }}:OSM" style="width:100%;height:600px">
     <noscript><p>{{ _("page.noscript") }}<p></noscript>
diff --git a/fietsboek/templates/edit.jinja2 b/fietsboek/templates/edit.jinja2
index b0f9849..fd26f6b 100644
--- a/fietsboek/templates/edit.jinja2
+++ b/fietsboek/templates/edit.jinja2
@@ -5,14 +5,14 @@
 {% block content %}
 <div class="container">
   <h1>{{ _("page.edit.title") }}</h1>
-  <div id="mainmap" class="gpxview:{{ request.route_path('gpx', id=track.id) }}:OSM" style="width:100%;height:600px">
+  <div id="mainmap" class="gpxview:{{ request.route_path('gpx', track_id=track.id) }}:OSM" style="width:100%;height:600px">
     <noscript><p>{{ _("page.noscript") }}<p></noscript>
   </div>
   <form method="POST">
     {{ edit_form.edit_track(track.title, track.date, track.visibility, track.description, track.text_tags(), badges, track.tagged_people) }}
     <div class="btn-group" role="group">
         <button type="submit" class="btn btn-primary"><i class="bi bi-save"></i> {{ _("page.edit.form.submit") }}</button>
-        <a href="{{ request.route_url('details', id=track.id) }}" class="btn btn-secondary"><i class="bi bi-x-circle"></i> {{ _("page.edit.form.cancel") }}</a>
+        <a href="{{ request.route_url('details', track_id=track.id) }}" class="btn btn-secondary"><i class="bi bi-x-circle"></i> {{ _("page.edit.form.cancel") }}</a>
     </div>
   </form>
 </div>
diff --git a/fietsboek/templates/finish_upload.jinja2 b/fietsboek/templates/finish_upload.jinja2
index e6072f4..a737464 100644
--- a/fietsboek/templates/finish_upload.jinja2
+++ b/fietsboek/templates/finish_upload.jinja2
@@ -5,14 +5,14 @@
 {% block content %}
 <div class="container">
   <h1>{{ _("page.upload.title") }}</h1>
-  <div id="mainmap" class="gpxview:{{ request.route_path('preview', id=preview_id) }}:OSM" style="width:100%;height:600px">
+  <div id="mainmap" class="gpxview:{{ request.route_path('preview', upload_id=preview_id) }}:OSM" style="width:100%;height:600px">
     <noscript><p>{{ _("page.noscript") }}<p></noscript>
   </div>
   <form method="POST">
     {{ edit_form.edit_track(upload_title, upload_date, upload_visibility, upload_description, upload_tags, badges, upload_tagged_people) }}
     <div class="btn-group" role="group">
       <button type="submit" class="btn btn-primary">{{ _("page.upload.form.submit") }}</button>
-      <a href="{{ request.route_url('cancel-upload', id=preview_id) }}" class="btn btn-danger">{{ _("page.upload.form.cancel") }}</a>
+      <a href="{{ request.route_url('cancel-upload', upload_id=preview_id) }}" class="btn btn-danger">{{ _("page.upload.form.cancel") }}</a>
     </div>
   </form>
 </div>
diff --git a/fietsboek/templates/home.jinja2 b/fietsboek/templates/home.jinja2
index e8c7d0c..5c7e0c8 100644
--- a/fietsboek/templates/home.jinja2
+++ b/fietsboek/templates/home.jinja2
@@ -12,7 +12,7 @@
       <a class="list-group-item list-group-item-secondary">{{ month_name(request, month.month) }} &mdash; {{ (month.total_length / 1000) | round(2) | format_decimal }} km</a>
       <div class="list-group">
         {% for track in month %}
-        <span class="list-group-item">{{ track.date.day }}: <a href="{{ request.route_url('details', id=track.id) }}" data-bs-toggle="tooltip" data-bs-container="body" data-bs-html="true" title="{{ track.html_tooltip(request.localizer) }}">{{ track.title | default(track.date, true) }}</a></span>
+        <span class="list-group-item">{{ track.date.day }}: <a href="{{ request.route_url('details', track_id=track.id) }}" data-bs-toggle="tooltip" data-bs-container="body" data-bs-html="true" title="{{ track.html_tooltip(request.localizer) }}">{{ track.title | default(track.date, true) }}</a></span>
         {% endfor %}
       </div>
       {% endfor %}
diff --git a/fietsboek/templates/util.jinja2 b/fietsboek/templates/util.jinja2
index d61127d..d334473 100644
--- a/fietsboek/templates/util.jinja2
+++ b/fietsboek/templates/util.jinja2
@@ -1,3 +1,3 @@
 {% macro render_badge(badge) -%}
-<div class="badge-container"><img src="{{ request.route_url('badge', id=badge.id) }}" data-bs-toggle="tooltip" title="{{ badge.title }}"></div>
+<div class="badge-container"><img src="{{ request.route_url('badge', badge_id=badge.id) }}" data-bs-toggle="tooltip" title="{{ badge.title }}"></div>
 {%- endmacro %}
diff --git a/fietsboek/views/detail.py b/fietsboek/views/detail.py
index 7d0337a..a1fe27f 100644
--- a/fietsboek/views/detail.py
+++ b/fietsboek/views/detail.py
@@ -3,9 +3,7 @@ import datetime
 
 from pyramid.view import view_config
 from pyramid.response import Response
-from pyramid.httpexceptions import HTTPForbidden, HTTPFound, HTTPNotFound
-
-from sqlalchemy import select
+from pyramid.httpexceptions import HTTPForbidden, HTTPFound
 
 from .. import models, util
 
@@ -19,10 +17,7 @@ def details(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Track).filter_by(id=request.matchdict["id"])
-    track = request.dbsession.execute(query).scalar_one_or_none()
-    if track is None:
-        return HTTPNotFound()
+    track = request.context
     if (not track.is_visible_to(request.identity)
             and request.GET.get('secret') != track.link_secret):
         return HTTPForbidden()
@@ -45,10 +40,7 @@ def gpx(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Track).filter_by(id=request.matchdict["id"])
-    track = request.dbsession.execute(query).scalar_one_or_none()
-    if track is None:
-        return HTTPNotFound()
+    track = request.context
     if (not track.is_visible_to(request.identity)
             and request.GET.get('secret') != track.link_secret):
         return HTTPForbidden()
@@ -64,8 +56,7 @@ def invalidate_share(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Track).filter_by(id=request.matchdict["id"])
-    track = request.dbsession.execute(query).scalar_one()
+    track = request.context
     if track.owner != request.identity:
         return HTTPForbidden()
     track.link_secret = util.random_alphanum_string()
@@ -81,9 +72,7 @@ def badge(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Badge).filter_by(id=request.matchdict["id"])
-    badge_object = request.dbsession.execute(query).scalar_one()
-    return Response(badge_object.image)
+    return Response(request.context.image)
 
 
 @view_config(route_name="add-comment", request_method="POST", permission="comment")
@@ -95,8 +84,7 @@ def add_comment(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Track).filter_by(id=request.matchdict["track_id"])
-    track = request.dbsession.execute(query).scalar_one()
+    track = request.context
     if (not track.is_visible_to(request.identity)
             and request.GET.get('secret') != track.link_secret):
         return HTTPForbidden()
diff --git a/fietsboek/views/edit.py b/fietsboek/views/edit.py
index aad3539..5e247b3 100644
--- a/fietsboek/views/edit.py
+++ b/fietsboek/views/edit.py
@@ -20,8 +20,7 @@ def edit(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Track).filter_by(id=request.matchdict["id"])
-    track = request.dbsession.execute(query).scalar_one()
+    track = request.context
     if request.identity != track.owner:
         return HTTPForbidden()
     badges = request.dbsession.execute(select(models.Badge)).scalars()
@@ -42,8 +41,7 @@ def do_edit(request):
     :rtype: pyramid.response.Response
     """
     # pylint: disable=duplicate-code
-    query = select(models.Track).filter_by(id=request.matchdict["id"])
-    track = request.dbsession.execute(query).scalar_one()
+    track = request.context
     if request.identity != track.owner:
         return HTTPForbidden()
 
@@ -64,4 +62,4 @@ def do_edit(request):
     tags = request.params.getall("tag[]")
     track.sync_tags(tags)
 
-    return HTTPFound(request.route_url('details', id=track.id))
+    return HTTPFound(request.route_url('details', track_id=track.id))
diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py
index f670d52..59123ee 100644
--- a/fietsboek/views/upload.py
+++ b/fietsboek/views/upload.py
@@ -72,7 +72,7 @@ def do_upload(request):
     request.dbsession.add(upload)
     request.dbsession.flush()
 
-    return HTTPFound(request.route_url('finish-upload', id=upload.id))
+    return HTTPFound(request.route_url('finish-upload', upload_id=upload.id))
 
 
 @view_config(route_name='preview', permission='upload')
@@ -85,8 +85,7 @@ def preview(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Upload).filter_by(id=request.matchdict["id"])
-    upload = request.dbsession.execute(query).scalar_one()
+    upload = request.context
     if upload.owner != request.identity:
         return HTTPForbidden()
     return Response(upload.gpx_data, content_type='application/gpx+xml')
@@ -102,8 +101,7 @@ def finish_upload(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Upload).filter_by(id=request.matchdict["id"])
-    upload = request.dbsession.execute(query).scalar_one()
+    upload = request.context
     if upload.owner != request.identity:
         return HTTPForbidden()
 
@@ -132,8 +130,7 @@ def do_finish_upload(request):
     :return: The HTTP response.
     :rtype: pyramid.response.Response
     """
-    query = select(models.Upload).filter_by(id=request.matchdict["id"])
-    upload = request.dbsession.execute(query).scalar_one()
+    upload = request.context
     if upload.owner != request.identity:
         return HTTPForbidden()
 
@@ -168,7 +165,7 @@ def do_finish_upload(request):
 
     request.session.flash(request.localizer.translate(_("flash.upload_success")))
 
-    return HTTPFound(request.route_url('details', id=track.id))
+    return HTTPFound(request.route_url('details', track_id=track.id))
 
 @view_config(route_name='cancel-upload', permission='upload')
 def cancel_upload(request):