From eec092f93d598613045a4863aa499b12a3c07a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Fri, 14 Aug 2026 09:38:35 +0000 Subject: [PATCH] refactor(api): reuse app definition queries for service API info (#40476) --- api/controllers/service_api/app/app.py | 13 +- .../app_definition_query_repository.py | 22 ++- api/services/app_definition_query_service.py | 16 ++ .../controllers/service_api/app/test_app.py | 184 ++++-------------- .../test_app_definition_query_repository.py | 113 ++++++++++- .../test_app_definition_query_service.py | 18 ++ 6 files changed, 209 insertions(+), 157 deletions(-) diff --git a/api/controllers/service_api/app/app.py b/api/controllers/service_api/app/app.py index ff0db136fcc..9fd70f97917 100644 --- a/api/controllers/service_api/app/app.py +++ b/api/controllers/service_api/app/app.py @@ -136,11 +136,8 @@ class AppInfoApi(Resource): Returns basic information about the application including name, description, tags, and mode. """ - tags = [tag.name for tag in app_model.tags] - return { - "name": app_model.name, - "description": app_model.description, - "tags": tags, - "mode": app_model.mode, - "author_name": app_model.author_name, - } + try: + summary = application_services().app_definitions.get_summary(app_model.id) + except AppDefinitionUnavailableError: + raise AppUnavailableError() from None + return dump_response(AppInfoResponse, summary) diff --git a/api/repositories/app_definition_query_repository.py b/api/repositories/app_definition_query_repository.py index 9fa4e6014aa..f05db4b654b 100644 --- a/api/repositories/app_definition_query_repository.py +++ b/api/repositories/app_definition_query_repository.py @@ -14,7 +14,12 @@ from models.agent_config_entities import AgentSoulConfig from models.model import App, AppMode, AppModelConfig, load_annotation_reply_config from models.tools import ApiToolProvider from models.workflow import Workflow -from services.app_definition_query_service import AppDefinitionQuery, AppParameterConfig, AppToolIconSource +from services.app_definition_query_service import ( + AppDefinitionQuery, + AppDefinitionSummary, + AppParameterConfig, + AppToolIconSource, +) def _get_public_agent_parameter_config(app: App, *, session: Session) -> AppParameterConfig: @@ -125,6 +130,21 @@ class AppDefinitionQueryRepository(AppDefinitionQuery): return tuple(records) + @override + def get_summary(self, app_id: str) -> AppDefinitionSummary | None: + with self._session_factory() as session: + app = session.get(App, app_id) + if app is None: + return None + + return AppDefinitionSummary( + name=app.name, + description=app.description, + tags=tuple(tag.name for tag in app.tags_with_session(session=session)), + mode=app.mode.value, + author_name=app.author_name_with_session(session=session), + ) + @staticmethod def _get_tools(session: Session, app: App) -> list[dict[str, Any]]: if app.mode in {AppMode.ADVANCED_CHAT, AppMode.WORKFLOW}: diff --git a/api/services/app_definition_query_service.py b/api/services/app_definition_query_service.py index 14a5cb8cf9b..823ea646f6b 100644 --- a/api/services/app_definition_query_service.py +++ b/api/services/app_definition_query_service.py @@ -20,6 +20,14 @@ class AppToolIconSource(NamedTuple): provider_icon: str | None +class AppDefinitionSummary(NamedTuple): + name: str + description: str | None + tags: tuple[str, ...] + mode: str + author_name: str | None + + class AppDefinitionQuery(Protocol): def get_published_parameter_config( self, @@ -30,6 +38,8 @@ class AppDefinitionQuery(Protocol): def get_tool_icon_sources(self, app_id: str) -> Sequence[AppToolIconSource] | None: ... + def get_summary(self, app_id: str) -> AppDefinitionSummary | None: ... + class AppDefinitionUnavailableError(ValueError): """Raised when an app definition is unavailable.""" @@ -95,3 +105,9 @@ class AppDefinitionQueryService: tool_icons[tool.tool_name] = _API_TOOL_FALLBACK_ICON.copy() return tool_icons + + def get_summary(self, app_id: str) -> AppDefinitionSummary: + summary = self._definitions.get_summary(app_id) + if summary is None: + raise AppDefinitionUnavailableError("App not found") + return summary diff --git a/api/tests/unit_tests/controllers/service_api/app/test_app.py b/api/tests/unit_tests/controllers/service_api/app/test_app.py index fbf4e88e83a..69169e9174c 100644 --- a/api/tests/unit_tests/controllers/service_api/app/test_app.py +++ b/api/tests/unit_tests/controllers/service_api/app/test_app.py @@ -1,13 +1,9 @@ """SQLite-backed tests for Service API application controllers. The authentication decorator resolves the app, tenant, and tenant owner before -the controller runs. Controller/model code then reads tags and author information -through model database properties. Tests bind those references to one explicit -scoped SQLite session and persist visibility and cross-tenant decoys instead of -fabricating ORM lookup results. +the controller delegates application queries to the App Definition service. """ -import json from collections.abc import Iterator from dataclasses import dataclass from types import SimpleNamespace @@ -26,26 +22,16 @@ from controllers.service_api.app.error import AgentNotPublishedError, AppUnavail from core.app.app_config.common.parameters_mapping import get_parameters_from_feature_dict from models.account import Account, Tenant, TenantAccountJoin, TenantAccountRole, TenantStatus from models.base import TypeBase -from models.enums import EndUserType -from models.model import ( - App, - AppAnnotationSetting, - AppMode, - AppModelConfig, - CustomizeTokenStrategy, - EndUser, - Site, - Tag, - TagBinding, - TagType, +from models.model import App, AppMode +from services.app_definition_query_service import ( + AppDefinitionNotPublishedError, + AppDefinitionSummary, + AppDefinitionUnavailableError, ) -from models.workflow import Workflow, WorkflowType -from services.app_definition_query_service import AppDefinitionNotPublishedError, AppDefinitionUnavailableError @dataclass(frozen=True) class _DatabaseBinding: - engine: Engine session: scoped_session[Session] @@ -57,15 +43,11 @@ class _Token: @dataclass(frozen=True) class AppDatabase: - """Persisted application graph used by the decorated controller methods.""" + """Persisted authentication state used by the decorated controller methods.""" session_maker: sessionmaker[Session] - registry: scoped_session[Session] tenant_id: str app_id: str - owner_id: str - config_id: str - workflow_id: str def update_app(self, **values: object) -> None: with self.session_maker.begin() as session: @@ -84,30 +66,6 @@ class AppDatabase: with self.session_maker.begin() as session: session.execute(table.delete().where(table.c.id == object_id)) - def replace_tags(self, *names: str) -> None: - """Replace the visible app's tenant-owned tag bindings with persisted tags.""" - with self.session_maker.begin() as session: - session.execute( - TagBinding.__table__.delete().where( - TagBinding.tenant_id == self.tenant_id, - TagBinding.target_id == self.app_id, - ) - ) - tags = [ - Tag(tenant_id=self.tenant_id, type=TagType.APP, name=name, created_by=self.owner_id) for name in names - ] - session.add_all(tags) - session.flush() - session.add_all( - TagBinding( - tenant_id=self.tenant_id, - tag_id=tag.id, - target_id=self.app_id, - created_by=self.owner_id, - ) - for tag in tags - ) - @pytest.fixture def flask_app() -> Flask: @@ -118,40 +76,26 @@ def flask_app() -> Flask: @pytest.fixture def app_db(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Iterator[AppDatabase]: - """Create the minimal controller/model schema and bind every DB reference explicitly.""" + """Create the minimal authentication schema and bind its database reference.""" tables = [ Tenant.__table__, Account.__table__, TenantAccountJoin.__table__, App.__table__, - AppModelConfig.__table__, - AppAnnotationSetting.__table__, - Workflow.__table__, - Tag.__table__, - TagBinding.__table__, - Site.__table__, - EndUser.__table__, ] TypeBase.metadata.create_all(sqlite_engine, tables=tables) maker = sessionmaker(bind=sqlite_engine, expire_on_commit=False) registry = scoped_session(maker) - binding = _DatabaseBinding(engine=sqlite_engine, session=registry) + binding = _DatabaseBinding(session=registry) monkeypatch.setattr("controllers.service_api.wraps.db", binding) - monkeypatch.setattr("models.model.db", binding) - monkeypatch.setattr("models.account.db", binding) tenant_id = str(uuid4()) app_id = str(uuid4()) owner_id = str(uuid4()) - config_id = str(uuid4()) - workflow_id = str(uuid4()) - other_tenant_id = str(uuid4()) with maker.begin() as session: tenant = Tenant(name="Visible tenant") tenant.id = tenant_id - other_tenant = Tenant(name="Other tenant") - other_tenant.id = other_tenant_id owner = Account(name="Test Author", email="owner@example.com") owner.id = owner_id app = App( @@ -163,37 +107,12 @@ def app_db(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Iterator[A icon_type=None, icon=None, icon_background=None, - app_model_config_id=config_id, - workflow_id=workflow_id, enable_site=True, enable_api=True, max_active_requests=None, created_by=owner_id, ) - config = AppModelConfig( - app_id=app_id, - opening_statement="Hello", - suggested_questions=json.dumps(["Question?"]), - user_input_form=json.dumps([{"text-input": {"label": "Name", "variable": "name", "required": True}}]), - ) - config.id = config_id - workflow = Workflow.new( - tenant_id=tenant_id, - app_id=app_id, - type=WorkflowType.WORKFLOW.value, - version="1", - graph=json.dumps({"nodes": [{"id": "start", "data": {"type": "start", "variables": []}}]}), - features=json.dumps({"suggested_questions": []}), - created_by=owner_id, - environment_variables=[], - conversation_variables=[], - rag_pipeline_variables=[], - ) - workflow.id = workflow_id - target_tag = Tag(tenant_id=tenant_id, type=TagType.APP, name="test-tag", created_by=owner_id) - other_tag = Tag(tenant_id=other_tenant_id, type=TagType.APP, name="foreign-tag", created_by=owner_id) - session.add_all([tenant, other_tenant, owner, app, config, workflow, target_tag, other_tag]) - session.flush() + session.add_all([tenant, owner, app]) session.add_all( [ TenantAccountJoin( @@ -202,49 +121,13 @@ def app_db(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Iterator[A current=True, role=TenantAccountRole.OWNER, ), - TagBinding( - tenant_id=tenant_id, - tag_id=target_tag.id, - target_id=app_id, - created_by=owner_id, - ), - # Same target ID but another tenant: App.tags must exclude it. - TagBinding( - tenant_id=other_tenant_id, - tag_id=other_tag.id, - target_id=app_id, - created_by=owner_id, - ), - Site( - app_id=app_id, - title="Published site", - icon_type=None, - icon=None, - icon_background=None, - description="Site decoy", - default_language="en-US", - customize_token_strategy=CustomizeTokenStrategy.MUST, - code="visible-site", - ), - EndUser( - tenant_id=other_tenant_id, - app_id=app_id, - type=EndUserType.BROWSER, - name="Cross-tenant visitor", - is_anonymous=False, - session_id="visitor-session", - ), ] ) database = AppDatabase( session_maker=maker, - registry=registry, tenant_id=tenant_id, app_id=app_id, - owner_id=owner_id, - config_id=config_id, - workflow_id=workflow_id, ) try: yield database @@ -357,43 +240,54 @@ def test_get_meta_maps_unavailable_definition_to_app_unavailable( } -@pytest.mark.parametrize("mode", [AppMode.CHAT, AppMode.COMPLETION, AppMode.WORKFLOW, AppMode.ADVANCED_CHAT]) -def test_get_info_reads_author_and_tenant_scoped_tags( +def test_get_info_queries_authenticated_app( flask_app: Flask, authenticated_controller: AppDatabase, - mode: AppMode, + monkeypatch: pytest.MonkeyPatch, ) -> None: - authenticated_controller.update_app(mode=mode) + app_definitions = Mock() + app_definitions.get_summary.return_value = AppDefinitionSummary( + name="Test App", + description="A test application", + tags=("test-tag",), + mode=AppMode.CHAT.value, + author_name="Test Author", + ) + monkeypatch.setattr( + app_controller, + "application_services", + Mock(return_value=SimpleNamespace(app_definitions=app_definitions)), + ) with flask_app.test_request_context("/info", headers={"Authorization": "Bearer token"}): response = AppInfoApi().get() + app_definitions.get_summary.assert_called_once_with(authenticated_controller.app_id) assert response == { "name": "Test App", "description": "A test application", "tags": ["test-tag"], - "mode": mode, + "mode": AppMode.CHAT.value, "author_name": "Test Author", } -@pytest.mark.parametrize( - "tag_names", - [(), ("tag-one", "tag-two", "tag-three")], - ids=["zero-tags", "multiple-tags"], -) -def test_get_info_handles_zero_or_multiple_tags( +@pytest.mark.usefixtures("authenticated_controller") +def test_get_info_maps_unavailable_app( flask_app: Flask, - authenticated_controller: AppDatabase, - tag_names: tuple[str, ...], + monkeypatch: pytest.MonkeyPatch, ) -> None: - authenticated_controller.replace_tags(*tag_names) + app_definitions = Mock() + app_definitions.get_summary.side_effect = AppDefinitionUnavailableError() + monkeypatch.setattr( + app_controller, + "application_services", + Mock(return_value=SimpleNamespace(app_definitions=app_definitions)), + ) with flask_app.test_request_context("/info", headers={"Authorization": "Bearer token"}): - response = AppInfoApi().get() - - assert len(response["tags"]) == len(tag_names) - assert set(response["tags"]) == set(tag_names) + with pytest.raises(AppUnavailableError): + AppInfoApi().get() @pytest.mark.parametrize("state", ["missing", "disabled", "archived", "ownerless"]) diff --git a/api/tests/unit_tests/repositories/test_app_definition_query_repository.py b/api/tests/unit_tests/repositories/test_app_definition_query_repository.py index 000ee3f8103..b6b7c529a06 100644 --- a/api/tests/unit_tests/repositories/test_app_definition_query_repository.py +++ b/api/tests/unit_tests/repositories/test_app_definition_query_repository.py @@ -4,11 +4,13 @@ import pytest from sqlalchemy.orm import Session, sessionmaker from core.tools.entities.tool_entities import ApiProviderSchemaType -from models.model import App, AppMode, AppModelConfig +from models.account import Account +from models.enums import TagType +from models.model import App, AppMode, AppModelConfig, Tag, TagBinding from models.tools import ApiToolProvider from models.workflow import Workflow, WorkflowKind, WorkflowType from repositories.app_definition_query_repository import AppDefinitionQueryRepository -from services.app_definition_query_service import AppParameterConfig, AppToolIconSource +from services.app_definition_query_service import AppDefinitionSummary, AppParameterConfig, AppToolIconSource _APP_ID = "11111111-1111-1111-1111-111111111111" _TENANT_ID = "22222222-2222-2222-2222-222222222222" @@ -16,9 +18,15 @@ _ACCOUNT_ID = "33333333-3333-3333-3333-333333333333" _WORKFLOW_ID = "44444444-4444-4444-4444-444444444444" _PROVIDER_ID = "55555555-5555-5555-5555-555555555555" _MISSING_PROVIDER_ID = "66666666-6666-6666-6666-666666666666" +_OTHER_TENANT_ID = "77777777-7777-7777-7777-777777777777" -def _persist_app(session: Session, *, mode: AppMode = AppMode.CHAT) -> App: +def _persist_app( + session: Session, + *, + mode: AppMode = AppMode.CHAT, + created_by: str | None = None, +) -> App: app = App( id=_APP_ID, tenant_id=_TENANT_ID, @@ -32,6 +40,7 @@ def _persist_app(session: Session, *, mode: AppMode = AppMode.CHAT) -> App: enable_api=True, is_public=True, max_active_requests=None, + created_by=created_by, ) session.add(app) session.flush() @@ -183,6 +192,104 @@ def test_get_public_parameter_config_reuses_standard_projection( assert result.features_dict["opening_statement"] == "Public config" +def test_get_summary_returns_none_for_missing_app(sqlite_session_factory: sessionmaker[Session]) -> None: + repository = AppDefinitionQueryRepository(session_factory=sqlite_session_factory) + + assert repository.get_summary(_APP_ID) is None + + +def test_get_summary_maps_app_mode_and_author(sqlite_session_factory: sessionmaker[Session]) -> None: + with sqlite_session_factory.begin() as session: + _persist_app(session, mode=AppMode.WORKFLOW, created_by=_ACCOUNT_ID) + account = Account(name="Test Author", email="owner@example.com") + account.id = _ACCOUNT_ID + session.add(account) + + result = AppDefinitionQueryRepository(session_factory=sqlite_session_factory).get_summary(_APP_ID) + + assert result == AppDefinitionSummary( + name="Parameter app", + description="", + tags=(), + mode=AppMode.WORKFLOW.value, + author_name="Test Author", + ) + + +def test_get_summary_returns_only_tenant_scoped_app_tags( + sqlite_session_factory: sessionmaker[Session], +) -> None: + with sqlite_session_factory.begin() as session: + app = _persist_app(session) + visible = Tag(tenant_id=_TENANT_ID, type=TagType.APP, name="visible", created_by=_ACCOUNT_ID) + visible_second = Tag( + tenant_id=_TENANT_ID, + type=TagType.APP, + name="visible-second", + created_by=_ACCOUNT_ID, + ) + foreign_tag = Tag( + tenant_id=_OTHER_TENANT_ID, + type=TagType.APP, + name="foreign-tag", + created_by=_ACCOUNT_ID, + ) + foreign_binding = Tag( + tenant_id=_TENANT_ID, + type=TagType.APP, + name="foreign-binding", + created_by=_ACCOUNT_ID, + ) + knowledge = Tag( + tenant_id=_TENANT_ID, + type=TagType.KNOWLEDGE, + name="knowledge", + created_by=_ACCOUNT_ID, + ) + session.add_all([visible, visible_second, foreign_tag, foreign_binding, knowledge]) + session.flush() + session.add_all( + [ + TagBinding( + tenant_id=_TENANT_ID, + tag_id=visible.id, + target_id=app.id, + created_by=_ACCOUNT_ID, + ), + TagBinding( + tenant_id=_TENANT_ID, + tag_id=visible_second.id, + target_id=app.id, + created_by=_ACCOUNT_ID, + ), + TagBinding( + tenant_id=_TENANT_ID, + tag_id=foreign_tag.id, + target_id=app.id, + created_by=_ACCOUNT_ID, + ), + TagBinding( + tenant_id=_OTHER_TENANT_ID, + tag_id=foreign_binding.id, + target_id=app.id, + created_by=_ACCOUNT_ID, + ), + TagBinding( + tenant_id=_TENANT_ID, + tag_id=knowledge.id, + target_id=app.id, + created_by=_ACCOUNT_ID, + ), + ] + ) + + result = AppDefinitionQueryRepository(session_factory=sqlite_session_factory).get_summary(_APP_ID) + + assert result is not None + assert set(result.tags) == {"visible", "visible-second"} + assert result.author_name is None + + def _tool(provider_type: str, provider_id: str, tool_name: str) -> dict[str, object]: return { "provider_type": provider_type, diff --git a/api/tests/unit_tests/services/test_app_definition_query_service.py b/api/tests/unit_tests/services/test_app_definition_query_service.py index fc33dbacdb8..de0ac4c1c65 100644 --- a/api/tests/unit_tests/services/test_app_definition_query_service.py +++ b/api/tests/unit_tests/services/test_app_definition_query_service.py @@ -8,6 +8,7 @@ from services.app_definition_query_service import ( AppDefinitionNotPublishedError, AppDefinitionQuery, AppDefinitionQueryService, + AppDefinitionSummary, AppDefinitionUnavailableError, AppParameterConfig, AppToolIconSource, @@ -127,3 +128,20 @@ def test_get_tool_icons_rejects_missing_app() -> None: with pytest.raises(AppDefinitionUnavailableError, match="App not found"): service.get_tool_icons("missing") + + +def test_get_summary_returns_repository_record() -> None: + service, definitions = _service() + summary = AppDefinitionSummary("Test App", "A test application", ("tag",), "chat", "Test Author") + definitions.get_summary.return_value = summary + + assert service.get_summary("app-1") == summary + definitions.get_summary.assert_called_once_with("app-1") + + +def test_get_summary_rejects_missing_app() -> None: + service, definitions = _service() + definitions.get_summary.return_value = None + + with pytest.raises(AppDefinitionUnavailableError, match="App not found"): + service.get_summary("missing")