diff --git a/backend/app/database/folders.py b/backend/app/database/folders.py index 8bf501dd3..06075211e 100644 --- a/backend/app/database/folders.py +++ b/backend/app/database/folders.py @@ -34,8 +34,7 @@ def db_create_folders_table() -> None: try: conn = sqlite3.connect(DATABASE_PATH) cursor = conn.cursor() - cursor.execute( - """ + cursor.execute(""" CREATE TABLE IF NOT EXISTS folders ( folder_id TEXT PRIMARY KEY, parent_folder_id TEXT, @@ -46,8 +45,7 @@ def db_create_folders_table() -> None: indexing_status TEXT DEFAULT 'not_started', FOREIGN KEY (parent_folder_id) REFERENCES folders(folder_id) ON DELETE CASCADE ) - """ - ) + """) conn.commit() finally: if conn is not None: @@ -366,13 +364,16 @@ def db_get_folder_ids_by_path_prefix(root_path: str) -> List[FolderIdPath]: cursor = conn.cursor() try: - # Use path LIKE with wildcard to match all subfolders + escaped_path = ( + root_path.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") + ) + cursor.execute( """ SELECT folder_id, folder_path FROM folders - WHERE folder_path LIKE ? || '%' + WHERE folder_path LIKE ? || '%' ESCAPE '\\' """, - (root_path,), + (escaped_path,), ) return cursor.fetchall() @@ -435,8 +436,7 @@ def db_get_all_folder_details() -> ( try: # COUNT(DISTINCT ...) because joining both media tables multiplies the # rows: a folder with 3 images and 4 videos yields 12 join rows. - cursor.execute( - """ + cursor.execute(""" SELECT f.folder_id, f.folder_path, @@ -452,8 +452,7 @@ def db_get_all_folder_details() -> ( LEFT JOIN videos v ON f.folder_id = v.folder_id GROUP BY f.folder_id ORDER BY f.folder_path - """ - ) + """) return cursor.fetchall() finally: conn.close() diff --git a/backend/app/routes/folders.py b/backend/app/routes/folders.py index c27e789a2..67501955d 100644 --- a/backend/app/routes/folders.py +++ b/backend/app/routes/folders.py @@ -44,6 +44,7 @@ folder_util_add_multiple_folder_trees, folder_util_delete_obsolete_folders, folder_util_get_filesystem_direct_child_folders, + folder_util_cleanup_thumbnails, ) from concurrent.futures import ProcessPoolExecutor from app.utils.images import ( @@ -397,6 +398,7 @@ def delete_folders(request: DeleteFoldersRequest): if not request.folder_ids: raise ValueError("No folder IDs provided") + folder_util_cleanup_thumbnails(request.folder_ids) deleted_count = db_delete_folders_batch(request.folder_ids) return DeleteFoldersResponse( diff --git a/backend/app/utils/folders.py b/backend/app/utils/folders.py index ec014f479..cbfaa3160 100644 --- a/backend/app/utils/folders.py +++ b/backend/app/utils/folders.py @@ -7,13 +7,70 @@ db_insert_folders_batch, db_update_parent_ids_for_subtree, db_delete_folders_batch, + db_get_folder_path_from_id, + db_get_folder_ids_by_path_prefix, ) +from app.database.images import db_get_images_by_folder_ids +from app.database.videos import db_get_videos_by_folder_ids from app.schemas.folders import ErrorResponse from app.logging.setup_logging import get_logger logger = get_logger(__name__) +def folder_util_cleanup_thumbnails(folder_ids: List[str]) -> int: + """ + Delete thumbnail files from disk for the given folders and all their subfolders. + Should be called before deleting folders from the database to prevent orphaned files. + """ + if not folder_ids: + return 0 + + all_folder_ids = set() + for folder_id in folder_ids: + all_folder_ids.add(folder_id) + path = db_get_folder_path_from_id(folder_id) + if path: + # Find all subfolders by path prefix + subfolders = db_get_folder_ids_by_path_prefix(path + os.sep) + for sub_id, _ in subfolders: + all_folder_ids.add(sub_id) + + all_folder_ids_list = list(all_folder_ids) + deleted_count = 0 + + # 1. Fetch images and delete thumbnails + images = db_get_images_by_folder_ids(all_folder_ids_list) + for _, _, thumbnail_path in images: + if thumbnail_path: + try: + os.remove(thumbnail_path) + deleted_count += 1 + except FileNotFoundError: + pass + except OSError as e: + logger.error(f"Error removing image thumbnail {thumbnail_path}: {e}") + raise + + # 2. Fetch videos and delete thumbnails + videos = db_get_videos_by_folder_ids(all_folder_ids_list) + for _, _, thumbnail_path in videos: + if thumbnail_path: + try: + os.remove(thumbnail_path) + deleted_count += 1 + except FileNotFoundError: + pass + except OSError as e: + logger.error(f"Error removing video thumbnail {thumbnail_path}: {e}") + raise + + if deleted_count > 0: + logger.info(f"Cleaned up {deleted_count} thumbnail file(s) for folder deletion") + + return deleted_count + + def folder_util_add_folder_tree( root_path, parent_folder_id=None, AI_Tagging=False, taggingCompleted=None ): @@ -127,6 +184,7 @@ def folder_util_delete_obsolete_folders( ] if folder_ids_to_delete: + folder_util_cleanup_thumbnails(folder_ids_to_delete) deleted_count = db_delete_folders_batch(folder_ids_to_delete) return deleted_count, list(folders_to_delete) diff --git a/backend/tests/test_folders.py b/backend/tests/test_folders.py index 18585de83..126e02df6 100644 --- a/backend/tests/test_folders.py +++ b/backend/tests/test_folders.py @@ -1058,15 +1058,50 @@ def test_db_get_folder_ids_by_path_prefix(self, test_db): ("folder-id-1", "/tmp/photos"), ("folder-id-2", "/tmp/photos/2024"), ("folder-id-3", "/other/documents"), + ("folder-id-4", "/tmp/photos%2024"), + ("folder-id-5", "/tmp/photos_2024"), + ("folder-id-6", "/tmp/photos\\2024"), + ("folder-id-7", "/tmp/photos/100%_sure"), + ("folder-id-8", "/tmp/photos/100%_sure/yes"), ], ) conn.commit() conn.close() + result = db_get_folder_ids_by_path_prefix("/tmp") # The query has no ORDER BY, so row order isn't part of the contract assert set(result) == { ("folder-id-1", "/tmp/photos"), ("folder-id-2", "/tmp/photos/2024"), + ("folder-id-4", "/tmp/photos%2024"), + ("folder-id-5", "/tmp/photos_2024"), + ("folder-id-6", "/tmp/photos\\2024"), + ("folder-id-7", "/tmp/photos/100%_sure"), + ("folder-id-8", "/tmp/photos/100%_sure/yes"), + } + + # Verify literal percent doesn't match everything + result = db_get_folder_ids_by_path_prefix("/tmp/photos%") + assert set(result) == { + ("folder-id-4", "/tmp/photos%2024"), + } + + # Verify literal underscore doesn't match single characters + result = db_get_folder_ids_by_path_prefix("/tmp/photos_") + assert set(result) == { + ("folder-id-5", "/tmp/photos_2024"), + } + + # Verify literal backslash + result = db_get_folder_ids_by_path_prefix("/tmp/photos\\") + assert set(result) == { + ("folder-id-6", "/tmp/photos\\2024"), + } + + # Verify combined percent and underscore, including appending os.sep + result = db_get_folder_ids_by_path_prefix("/tmp/photos/100%_sure" + os.sep) + assert set(result) == { + ("folder-id-8", "/tmp/photos/100%_sure/yes"), } def test_db_get_folder_ids_by_paths(self, test_db): @@ -1349,3 +1384,104 @@ def test_complete_folder_lifecycle( mock_enable_batch.assert_called_once_with(folder_ids) mock_delete_batch.assert_called_once_with(folder_ids) + + +class TestFolderUtils: + @patch("app.utils.folders.os.remove") + def test_folder_util_cleanup_thumbnails(self, mock_remove, test_db): + conn = sqlite3.connect(test_db) + conn.execute( + "CREATE TABLE IF NOT EXISTS folders (folder_id TEXT, folder_path TEXT)" + ) + conn.execute( + "CREATE TABLE IF NOT EXISTS images (id TEXT, path TEXT, thumbnailPath TEXT, folder_id TEXT)" + ) + conn.execute( + "CREATE TABLE IF NOT EXISTS videos (id TEXT, path TEXT, thumbnailPath TEXT, folder_id TEXT)" + ) + + conn.executemany( + "INSERT INTO folders (folder_id, folder_path) VALUES (?, ?)", + [ + ("f1", "/test/folder"), + ("f2", "/test/folder/sub"), + ("f3", "/test/folder_sibling"), + ], + ) + conn.executemany( + "INSERT INTO images (id, thumbnailPath, folder_id) VALUES (?, ?, ?)", + [ + ("img1", "/thumb/img1.jpg", "f1"), + ("img2", "/thumb/img2.jpg", "f2"), + ("img3", "/thumb/img3.jpg", "f3"), + ], + ) + conn.executemany( + "INSERT INTO videos (id, thumbnailPath, folder_id) VALUES (?, ?, ?)", + [ + ("vid1", "/thumb/vid1.jpg", "f1"), + ], + ) + conn.commit() + conn.close() + + from app.utils.folders import folder_util_cleanup_thumbnails + + mock_remove.return_value = None + count = folder_util_cleanup_thumbnails(["f1"]) + + assert mock_remove.call_count == 3 + mock_remove.assert_any_call("/thumb/img1.jpg") + mock_remove.assert_any_call("/thumb/img2.jpg") + mock_remove.assert_any_call("/thumb/vid1.jpg") + assert count == 3 + + @patch("app.utils.folders.os.remove") + def test_folder_util_cleanup_thumbnails_errors(self, mock_remove, test_db): + conn = sqlite3.connect(test_db) + conn.execute( + "CREATE TABLE IF NOT EXISTS folders (folder_id TEXT, folder_path TEXT)" + ) + conn.execute( + "CREATE TABLE IF NOT EXISTS images (id TEXT, path TEXT, thumbnailPath TEXT, folder_id TEXT)" + ) + conn.execute( + "INSERT INTO folders (folder_id, folder_path) VALUES ('f1', '/test')" + ) + conn.execute( + "INSERT INTO images (id, thumbnailPath, folder_id) VALUES ('img1', '/thumb/img1.jpg', 'f1')" + ) + conn.commit() + conn.close() + + from app.utils.folders import folder_util_cleanup_thumbnails + + # Test FileNotFoundError is ignored + mock_remove.side_effect = FileNotFoundError() + count = folder_util_cleanup_thumbnails(["f1"]) + assert count == 0 + + # Test other OSError is raised + mock_remove.side_effect = PermissionError() + with pytest.raises(PermissionError): + folder_util_cleanup_thumbnails(["f1"]) + + @patch("app.utils.folders.db_delete_folders_batch") + @patch("app.utils.folders.folder_util_cleanup_thumbnails") + def test_folder_util_delete_obsolete_folders(self, mock_cleanup, mock_delete_db): + from app.utils.folders import folder_util_delete_obsolete_folders + + db_child_folders = [("f1", "/test/obsolete"), ("f2", "/test/active")] + folders_to_delete = {"/test/obsolete"} + + mock_delete_db.return_value = 1 + + deleted_count, deleted_list = folder_util_delete_obsolete_folders( + db_child_folders, folders_to_delete + ) + + assert deleted_count == 1 + assert deleted_list == ["/test/obsolete"] + + mock_cleanup.assert_called_once_with(["f1"]) + mock_delete_db.assert_called_once_with(["f1"])