From aeb7687e2cf8964670c0b50edd371856e5efbd10 Mon Sep 17 00:00:00 2001 From: EvanYao <2869018789@qq.com> Date: Thu, 14 May 2026 14:42:22 +0800 Subject: [PATCH] fix: add null check in get_recommend_app_detail before accessing result['id'] (#36153) --- api/services/recommended_app_service.py | 4 +- .../services/test_recommended_app_service.py | 26 +++++++++++ .../services/test_recommended_app_service.py | 46 +++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 api/tests/unit_tests/services/test_recommended_app_service.py diff --git a/api/services/recommended_app_service.py b/api/services/recommended_app_service.py index 134dd37a3e..4e189e6e7c 100644 --- a/api/services/recommended_app_service.py +++ b/api/services/recommended_app_service.py @@ -47,7 +47,9 @@ class RecommendedAppService: """ mode = dify_config.HOSTED_FETCH_APP_TEMPLATES_MODE retrieval_instance = RecommendAppRetrievalFactory.get_recommend_app_factory(mode)() - result: dict[str, Any] = retrieval_instance.get_recommend_app_detail(app_id) + result: dict[str, Any] | None = retrieval_instance.get_recommend_app_detail(app_id) + if result is None: + return None if FeatureService.get_system_features().enable_trial_app: app_id = result["id"] trial_app_model = db.session.scalar(select(TrialApp).where(TrialApp.app_id == app_id).limit(1)) diff --git a/api/tests/test_containers_integration_tests/services/test_recommended_app_service.py b/api/tests/test_containers_integration_tests/services/test_recommended_app_service.py index ccc4188dbf..ddaf08c0a0 100644 --- a/api/tests/test_containers_integration_tests/services/test_recommended_app_service.py +++ b/api/tests/test_containers_integration_tests/services/test_recommended_app_service.py @@ -352,6 +352,32 @@ class TestRecommendedAppServiceTrialFeatures: assert result["id"] == app_id assert result["can_trial"] is has_trial_app + def test_get_detail_returns_none_when_not_found_and_trial_enabled( + self, + db_session_with_containers: Session, + monkeypatch: pytest.MonkeyPatch, + ): + """Regression: accessing result['id'] when result is None must not crash.""" + retrieval_instance = MagicMock() + retrieval_instance.get_recommend_app_detail.return_value = None + retrieval_factory = MagicMock(return_value=retrieval_instance) + monkeypatch.setattr(service_module.dify_config, "HOSTED_FETCH_APP_TEMPLATES_MODE", "remote", raising=False) + monkeypatch.setattr( + service_module.RecommendAppRetrievalFactory, + "get_recommend_app_factory", + MagicMock(return_value=retrieval_factory), + ) + monkeypatch.setattr( + service_module.FeatureService, + "get_system_features", + MagicMock(return_value=SimpleNamespace(enable_trial_app=True)), + ) + + result = RecommendedAppService.get_recommend_app_detail("nonexistent") + + assert result is None + retrieval_instance.get_recommend_app_detail.assert_called_once_with("nonexistent") + def test_add_trial_app_record_increments_count_for_existing(self, db_session_with_containers: Session): app_id = str(uuid.uuid4()) account_id = str(uuid.uuid4()) diff --git a/api/tests/unit_tests/services/test_recommended_app_service.py b/api/tests/unit_tests/services/test_recommended_app_service.py new file mode 100644 index 0000000000..030d0d73fe --- /dev/null +++ b/api/tests/unit_tests/services/test_recommended_app_service.py @@ -0,0 +1,46 @@ +"""Unit tests for RecommendedAppService.get_recommend_app_detail null handling. + +Regression tests for #36096: accessing result['id'] when the retrieval +returns None causes a TypeError / KeyError in self-hosted mode. +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +from services.recommended_app_service import RecommendedAppService + + +class TestGetRecommendAppDetailNullCheck: + @patch("services.recommended_app_service.FeatureService", autospec=True) + @patch("services.recommended_app_service.RecommendAppRetrievalFactory", autospec=True) + @patch("services.recommended_app_service.dify_config", autospec=True) + def test_returns_none_when_retrieval_returns_none_and_trial_disabled( + self, mock_config, mock_factory_class, mock_feature_service + ): + mock_config.HOSTED_FETCH_APP_TEMPLATES_MODE = "remote" + mock_instance = MagicMock() + mock_instance.get_recommend_app_detail.return_value = None + mock_factory_class.get_recommend_app_factory.return_value = MagicMock(return_value=mock_instance) + mock_feature_service.get_system_features.return_value = SimpleNamespace(enable_trial_app=False) + + result = RecommendedAppService.get_recommend_app_detail("nonexistent") + + assert result is None + + @patch("services.recommended_app_service.FeatureService", autospec=True) + @patch("services.recommended_app_service.RecommendAppRetrievalFactory", autospec=True) + @patch("services.recommended_app_service.dify_config", autospec=True) + def test_returns_none_when_retrieval_returns_none_and_trial_enabled( + self, mock_config, mock_factory_class, mock_feature_service + ): + """Regression for #36096: must not crash when result is None and enable_trial_app is True.""" + mock_config.HOSTED_FETCH_APP_TEMPLATES_MODE = "remote" + mock_instance = MagicMock() + mock_instance.get_recommend_app_detail.return_value = None + mock_factory_class.get_recommend_app_factory.return_value = MagicMock(return_value=mock_instance) + mock_feature_service.get_system_features.return_value = SimpleNamespace(enable_trial_app=True) + + result = RecommendedAppService.get_recommend_app_detail("nonexistent") + + assert result is None + mock_instance.get_recommend_app_detail.assert_called_once_with("nonexistent")