implement image upload

15 files changed, 389 insertions, 8 deletions

diff --git a/.gitignore b/.gitignore
index 3afaa13..a833b62 100644
--- a/.gitignore
+++ b/.gitignore
@@ -22,3 +22,4 @@ coverage
 test
 *.sqlite
 .venv/
+/data
diff --git a/development.ini b/development.ini
index 47e0231..eb33586 100644
--- a/development.ini
+++ b/development.ini
@@ -18,11 +18,12 @@ pyramid.includes =
     pyramid_debugtoolbar
 
 sqlalchemy.url = sqlite:///%(here)s/fietsboek.sqlite
+fietsboek.data_dir = %(here)s/data
 
 retry.attempts = 3
 
 email.from = fietsboek@kingdread.de
-email.smtp_url = debug://localhost:1025
+email.smtp_url = debug://
 
 available_locales = en de
 enable_account_registration = true
diff --git a/fietsboek/__init__.py b/fietsboek/__init__.py
index 4a28dd4..bd2d705 100644
--- a/fietsboek/__init__.py
+++ b/fietsboek/__init__.py
@@ -2,6 +2,8 @@
 
 For more information, see the README or the included documentation.
 """
+from pathlib import Path
+
 from pyramid.config import Configurator
 from pyramid.session import SignedCookieSessionFactory
 from pyramid.csrf import CookieCSRFStoragePolicy
@@ -9,6 +11,7 @@ from pyramid.settings import asbool, aslist
 from pyramid.i18n import default_locale_negotiator
 
 from .security import SecurityPolicy
+from .data import DataManager
 from . import jinja2 as fiets_jinja2
 
 
@@ -46,6 +49,13 @@ def main(global_config, **settings):
     if settings.get('session_key', '<EDIT THIS>') == '<EDIT THIS>':
         raise ValueError("Please set a session signing key (session_key) in your settings!")
 
+    if 'fietsboek.data_dir' not in settings:
+        raise ValueError("Please set a data directory (fietsboek.data_dir) in your settings!")
+
+    def data_manager(request):
+        data_dir = request.registry.settings["fietsboek.data_dir"]
+        return DataManager(Path(data_dir))
+
     settings['enable_account_registration'] = asbool(
         settings.get('enable_account_registration', 'false'))
     settings['available_locales'] = aslist(
@@ -63,6 +73,7 @@ def main(global_config, **settings):
         config.set_csrf_storage_policy(CookieCSRFStoragePolicy())
         config.set_default_csrf_options(require_csrf=True)
         config.set_locale_negotiator(locale_negotiator)
+        config.add_request_method(data_manager, reify=True)
 
     jinja2_env = config.get_jinja2_environment()
     jinja2_env.filters['format_decimal'] = fiets_jinja2.filter_format_decimal
diff --git a/fietsboek/data.py b/fietsboek/data.py
new file mode 100644
index 0000000..992ddf7
--- /dev/null
+++ b/fietsboek/data.py
@@ -0,0 +1,118 @@
+"""Data manager for fietsboek.
+
+Data are objects that belong to a track (such as images), but are not stored in
+the database itself. This module makes access to such data objects easier.
+"""
+import random
+import string
+import shutil
+import uuid
+
+from .util import secure_filename
+
+
+def generate_filename(filename):
+    """Generates a safe-to-use filename for uploads.
+
+    If possible, tries to keep parts of the original filename intact, such as
+    the extension.
+
+    :param filename: The original filename.
+    :type filename: str
+    :return: The generated filename.
+    :rtype: str
+    """
+    if filename:
+        good_name = secure_filename(filename)
+        if good_name:
+            random_prefix = "".join(random.choice(string.ascii_lowercase) for _ in range(5))
+            return f"{random_prefix}_{good_name}"
+
+    return str(uuid.uuid4())
+
+
+class DataManager:
+    """Data manager.
+
+    The data manager is usually provided as ``request.data_manager`` and can be
+    used to access track's images and other on-disk data.
+
+    :ivar data_dir: Path to the data folder.
+    :vartype data_dir: pathlib.Path
+    """
+
+    def __init__(self, data_dir):
+        self.data_dir = data_dir
+
+    def _track_data_dir(self, track_id):
+        return self.data_dir / "tracks" / str(track_id)
+
+    def images(self, track_id):
+        """Returns a list of images that belong to a track.
+
+        :param track_id: Numerical ID of the track.
+        :type track_id: int
+        :return: A list of image IDs.
+        :rtype: list[str]
+        """
+        image_dir = self._track_data_dir(track_id) / "images"
+        if not image_dir.exists():
+            return []
+        images = []
+        for image in image_dir.iterdir():
+            images.append(image.name)
+        return images
+
+    def image_path(self, track_id, image_id):
+        """Returns a path to a saved image.
+
+        :raises FileNotFoundError: If the given image could not be found.
+        :param track_id: ID of the track.
+        :type track_id: int
+        :param image_id: ID of the image.
+        :type image_id: str
+        :return: A path pointing to the requested image.
+        :rtype: pathlib.Path
+        """
+        image = self._track_data_dir(track_id) / "images" / secure_filename(image_id)
+        if not image.exists():
+            raise FileNotFoundError("The requested image does not exist")
+        return image
+
+    def add_image(self, track_id, image, filename=None):
+        """Saves an image to a track.
+
+        :param track_id: ID of the track.
+        :type track_id: int
+        :param image: The image, as a file-like object to read from.
+        :type image: file
+        :param filename: The image's original filename.
+        :type filename: str
+        :return: The ID of the saved image.
+        :rtype: str
+        """
+        image_dir = self._track_data_dir(track_id) / "images"
+        image_dir.mkdir(parents=True, exist_ok=True)
+
+        filename = generate_filename(filename)
+        path = image_dir / filename
+        with open(path, "wb") as fobj:
+            shutil.copyfileobj(image, fobj)
+
+        return filename
+
+    def delete_image(self, track_id, image_id):
+        """Deletes an image from a track.
+
+        :raises FileNotFoundError: If the given image could not be found.
+        :param track_id: ID of the track.
+        :type track_id: int
+        :param image_id: ID of the image.
+        :type image_id: str
+        """
+        # Be sure to not delete anything else than the image file
+        image_id = secure_filename(image_id)
+        if '/' in image_id or '\\' in image_id:
+            return
+        path = self.image_path(track_id, image_id)
+        path.unlink()
diff --git a/fietsboek/routes.py b/fietsboek/routes.py
index a28f926..9d705c1 100644
--- a/fietsboek/routes.py
+++ b/fietsboek/routes.py
@@ -35,6 +35,8 @@ def includeme(config):
                      factory='fietsboek.models.Track.factory')
     config.add_route('add-comment', '/track/{track_id}/comment',
                      factory='fietsboek.models.Track.factory')
+    config.add_route('image', '/track/{track_id}/images/{image_name}',
+                     factory='fietsboek.models.Track.factory')
 
     config.add_route('badge', '/badge/{badge_id}',
                      factory='fietsboek.models.Badge.factory')
diff --git a/fietsboek/static/fietsboek.js b/fietsboek/static/fietsboek.js
index d27e21e..e403628 100644
--- a/fietsboek/static/fietsboek.js
+++ b/fietsboek/static/fietsboek.js
@@ -69,6 +69,48 @@ function removeFriendClicked(event) {
   button.parentNode.parentNode.removeChild(button.parentNode);
 }
 
+function imageSelectorChanged(event) {
+  console.log(event.target.files);
+
+  for (var file of event.target.files) {
+    const input = document.createElement("input");
+    input.type = "file";
+    input.hidden = true;
+    input.name = "image[]";
+
+    const transfer = new DataTransfer();
+    transfer.items.add(file);
+    input.files = transfer.files;
+
+    let preview = document.querySelector("#trackImagePreviewBlueprint").cloneNode(true);
+    preview.removeAttribute("id");
+    preview.querySelector("img").src = URL.createObjectURL(file);
+    preview.querySelector("button.delete-image").addEventListener("click", deleteImageButtonClicked);
+    preview.appendChild(input);
+
+    document.querySelector("#trackImageList").appendChild(preview);
+  }
+
+  event.target.value = "";
+}
+
+function deleteImageButtonClicked(event) {
+  let preview = event.target.closest("div.track-image-preview");
+  /* If this was a image yet to be uploaded, simply remove it */
+  let input = preview.querySelector("input[type=file]");
+  if (input) {
+    preview.parentNode.removeChild(preview);
+    return;
+  }
+
+  /* Otherwise, we need to remove it but also insert a "delete-image" input */
+  let deleter = preview.querySelector("input[type=hidden]");
+  deleter.removeAttribute("disabled");
+  preview.removeChild(deleter);
+  preview.parentNode.appendChild(deleter);
+  preview.parentNode.removeChild(preview);
+}
+
 document.addEventListener('DOMContentLoaded', function(event) {
   /* Enable the "Add tag" button in the track edit page */
   let $ = (selector) => document.querySelector(selector);
@@ -150,4 +192,15 @@ document.addEventListener('DOMContentLoaded', function(event) {
       $("#archiveDownloadButton").disabled = (checked.length == 0);
     });
   });
+
+  /* Enable the image selector */
+  var button = $("#imageSelector");
+  if (button) {
+    button.addEventListener("change", imageSelectorChanged);
+  }
+
+  /* Enable the "delete image" buttons for the already existing images */
+  document.querySelectorAll("button.delete-image").forEach((b) => {
+    b.addEventListener("click", deleteImageButtonClicked);
+  });
 });
diff --git a/fietsboek/static/theme.css b/fietsboek/static/theme.css
index 831b1f6..076b519 100644
--- a/fietsboek/static/theme.css
+++ b/fietsboek/static/theme.css
@@ -18,6 +18,42 @@ strong {
   align-items: center;
 }
 
+.carousel-item img {
+  max-height: 700px;
+  margin: auto;
+}
+
+#trackImageList {
+  display: flex;
+  flex-wrap: wrap;
+}
+
+.track-image-preview button {
+  position: absolute;
+  z-index: 5;
+  background-color: white;
+  right: 0px;
+}
+
+.track-image-preview img {
+  max-width: 100%;
+  max-height: 100%;
+  position: absolute;
+  top: 50%;
+  left: 50%;
+  transform: translate(-50%, -50%);
+}
+
+.track-image-preview {
+  position: relative;
+  width: min(317px, 100%);
+  height: calc(9 / 16 * 317px);
+  border-radius: 5px;
+  border: 1px solid grey;
+  margin-right: 5px;
+  margin-bottom: 5px;
+}
+
 .track-description img {
     max-width: 100%;
 }
diff --git a/fietsboek/templates/details.jinja2 b/fietsboek/templates/details.jinja2
index 4d88224..2ec62ec 100644
--- a/fietsboek/templates/details.jinja2
+++ b/fietsboek/templates/details.jinja2
@@ -137,6 +137,27 @@
   </div>
   <hr>
   {% endif %}
+  {% if images %}
+  <div id="trackImageShowcase" class="carousel carousel-dark slide" data-bs-ride="carousel">
+    <div class="carousel-inner">
+      {% for image in images %}
+      <div class="carousel-item{% if loop.first %} active{% endif %}">
+        <a href="{{ image }}" target="_blank">
+          <img src="{{ image }}" class="d-block">
+        </a>
+      </div>
+      {% endfor %}
+    </div>
+    <button class="carousel-control-prev" type="button" data-bs-target="#trackImageShowcase" data-bs-slide="prev">
+      <span class="carousel-control-prev-icon" aria-hidden="true"></span>
+      <span class="visually-hidden">Previous</span>
+    </button>
+    <button class="carousel-control-next" type="button" data-bs-target="#trackImageShowcase" data-bs-slide="next">
+      <span class="carousel-control-next-icon" aria-hidden="true"></span>
+      <span class="visually-hidden">Next</span>
+    </button>
+  </div>
+  {% endif %}
   <h2>{{ _("page.details.comments") }}</h2>
   {% for comment in track.comments %}
   <div class="card mb-3">
diff --git a/fietsboek/templates/edit.jinja2 b/fietsboek/templates/edit.jinja2
index 6053509..4e64f69 100644
--- a/fietsboek/templates/edit.jinja2
+++ b/fietsboek/templates/edit.jinja2
@@ -9,8 +9,8 @@
   <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_raw, track.date_tz or 0, track.visibility, track.description, track.text_tags(), badges, track.tagged_people) }}
+  <form method="POST" enctype="multipart/form-data">
+    {{ edit_form.edit_track(track.title, track.date_raw, track.date_tz or 0, track.visibility, track.description, track.text_tags(), badges, track.tagged_people, images) }}
     {{ util.hidden_csrf_input() }}
     <div class="btn-group" role="group">
         <button type="submit" class="btn btn-primary"><i class="bi bi-save"></i> {{ _("page.edit.form.submit") }}</button>
diff --git a/fietsboek/templates/edit_form.jinja2 b/fietsboek/templates/edit_form.jinja2
index 3779d5d..0ba933e 100644
--- a/fietsboek/templates/edit_form.jinja2
+++ b/fietsboek/templates/edit_form.jinja2
@@ -1,4 +1,4 @@
-{% macro edit_track(title, date, date_tz, visibility, description, tags, badges, friends) %}
+{% macro edit_track(title, date, date_tz, visibility, description, tags, badges, friends, images) %}
 <div class="mb-3">
   <label for="formTitle" class="form-label">{{ _("page.track.form.title") }}</label>
   <input class="form-control" type="text" id="formTitle" name="title" value="{{ title | default("", true) }}">
@@ -87,4 +87,24 @@
   <label for="formDesc" class="form-label">{{ _("page.track.form.description") }}</label>
   <textarea class="form-control" id="formDesc" name="description" rows="5">{{ description | default("", true) }}</textarea>
 </div>
+<div class="mb-3">
+  <div id="trackImageList">
+    {% for image in images %}
+    <div class="track-image-preview">
+      <button type="button" class="btn-close delete-image" aria-label="{{ _("page.track.form.remove_image") }}"></button>
+      <input type="hidden" name="delete-image[]" value="{{ image.name }}" disabled>
+      <img src="{{ image.url }}">
+    </div>
+    {% endfor %}
+  </div>
+  <input type="file" name="image[]" id="imageSelector" class="form-control" accept="image/*" style="display:none;" multiple>
+  <button type="button" onclick="document.querySelector('#imageSelector').click()" class="btn btn-primary"><i class="bi bi-images"></i> {{ _("page.track.form.select_images") }}</button>
+</div>
+<!-- Mode hidden templates -->
+<div style="display:none;">
+  <div id="trackImagePreviewBlueprint" class="track-image-preview">
+    <button type="button" class="btn-close delete-image" aria-label="{{ _("page.track.form.remove_image") }}"></button>
+    <img>
+  </div>
+</div>
 {% endmacro %}
diff --git a/fietsboek/templates/finish_upload.jinja2 b/fietsboek/templates/finish_upload.jinja2
index 58c67d4..d6f9f00 100644
--- a/fietsboek/templates/finish_upload.jinja2
+++ b/fietsboek/templates/finish_upload.jinja2
@@ -9,8 +9,8 @@
   <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_date_tz, upload_visibility, upload_description, upload_tags, badges, upload_tagged_people) }}
+  <form method="POST" enctype="multipart/form-data">
+    {{ edit_form.edit_track(upload_title, upload_date, upload_date_tz, upload_visibility, upload_description, upload_tags, badges, upload_tagged_people, []) }}
     {{ util.hidden_csrf_input() }}
     <div class="btn-group" role="group">
       <button type="submit" class="btn btn-primary">{{ _("page.upload.form.submit") }}</button>
diff --git a/fietsboek/util.py b/fietsboek/util.py
index 1014c87..bab2074 100644
--- a/fietsboek/util.py
+++ b/fietsboek/util.py
@@ -2,6 +2,9 @@
 import random
 import string
 import datetime
+import re
+import os
+import unicodedata
 
 # Compat for Python < 3.9
 import importlib_resources
@@ -25,6 +28,22 @@ ALLOWED_ATTRIBUTES = dict(bleach.sanitizer.ALLOWED_ATTRIBUTES)
 ALLOWED_ATTRIBUTES['img'] = ['alt', 'src']
 
 
+_filename_ascii_strip_re = re.compile(r"[^A-Za-z0-9_.-]")
+_windows_device_files = (
+    "CON",
+    "AUX",
+    "COM1",
+    "COM2",
+    "COM3",
+    "COM4",
+    "LPT1",
+    "LPT2",
+    "LPT3",
+    "PRN",
+    "NUL",
+)
+
+
 def safe_markdown(md_source):
     """Transform a markdown document into a safe HTML document.
 
