diff --git a/api/controllers/console/agent/roster.py b/api/controllers/console/agent/roster.py index d4a72b75d92..377c40877e5 100644 --- a/api/controllers/console/agent/roster.py +++ b/api/controllers/console/agent/roster.py @@ -5,16 +5,21 @@ from flask_restx import Resource from pydantic import AliasChoices, BaseModel, Field, field_validator from sqlalchemy import func, select -from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models +from controllers.common.schema import ( + query_params_from_model, + query_params_from_request, + register_response_schema_models, + register_schema_models, +) from controllers.console import console_ns from controllers.console.agent.app_helpers import resolve_agent_app_model, resolve_agent_runtime_app_model from controllers.console.apikey import ApiKeyItem, ApiKeyList, BaseApiKeyListResource, BaseApiKeyResource from controllers.console.app.app import ( - AppDetailWithSite as GenericAppDetailWithSite, + APP_LIST_QUERY_ARRAY_FIELDS, + AppListQuery, ) from controllers.console.app.app import ( - AppListQuery, - _normalize_app_list_query_args, + AppDetailWithSite as GenericAppDetailWithSite, ) from controllers.console.app.app import ( AppPagination as GenericAppPagination, @@ -197,11 +202,9 @@ class AgentLogsQuery(BaseModel): def empty_list_values_to_list(cls, value: object) -> list[str]: if value in (None, ""): return [] - if isinstance(value, str): - return [value] if isinstance(value, list): - return [item for item in value if item] - return [] + return [str(item).strip() for item in value if str(item).strip()] + raise ValueError("Unsupported query list type.") @field_validator("sort_by") @classmethod @@ -506,16 +509,11 @@ def _parse_observability_time_range(start: str | None, end: str | None, account: abort(400, description=str(exc)) -def _multi_query_values(name: str, legacy_name: str | None = None) -> list[str]: - values: list[str] = [] - for query_name in (name, f"{name}[]"): - values.extend(request.args.getlist(query_name)) - if legacy_name: - values.extend(request.args.getlist(legacy_name)) - parsed: list[str] = [] - for value in values: - parsed.extend(item.strip() for item in value.split(",") if item.strip()) - return parsed +def _query_values(name: str, alias_name: str | None = None) -> list[str]: + values = request.args.getlist(name) + if alias_name: + values.extend(request.args.getlist(alias_name)) + return [value.strip() for value in values if value.strip()] @console_ns.route("/agent") @@ -528,7 +526,7 @@ class AgentAppListApi(Resource): @with_current_user @with_current_tenant_id def get(self, current_tenant_id: str, current_user: Account): - args = AppListQuery.model_validate(_normalize_app_list_query_args(request.args)) + args = query_params_from_request(AppListQuery, list_fields=APP_LIST_QUERY_ARRAY_FIELDS) params = AppListParams( page=args.page, limit=args.limit, @@ -893,8 +891,8 @@ class AgentLogsApi(Resource): def get(self, tenant_id: str, current_user: Account, agent_id: UUID): app_model = resolve_agent_runtime_app_model(tenant_id=tenant_id, agent_id=agent_id) query_data: dict[str, object] = dict(request.args.to_dict(flat=True)) - query_data["sources"] = _multi_query_values("sources", "source") - query_data["statuses"] = _multi_query_values("statuses", "status") + query_data["sources"] = _query_values("sources", "source") + query_data["statuses"] = _query_values("statuses", "status") query = AgentLogsQuery.model_validate(query_data) start, end = _parse_observability_time_range(query.start, query.end, current_user) try: @@ -930,8 +928,8 @@ class AgentLogMessagesApi(Resource): def get(self, tenant_id: str, current_user: Account, agent_id: UUID, conversation_id: UUID): app_model = resolve_agent_runtime_app_model(tenant_id=tenant_id, agent_id=agent_id) query_data: dict[str, object] = dict(request.args.to_dict(flat=True)) - query_data["sources"] = _multi_query_values("sources", "source") - query_data["statuses"] = _multi_query_values("statuses", "status") + query_data["sources"] = _query_values("sources", "source") + query_data["statuses"] = _query_values("statuses", "status") query = AgentLogsQuery.model_validate(query_data) start, end = _parse_observability_time_range(query.start, query.end, current_user) try: diff --git a/api/controllers/console/app/app.py b/api/controllers/console/app/app.py index 49516883560..a1423318c72 100644 --- a/api/controllers/console/app/app.py +++ b/api/controllers/console/app/app.py @@ -1,5 +1,4 @@ import logging -import re import uuid from collections.abc import Sequence from datetime import datetime @@ -10,7 +9,6 @@ from flask_restx import Resource from pydantic import AliasChoices, BaseModel, Field, computed_field, field_validator from sqlalchemy import select from sqlalchemy.orm import Session -from werkzeug.datastructures import MultiDict from werkzeug.exceptions import BadRequest, NotFound from configs import dify_config @@ -19,6 +17,7 @@ from controllers.common.fields import RedirectUrlResponse, SimpleResultResponse from controllers.common.helpers import FileInfo from controllers.common.schema import ( query_params_from_model, + query_params_from_request, register_enum_models, register_response_schema_models, register_schema_models, @@ -75,10 +74,9 @@ ALLOW_CREATE_APP_MODES = ["chat", "agent-chat", "advanced-chat", "workflow", "co register_enum_models(console_ns, IconType) _logger = logging.getLogger(__name__) -_TAG_IDS_BRACKET_PATTERN = re.compile(r"^tag_ids\[(\d+)\]$") -_CREATOR_IDS_BRACKET_PATTERN = re.compile(r"^creator_ids\[(\d+)\]$") AppListMode = Literal["completion", "chat", "advanced-chat", "workflow", "agent-chat", "agent", "channel", "all"] DEFAULT_APP_LIST_MODE: AppListMode = "all" +APP_LIST_QUERY_ARRAY_FIELDS = ("tag_ids", "creator_ids") class AppListBaseQuery(BaseModel): @@ -139,34 +137,6 @@ class StarredAppListQuery(AppListBaseQuery): pass -def _normalize_app_list_query_args(query_args: MultiDict[str, str]) -> dict[str, str | list[str]]: - normalized: dict[str, str | list[str]] = {} - indexed_tag_ids: list[tuple[int, str]] = [] - indexed_creator_ids: list[tuple[int, str]] = [] - - for key in query_args: - match = _TAG_IDS_BRACKET_PATTERN.fullmatch(key) - if match: - indexed_tag_ids.extend((int(match.group(1)), value) for value in query_args.getlist(key)) - continue - - match = _CREATOR_IDS_BRACKET_PATTERN.fullmatch(key) - if match: - indexed_creator_ids.extend((int(match.group(1)), value) for value in query_args.getlist(key)) - continue - - value = query_args.get(key) - if value is not None: - normalized[key] = value - - if indexed_tag_ids: - normalized["tag_ids"] = [value for _, value in sorted(indexed_tag_ids)] - if indexed_creator_ids: - normalized["creator_ids"] = [value for _, value in sorted(indexed_creator_ids)] - - return normalized - - class CreateAppPayload(BaseModel): name: str = Field(..., min_length=1, description="App name") description: str | None = Field(default=None, description="App description (max 400 chars)", max_length=400) @@ -599,7 +569,7 @@ class AppListApi(Resource): @with_current_tenant_id def get(self, current_tenant_id: str, current_user_id: str, session: Session): """Get app list""" - args = AppListQuery.model_validate(_normalize_app_list_query_args(request.args)) + args = query_params_from_request(AppListQuery, list_fields=APP_LIST_QUERY_ARRAY_FIELDS) params = AppListParams( page=args.page, limit=args.limit, @@ -699,7 +669,7 @@ class StarredAppListApi(Resource): @with_current_user_id @with_current_tenant_id def get(self, current_tenant_id: str, current_user_id: str, session: Session): - args = StarredAppListQuery.model_validate(_normalize_app_list_query_args(request.args)) + args = query_params_from_request(StarredAppListQuery, list_fields=APP_LIST_QUERY_ARRAY_FIELDS) params = StarredAppListParams( page=args.page, limit=args.limit, diff --git a/api/controllers/console/snippets/payloads.py b/api/controllers/console/snippets/payloads.py index 70730e754de..a3c90626d30 100644 --- a/api/controllers/console/snippets/payloads.py +++ b/api/controllers/console/snippets/payloads.py @@ -21,14 +21,19 @@ class SnippetListQuery(BaseModel): @field_validator("creators", mode="before") @classmethod def parse_creators(cls, value: object) -> list[str] | None: - """Normalize creators filter from query string or list input.""" - return cls._normalize_string_list(value) + """Normalize creator filters without comma-splitting list values.""" + if value is None: + return None + if isinstance(value, str): + item = value.strip() + return [item] if item else None + return cls._normalize_string_list(value, "creators") @field_validator("tag_ids", mode="before") @classmethod def parse_tag_ids(cls, value: object) -> list[str] | None: - """Normalize and validate tag IDs from query string or list input.""" - items = cls._normalize_string_list(value) + """Normalize and validate tag IDs from repeated query parameters.""" + items = cls._normalize_string_list(value, "tag_ids") if not items: return None try: @@ -37,14 +42,12 @@ class SnippetListQuery(BaseModel): raise ValueError("Invalid UUID format in tag_ids.") from exc @staticmethod - def _normalize_string_list(value: object) -> list[str] | None: + def _normalize_string_list(value: object, field_name: str) -> list[str] | None: if value is None: return None - if isinstance(value, str): - return [item.strip() for item in value.split(",") if item.strip()] or None if isinstance(value, list): return [str(item).strip() for item in value if str(item).strip()] or None - return None + raise ValueError(f"Unsupported {field_name} type.") class IconInfo(BaseModel): diff --git a/api/controllers/console/workspace/snippets.py b/api/controllers/console/workspace/snippets.py index 137bae66358..3a8db2dcaa1 100644 --- a/api/controllers/console/workspace/snippets.py +++ b/api/controllers/console/workspace/snippets.py @@ -1,5 +1,4 @@ import logging -import re from datetime import datetime from typing import Any from urllib.parse import quote @@ -9,11 +8,14 @@ from flask_restx import Resource, marshal from pydantic import Field as PydanticField from pydantic import field_validator from sqlalchemy.orm import Session, sessionmaker -from werkzeug.datastructures import MultiDict from werkzeug.exceptions import NotFound from controllers.common.fields import TextFileResponse -from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models +from controllers.common.schema import ( + query_params_from_model, + register_response_schema_models, + register_schema_models, +) from controllers.console import console_ns from controllers.console.snippets.payloads import ( CreateSnippetPayload, @@ -44,9 +46,6 @@ from services.snippet_dsl_service import ImportStatus, SnippetDslService from services.snippet_service import SnippetService logger = logging.getLogger(__name__) -_TAG_IDS_BRACKET_PATTERN = re.compile(r"^tag_ids\[(\d+)\]$") -_CREATOR_IDS_BRACKET_PATTERN = re.compile(r"^creator_ids\[(\d+)\]$") -_CREATORS_BRACKET_PATTERN = re.compile(r"^creators\[(\d+)\]$") class SnippetImportResponse(ResponseModel): @@ -142,40 +141,15 @@ def _snippet_service() -> SnippetService: return SnippetService(sessionmaker(bind=db.engine, expire_on_commit=False)) -def _normalize_snippet_list_query_args(query_args: MultiDict[str, str]) -> dict[str, str | list[str]]: - normalized: dict[str, str | list[str]] = {} - indexed_tag_ids: list[tuple[int, str]] = [] - indexed_creator_ids: list[tuple[int, str]] = [] +def _snippet_list_query_from_request() -> SnippetListQuery: + query_data: dict[str, str | list[str]] = dict(request.args.to_dict()) + query_data["tag_ids"] = request.args.getlist("tag_ids") - for key in query_args: - match = _TAG_IDS_BRACKET_PATTERN.fullmatch(key) - if match: - indexed_tag_ids.extend((int(match.group(1)), value) for value in query_args.getlist(key)) - continue + creator_ids = request.args.getlist("creators") or request.args.getlist("creator_ids") + if creator_ids: + query_data["creators"] = creator_ids - match = _CREATOR_IDS_BRACKET_PATTERN.fullmatch(key) or _CREATORS_BRACKET_PATTERN.fullmatch(key) - if match: - indexed_creator_ids.extend((int(match.group(1)), value) for value in query_args.getlist(key)) - continue - - if key in {"tag_ids", "creators", "creator_ids"}: - values = query_args.getlist(key) - if values: - normalized["creators" if key in {"creators", "creator_ids"} else key] = ( - values if len(values) > 1 else values[0] - ) - continue - - value = query_args.get(key) - if value is not None: - normalized[key] = value - - if indexed_tag_ids: - normalized["tag_ids"] = [value for _, value in sorted(indexed_tag_ids)] - if indexed_creator_ids: - normalized["creators"] = [value for _, value in sorted(indexed_creator_ids)] - - return normalized + return SnippetListQuery.model_validate(query_data) # Register Pydantic models with Swagger @@ -210,7 +184,7 @@ class CustomizedSnippetsApi(Resource): @with_current_tenant_id def get(self, current_tenant_id: str): """List customized snippets with pagination and search.""" - query = SnippetListQuery.model_validate(_normalize_snippet_list_query_args(request.args)) + query = _snippet_list_query_from_request() snippet_service = _snippet_service() snippets, total, has_more = snippet_service.get_snippets( diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py index 4125e7d8de8..7a3f158b0c3 100644 --- a/api/controllers/console/workspace/tool_providers.py +++ b/api/controllers/console/workspace/tool_providers.py @@ -11,7 +11,12 @@ from werkzeug.exceptions import Forbidden from configs import dify_config from controllers.common.fields import BinaryFileResponse, RedirectResponse, SimpleResultResponse -from controllers.common.schema import query_params_from_model, register_response_schema_models, register_schema_models +from controllers.common.schema import ( + query_params_from_model, + query_params_from_request, + register_response_schema_models, + register_schema_models, +) from controllers.console import console_ns from controllers.console.wraps import ( RBACPermission, @@ -445,16 +450,17 @@ class ToolBuiltinProviderGetCredentialsApi(Resource): def get(self, tenant_id: str, user: Account, provider: str): # Optional list of credential IDs to include even if visibility would hide them # (used when a workflow/agent node still references another member's only_me credential). - include_credential_ids = request.args.getlist("include_credential_ids") or [ - s for s in (request.args.get("include_credential_ids") or "").split(",") if s - ] + query = query_params_from_request( + BuiltinCredentialListQuery, + list_fields=("include_credential_ids",), + ) return jsonable_encoder( BuiltinToolManageService.get_builtin_tool_provider_credentials( tenant_id=tenant_id, provider_name=provider, user=user, - include_credential_ids=include_credential_ids or None, + include_credential_ids=query.include_credential_ids or None, ) ) @@ -1049,16 +1055,17 @@ class ToolBuiltinProviderGetCredentialInfoApi(Resource): @with_current_user @with_current_tenant_id def get(self, tenant_id: str, user: Account, provider: str): - include_credential_ids = request.args.getlist("include_credential_ids") or [ - s for s in (request.args.get("include_credential_ids") or "").split(",") if s - ] + query = query_params_from_request( + BuiltinCredentialListQuery, + list_fields=("include_credential_ids",), + ) return jsonable_encoder( BuiltinToolManageService.get_builtin_tool_provider_credential_info( tenant_id=tenant_id, provider=provider, user=user, - include_credential_ids=include_credential_ids or None, + include_credential_ids=query.include_credential_ids or None, ) ) diff --git a/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py b/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py index e4f347ca61e..e1351875fa7 100644 --- a/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py +++ b/api/tests/unit_tests/controllers/console/agent/test_agent_controllers.py @@ -1061,8 +1061,8 @@ def test_agent_observability_routes_resolve_app_from_agent_id( account = SimpleNamespace(id=account_id, timezone="UTC") with app.test_request_context( "/console/api/agent/00000000-0000-0000-0000-000000000001/logs" - "?page=2&limit=5&keyword=hello&statuses[]=success&statuses[]=failed&sources[]=webapp:app-1" - "&sources[]=workflow:app-2:workflow-1:v1:node-1&sort_by=created_at&sort_order=asc" + "?page=2&limit=5&keyword=hello&statuses=success&statuses=failed&sources=webapp:app-1" + "&sources=workflow:app-2:workflow-1:v1:node-1&sort_by=created_at&sort_order=asc" ): logs = unwrap(AgentLogsApi.get)(AgentLogsApi(), "tenant-1", account, agent_id) diff --git a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py index 7801fe83336..e7784f8fd94 100644 --- a/api/tests/unit_tests/controllers/console/app/test_app_response_models.py +++ b/api/tests/unit_tests/controllers/console/app/test_app_response_models.py @@ -196,38 +196,44 @@ def _dummy_workflow(): ) -def test_app_list_query_normalizes_orpc_bracket_tag_ids(app_module): +def test_app_list_query_reads_repeated_tag_ids(app_module): first_tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" second_tag_id = "3c39395b-6d1f-4030-8b17-eaa7cc85221c" query_args = MultiDict( [ ("page", "1"), ("limit", "30"), - ("tag_ids[1]", second_tag_id), - ("tag_ids[0]", first_tag_id), + ("tag_ids", first_tag_id), + ("tag_ids", second_tag_id), ] ) - normalized = app_module._normalize_app_list_query_args(query_args) - query = app_module.AppListQuery.model_validate(normalized) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=query_args, + ) assert query.tag_ids == [first_tag_id, second_tag_id] -def test_app_list_query_normalizes_orpc_bracket_creator_ids(app_module): +def test_app_list_query_reads_repeated_creator_ids(app_module): first_creator_id = "9e8959cf-a67b-4d34-9906-1d687517b248" second_creator_id = "1886f96a-5bf0-42bf-961d-8d2129049076" query_args = MultiDict( [ ("page", "1"), ("limit", "30"), - ("creator_ids[1]", second_creator_id), - ("creator_ids[0]", first_creator_id), + ("creator_ids", first_creator_id), + ("creator_ids", second_creator_id), ] ) - normalized = app_module._normalize_app_list_query_args(query_args) - query = app_module.AppListQuery.model_validate(normalized) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=query_args, + ) assert query.creator_ids == [first_creator_id, second_creator_id] @@ -243,16 +249,12 @@ def test_app_list_query_preserves_regular_query_params(app_module): ] ) - normalized = app_module._normalize_app_list_query_args(query_args) - query = app_module.AppListQuery.model_validate(normalized) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=query_args, + ) - assert normalized == { - "page": "2", - "limit": "50", - "mode": "chat", - "name": "Sales Copilot", - "is_created_by_me": "true", - } assert query.page == 2 assert query.limit == 50 assert query.mode == "chat" @@ -261,59 +263,67 @@ def test_app_list_query_preserves_regular_query_params(app_module): assert query.tag_ids is None -def test_app_list_query_normalizes_empty_bracket_tag_ids_to_none(app_module): +def test_app_list_query_normalizes_empty_repeated_tag_ids_to_none(app_module): query_args = MultiDict( [ - ("tag_ids[0]", ""), - ("tag_ids[1]", " "), + ("tag_ids", ""), + ("tag_ids", " "), ] ) - normalized = app_module._normalize_app_list_query_args(query_args) - query = app_module.AppListQuery.model_validate(normalized) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=query_args, + ) - assert normalized == {"tag_ids": ["", " "]} assert query.tag_ids is None -def test_app_list_query_rejects_invalid_bracket_tag_id(app_module): - normalized = app_module._normalize_app_list_query_args(MultiDict([("tag_ids[0]", "not-a-uuid")])) - +def test_app_list_query_rejects_invalid_repeated_tag_id(app_module): with pytest.raises(ValidationError): - app_module.AppListQuery.model_validate(normalized) + app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=MultiDict([("tag_ids", "not-a-uuid")]), + ) -def test_app_list_query_rejects_invalid_bracket_creator_id(app_module): - normalized = app_module._normalize_app_list_query_args(MultiDict([("creator_ids[0]", "not-a-uuid")])) - +def test_app_list_query_rejects_invalid_repeated_creator_id(app_module): with pytest.raises(ValidationError): - app_module.AppListQuery.model_validate(normalized) + app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=MultiDict([("creator_ids", "not-a-uuid")]), + ) -def test_app_list_query_sorts_bracket_tag_ids_by_index(app_module): - first_tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" - second_tag_id = "3c39395b-6d1f-4030-8b17-eaa7cc85221c" - third_tag_id = "9d5ec0f7-4f2b-4e7f-9c13-1e7a034d0eb1" +def test_app_list_query_ignores_indexed_tag_ids(app_module): + tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" query_args = MultiDict( [ - ("tag_ids[2]", third_tag_id), - ("tag_ids[1]", second_tag_id), - ("tag_ids[0]", first_tag_id), + ("tag_ids[0]", tag_id), ] ) - normalized = app_module._normalize_app_list_query_args(query_args) - query = app_module.AppListQuery.model_validate(normalized) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=query_args, + ) - assert query.tag_ids == [first_tag_id, second_tag_id, third_tag_id] + assert query.tag_ids is None -def test_app_list_query_rejects_flat_tag_ids(app_module): +def test_app_list_query_accepts_single_repeated_tag_id(app_module): tag_id = "8c4ef3d1-58a1-4d94-8a1c-1c171d889e08" - normalized = app_module._normalize_app_list_query_args(MultiDict([("tag_ids", tag_id)])) + query = app_module.query_params_from_request( + app_module.AppListQuery, + list_fields=app_module.APP_LIST_QUERY_ARRAY_FIELDS, + args=MultiDict([("tag_ids", tag_id)]), + ) - with pytest.raises(ValidationError): - app_module.AppListQuery.model_validate(normalized) + assert query.tag_ids == [tag_id] def test_create_app_endpoint_rejects_agent_mode(app_module, monkeypatch: pytest.MonkeyPatch): diff --git a/api/tests/unit_tests/controllers/console/snippets/test_snippet_list_query.py b/api/tests/unit_tests/controllers/console/snippets/test_snippet_list_query.py index 6ef3474461d..019e0f12dff 100644 --- a/api/tests/unit_tests/controllers/console/snippets/test_snippet_list_query.py +++ b/api/tests/unit_tests/controllers/console/snippets/test_snippet_list_query.py @@ -1,29 +1,35 @@ import pytest from pydantic import ValidationError -from werkzeug.datastructures import MultiDict from controllers.console.snippets.payloads import SnippetListQuery -from controllers.console.workspace.snippets import _normalize_snippet_list_query_args -def test_snippet_list_query_accepts_comma_separated_tag_ids() -> None: +def test_snippet_list_query_accepts_tag_id_lists() -> None: first = "11111111-1111-1111-1111-111111111111" second = "22222222-2222-2222-2222-222222222222" - query = SnippetListQuery.model_validate({"tag_ids": f"{first},{second}"}) + query = SnippetListQuery.model_validate({"tag_ids": [first, second]}) assert query.tag_ids == [first, second] def test_snippet_list_query_returns_none_for_blank_tag_ids() -> None: - query = SnippetListQuery.model_validate({"tag_ids": " , "}) + query = SnippetListQuery.model_validate({"tag_ids": ["", " "]}) assert query.tag_ids is None def test_snippet_list_query_rejects_invalid_tag_id() -> None: with pytest.raises(ValidationError, match="Invalid UUID format in tag_ids"): - SnippetListQuery.model_validate({"tag_ids": "not-a-uuid"}) + SnippetListQuery.model_validate({"tag_ids": ["not-a-uuid"]}) + + +def test_snippet_list_query_rejects_comma_separated_tag_ids() -> None: + first = "11111111-1111-1111-1111-111111111111" + second = "22222222-2222-2222-2222-222222222222" + + with pytest.raises(ValidationError, match="Unsupported tag_ids type"): + SnippetListQuery.model_validate({"tag_ids": f"{first},{second}"}) def test_snippet_list_query_accepts_creator_id_alias() -> None: @@ -40,41 +46,6 @@ def test_snippet_list_query_normalizes_creator_lists() -> None: assert query.creators == ["account-1", "account-2"] -def test_snippet_list_query_ignores_unsupported_list_value_type() -> None: - query = SnippetListQuery.model_validate({"creators": {"bad": "value"}}) - - assert query.creators is None - - -def test_normalize_snippet_list_query_accepts_indexed_creator_ids() -> None: - first = "9e8959cf-a67b-4d34-9906-1d687517b248" - second = "1886f96a-5bf0-42bf-961d-8d2129049076" - - normalized = _normalize_snippet_list_query_args( - MultiDict( - [ - ("creator_ids[1]", second), - ("creator_ids[0]", first), - ("keyword", "search"), - ] - ) - ) - - assert normalized == {"keyword": "search", "creators": [first, second]} - - -def test_normalize_snippet_list_query_accepts_indexed_tag_ids() -> None: - first = "11111111-1111-1111-1111-111111111111" - second = "22222222-2222-2222-2222-222222222222" - - normalized = _normalize_snippet_list_query_args( - MultiDict( - [ - ("tag_ids[1]", second), - ("tag_ids[0]", first), - ("keyword", "search"), - ] - ) - ) - - assert normalized == {"keyword": "search", "tag_ids": [first, second]} +def test_snippet_list_query_rejects_unsupported_list_value_type() -> None: + with pytest.raises(ValidationError, match="Unsupported creators type"): + SnippetListQuery.model_validate({"creators": {"bad": "value"}}) diff --git a/api/tests/unit_tests/controllers/console/workspace/test_snippets.py b/api/tests/unit_tests/controllers/console/workspace/test_snippets.py index 248e526e78f..8946b55e09f 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_snippets.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_snippets.py @@ -65,48 +65,37 @@ def _snippet(**overrides) -> SimpleNamespace: return SimpleNamespace(**data) -def test_normalize_snippet_list_query_args_sorts_indexed_values(): - query_args = snippets_module.MultiDict( - [ - ("tag_ids[1]", "tag-b"), - ("tag_ids[0]", "tag-a"), - ("creator_ids[1]", "account-b"), - ("creator_ids[0]", "account-a"), - ("keyword", "search"), - ] - ) +def test_snippet_list_query_reads_repeated_values(app: Flask): + tag_id = "11111111-1111-1111-1111-111111111111" + other_tag_id = "22222222-2222-2222-2222-222222222222" - assert snippets_module._normalize_snippet_list_query_args(query_args) == { - "tag_ids": ["tag-a", "tag-b"], - "creators": ["account-a", "account-b"], - "keyword": "search", - } + with app.test_request_context( + f"/workspaces/current/customized-snippets?tag_ids={tag_id}&tag_ids={other_tag_id}" + "&creators=account-a&creators=account-b&keyword=search" + ): + query = snippets_module._snippet_list_query_from_request() + + assert query.tag_ids == [tag_id, other_tag_id] + assert query.creators == ["account-a", "account-b"] + assert query.keyword == "search" -def test_normalize_snippet_list_query_args_accepts_generated_creator_keys(): - query_args = snippets_module.MultiDict( - [ - ("creators[1]", "account-b"), - ("creators[0]", "account-a"), - ] - ) +def test_snippet_list_query_reads_creator_ids_alias(app: Flask): + with app.test_request_context( + "/workspaces/current/customized-snippets?creator_ids=account-a&creator_ids=account-b" + ): + query = snippets_module._snippet_list_query_from_request() - assert snippets_module._normalize_snippet_list_query_args(query_args) == { - "creators": ["account-a", "account-b"], - } + assert query.creators == ["account-a", "account-b"] -def test_normalize_snippet_list_query_args_accepts_repeated_creator_keys(): - query_args = snippets_module.MultiDict( - [ - ("creators", "account-a"), - ("creators", "account-b"), - ] - ) +def test_snippet_list_query_ignores_indexed_values(app: Flask): + tag_id = "11111111-1111-1111-1111-111111111111" + with app.test_request_context(f"/workspaces/current/customized-snippets?tag_ids[0]={tag_id}&creators[0]=account-a"): + query = snippets_module._snippet_list_query_from_request() - assert snippets_module._normalize_snippet_list_query_args(query_args) == { - "creators": ["account-a", "account-b"], - } + assert query.tag_ids is None + assert query.creators is None def test_list_snippets_returns_pagination(app: Flask, monkeypatch: pytest.MonkeyPatch): @@ -119,7 +108,7 @@ def test_list_snippets_returns_pagination(app: Flask, monkeypatch: pytest.Monkey handler = unwrap(api.get) with app.test_request_context( - f"/workspaces/current/customized-snippets?page=2&limit=10&tag_ids[0]={tag_id}&creators[0]=account-2" + f"/workspaces/current/customized-snippets?page=2&limit=10&tag_ids={tag_id}&creators=account-2" ): response, status_code = handler(api, "tenant-1") diff --git a/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py b/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py index e9c23da428e..2a576d1c920 100644 --- a/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py +++ b/api/tests/unit_tests/controllers/console/workspace/test_tool_providers.py @@ -191,6 +191,30 @@ def test_builtin_provider_credentials_get(app: Flask, controller_module, monkeyp ) +def test_builtin_provider_credentials_get_reads_repeated_include_ids( + app: Flask, controller_module, monkeypatch: pytest.MonkeyPatch +): + user = _mock_account("user-tenant-cred") + service_mock = MagicMock(return_value=[{"cred": 1}]) + monkeypatch.setattr( + controller_module.BuiltinToolManageService, + "get_builtin_tool_provider_credentials", + service_mock, + ) + + with app.test_request_context("/creds?include_credential_ids=cred-1&include_credential_ids=cred-2", method="GET"): + api = controller_module.ToolBuiltinProviderGetCredentialsApi() + resp = unwrap(api.get)(api, "tenant-cred", user, provider="demo") + + assert resp == [{"cred": 1}] + service_mock.assert_called_once_with( + tenant_id="tenant-cred", + provider_name="demo", + user=user, + include_credential_ids=["cred-1", "cred-2"], + ) + + def test_api_provider_remote_schema_get(app: Flask, controller_module, monkeypatch: pytest.MonkeyPatch): user = _mock_account() service_mock = MagicMock(return_value={"schema": "ok"}) diff --git a/web/service/client.spec.ts b/web/service/client.spec.ts index 5eedf391176..d596c7da478 100644 --- a/web/service/client.spec.ts +++ b/web/service/client.spec.ts @@ -4,6 +4,7 @@ import type { MutationFunctionContext, QueryFunctionContext } from '@tanstack/re import type { consoleQuery as ConsoleQuery } from './client' import { QueryClient } from '@tanstack/react-query' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { normalizeConsoleOpenAPIURL } from './console-openapi-url' const loadGetBaseURL = async (isClientValue: boolean) => { vi.resetModules() @@ -294,6 +295,46 @@ describe('consoleQuery transport context', () => { }) }) +// Scenario: console OpenAPI query arrays follow backend parser expectations. +describe('normalizeConsoleOpenAPIURL', () => { + it('should serialize repeated-only query arrays as repeated params', () => { + const url = normalizeConsoleOpenAPIURL( + 'https://example.com/console/api/agent/agent-1/logs?sources%5B1%5D=debug&sources%5B0%5D=api&statuses%5B0%5D=success&keyword=test', + ) + const searchParams = new URL(url).searchParams + + expect(searchParams.getAll('sources')).toEqual(['api', 'debug']) + expect(searchParams.getAll('statuses')).toEqual(['success']) + expect(searchParams.get('keyword')).toBe('test') + expect(searchParams.has('sources[0]')).toBe(false) + expect(searchParams.has('statuses[0]')).toBe(false) + }) + + it('should serialize app list query arrays as repeated params', () => { + const url = normalizeConsoleOpenAPIURL( + 'https://example.com/console/api/apps?tag_ids%5B0%5D=tag-1&creator_ids%5B0%5D=user-1', + ) + const searchParams = new URL(url).searchParams + + expect(searchParams.getAll('tag_ids')).toEqual(['tag-1']) + expect(searchParams.getAll('creator_ids')).toEqual(['user-1']) + expect(searchParams.has('tag_ids[0]')).toBe(false) + expect(searchParams.has('creator_ids[0]')).toBe(false) + }) + + it('should serialize snippet list query arrays as repeated params', () => { + const url = normalizeConsoleOpenAPIURL( + 'https://example.com/console/api/workspaces/current/customized-snippets?tag_ids%5B0%5D=tag-1&creators%5B0%5D=user-1', + ) + const searchParams = new URL(url).searchParams + + expect(searchParams.getAll('tag_ids')).toEqual(['tag-1']) + expect(searchParams.getAll('creators')).toEqual(['user-1']) + expect(searchParams.has('tag_ids[0]')).toBe(false) + expect(searchParams.has('creators[0]')).toBe(false) + }) +}) + // Scenario: oRPC mutation defaults own shared Agent roster cache behavior. describe('consoleQuery agent mutation defaults', () => { beforeEach(() => { diff --git a/web/service/client.ts b/web/service/client.ts index e1a55439433..e0527885df6 100644 --- a/web/service/client.ts +++ b/web/service/client.ts @@ -26,6 +26,7 @@ import { isClient } from '@/utils/client' // eslint-disable-next-line no-restricted-imports import { request } from './base' import { createConsoleDynamicLink } from './console-link' +import { normalizeConsoleOpenAPIURL } from './console-openapi-url' function getMarketplaceHeaders() { return new Headers({ @@ -44,38 +45,6 @@ function isURL(path: string) { } } -const trialAppDatasetsPathPattern = /\/trial-apps\/[^/]+\/datasets$/ -const indexedIdsQueryParamPattern = /^ids\[(\d+)\]$/ - -function normalizeConsoleOpenAPIURL(url: string | URL) { - const normalizedUrl = new URL(url) - - if (!trialAppDatasetsPathPattern.test(normalizedUrl.pathname)) - return normalizedUrl.href - - const ids: Array<{ index: number, value: string }> = [] - const indexedKeys = new Set() - - normalizedUrl.searchParams.forEach((value, key) => { - const match = indexedIdsQueryParamPattern.exec(key) - if (!match) - return - - indexedKeys.add(key) - ids.push({ index: Number(match[1]), value }) - }) - - if (!ids.length) - return normalizedUrl.href - - indexedKeys.forEach(key => normalizedUrl.searchParams.delete(key)) - ids - .sort((a, b) => a.index - b.index) - .forEach(({ value }) => normalizedUrl.searchParams.append('ids', value)) - - return normalizedUrl.href -} - export function getBaseURL(path: string) { const url = new URL(path, isURL(path) ? undefined : isClient ? window.location.origin : 'http://localhost') diff --git a/web/service/console-openapi-url.ts b/web/service/console-openapi-url.ts new file mode 100644 index 00000000000..e6b71087bb2 --- /dev/null +++ b/web/service/console-openapi-url.ts @@ -0,0 +1,64 @@ +type QueryArrayCompatibilityRule = { + path: RegExp + fields: readonly string[] +} + +// Keep oRPC query-array compatibility at the console OpenAPI transport boundary instead of +// making backend handlers accept multiple serialized forms. oRPC v2 supports custom query +// parameter serialization, so this bridge can be removed once we configure repeated keys there. +const repeatedQueryArrayRules: readonly QueryArrayCompatibilityRule[] = [ + { path: /\/agent$/, fields: ['tag_ids', 'creator_ids'] }, + { path: /\/agent\/[^/]+\/logs$/, fields: ['sources', 'statuses'] }, + { path: /\/agent\/[^/]+\/logs\/[^/]+\/messages$/, fields: ['sources', 'statuses'] }, + { path: /\/apps$/, fields: ['tag_ids', 'creator_ids'] }, + { path: /\/apps\/starred$/, fields: ['tag_ids', 'creator_ids'] }, + { path: /\/datasets$/, fields: ['ids', 'tag_ids'] }, + { path: /\/datasets\/[^/]+\/documents\/[^/]+\/segment\/[^/]+$/, fields: ['segment_id'] }, + { path: /\/datasets\/[^/]+\/documents\/[^/]+\/segments$/, fields: ['segment_id', 'status'] }, + { path: /\/trial-apps\/[^/]+\/datasets$/, fields: ['ids'] }, + { path: /\/workspaces\/current\/customized-snippets$/, fields: ['tag_ids', 'creators'] }, + { path: /\/workspaces\/current\/tool-provider\/builtin\/[^/]+\/credential\/info$/, fields: ['include_credential_ids'] }, + { path: /\/workspaces\/current\/tool-provider\/builtin\/[^/]+\/credentials$/, fields: ['include_credential_ids'] }, +] + +const escapeRegExp = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + +const getRepeatedQueryArrayFields = (pathname: string) => + repeatedQueryArrayRules.find(rule => rule.path.test(pathname))?.fields ?? [] + +const rewriteIndexedQueryArrayParam = (url: URL, field: string) => { + const indexedParamPattern = new RegExp(`^${escapeRegExp(field)}\\[(\\d+)\\]$`) + const values: Array<{ index: number, value: string }> = [] + const indexedKeys = new Set() + + url.searchParams.forEach((value, key) => { + const match = indexedParamPattern.exec(key) + if (!match) + return + + indexedKeys.add(key) + values.push({ index: Number(match[1]), value }) + }) + + if (!values.length) + return false + + indexedKeys.forEach(key => url.searchParams.delete(key)) + values + .sort((a, b) => a.index - b.index) + .forEach(({ value }) => url.searchParams.append(field, value)) + + return true +} + +export function normalizeConsoleOpenAPIURL(url: string | URL) { + const normalizedUrl = new URL(url) + const repeatedQueryArrayFields = getRepeatedQueryArrayFields(normalizedUrl.pathname) + + if (!repeatedQueryArrayFields.length) + return normalizedUrl.href + + repeatedQueryArrayFields.forEach(field => rewriteIndexedQueryArrayParam(normalizedUrl, field)) + + return normalizedUrl.href +}