tests: [WIP] Test that foreign keys are enabled in sqlite

Author: natkam

grader_service/autograding/git_manager.py

@@ -159,7 +159,6 @@ def _run_git(self, command: list[str], cwd: Optional[str]) -> None:
             self.log.error(e)
             raise
 
-    # TODO: Can I decorate executable_validator with both "git_executable" and "convert_executable"?
     @validate("git_executable")
     def _validate_executable(self, proposal):
         return executable_validator(proposal)

grader_service/orm/base.py

@@ -28,6 +28,7 @@ def set_sqlite_pragma(dbapi_connection, connection_record):
         dbapi_connection.autocommit = True
 
         cursor = dbapi_connection.cursor()
+        # Note: this is a SQLite-specific pragma
         cursor.execute("PRAGMA foreign_keys=ON")
         cursor.close()
 

grader_service/tests/conftest.py

@@ -10,6 +10,8 @@
 import pytest
 from alembic import config
 from alembic.command import upgrade
+from sqlalchemy import event
+from sqlalchemy.engine import Engine
 from sqlalchemy.orm import Session, scoped_session, sessionmaker
 
 from grader_service import GraderService, handlers
@@ -18,7 +20,51 @@
 from grader_service.orm import User
 from grader_service.registry import HandlerPathRegistry
 from grader_service.server import GraderServer
-from grader_service.tests.handlers.db_util import insert_assignments, insert_lectures
+from grader_service.tests.handlers.db_util import (
+    insert_assignments,
+    insert_default_user,
+    insert_lectures,
+)
+
+
+@pytest.fixture(scope="function")
+def set_database_type_to_sqlite():
+    """
+    Set the DATABASE_TYPE env var to "sqlite" and register a listener
+    which enables foreign keys support for SQLite database.
+    Unset the var after the test runs.
+
+    This is a hack, because normally `DATABASE_TYPE` is not set when tests run.
+    It also cannot be set to "sqlite" by default, because we have some tests
+    running on PostgreSQL, where executing the sqlite pragma would cause an error.
+
+    Outside the test setting, the DATABASE_TYPE environment variable is set, and
+    the event listener is registered in grader_service/orm/base.py.
+    """
+    os.environ["DATABASE_TYPE"] = "sqlite"
+
+    # Original code taken from the SQLAlchemy documentation, here slightly adjusted:
+    # 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):
+        database_type = os.getenv("DATABASE_TYPE")
+        if database_type == "sqlite":
+            # the sqlite3 driver will not set PRAGMA foreign_keys
+            # if autocommit=False; set to True temporarily
+            ac = dbapi_connection.autocommit
+            dbapi_connection.autocommit = True
+
+            cursor = dbapi_connection.cursor()
+            # Note: this is a SQLite-specific pragma
+            cursor.execute("PRAGMA foreign_keys=ON")
+            cursor.close()
+
+            # restore previous autocommit setting
+            dbapi_connection.autocommit = ac
+
+    yield
+    # Unset the variable for other tests, which may use a different database type
+    os.environ.pop("DATABASE_TYPE")
 
 
 @pytest.fixture(scope="function")
@@ -68,6 +114,7 @@ def sql_alchemy_sessionmaker(db_test_config):
         upgrade(db_test_config, "head")
     insert_lectures(engine)
     insert_assignments(engine)
+    insert_default_user(engine)
     yield session_maker
 
 

grader_service/tests/handlers/db_util.py

@@ -148,6 +148,12 @@ def insert_student(ex: Engine, username: str, lecture_id: int) -> User:
     return user
 
 
+def insert_default_user(ex: Engine) -> None:
+    session: Session = sessionmaker(ex)()
+    session.add(User(name="ubuntu", display_name="ubuntu"))
+    session.commit()
+
+
 def create_user_submission_with_repo(
     engine: Engine, gitbase_dir: Path, student: User, assignment_id: int, lecture_code: str
 ) -> Submission:

grader_service/tests/handlers/test_assignment_handler.py

@@ -7,15 +7,46 @@
 from http import HTTPStatus
 
 import pytest
+from sqlalchemy.exc import IntegrityError
 from tornado.httpclient import HTTPClientError
 
 from grader_service.api.models.assignment import Assignment
 from grader_service.api.models.assignment_settings import AssignmentSettings
+from grader_service.orm import Assignment as AssignmentORM
+from grader_service.orm import Submission
 from grader_service.server import GraderServer
 
 from .db_util import insert_assignments, insert_submission
 
 
+def test_foreign_key_constraints_in_sqlite(
+    set_database_type_to_sqlite, sql_alchemy_engine, sql_alchemy_sessionmaker, default_user
+):
+    """Make sure the FK constraints are enabled in SQLite on engine connection."""
+
+    # Note: tests use an sqlite db by default (see the `alembic_test.ini` file),
+    # but `DATABASE_TYPE` environment variable is not set. We use the fixture
+    # `set_database_type_to_sqlite` to set it to "sqlite" for this test.
+
+    a_id = 1  # This assignment exists in the test database
+    session = sql_alchemy_sessionmaker()
+    engine = session.get_bind()
+    sub = insert_submission(engine, a_id, "ubuntu", 1)
+
+    session = sql_alchemy_sessionmaker()
+
+    with pytest.raises(IntegrityError, match="FOREIGN KEY constraint failed"):
+        # Try to delete an existing assignment with a submission
+        session.query(AssignmentORM).filter(AssignmentORM.id == a_id).delete()
+        session.commit()
+
+    assign = session.query(AssignmentORM).filter(AssignmentORM.id == a_id).one_or_none()
+    sub_2 = session.query(Submission).filter(Submission.id == sub.id).one_or_none()
+    assert assign is not None
+    assert sub_2.id == assign.submissions[0].id
+    session.close()
+
+
 async def test_get_assignments(
     app: GraderServer,
     service_base_url,