@@ -289,3 +308,53 @@ def read_localized_resource(locale_name, path, raise_on_error=False):
     if raise_on_error:
         raise FileNotFoundError(f"Resource {path!r} not found")
     return f"{locale_name}:{path}"
+
+
+def secure_filename(filename):
+    r"""Pass it a filename and it will return a secure version of it.  This
+    filename can then safely be stored on a regular file system and passed
+    to :func:`os.path.join`.  The filename returned is an ASCII only string
+    for maximum portability.
+    On windows systems the function also makes sure that the file is not
+    named after one of the special device files.
+
+    >>> secure_filename("My cool movie.mov")
+    'My_cool_movie.mov'
+    >>> secure_filename("../../../etc/passwd")
+    'etc_passwd'
+    >>> secure_filename('i contain cool \xfcml\xe4uts.txt')
+    'i_contain_cool_umlauts.txt'
+
+    The function might return an empty filename.  It's your responsibility
+    to ensure that the filename is unique and that you abort or
+    generate a random filename if the function returned an empty one.
+
+    :param filename: the filename to secure
+    :type filename: str
+    :return: The secure filename.
+    :rtype: str
+    """
+    # Taken from
+    # https://github.com/pallets/werkzeug/blob/main/src/werkzeug/utils.py
+
+    filename = unicodedata.normalize("NFKD", filename)
+    filename = filename.encode("ascii", "ignore").decode("ascii")
+
+    for sep in os.path.sep, os.path.altsep:
+        if sep:
+            filename = filename.replace(sep, " ")
+    filename = str(_filename_ascii_strip_re.sub("", "_".join(filename.split()))).strip(
+        "._"
+    )
+
+    # on nt a couple of special files are present in each folder.  We
+    # have to ensure that the target file is not such a filename.  In
+    # this case we prepend an underline
+    if (
+        os.name == "nt"
+        and filename
+        and filename.split(".", maxsplit=1)[0].upper() in _windows_device_files
+    ):
+        filename = f"_{filename}"
+
+    return filename
diff --git a/fietsboek/views/detail.py b/fietsboek/views/detail.py
index a15b29d..3953f47 100644
--- a/fietsboek/views/detail.py
+++ b/fietsboek/views/detail.py
@@ -2,9 +2,9 @@
 import datetime
 
 from pyramid.view import view_config
