fix: close dangling session, migration corrections

src/tagstudio/core/library/alchemy/library.py

@@ -221,7 +221,6 @@ class Library:
         if self.engine:
             self.engine.dispose()
         self.library_dir = None
-        self.folder = None
         self.included_files = set()
 
         self.dupe_entries_count = -1
@@ -378,8 +377,7 @@ class Library:
 
         return self.open_sqlite_library(library_dir, in_memory)
 
-    @staticmethod
-    def __get_engine(library_dir: Path, in_memory: bool, sql_filename: str):
+    def _get_engine(self, library_dir: Path, in_memory: bool, sql_filename: str):
         connection_string = URL.create(
             drivername="sqlite",
             database=(
@@ -407,7 +405,7 @@ class Library:
     def create_sqlite_library(
         self, library_dir: Path, in_memory: bool, sql_filename: str = SQL_FILENAME
     ) -> LibraryStatus:
-        self.engine = self.__get_engine(library_dir, in_memory, sql_filename)
+        self.engine = self._get_engine(library_dir, in_memory, sql_filename)
 
         logger.info(
             "[Library] Opening SQLite Library",
@@ -505,21 +503,21 @@ class Library:
     ) -> LibraryStatus:
         logger.info("[Library] Opening SQLite Library", library_dir=library_dir)
 
-        self.engine = self.__get_engine(library_dir, in_memory, sql_filename)
+        self.engine = self._get_engine(library_dir, in_memory, sql_filename)
+        self.library_dir = library_dir
 
         try:
-            migrations = DBMigrations(library_dir, self.engine)
+            migrations = DBMigrations(self)
 
             # save backup if patches will be applied
             if migrations.required:
-                Library.save_library_backup_to_disk(library_dir)
+                self.save_library_backup_to_disk()
 
             migrations.run()
         except MigrationError as e:
+            self.library_dir = None
             return LibraryStatus(success=False, message=e.args[0])
 
-        # everything is fine, set the library path
-        self.library_dir = library_dir
         return LibraryStatus(success=True, library_path=library_dir)
 
     @property
@@ -669,37 +667,32 @@ class Library:
         with Session(self.engine) as session:
             return unwrap(session.scalar(select(func.count(Entry.id))))
 
-    @staticmethod
-    def _all_entries(session: Session, with_joins: bool = False) -> Iterator[Entry]:
-        """Load entries without joins."""
-        stmt = select(Entry)
-        if with_joins:
-            # load Entry with all joins and all tags
-            stmt = (
-                stmt.outerjoin(Entry.text_fields)
-                .outerjoin(Entry.datetime_fields)
-                .outerjoin(Entry.tags)
-            )
-            stmt = stmt.options(
-                contains_eager(Entry.text_fields),
-                contains_eager(Entry.datetime_fields),
-                contains_eager(Entry.tags),
-            )
-
-        stmt = stmt.distinct()
-
-        entries = session.execute(stmt).scalars()
-        if with_joins:
-            entries = entries.unique()
-
-        for entry in entries:
-            yield entry
-            session.expunge(entry)
-
     def all_entries(self, with_joins: bool = False) -> Iterator[Entry]:
         """Load entries without joins."""
         with Session(self.engine) as session:
-            return Library._all_entries(session, with_joins)
+            stmt = select(Entry)
+            if with_joins:
+                # load Entry with all joins and all tags
+                stmt = (
+                    stmt.outerjoin(Entry.text_fields)
+                    .outerjoin(Entry.datetime_fields)
+                    .outerjoin(Entry.tags)
+                )
+                stmt = stmt.options(
+                    contains_eager(Entry.text_fields),
+                    contains_eager(Entry.datetime_fields),
+                    contains_eager(Entry.tags),
+                )
+
+            stmt = stmt.distinct()
+
+            entries = session.execute(stmt).scalars()
+            if with_joins:
+                entries = entries.unique()
+
+            for entry in entries:
+                yield entry
+                session.expunge(entry)
 
     @property
     def tags(self) -> list[Tag]:
@@ -1447,17 +1440,16 @@ class Library:
                 session.rollback()
                 return None
 
-    @staticmethod
-    def save_library_backup_to_disk(library_dir: Path) -> Path:
-        assert isinstance(library_dir, Path)
-        makedirs(str(library_dir / TS_FOLDER_NAME / BACKUP_FOLDER_NAME), exist_ok=True)
+    def save_library_backup_to_disk(self) -> Path:
+        assert isinstance(self.library_dir, Path)
+        makedirs(str(self.library_dir / TS_FOLDER_NAME / BACKUP_FOLDER_NAME), exist_ok=True)
 
         filename = f"ts_library_backup_{datetime.now(UTC).strftime('%Y_%m_%d_%H%M%S')}.sqlite"
 
-        target_path = library_dir / TS_FOLDER_NAME / BACKUP_FOLDER_NAME / filename
+        target_path = self.library_dir / TS_FOLDER_NAME / BACKUP_FOLDER_NAME / filename
 
         shutil.copy2(
-            library_dir / TS_FOLDER_NAME / SQL_FILENAME,
+            self.library_dir / TS_FOLDER_NAME / SQL_FILENAME,
             target_path,
         )
 
@@ -1749,12 +1741,8 @@ class Library:
         Args:
             key(str): The key for the name of the version type to set.
         """
-        return Library._get_version(self.engine, key)
-
-    @staticmethod
-    def _get_version(engine, key: str) -> int:
-        with Session(engine) as session:
-            engine = sqlalchemy.inspect(engine)
+        with Session(self.engine) as session:
+            engine = sqlalchemy.inspect(self.engine)
             try:
                 # "Version" table added in DB_VERSION 101
                 if engine and engine.has_table("versions"):

src/tagstudio/core/library/alchemy/migrations.py

@@ -4,11 +4,11 @@
 
 from collections.abc import Callable
 from pathlib import Path
-from typing import override
+from typing import TYPE_CHECKING, override
 
 import structlog
 import ujson
-from sqlalchemy import Engine, and_, delete, select, text, update
+from sqlalchemy import and_, delete, select, text, update
 from sqlalchemy.orm import Session
 
 from tagstudio.core.constants import IGNORE_NAME, TAG_ARCHIVED, TS_FOLDER_NAME
@@ -21,11 +21,14 @@ from tagstudio.core.library.alchemy.constants import (
 )
 from tagstudio.core.library.alchemy.fields import LEGACY_FIELD_MAP, DatetimeField, TextField
 from tagstudio.core.library.alchemy.joins import TagParent
-from tagstudio.core.library.alchemy.models import Tag, TagColorGroup, Version
+from tagstudio.core.library.alchemy.models import Entry, Tag, TagColorGroup, Version
 from tagstudio.core.library.ignore import migrate_ext_list
 from tagstudio.core.utils.types import unwrap
 from tagstudio.qt.translations import Translations
 
+if TYPE_CHECKING:
+    from tagstudio.core.library.alchemy.library import Library
+
 logger = structlog.get_logger(__name__)
 
 
@@ -38,20 +41,19 @@ class DBMigration:
     initial_version: int | None = None
 
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log: Callable[[str], str]):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]) -> None:  # pyright: ignore[reportUnusedParameter]
         raise NotImplementedError
 
 
 class DBMigrations:
-    def __init__(self, library_dir: Path, engine: Engine) -> None:
-        from tagstudio.core.library.alchemy.library import Library
+    def __init__(self, library: "Library") -> None:
 
-        self.library_dir = library_dir
-        self.engine = engine
+        self.lib = library
+        self.engine = self.lib.engine
 
         # Don't check DB version when creating new library
-        self.loaded_db_version = Library._get_version(engine, DB_VERSION_CURRENT_KEY)
-        self.initial_db_version = Library._get_version(engine, DB_VERSION_INITIAL_KEY)
+        self.loaded_db_version = self.lib.get_version(DB_VERSION_CURRENT_KEY)
+        self.initial_db_version = self.lib.get_version(DB_VERSION_INITIAL_KEY)
 
         # ======================== Library Database Version Checking =======================
         # DB_VERSION 6 is the first supported SQLite DB version.
@@ -107,7 +109,7 @@ class DBMigrations:
                     # any error causes transaction to rollback
                     migration.run(
                         session,
-                        self.library_dir,
+                        self.lib,
                         lambda msg, v=migration.version: f"[Library][Migration][{v}] {msg}",
                     )
                     self.loaded_db_version = migration.version
@@ -146,7 +148,7 @@ class MigrationTo7(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB from DB_VERSION 6 to 7."""
         logger.info(fmt_log("Applying patches to DB_VERSION: 6 library..."))
         # Repair tags that may have a disambiguation_id pointing towards a deleted tag.
@@ -166,7 +168,7 @@ class MigrationTo8(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB from DB_VERSION 7 to 8."""
         # Add the missing color_border column to the TagColorGroups table.
         session.execute(
@@ -219,7 +221,7 @@ class MigrationTo9(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB from DB_VERSION 8 to 9."""
         # Apply database schema changes
         add_filename_column = text(
@@ -230,9 +232,8 @@ class MigrationTo9(DBMigration):
         logger.info(fmt_log("Added filename column to entries table"))
 
         # Populate the new filename column.
-        from tagstudio.core.library.alchemy.library import Library
-
-        for entry in Library._all_entries(session):
+        entries = session.execute(select(Entry).distinct()).scalars()
+        for entry in entries:
             entry.filename = entry.path.name
             session.merge(entry)
         session.flush()
@@ -244,7 +245,7 @@ class MigrationTo100(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 100."""
         # Repair parent-child tag relationships that are the wrong way around.
         stmt = update(TagParent).values(
@@ -261,7 +262,7 @@ class MigrationTo101(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 101."""
         # Create versions table
         session.execute(
@@ -284,7 +285,7 @@ class MigrationTo102(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 102."""
         # delete TagParents with a dangling parent reference
         stmt = delete(TagParent).where(TagParent.parent_id.not_in(select(Tag.id).distinct()))
@@ -298,7 +299,7 @@ class MigrationTo103(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB from DB_VERSION 102 to 103."""
         # add the new hidden column for tags
         session.execute(text("ALTER TABLE tags ADD COLUMN is_hidden BOOLEAN NOT NULL DEFAULT 0"))
@@ -316,17 +317,17 @@ class MigrationTo104(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB from DB_VERSION 103 to 104."""
         # Convert file extension list to ts_ignore file, if a .ts_ignore file does not exist
-        cls.__migrate_sql_to_ts_ignore(session, library_dir)
+        cls.__migrate_sql_to_ts_ignore(session, library)
         session.execute(text("DROP TABLE preferences"))
         session.flush()
 
     @classmethod
-    def __migrate_sql_to_ts_ignore(cls, session: Session, library_dir: Path):
+    def __migrate_sql_to_ts_ignore(cls, session: Session, library: "Library"):
         # Do not continue if existing '.ts_ignore' file is found
-        ts_ignore = library_dir / TS_FOLDER_NAME / IGNORE_NAME
+        ts_ignore = unwrap(library.library_dir) / TS_FOLDER_NAME / IGNORE_NAME
         if Path(ts_ignore).exists():
             return
 
@@ -349,7 +350,7 @@ class MigrationTo200(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 200."""
         # Drop unused 'boolean_fields' and 'value_type' tables
         logger.info(fmt_log("Dropping boolean_fields and value_type tables..."))
@@ -463,7 +464,7 @@ class MigrationTo201(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 201."""
         create_text_fields_table = text("""
         CREATE TABLE text_fields_new (
@@ -519,7 +520,7 @@ class MigrationTo202(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         """Migrate DB to DB_VERSION 202."""
         stmt = delete(TagParent).where(TagParent.child_id.not_in(select(Tag.id).distinct()))
         session.execute(stmt)
@@ -532,7 +533,7 @@ class MigrationTo300(DBMigration):
 
     @override
     @classmethod
-    def run(cls, session: Session, library_dir: Path, fmt_log):
+    def run(cls, session: Session, library: "Library", fmt_log: Callable[[str], str]):
         ## remove folder_id column from entries table
         # create new table in the desired scheme (without folder_id column)
         session.execute(

src/tagstudio/qt/ts_qt.py

@@ -849,7 +849,7 @@ class QtDriver(DriverMixin, QObject):
         logger.info("Backing Up Library...")
         self.main_window.status_bar.showMessage(Translations["status.library_backup_in_progress"])
         start_time = time.time()
-        target_path = Library.save_library_backup_to_disk(unwrap(self.lib.library_dir))
+        target_path = self.lib.save_library_backup_to_disk()
         end_time = time.time()
         self.main_window.status_bar.showMessage(
             Translations.format(
@@ -1651,7 +1651,8 @@ class QtDriver(DriverMixin, QObject):
         else:
             self._init_library(path, open_status)
 
-    def _init_library(self, path: Path, open_status: LibraryStatus):
+    def _init_library(self, path: Path, open_status: LibraryStatus, is_test: bool = False):
+        # TODO: Don't have an is_test parameter, the frontend and backend tasks here can be split.
         if not open_status.success:
             self.show_error_message(
                 error_name=open_status.message
@@ -1661,7 +1662,8 @@ class QtDriver(DriverMixin, QObject):
             return open_status
 
         assert self.lib.library_dir
-        self.init_workers()
+        if not is_test:
+            self.init_workers()
         Ignore.get_patterns(self.lib.library_dir, include_global=True)
         self.__reset_navigation()
 

tests/qt/test_file_path_options.py

@@ -144,7 +144,7 @@ def test_title_update(
     qt_driver.main_window.menu_bar.folders_to_tags_action = QAction(menu_bar)
 
     # Trigger the update
-    qt_driver._init_library(library_dir, open_status)
+    qt_driver._init_library(library_dir, open_status, is_test=True)
 
     # Assert the title is updated correctly
     qt_driver.main_window.setWindowTitle.assert_called_with(expected_title(library_dir, base_title))