From de7e43a7ef0099548b15086844c99bfa7baa303f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9D=9E=E6=B3=95=E6=93=8D=E4=BD=9C?= Date: Thu, 20 Aug 2026 13:13:43 +0000 Subject: [PATCH] refactor(api): decouple recommended app runtime admission (#40257) --- api/.importlinter | 2 +- api/controllers/console/app/wraps.py | 19 ++- api/controllers/console/explore/error.py | 6 + api/controllers/console/explore/wraps.py | 14 ++- api/extensions/ext_application_services.py | 4 +- api/services/app_service.py | 7 ++ api/services/feature_service.py | 4 + api/services/recommended_app_query_compat.py | 6 + api/services/recommended_app_query_service.py | 20 ++- api/services/recommended_app_service.py | 27 ---- api/tests/unit_tests/.ruff.toml | 1 - .../controllers/console/app/test_wraps.py | 117 +++++++++++------- .../controllers/console/explore/test_wraps.py | 27 ++-- ...test_feature_service_deployment_edition.py | 21 ++++ .../test_recommended_app_query_compat.py | 14 +++ .../test_recommended_app_query_service.py | 56 ++++++--- .../services/test_recommended_app_service.py | 90 -------------- 17 files changed, 217 insertions(+), 218 deletions(-) delete mode 100644 api/services/recommended_app_service.py delete mode 100644 api/tests/unit_tests/services/test_recommended_app_service.py diff --git a/api/.importlinter b/api/.importlinter index fb5b2d729df..6344502ac66 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -264,8 +264,8 @@ forbidden_modules = flask models repositories + services.feature_service services.recommend_app services.recommended_app_query_compat - services.recommended_app_service sqlalchemy werkzeug diff --git a/api/controllers/console/app/wraps.py b/api/controllers/console/app/wraps.py index 04047f86c8b..45e8015f723 100644 --- a/api/controllers/console/app/wraps.py +++ b/api/controllers/console/app/wraps.py @@ -2,7 +2,8 @@ `get_app_model` still supports legacy handlers backed by Flask-SQLAlchemy's scoped session. Trial app handlers compose `get_app_model_with_trial` under -`controllers.common.session.with_session` and always reuse that request session. +`controllers.common.session.with_session`; preview admission finishes before +the request Session loads the accepted App. """ from collections.abc import Callable @@ -16,11 +17,12 @@ from configs import dify_config from controllers.common.session import with_session from controllers.common.wraps import RBACPermission, RBACResourceScope, enforce_rbac_access from controllers.console.app.error import AppNotFoundError +from extensions.ext_application_services import application_services from extensions.ext_database import db from libs.login import current_account_with_tenant -from models import App, AppMode, TrialApp +from models import App, AppMode from models.agent import AgentScope -from services.recommended_app_service import RecommendedAppService +from services.app_service import AppService __all__ = [ "agent_manage_required_for_agent_app", @@ -49,11 +51,10 @@ def _load_app_model_from_scoped_session(app_id: str) -> App | None: def _load_app_model_with_trial(session: Session, app_id: str) -> App | None: - """Load a normal app through its trial registration without applying current-tenant scope.""" - app_model = session.scalar( - select(App).join(TrialApp, TrialApp.app_id == App.id).where(App.id == app_id, App.status == "normal").limit(1) - ) - return app_model + """Load a normal App after preview admission completes outside the request Session.""" + if not application_services().recommended_app_queries.is_previewable(app_id): + return None + return AppService.get_normal_app_by_id(app_id, session) def agent_manage_required_for_agent_app[**P, R](view: Callable[P, R]) -> Callable[P, R]: @@ -220,8 +221,6 @@ def get_app_model_with_trial[**P, R]( if session is None: raise RuntimeError("get_app_model_with_trial requires @with_session") app_model = _load_app_model_with_trial(session, app_id) - if app_model is None: - app_model = RecommendedAppService.get_app(app_id, session=session) if not app_model: raise AppNotFoundError() diff --git a/api/controllers/console/explore/error.py b/api/controllers/console/explore/error.py index 201277f4885..6f0ba500b84 100644 --- a/api/controllers/console/explore/error.py +++ b/api/controllers/console/explore/error.py @@ -57,3 +57,9 @@ class TrialAppLimitExceeded(BaseHTTPException): error_code = "trial_app_limit_exceeded" code = 403 description = "The user has exceeded the trial app limit." + + +class TrialAppFeatureDisabledError(BaseHTTPException): + error_code = "trial_app_feature_disabled" + code = 403 + description = "Trial app feature is not enabled." diff --git a/api/controllers/console/explore/wraps.py b/api/controllers/console/explore/wraps.py index 1f4da57f9aa..a09341f649d 100644 --- a/api/controllers/console/explore/wraps.py +++ b/api/controllers/console/explore/wraps.py @@ -2,19 +2,23 @@ from collections.abc import Callable from functools import wraps from typing import Concatenate -from flask import abort from flask_restx import Resource from sqlalchemy import select from werkzeug.exceptions import NotFound -from controllers.console.explore.error import AppAccessDeniedError, TrialAppLimitExceeded, TrialAppNotAllowed +from controllers.console.explore.error import ( + AppAccessDeniedError, + TrialAppFeatureDisabledError, + TrialAppLimitExceeded, + TrialAppNotAllowed, +) from controllers.console.wraps import account_initialization_required +from extensions.ext_application_services import application_services from extensions.ext_database import db from libs.login import current_account_with_tenant, login_required from models import AccountTrialAppRecord, App, InstalledApp, TrialApp from services.enterprise.enterprise_service import EnterpriseService from services.feature_service import FeatureService -from services.recommended_app_service import RecommendedAppService def installed_app_required[**P, R](view: Callable[Concatenate[InstalledApp, P], R] | None = None): @@ -107,8 +111,8 @@ def trial_app_required[**P, R](view: Callable[Concatenate[App, P], R] | None = N def trial_feature_enable[**P, R](view: Callable[P, R]): @wraps(view) def decorated(*args: P.args, **kwargs: P.kwargs): - if not RecommendedAppService.is_trial_app_enabled(): - abort(403, "Trial app feature is not enabled.") + if not application_services().recommended_app_queries.is_trial_enabled(): + raise TrialAppFeatureDisabledError() return view(*args, **kwargs) return decorated diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index 9faacb78900..42d616053b8 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -51,7 +51,6 @@ from services.file_service import FileService from services.init_validation_service import InitValidationService from services.recommended_app_query_compat import LegacyRecommendedAppCatalogGateway from services.recommended_app_query_service import RecommendedAppQueryService -from services.recommended_app_service import RecommendedAppService from services.schema_definition_service import SchemaDefinitionService from services.setup_adapters import RedisSetupLock, RegisterServiceAccountProvisioner from services.setup_service import SetupService @@ -124,6 +123,7 @@ def build_application_services( data_source_api_key_auth_bindings = SQLAlchemyDataSourceApiKeyAuthBindingRepository(session_factory=database_client) app_definition_repository = AppDefinitionQueryRepository(session_factory=database_client) feature_gateway = FeatureServiceGateway() + trial_app_enabled = FeatureService.is_trial_app_enabled() return ApplicationServices( accounts=AccountServices( profile=AccountProfileService(accounts=SQLAlchemyAccountRepository(database_client)), @@ -186,7 +186,7 @@ def build_application_services( recommended_app_queries=RecommendedAppQueryService( catalog=LegacyRecommendedAppCatalogGateway(session_factory=database_client), trial_apps=TrialAppQueryRepository(session_factory=database_client), - is_trial_enabled=RecommendedAppService.is_trial_app_enabled, + trial_enabled=trial_app_enabled, ), trial_app_usage=TrialAppUsageRepository(session_factory=database_client), workspace_queries=WorkspaceQueryService( diff --git a/api/services/app_service.py b/api/services/app_service.py index b6571937341..fdf356ac1a7 100644 --- a/api/services/app_service.py +++ b/api/services/app_service.py @@ -289,6 +289,13 @@ class AppService: ) -> App | None: return session.get(App, app_id) + @staticmethod + def get_normal_app_by_id( + app_id: str, + session: Session, + ) -> App | None: + return session.scalar(select(App).where(App.id == app_id, App.status == "normal").limit(1)) + @staticmethod def get_visible_app_by_id( app_id: str, diff --git a/api/services/feature_service.py b/api/services/feature_service.py index d1e80a65fd4..840aa4dda06 100644 --- a/api/services/feature_service.py +++ b/api/services/feature_service.py @@ -163,6 +163,10 @@ class FeatureService: def is_webapp_auth_enabled() -> bool: return dify_config.DEPLOYMENT_EDITION == DeploymentEdition.ENTERPRISE + @staticmethod + def is_trial_app_enabled() -> bool: + return dify_config.DEPLOYMENT_EDITION == DeploymentEdition.CLOUD and dify_config.ENABLE_TRIAL_APP + @classmethod def _fulfill_system_params_from_env(cls, system_features: feature_entities.SystemFeatureModel): system_features.enable_email_code_login = dify_config.ENABLE_EMAIL_CODE_LOGIN diff --git a/api/services/recommended_app_query_compat.py b/api/services/recommended_app_query_compat.py index f6ef3ed74aa..468fe185e39 100644 --- a/api/services/recommended_app_query_compat.py +++ b/api/services/recommended_app_query_compat.py @@ -23,6 +23,12 @@ class LegacyRecommendedAppCatalogGateway(RecommendedAppCatalogGateway): def __init__(self, session_factory: sessionmaker[Session]) -> None: self._session_factory = session_factory + @override + def is_recommended(self, app_id: str) -> bool: + retrieval = self._configured_retrieval() + with self._session_factory() as session: + return retrieval.get_recommend_app_detail(app_id, session=session) is not None + @override def list_recommended(self, language: str) -> RecommendedAppCatalogPage: retrieval = self._configured_retrieval() diff --git a/api/services/recommended_app_query_service.py b/api/services/recommended_app_query_service.py index 4efcff4866a..d9ad5d9b9e7 100644 --- a/api/services/recommended_app_query_service.py +++ b/api/services/recommended_app_query_service.py @@ -1,6 +1,6 @@ """Application service for querying the recommended app catalog.""" -from collections.abc import Callable, Sequence, Set +from collections.abc import Sequence, Set from typing import NamedTuple, Protocol from constants.languages import languages @@ -44,6 +44,8 @@ class RecommendedAppDetailRecord(NamedTuple): class RecommendedAppCatalogGateway(Protocol): + def is_recommended(self, app_id: str) -> bool: ... + def list_recommended(self, language: str) -> RecommendedAppCatalogPage: ... def list_builtin(self, language: str) -> RecommendedAppCatalogPage: ... @@ -99,11 +101,19 @@ class RecommendedAppQueryService: *, catalog: RecommendedAppCatalogGateway, trial_apps: TrialAppQuery, - is_trial_enabled: Callable[[], bool], + trial_enabled: bool, ) -> None: self._catalog = catalog self._trial_apps = trial_apps - self._is_trial_enabled = is_trial_enabled + self._trial_enabled = trial_enabled + + def is_trial_enabled(self) -> bool: + return self._trial_enabled + + def is_previewable(self, app_id: str) -> bool: + if app_id in self._trial_apps.existing_ids((app_id,)): + return True + return self._catalog.is_recommended(app_id) def list_recommended( self, @@ -137,7 +147,7 @@ class RecommendedAppQueryService: raise RecommendedAppNotFoundError can_trial = False - if self._is_trial_enabled(): + if self._trial_enabled: can_trial = detail.id in self._trial_apps.existing_ids((detail.id,)) return RecommendedAppDetailSummary( @@ -152,7 +162,7 @@ class RecommendedAppQueryService: def _with_trial_status(self, apps: Sequence[RecommendedAppRecord]) -> tuple[RecommendedAppSummary, ...]: trial_app_ids: Set[str] = set() - if self._is_trial_enabled(): + if self._trial_enabled: trial_app_ids = self._trial_apps.existing_ids([app.app_id for app in apps]) return tuple( diff --git a/api/services/recommended_app_service.py b/api/services/recommended_app_service.py deleted file mode 100644 index 0bc0f8b47df..00000000000 --- a/api/services/recommended_app_service.py +++ /dev/null @@ -1,27 +0,0 @@ -from sqlalchemy import select -from sqlalchemy.orm import Session - -from configs import dify_config -from enums import DeploymentEdition -from models.model import App -from services.recommend_app.recommend_app_factory import RecommendAppRetrievalFactory - - -class RecommendedAppService: - """Own recommended app runtime admission.""" - - @staticmethod - def is_trial_app_enabled() -> bool: - """Return whether trial execution is enabled for this deployment.""" - return dify_config.DEPLOYMENT_EDITION == DeploymentEdition.CLOUD and dify_config.ENABLE_TRIAL_APP - - @classmethod - def get_app(cls, app_id: str, *, session: Session) -> App | None: - """Return a normal app only when it belongs to the recommended catalog.""" - mode = dify_config.HOSTED_FETCH_APP_TEMPLATES_MODE - retrieval_instance = RecommendAppRetrievalFactory.get_recommend_app_factory(mode)() - recommended_app_detail = retrieval_instance.get_recommend_app_detail(app_id, session=session) - if recommended_app_detail is None: - return None - - return session.scalar(select(App).where(App.id == app_id, App.status == "normal").limit(1)) diff --git a/api/tests/unit_tests/.ruff.toml b/api/tests/unit_tests/.ruff.toml index aa04b37fd7c..5255ad121b5 100644 --- a/api/tests/unit_tests/.ruff.toml +++ b/api/tests/unit_tests/.ruff.toml @@ -379,7 +379,6 @@ extend-select = ["ANN401", "ARG"] "services/test_oauth_server_service.py" = ["ARG002"] "services/test_operation_service.py" = ["TID251"] "services/test_rag_pipeline_task_proxy.py" = ["ARG002"] -"services/test_recommended_app_service.py" = ["ARG001"] "services/test_schedule_service.py" = ["ANN401", "TID251"] "services/test_snippet_service.py" = ["ARG001", "ARG002"] "services/test_summary_index_service.py" = ["ARG001"] diff --git a/api/tests/unit_tests/controllers/console/app/test_wraps.py b/api/tests/unit_tests/controllers/console/app/test_wraps.py index 2f94aedaf52..4c29863b5db 100644 --- a/api/tests/unit_tests/controllers/console/app/test_wraps.py +++ b/api/tests/unit_tests/controllers/console/app/test_wraps.py @@ -7,6 +7,7 @@ from unittest.mock import MagicMock from uuid import uuid4 import pytest +from sqlalchemy import event, text from sqlalchemy.orm import Session from controllers.common import session as session_module @@ -15,7 +16,7 @@ from controllers.console.app import completion as completion_module from controllers.console.app import workflow as workflow_module from controllers.console.app import wraps as wraps_module from controllers.console.app.error import AppNotFoundError -from models.model import App, AppMode, TrialApp +from models.model import App, AppMode def _persist_app(sqlite_session: Session, *, mode: AppMode = AppMode.CHAT) -> App: @@ -57,12 +58,48 @@ def test_get_app_model_rejects_wrong_mode(monkeypatch: pytest.MonkeyPatch, sqlit handler(app_id=app_model.id) -def test_get_app_model_with_trial_requires_trial_app_registration( +def test_load_app_model_with_trial_rejects_app_outside_preview_admission( + monkeypatch: pytest.MonkeyPatch, +) -> None: + session = MagicMock(spec=Session) + app_loader = MagicMock() + recommended_app_queries = MagicMock() + recommended_app_queries.is_previewable.return_value = False + monkeypatch.setattr( + wraps_module, + "application_services", + lambda: SimpleNamespace(recommended_app_queries=recommended_app_queries), + ) + monkeypatch.setattr(wraps_module.AppService, "get_normal_app_by_id", app_loader) + + assert wraps_module._load_app_model_with_trial(session, "app-1") is None + recommended_app_queries.is_previewable.assert_called_once_with("app-1") + app_loader.assert_not_called() + + +def test_load_app_model_with_trial_rejects_non_normal_app( monkeypatch: pytest.MonkeyPatch, sqlite_session: Session ) -> None: app_model = _persist_app(sqlite_session) - recommended_get_app = MagicMock(return_value=None) - monkeypatch.setattr(wraps_module.RecommendedAppService, "get_app", recommended_get_app) + app_id = app_model.id + sqlite_session.execute(text("UPDATE apps SET status = 'disabled' WHERE id = :app_id"), {"app_id": app_id}) + sqlite_session.commit() + recommended_app_queries = MagicMock() + recommended_app_queries.is_previewable.return_value = True + monkeypatch.setattr( + wraps_module, + "application_services", + lambda: SimpleNamespace(recommended_app_queries=recommended_app_queries), + ) + + assert wraps_module._load_app_model_with_trial(sqlite_session, app_id) is None + + +def test_get_app_model_with_trial_rejects_app_outside_preview_admission( + monkeypatch: pytest.MonkeyPatch, unbound_session: Session +) -> None: + app_loader = MagicMock(return_value=None) + monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", app_loader) class Handler: @wraps_module.get_app_model_with_trial @@ -70,47 +107,9 @@ def test_get_app_model_with_trial_requires_trial_app_registration( return app_model.id with pytest.raises(AppNotFoundError): - Handler().get(sqlite_session, app_id=app_model.id) + Handler().get(unbound_session, app_id="app-1") - recommended_get_app.assert_called_once_with(app_model.id, session=sqlite_session) - - -def test_get_app_model_with_trial_falls_back_to_recommended_app( - monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal", tenant_id="t1") - trial_app_loader = MagicMock(return_value=None) - recommended_get_app = MagicMock(return_value=app_model) - monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", trial_app_loader) - monkeypatch.setattr(wraps_module.RecommendedAppService, "get_app", recommended_get_app) - - class Handler: - @wraps_module.get_app_model_with_trial - def get(self, _injected_session, app_model): - return app_model.id - - assert Handler().get(unbound_session, app_id="app-1") == "app-1" - trial_app_loader.assert_called_once_with(unbound_session, "app-1") - recommended_get_app.assert_called_once_with("app-1", session=unbound_session) - - -def test_get_app_model_with_trial_prefers_trial_registration( - monkeypatch: pytest.MonkeyPatch, unbound_session: Session -) -> None: - app_model = SimpleNamespace(id="app-1", mode=AppMode.CHAT.value, status="normal", tenant_id="t1") - trial_app_loader = MagicMock(return_value=app_model) - recommended_get_app = MagicMock() - monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", trial_app_loader) - monkeypatch.setattr(wraps_module.RecommendedAppService, "get_app", recommended_get_app) - - class Handler: - @wraps_module.get_app_model_with_trial - def get(self, _injected_session, app_model): - return app_model.id - - assert Handler().get(unbound_session, app_id="app-1") == "app-1" - trial_app_loader.assert_called_once_with(unbound_session, "app-1") - recommended_get_app.assert_not_called() + app_loader.assert_called_once_with(unbound_session, "app-1") def test_get_app_model_requires_app_id() -> None: @@ -145,12 +144,32 @@ def test_get_app_model_prefers_injected_session( assert Handler().get(sqlite_session, app_id=app_model.id) == app_model.id -def test_get_app_model_with_trial_prefers_injected_session( +def test_preview_admission_precedes_request_session_transaction( monkeypatch: pytest.MonkeyPatch, sqlite_session: Session ) -> None: app_model = _persist_app(sqlite_session) - sqlite_session.add(TrialApp(app_id=app_model.id, tenant_id=app_model.tenant_id)) - sqlite_session.commit() + app_id = app_model.id + sqlite_session.rollback() + request_transaction_begins = 0 + + def record_request_transaction_begin(_session, _transaction, _connection) -> None: + nonlocal request_transaction_begins + request_transaction_begins += 1 + + event.listen(sqlite_session, "after_begin", record_request_transaction_begin) + recommended_app_queries = MagicMock() + + def assert_request_session_has_not_started(_app_id: str) -> bool: + assert request_transaction_begins == 0 + assert sqlite_session.in_transaction() is False + return True + + recommended_app_queries.is_previewable.side_effect = assert_request_session_has_not_started + monkeypatch.setattr( + wraps_module, + "application_services", + lambda: SimpleNamespace(recommended_app_queries=recommended_app_queries), + ) monkeypatch.setattr( wraps_module.db, "session", @@ -165,7 +184,9 @@ def test_get_app_model_with_trial_prefers_injected_session( assert injected_session is sqlite_session return app_model.id - assert Handler().get(app_id=app_model.id) == app_model.id + assert Handler().get(app_id=app_id) == app_id + recommended_app_queries.is_previewable.assert_called_once_with(app_id) + assert request_transaction_begins == 1 def test_get_app_model_with_trial_requires_injected_session() -> None: diff --git a/api/tests/unit_tests/controllers/console/explore/test_wraps.py b/api/tests/unit_tests/controllers/console/explore/test_wraps.py index a1da7916f02..fee341593d7 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_wraps.py +++ b/api/tests/unit_tests/controllers/console/explore/test_wraps.py @@ -1,15 +1,16 @@ from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import MagicMock, patch from uuid import uuid4 import pytest from sqlalchemy.orm import Session, scoped_session -from werkzeug.exceptions import Forbidden, NotFound +from werkzeug.exceptions import NotFound import controllers.console.explore.wraps as wraps_module import models.model as model_module from controllers.console.explore.error import ( AppAccessDeniedError, + TrialAppFeatureDisabledError, TrialAppLimitExceeded, TrialAppNotAllowed, ) @@ -260,23 +261,27 @@ def test_trial_feature_enable_disabled(): def view(): return "ok" - with patch( - "controllers.console.explore.wraps.RecommendedAppService.is_trial_app_enabled", - return_value=False, - ): - with pytest.raises(Forbidden): + services = MagicMock() + services.recommended_app_queries.is_trial_enabled.return_value = False + with patch("controllers.console.explore.wraps.application_services", return_value=services): + with pytest.raises(TrialAppFeatureDisabledError) as exc_info: view() + assert exc_info.value.data == { + "code": "trial_app_feature_disabled", + "message": "Trial app feature is not enabled.", + "status": 403, + } + def test_trial_feature_enable_enabled(): @trial_feature_enable def view(): return "ok" - with patch( - "controllers.console.explore.wraps.RecommendedAppService.is_trial_app_enabled", - return_value=True, - ): + services = MagicMock() + services.recommended_app_queries.is_trial_enabled.return_value = True + with patch("controllers.console.explore.wraps.application_services", return_value=services): assert view() == "ok" diff --git a/api/tests/unit_tests/services/test_feature_service_deployment_edition.py b/api/tests/unit_tests/services/test_feature_service_deployment_edition.py index 63ebb75c4ba..e0a9bd68685 100644 --- a/api/tests/unit_tests/services/test_feature_service_deployment_edition.py +++ b/api/tests/unit_tests/services/test_feature_service_deployment_edition.py @@ -43,3 +43,24 @@ def test_get_system_features_uses_configured_deployment_edition( fulfill_from_enterprise.assert_called_once_with(result) else: fulfill_from_enterprise.assert_not_called() + + +@pytest.mark.parametrize( + ("edition", "feature_enabled", "expected"), + [ + (DeploymentEdition.CLOUD, True, True), + (DeploymentEdition.CLOUD, False, False), + (DeploymentEdition.COMMUNITY, True, False), + (DeploymentEdition.ENTERPRISE, True, False), + ], +) +def test_trial_app_policy_is_cloud_only( + monkeypatch: pytest.MonkeyPatch, + edition: DeploymentEdition, + feature_enabled: bool, + expected: bool, +) -> None: + monkeypatch.setattr("services.feature_service.dify_config.DEPLOYMENT_EDITION", edition) + monkeypatch.setattr("services.feature_service.dify_config.ENABLE_TRIAL_APP", feature_enabled) + + assert FeatureService.is_trial_app_enabled() is expected diff --git a/api/tests/unit_tests/services/test_recommended_app_query_compat.py b/api/tests/unit_tests/services/test_recommended_app_query_compat.py index 82e75a2ba1e..f1bf8c8f15c 100644 --- a/api/tests/unit_tests/services/test_recommended_app_query_compat.py +++ b/api/tests/unit_tests/services/test_recommended_app_query_compat.py @@ -33,6 +33,20 @@ def gateway_dependencies( return LegacyRecommendedAppCatalogGateway(session_factory), session, retrieval, get_factory +@pytest.mark.parametrize(("detail", "expected"), [(object(), True), (None, False)]) +def test_is_recommended_uses_configured_retrieval_without_mapping_detail( + detail: object | None, + expected: bool, + gateway_dependencies: tuple[LegacyRecommendedAppCatalogGateway, MagicMock, MagicMock, MagicMock], +) -> None: + gateway, session, retrieval, get_factory = gateway_dependencies + retrieval.get_recommend_app_detail.return_value = detail + + assert gateway.is_recommended("app-1") is expected + get_factory.assert_called_once_with("remote") + retrieval.get_recommend_app_detail.assert_called_once_with("app-1", session=session) + + @pytest.mark.parametrize("app_source_kind", ["mapping", "orm"]) def test_list_recommended_selects_configured_retrieval_and_maps_mixed_results( app_source_kind: str, diff --git a/api/tests/unit_tests/services/test_recommended_app_query_service.py b/api/tests/unit_tests/services/test_recommended_app_query_service.py index 443a739a481..d035c5edb78 100644 --- a/api/tests/unit_tests/services/test_recommended_app_query_service.py +++ b/api/tests/unit_tests/services/test_recommended_app_query_service.py @@ -45,21 +45,43 @@ def _service( *, catalog: MagicMock, trial_apps: MagicMock | None = None, - trial_enabled: MagicMock | None = None, -) -> tuple[RecommendedAppQueryService, MagicMock, MagicMock]: + trial_enabled: bool = False, +) -> tuple[RecommendedAppQueryService, MagicMock]: trial_apps = trial_apps or MagicMock() - trial_enabled = trial_enabled or MagicMock(return_value=False) return ( RecommendedAppQueryService( catalog=catalog, trial_apps=trial_apps, - is_trial_enabled=trial_enabled, + trial_enabled=trial_enabled, ), trial_apps, - trial_enabled, ) +def test_is_previewable_accepts_trial_registration_without_querying_catalog() -> None: + catalog = MagicMock() + trial_apps = MagicMock() + trial_apps.existing_ids.return_value = frozenset({"app-1"}) + service, _ = _service(catalog=catalog, trial_apps=trial_apps) + + assert service.is_previewable("app-1") is True + trial_apps.existing_ids.assert_called_once_with(("app-1",)) + catalog.is_recommended.assert_not_called() + + +@pytest.mark.parametrize("expected", [True, False]) +def test_is_previewable_falls_back_to_catalog(expected: bool) -> None: + catalog = MagicMock() + catalog.is_recommended.return_value = expected + trial_apps = MagicMock() + trial_apps.existing_ids.return_value = frozenset() + service, _ = _service(catalog=catalog, trial_apps=trial_apps) + + assert service.is_previewable("app-1") is expected + trial_apps.existing_ids.assert_called_once_with(("app-1",)) + catalog.is_recommended.assert_called_once_with("app-1") + + @pytest.mark.parametrize( ("requested_language", "interface_language", "expected"), [ @@ -76,7 +98,7 @@ def test_list_recommended_resolves_language( ) -> None: catalog = MagicMock() catalog.list_recommended.return_value = _page("app-1") - service, _, _ = _service(catalog=catalog) + service, _ = _service(catalog=catalog) service.list_recommended( requested_language=requested_language, @@ -90,7 +112,7 @@ def test_list_recommended_falls_back_to_builtin_en_us_when_empty() -> None: catalog = MagicMock() catalog.list_recommended.return_value = _page(categories=("remote",)) catalog.list_builtin.return_value = _page("builtin-app", categories=("builtin",)) - service, _, _ = _service(catalog=catalog) + service, _ = _service(catalog=catalog) result = service.list_recommended(requested_language="ja-JP", interface_language=None) @@ -102,7 +124,7 @@ def test_list_recommended_falls_back_to_builtin_en_us_when_empty() -> None: def test_list_recommended_disables_upstream_trial_without_querying_trial_apps() -> None: catalog = MagicMock() catalog.list_recommended.return_value = _page("app-1") - service, trial_apps, _ = _service(catalog=catalog) + service, trial_apps = _service(catalog=catalog) result = service.list_recommended(requested_language="en-US", interface_language=None) @@ -115,10 +137,10 @@ def test_list_recommended_enriches_trial_status_in_one_bulk_query() -> None: catalog.list_recommended.return_value = _page("app-1", "app-2") trial_apps = MagicMock() trial_apps.existing_ids.return_value = frozenset({"app-1"}) - service, _, _ = _service( + service, _ = _service( catalog=catalog, trial_apps=trial_apps, - trial_enabled=MagicMock(return_value=True), + trial_enabled=True, ) result = service.list_recommended(requested_language="en-US", interface_language=None) @@ -130,7 +152,7 @@ def test_list_recommended_enriches_trial_status_in_one_bulk_query() -> None: def test_list_learn_dify_does_not_apply_general_builtin_fallback_or_return_categories() -> None: catalog = MagicMock() catalog.list_learn_dify.return_value = _page(categories=("ignored",)) - service, _, _ = _service(catalog=catalog) + service, _ = _service(catalog=catalog) result = service.list_learn_dify(requested_language="invalid", interface_language="fr-FR") @@ -140,15 +162,13 @@ def test_list_learn_dify_does_not_apply_general_builtin_fallback_or_return_categ assert not hasattr(result, "categories") -def test_get_detail_raises_not_found_before_reading_trial_policy() -> None: +def test_get_detail_raises_not_found_without_querying_trial_apps() -> None: catalog = MagicMock() catalog.get_detail.return_value = None - trial_enabled = MagicMock(side_effect=AssertionError("missing detail must not inspect trial policy")) - service, trial_apps, _ = _service(catalog=catalog, trial_enabled=trial_enabled) + service, trial_apps = _service(catalog=catalog, trial_enabled=True) with pytest.raises(RecommendedAppNotFoundError): service.get_detail("missing") - trial_enabled.assert_not_called() trial_apps.existing_ids.assert_not_called() @@ -162,7 +182,7 @@ def test_get_detail_does_not_query_trial_apps_when_disabled() -> None: mode="chat", export_data="{}", ) - service, trial_apps, _ = _service(catalog=catalog) + service, trial_apps = _service(catalog=catalog) result = service.get_detail("route-app-id") @@ -183,10 +203,10 @@ def test_get_detail_uses_catalog_result_id_for_trial_status(existing_ids: frozen ) trial_apps = MagicMock() trial_apps.existing_ids.return_value = existing_ids - service, _, _ = _service( + service, _ = _service( catalog=catalog, trial_apps=trial_apps, - trial_enabled=MagicMock(return_value=True), + trial_enabled=True, ) result = service.get_detail("route-app-id") diff --git a/api/tests/unit_tests/services/test_recommended_app_service.py b/api/tests/unit_tests/services/test_recommended_app_service.py deleted file mode 100644 index 0e8d279dce2..00000000000 --- a/api/tests/unit_tests/services/test_recommended_app_service.py +++ /dev/null @@ -1,90 +0,0 @@ -"""Unit tests for the remaining recommended app command/runtime service.""" - -import uuid -from collections.abc import Callable -from unittest.mock import MagicMock - -import pytest -from sqlalchemy.orm import Session - -from enums import DeploymentEdition -from models.model import App, AppMode -from services import recommended_app_service as service_module -from services.recommended_app_service import RecommendedAppService - - -@pytest.fixture(autouse=True) -def _recommended_app_config(config_overrides: Callable[..., None]) -> None: - config_overrides(HOSTED_FETCH_APP_TEMPLATES_MODE="remote") - - -@pytest.mark.parametrize( - ("edition", "feature_enabled", "expected"), - [ - (DeploymentEdition.CLOUD, True, True), - (DeploymentEdition.CLOUD, False, False), - (DeploymentEdition.COMMUNITY, True, False), - (DeploymentEdition.ENTERPRISE, True, False), - ], -) -def test_trial_app_policy_is_cloud_only( - config_overrides: Callable[..., None], - edition: DeploymentEdition, - feature_enabled: bool, - expected: bool, -) -> None: - config_overrides(DEPLOYMENT_EDITION=edition, ENABLE_TRIAL_APP=feature_enabled) - - assert RecommendedAppService.is_trial_app_enabled() is expected - - -def _persist_app(session: Session, *, name: str) -> App: - app = App( - tenant_id=str(uuid.uuid4()), - name=name, - mode=AppMode.CHAT, - enable_site=True, - enable_api=True, - ) - app.id = str(uuid.uuid4()) - session.add(app) - session.commit() - return app - - -def _configure_recommended_detail( - monkeypatch: pytest.MonkeyPatch, - *, - result: dict[str, str] | None, -) -> MagicMock: - retrieval = MagicMock() - retrieval.get_recommend_app_detail.return_value = result - retrieval_type = MagicMock(return_value=retrieval) - monkeypatch.setattr( - service_module.RecommendAppRetrievalFactory, - "get_recommend_app_factory", - MagicMock(return_value=retrieval_type), - ) - return retrieval - - -def test_get_app_returns_normal_recommended_app(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> None: - app = _persist_app(sqlite_session, name="Recommended App") - retrieval = _configure_recommended_detail(monkeypatch, result={"id": app.id}) - - result = RecommendedAppService.get_app(app.id, session=sqlite_session) - - assert result is app - retrieval.get_recommend_app_detail.assert_called_once_with(app.id, session=sqlite_session) - - -def test_get_app_returns_none_when_app_is_not_recommended( - monkeypatch: pytest.MonkeyPatch, sqlite_session: Session -) -> None: - app = _persist_app(sqlite_session, name="Private App") - retrieval = _configure_recommended_detail(monkeypatch, result=None) - - result = RecommendedAppService.get_app(app.id, session=sqlite_session) - - assert result is None - retrieval.get_recommend_app_detail.assert_called_once_with(app.id, session=sqlite_session)