Merge pull request #848 from doxxx/fk-delete-cascade

fix: cascade deletes for progress syncing tables

Author: crocodilestick

CONTRIBUTORS

@@ -314,7 +314,6 @@ Copyright (C) 2024-2026 Calibre-Web Automated contributors
 - zelazna (1 commits)
 - zhiyue (1 commits)
 # Fork Contributors (crocodilestick/calibre-web-automated)
-
 - crocodilestick (962 commits)
 - jmarmstrong1207 (73 commits)
 - demitrix (30 commits)

cps/progress_syncing/models.py

@@ -24,6 +24,66 @@
 log = logger.create()
 
 
+def check_cascade_delete_on_fk(conn, table_name, expected_cascades, db_name=None):
+    """
+    Check if foreign keys on a table have ON DELETE CASCADE.
+
+    Args:
+        conn: SQLAlchemy connection object or sqlite3 connection
+        table_name: Name of the table to check
+        expected_cascades: Dict mapping column name to expected foreign key table
+                          e.g., {'book': 'books', 'user_id': 'user'}
+
+    Returns:
+        bool: True if all foreign keys have CASCADE, False otherwise
+    """
+    try:
+        # Detect connection type
+        is_sqlalchemy = hasattr(conn, 'execute') and hasattr(conn.execute.__self__, 'dialect')
+
+        def execute_sql(sql):
+            if is_sqlalchemy:
+                return conn.execute(text(sql))
+            else:
+                return conn.execute(sql)
+
+        db_prefix = f"{db_name}." if db_name else ""
+
+        # Check if table exists
+        result = execute_sql(
+            f"SELECT name FROM {db_prefix}sqlite_master WHERE type='table' AND name='{table_name}'"
+        )
+        if not result.fetchone():
+            return True  # Table doesn't exist, no migration needed
+
+        # Get foreign key info
+        pragma_prefix = f"{db_name}." if db_name else ""
+        fk_info = execute_sql(f"PRAGMA {pragma_prefix}foreign_key_list({table_name})").fetchall()
+
+        if not fk_info:
+            return True  # No foreign keys defined
+
+        # Check each foreign key for CASCADE
+        for fk in fk_info:
+            # PRAGMA foreign_key_list returns: (id, seq, table, from, to, on_update, on_delete, match)
+            from_column = fk[3]  # Column in this table
+            to_table = fk[2]     # Referenced table
+            on_delete = fk[6]    # ON DELETE action
+
+            # Check if this is an expected foreign key
+            if from_column in expected_cascades:
+                expected_table = expected_cascades[from_column]
+                if to_table == expected_table and on_delete != 'CASCADE':
+                    log.debug(f"Table {table_name}.{from_column} FK to {to_table} missing CASCADE (has: {on_delete})")
+                    return False
+
+        return True
+
+    except Exception as e:
+        log.error(f"Error checking CASCADE on {table_name}: {e}")
+        return True  # Assume OK to avoid breaking startup
+
+
 def ensure_calibre_db_tables(conn):
     """
     Ensure progress syncing tables for metadata.db (Calibre library) exist.
@@ -67,6 +127,8 @@ def ensure_checksum_table(conn):
     This function is called during database initialization to create the checksum
     table if it doesn't exist. If the table exists with a different schema, a
     warning is logged and the table is left as-is to allow for proper migration.
+    If the schema matches but the foreign key is missing ON DELETE CASCADE,
+    the table is rebuilt in-place to add CASCADE and orphan rows are dropped.
 
     Args:
         conn: SQLAlchemy connection object or sqlite3 connection
@@ -119,6 +181,55 @@ def execute_sql(sql):
                 )
                 return  # Skip creation, table exists but needs migration
 
+            # Check if foreign keys have CASCADE DELETE
+            cascade_db_name = "calibre" if is_calibre_attached else None
+            has_cascade = check_cascade_delete_on_fk(
+                conn,
+                "book_format_checksums",
+                {'book': 'books'},
+                db_name=cascade_db_name,
+            )
+            if not has_cascade:
+                log.warning(
+                    "book_format_checksums table missing ON DELETE CASCADE. "
+                    "Attempting in-place migration to add CASCADE..."
+                )
+                try:
+                    temp_table = f"{table_prefix}book_format_checksums_new"
+                    books_table = f"{table_prefix}books"
+
+                    execute_sql(f"DROP TABLE IF EXISTS {temp_table}")
+                    execute_sql(f"""
+                        CREATE TABLE {temp_table} (
+                            id INTEGER PRIMARY KEY AUTOINCREMENT,
+                            book INTEGER NOT NULL,
+                            format TEXT NOT NULL COLLATE NOCASE,
+                            checksum TEXT NOT NULL,
+                            version TEXT NOT NULL DEFAULT 'koreader',
+                            created TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+                            FOREIGN KEY (book) REFERENCES books(id) ON DELETE CASCADE
+                        )
+                    """)
+                    execute_sql(f"""
+                        INSERT INTO {temp_table} (id, book, format, checksum, version, created)
+                        SELECT bfc.id, bfc.book, bfc.format, bfc.checksum, bfc.version, bfc.created
+                        FROM {table_name} AS bfc
+                        WHERE EXISTS (
+                            SELECT 1 FROM {books_table} AS b WHERE b.id = bfc.book
+                        )
+                    """)
+                    execute_sql(f"DROP TABLE {table_name}")
+                    execute_sql(f"ALTER TABLE {temp_table} RENAME TO book_format_checksums")
+                    execute_sql(f"CREATE INDEX {table_prefix}idx_checksum ON book_format_checksums(checksum)")
+                    execute_sql(f"CREATE INDEX {table_prefix}idx_checksum_version ON book_format_checksums(checksum, version)")
+                    execute_sql(f"CREATE INDEX {table_prefix}idx_book_format ON book_format_checksums(book, format)")
+                    execute_sql(f"CREATE INDEX {table_prefix}idx_created ON book_format_checksums(created)")
+                    conn.commit()
+                    log.info("Migrated book_format_checksums to add ON DELETE CASCADE")
+                except Exception as migration_error:
+                    log.error(f"Failed to migrate book_format_checksums for CASCADE: {migration_error}")
+                    table_exists = True
+
         if not table_exists:
             # Create table for book format checksums
             execute_sql(f"""