-from pyramid.response import Response
+from pyramid.response import Response, FileResponse
 from pyramid.i18n import TranslationString as _
-from pyramid.httpexceptions import HTTPFound
+from pyramid.httpexceptions import HTTPFound, HTTPNotFound
 
 from .. import models, util
 
@@ -22,12 +22,17 @@ def details(request):
     track = request.context
     description = util.safe_markdown(track.description)
     show_edit_link = (track.owner == request.identity)
+    images = [
+        request.route_url("image", track_id=track.id, image_name=image)
+        for image in request.data_manager.images(track.id)
+    ]
     return {
         'track': track,
         'show_edit_link': show_edit_link,
         'mps_to_kph': util.mps_to_kph,
         'comment_md_to_html': util.safe_markdown,
         'description': description,
+        'images': images,
     }
 
 
@@ -85,6 +90,27 @@ def badge(request):
     return Response(request.context.image)
 
 
+@view_config(route_name='image', http_cache=3600, permission='track.view')
+def image(request):
+    """Returns the image data for the requested image.
+
+    This ensures that the image is sent efficiently, by delegating to the WSGI
+    file wrapper if possible.
+
+    :param request: The Pyramid request.
+    :type request: pyramid.request.Request
+    :return: The HTTP response.
+    :rtype: pyramid.response.Response
+    """
+    track = request.context
+    try:
+        image_path = request.data_manager.image_path(track.id, request.matchdict['image_name'])
+    except FileNotFoundError:
+        return HTTPNotFound()
+    else:
+        return FileResponse(image_path, request)
+
+
 @view_config(route_name="add-comment", request_method="POST", permission="track.comment")
 def add_comment(request):
     """Endpoint to add a comment to a track.
diff --git a/fietsboek/views/edit.py b/fietsboek/views/edit.py
index aabbcbc..04ed000 100644
--- a/fietsboek/views/edit.py
+++ b/fietsboek/views/edit.py
@@ -1,5 +1,6 @@
 """Views for editing a track."""
 import datetime
+from collections import namedtuple
 
 from pyramid.view import view_config
 from pyramid.httpexceptions import HTTPFound, HTTPBadRequest
@@ -10,6 +11,9 @@ from .. import models, util
 from ..models.track import Visibility
 
 
+_Image = namedtuple("_Image", "name url")
+
+
 @view_config(route_name='edit', renderer='fietsboek:templates/edit.jinja2',
              permission='track.edit', request_method='GET')
 def edit(request):
@@ -23,9 +27,14 @@ def edit(request):
     track = request.context
     badges = request.dbsession.execute(select(models.Badge)).scalars()
     badges = [(badge in track.badges, badge) for badge in badges]
+    images = [
+        _Image(image, request.route_url("image", track_id=track.id, image_name=image))
+        for image in request.data_manager.images(track.id)
+    ]
     return {
         'track': track,
         'badges': badges,
+        'images': images,
     }
 
 
@@ -61,4 +70,12 @@ def do_edit(request):
     tags = request.params.getall("tag[]")
     track.sync_tags(tags)
 
+    for image in request.params.getall("image[]"):
+        if image == b"":
+            continue
+        request.data_manager.add_image(track.id, image.file, image.filename)
+
+    for image in request.params.getall("delete-image[]"):
+        request.data_manager.delete_image(track.id, image)
+
     return HTTPFound(request.route_url('details', track_id=track.id))
diff --git a/fietsboek/views/upload.py b/fietsboek/views/upload.py
index 979291e..4f42b80 100644
--- a/fietsboek/views/upload.py
+++ b/fietsboek/views/upload.py
@@ -169,6 +169,12 @@ def do_finish_upload(request):
     track.ensure_cache()
     request.dbsession.add(track.cache)
 
+    # Don't forget to add the images
+    for image in request.params.getall("image[]"):
+        if image == b"":
+            continue
+        request.data_manager.add_image(track.id, image.file, image.filename)
+
     request.session.flash(request.localizer.translate(_("flash.upload_success")))
 
     return HTTPFound(request.route_url('details', track_id=track.id))