Merge branch 'postgres-support'HEADmaster

14 files changed, 258 insertions, 51 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9263e91..20307c4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -4,7 +4,7 @@ variables:
   PIP_CACHE_DIR: "$CI_PROJECT_DIR/.cache/pip"
 
 default:
-  image: python:bullseye
+  image: python:bookworm
   # Pip's cache doesn't store the python packages
   # https://pip.pypa.io/en/stable/topics/caching/
   #
@@ -17,16 +17,15 @@ default:
 
   before_script:
     - python --version  # For debugging
+    - dpkg -s libsqlite3-0
     - pip install tox
 
 test:
+  parallel:
+    matrix:
+      - DB: ["sqlite", "postgres"]
   script:
-    - pip install poetry && pip install "playwright=="$(poetry show playwright | grep version | cut -f 2 -d ":" | tr -d " ")
-    - playwright install firefox
-    - playwright install-deps
-    - apt install -y redis-server
-    - redis-server >/dev/null 2>&1 &
-    - tox -e python -- --browser firefox
+    - ci/run_tests.sh $DB
 
 # test-pypy:
 #   image: pypy:3
diff --git a/ci/run_tests.sh b/ci/run_tests.sh
new file mode 100755
index 0000000..200f794
--- /dev/null
+++ b/ci/run_tests.sh
@@ -0,0 +1,48 @@
+#!/bin/bash
+set -euxo pipefail
+
+DB=$1
+
+PPATH="/usr/lib/postgresql/15/bin/"
+
+setup_postgres() {
+    apt update
+    apt install -y postgresql postgresql-client sudo
+    echo -n "postgres" >/tmp/pw
+    mkdir /tmp/postgres-db
+    chown postgres:postgres /tmp/postgres-db
+    sudo -u postgres "$PPATH/initdb" --pwfile /tmp/pw -U postgres /tmp/postgres-db
+    sudo -u postgres "$PPATH/postgres" -D /tmp/postgres-db >/dev/null 2>&1 &
+}
+
+setup_redis() {
+    apt install -y redis-server
+    redis-server >/dev/null 2>&1 &
+}
+
+setup_playwright() {
+    pip install poetry
+    pip install "playwright=="$(poetry show playwright | grep version | cut -f 2 -d ":" | tr -d " ")
+    playwright install firefox
+    playwright install-deps
+}
+
+case "$DB" in
+    "sqlite")
+        ;;
+
+    "postgres")
+        setup_postgres
+        sed -i 's|^sqlalchemy.url = .*$|sqlalchemy.url = postgresql://postgres:postgres@localhost/postgres|' testing.ini
+        ;;
+
+    *)
+        echo "Unknown database: $DB"
+        exit 1
+        ;;
+esac
+
+pip install tox
+setup_playwright
+setup_redis
+tox -e python -- --browser firefox
diff --git a/doc/administration/installation.rst b/doc/administration/installation.rst
index b207784..a7c5c27 100644
--- a/doc/administration/installation.rst
+++ b/doc/administration/installation.rst
@@ -6,14 +6,16 @@ This document will outline the installation process of Fietsboek, step-by-step.
 Requirements
 ------------
 
-Fietsboek has the following requirements (apart from the Python modules, which
-will be installed by ``pip``):
+Fietsboek has the following requirements:
 
+* A Linux system
 * Python 3.10 or later
-* A `redis <https://redis.com/>`__ server, used for caching and temporary data
-* (Optionally) an SQL database server like `PostgreSQL
-  <https://www.postgresql.org/>`__ or `MariaDB <https://mariadb.org/>`__ (if
-  SQLite is not enough)
+* A `redis <https://redis.com/>`__ server
+* (Optionally) an SQL database server:
+  * `PostgreSQL <https://www.postgresql.org/>`__
+
+Other systems (such as BSD as operating system, or MariaDB as SQL server) might
+work, but it is not tested.
 
 In addition, if you run on a different interpreter than CPython, you might need
 a working Rust toolchain (``rustc`` and ``cargo``) installed. This is because