@@ -129,7 +240,7 @@ def execute_sql(sql):
                     checksum TEXT NOT NULL,
                     version TEXT NOT NULL DEFAULT 'koreader',
                     created TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
-                    FOREIGN KEY (book) REFERENCES books(id)
+                    FOREIGN KEY (book) REFERENCES books(id) ON DELETE CASCADE
                 )
             """)
             execute_sql(f"CREATE INDEX {table_prefix}idx_checksum ON book_format_checksums(checksum)")
@@ -152,6 +263,8 @@ def ensure_kosync_progress_table(conn):
     This function is called during database initialization to create the KOSync
     progress table if it doesn't exist. If the table exists with a different schema,
     a warning is logged and the table is left as-is to allow for proper migration.
+    If the schema matches but the foreign key is missing ON DELETE CASCADE,
+    the table is rebuilt in-place to add CASCADE and orphan rows are dropped.
 
     Args:
         conn: SQLAlchemy connection object or sqlite3 connection
@@ -191,6 +304,46 @@ def execute_sql(sql):
                 )
                 return  # Skip creation, table exists but needs migration
 
+            # Check if foreign keys have CASCADE DELETE
+            has_cascade = check_cascade_delete_on_fk(conn, "kosync_progress", {'user_id': 'user'})
+            if not has_cascade:
+                log.warning(
+                    "kosync_progress table missing ON DELETE CASCADE. "
+                    "Attempting in-place migration to add CASCADE..."
+                )
+                try:
+                    execute_sql("DROP TABLE IF EXISTS kosync_progress_new")
+                    execute_sql("""
+                        CREATE TABLE kosync_progress_new (
+                            id INTEGER PRIMARY KEY AUTOINCREMENT,
+                            user_id INTEGER NOT NULL,
+                            document TEXT NOT NULL,
+                            progress TEXT NOT NULL,
+                            percentage REAL NOT NULL,
+                            device TEXT NOT NULL,
+                            device_id TEXT,
+                            timestamp TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+                            FOREIGN KEY (user_id) REFERENCES user(id) ON DELETE CASCADE
+                        )
+                    """)
+                    execute_sql("""
+                        INSERT INTO kosync_progress_new (id, user_id, document, progress, percentage, device, device_id, timestamp)
+                        SELECT kp.id, kp.user_id, kp.document, kp.progress, kp.percentage, kp.device, kp.device_id, kp.timestamp
+                        FROM kosync_progress AS kp
+                        WHERE EXISTS (
+                            SELECT 1 FROM user AS u WHERE u.id = kp.user_id
+                        )
+                    """)
+                    execute_sql("DROP TABLE kosync_progress")
+                    execute_sql("ALTER TABLE kosync_progress_new RENAME TO kosync_progress")
+                    execute_sql("CREATE INDEX idx_kosync_user_document ON kosync_progress(user_id, document)")
+                    execute_sql("CREATE INDEX idx_kosync_document ON kosync_progress(document)")
+                    conn.commit()
+                    log.info("Migrated kosync_progress to add ON DELETE CASCADE")
+                except Exception as migration_error:
+                    log.error(f"Failed to migrate kosync_progress for CASCADE: {migration_error}")
+                    table_exists = True
+
         if not table_exists:
             # Create table for KOSync reading progress
             execute_sql("""
