diff options
| -rw-r--r-- | .gitlab-ci.yml | 13 | ||||
| -rwxr-xr-x | ci/run_tests.sh | 48 | ||||
| -rw-r--r-- | doc/administration/installation.rst | 28 | ||||
| -rw-r--r-- | fietsboek/__init__.py | 19 | ||||
| -rw-r--r-- | fietsboek/alembic/versions/20220808_d085998b49ca.py | 11 | ||||
| -rw-r--r-- | fietsboek/alembic/versions/20230203_3149aa2d0114.py | 2 | ||||
| -rw-r--r-- | fietsboek/alembic/versions/20250607_2ebe1bf66430.py | 46 | ||||
| -rw-r--r-- | fietsboek/models/track.py | 23 | ||||
| -rw-r--r-- | testing.ini | 2 | ||||
| -rw-r--r-- | tests/bootstrap/test_new_instance.py | 90 | ||||
| -rw-r--r-- | tests/conftest.py | 1 | ||||
| -rw-r--r-- | tests/integration/test_browse.py | 16 | ||||
| -rw-r--r-- | tests/playwright/conftest.py | 6 | ||||
| -rw-r--r-- | tox.ini | 4 |
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: @@ -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 |