From c530a5d2723270ab566328b52d50723333253647 Mon Sep 17 00:00:00 2001 From: WOLIKIMCHENG <35391914+WOLIKIMCHENG@users.noreply.github.com> Date: Fri, 29 May 2026 14:25:48 +0800 Subject: [PATCH] fix(api): validate annotation list pagination query (#36807) Co-authored-by: root Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- api/controllers/service_api/app/annotation.py | 30 +++++--- api/openapi/markdown/service-swagger.md | 16 +++++ .../service_api/app/test_annotation.py | 71 +++++++++++++++++-- .../generated/api/service/orpc.gen.ts | 2 + .../generated/api/service/types.gen.ts | 12 +++- .../generated/api/service/zod.gen.ts | 15 ++++ 6 files changed, 129 insertions(+), 17 deletions(-) diff --git a/api/controllers/service_api/app/annotation.py b/api/controllers/service_api/app/annotation.py index 12ab692adb..8906d544e8 100644 --- a/api/controllers/service_api/app/annotation.py +++ b/api/controllers/service_api/app/annotation.py @@ -6,7 +6,7 @@ from flask_restx import Resource from flask_restx.api import HTTPStatus from pydantic import BaseModel, Field, TypeAdapter -from controllers.common.schema import register_schema_models +from controllers.common.schema import query_params_from_model, register_schema_models from controllers.console.wraps import edit_permission_required from controllers.service_api import service_api_ns from controllers.service_api.wraps import validate_app_token @@ -32,8 +32,19 @@ class AnnotationReplyActionPayload(BaseModel): embedding_model_name: str = Field(description="Embedding model name") +class AnnotationListQuery(BaseModel): + page: int = Field(default=1, ge=1, description="Page number") + limit: int = Field(default=20, ge=1, description="Number of annotations per page") + keyword: str = Field(default="", description="Keyword to search annotations") + + register_schema_models( - service_api_ns, AnnotationCreatePayload, AnnotationReplyActionPayload, Annotation, AnnotationList + service_api_ns, + AnnotationCreatePayload, + AnnotationReplyActionPayload, + AnnotationListQuery, + Annotation, + AnnotationList, ) @@ -100,6 +111,7 @@ class AnnotationReplyActionStatusApi(Resource): class AnnotationListApi(Resource): @service_api_ns.doc("list_annotations") @service_api_ns.doc(description="List annotations for the application") + @service_api_ns.doc(params=query_params_from_model(AnnotationListQuery)) @service_api_ns.doc( responses={ 200: "Annotations retrieved successfully", @@ -114,18 +126,18 @@ class AnnotationListApi(Resource): @validate_app_token def get(self, app_model: App): """List annotations for the application.""" - page = request.args.get("page", default=1, type=int) - limit = request.args.get("limit", default=20, type=int) - keyword = request.args.get("keyword", default="", type=str) + query = AnnotationListQuery.model_validate(request.args.to_dict(flat=True)) - annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id(app_model.id, page, limit, keyword) + annotation_list, total = AppAnnotationService.get_annotation_list_by_app_id( + app_model.id, query.page, query.limit, query.keyword + ) annotation_models = TypeAdapter(list[Annotation]).validate_python(annotation_list, from_attributes=True) response = AnnotationList( data=annotation_models, - has_more=len(annotation_list) == limit, - limit=limit, + has_more=len(annotation_list) == query.limit, + limit=query.limit, total=total, - page=page, + page=query.page, ) return response.model_dump(mode="json") diff --git a/api/openapi/markdown/service-swagger.md b/api/openapi/markdown/service-swagger.md index 071b1b526c..11881c5d38 100644 --- a/api/openapi/markdown/service-swagger.md +++ b/api/openapi/markdown/service-swagger.md @@ -112,6 +112,14 @@ List annotations for the application List annotations for the application +##### Parameters + +| Name | Located in | Description | Required | Schema | +| ---- | ---------- | ----------- | -------- | ------ | +| keyword | query | Keyword to search annotations | No | string | +| limit | query | Number of annotations per page | No | integer | +| page | query | Page number | No | integer | + ##### Responses | Code | Description | Schema | @@ -2169,6 +2177,14 @@ Returns a list of available models for the specified model type. | page | integer | | Yes | | total | integer | | Yes | +#### AnnotationListQuery + +| Name | Type | Description | Required | +| ---- | ---- | ----------- | -------- | +| keyword | string | Keyword to search annotations | No | +| limit | integer | Number of annotations per page | No | +| page | integer | Page number | No | + #### AnnotationReplyActionPayload | Name | Type | Description | Required | diff --git a/api/tests/unit_tests/controllers/service_api/app/test_annotation.py b/api/tests/unit_tests/controllers/service_api/app/test_annotation.py index 2ab5547cd4..6d586d31a9 100644 --- a/api/tests/unit_tests/controllers/service_api/app/test_annotation.py +++ b/api/tests/unit_tests/controllers/service_api/app/test_annotation.py @@ -19,10 +19,12 @@ from unittest.mock import Mock import pytest from flask import Flask from flask_restx.api import HTTPStatus +from pydantic import ValidationError from controllers.service_api.app.annotation import ( AnnotationCreatePayload, AnnotationListApi, + AnnotationListQuery, AnnotationReplyActionApi, AnnotationReplyActionPayload, AnnotationReplyActionStatusApi, @@ -106,6 +108,28 @@ class TestAnnotationReplyActionPayload: assert payload.score_threshold == 0.0 +class TestAnnotationListQuery: + def test_defaults(self) -> None: + query = AnnotationListQuery.model_validate({}) + + assert query.page == 1 + assert query.limit == 20 + assert query.keyword == "" + + def test_valid_numeric_strings(self) -> None: + query = AnnotationListQuery.model_validate({"page": "2", "limit": "5", "keyword": "refund"}) + + assert query.page == 2 + assert query.limit == 5 + assert query.keyword == "refund" + + @pytest.mark.parametrize("field", ["page", "limit"]) + @pytest.mark.parametrize("value", ["abc", "1.5", "1e2", "", "0", "-1"]) + def test_invalid_explicit_pagination_value(self, field: str, value: str) -> None: + with pytest.raises(ValidationError): + AnnotationListQuery.model_validate({field: value}) + + # --------------------------------------------------------------------------- # Model and Error Pattern Tests # --------------------------------------------------------------------------- @@ -232,22 +256,55 @@ class TestAnnotationReplyActionStatusApi: class TestAnnotationListApi: - def test_get(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + def test_get_uses_defaults(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0) - monkeypatch.setattr( - AppAnnotationService, - "get_annotation_list_by_app_id", - lambda *_args, **_kwargs: ([annotation], 1), - ) + get_mock = Mock(return_value=([annotation], 1)) + monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock) api = AnnotationListApi() handler = _unwrap(api.get) app_model = SimpleNamespace(id="app") - with app.test_request_context("/apps/annotations?page=1&limit=1", method="GET"): + with app.test_request_context("/apps/annotations", method="GET"): + response = handler(api, app_model=app_model) + + assert response["page"] == 1 + assert response["limit"] == 20 + get_mock.assert_called_once_with("app", 1, 20, "") + + def test_get_accepts_valid_numeric_strings(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0) + get_mock = Mock(return_value=([annotation], 1)) + monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock) + + api = AnnotationListApi() + handler = _unwrap(api.get) + app_model = SimpleNamespace(id="app") + + with app.test_request_context("/apps/annotations?page=2&limit=5&keyword=refund", method="GET"): response = handler(api, app_model=app_model) assert response["total"] == 1 + assert response["page"] == 2 + assert response["limit"] == 5 + get_mock.assert_called_once_with("app", 2, 5, "refund") + + @pytest.mark.parametrize("query_string", ["page=abc&limit=5", "page=1&limit=abc", "page=&limit=5", "limit=0"]) + def test_get_rejects_invalid_explicit_pagination_value( + self, app: Flask, monkeypatch: pytest.MonkeyPatch, query_string: str + ) -> None: + get_mock = Mock(return_value=([], 0)) + monkeypatch.setattr(AppAnnotationService, "get_annotation_list_by_app_id", get_mock) + + api = AnnotationListApi() + handler = _unwrap(api.get) + app_model = SimpleNamespace(id="app") + + with app.test_request_context(f"/apps/annotations?{query_string}", method="GET"): + with pytest.raises(ValidationError): + handler(api, app_model=app_model) + + get_mock.assert_not_called() def test_create(self, app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: annotation = SimpleNamespace(id="a1", question="q", content="a", created_at=0) diff --git a/packages/contracts/generated/api/service/orpc.gen.ts b/packages/contracts/generated/api/service/orpc.gen.ts index 43b2d4402b..2c046d3054 100644 --- a/packages/contracts/generated/api/service/orpc.gen.ts +++ b/packages/contracts/generated/api/service/orpc.gen.ts @@ -24,6 +24,7 @@ import { zGetAppFeedbacksResponse, zGetAppsAnnotationReplyByActionStatusByJobIdPath, zGetAppsAnnotationReplyByActionStatusByJobIdResponse, + zGetAppsAnnotationsQuery, zGetAppsAnnotationsResponse, zGetConversationsByCIdVariablesPath, zGetConversationsByCIdVariablesQuery, @@ -379,6 +380,7 @@ export const get4 = oc summary: 'List annotations for the application', tags: ['service_api'], }) + .input(z.object({ query: zGetAppsAnnotationsQuery.optional() })) .output(zGetAppsAnnotationsResponse) /** diff --git a/packages/contracts/generated/api/service/types.gen.ts b/packages/contracts/generated/api/service/types.gen.ts index cd84f94d81..4e187e7202 100644 --- a/packages/contracts/generated/api/service/types.gen.ts +++ b/packages/contracts/generated/api/service/types.gen.ts @@ -25,6 +25,12 @@ export type AnnotationList = { total: number } +export type AnnotationListQuery = { + keyword?: string + limit?: number + page?: number +} + export type AnnotationReplyActionPayload = { embedding_model_name: string embedding_provider_name: string @@ -969,7 +975,11 @@ export type GetAppsAnnotationReplyByActionStatusByJobIdResponse export type GetAppsAnnotationsData = { body?: never path?: never - query?: never + query?: { + keyword?: string + limit?: number + page?: number + } url: '/apps/annotations' } diff --git a/packages/contracts/generated/api/service/zod.gen.ts b/packages/contracts/generated/api/service/zod.gen.ts index e3008ddfbf..ae7c5cbf6c 100644 --- a/packages/contracts/generated/api/service/zod.gen.ts +++ b/packages/contracts/generated/api/service/zod.gen.ts @@ -32,6 +32,15 @@ export const zAnnotationList = z.object({ total: z.int(), }) +/** + * AnnotationListQuery + */ +export const zAnnotationListQuery = z.object({ + keyword: z.string().optional().default(''), + limit: z.int().gte(1).optional().default(20), + page: z.int().gte(1).optional().default(1), +}) + /** * AnnotationReplyActionPayload */ @@ -1216,6 +1225,12 @@ export const zGetAppsAnnotationReplyByActionStatusByJobIdResponse = z.record( z.unknown(), ) +export const zGetAppsAnnotationsQuery = z.object({ + keyword: z.string().optional().default(''), + limit: z.int().gte(1).optional().default(20), + page: z.int().gte(1).optional().default(1), +}) + /** * Annotations retrieved successfully */