diff --git a/backend/adapters/services/tests/test_steamgriddb.py b/backend/adapters/services/tests/test_steamgriddb.py index 0ccd3f339..3560c126a 100644 --- a/backend/adapters/services/tests/test_steamgriddb.py +++ b/backend/adapters/services/tests/test_steamgriddb.py @@ -16,7 +16,7 @@ from adapters.services.steamgriddb_types import ( SGDBTag, SGDBType, ) -from fastapi import HTTPException +from fastapi import HTTPException, status INVALID_GAME_ID = 999999 @@ -118,7 +118,7 @@ class TestSteamGridDBServiceUnit: with patch("adapters.services.steamgriddb.ctx_aiohttp_session", mock_context): with pytest.raises(HTTPException) as exc_info: await service._request("https://steamgriddb.com/api/v2/search/test") - assert exc_info.value.status_code == 401 + assert exc_info.value.status_code == status.HTTP_401_UNAUTHORIZED @pytest.mark.asyncio async def test_request_other_client_error(self, service): @@ -644,7 +644,7 @@ class TestSteamGridDBServiceIntegration: assert isinstance(result, dict) except HTTPException as exc: # Should be authentication error with 401 status - assert exc.status_code == 401 + assert exc.status_code == status.HTTP_401_UNAUTHORIZED @pytest.mark.asyncio @pytest.mark.vcr diff --git a/backend/alembic/versions/0014_asset_files.py b/backend/alembic/versions/0014_asset_files.py index cc433c261..a6755bf9e 100644 --- a/backend/alembic/versions/0014_asset_files.py +++ b/backend/alembic/versions/0014_asset_files.py @@ -40,7 +40,7 @@ def migrate_to_supported_engine() -> None: # Skip if sqlite database is not mounted if not os.path.exists(f"{SQLITE_DB_BASE_PATH}/romm.db"): - return + return None engine = create_engine(ConfigManager.get_db_engine(), pool_pre_ping=True) session = sessionmaker(bind=engine, expire_on_commit=False) diff --git a/backend/config/config_manager.py b/backend/config/config_manager.py index 6f747dd73..0d7f7c38f 100644 --- a/backend/config/config_manager.py +++ b/backend/config/config_manager.py @@ -264,7 +264,7 @@ class ConfigManager: platform_bindings = self.config.PLATFORMS_BINDING if fs_slug in platform_bindings: log.warning(f"Binding for {hl(fs_slug)} already exists") - return + return None platform_bindings[fs_slug] = slug self.config.PLATFORMS_BINDING = platform_bindings @@ -285,7 +285,7 @@ class ConfigManager: platform_versions = self.config.PLATFORMS_VERSIONS if fs_slug in platform_versions: log.warning(f"Version for {hl(fs_slug)} already exists") - return + return None platform_versions[fs_slug] = slug self.config.PLATFORMS_VERSIONS = platform_versions @@ -308,7 +308,7 @@ class ConfigManager: log.warning( f"{hl(exclusion_value)} already excluded in {hl(exclusion_type, color=BLUE)}" ) - return + return None config_item.append(exclusion_value) self.config.__setattr__(exclusion_type, config_item) diff --git a/backend/endpoints/auth.py b/backend/endpoints/auth.py index 01626c172..b79575c2e 100644 --- a/backend/endpoints/auth.py +++ b/backend/endpoints/auth.py @@ -4,7 +4,6 @@ from typing import Annotated, Final from config import OIDC_ENABLED, OIDC_REDIRECT_URI from decorators.auth import oauth from endpoints.forms.identity import OAuth2RequestForm -from endpoints.responses import MessageResponse from endpoints.responses.oauth import TokenResponse from exceptions.auth_exceptions import ( AuthCredentialsException, @@ -35,7 +34,7 @@ router = APIRouter( def login( request: Request, credentials=Depends(HTTPBasic()), # noqa -) -> MessageResponse: +) -> None: """Session login endpoint Args: @@ -45,9 +44,6 @@ def login( Raises: CredentialsException: Invalid credentials UserDisabledException: Auth is disabled - - Returns: - MessageResponse: Standard message response """ user = auth_handler.authenticate_user(credentials.username, credentials.password) @@ -63,24 +59,17 @@ def login( now = datetime.now(timezone.utc) db_user_handler.update_user(user.id, {"last_login": now, "last_active": now}) - return {"msg": "Successfully logged in"} - -@router.post("/logout") -def logout(request: Request) -> MessageResponse: +@router.post("/logout", status_code=status.HTTP_200_OK) +def logout(request: Request) -> None: """Session logout endpoint Args: request (Request): Fastapi Request object - - Returns: - MessageResponse: Standard message response """ request.session.clear() - return {"msg": "Successfully logged out"} - @router.post("/token") async def token(form_data: Annotated[OAuth2RequestForm, Depends()]) -> TokenResponse: @@ -273,14 +262,14 @@ async def auth_openid(request: Request): return RedirectResponse(url="/") -@router.post("/forgot-password") -def request_password_reset(username: str = Body(..., embed=True)) -> MessageResponse: - """ "Request a password reset link for the user. +@router.post("/forgot-password", status_code=status.HTTP_200_OK) +def request_password_reset(username: str = Body(..., embed=True)) -> None: + """Request a password reset link for the user. Args: username (str): Username of the user requesting the reset Returns: - MessageResponse: Confirmation message + None: Returns 200 OK status """ user = db_user_handler.get_user_by_username(username) @@ -291,14 +280,12 @@ def request_password_reset(username: str = Body(..., embed=True)) -> MessageResp f"Reset password link requested for a user {hl(username, color=CYAN)}, but that username does not exist." ) - return {"msg": "If the username exists, a reset link has been sent."} - -@router.post("/reset-password") +@router.post("/reset-password", status_code=status.HTTP_200_OK) def reset_password( token: str = Body(..., embed=True), new_password: str = Body(..., embed=True), -) -> MessageResponse: +) -> None: """Reset password using the token. Args: @@ -306,7 +293,7 @@ def reset_password( new_password (str): New user password Returns: - MessageResponse: Confirmation message + None: Returns 200 OK status """ user = auth_handler.verify_password_reset_token(token) @@ -315,4 +302,3 @@ def reset_password( log.info( f"Password was successfully reset for user {hl(user.username, color=CYAN)}." ) - return {"msg": "Password has been reset successfully."} diff --git a/backend/endpoints/collections.py b/backend/endpoints/collections.py index 2b173f3ca..476685a4f 100644 --- a/backend/endpoints/collections.py +++ b/backend/endpoints/collections.py @@ -3,7 +3,6 @@ from io import BytesIO from config import str_to_bool from decorators.auth import protected_route -from endpoints.responses import MessageResponse from endpoints.responses.collection import ( CollectionSchema, SmartCollectionSchema, @@ -264,7 +263,7 @@ async def update_collection( request (Request): Fastapi Request object Returns: - MessageResponse: Standard message response + CollectionSchema: Updated collection """ data = await request.form() @@ -390,7 +389,7 @@ async def update_smart_collection( @protected_route(router.delete, "/{id}", [Scope.COLLECTIONS_WRITE]) -async def delete_collections(request: Request, id: int) -> MessageResponse: +async def delete_collection(request: Request, id: int) -> None: """Delete collections endpoint Args: @@ -401,9 +400,6 @@ async def delete_collections(request: Request, id: int) -> MessageResponse: Raises: HTTPException: Collection not found - - Returns: - MessageResponse: Standard message response """ collection = db_collection_handler.get_collection(id) @@ -421,19 +417,14 @@ async def delete_collections(request: Request, id: int) -> MessageResponse: f"Couldn't find resources to delete for {hl(collection.name, color=BLUE)}" ) - return {"msg": f"{collection.name} deleted successfully!"} - @protected_route(router.delete, "/smart/{id}", [Scope.COLLECTIONS_WRITE]) -async def delete_smart_collection(request: Request, id: int) -> MessageResponse: +async def delete_smart_collection(request: Request, id: int) -> None: """Delete smart collection endpoint Args: request (Request): Fastapi Request object id (int): Smart collection id - - Returns: - MessageResponse: Standard message response """ smart_collection = db_collection_handler.get_smart_collection(id) @@ -446,5 +437,3 @@ async def delete_smart_collection(request: Request, id: int) -> MessageResponse: log.info(f"Deleting {hl(smart_collection.name, color=BLUE)} from database") db_collection_handler.delete_smart_collection(id) - - return {"msg": f"{smart_collection.name} deleted successfully!"} diff --git a/backend/endpoints/configs.py b/backend/endpoints/configs.py index b195ef13c..67f92e2e2 100644 --- a/backend/endpoints/configs.py +++ b/backend/endpoints/configs.py @@ -1,6 +1,5 @@ from config.config_manager import config_manager as cm from decorators.auth import protected_route -from endpoints.responses import MessageResponse from endpoints.responses.config import ConfigResponse from exceptions.config_exceptions import ( ConfigNotReadableException, @@ -45,7 +44,7 @@ def get_config() -> ConfigResponse: @protected_route(router.post, "/system/platforms", [Scope.PLATFORMS_WRITE]) -async def add_platform_binding(request: Request) -> MessageResponse: +async def add_platform_binding(request: Request) -> None: """Add platform binding to the configuration""" data = await request.json() @@ -60,11 +59,9 @@ async def add_platform_binding(request: Request) -> MessageResponse: status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - return {"msg": f"{fs_slug} binded to: {slug} successfully!"} - @protected_route(router.delete, "/system/platforms/{fs_slug}", [Scope.PLATFORMS_WRITE]) -async def delete_platform_binding(request: Request, fs_slug: str) -> MessageResponse: +async def delete_platform_binding(request: Request, fs_slug: str) -> None: """Delete platform binding from the configuration""" try: @@ -75,11 +72,9 @@ async def delete_platform_binding(request: Request, fs_slug: str) -> MessageResp status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - return {"msg": f"{fs_slug} bind removed successfully!"} - @protected_route(router.post, "/system/versions", [Scope.PLATFORMS_WRITE]) -async def add_platform_version(request: Request) -> MessageResponse: +async def add_platform_version(request: Request) -> None: """Add platform version to the configuration""" data = await request.json() @@ -94,11 +89,9 @@ async def add_platform_version(request: Request) -> MessageResponse: status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - return {"msg": f"Added {fs_slug} as version of: {slug} successfully!"} - @protected_route(router.delete, "/system/versions/{fs_slug}", [Scope.PLATFORMS_WRITE]) -async def delete_platform_version(request: Request, fs_slug: str) -> MessageResponse: +async def delete_platform_version(request: Request, fs_slug: str) -> None: """Delete platform version from the configuration""" try: @@ -109,11 +102,9 @@ async def delete_platform_version(request: Request, fs_slug: str) -> MessageResp status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - return {"msg": f"{fs_slug} version removed successfully!"} - @protected_route(router.post, "/exclude", [Scope.PLATFORMS_WRITE]) -async def add_exclusion(request: Request) -> MessageResponse: +async def add_exclusion(request: Request) -> None: """Add platform exclusion to the configuration""" data = await request.json() @@ -127,10 +118,6 @@ async def add_exclusion(request: Request) -> MessageResponse: status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - return { - "msg": f"Exclusion {exclusion_value} added to {exclusion_type} successfully!" - } - @protected_route( router.delete, @@ -139,7 +126,7 @@ async def add_exclusion(request: Request) -> MessageResponse: ) async def delete_exclusion( request: Request, exclusion_type: str, exclusion_value: str -) -> MessageResponse: +) -> None: """Delete platform binding from the configuration""" try: @@ -149,7 +136,3 @@ async def delete_exclusion( raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail=exc.message ) from exc - - return { - "msg": f"Exclusion {exclusion_value} removed from {exclusion_type} successfully!" - } diff --git a/backend/endpoints/firmware.py b/backend/endpoints/firmware.py index 864bc84ce..bb4014b45 100644 --- a/backend/endpoints/firmware.py +++ b/backend/endpoints/firmware.py @@ -1,6 +1,6 @@ from config import DISABLE_DOWNLOAD_ENDPOINT_AUTH from decorators.auth import protected_route -from endpoints.responses import MessageResponse +from endpoints.responses import BulkOperationResponse from endpoints.responses.firmware import AddFirmwareResponse, FirmwareSchema from fastapi import File, HTTPException, Request, UploadFile, status from fastapi.responses import FileResponse @@ -200,7 +200,7 @@ def get_firmware_content( @protected_route(router.post, "/delete", [Scope.FIRMWARE_WRITE]) async def delete_firmware( request: Request, -) -> MessageResponse: +) -> BulkOperationResponse: """Delete firmware endpoint Args: @@ -211,33 +211,47 @@ async def delete_firmware( delete_from_fs (bool, optional): Flag to delete rom from filesystem. Defaults to False. Returns: - MessageResponse: Standard message response + BulkOperationResponse: Bulk operation response with details """ data: dict = await request.json() firmware_ids: list = data["firmware"] delete_from_fs: list = data["delete_from_fs"] + successful_items = 0 + failed_items = 0 + errors = [] + for id in firmware_ids: firmware = db_firmware_handler.get_firmware(id) if not firmware: - error = f"Firmware with ID {hl(id)} not found" - log.error(error) - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=error) + failed_items += 1 + errors.append(f"Firmware with ID {id} not found") + continue - log.info(f"Deleting {hl(firmware.file_name)} from database") - db_firmware_handler.delete_firmware(id) + try: + log.info(f"Deleting {hl(firmware.file_name)} from database") + db_firmware_handler.delete_firmware(id) - if id in delete_from_fs: - log.info(f"Deleting {hl(firmware.file_name)} from filesystem") - try: - file_path = f"{firmware.file_path}/{firmware.file_name}" - await fs_firmware_handler.remove_file(file_path=file_path) - except FileNotFoundError as exc: - error = f"Firmware file {hl(firmware.file_name)} not found for platform {hl(firmware.platform_slug)}" - log.error(error) - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error - ) from exc + if id in delete_from_fs: + log.info(f"Deleting {hl(firmware.file_name)} from filesystem") + try: + file_path = f"{firmware.file_path}/{firmware.file_name}" + await fs_firmware_handler.remove_file(file_path=file_path) + except FileNotFoundError: + error = f"Firmware file {hl(firmware.file_name)} not found for platform {hl(firmware.platform_slug)}" + log.error(error) + errors.append(error) + failed_items += 1 + continue - return {"msg": f"{len(firmware_ids)} firmware files deleted successfully!"} + successful_items += 1 + except Exception as e: + failed_items += 1 + errors.append(f"Failed to delete firmware {id}: {str(e)}") + + return { + "successful_items": successful_items, + "failed_items": failed_items, + "errors": errors, + } diff --git a/backend/endpoints/platform.py b/backend/endpoints/platform.py index cab69e243..8270287be 100644 --- a/backend/endpoints/platform.py +++ b/backend/endpoints/platform.py @@ -2,7 +2,6 @@ from datetime import datetime, timezone from typing import Annotated from decorators.auth import protected_route -from endpoints.responses import MessageResponse from endpoints.responses.platform import PlatformSchema from exceptions.endpoint_exceptions import PlatformNotFoundInDatabaseException from exceptions.fs_exceptions import PlatformAlreadyExistsException @@ -203,7 +202,7 @@ async def update_platform( async def delete_platform( request: Request, id: Annotated[int, PathVar(description="Platform id.", ge=1)], -) -> MessageResponse: +) -> None: """Delete a platform.""" platform = db_platform_handler.get_platform(id) @@ -214,5 +213,3 @@ async def delete_platform( f"Deleting {hl(platform.name, color=BLUE)} [{hl(platform.fs_slug)}] from database" ) db_platform_handler.delete_platform(id) - - return {"msg": f"{platform.name} - [{platform.fs_slug}] deleted successfully!"} diff --git a/backend/endpoints/responses/__init__.py b/backend/endpoints/responses/__init__.py index 94a0a4bf4..e6e0f9898 100644 --- a/backend/endpoints/responses/__init__.py +++ b/backend/endpoints/responses/__init__.py @@ -1,5 +1,21 @@ from typing import TypedDict +from rq_scheduler.scheduler import JobStatus -class MessageResponse(TypedDict): - msg: str + +class TaskExecutionResponse(TypedDict): + task_name: str + task_id: str + status: JobStatus | None + queued_at: str + + +class TaskStatusResponse(TaskExecutionResponse): + started_at: str | None + ended_at: str | None + + +class BulkOperationResponse(TypedDict): + successful_items: int + failed_items: int + errors: list[str] diff --git a/backend/endpoints/responses/identity.py b/backend/endpoints/responses/identity.py index 590e13037..cd96572a6 100644 --- a/backend/endpoints/responses/identity.py +++ b/backend/endpoints/responses/identity.py @@ -8,7 +8,7 @@ from .base import BaseModel RAProgression = TypedDict( # type: ignore[misc] "RAProgression", - {k: NotRequired[v] for k, v in get_type_hints(RAUserProgression).items()}, + {k: NotRequired[v] for k, v in get_type_hints(RAUserProgression).items()}, # type: ignore[misc] total=False, ) diff --git a/backend/endpoints/rom.py b/backend/endpoints/rom.py index 6808b6de8..0ea602038 100644 --- a/backend/endpoints/rom.py +++ b/backend/endpoints/rom.py @@ -15,7 +15,7 @@ from config import ( str_to_bool, ) from decorators.auth import protected_route -from endpoints.responses import MessageResponse +from endpoints.responses import BulkOperationResponse from endpoints.responses.rom import ( DetailedRomSchema, RomFileSchema, @@ -874,40 +874,56 @@ async def delete_roms( default_factory=list, ), ], -) -> MessageResponse: +) -> BulkOperationResponse: """Delete roms.""" + successful_items = 0 + failed_items = 0 + errors = [] + for id in roms: rom = db_rom_handler.get_rom(id) if not rom: - raise RomNotFoundInDatabaseException(id) - - log.info( - f"Deleting {hl(str(rom.name or 'ROM'), color=BLUE)} [{hl(rom.fs_name)}] from database" - ) - db_rom_handler.delete_rom(id) + failed_items += 1 + errors.append(f"ROM with ID {id} not found") + continue try: - await fs_resource_handler.remove_directory(rom.fs_resources_path) - except FileNotFoundError: - log.warning( - f"Couldn't find resources to delete for {hl(str(rom.name or 'ROM'), color=BLUE)}" + log.info( + f"Deleting {hl(str(rom.name or 'ROM'), color=BLUE)} [{hl(rom.fs_name)}] from database" ) + db_rom_handler.delete_rom(id) - if id in delete_from_fs: - log.info(f"Deleting {hl(rom.fs_name)} from filesystem") try: - file_path = f"{rom.fs_path}/{rom.fs_name}" - await fs_rom_handler.remove_file(file_path=file_path) - except FileNotFoundError as exc: - error = f"Rom file {hl(rom.fs_name)} not found for platform {hl(rom.platform_display_name, color=BLUE)}[{hl(rom.platform_slug)}]" - log.error(error) - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, detail=error - ) from exc + await fs_resource_handler.remove_directory(rom.fs_resources_path) + except FileNotFoundError: + log.warning( + f"Couldn't find resources to delete for {hl(str(rom.name or 'ROM'), color=BLUE)}" + ) - return {"msg": f"{len(roms)} roms deleted successfully!"} + if id in delete_from_fs: + log.info(f"Deleting {hl(rom.fs_name)} from filesystem") + try: + file_path = f"{rom.fs_path}/{rom.fs_name}" + await fs_rom_handler.remove_file(file_path=file_path) + 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)}]" + log.error(error) + errors.append(error) + failed_items += 1 + continue + + successful_items += 1 + except Exception as e: + failed_items += 1 + errors.append(f"Failed to delete ROM {id}: {str(e)}") + + return { + "successful_items": successful_items, + "failed_items": failed_items, + "errors": errors, + } @protected_route( diff --git a/backend/endpoints/sockets/scan.py b/backend/endpoints/sockets/scan.py index ef755941a..74cdd8019 100644 --- a/backend/endpoints/sockets/scan.py +++ b/backend/endpoints/sockets/scan.py @@ -452,14 +452,14 @@ async def scan_platforms( if not metadata_sources: log.error("No metadata sources provided") await sm.emit("scan:done_ko", "No metadata sources provided") - return + return None try: fs_platforms: list[str] = await fs_platform_handler.get_platforms() except FolderStructureNotMatchException as e: log.error(e) await sm.emit("scan:done_ko", e.message) - return + return None scan_stats = ScanStats() @@ -508,7 +508,6 @@ async def scan_platforms( await sm.emit("scan:done", scan_stats.__dict__) except ScanStoppedException: await stop_scan() - return except Exception as e: log.error(f"Error in scan_platform: {e}") # Catch all exceptions and emit error to the client diff --git a/backend/endpoints/tasks.py b/backend/endpoints/tasks.py index 13cb10973..f5d65c62f 100644 --- a/backend/endpoints/tasks.py +++ b/backend/endpoints/tasks.py @@ -1,13 +1,16 @@ +from datetime import datetime, timezone + from config import ( ENABLE_RESCAN_ON_FILESYSTEM_CHANGE, RESCAN_ON_FILESYSTEM_CHANGE_DELAY, ) from decorators.auth import protected_route -from endpoints.responses import MessageResponse +from endpoints.responses import TaskExecutionResponse, TaskStatusResponse from endpoints.responses.tasks import GroupedTasksDict, TaskInfo from fastapi import HTTPException, Request from handler.auth.constants import Scope from handler.redis_handler import low_prio_queue +from rq.job import Job from tasks.manual.cleanup_orphaned_resources import cleanup_orphaned_resources_task from tasks.scheduled.scan_library import scan_library_task from tasks.scheduled.update_launchbox_metadata import update_launchbox_metadata_task @@ -34,14 +37,12 @@ manual_tasks: dict[str, Task] = { def _build_task_info(name: str, task: Task) -> TaskInfo: """Builds a TaskInfo object from task details.""" return TaskInfo( - { - "name": name, - "title": task.title, - "description": task.description, - "enabled": task.enabled, - "manual_run": task.manual_run, - "cron_string": task.cron_string or "", - } + name=name, + title=task.title, + description=task.description, + enabled=task.enabled, + manual_run=task.manual_run, + cron_string=task.cron_string or "", ) @@ -70,28 +71,64 @@ async def list_tasks(request: Request) -> GroupedTasksDict: # Add the adhoc watcher task grouped_tasks["watcher"].append( TaskInfo( - { - "name": "filesystem_watcher", - "manual_run": False, - "title": "Rescan on filesystem change", - "description": f"Runs a scan when a change is detected in the library path, with a {RESCAN_ON_FILESYSTEM_CHANGE_DELAY} minute delay", - "enabled": ENABLE_RESCAN_ON_FILESYSTEM_CHANGE, - "cron_string": "", - } + name="filesystem_watcher", + title="Rescan on filesystem change", + description=f"Runs a scan when a change is detected in the library path, with a {RESCAN_ON_FILESYSTEM_CHANGE_DELAY} minute delay", + enabled=ENABLE_RESCAN_ON_FILESYSTEM_CHANGE, + manual_run=False, + cron_string="", ) ) return grouped_tasks +@protected_route(router.get, "/{task_id}", [Scope.TASKS_RUN]) +async def get_task_by_id(request: Request, task_id: str) -> TaskStatusResponse: + """Get the status of a task by its job ID. + + Args: + request (Request): FastAPI Request object + task_id (str): Job ID of the task to retrieve status for + Returns: + TaskStatusResponse: Task status information + """ + try: + job = Job.fetch(task_id, connection=low_prio_queue.connection) + except Exception as e: + raise HTTPException( + status_code=404, + detail=f"Task with ID '{task_id}' not found", + ) from e + + # Convert datetime objects to ISO format strings + queued_at = job.created_at.isoformat() if job.created_at else None + started_at = job.started_at.isoformat() if job.started_at else None + ended_at = job.ended_at.isoformat() if job.ended_at else None + + # Get task name from job metadata or function name + task_name = ( + job.meta.get("task_name") or job.func_name if job.meta else job.func_name + ) + + return TaskStatusResponse( + task_name=str(task_name), + task_id=task_id, + status=job.get_status(), + queued_at=queued_at or "", + started_at=started_at, + ended_at=ended_at, + ) + + @protected_route(router.post, "/run", [Scope.TASKS_RUN]) -async def run_all_tasks(request: Request) -> MessageResponse: +async def run_all_tasks(request: Request) -> list[TaskExecutionResponse]: """Run all runnable tasks endpoint Args: request (Request): FastAPI Request object Returns: - MessageResponse: Standard message response + TaskExecutionResponse: Task execution response with details """ # Filter only runnable tasks runnable_tasks = { @@ -101,23 +138,36 @@ async def run_all_tasks(request: Request) -> MessageResponse: } if not runnable_tasks: - return {"msg": "No runnable tasks available to run"} + raise HTTPException( + status_code=400, + detail="No runnable tasks available to run", + ) - for _task_name, task_instance in runnable_tasks.items(): - low_prio_queue.enqueue(task_instance.run) + jobs = [ + (task_name, low_prio_queue.enqueue(task_instance.run)) + for task_name, task_instance in runnable_tasks.items() + ] - return {"msg": "All tasks launched, check the logs for details"} + return [ + { + "task_name": task_name, + "task_id": job.get_id(), + "status": job.get_status(), + "queued_at": datetime.now(timezone.utc).isoformat(), + } + for (task_name, job) in jobs + ] @protected_route(router.post, "/run/{task_name}", [Scope.TASKS_RUN]) -async def run_single_task(request: Request, task_name: str) -> MessageResponse: +async def run_single_task(request: Request, task_name: str) -> TaskExecutionResponse: """Run a single task endpoint. Args: request (Request): FastAPI Request object task_name (str): Name of the task to run Returns: - MessageResponse: Standard message response + TaskExecutionResponse: Task execution response with details """ all_tasks = {**manual_tasks, **scheduled_tasks} @@ -135,6 +185,11 @@ async def run_single_task(request: Request, task_name: str) -> MessageResponse: detail=f"Task '{task_name}' cannot be run", ) - low_prio_queue.enqueue(task_instance.run) + job = low_prio_queue.enqueue(task_instance.run) - return {"msg": f"Task '{task_name}' launched, check the logs for details"} + return { + "task_name": task_name, + "task_id": job.get_id(), + "status": job.get_status(), + "queued_at": datetime.now(timezone.utc).isoformat(), + } diff --git a/backend/endpoints/tests/test_assets.py b/backend/endpoints/tests/test_assets.py index 48df14b36..efe1c527b 100644 --- a/backend/endpoints/tests/test_assets.py +++ b/backend/endpoints/tests/test_assets.py @@ -1,4 +1,5 @@ import pytest +from fastapi import status from fastapi.testclient import TestClient from main import app @@ -15,7 +16,7 @@ def test_delete_saves(client, access_token, save): headers={"Authorization": f"Bearer {access_token}"}, json={"saves": [save.id]}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert len(body) == 1 @@ -27,7 +28,7 @@ def test_delete_states(client, access_token, state): headers={"Authorization": f"Bearer {access_token}"}, json={"states": [state.id]}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert len(body) == 1 diff --git a/backend/endpoints/tests/test_config.py b/backend/endpoints/tests/test_config.py index d8ff367c9..8fa5f5537 100644 --- a/backend/endpoints/tests/test_config.py +++ b/backend/endpoints/tests/test_config.py @@ -1,4 +1,5 @@ import pytest +from fastapi import status from fastapi.testclient import TestClient from main import app @@ -11,7 +12,7 @@ def client(): def test_config(client): response = client.get("/api/config") - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK config = response.json() assert config.get("EXCLUDED_PLATFORMS") == [] diff --git a/backend/endpoints/tests/test_heartbeat.py b/backend/endpoints/tests/test_heartbeat.py index 73cd291d3..7fb62f024 100644 --- a/backend/endpoints/tests/test_heartbeat.py +++ b/backend/endpoints/tests/test_heartbeat.py @@ -1,4 +1,5 @@ import pytest +from fastapi import status from fastapi.testclient import TestClient from main import app from utils import get_version @@ -12,7 +13,7 @@ def client(): def test_heartbeat(client): response = client.get("/api/heartbeat") - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK heartbeat = response.json() diff --git a/backend/endpoints/tests/test_identity.py b/backend/endpoints/tests/test_identity.py index 27eab2044..e15331868 100644 --- a/backend/endpoints/tests/test_identity.py +++ b/backend/endpoints/tests/test_identity.py @@ -5,6 +5,7 @@ from unittest import mock import pytest from endpoints.auth import ACCESS_TOKEN_EXPIRE_MINUTES +from fastapi import status from fastapi.testclient import TestClient from handler.auth import oauth_handler from handler.database.users_handler import DBUsersHandler @@ -28,28 +29,26 @@ def clear_cache(): def test_login_logout(client, admin_user): response = client.get("/api/login") - assert response.status_code == 405 + assert response.status_code == status.HTTP_405_METHOD_NOT_ALLOWED basic_auth = base64.b64encode(b"test_admin:test_admin_password").decode("ascii") response = client.post( "/api/login", headers={"Authorization": f"Basic {basic_auth}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK assert response.cookies.get("romm_session") - assert response.json()["msg"] == "Successfully logged in" response = client.post("/api/logout") - assert response.status_code == 200 - assert response.json()["msg"] == "Successfully logged out" + assert response.status_code == status.HTTP_200_OK def test_get_all_users(client, access_token): response = client.get( "/api/users", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK users = response.json() assert len(users) == 1 @@ -61,7 +60,7 @@ def test_get_user(client, access_token, editor_user): f"/api/users/{editor_user.id}", headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK user = response.json() assert user["username"] == "test_editor" @@ -158,7 +157,7 @@ def test_update_user(client, access_token, editor_user): data={"username": "editor_user_new_username", "role": "viewer"}, headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK user = response.json() assert user["role"] == "viewer" @@ -169,7 +168,4 @@ def test_delete_user(client, access_token, editor_user): f"/api/users/{editor_user.id}", headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 - - body = response.json() - assert body["msg"] == "User successfully deleted" + assert response.status_code == status.HTTP_200_OK diff --git a/backend/endpoints/tests/test_oauth.py b/backend/endpoints/tests/test_oauth.py index 12965acc3..7ea5a0011 100644 --- a/backend/endpoints/tests/test_oauth.py +++ b/backend/endpoints/tests/test_oauth.py @@ -1,5 +1,6 @@ import pytest from endpoints.auth import ACCESS_TOKEN_EXPIRE_MINUTES +from fastapi import status from fastapi.exceptions import HTTPException from fastapi.testclient import TestClient from handler.auth.constants import EDIT_SCOPES @@ -20,7 +21,7 @@ def test_refreshing_oauth_token_basic(client, refresh_token): "refresh_token": refresh_token, }, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert body["access_token"] @@ -37,7 +38,7 @@ def test_refreshing_oauth_token_without_refresh_token(client): }, ) except HTTPException as e: - assert e.status_code == 400 + assert e.status_code == status.HTTP_400_BAD_REQUEST assert e.detail == "Missing refresh token" @@ -51,7 +52,7 @@ def test_refreshing_oauth_token_with_invalid_refresh_token(client): }, ) except HTTPException as e: - assert e.status_code == 400 + assert e.status_code == status.HTTP_400_BAD_REQUEST assert e.detail == "Invalid refresh token" @@ -64,7 +65,7 @@ def test_auth_via_upass(client, admin_user): "password": "test_admin_password", }, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert body["access_token"] @@ -84,7 +85,7 @@ def test_auth_via_upass_with_invalid_credentials(client, admin_user): }, ) except HTTPException as e: - assert e.status_code == 401 + assert e.status_code == status.HTTP_401_UNAUTHORIZED assert e.detail == "Invalid username or password" @@ -100,7 +101,7 @@ def test_auth_via_upass_with_excess_scopes(client, viewer_user): }, ) except HTTPException as e: - assert e.status_code == 403 + assert e.status_code == status.HTTP_403_FORBIDDEN assert e.detail == "Insufficient scope" @@ -113,5 +114,5 @@ def test_auth_with_invalid_grant_type(client): }, ) except HTTPException as e: - assert e.status_code == 400 + assert e.status_code == status.HTTP_400_BAD_REQUEST assert e.detail == "Invalid or unsupported grant type" diff --git a/backend/endpoints/tests/test_platform.py b/backend/endpoints/tests/test_platform.py index 06e430a7e..d7596f0b1 100644 --- a/backend/endpoints/tests/test_platform.py +++ b/backend/endpoints/tests/test_platform.py @@ -1,4 +1,5 @@ import pytest +from fastapi import status from fastapi.testclient import TestClient from main import app @@ -11,12 +12,12 @@ def client(): def test_get_platforms(client, access_token, platform): response = client.get("/api/platforms") - assert response.status_code == 403 + assert response.status_code == status.HTTP_403_FORBIDDEN response = client.get( "/api/platforms", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK platforms = response.json() assert len(platforms) == 1 diff --git a/backend/endpoints/tests/test_raw.py b/backend/endpoints/tests/test_raw.py index 9caeafd3f..8a81b3709 100644 --- a/backend/endpoints/tests/test_raw.py +++ b/backend/endpoints/tests/test_raw.py @@ -1,4 +1,5 @@ import pytest +from fastapi import status from fastapi.testclient import TestClient from main import app @@ -13,12 +14,12 @@ def test_get_raw_asset(client, access_token): response = client.get( "/api/raw/assets/users/557365723a31/saves/n64/mupen64/Super Mario 64 (J) (Rev A).sav" ) - assert response.status_code == 403 + assert response.status_code == status.HTTP_403_FORBIDDEN response = client.get( "/api/raw/assets/users/557365723a31/saves/n64/mupen64/Super Mario 64 (J) (Rev A).sav", headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK assert "SUPER_MARIO_64_SAVE_FILE" in response.text assert response.headers["content-type"] == "text/plain; charset=utf-8" diff --git a/backend/endpoints/tests/test_rom.py b/backend/endpoints/tests/test_rom.py index 74cac13bd..179add401 100644 --- a/backend/endpoints/tests/test_rom.py +++ b/backend/endpoints/tests/test_rom.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from fastapi import status from fastapi.testclient import TestClient from handler.filesystem.roms_handler import FSRomsHandler from handler.metadata.igdb_handler import IGDBHandler, IGDBRom @@ -18,7 +19,7 @@ def test_get_rom(client, access_token, rom): f"/api/roms/{rom.id}", headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert body["id"] == rom.id @@ -30,7 +31,7 @@ def test_get_all_roms(client, access_token, rom, platform): headers={"Authorization": f"Bearer {access_token}"}, params={"platform_id": platform.id}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() @@ -72,7 +73,7 @@ def test_update_rom(rename_fs_rom_mock, get_rom_by_id_mock, client, access_token "age_ratings": "[1, 2]", }, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() assert body["fs_name"] == "Metroid Prime Remastered.zip" @@ -87,7 +88,7 @@ def test_delete_roms(client, access_token, rom): headers={"Authorization": f"Bearer {access_token}"}, json={"roms": [rom.id], "delete_from_fs": []}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK body = response.json() - assert body["msg"] == "1 roms deleted successfully!" + assert body["successful_items"] == 1 diff --git a/backend/endpoints/tests/test_tasks.py b/backend/endpoints/tests/test_tasks.py index 81c0f516a..a6a2699ca 100644 --- a/backend/endpoints/tests/test_tasks.py +++ b/backend/endpoints/tests/test_tasks.py @@ -89,7 +89,7 @@ class TestListTasks: "/api/tasks", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK data = response.json() # Check structure @@ -137,7 +137,7 @@ class TestListTasks: "/api/tasks", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK data = response.json() assert data["scheduled"] == [] @@ -172,13 +172,18 @@ class TestListTasks: response = client.get( "/api/tasks", headers={"Authorization": f"Bearer {token}"} ) - assert response.status_code == 403 + assert response.status_code == status.HTTP_403_FORBIDDEN class TestRunAllTasks: """Test suite for the run_all_tasks endpoint""" - @patch("endpoints.tasks.low_prio_queue") + @patch( + "endpoints.tasks.low_prio_queue.enqueue", + return_value=Mock( + get_id=Mock(return_value="1"), get_status=Mock(return_value="queued") + ), + ) @patch( "endpoints.tasks.manual_tasks", { @@ -201,14 +206,12 @@ class TestRunAllTasks: "/api/tasks/run", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK data = response.json() - assert data["msg"] == "All tasks launched, check the logs for details" - - # Verify that enqueue was called for each runnable task - assert ( - mock_queue.enqueue.call_count == 3 - ) # task1, task2, task3 (task4 is disabled) + assert len(data) == 3 + assert data[0]["task_name"] == "task1" + assert data[1]["task_name"] == "task2" + assert data[2]["task_name"] == "task3" @patch("endpoints.tasks.low_prio_queue") @patch("endpoints.tasks.manual_tasks", {}) @@ -219,12 +222,12 @@ class TestRunAllTasks: "/api/tasks/run", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() - assert data["msg"] == "No runnable tasks available to run" + assert data["detail"] == "No runnable tasks available to run" # Verify that enqueue was not called - mock_queue.enqueue.assert_not_called() + mock_queue.assert_not_called() @patch("endpoints.tasks.low_prio_queue") @patch( @@ -245,9 +248,9 @@ class TestRunAllTasks: "/api/tasks/run", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() - assert data["msg"] == "No runnable tasks available to run" + assert data["detail"] == "No runnable tasks available to run" # Verify that enqueue was not called since no tasks are both enabled and manual mock_queue.enqueue.assert_not_called() @@ -261,7 +264,12 @@ class TestRunAllTasks: class TestRunSingleTask: """Test suite for the run_single_task endpoint""" - @patch("endpoints.tasks.low_prio_queue") + @patch( + "endpoints.tasks.low_prio_queue.enqueue", + return_value=Mock( + get_id=Mock(return_value="1"), get_status=Mock(return_value="queued") + ), + ) @patch( "endpoints.tasks.manual_tasks", {"test_task": Mock(spec=Task, enabled=True, manual_run=True, run=Mock())}, @@ -274,12 +282,15 @@ class TestRunSingleTask: headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK data = response.json() - assert data["msg"] == "Task 'test_task' launched, check the logs for details" - # Verify that enqueue was called - mock_queue.enqueue.assert_called_once() + assert data["task_name"] == "test_task" + assert data["task_id"] == "1" + assert data["status"] == "queued" + assert "queued_at" in data + + mock_queue.assert_called_once() @patch("endpoints.tasks.manual_tasks", {}) @patch("endpoints.tasks.scheduled_tasks", {}) @@ -290,10 +301,9 @@ class TestRunSingleTask: headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 404 + assert response.status_code == status.HTTP_404_NOT_FOUND data = response.json() assert "not found" in data["detail"].lower() - assert "available tasks are" in data["detail"] @patch("endpoints.tasks.low_prio_queue") @patch( @@ -308,13 +318,10 @@ class TestRunSingleTask: headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() assert "cannot be run" in data["detail"].lower() - # Verify that enqueue was not called - mock_queue.enqueue.assert_not_called() - @patch("endpoints.tasks.low_prio_queue") @patch( "endpoints.tasks.manual_tasks", @@ -332,19 +339,140 @@ class TestRunSingleTask: headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 400 + assert response.status_code == status.HTTP_400_BAD_REQUEST data = response.json() assert "cannot be run" in data["detail"].lower() - # Verify that enqueue was not called - mock_queue.enqueue.assert_not_called() - def test_run_single_task_unauthorized(self, client): - """Test that unauthorized requests are rejected""" + """Test running a task without authentication""" response = client.post("/api/tasks/run/test_task") assert response.status_code == status.HTTP_403_FORBIDDEN +class TestGetTaskById: + """Test suite for the get_task_by_id endpoint""" + + @patch("endpoints.tasks.low_prio_queue") + @patch("endpoints.tasks.Job.fetch") + def test_get_task_by_id_success( + self, mock_job_fetch, mock_queue, client, access_token + ): + """Test successful retrieval of a task by job ID""" + # Mock job object with all necessary attributes + mock_job = Mock() + mock_job.created_at = Mock() + mock_job.created_at.isoformat.return_value = "2023-01-01T00:00:00" + mock_job.started_at = Mock() + mock_job.started_at.isoformat.return_value = "2023-01-01T00:01:00" + mock_job.ended_at = Mock() + mock_job.ended_at.isoformat.return_value = "2023-01-01T00:02:00" + mock_job.meta = {"task_name": "test_task"} + mock_job.func_name = "test_task" + mock_job.get_status.return_value = "finished" + + mock_job_fetch.return_value = mock_job + + response = client.get( + "/api/tasks/test-job-id-123", + headers={"Authorization": f"Bearer {access_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + + assert data["task_name"] == "test_task" + assert data["task_id"] == "test-job-id-123" + assert data["status"] == "finished" + assert data["queued_at"] == "2023-01-01T00:00:00" + assert data["started_at"] == "2023-01-01T00:01:00" + assert data["ended_at"] == "2023-01-01T00:02:00" + + mock_job_fetch.assert_called_once_with( + "test-job-id-123", connection=mock_queue.connection + ) + + @patch("endpoints.tasks.low_prio_queue") + @patch("endpoints.tasks.Job.fetch") + def test_get_task_by_id_not_found( + self, mock_job_fetch, mock_queue, client, access_token + ): + """Test retrieval of a non-existent task by job ID""" + mock_job_fetch.side_effect = Exception("Job not found") + + response = client.get( + "/api/tasks/nonexistent-job-id", + headers={"Authorization": f"Bearer {access_token}"}, + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + data = response.json() + assert "not found" in data["detail"].lower() + + @patch("endpoints.tasks.low_prio_queue") + @patch("endpoints.tasks.Job.fetch") + def test_get_task_by_id_with_exception_info( + self, mock_job_fetch, mock_queue, client, access_token + ): + """Test retrieval of a task that failed with exception""" + mock_job = Mock() + mock_job.created_at = Mock() + mock_job.created_at.isoformat.return_value = "2023-01-01T00:00:00" + mock_job.started_at = Mock() + mock_job.started_at.isoformat.return_value = "2023-01-01T00:01:00" + mock_job.ended_at = Mock() + mock_job.ended_at.isoformat.return_value = "2023-01-01T00:01:30" + mock_job.meta = {"task_name": "test_task"} + mock_job.func_name = "test_task" + mock_job.get_status.return_value = "failed" + + mock_job_fetch.return_value = mock_job + + response = client.get( + "/api/tasks/failed-job-id", + headers={"Authorization": f"Bearer {access_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + + assert data["status"] == "failed" + + @patch("endpoints.tasks.low_prio_queue") + @patch("endpoints.tasks.Job.fetch") + def test_get_task_by_id_no_metadata( + self, mock_job_fetch, mock_queue, client, access_token + ): + """Test retrieval of a task with no metadata""" + mock_job = Mock() + mock_job.created_at = Mock() + mock_job.created_at.isoformat.return_value = "2023-01-01T00:00:00" + mock_job.started_at = None + mock_job.ended_at = None + mock_job.meta = None + mock_job.func_name = "test_task" + mock_job.get_status.return_value = "queued" + + mock_job_fetch.return_value = mock_job + + response = client.get( + "/api/tasks/queued-job-id", + headers={"Authorization": f"Bearer {access_token}"}, + ) + + assert response.status_code == status.HTTP_200_OK + data = response.json() + + assert data["task_name"] == "test_task" + assert data["status"] == "queued" + assert data["started_at"] is None + assert data["ended_at"] is None + + def test_get_task_by_id_unauthorized(self, client): + """Test retrieval of a task without authentication""" + response = client.get("/api/tasks/test-job-id") + assert response.status_code == status.HTTP_403_FORBIDDEN + + class TestTaskInfoBuilding: """Test suite for the _build_task_info helper function""" @@ -381,7 +509,7 @@ class TestTaskInfoBuilding: "/api/tasks", headers={"Authorization": f"Bearer {access_token}"} ) - assert response.status_code == 200 + assert response.status_code == status.HTTP_200_OK # The mock ensures the structure is correct @@ -390,14 +518,19 @@ class TestIntegration: @patch("endpoints.tasks.ENABLE_RESCAN_ON_FILESYSTEM_CHANGE", True) @patch("endpoints.tasks.RESCAN_ON_FILESYSTEM_CHANGE_DELAY", 5) - @patch("endpoints.tasks.low_prio_queue") + @patch( + "endpoints.tasks.low_prio_queue.enqueue", + return_value=Mock( + get_id=Mock(return_value="1"), get_status=Mock(return_value="queued") + ), + ) def test_full_workflow(self, mock_queue, client, access_token): """Test a complete workflow: list tasks, then run a specific task""" # First, list all tasks list_response = client.get( "/api/tasks", headers={"Authorization": f"Bearer {access_token}"} ) - assert list_response.status_code == 200 + assert list_response.status_code == status.HTTP_200_OK # Then run a specific task (if any exist) with patch( @@ -413,8 +546,8 @@ class TestIntegration: "/api/tasks/run/workflow_task", headers={"Authorization": f"Bearer {access_token}"}, ) - assert run_response.status_code == 200 - assert mock_queue.enqueue.called + assert run_response.status_code == status.HTTP_200_OK + assert mock_queue.called def test_error_handling(self, client, access_token): """Test error handling for various scenarios""" @@ -423,4 +556,4 @@ class TestIntegration: "/api/tasks/run/invalid_task_name_with_special_chars!@#", headers={"Authorization": f"Bearer {access_token}"}, ) - assert response.status_code == 404 + assert response.status_code == status.HTTP_404_NOT_FOUND diff --git a/backend/endpoints/user.py b/backend/endpoints/user.py index e44673114..a875cced8 100644 --- a/backend/endpoints/user.py +++ b/backend/endpoints/user.py @@ -2,7 +2,6 @@ from typing import Annotated, Any from decorators.auth import protected_route from endpoints.forms.identity import UserForm -from endpoints.responses import MessageResponse from endpoints.responses.identity import InviteLinkSchema, UserSchema from fastapi import Body, Form, HTTPException, Request, status from handler.auth import auth_handler @@ -317,7 +316,7 @@ async def update_user( @protected_route(router.delete, "/{id}", [Scope.USERS_WRITE]) -async def delete_user(request: Request, id: int) -> MessageResponse: +async def delete_user(request: Request, id: int) -> None: """Delete user endpoint Args: @@ -328,9 +327,6 @@ async def delete_user(request: Request, id: int) -> MessageResponse: HTTPException: User is not found in database HTTPException: User deleting itself HTTPException: User is the last admin user - - Returns: - MessageResponse: Standard message response """ user = db_user_handler.get_user(id) @@ -356,11 +352,23 @@ async def delete_user(request: Request, id: int) -> MessageResponse: except FileNotFoundError: log.warning(f"Couldn't find avatar directory to delete for {user.username}") - return {"msg": "User successfully deleted"} +@protected_route( + router.post, "/{id}/ra/refresh", [Scope.ME_WRITE], status_code=status.HTTP_200_OK +) +async def refresh_retro_achievements(request: Request, id: int) -> None: + """Refresh RetroAchievements data for a user. -@protected_route(router.post, "/{id}/ra/refresh", [Scope.ME_WRITE]) -async def refresh_retro_achievements(request: Request, id: int) -> MessageResponse: + Args: + request (Request): FastAPI Request object + id (int): User ID + + Raises: + HTTPException: User not found or no RetroAchievements username set + + Returns: + None: Returns 200 OK status + """ user = db_user_handler.get_user(id) if user and user.ra_username: user_progression = await meta_ra_handler.get_user_progression(user.ra_username) @@ -370,9 +378,9 @@ async def refresh_retro_achievements(request: Request, id: int) -> MessageRespon "ra_progression": user_progression, }, ) - return {"msg": "RetroAchievements successfully refreshed"} - else: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail="User does not have a RetroAchievements username set", - ) + return None + + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="User does not have a RetroAchievements username set", + ) diff --git a/backend/handler/auth/middleware.py b/backend/handler/auth/middleware.py index a161f5704..24bae723d 100644 --- a/backend/handler/auth/middleware.py +++ b/backend/handler/auth/middleware.py @@ -16,7 +16,7 @@ class CustomCSRFMiddleware(CSRFMiddleware): # Skip CSRF check if not an HTTP request, like websockets if scope["type"] != "http": await self.app(scope, receive, send) - return + return None request = Request(scope, receive) @@ -24,7 +24,7 @@ class CustomCSRFMiddleware(CSRFMiddleware): auth_scheme = request.headers.get("Authorization", "").split(" ", 1)[0].lower() if auth_scheme == "bearer" or auth_scheme == "basic": await self.app(scope, receive, send) - return + return None await super().__call__(scope, receive, send) @@ -97,7 +97,7 @@ class SessionMiddleware: async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if scope["type"] not in ("http", "websocket"): # pragma: no cover await self.app(scope, receive, send) - return + return None connection = HTTPConnection(scope) initial_session_was_empty = True diff --git a/backend/handler/auth/tests/test_auth.py b/backend/handler/auth/tests/test_auth.py index d3924fa3d..eaf715d07 100644 --- a/backend/handler/auth/tests/test_auth.py +++ b/backend/handler/auth/tests/test_auth.py @@ -1,6 +1,7 @@ from base64 import b64encode import pytest +from fastapi import status from fastapi.exceptions import HTTPException from handler.auth import auth_handler, oauth_handler from handler.auth.constants import EDIT_SCOPES @@ -51,7 +52,7 @@ async def test_get_current_active_user_from_session_bad_username(editor_user: Us try: await auth_handler.get_current_active_user_from_session(conn) except HTTPException as e: - assert e.status_code == 403 + assert e.status_code == status.HTTP_403_FORBIDDEN assert e.detail == "User not found" @@ -69,7 +70,7 @@ async def test_get_current_active_user_from_session_disabled_user(editor_user: U try: await auth_handler.get_current_active_user_from_session(conn) except HTTPException as e: - assert e.status_code == 403 + assert e.status_code == status.HTTP_403_FORBIDDEN assert e.detail == "Inactive user test_editor" diff --git a/backend/handler/auth/tests/test_oauth.py b/backend/handler/auth/tests/test_oauth.py index 4978aaab7..9b3749062 100644 --- a/backend/handler/auth/tests/test_oauth.py +++ b/backend/handler/auth/tests/test_oauth.py @@ -1,6 +1,6 @@ import pytest from decorators.auth import protected_route -from fastapi import Request +from fastapi import Request, status from fastapi.exceptions import HTTPException from handler.auth import oauth_handler from handler.database import db_user_handler @@ -62,7 +62,7 @@ async def test_get_current_active_user_from_bearer_token_disabled_user(admin_use try: await oauth_handler.get_current_active_user_from_bearer_token(token) except HTTPException as e: - assert e.status_code == 401 + assert e.status_code == status.HTTP_401_UNAUTHORIZED assert e.detail == "Disabled user" diff --git a/backend/handler/filesystem/resources_handler.py b/backend/handler/filesystem/resources_handler.py index e20e7136c..77063f66a 100644 --- a/backend/handler/filesystem/resources_handler.py +++ b/backend/handler/filesystem/resources_handler.py @@ -4,6 +4,7 @@ from pathlib import Path import httpx from config import RESOURCES_BASE_PATH +from fastapi import status from logger.logger import log from models.collection import Collection from models.rom import Rom @@ -66,7 +67,7 @@ class FSResourcesHandler(FSHandler): httpx_client = ctx_httpx_client.get() try: async with httpx_client.stream("GET", url_cover, timeout=120) as response: - if response.status_code == 200: + if response.status_code == status.HTTP_200_OK: async with await self.write_file_streamed( path=cover_file, filename=f"{size.value}.png" ) as f: @@ -182,7 +183,7 @@ class FSResourcesHandler(FSHandler): async with httpx_client.stream( "GET", url_screenhot, timeout=120 ) as response: - if response.status_code == 200: + if response.status_code == status.HTTP_200_OK: async with await self.write_file_streamed( path=screenshot_path, filename=f"{idx}.jpg" ) as f: @@ -233,7 +234,7 @@ class FSResourcesHandler(FSHandler): httpx_client = ctx_httpx_client.get() try: async with httpx_client.stream("GET", url_manual, timeout=120) as response: - if response.status_code == 200: + if response.status_code == status.HTTP_200_OK: async with await self.write_file_streamed( path=manual_path, filename=f"{rom.id}.pdf" ) as f: @@ -275,7 +276,7 @@ class FSResourcesHandler(FSHandler): try: async with httpx_client.stream("GET", url, timeout=120) as response: - if response.status_code == 200: + if response.status_code == status.HTTP_200_OK: async with await self.write_file_streamed( path=directory, filename=filename ) as f: diff --git a/backend/handler/filesystem/roms_handler.py b/backend/handler/filesystem/roms_handler.py index b00e3a065..47ff6b153 100644 --- a/backend/handler/filesystem/roms_handler.py +++ b/backend/handler/filesystem/roms_handler.py @@ -138,7 +138,7 @@ def read_zip_file(file: str | os.PathLike[str] | IO[bytes]) -> Iterator[bytes]: yield chunk # We only need to read the first file in the archive - return + return None except zipfile.BadZipFile: if isinstance(file, Path): for chunk in read_basic_file(file): @@ -164,7 +164,7 @@ def read_tar_file( yield chunk # We only need to read the first file in the archive - return + return None except tarfile.ReadError: for chunk in read_basic_file(file_path): yield chunk diff --git a/backend/handler/metadata/igdb_handler.py b/backend/handler/metadata/igdb_handler.py index 211523fd3..82471dba6 100644 --- a/backend/handler/metadata/igdb_handler.py +++ b/backend/handler/metadata/igdb_handler.py @@ -678,7 +678,7 @@ class TwitchAuth(MetadataHandler): timeout=self.timeout, ) - if res.status_code == 400: + if res.status_code == status.HTTP_400_BAD_REQUEST: log.critical("IGDB Error: Invalid IGDB_CLIENT_ID or IGDB_CLIENT_SECRET") return "" diff --git a/backend/tasks/manual/cleanup_orphaned_resources.py b/backend/tasks/manual/cleanup_orphaned_resources.py index 47b307a39..dcc120fe1 100644 --- a/backend/tasks/manual/cleanup_orphaned_resources.py +++ b/backend/tasks/manual/cleanup_orphaned_resources.py @@ -28,7 +28,7 @@ class CleanupOrphanedResourcesTask(Task): roms_resources_path = os.path.join(RESOURCES_BASE_PATH, "roms") if not os.path.exists(roms_resources_path): log.info("Resources path does not exist, skipping cleanup") - return + return None existing_platforms = { str(platform.id) for platform in db_platform_handler.get_platforms() diff --git a/backend/tasks/scheduled/scan_library.py b/backend/tasks/scheduled/scan_library.py index 6d4b99f4e..47e21874d 100644 --- a/backend/tasks/scheduled/scan_library.py +++ b/backend/tasks/scheduled/scan_library.py @@ -30,7 +30,7 @@ class ScanLibraryTask(PeriodicTask): if not ENABLE_SCHEDULED_RESCAN: log.info("Scheduled library scan not enabled, unscheduling...") self.unschedule() - return + return None source_mapping: dict[str, bool] = { MetadataSource.IGDB: IGDB_API_ENABLED, @@ -46,7 +46,7 @@ class ScanLibraryTask(PeriodicTask): if not metadata_sources: log.warning("No metadata sources enabled, unscheduling library scan") self.unschedule() - return + return None log.info("Scheduled library scan started...") await scan_platforms( diff --git a/backend/tasks/scheduled/update_launchbox_metadata.py b/backend/tasks/scheduled/update_launchbox_metadata.py index 7caabcac8..005fd4778 100644 --- a/backend/tasks/scheduled/update_launchbox_metadata.py +++ b/backend/tasks/scheduled/update_launchbox_metadata.py @@ -39,12 +39,12 @@ class UpdateLaunchboxMetadataTask(RemoteFilePullTask): async def run(self, force: bool = False) -> None: if not LAUNCHBOX_API_ENABLED: log.warning("Launchbox API is not enabled, skipping metadata update") - return + return None content = await super().run(force) if content is None: log.warning("No content received from launchbox metadata update") - return + return None try: zip_file_bytes = BytesIO(content) @@ -237,7 +237,7 @@ class UpdateLaunchboxMetadataTask(RemoteFilePullTask): except zipfile.BadZipFile: log.error("Bad zip file in launchbox metadata update") - return + return None log.info("Scheduled launchbox metadata update completed!") diff --git a/backend/tasks/scheduled/update_switch_titledb.py b/backend/tasks/scheduled/update_switch_titledb.py index be011e969..3f7c7a519 100644 --- a/backend/tasks/scheduled/update_switch_titledb.py +++ b/backend/tasks/scheduled/update_switch_titledb.py @@ -31,7 +31,7 @@ class UpdateSwitchTitleDBTask(RemoteFilePullTask): async def run(self, force: bool = False) -> None: content = await super().run(force) if content is None: - return + return None index_json = json.loads(content) relevant_data = {k: v for k, v in index_json.items() if k and v} diff --git a/backend/tasks/tasks.py b/backend/tasks/tasks.py index a6db89ea1..92fa13e10 100644 --- a/backend/tasks/tasks.py +++ b/backend/tasks/tasks.py @@ -64,7 +64,7 @@ class PeriodicTask(Task): if self._get_existing_job(): log.info(f"{self.description.capitalize()} is already scheduled.") - return + return None if self.cron_string: return tasks_scheduler.cron( @@ -80,7 +80,7 @@ class PeriodicTask(Task): if not job: log.info(f"{self.description.capitalize()} is not scheduled.") - return + return None tasks_scheduler.cancel(job) log.info(f"{self.description.capitalize()} unscheduled.") diff --git a/backend/tools/xml_diagnostics.py b/backend/tools/xml_diagnostics.py index 2a3687e20..81f4df6c8 100644 --- a/backend/tools/xml_diagnostics.py +++ b/backend/tools/xml_diagnostics.py @@ -44,7 +44,7 @@ def diagnose_xml(filename): chunk_number += 1 except Exception as e: print(f"Error reading file: {e}") - return + return None # Then try SAX parsing for detailed error reporting parser = make_parser() diff --git a/backend/watcher.py b/backend/watcher.py index 0e9617d84..cae397268 100644 --- a/backend/watcher.py +++ b/backend/watcher.py @@ -51,13 +51,13 @@ class EventHandler(FileSystemEventHandler): event: The event object representing the file system event. """ if not ENABLE_RESCAN_ON_FILESYSTEM_CHANGE: - return + return None src_path = os.fsdecode(event.src_path) # Ignore .DS_Store files if src_path.endswith(".DS_Store"): - return + return None event_src = src_path.split(path)[-1] fs_slug = event_src.split("/")[1] @@ -71,11 +71,11 @@ class EventHandler(FileSystemEventHandler): if job.func_name == "endpoints.sockets.scan.scan_platforms": if job.args[0] == []: log.info("Full rescan already scheduled") - return + return None if db_platform and db_platform.id in job.args[0]: log.info(f"Scan already scheduled for {hl(fs_slug)}") - return + return None time_delta = timedelta(minutes=RESCAN_ON_FILESYSTEM_CHANGE_DELAY) rescan_in_msg = f"rescanning in {hl(str(RESCAN_ON_FILESYSTEM_CHANGE_DELAY), color=CYAN)} minutes." @@ -93,7 +93,6 @@ class EventHandler(FileSystemEventHandler): [db_platform.id], scan_type=ScanType.QUICK, ) - return if __name__ == "__main__": diff --git a/frontend/src/__generated__/index.ts b/frontend/src/__generated__/index.ts index 789cfd47e..5f23760fd 100644 --- a/frontend/src/__generated__/index.ts +++ b/frontend/src/__generated__/index.ts @@ -17,6 +17,7 @@ export type { Body_update_collection_api_collections__id__put } from './models/B export type { Body_update_platform_api_platforms__id__put } from './models/Body_update_platform_api_platforms__id__put'; export type { Body_update_rom_api_roms__id__put } from './models/Body_update_rom_api_roms__id__put'; export type { Body_update_rom_user_api_roms__id__props_put } from './models/Body_update_rom_user_api_roms__id__props_put'; +export type { BulkOperationResponse } from './models/BulkOperationResponse'; export type { CollectionSchema } from './models/CollectionSchema'; export type { ConfigResponse } from './models/ConfigResponse'; export type { CustomLimitOffsetPage_SimpleRomSchema_ } from './models/CustomLimitOffsetPage_SimpleRomSchema_'; @@ -32,8 +33,8 @@ export type { IGDBAgeRating } from './models/IGDBAgeRating'; export type { IGDBMetadataPlatform } from './models/IGDBMetadataPlatform'; export type { IGDBRelatedGame } from './models/IGDBRelatedGame'; export type { InviteLinkSchema } from './models/InviteLinkSchema'; +export type { JobStatus } from './models/JobStatus'; export type { LaunchboxImage } from './models/LaunchboxImage'; -export type { MessageResponse } from './models/MessageResponse'; export type { MetadataSourcesDict } from './models/MetadataSourcesDict'; export type { MobyMetadataPlatform } from './models/MobyMetadataPlatform'; export type { OIDCDict } from './models/OIDCDict'; @@ -63,7 +64,9 @@ export type { SmartCollectionSchema } from './models/SmartCollectionSchema'; export type { StateSchema } from './models/StateSchema'; export type { StatsReturn } from './models/StatsReturn'; export type { SystemDict } from './models/SystemDict'; +export type { TaskExecutionResponse } from './models/TaskExecutionResponse'; export type { TaskInfo } from './models/TaskInfo'; +export type { TaskStatusResponse } from './models/TaskStatusResponse'; export type { TinfoilFeedFileSchema } from './models/TinfoilFeedFileSchema'; export type { TinfoilFeedSchema } from './models/TinfoilFeedSchema'; export type { TinfoilFeedTitleDBSchema } from './models/TinfoilFeedTitleDBSchema'; diff --git a/frontend/src/__generated__/models/MessageResponse.ts b/frontend/src/__generated__/models/BulkOperationResponse.ts similarity index 53% rename from frontend/src/__generated__/models/MessageResponse.ts rename to frontend/src/__generated__/models/BulkOperationResponse.ts index 2408ce843..e73bb47db 100644 --- a/frontend/src/__generated__/models/MessageResponse.ts +++ b/frontend/src/__generated__/models/BulkOperationResponse.ts @@ -2,7 +2,9 @@ /* istanbul ignore file */ /* tslint:disable */ /* eslint-disable */ -export type MessageResponse = { - msg: string; +export type BulkOperationResponse = { + successful_items: number; + failed_items: number; + errors: Array; }; diff --git a/frontend/src/__generated__/models/JobStatus.ts b/frontend/src/__generated__/models/JobStatus.ts new file mode 100644 index 000000000..7a4998322 --- /dev/null +++ b/frontend/src/__generated__/models/JobStatus.ts @@ -0,0 +1,8 @@ +/* generated using openapi-typescript-codegen -- do not edit */ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +/** + * The Status of Job within its lifecycle at any given time. + */ +export type JobStatus = 'queued' | 'finished' | 'failed' | 'started' | 'deferred' | 'scheduled' | 'stopped' | 'canceled'; diff --git a/frontend/src/__generated__/models/TaskExecutionResponse.ts b/frontend/src/__generated__/models/TaskExecutionResponse.ts new file mode 100644 index 000000000..66cc3d692 --- /dev/null +++ b/frontend/src/__generated__/models/TaskExecutionResponse.ts @@ -0,0 +1,12 @@ +/* generated using openapi-typescript-codegen -- do not edit */ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +import type { JobStatus } from './JobStatus'; +export type TaskExecutionResponse = { + task_name: string; + task_id: string; + status: (JobStatus | null); + queued_at: string; +}; + diff --git a/frontend/src/__generated__/models/TaskStatusResponse.ts b/frontend/src/__generated__/models/TaskStatusResponse.ts new file mode 100644 index 000000000..65b214761 --- /dev/null +++ b/frontend/src/__generated__/models/TaskStatusResponse.ts @@ -0,0 +1,14 @@ +/* generated using openapi-typescript-codegen -- do not edit */ +/* istanbul ignore file */ +/* tslint:disable */ +/* eslint-disable */ +import type { JobStatus } from './JobStatus'; +export type TaskStatusResponse = { + task_name: string; + task_id: string; + status: (JobStatus | null); + queued_at: string; + started_at: (string | null); + ended_at: (string | null); +}; + diff --git a/frontend/src/components/Settings/Administration/TaskOption.vue b/frontend/src/components/Settings/Administration/TaskOption.vue index 42ebc040c..e5fdfc897 100644 --- a/frontend/src/components/Settings/Administration/TaskOption.vue +++ b/frontend/src/components/Settings/Administration/TaskOption.vue @@ -1,7 +1,7 @@