From 9a4c608b2ee8cc1090aa4eca40a24095984b492e Mon Sep 17 00:00:00 2001 From: Arash Date: Mon, 3 Feb 2025 14:55:53 +0100 Subject: [PATCH] Refactor error handling in CredentialsService to raise ObjectNotFound for undefined services and enhance test coverage for invalid credential scenarios --- .../webapps/galaxy/services/credentials.py | 2 +- test/integration/test_credentials.py | 25 ++++++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/webapps/galaxy/services/credentials.py b/lib/galaxy/webapps/galaxy/services/credentials.py index 3ed75fa58e0a..7e6623ef2546 100644 --- 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 index 35cd6f34a796..e86ddd44fecb 100644 --- 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)