@@ -97,6 +99,18 @@ parser to process the GPX files:
 
     .. _issue #7: https://gitlab.com/dunj3/fietsboek/-/issues/7
 
+Optional: Install Database Drivers
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+If you decide to use a database other than SQLite, you must install the
+required drivers:
+
+**PostgreSQL**:
+
+.. code:: bash
+
+    .venv/bin/pip install psycopg2
+
 Configuring Fietsboek
 ---------------------
 
@@ -119,7 +133,7 @@ other update tasks. You can use it to set up the initial database schema:
     instead of a specific version, you must also run ``.venv/bin/alembic -c
     production.ini upgrade head``.
 
-    See :doc:`../developer/updater` ("Furhter notes") for more information.
+    See :doc:`../developer/updater` ("Further notes") for more information.
 
 Adding an Administrator User
 ----------------------------
diff --git a/fietsboek/__init__.py b/fietsboek/__init__.py
index 797d38b..9cd58ed 100644
--- a/fietsboek/__init__.py
+++ b/fietsboek/__init__.py
@@ -20,6 +20,7 @@ from pathlib import Path
 from typing import Callable, Optional
 
 import redis
+import sqlalchemy
 from pyramid.config import Configurator
 from pyramid.csrf import CookieCSRFStoragePolicy
 from pyramid.httpexceptions import HTTPServiceUnavailable
@@ -119,6 +120,22 @@ def check_update_state(config_uri: str):
         LOGGER.warning("Could not determine version state of the data - check `fietsupdate status`")
 
 
+def check_db_engine(sqlalchemy_uri: str):
+    """Checks whether we "support" the given SQL engine.
+
+    :param sqlalchemy_uri: The configured SQLAlchemy URL.
+    """
+    engine = sqlalchemy.create_engine(sqlalchemy_uri)
+    match engine.name:
+        case "sqlite":
+            pass
+        case _:
+            LOGGER.warning(
+                "The configured SQL backend is not well tested in combination with fietsboek. "
+                "Use it at your own risk."
+            )
+
+
 def main(global_config, **settings):
     """This function returns a Pyramid WSGI application."""
     # Avoid a circular import by not importing at the top level
@@ -132,6 +149,8 @@ def main(global_config, **settings):
     parsed_config = mod_config.parse(settings)
     settings["jinja2.newstyle"] = True
 
+    check_db_engine(parsed_config.sqlalchemy_url)
+
     def data_manager(request):
         return DataManager(Path(request.config.data_dir))
 
diff --git a/fietsboek/alembic/versions/20220808_d085998b49ca.py b/fietsboek/alembic/versions/20220808_d085998b49ca.py
index d6353d2..2c5b71d 100644
--- a/fietsboek/alembic/versions/20220808_d085998b49ca.py
+++ b/fietsboek/alembic/versions/20220808_d085998b49ca.py
@@ -14,9 +14,18 @@ down_revision = '091ce24409fe'
 branch_labels = None
 depends_on = None
 
+is_postgres = op.get_bind().dialect.name == "postgresql"
+
 def upgrade():
-    op.add_column('tracks', sa.Column('type', sa.Enum('ORGANIC', 'SYNTHETIC', name='tracktype'), nullable=True))
+    if is_postgres:
+        tracktype = sa.dialects.postgresql.ENUM("ORGANIC", "SYNTHETIC", name="tracktype")
+        tracktype.create(op.get_bind())
+        op.add_column("tracks", sa.Column("type", tracktype, nullable=True))
+    else:
+        op.add_column('tracks', sa.Column('type', sa.Enum('ORGANIC', 'SYNTHETIC', name='tracktype'), nullable=True))
     op.execute("UPDATE tracks SET type='ORGANIC';")
 
 def downgrade():
     op.drop_column('tracks', 'type')
+    if is_postgres:
+        op.execute("DROP TYPE tracktype;")
diff --git a/fietsboek/alembic/versions/20230203_3149aa2d0114.py b/fietsboek/alembic/versions/20230203_3149aa2d0114.py
index eb9ef78..ced8639 100644
--- a/fietsboek/alembic/versions/20230203_3149aa2d0114.py
+++ b/fietsboek/alembic/versions/20230203_3149aa2d0114.py
@@ -16,7 +16,7 @@ depends_on = None
 
 def upgrade():
     op.add_column('tracks', sa.Column('transformers', sa.JSON(), nullable=True))
