Merge branch 'multi-download'

12 files changed, 248 insertions, 37 deletions

diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py
index d2e736a..95344d9 100644
--- a/fietsboek/models/track.py
+++ b/fietsboek/models/track.py
@@ -276,7 +276,7 @@ class Track(Base):
     def date(self, value):
         if value.tzinfo is None:
             LOGGER.debug('Non-aware datetime passed (track_id=%d, value=%s), assuming offset=0',
-                         self.id, value)
+                         self.id or -1, value)
             self.date_tz = 0
         else:
             self.date_tz = value.tzinfo.utcoffset(value).total_seconds() // 60
@@ -577,7 +577,7 @@ class TrackCache(Base):
     def start_time(self, value):
         if value.tzinfo is None:
             LOGGER.debug('Non-aware datetime passed (cache_id=%d, value=%s), assuming offset=0',
-                         self.id, value)
+                         self.id or -1, value)
             self.start_time_tz = 0
         else:
             self.start_time_tz = value.tzinfo.utcoffset(value).total_seconds() // 60
@@ -600,7 +600,7 @@ class TrackCache(Base):
     def end_time(self, value):
         if value.tzinfo is None:
             LOGGER.debug('Non-aware datetime passed (cache_id=%d, value=%s), assuming offset=0',
-                         self.id, value)
+                         self.id or -1, value)
             self.end_time_tz = 0
         else:
             self.end_time_tz = value.tzinfo.utcoffset(value).total_seconds() // 60
diff --git a/fietsboek/routes.py b/fietsboek/routes.py
index 2ea529c..a28f926 100644
--- a/fietsboek/routes.py
+++ b/fietsboek/routes.py
@@ -9,6 +9,8 @@ def includeme(config):
     config.add_route('logout', '/logout')
     config.add_route('browse', '/track/')
 
+    config.add_route('track-archive', '/track/archive')
+
     config.add_route('password-reset', '/password-reset')
     config.add_route('use-token', '/token/{uuid}')
     config.add_route('create-account', '/create-account')
diff --git a/fietsboek/static/fietsboek.js b/fietsboek/static/fietsboek.js
index dd5b9bf..d27e21e 100644
--- a/fietsboek/static/fietsboek.js
+++ b/fietsboek/static/fietsboek.js
@@ -130,4 +130,24 @@ document.addEventListener('DOMContentLoaded', function(event) {
     }, false)
   })
 
+  /* Enable the "Download archive" button */
+  var button = $("#archiveDownloadButton");
+  if (button) {
+    button.addEventListener('click', () => {
+      let checked = document.querySelectorAll(".archive-checkbox:checked");
+      let url = new URL("/track/archive", window.location);
+      checked.forEach((c) => {
+        url.searchParams.append("track_id[]", c.value);
+      });
+      window.location.assign(url);
+    });
+  }
+
+  /* Enable checkbox listeners */
+  document.querySelectorAll(".archive-checkbox").forEach((c) => {
+    c.addEventListener("change", () => {
+      let checked = document.querySelectorAll(".archive-checkbox:checked");
+      $("#archiveDownloadButton").disabled = (checked.length == 0);
+    });
+  });
 });
diff --git a/fietsboek/templates/browse.jinja2 b/fietsboek/templates/browse.jinja2
index 10b16e3..2732984 100644
--- a/fietsboek/templates/browse.jinja2
+++ b/fietsboek/templates/browse.jinja2
@@ -2,9 +2,11 @@
 {% block content %}
 <div class="container">
   <h1>{{ _("page.browse.title") }}</h1>
+  {% if tracks %}
   {% for track in tracks %}
   <div class="card mb-3">
     <h5 class="card-header">
+      <input type="checkbox" class="form-check-input archive-checkbox" name="track_id[]" value="{{ track.id }}">
       <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 %}
@@ -55,7 +57,8 @@
     </div>
   </div>
   {% endfor %}
-  {% if not tracks %}
+  <button type="button" class="btn btn-primary" id="archiveDownloadButton" disabled><i class="bi bi-file-earmark-zip"></i> {{ _("page.browse.download_multiple") }}</button>
+  {% else %}
   <p>{{ _("page.browse.no_tracks") }}</p>
   {% endif %}
 </div>
diff --git a/fietsboek/views/browse.py b/fietsboek/views/browse.py
index 3739061..17dd45a 100644
--- a/fietsboek/views/browse.py
+++ b/fietsboek/views/browse.py
@@ -1,11 +1,37 @@
 """Views for browsing all tracks."""
+from io import RawIOBase
+from zipfile import ZipFile, ZIP_DEFLATED
+
 from pyramid.view import view_config
+from pyramid.httpexceptions import HTTPForbidden, HTTPNotFound
+from pyramid.response import Response
 
 from sqlalchemy import select
 
 from .. import models, util
 
 
