refactor(robot-server): RTP handling improvements (#15919)

# Overview

This PR addresses some refactor suggestions made in previous PRs

## Changelog

- check if data file directory exists before deleting it
- refactored `FileInUseError`'s message formatting
- small code improvement in `completed_analysis_store` 

## Risk assessment

None.

robot-server/robot_server/data_files/data_files_store.py

@@ -145,26 +145,17 @@ def remove(self, file_id: str) -> None:
                 transaction.execute(select_ids_used_in_runs).scalars().all()
             )
             if len(files_used_in_analyses) + len(files_used_in_runs) > 0:
-                analysis_usage_text = (
-                    f" analyses: {files_used_in_analyses}"
-                    if len(files_used_in_analyses) > 0
-                    else None
-                )
-                runs_usage_text = (
-                    f" runs: {files_used_in_runs}"
-                    if len(files_used_in_runs) > 0
-                    else None
-                )
-                conjunction = " and " if analysis_usage_text and runs_usage_text else ""
+
                 raise FileInUseError(
                     data_file_id=file_id,
-                    message=f"Cannot remove file {file_id} as it is being used in"
-                    f" existing{analysis_usage_text or ''}{conjunction}{runs_usage_text or ''}.",
+                    ids_used_in_runs=files_used_in_runs,
+                    ids_used_in_analyses=files_used_in_analyses,
                 )
-            transaction.execute(delete_statement)
-
+            result = transaction.execute(delete_statement)
+        if result.rowcount < 1:
+            raise FileIdNotFoundError(file_id)
         file_dir = self._data_files_directory.joinpath(file_id)
-        if file_dir:
+        if file_dir.exists():
             for file in file_dir.glob("*"):
                 file.unlink()
             file_dir.rmdir()

robot-server/robot_server/data_files/models.py

@@ -1,5 +1,6 @@
 """Data files models."""
 from datetime import datetime
+from typing import Set
 
 from pydantic import Field
 
@@ -30,7 +31,25 @@ def __init__(self, data_file_id: str) -> None:
 class FileInUseError(GeneralError):
     """Error raised when a file being removed is in use."""
 
-    def __init__(self, data_file_id: str, message: str) -> None:
+    def __init__(
+        self,
+        data_file_id: str,
+        ids_used_in_runs: Set[str],
+        ids_used_in_analyses: Set[str],
+    ) -> None:
+        analysis_usage_text = (
+            f" analyses: {ids_used_in_analyses}"
+            if len(ids_used_in_analyses) > 0
+            else None
+        )
+        runs_usage_text = (
+            f" runs: {ids_used_in_runs}" if len(ids_used_in_runs) > 0 else None
+        )
+        conjunction = " and " if analysis_usage_text and runs_usage_text else ""
+        message = (
+            f"Cannot remove file {data_file_id} as it is being used in"
+            f" existing{analysis_usage_text or ''}{conjunction}{runs_usage_text or ''}."
+        )
         super().__init__(
             message=message,
             detail={"dataFileId": data_file_id},

robot-server/robot_server/protocols/completed_analysis_store.py

@@ -280,10 +280,13 @@ def get_primitive_rtps_by_analysis_id(
         with self._sql_engine.begin() as transaction:
             results = transaction.execute(statement).all()
 
-        rtps: Dict[str, PrimitiveAllowedTypes] = {}
-        for row in results:
-            param = PrimitiveParameterResource.from_sql_row(row)
-            rtps.update({param.parameter_variable_name: param.parameter_value})
+        param_resources = [
+            PrimitiveParameterResource.from_sql_row(row) for row in results
+        ]
+        rtps = {
+            param.parameter_variable_name: param.parameter_value
+            for param in param_resources
+        }
         return rtps
 
     def get_csv_rtps_by_analysis_id(

robot-server/tests/data_files/test_data_files_store.py

@@ -265,3 +265,9 @@ async def test_remove_raises_in_file_in_use(
     expected_error_message = "Cannot remove file file-id as it is being used in existing analyses: {'analysis-id'}."
     with pytest.raises(FileInUseError, match=expected_error_message):
         subject.remove(file_id="file-id")
+
+
+def test_remove_raise_for_nonexistent_id(subject: DataFilesStore) -> None:
+    """It should raise FileIdNotFound error."""
+    with pytest.raises(FileIdNotFoundError, match="Data file file-id was not found."):
+        subject.remove(file_id="file-id")