fix delete_job_endpoint and get_jobs_job_id with wrong_id

Author: alinbutu

services/staging/rs_server_staging/main.py

@@ -286,10 +286,20 @@ async def execute_process(req: Request, resource: str, data: ProcessMetadataMode
 @router.get("/jobs/{job_id}")
 async def get_job_status_endpoint(job_id: str = Path(..., title="The ID of the job")):
     """Used to get status of processing job."""
-    job = app.extra["process_manager"].get_job(job_id)
-    if job:
-        return job
-    raise HTTPException(status_code=HTTP_404_NOT_FOUND, detail=f"Job with ID {job_id} not found")
+    try:
+        return app.extra["process_manager"].get_job(job_id)
+    except JobNotFoundError:
+        # Handle case when job_id is not found
+        return JSONResponse(
+            status_code=HTTP_404_NOT_FOUND,
+            content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
+        )
+    except Exception as e:  # pylint: disable=broad-exception-caught
+        # Catch any other unexpected exceptions
+        return JSONResponse(
+            status_code=HTTP_500_INTERNAL_SERVER_ERROR,
+            content={"title": "Internal Server Error", "detail": str(e)},
+        )
 
 
 @router.get("/jobs")
@@ -305,10 +315,21 @@ async def get_jobs_endpoint():
 @router.delete("/jobs/{job_id}")
 async def delete_job_endpoint(job_id: str = Path(..., title="The ID of the job to delete")):
     """Deletes a specific job from the database."""
-    success = app.extra["process_manager"].delete_job(job_id)
-    if success:
+    try:
+        app.extra["process_manager"].delete_job(job_id)
         return {"message": f"Job {job_id} deleted successfully"}
-    raise HTTPException(status_code=HTTP_404_NOT_FOUND, detail=f"Job with ID {job_id} not found")
+    except JobNotFoundError:
+        # Handle case when job_id is not found
+        return JSONResponse(
+            status_code=HTTP_404_NOT_FOUND,
+            content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
+        )
+    except Exception as e:  # pylint: disable=broad-exception-caught
+        # Catch any other unexpected exceptions
+        return JSONResponse(
+            status_code=HTTP_500_INTERNAL_SERVER_ERROR,
+            content={"title": "Internal Server Error", "detail": str(e)},
+        )
 
 
 @router.get("/jobs/{job_id}/results")
@@ -322,7 +343,7 @@ async def get_specific_job_result_endpoint(job_id: str = Path(..., title="The ID
         # Handle case when job_id is not found
         return JSONResponse(
             status_code=HTTP_404_NOT_FOUND,
-            content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found."},
+            content={"title": "No Such Job", "detail": f"Job with ID {job_id} not found"},
         )
     except Exception as e:  # pylint: disable=broad-exception-caught
         # Catch any other unexpected exceptions

services/staging/tests/test_staging.py

@@ -226,7 +226,7 @@ async def test_get_jobs_endpoint(mocker, set_db_env_var, staging_client):  # pyl
         (
             {"identifier": "non_existing_id"},
             HTTP_404_NOT_FOUND,
-            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found."},
+            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
         ),
         (
             {"identifier": "trigger_500"},
@@ -266,14 +266,14 @@ async def test_get_job(
     # Mock app.extra to ensure 'db_table' exists
     mock_db_table = mocker.MagicMock()
 
+    # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
     if expected_status == HTTP_404_NOT_FOUND:
-        # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
         mock_db_table.get_job.side_effect = JobNotFoundError
+    # Simulate an unexpected exception (HTTP 500)
     elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
-        # Simulate an unexpected exception (HTTP 500)
         mock_db_table.get_job.side_effect = Exception("Unexpected error occurred")
+    # Return an existing job normally (HTTP 200)
     else:
-        # Return an existing job normally (HTTP 200)
         mock_db_table.get_job.return_value = next(
             job for job in mock_jobs if job["identifier"] == expected_job["identifier"]
         )
@@ -296,7 +296,7 @@ async def test_get_job(
         (
             {"identifier": "non_existing_id"},
             HTTP_404_NOT_FOUND,
-            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found."},
+            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
         ),
         (
             {"identifier": "trigger_500"},
@@ -336,14 +336,14 @@ async def test_get_job_result(
     # Mock app.extra to ensure 'db_table' exists
     mock_db_table = mocker.MagicMock()
 
+    # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
     if expected_status == HTTP_404_NOT_FOUND:
-        # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
         mock_db_table.get_job.side_effect = JobNotFoundError
+    # Simulate an unexpected exception (HTTP 500)
     elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
-        # Simulate an unexpected exception (HTTP 500)
         mock_db_table.get_job.side_effect = Exception("Unexpected error occurred")
+    # Return an existing job normally (HTTP 200)
     else:
-        # Return an existing job normally (HTTP 200)
         mock_db_table.get_job.return_value = next(
             job for job in mock_jobs if job["identifier"] == expected_job["identifier"]
         )
@@ -367,7 +367,7 @@ async def test_get_job_result(
         (
             {"identifier": "non_existing_id"},
             HTTP_404_NOT_FOUND,
-            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found."},
+            {"title": "No Such Job", "detail": "Job with ID non_existing_id not found"},
         ),
         (
             {"identifier": "trigger_500"},
@@ -399,25 +399,29 @@ async def test_delete_job_endpoint(
     Args:
         mocker: A mocker object used to create mocks and patches for testing.
         staging_client: A test client for making requests to the FastAPI application.
+        mock_jobs: Fixture used to mock output of tiny db jobs
         expected_job (dict): The expected job dictionary containing job_id,
             status, progress, and message for the job to be deleted.
+        expected_status: response HTTP status code
+        expected_response: response body (JSON object)
 
     Assertions:
         - Asserts that the response status code is 200 if the job is successfully deleted.
         - Asserts that the response status code is 404 if the job does not exist.
+        - Asserts that the response status code is 500 if other exception occurs.
     """
     # Mock app.extra to ensure 'db_table' exists
     mock_db_table = mocker.MagicMock()
 
+    # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
     if expected_status == HTTP_404_NOT_FOUND:
-        # Simulate JobNotFoundError for non-existing jobs (HTTP 404)
-        mock_db_table.get_job.side_effect = JobNotFoundError
+        mock_db_table.delete_job.side_effect = JobNotFoundError
+    # Simulate an unexpected exception (HTTP 500)
     elif expected_status == HTTP_500_INTERNAL_SERVER_ERROR:
-        # Simulate an unexpected exception (HTTP 500)
-        mock_db_table.get_job.side_effect = Exception("Unexpected error occurred")
+        mock_db_table.delete_job.side_effect = Exception("Unexpected error occurred")
+    # Return an existing job normally (HTTP 200)
     else:
-        # Return an existing job normally (HTTP 200)
-        mock_db_table.get_job.return_value = next(
+        mock_db_table.delete_job.return_value = next(
             job for job in mock_jobs if job["identifier"] == expected_job["identifier"]
         )