@@ -203,7 +356,7 @@ def execute_sql(sql):
                     device TEXT NOT NULL,
                     device_id TEXT,
                     timestamp TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
-                    FOREIGN KEY (user_id) REFERENCES user(id)
+                    FOREIGN KEY (user_id) REFERENCES user(id) ON DELETE CASCADE
                 )
             """)
             execute_sql("CREATE INDEX idx_kosync_user_document ON kosync_progress(user_id, document)")
@@ -241,7 +394,7 @@ class BookFormatChecksum(CalibreBase):
     __tablename__ = 'book_format_checksums'
 
     id = Column(Integer, primary_key=True, autoincrement=True)
-    book = Column(Integer, ForeignKey('books.id'), nullable=False)
+    book = Column(Integer, ForeignKey('books.id', ondelete='CASCADE'), nullable=False)
     format = Column(String(collation='NOCASE'), nullable=False)
     checksum = Column(String(32), nullable=False)  # MD5 hex digest is always 32 chars
     version = Column(String, nullable=False, default='koreader')  # Algorithm version identifier
@@ -281,7 +434,7 @@ class KOSyncProgress(AppBase):
     __tablename__ = 'kosync_progress'
 
     id = Column(Integer, primary_key=True)
-    user_id = Column(Integer, ForeignKey('user.id'), nullable=False)
+    user_id = Column(Integer, ForeignKey('user.id', ondelete='CASCADE'), nullable=False)
     document = Column(String, nullable=False)
     progress = Column(String, nullable=False)
     percentage = Column(Float, nullable=False)

tests/unit/test_generate_checksums.py

@@ -62,7 +62,7 @@ def create_minimal_calibre_library(library_path: Path):
             format TEXT NOT NULL,
             uncompressed_size INTEGER DEFAULT 0,
             name TEXT NOT NULL,
-            FOREIGN KEY (book) REFERENCES books(id)
+            FOREIGN KEY (book) REFERENCES books(id) ON DELETE CASCADE
         )
     ''')
 
@@ -74,7 +74,7 @@ def create_minimal_calibre_library(library_path: Path):
             checksum TEXT NOT NULL,
             version TEXT NOT NULL DEFAULT 'koreader',
             created TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
-            FOREIGN KEY (book) REFERENCES books(id)
+            FOREIGN KEY (book) REFERENCES books(id) ON DELETE CASCADE
         )
     ''')