-    op.execute('UPDATE tracks SET transformers="[]";')
+    op.execute("UPDATE tracks SET transformers='[]';")
 
 def downgrade():
     op.drop_column('tracks', 'transformers')
diff --git a/fietsboek/alembic/versions/20250607_2ebe1bf66430.py b/fietsboek/alembic/versions/20250607_2ebe1bf66430.py
new file mode 100644
index 0000000..d7c811e
--- /dev/null
+++ b/fietsboek/alembic/versions/20250607_2ebe1bf66430.py
@@ -0,0 +1,46 @@
+"""switch transfomers from JSON to TEXT
+
+Revision ID: 2ebe1bf66430
+Revises: 4566843039d6
+Create Date: 2025-06-07 23:24:33.182649
+
+"""
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = '2ebe1bf66430'
+down_revision = '4566843039d6'
+branch_labels = None
+depends_on = None
+
+is_sqlite = op.get_bind().dialect.name == "sqlite"
+
+def upgrade():
+    if is_sqlite:
+        op.add_column('tracks', sa.Column('transformers_text', sa.Text, nullable=True))
+        op.execute('UPDATE tracks SET transformers_text=transformers;')
+        try:
+            op.drop_column('tracks', 'transformers')
+        except sa.exc.OperationalError as exc:
+            logging.getLogger(__name__).warning(
+                "Your SQLite version does not support dropping a column. "
+                "We're setting the content to NULL instead: %s",
+                exc,
+            )
+            op.execute("UPDATE tracks SET transformers = NULL;")
+            op.alter_column("tracks", "transformers", new_column_name="transformers_old_delete_this_column")
+        op.alter_column('tracks', 'transformers_text', new_column_name='transformers')
+    else:
+        op.alter_column('tracks', 'transformers', type_=sa.Text)
+
+def downgrade():
+    if is_sqlite:
+        op.add_column('tracks', sa.Column('transfomers_json', sa.JSON, nullable=True))
+        op.execute('UPDATE tracks SET transformers_json=transformers;')
+        op.drop_column('tracks', 'transformers')
+        op.alter_column('tracks', 'transformers_json', new_column_name='transformers')
+    else:
+        op.alter_column('tracks', 'transformers', type_=sa.JSON)
diff --git a/fietsboek/models/track.py b/fietsboek/models/track.py
index 0737982..0921437 100644
--- a/fietsboek/models/track.py
+++ b/fietsboek/models/track.py
@@ -15,11 +15,13 @@ meta information.
 import datetime
 import enum
 import gzip
+import json
 import logging
 from itertools import chain
 from typing import TYPE_CHECKING, Optional, Union
 
 import gpxpy