+class Stream(RawIOBase):
+    """A :class:`Stream` represents an in-memory buffered FIFO.
+
+    This is useful for the zipfile module, as it needs a file-like object, but
+    we do not want to create an actual temporary file.
+    """
+
+    def __init__(self):
+        super().__init__()
+        self.buffer = []
+
+    def write(self, b):
+        self.buffer.append(b)
+        return len(b)
+
+    def readall(self):
+        buf = self.buffer
+        self.buffer = []
+        return b"".join(buf)
+
+
 def visible_tracks(dbsession, user):
     """Returns all visible tracks for the given user.
 
@@ -43,3 +69,44 @@ def browse(request):
         'tracks': tracks,
         'mps_to_kph': util.mps_to_kph,
     }
+
+
+@view_config(route_name="track-archive", request_method="GET")
+def archive(request):
+    """Packs multiple tracks into a single archive.
+
+    :param request: The Pyramid request.
+    :type request: pyramid.request.Request
+    :return: The HTTP response.
+    :rtype: pyramid.response.Response
+    """
+    # We need to create a separate session, otherwise we will get detached instances
+    session = request.registry['dbsession_factory']()
+
+    track_ids = set(map(int, request.params.getall("track_id[]")))
+    tracks = session.execute(
+        select(models.Track).filter(models.Track.id.in_(track_ids))).scalars().fetchall()
+
+    if len(tracks) != len(track_ids):
+        return HTTPNotFound()
+
+    for track in tracks:
+        if not track.is_visible_to(request.identity):
+            return HTTPForbidden()
+
+    def generate():
+        try:
+            stream = Stream()
+            with ZipFile(stream, "w", ZIP_DEFLATED) as zipfile:
+                for track in tracks:
+                    zipfile.writestr(f"track_{track.id}.gpx", track.gpx_data)
+                    yield stream.readall()
+            yield stream.readall()
+        finally:
+            session.close()
+
+    return Response(
+        app_iter=generate(),
+        content_type="application/zip",
+        content_disposition="attachment; filename=tracks.zip",
+    )
diff --git a/pytest.ini b/pytest.ini
index 1fffd6f..f0d41e6 100644
--- a/pytest.ini
+++ b/pytest.ini
@@ -1,5 +1,6 @@
 [pytest]
-addopts = --strict-markers
+addopts = --strict-markers --import-mode=importlib
+pythonpath = tests
 
 testpaths =
     fietsboek
diff --git a/tests/conftest.py b/tests/conftest.py
index b8e3090..79f0245 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -141,3 +141,25 @@ def route_path(app_request):
     def get_route_path(*args, **kwargs):
         return app_request.route_path(*args, **kwargs)
     return get_route_path
+
+
+@pytest.fixture()
+def logged_in(dbsession, testapp, route_path):
+    """
+    A fixture that represents a logged in state.
+
+    This automatically creates a user and returns the created user.
+
+    Returns the user that was logged in.
+    """
+    user = models.User(email='foo@bar.com', is_verified=True)
+    user.set_password("foobar")
+    dbsession.add(user)
+
+    login = testapp.get(route_path('login'))
+    form = login.form
+    form['email'] = 'foo@bar.com'
+    form['password'] = 'foobar'
+    response = form.submit()
+    assert response.status_code == 302
+    return user
diff --git a/tests/integration/test_browse.py b/tests/integration/test_browse.py
new file mode 100644
index 0000000..cfd1d71
--- /dev/null
+++ b/tests/integration/test_browse.py
@@ -0,0 +1,91 @@
+import io
+import zipfile
+from contextlib import contextmanager
+from datetime import datetime
+
+from sqlalchemy import select, func
+from webtest import Upload
+
+from testutils import load_gpx_asset
+from fietsboek import models
+from fietsboek.models.track import Visibility
+
+
+@contextmanager
+def added_tracks(tm, dbsession, owner):
+    """Adds some tracks to the database session.
+
+    This function should be used as a context manager and it ensures that the
+    added tracks are deleted again after the test, to make a clean slate for
+    the next test.
+    """
+    # The normal transaction is "doomed", so we need to abort it, start a fresh
+    # one, and then explicitely commit it, otherwise we will not persist the
+    # objects to the database.
+    tm.abort()
+
+    tracks = []
+    with tm:
+        track = models.Track(
+            owner=owner,
+            title="Foobar",
+            visibility=Visibility.PUBLIC,
+            description="A foo'd track",
+            badges=[],
+            link_secret="foobar",
+            tagged_people=[],
+        )
+        track.date = datetime(2022, 3, 14, 9, 26, 54)
+        track.gpx_data = load_gpx_asset("MyTourbook_1.gpx.gz")
+        dbsession.add(track)
+        tracks.append(track)
+
+        track = models.Track(
+            owner=owner,
+            title="Barfoo",
+            visibility=Visibility.PUBLIC,
+            description="A bar'd track",
+            badges=[],
+            link_secret="barfoo",
+            tagged_people=[],
+        )
+        track.date = datetime(2022, 10, 29, 13, 37, 11)
+        track.gpx_data = load_gpx_asset("Teasi_1.gpx.gz")
+        dbsession.add(track)
+        tracks.append(track)
+
+    tm.begin()
+    tm.doom()
+
+    try:
+        yield tracks
+    finally:
+        tm.abort()
+        with tm:
+            for track in tracks:
+                dbsession.delete(track)
+        tm.begin()
+        tm.doom()
+
+
+def test_browse(testapp, dbsession, route_path, logged_in, tm):
+    # Ensure there are some tracks in the database
+    with added_tracks(tm, dbsession, logged_in):
+        # Now go to the browse page
+        browse = testapp.get(route_path('browse'))
+
+        assert "Foobar" in browse.text
+        assert "Barfoo" in browse.text
+
+
+def test_archive(testapp, dbsession, route_path, logged_in, tm):
+    with added_tracks(tm, dbsession, logged_in):
+        archive = testapp.get(
+            route_path('track-archive', _query=[("track_id[]", "1"), ("track_id[]", "2")])
+        )
+        result = io.BytesIO(archive.body)
+
+        with zipfile.ZipFile(result, 'r') as zipped:
+            assert len(zipped.namelist()) == 2
+            assert "track_1.gpx" in zipped.namelist()
+            assert "track_2.gpx" in zipped.namelist()
diff --git a/tests/integration/test_upload.py b/tests/integration/test_upload.py
index 651aedf..1903cbe 100644
--- a/tests/integration/test_upload.py
+++ b/tests/integration/test_upload.py
@@ -1,34 +1,9 @@
-import gzip
-from pathlib import Path
-
-import pytest
 from sqlalchemy import select, func
 from webtest import Upload
 
+from testutils import load_gpx_asset
 from fietsboek import models
 
-
-@pytest.fixture()
-def logged_in(dbsession, testapp, route_path):
-    user = models.User(email='foo@bar.com', is_verified=True)
-    user.set_password("foobar")
-    dbsession.add(user)
-
-    login = testapp.get(route_path('login'))
-    form = login.form
-    form['email'] = 'foo@bar.com'
-    form['password'] = 'foobar'
-    response = form.submit()
-    assert response.status_code == 302
-
-
-def load_gpx_asset(filename):
-    asset_dir = Path(__file__).parent.parent / 'assets'
-    test_file = asset_dir / filename
-    with gzip.open(test_file, 'rb') as fobj:
-        return fobj.read()
-
-
 def test_upload_forbidden(testapp, route_path):
     upload_form = testapp.get(route_path('upload'), status="4*")
 
diff --git a/tests/testutils.py b/tests/testutils.py
new file mode 100644
index 0000000..3ddbdbe
--- /dev/null
+++ b/tests/testutils.py
@@ -0,0 +1,20 @@
+"""Various utility functions for testing."""
+import gzip
+from pathlib import Path
+
+
+def load_gpx_asset(filename):
+    """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()
diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py
index e63bd01..debf229 100644
--- a/tests/unit/test_util.py
+++ b/tests/unit/test_util.py
@@ -1,11 +1,10 @@
-import gzip
 from datetime import timedelta
