From 8a8dbd9aa1c0be21cffae6a405843d906d2123f9 Mon Sep 17 00:00:00 2001 From: Eddy Zhang Date: Mon, 31 Aug 2026 22:53:13 +1000 Subject: [PATCH 1/4] refactor(console): dep-inject model provider payloads with @model_validate Moves the eight inline parses in console/workspace/model_providers.py onto the @model_validate decorator. Six read the JSON body; the two get handlers already read request.args.to_dict(flat=True), which is what the decorator's GET branch does, so they convert the same way AccountAvatarApi.get and EducationAutoCompleteApi.get did. request is no longer used in the module, so the flask import drops it. The existing tests unwrap the view and pass the model themselves, which leaves the decorators unexercised, so this also adds one bound-method test asserting the 422 the decorator raises for an invalid query. Removing any decorator now fails it. --- .../console/workspace/model_providers.py | 43 ++++------ .../console/workspace/test_model_providers.py | 82 +++++++++++++++---- 2 files changed, 82 insertions(+), 43 deletions(-) diff --git a/api/controllers/console/workspace/model_providers.py b/api/controllers/console/workspace/model_providers.py index ca0b536f85ed50..069c95c72cc3ea 100644 --- a/api/controllers/console/workspace/model_providers.py +++ b/api/controllers/console/workspace/model_providers.py @@ -1,7 +1,7 @@ import io from typing import Any, Literal -from flask import request, send_file +from flask import send_file from flask_restx import Resource from pydantic import BaseModel, Field, field_validator from sqlalchemy.orm import Session @@ -15,6 +15,7 @@ RBACResourceScope, account_initialization_required, is_admin_or_owner_required, + model_validate, rbac_permission_required, setup_required, with_current_tenant_id, @@ -154,9 +155,8 @@ class ModelProviderListApi(Resource): @login_required @account_initialization_required @with_current_tenant_id - def get(self, tenant_id: str): - payload = request.args.to_dict(flat=True) - args = ParserModelList.model_validate(payload) + @model_validate(ParserModelList) + def get(self, args: ParserModelList, tenant_id: str): model_provider_service = ModelProviderService() provider_list = model_provider_service.get_provider_list(tenant_id=tenant_id, model_type=args.model_type) @@ -212,10 +212,9 @@ class ModelProviderCredentialApi(Resource): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_MANAGE, resource_required=False) @account_initialization_required @with_current_tenant_id - def get(self, tenant_id: str, provider: str): + @model_validate(ParserCredentialId) + def get(self, args: ParserCredentialId, tenant_id: str, provider: str): # if credential_id is not provided, return current used credential - payload = request.args.to_dict(flat=True) - args = ParserCredentialId.model_validate(payload) model_provider_service = ModelProviderService() credentials = model_provider_service.get_provider_credential( @@ -232,9 +231,8 @@ def get(self, tenant_id: str, provider: str): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_CREATE, resource_required=False) @account_initialization_required @with_current_tenant_id - def post(self, current_tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserCredentialCreate.model_validate(payload) + @model_validate(ParserCredentialCreate) + def post(self, args: ParserCredentialCreate, current_tenant_id: str, provider: str): model_provider_service = ModelProviderService() @@ -258,9 +256,8 @@ def post(self, current_tenant_id: str, provider: str): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_MANAGE, resource_required=False) @account_initialization_required @with_current_tenant_id - def put(self, current_tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserCredentialUpdate.model_validate(payload) + @model_validate(ParserCredentialUpdate) + def put(self, args: ParserCredentialUpdate, current_tenant_id: str, provider: str): model_provider_service = ModelProviderService() @@ -285,9 +282,8 @@ def put(self, current_tenant_id: str, provider: str): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_MANAGE, resource_required=False) @account_initialization_required @with_current_tenant_id - def delete(self, current_tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserCredentialDelete.model_validate(payload) + @model_validate(ParserCredentialDelete) + def delete(self, args: ParserCredentialDelete, current_tenant_id: str, provider: str): model_provider_service = ModelProviderService() model_provider_service.remove_provider_credential( @@ -307,9 +303,8 @@ class ModelProviderCredentialSwitchApi(Resource): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_USE, resource_required=False) @account_initialization_required @with_current_tenant_id - def post(self, current_tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserCredentialSwitch.model_validate(payload) + @model_validate(ParserCredentialSwitch) + def post(self, args: ParserCredentialSwitch, current_tenant_id: str, provider: str): service = ModelProviderService() service.switch_active_provider_credential( @@ -332,9 +327,8 @@ class ModelProviderValidateApi(Resource): @login_required @account_initialization_required @with_current_tenant_id - def post(self, current_tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserCredentialValidate.model_validate(payload) + @model_validate(ParserCredentialValidate) + def post(self, args: ParserCredentialValidate, current_tenant_id: str, provider: str): tenant_id = current_tenant_id @@ -388,9 +382,8 @@ class PreferredProviderTypeUpdateApi(Resource): @rbac_permission_required(RBACResourceScope.WORKSPACE, RBACPermission.CREDENTIAL_USE, resource_required=False) @account_initialization_required @with_current_tenant_id - def post(self, tenant_id: str, provider: str): - payload = console_ns.payload or {} - args = ParserPreferredProviderType.model_validate(payload) + @model_validate(ParserPreferredProviderType) + def post(self, args: ParserPreferredProviderType, tenant_id: str, provider: str): model_provider_service = ModelProviderService() model_provider_service.switch_preferred_provider( diff --git a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py index a4f14e5255ddcb..334ef0039bc15a 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py @@ -2,10 +2,10 @@ from unittest.mock import patch import pytest -from flask import Flask, g +from flask import Flask, g, request from pydantic_core import ValidationError from sqlalchemy.orm import Session -from werkzeug.exceptions import Forbidden +from werkzeug.exceptions import Forbidden, UnprocessableEntity from configs import dify_config from controllers.console.workspace.model_providers import ( @@ -17,6 +17,14 @@ ModelProviderPaymentCheckoutUrlApi, ModelProviderSummaryListApi, ModelProviderValidateApi, + ParserCredentialCreate, + ParserCredentialDelete, + ParserCredentialId, + ParserCredentialSwitch, + ParserCredentialUpdate, + ParserCredentialValidate, + ParserModelList, + ParserPreferredProviderType, PreferredProviderTypeUpdateApi, ) from core.entities.provider_entities import CredentialConfiguration @@ -116,6 +124,15 @@ def expected_provider_payload() -> dict[str, object]: } +def _payload() -> dict: + """Mirror the source the ``@model_validate`` decorator reads for the request in scope. + + The tests build their contexts with ``test_request_context``, which defaults to GET even for + handlers mounted on POST, so fall back to the JSON body whenever the query string is empty. + """ + return request.args.to_dict(flat=True) or (request.get_json(silent=True) or {}) + + class TestModelProviderListApi: def test_get_success(self, app: Flask): api = ModelProviderListApi() @@ -129,7 +146,7 @@ def test_get_success(self, app: Flask): return_value=[provider], ) as get_provider_list, ): - result = method(api, "tenant1") + result = method(api, ParserModelList.model_validate(_payload()), "tenant1") get_provider_list.assert_called_once_with(tenant_id="tenant1", model_type=ModelType.LLM) assert result == {"data": [expected_provider_payload()]} @@ -145,7 +162,7 @@ def test_get_without_model_type_passes_none(self, app: Flask): return_value=[], ) as get_provider_list, ): - result = method(api, "tenant1") + result = method(api, ParserModelList.model_validate(_payload()), "tenant1") get_provider_list.assert_called_once_with(tenant_id="tenant1", model_type=None) assert result == {"data": []} @@ -297,7 +314,7 @@ def test_get_success(self, app: Flask): }, ) as get_provider_credential, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialId.model_validate(_payload()), "tenant1", provider="openai") get_provider_credential.assert_called_once_with( tenant_id="tenant1", provider="openai", credential_id=VALID_UUID @@ -321,7 +338,7 @@ def test_get_current_credential_without_id(self, app: Flask): return_value=None, ) as get_provider_credential, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialId.model_validate(_payload()), "tenant1", provider="openai") get_provider_credential.assert_called_once_with(tenant_id="tenant1", provider="openai", credential_id=None) assert result == {"credentials": None} @@ -332,7 +349,7 @@ def test_get_invalid_uuid(self, app: Flask): with app.test_request_context(f"/?credential_id={INVALID_UUID}"): with pytest.raises(ValidationError): - method(api, "tenant1", provider="openai") + method(api, ParserCredentialId.model_validate(_payload()), "tenant1", provider="openai") def test_post_create_success(self, app: Flask): api = ModelProviderCredentialApi() @@ -347,7 +364,9 @@ def test_post_create_success(self, app: Flask): return_value=None, ) as create_provider_credential, ): - result, status = method(api, "tenant1", provider="openai") + result, status = method( + api, ParserCredentialCreate.model_validate(_payload()), "tenant1", provider="openai" + ) create_provider_credential.assert_called_once_with( tenant_id="tenant1", @@ -372,7 +391,7 @@ def test_post_create_validation_error(self, app: Flask): ), ): with pytest.raises(ValueError): - method(api, "tenant1", provider="openai") + method(api, ParserCredentialCreate.model_validate(_payload()), "tenant1", provider="openai") def test_put_update_success(self, app: Flask): api = ModelProviderCredentialApi() @@ -387,7 +406,7 @@ def test_put_update_success(self, app: Flask): return_value=None, ) as update_provider_credential, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialUpdate.model_validate(_payload()), "tenant1", provider="openai") update_provider_credential.assert_called_once_with( tenant_id="tenant1", @@ -406,7 +425,7 @@ def test_put_invalid_uuid(self, app: Flask): with app.test_request_context("/", json=payload): with pytest.raises(ValidationError): - method(api, "tenant1", provider="openai") + method(api, ParserCredentialUpdate.model_validate(_payload()), "tenant1", provider="openai") def test_delete_success(self, app: Flask): api = ModelProviderCredentialApi() @@ -421,7 +440,9 @@ def test_delete_success(self, app: Flask): return_value=None, ) as remove_provider_credential, ): - result, status = method(api, "tenant1", provider="openai") + result, status = method( + api, ParserCredentialDelete.model_validate(_payload()), "tenant1", provider="openai" + ) remove_provider_credential.assert_called_once_with( tenant_id="tenant1", provider="openai", credential_id=VALID_UUID @@ -444,7 +465,7 @@ def test_switch_success(self, app: Flask): return_value=None, ) as switch_active_provider_credential, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialSwitch.model_validate(_payload()), "tenant1", provider="openai") switch_active_provider_credential.assert_called_once_with( tenant_id="tenant1", @@ -461,7 +482,7 @@ def test_switch_invalid_uuid(self, app: Flask): with app.test_request_context("/", json=payload): with pytest.raises(ValidationError): - method(api, "tenant1", provider="openai") + method(api, ParserCredentialSwitch.model_validate(_payload()), "tenant1", provider="openai") class TestModelProviderValidateApi: @@ -478,7 +499,7 @@ def test_validate_success(self, app: Flask): return_value=None, ) as validate_provider_credentials, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialValidate.model_validate(_payload()), "tenant1", provider="openai") validate_provider_credentials.assert_called_once_with( tenant_id="tenant1", provider="openai", credentials={"a": "b"} @@ -498,7 +519,7 @@ def test_validate_failure(self, app: Flask): side_effect=CredentialsValidateFailedError("bad"), ), ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserCredentialValidate.model_validate(_payload()), "tenant1", provider="openai") assert result == {"result": "error", "error": "bad"} @@ -549,7 +570,7 @@ def test_update_success(self, app: Flask): return_value=None, ) as switch_preferred_provider, ): - result = method(api, "tenant1", provider="openai") + result = method(api, ParserPreferredProviderType.model_validate(_payload()), "tenant1", provider="openai") switch_preferred_provider.assert_called_once_with( tenant_id="tenant1", provider="openai", preferred_provider_type="custom" @@ -564,7 +585,7 @@ def test_invalid_enum(self, app: Flask): with app.test_request_context("/", json=payload): with pytest.raises(ValidationError): - method(api, "tenant1", provider="openai") + method(api, ParserPreferredProviderType.model_validate(_payload()), "tenant1", provider="openai") class TestModelProviderPaymentCheckoutUrlApi: @@ -618,3 +639,28 @@ def test_checkout_rejects_non_privileged_role(self, app: Flask): api.get(provider="anthropic") get_model_provider_payment_link.assert_not_called() + + +class TestModelValidateDecorator: + def test_invalid_query_is_rejected_before_the_handler_runs(self, app: Flask): + """The tests above unwrap the view, so this is what covers the decorator itself.""" + api = ModelProviderListApi() + + with ( + app.test_request_context("/?model_type=not-a-model-type"), + config_overrides_context(LOGIN_DISABLED=True, RBAC_ENABLED=False), + patch("controllers.console.wraps._is_setup_completed", return_value=True), + patch( + "controllers.console.wraps.current_account_with_tenant", + return_value=(make_account(), "tenant1"), + ), + patch( + "controllers.console.workspace.model_providers.ModelProviderService.get_provider_list", + ) as get_provider_list, + ): + g._login_user = make_account() + with pytest.raises(UnprocessableEntity) as exc_info: + api.get() + + assert exc_info.value.code == 422 + get_provider_list.assert_not_called() From 69ebc86325a17d1b06edc5775c506ec88846d8f1 Mon Sep 17 00:00:00 2001 From: Eddy Zhang Date: Tue, 1 Sep 2026 01:25:17 +1000 Subject: [PATCH 2/4] test(console): cover every added decorator, not just one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The decorator test only exercised ModelProviderListApi; dropping any of the other seven decorators left the suite green. It is now parametrized over all eight handlers, covering both sources the decorator reads — query args for the two get handlers, the JSON body for the rest — and each of the eight is individually detected when its decorator is removed. Also drops the blank lines the parse statements left behind, and reattaches the credential comment to the call it describes. --- .../console/workspace/model_providers.py | 10 +---- .../console/workspace/test_model_providers.py | 41 ++++++++++++++----- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/api/controllers/console/workspace/model_providers.py b/api/controllers/console/workspace/model_providers.py index 069c95c72cc3ea..fe844204d276bd 100644 --- a/api/controllers/console/workspace/model_providers.py +++ b/api/controllers/console/workspace/model_providers.py @@ -157,7 +157,6 @@ class ModelProviderListApi(Resource): @with_current_tenant_id @model_validate(ParserModelList) def get(self, args: ParserModelList, tenant_id: str): - model_provider_service = ModelProviderService() provider_list = model_provider_service.get_provider_list(tenant_id=tenant_id, model_type=args.model_type) @@ -214,9 +213,8 @@ class ModelProviderCredentialApi(Resource): @with_current_tenant_id @model_validate(ParserCredentialId) def get(self, args: ParserCredentialId, tenant_id: str, provider: str): - # if credential_id is not provided, return current used credential - model_provider_service = ModelProviderService() + # if credential_id is not provided, return current used credential credentials = model_provider_service.get_provider_credential( tenant_id=tenant_id, provider=provider, credential_id=args.credential_id ) @@ -233,7 +231,6 @@ def get(self, args: ParserCredentialId, tenant_id: str, provider: str): @with_current_tenant_id @model_validate(ParserCredentialCreate) def post(self, args: ParserCredentialCreate, current_tenant_id: str, provider: str): - model_provider_service = ModelProviderService() try: @@ -258,7 +255,6 @@ def post(self, args: ParserCredentialCreate, current_tenant_id: str, provider: s @with_current_tenant_id @model_validate(ParserCredentialUpdate) def put(self, args: ParserCredentialUpdate, current_tenant_id: str, provider: str): - model_provider_service = ModelProviderService() try: @@ -284,7 +280,6 @@ def put(self, args: ParserCredentialUpdate, current_tenant_id: str, provider: st @with_current_tenant_id @model_validate(ParserCredentialDelete) def delete(self, args: ParserCredentialDelete, current_tenant_id: str, provider: str): - model_provider_service = ModelProviderService() model_provider_service.remove_provider_credential( tenant_id=current_tenant_id, provider=provider, credential_id=args.credential_id @@ -305,7 +300,6 @@ class ModelProviderCredentialSwitchApi(Resource): @with_current_tenant_id @model_validate(ParserCredentialSwitch) def post(self, args: ParserCredentialSwitch, current_tenant_id: str, provider: str): - service = ModelProviderService() service.switch_active_provider_credential( tenant_id=current_tenant_id, @@ -329,7 +323,6 @@ class ModelProviderValidateApi(Resource): @with_current_tenant_id @model_validate(ParserCredentialValidate) def post(self, args: ParserCredentialValidate, current_tenant_id: str, provider: str): - tenant_id = current_tenant_id model_provider_service = ModelProviderService() @@ -384,7 +377,6 @@ class PreferredProviderTypeUpdateApi(Resource): @with_current_tenant_id @model_validate(ParserPreferredProviderType) def post(self, args: ParserPreferredProviderType, tenant_id: str, provider: str): - model_provider_service = ModelProviderService() model_provider_service.switch_preferred_provider( tenant_id=tenant_id, provider=provider, preferred_provider_type=args.preferred_provider_type diff --git a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py index 334ef0039bc15a..48137524cee142 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py @@ -1,4 +1,5 @@ from inspect import unwrap +from types import SimpleNamespace from unittest.mock import patch import pytest @@ -34,6 +35,7 @@ from graphon.model_runtime.entities.provider_entities import ConfigurateMethod from graphon.model_runtime.errors.validate import CredentialsValidateFailedError from models import Account +from models.account import TenantAccountRole from models.provider import ProviderType from services.entities.model_provider_entities import ( CustomConfigurationResponse, @@ -642,25 +644,42 @@ def test_checkout_rejects_non_privileged_role(self, app: Flask): class TestModelValidateDecorator: - def test_invalid_query_is_rejected_before_the_handler_runs(self, app: Flask): - """The tests above unwrap the view, so this is what covers the decorator itself.""" - api = ModelProviderListApi() + """The tests above unwrap the view, so this is what covers the decorators themselves.""" + + @pytest.mark.parametrize( + ("api_cls", "verb", "url", "body", "kwargs"), + [ + (ModelProviderListApi, "GET", "/?model_type=not-a-model-type", None, {}), + (ModelProviderCredentialApi, "GET", "/?credential_id=not-a-uuid", None, {"provider": "openai"}), + (ModelProviderCredentialApi, "POST", "/", {}, {"provider": "openai"}), + (ModelProviderCredentialApi, "PUT", "/", {}, {"provider": "openai"}), + (ModelProviderCredentialApi, "DELETE", "/", {}, {"provider": "openai"}), + (ModelProviderCredentialSwitchApi, "POST", "/", {}, {"provider": "openai"}), + (ModelProviderValidateApi, "POST", "/", {}, {"provider": "openai"}), + (PreferredProviderTypeUpdateApi, "POST", "/", {}, {"provider": "openai"}), + ], + ) + def test_invalid_input_is_rejected_before_the_handler_runs( + self, app: Flask, api_cls, verb: str, url: str, body, kwargs + ): + api = api_cls() + account = make_account() + account.role = TenantAccountRole.OWNER with ( - app.test_request_context("/?model_type=not-a-model-type"), + app.test_request_context(url, method=verb, json=body), config_overrides_context(LOGIN_DISABLED=True, RBAC_ENABLED=False), patch("controllers.console.wraps._is_setup_completed", return_value=True), patch( "controllers.console.wraps.current_account_with_tenant", - return_value=(make_account(), "tenant1"), + return_value=(account, "tenant1"), ), - patch( - "controllers.console.workspace.model_providers.ModelProviderService.get_provider_list", - ) as get_provider_list, + patch("libs.login.current_user", SimpleNamespace(_get_current_object=lambda: account)), + patch("controllers.console.workspace.model_providers.ModelProviderService") as service, ): - g._login_user = make_account() + g._login_user = account with pytest.raises(UnprocessableEntity) as exc_info: - api.get() + getattr(api, verb.lower())(**kwargs) assert exc_info.value.code == 422 - get_provider_list.assert_not_called() + service.assert_not_called() From 733c7bccf4499beff6e05abc531f636c39b308fd Mon Sep 17 00:00:00 2001 From: Eddy Zhang Date: Tue, 1 Sep 2026 01:54:52 +1000 Subject: [PATCH 3/4] test(console): annotate the parametrized decorator test CI's Python Style job runs pyrefly over tests/ with its own config, which the new parametrized test tripped: unannotated parameters, a bare dict return on the payload helper, and typing.Any (banned by ruff TID251). Annotated with object, leaving the file at its baseline diagnostic count. --- .../console/workspace/test_model_providers.py | 36 +++++++++++++------ 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py index 48137524cee142..8d9a2048c05284 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py @@ -4,6 +4,7 @@ import pytest from flask import Flask, g, request +from flask_restx import Resource from pydantic_core import ValidationError from sqlalchemy.orm import Session from werkzeug.exceptions import Forbidden, UnprocessableEntity @@ -126,13 +127,17 @@ def expected_provider_payload() -> dict[str, object]: } -def _payload() -> dict: +def _payload() -> dict[str, object]: """Mirror the source the ``@model_validate`` decorator reads for the request in scope. The tests build their contexts with ``test_request_context``, which defaults to GET even for handlers mounted on POST, so fall back to the JSON body whenever the query string is empty. """ - return request.args.to_dict(flat=True) or (request.get_json(silent=True) or {}) + args: dict[str, object] = dict(request.args.to_dict(flat=True)) + if args: + return args + body = request.get_json(silent=True) + return dict(body) if isinstance(body, dict) else {} class TestModelProviderListApi: @@ -646,22 +651,31 @@ def test_checkout_rejects_non_privileged_role(self, app: Flask): class TestModelValidateDecorator: """The tests above unwrap the view, so this is what covers the decorators themselves.""" + EMPTY_BODY: dict[str, object] = {} + EMPTY_KWARGS: dict[str, object] = {} + @pytest.mark.parametrize( ("api_cls", "verb", "url", "body", "kwargs"), [ - (ModelProviderListApi, "GET", "/?model_type=not-a-model-type", None, {}), + (ModelProviderListApi, "GET", "/?model_type=not-a-model-type", None, EMPTY_KWARGS), (ModelProviderCredentialApi, "GET", "/?credential_id=not-a-uuid", None, {"provider": "openai"}), - (ModelProviderCredentialApi, "POST", "/", {}, {"provider": "openai"}), - (ModelProviderCredentialApi, "PUT", "/", {}, {"provider": "openai"}), - (ModelProviderCredentialApi, "DELETE", "/", {}, {"provider": "openai"}), - (ModelProviderCredentialSwitchApi, "POST", "/", {}, {"provider": "openai"}), - (ModelProviderValidateApi, "POST", "/", {}, {"provider": "openai"}), - (PreferredProviderTypeUpdateApi, "POST", "/", {}, {"provider": "openai"}), + (ModelProviderCredentialApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), + (ModelProviderCredentialApi, "PUT", "/", EMPTY_BODY, {"provider": "openai"}), + (ModelProviderCredentialApi, "DELETE", "/", EMPTY_BODY, {"provider": "openai"}), + (ModelProviderCredentialSwitchApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), + (ModelProviderValidateApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), + (PreferredProviderTypeUpdateApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), ], ) def test_invalid_input_is_rejected_before_the_handler_runs( - self, app: Flask, api_cls, verb: str, url: str, body, kwargs - ): + self, + app: Flask, + api_cls: type[Resource], + verb: str, + url: str, + body: dict[str, object] | None, + kwargs: dict[str, object], + ) -> None: api = api_cls() account = make_account() account.role = TenantAccountRole.OWNER From 59d89c6e9e63dd2a9db3576dc3253f3abe9a26d3 Mon Sep 17 00:00:00 2001 From: Eddy Zhang Date: Tue, 1 Sep 2026 02:24:13 +1000 Subject: [PATCH 4/4] test(console): parametrize on the call itself, not a getattr scripts/check_no_new_getattr.py rejects net-new getattr() in changed code, and the parametrized decorator test dispatched through getattr(api, verb). Each row now carries the bound call directly. --- .../console/workspace/test_model_providers.py | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py index 8d9a2048c05284..2af0ce3db7abf1 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_model_providers.py @@ -1,10 +1,10 @@ +from collections.abc import Callable from inspect import unwrap from types import SimpleNamespace from unittest.mock import patch import pytest from flask import Flask, g, request -from flask_restx import Resource from pydantic_core import ValidationError from sqlalchemy.orm import Session from werkzeug.exceptions import Forbidden, UnprocessableEntity @@ -655,28 +655,31 @@ class TestModelValidateDecorator: EMPTY_KWARGS: dict[str, object] = {} @pytest.mark.parametrize( - ("api_cls", "verb", "url", "body", "kwargs"), + ("verb", "url", "body", "call"), [ - (ModelProviderListApi, "GET", "/?model_type=not-a-model-type", None, EMPTY_KWARGS), - (ModelProviderCredentialApi, "GET", "/?credential_id=not-a-uuid", None, {"provider": "openai"}), - (ModelProviderCredentialApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), - (ModelProviderCredentialApi, "PUT", "/", EMPTY_BODY, {"provider": "openai"}), - (ModelProviderCredentialApi, "DELETE", "/", EMPTY_BODY, {"provider": "openai"}), - (ModelProviderCredentialSwitchApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), - (ModelProviderValidateApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), - (PreferredProviderTypeUpdateApi, "POST", "/", EMPTY_BODY, {"provider": "openai"}), + ("GET", "/?model_type=not-a-model-type", None, lambda: ModelProviderListApi().get()), + ( + "GET", + "/?credential_id=not-a-uuid", + None, + lambda: ModelProviderCredentialApi().get(provider="openai"), + ), + ("POST", "/", EMPTY_BODY, lambda: ModelProviderCredentialApi().post(provider="openai")), + ("PUT", "/", EMPTY_BODY, lambda: ModelProviderCredentialApi().put(provider="openai")), + ("DELETE", "/", EMPTY_BODY, lambda: ModelProviderCredentialApi().delete(provider="openai")), + ("POST", "/", EMPTY_BODY, lambda: ModelProviderCredentialSwitchApi().post(provider="openai")), + ("POST", "/", EMPTY_BODY, lambda: ModelProviderValidateApi().post(provider="openai")), + ("POST", "/", EMPTY_BODY, lambda: PreferredProviderTypeUpdateApi().post(provider="openai")), ], ) def test_invalid_input_is_rejected_before_the_handler_runs( self, app: Flask, - api_cls: type[Resource], verb: str, url: str, body: dict[str, object] | None, - kwargs: dict[str, object], + call: Callable[[], object], ) -> None: - api = api_cls() account = make_account() account.role = TenantAccountRole.OWNER @@ -693,7 +696,7 @@ def test_invalid_input_is_rejected_before_the_handler_runs( ): g._login_user = account with pytest.raises(UnprocessableEntity) as exc_info: - getattr(api, verb.lower())(**kwargs) + call() assert exc_info.value.code == 422 service.assert_not_called()