Refactor error handling in CredentialsService to raise ObjectNotFound…

… for undefined services and enhance test coverage for invalid credential scenarios
arash77 · Feb 11, 2025 · 9a4c608 · 9a4c608
1 parent 243f9db
commit 9a4c608
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 16 deletions.
diff --git a/lib/galaxy/webapps/galaxy/services/credentials.py b/lib/galaxy/webapps/galaxy/services/credentials.py
@@ -209,7 +209,7 @@ def _create_or_update_credentials(
                 user, source_id, source_version, service_name, service_version
             )
             if source_credentials is None:
-                raise RequestParameterInvalidException(
+                raise ObjectNotFound(
                     f"Service '{service_name}' with version '{service_version}' is not defined"
                     f"in {source_type} with id {source_id} and version {source_version}."
                 )

diff --git a/test/integration/test_credentials.py b/test/integration/test_credentials.py
@@ -110,21 +110,9 @@ def test_delete_credentials_group(self):
 
     @skip_without_tool(CREDENTIALS_TEST_TOOL)
     def test_provide_credential_invalid_group(self):
-        payload = {
-            "source_type": "tool",
-            "source_id": CREDENTIALS_TEST_TOOL,
-            "source_version": "test",
-            "credentials": [
-                {
-                    "name": "service1",
-                    "version": "1",
-                    "current_group": "invalid_group_name",
-                    "groups": [{"name": "default", "variables": [], "secrets": []}],
-                }
-            ],
-        }
-        response = self._post("/api/users/current/credentials", data=payload, json=True)
-        self._assert_status_code_is(response, 400)
+        payload = self._build_credentials_payload()
+        payload["credentials"][0]["current_group"] = "invalid_group_name"
+        self._provide_user_credentials(payload, status_code=400)
 
     def test_invalid_source_type(self):
         payload = self._build_credentials_payload(source_type="invalid_source_type")
@@ -148,6 +136,13 @@ def test_not_existing_service_version(self):
         payload = self._build_credentials_payload(service_version="nonexistent_service_version")
         self._provide_user_credentials(payload, status_code=404)
 
+    @skip_without_tool(CREDENTIALS_TEST_TOOL)
+    def test_invalid_credential_name(self):
+        for key in ["variables", "secrets"]:
+            payload = self._build_credentials_payload()
+            payload["credentials"][0]["groups"][0][key][0]["name"] = "invalid_name"
+            self._provide_user_credentials(payload, status_code=400)
+
     def test_delete_nonexistent_service_credentials(self):
         response = self._delete("/api/users/current/credentials/f2db41e1fa331b3e")
         self._assert_status_code_is(response, 400)