-from pathlib import Path
 
 import pytest
 import gpxpy
 from markupsafe import Markup
 
+from testutils import load_gpx_asset
 from fietsboek import util
 
 
@@ -59,10 +58,7 @@ def test_round_timedelta_to_multiple(delta, multiple, expected):
     ("Teasi_1.gpx.gz", timedelta(hours=2)),
 ])
 def test_guess_gpx_timezone(gpx_file, offset):
-    asset_dir = Path(__file__).parent.parent / 'assets'
-    test_file = asset_dir / gpx_file
-    with gzip.open(test_file, 'rb') as fobj:
-        parsed_gpx = gpxpy.parse(fobj)
+    parsed_gpx = gpxpy.parse(load_gpx_asset(gpx_file))
     timezone = util.guess_gpx_timezone(parsed_gpx)
     # Here we hope (and assume) that utcoffset is ignored. This is true for
     # datetime.timezone objects, but may not be for other datetime.tzinfo
diff --git a/tests/unit/views/test_browse.py b/tests/unit/views/test_browse.py
new file mode 100644
index 0000000..93eb0ae
--- /dev/null
+++ b/tests/unit/views/test_browse.py
@@ -0,0 +1,14 @@
+from fietsboek.views.browse import Stream
+
+
+class TestStream:
+    def test_write(self):
+        stream = Stream()
+        n = stream.write(b"foobar")
+        assert n == 6
+
+    def test_write_read(self):
+        stream = Stream()
+        stream.write(b"foo")
+        stream.write(b"bar")
+        assert stream.readall() == b"foobar"