diff --git a/backend/endpoints/roms/__init__.py b/backend/endpoints/roms/__init__.py index bceb04d62..6d8a536ba 100644 --- a/backend/endpoints/roms/__init__.py +++ b/backend/endpoints/roms/__init__.py @@ -1483,13 +1483,18 @@ async def delete_roms( await fs_rom_handler.remove_file(file_path=rom_path) # Clean up empty parent directory if it becomes empty parent = full_path.parent - if ( - parent != fs_rom_handler.base_path - and parent.is_dir() - and not any(parent.iterdir()) - ): - await fs_rom_handler.remove_directory( - str(parent.relative_to(fs_rom_handler.base_path)) + try: + if ( + parent != fs_rom_handler.base_path + and parent.is_dir() + and not any(parent.iterdir()) + ): + await fs_rom_handler.remove_directory( + str(parent.relative_to(fs_rom_handler.base_path)) + ) + except OSError as dir_err: + log.warning( + f"Couldn't clean up empty parent directory for {hl(rom.fs_name)}: {dir_err}" ) except FileNotFoundError: error = f"Rom file {hl(rom.fs_name)} not found for platform {hl(rom.platform_display_name, color=BLUE)}[{hl(rom.platform_slug)}]" diff --git a/backend/tests/endpoints/roms/test_rom.py b/backend/tests/endpoints/roms/test_rom.py index 15664f70c..50c12a80e 100644 --- a/backend/tests/endpoints/roms/test_rom.py +++ b/backend/tests/endpoints/roms/test_rom.py @@ -124,6 +124,10 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom): assert body["successful_items"] == 1 +@patch( + "endpoints.roms.fs_rom_handler.remove_directory", + new_callable=AsyncMock, +) @patch( "endpoints.roms.fs_rom_handler.remove_file", new_callable=AsyncMock, @@ -134,6 +138,7 @@ def test_delete_roms(client: TestClient, access_token: str, rom: Rom): def test_delete_roms_from_fs_flat( mock_validate_path, mock_remove_file, + mock_remove_directory, client: TestClient, access_token: str, rom: Rom, @@ -158,6 +163,53 @@ def test_delete_roms_from_fs_flat( assert body["successful_items"] == 1 assert body["failed_items"] == 0 mock_remove_file.assert_called_once() + mock_remove_directory.assert_not_called() + + +@patch( + "endpoints.roms.fs_rom_handler.remove_directory", + new_callable=AsyncMock, +) +@patch( + "endpoints.roms.fs_rom_handler.remove_file", + new_callable=AsyncMock, +) +@patch( + "endpoints.roms.fs_rom_handler.validate_path", +) +def test_delete_roms_from_fs_flat_cleans_empty_parent( + mock_validate_path, + mock_remove_file, + mock_remove_directory, + client: TestClient, + access_token: str, + rom: Rom, +): + """Test that empty parent directories are cleaned up after flat ROM file deletion.""" + from pathlib import Path + from unittest.mock import MagicMock + + mock_path = MagicMock(spec=Path) + mock_path.is_dir.return_value = False + # Parent is not the base_path, is a dir, and is empty + mock_path.parent.__ne__ = lambda self, other: True + mock_path.parent.is_dir.return_value = True + mock_path.parent.__iter__ = lambda self: iter([]) # empty directory + mock_validate_path.return_value = mock_path + + response = client.post( + "/api/roms/delete", + headers={"Authorization": f"Bearer {access_token}"}, + json={"roms": [rom.id], "delete_from_fs": [rom.id]}, + ) + assert response.status_code == status.HTTP_200_OK + + body = response.json() + assert body["successful_items"] == 1 + assert body["failed_items"] == 0 + mock_remove_file.assert_called_once() + # remove_directory should be called to clean up the empty parent dir + mock_remove_directory.assert_called_once() @patch(