+import sqlalchemy.types
 from babel.numbers import format_decimal
 from markupsafe import Markup
 from pyramid.authorization import (
@@ -34,7 +36,6 @@ from pyramid.httpexceptions import HTTPNotFound
 from pyramid.i18n import Localizer
 from pyramid.i18n import TranslationString as _
 from sqlalchemy import (
-    JSON,
     Column,
     DateTime,
     Enum,
@@ -58,6 +59,24 @@ if TYPE_CHECKING:
 LOGGER = logging.getLogger(__name__)
 
 
+class JsonText(sqlalchemy.types.TypeDecorator):
+    """Saves objects serialized as JSON but keeps the column as a Text."""
+
+    # This is straight from the SQLAlchemy documentation, so the non-overriden
+    # methods should be fine.
+    # pylint: disable=too-many-ancestors,abstract-method
+
+    impl = sqlalchemy.types.Text
+
+    cache_ok = True
+
+    def process_bind_param(self, value, dialect):
+        return json.dumps(value)
+
+    def process_result_value(self, value, dialect):
+        return json.loads(value)
+
+
 class Tag(Base):
     """A tag is a single keyword associated with a track.
 
@@ -213,7 +232,7 @@ class Track(Base):
     visibility = Column(Enum(Visibility))
     link_secret = Column(Text)
     type = Column(Enum(TrackType))
-    transformers = Column(JSON)
+    transformers = Column(JsonText)
 
     owner: Mapped["models.User"] = relationship("User", back_populates="tracks")
     cache: Mapped[Optional["TrackCache"]] = relationship(
diff --git a/testing.ini b/testing.ini
index 82fddfd..ed53bdc 100644
--- a/testing.ini
+++ b/testing.ini
@@ -12,6 +12,8 @@ pyramid.debug_notfound = false
 pyramid.debug_routematch = false
 pyramid.default_locale_name = en
 
+# The sqlalchemy.url is overwritten by the test setup script for different
+# database engines. We leave sqlite here as default so a local tox run works fine.
 sqlalchemy.url = sqlite:///%(here)s/testing.sqlite
 # The pytest tests usually overwrite this with a temporary directory. Since
 # this is cleaned on test teardown, we don't want to accidentally delete data
diff --git a/tests/bootstrap/test_new_instance.py b/tests/bootstrap/test_new_instance.py
index dc3076e..05076f4 100644
--- a/tests/bootstrap/test_new_instance.py
+++ b/tests/bootstrap/test_new_instance.py
@@ -2,6 +2,7 @@
 script, as described in the documentation.
 """
 
+import configparser
 import contextlib
 import logging
 import os
@@ -10,6 +11,8 @@ import subprocess
 import venv
 from pathlib import Path
 
+import sqlalchemy
+
 LOGGER = logging.getLogger(__name__)
 REPO_BASE = Path(__file__).parent.parent.parent
 
@@ -51,31 +54,66 @@ def create_config(config_name: Path):
     Path("data").mkdir()
 
 
+def cleanup_database(config_name: Path):
+    """Connects to the database and ensures everything is reset.
+
+    :param config_name: Path to the config file.
+    """
+    if not config_name.exists():
+        return
+
+    parser = configparser.ConfigParser()
+    parser["DEFAULT"]["here"] = str(config_name.parent)
+    parser.read(config_name)
+
+    db_url = parser["app:main"]["sqlalchemy.url"]
+    engine = sqlalchemy.create_engine(db_url)
+
+    match engine.name:
+        case "sqlite":
+            pass
+        case "postgresql":
+            with engine.connect() as connection:
+                connection.execute(sqlalchemy.text("DROP SCHEMA public CASCADE;"))
+                connection.execute(sqlalchemy.text("CREATE SCHEMA public;"))
+                connection.commit()
+
+
 def test_setup_via_fietsupdate(tmpdir):
     with chdir(tmpdir):
-        # We create a new temporary virtual environment with a fresh install, just
-        # to be sure there's as little interference as possible.
-        LOGGER.info("Installing Fietsboek into clean env")
-        binaries_path = install_fietsboek(tmpdir / "venv")
-
-        LOGGER.info("Creating a test configuration")
-        create_config(Path("testing.ini"))
-
-        # Try to run the migrations
-        subprocess.check_call(
-            [binaries_path / "fietsupdate", "update", "-c", "testing.ini", "-f"]
-        )
-
-        # Also try to add an administrator
-        subprocess.check_call([
-            binaries_path / "fietsctl",
-            "user",
-            "add",
-            "-c", "testing.ini",
-            "--email", "foobar@example.com",
-            "--name", "Foo Bar",
-            "--password", "raboof",
-            "--admin",
-        ])
-
-        assert True
+        try:
+            # We create a new temporary virtual environment with a fresh install, just
+            # to be sure there's as little interference as possible.
+            LOGGER.info("Installing Fietsboek into clean env")
+            binaries_path = install_fietsboek(tmpdir / "venv")
+
+            LOGGER.info("Installing additional SQL engines")
+            subprocess.check_call(
+                [binaries_path / "pip", "install", "psycopg2"]
+            )
+
+            LOGGER.info("Creating a test configuration")
+            create_config(Path("testing.ini"))
+
+            # Try to run the migrations
+            subprocess.check_call(
+                [binaries_path / "fietsupdate", "update", "-c", "testing.ini", "-f"]
+            )
+
+            # Also try to add an administrator
+            subprocess.check_call([
+                binaries_path / "fietsctl",
+                "user",
+                "add",
+                "-c", "testing.ini",
+                "--email", "foobar@example.com",
+                "--name", "Foo Bar",
+                "--password", "raboof",
+                "--admin",
+            ])
+
+            assert True
+        finally:
+            # Clean up the database. This is important with anything but SQLite, as
+            # the tables would otherwise persist and interfere with the other tests.
+            cleanup_database(Path("testing.ini"))
diff --git a/tests/conftest.py b/tests/conftest.py
index cd74b0b..652d443 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -52,6 +52,7 @@ def dbengine(app_settings, ini_file):
 
     yield engine
 
+    engine.dispose()
     Base.metadata.drop_all(bind=engine)
     alembic.command.stamp(alembic_cfg, None, purge=True)
 
diff --git a/tests/integration/test_browse.py b/tests/integration/test_browse.py
index 875821d..46ec329 100644
--- a/tests/integration/test_browse.py
+++ b/tests/integration/test_browse.py
@@ -22,6 +22,7 @@ def added_tracks(tm, dbsession, owner, data_manager):
     tm.abort()
 
     tracks = []
+    track_ids = []
     with tm:
         track = models.Track(
             owner=owner,
@@ -37,6 +38,7 @@ def added_tracks(tm, dbsession, owner, data_manager):
         dbsession.flush()
         data_manager.initialize(track.id).compress_gpx(load_gpx_asset("MyTourbook_1.gpx.gz"))
         tracks.append(track)
+        track_ids.append(track.id)
 
         track = models.Track(
             owner=owner,
@@ -52,12 +54,13 @@ def added_tracks(tm, dbsession, owner, data_manager):
         dbsession.flush()
         data_manager.initialize(track.id).compress_gpx(load_gpx_asset("Teasi_1.gpx.gz"))
         tracks.append(track)
+        track_ids.append(track.id)
 
     tm.begin()
     tm.doom()
 
     try:
-        yield tracks
+        yield track_ids
     finally:
         tm.abort()
         with tm:
@@ -80,13 +83,16 @@ def test_browse(testapp, dbsession, route_path, logged_in, tm, data_manager):
 
 def test_archive(testapp, dbsession, route_path, logged_in, tm, data_manager):
     # pylint: disable=too-many-positional-arguments
-    with added_tracks(tm, dbsession, logged_in, data_manager):
+    with added_tracks(tm, dbsession, logged_in, data_manager) as tracks:
         archive = testapp.get(
-            route_path('track-archive', _query=[("track_id[]", "1"), ("track_id[]", "2")])
+            route_path(
+                'track-archive',
+                _query=[("track_id[]", tracks[0]), ("track_id[]", tracks[1])],
+            )
         )
         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()
+            assert f"track_{tracks[0]}.gpx" in zipped.namelist()
+            assert f"track_{tracks[1]}.gpx" in zipped.namelist()
diff --git a/tests/playwright/conftest.py b/tests/playwright/conftest.py
index e914b01..adf5ef3 100644
--- a/tests/playwright/conftest.py
+++ b/tests/playwright/conftest.py
@@ -57,7 +57,11 @@ def dbaccess(app):
     through and the running WSGI app cannot read them.
     """
     session_factory = app.registry["dbsession_factory"]
-    return session_factory()
+    factory = session_factory()
+
+    yield factory
+
+    factory.close()
 
 
 class Helper:
diff --git a/tox.ini b/tox.ini
index 17d6bdb..3fad742 100644
--- a/tox.ini
+++ b/tox.ini
@@ -11,7 +11,9 @@ envlist = python,pylint,pylint-tests,flake,mypy,black,isort
 isolated_build = true
 
 [testenv]
-deps = poetry
+deps =
+    poetry
+    psycopg2
 skip_install = true
 passenv =
     TERM