From d86b8240101793800cfc0308e107a25313bfe421 Mon Sep 17 00:00:00 2001 From: hjlarry Date: Sun, 9 Aug 2026 14:40:54 +0800 Subject: [PATCH] refactor(api): decouple recommended app runtime admission --- api/.importlinter | 2 +- api/controllers/console/app/wraps.py | 12 ++- api/controllers/console/explore/wraps.py | 3 +- api/extensions/ext_application_services.py | 3 +- 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 | 5 ++ api/services/recommended_app_service.py | 27 ------ api/tests/unit_tests/.ruff.toml | 1 - .../controllers/console/app/test_wraps.py | 71 +++++++++++++-- .../controllers/console/explore/test_wraps.py | 4 +- ...test_feature_service_deployment_edition.py | 23 +++++ .../test_recommended_app_query_compat.py | 14 +++ .../test_recommended_app_query_service.py | 12 +++ .../services/test_recommended_app_service.py | 87 ------------------- 16 files changed, 148 insertions(+), 133 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 778e21952d1e87..8328591c1055e6 100644 --- a/api/.importlinter +++ b/api/.importlinter @@ -138,9 +138,9 @@ 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 04047f86c8b17f..2ce1d4dfac91e3 100644 --- a/api/controllers/console/app/wraps.py +++ b/api/controllers/console/app/wraps.py @@ -16,11 +16,12 @@ 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.agent import AgentScope -from services.recommended_app_service import RecommendedAppService +from services.app_service import AppService __all__ = [ "agent_manage_required_for_agent_app", @@ -56,6 +57,13 @@ def _load_app_model_with_trial(session: Session, app_id: str) -> App | None: return app_model +def _load_recommended_app_model(session: Session, app_id: str) -> App | None: + """Load a normal App in the request Session after catalog admission succeeds.""" + if not application_services().recommended_app_queries.is_recommended(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]: """Gate generic app management routes that target an Agent App. @@ -221,7 +229,7 @@ def decorated_view(*args: P.args, **kwargs: P.kwargs) -> R: 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) + app_model = _load_recommended_app_model(session, app_id) if not app_model: raise AppNotFoundError() diff --git a/api/controllers/console/explore/wraps.py b/api/controllers/console/explore/wraps.py index 1f4da57f9aa5a1..c9111ebd01353c 100644 --- a/api/controllers/console/explore/wraps.py +++ b/api/controllers/console/explore/wraps.py @@ -14,7 +14,6 @@ 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,7 +106,7 @@ def decorated(app_id: str, *args: P.args, **kwargs: P.kwargs): 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(): + if not FeatureService.is_trial_app_enabled(): abort(403, "Trial app feature is not enabled.") return view(*args, **kwargs) diff --git a/api/extensions/ext_application_services.py b/api/extensions/ext_application_services.py index ebb6d594dc7d2b..52ddb06065328e 100644 --- a/api/extensions/ext_application_services.py +++ b/api/extensions/ext_application_services.py @@ -22,7 +22,6 @@ from services.feature_service_gateway import FeatureServiceGateway 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 @@ -69,7 +68,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, + is_trial_enabled=FeatureService.is_trial_app_enabled, ), trial_app_usage=TrialAppUsageService( usage=TrialAppUsageRepository(session_factory=database_client), diff --git a/api/services/app_service.py b/api/services/app_service.py index 1682de7e84073a..3e58c03d4fa918 100644 --- a/api/services/app_service.py +++ b/api/services/app_service.py @@ -287,6 +287,13 @@ def get_app_by_id( ) -> 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 23cc41d83d9c75..86988e749c4c3f 100644 --- a/api/services/feature_service.py +++ b/api/services/feature_service.py @@ -162,6 +162,10 @@ def get_license(cls) -> feature_entities.LicenseModel: def is_explore_banner_enabled() -> bool: return dify_config.DEPLOYMENT_EDITION == DeploymentEdition.CLOUD and dify_config.ENABLE_EXPLORE_BANNER + @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 f6ef3ed74aa4a3..468fe185e39f52 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 3fd05d47ee4ded..a21f2174632323 100644 --- a/api/services/recommended_app_query_service.py +++ b/api/services/recommended_app_query_service.py @@ -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: ... @@ -101,6 +103,9 @@ def __init__( self._trial_apps = trial_apps self._is_trial_enabled = is_trial_enabled + def is_recommended(self, app_id: str) -> bool: + return self._catalog.is_recommended(app_id) + def list_recommended( self, *, diff --git a/api/services/recommended_app_service.py b/api/services/recommended_app_service.py deleted file mode 100644 index d18751e6f9c1bc..00000000000000 --- 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.deployment_edition 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 2e9b34cf2088ff..e379871331ddf7 100644 --- a/api/tests/unit_tests/.ruff.toml +++ b/api/tests/unit_tests/.ruff.toml @@ -381,7 +381,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 2f94aedaf52385..fc3d8448e32be5 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 uuid import uuid4 import pytest +from sqlalchemy import text from sqlalchemy.orm import Session from controllers.common import session as session_module @@ -57,12 +58,64 @@ def handler(app_model): handler(app_id=app_model.id) +def test_load_recommended_app_model_rejects_app_outside_catalog( + monkeypatch: pytest.MonkeyPatch, +) -> None: + session = MagicMock(spec=Session) + app_loader = MagicMock() + recommended_app_queries = MagicMock() + recommended_app_queries.is_recommended.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_recommended_app_model(session, "app-1") is None + recommended_app_queries.is_recommended.assert_called_once_with("app-1") + app_loader.assert_not_called() + + +def test_load_recommended_app_model_uses_request_session( + monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_model = _persist_app(sqlite_session) + recommended_app_queries = MagicMock() + recommended_app_queries.is_recommended.return_value = True + monkeypatch.setattr( + wraps_module, + "application_services", + lambda: SimpleNamespace(recommended_app_queries=recommended_app_queries), + ) + + assert wraps_module._load_recommended_app_model(sqlite_session, app_model.id) is app_model + + +def test_load_recommended_app_model_rejects_non_normal_app( + monkeypatch: pytest.MonkeyPatch, sqlite_session: Session +) -> None: + app_model = _persist_app(sqlite_session) + 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_recommended.return_value = True + monkeypatch.setattr( + wraps_module, + "application_services", + lambda: SimpleNamespace(recommended_app_queries=recommended_app_queries), + ) + + assert wraps_module._load_recommended_app_model(sqlite_session, app_id) is None + + def test_get_app_model_with_trial_requires_trial_app_registration( 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) + recommended_app_loader = MagicMock(return_value=None) + monkeypatch.setattr(wraps_module, "_load_recommended_app_model", recommended_app_loader) class Handler: @wraps_module.get_app_model_with_trial @@ -72,7 +125,7 @@ def get(self, _injected_session, app_model): with pytest.raises(AppNotFoundError): Handler().get(sqlite_session, app_id=app_model.id) - recommended_get_app.assert_called_once_with(app_model.id, session=sqlite_session) + recommended_app_loader.assert_called_once_with(sqlite_session, app_model.id) def test_get_app_model_with_trial_falls_back_to_recommended_app( @@ -80,9 +133,9 @@ def test_get_app_model_with_trial_falls_back_to_recommended_app( ) -> 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) + recommended_app_loader = 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) + monkeypatch.setattr(wraps_module, "_load_recommended_app_model", recommended_app_loader) class Handler: @wraps_module.get_app_model_with_trial @@ -91,7 +144,7 @@ def get(self, _injected_session, app_model): 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) + recommended_app_loader.assert_called_once_with(unbound_session, "app-1") def test_get_app_model_with_trial_prefers_trial_registration( @@ -99,9 +152,9 @@ def test_get_app_model_with_trial_prefers_trial_registration( ) -> 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() + recommended_app_loader = MagicMock() monkeypatch.setattr(wraps_module, "_load_app_model_with_trial", trial_app_loader) - monkeypatch.setattr(wraps_module.RecommendedAppService, "get_app", recommended_get_app) + monkeypatch.setattr(wraps_module, "_load_recommended_app_model", recommended_app_loader) class Handler: @wraps_module.get_app_model_with_trial @@ -110,7 +163,7 @@ def get(self, _injected_session, app_model): 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() + recommended_app_loader.assert_not_called() def test_get_app_model_requires_app_id() -> 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 f2eb8523bbf9f9..96a3c653a559c4 100644 --- a/api/tests/unit_tests/controllers/console/explore/test_wraps.py +++ b/api/tests/unit_tests/controllers/console/explore/test_wraps.py @@ -256,7 +256,7 @@ def view(): return "ok" with patch( - "controllers.console.explore.wraps.RecommendedAppService.is_trial_app_enabled", + "controllers.console.explore.wraps.FeatureService.is_trial_app_enabled", return_value=False, ): with pytest.raises(Forbidden): @@ -269,7 +269,7 @@ def view(): return "ok" with patch( - "controllers.console.explore.wraps.RecommendedAppService.is_trial_app_enabled", + "controllers.console.explore.wraps.FeatureService.is_trial_app_enabled", return_value=True, ): 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 cba78b23eb5c9a..5fbb6655cbbf0c 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 @@ -34,3 +34,26 @@ def test_get_system_features_resolves_deployment_edition( assert result.deployment_edition is expected assert result.model_dump(mode="json")["deployment_edition"] == expected.value + + +@pytest.mark.parametrize( + ("edition", "enterprise_enabled", "feature_enabled", "expected"), + [ + ("CLOUD", False, True, True), + ("CLOUD", False, False, False), + ("SELF_HOSTED", False, True, False), + ("SELF_HOSTED", True, True, False), + ], +) +def test_trial_app_policy_is_cloud_only( + monkeypatch: pytest.MonkeyPatch, + edition: str, + enterprise_enabled: bool, + feature_enabled: bool, + expected: bool, +) -> None: + monkeypatch.setattr("services.feature_service.dify_config.EDITION", edition) + monkeypatch.setattr("services.feature_service.dify_config.ENTERPRISE_ENABLED", enterprise_enabled) + 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 82e75a2ba1ed59..f1bf8c8f15c175 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 1b4a5de2699b35..9fbc340f3c35e2 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 @@ -59,6 +59,18 @@ def _service( ) +@pytest.mark.parametrize("expected", [True, False]) +def test_is_recommended_delegates_to_catalog(expected: bool) -> None: + catalog = MagicMock() + catalog.is_recommended.return_value = expected + service, trial_apps, trial_enabled = _service(catalog=catalog) + + assert service.is_recommended("app-1") is expected + catalog.is_recommended.assert_called_once_with("app-1") + trial_apps.existing_ids.assert_not_called() + trial_enabled.assert_not_called() + + @pytest.mark.parametrize( ("requested_language", "interface_language", "expected"), [ 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 aef8ea0a8790dc..00000000000000 --- a/api/tests/unit_tests/services/test_recommended_app_service.py +++ /dev/null @@ -1,87 +0,0 @@ -"""Unit tests for the remaining recommended app command/runtime service.""" - -import uuid -from unittest.mock import MagicMock - -import pytest -from sqlalchemy.orm import Session - -from models.model import App, AppMode -from services import recommended_app_service as service_module -from services.recommended_app_service import RecommendedAppService - - -@pytest.mark.parametrize( - ("edition", "enterprise_enabled", "feature_enabled", "expected"), - [ - ("CLOUD", False, True, True), - ("CLOUD", False, False, False), - ("SELF_HOSTED", False, True, False), - ("SELF_HOSTED", True, True, False), - ], -) -def test_trial_app_policy_is_cloud_only( - monkeypatch: pytest.MonkeyPatch, - edition: str, - enterprise_enabled: bool, - feature_enabled: bool, - expected: bool, -) -> None: - monkeypatch.setattr(service_module.dify_config, "EDITION", edition) - monkeypatch.setattr(service_module.dify_config, "ENTERPRISE_ENABLED", enterprise_enabled) - monkeypatch.setattr(service_module.dify_config, "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.dify_config, "HOSTED_FETCH_APP_TEMPLATES_MODE", "remote") - 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)