From 3ff2ec2ec56f665408384a797e019a239aaf4b41 Mon Sep 17 00:00:00 2001 From: Alexander Hungenberg Date: Thu, 23 Nov 2023 15:01:54 +0100 Subject: [PATCH] ensure that sqlite databases enforce foreign key constraints (#432) --- src/foxops/database/engine.py | 17 +++++++++++++++++ src/foxops/database/repositories/change.py | 13 +++++++++++++ src/foxops/dependencies.py | 5 +++-- tests/conftest.py | 15 ++++----------- tests/database/test_incarnation_repository.py | 4 +++- 5 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 src/foxops/database/engine.py diff --git a/src/foxops/database/engine.py b/src/foxops/database/engine.py new file mode 100644 index 00000000..16d301f3 --- /dev/null +++ b/src/foxops/database/engine.py @@ -0,0 +1,17 @@ +from sqlalchemy import Pool +from sqlalchemy.event import listen +from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine + + +def create_engine(connection_string: str) -> AsyncEngine: + # enforce foreign key constraints on SQLite: + # https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#foreign-key-support + def set_sqlite_pragma(dbapi_connection, connection_record): + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + if connection_string.startswith("sqlite+"): + listen(Pool, "connect", set_sqlite_pragma) + + return create_async_engine(connection_string, future=True, echo=False, pool_pre_ping=True) diff --git a/src/foxops/database/repositories/change.py b/src/foxops/database/repositories/change.py index cb59cba4..f5d8253b 100644 --- a/src/foxops/database/repositories/change.py +++ b/src/foxops/database/repositories/change.py @@ -9,6 +9,7 @@ from foxops.database.schema import change, incarnations from foxops.errors import FoxopsError, IncarnationNotFoundError +from foxops.logger import get_logger class ChangeConflictError(FoxopsError): @@ -90,6 +91,8 @@ class ChangeRepository: def __init__(self, engine: AsyncEngine) -> None: self.engine = engine + self.log = get_logger(component=self.__class__.__name__) + async def create_change( self, incarnation_id: int, @@ -154,6 +157,14 @@ async def create_incarnation_with_first_change( requested_data: str, template_data_full: str, ) -> ChangeInDB: + logger = self.log.bind(function="create_incarnation_with_first_change") + logger.debug( + "starting transaction", + incarnation_repository=incarnation_repository, + target_directory=target_directory, + template_repository=template_repository, + ) + async with self.engine.begin() as conn: query_insert_incarnation = ( insert(incarnations) @@ -167,6 +178,8 @@ async def create_incarnation_with_first_change( result = await conn.execute(query_insert_incarnation) incarnation_id = result.one()[0] + logger.debug("inserted incarnation", incarnation_id=incarnation_id) + query_insert_change = ( insert(change) .values( diff --git a/src/foxops/dependencies.py b/src/foxops/dependencies.py index 5a255809..498a4b6b 100644 --- a/src/foxops/dependencies.py +++ b/src/foxops/dependencies.py @@ -3,8 +3,9 @@ from fastapi import Depends, HTTPException, Request, status from fastapi.openapi.models import APIKey, APIKeyIn from fastapi.security.base import SecurityBase -from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine +from sqlalchemy.ext.asyncio import AsyncEngine +from foxops.database.engine import create_engine from foxops.database.repositories.change import ChangeRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository from foxops.hosters import Hoster @@ -41,7 +42,7 @@ def get_database_engine(request: Request, settings: DatabaseSettings = Depends(g if hasattr(request.app.state, "database"): return request.app.state.database - async_engine = create_async_engine(settings.url.get_secret_value(), future=True, echo=False, pool_pre_ping=True) + async_engine = create_engine(settings.url.get_secret_value()) request.app.state.database = async_engine return async_engine diff --git a/tests/conftest.py b/tests/conftest.py index 577f496b..c39eadbf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,10 +9,10 @@ import pytest from fastapi import FastAPI from httpx import AsyncClient -from sqlalchemy import Engine, event -from sqlalchemy.ext.asyncio import AsyncEngine, create_async_engine +from sqlalchemy.ext.asyncio import AsyncEngine from foxops.__main__ import create_app +from foxops.database.engine import create_engine from foxops.database.repositories.change import ChangeRepository from foxops.database.repositories.incarnation.repository import IncarnationRepository from foxops.database.schema import meta @@ -60,15 +60,8 @@ def use_testing_gitconfig(): @pytest.fixture(name="test_async_engine") async def test_async_engine() -> AsyncGenerator[AsyncEngine, None]: - # enforce foreign key constraints on SQLite: - # https://docs.sqlalchemy.org/en/20/dialects/sqlite.html#foreign-key-support - @event.listens_for(Engine, "connect") - def set_sqlite_pragma(dbapi_connection, connection_record): - cursor = dbapi_connection.cursor() - cursor.execute("PRAGMA foreign_keys=ON") - cursor.close() - - async_engine = create_async_engine("sqlite+aiosqlite://", future=True, echo=False, pool_pre_ping=True) + async_engine = create_engine("sqlite+aiosqlite://") + async with async_engine.begin() as conn: await conn.run_sync(meta.create_all) diff --git a/tests/database/test_incarnation_repository.py b/tests/database/test_incarnation_repository.py index 074d6243..b57dc14a 100644 --- a/tests/database/test_incarnation_repository.py +++ b/tests/database/test_incarnation_repository.py @@ -1,5 +1,6 @@ from pytest import fixture, raises +from foxops.database.repositories.change import ChangeRepository from foxops.database.repositories.incarnation.errors import ( IncarnationAlreadyExistsError, IncarnationNotFoundError, @@ -93,12 +94,13 @@ async def test_list_returns_existing_incarnations(incarnation_repository: Incarn async def test_delete_by_id_returns_none_when_deleting_existing_incarnation( - incarnation_repository: IncarnationRepository, incarnation_id: int + incarnation_repository: IncarnationRepository, change_repository: ChangeRepository, incarnation_id: int ): # WHEN await incarnation_repository.delete_by_id(incarnation_id) # THEN + assert len(await change_repository.list_changes(incarnation_id)) == 0 with raises(IncarnationNotFoundError): await incarnation_repository.get_by_id(incarnation_id)