From 721298ee4d081bb2dee75e5149fd9058c2f876cf Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Tue, 7 Jul 2026 15:33:10 +0900 Subject: [PATCH] . . . --- api/controllers/console/datasets/external.py | 17 +- .../external_knowledge_entities.py | 18 +- api/services/external_knowledge_service.py | 23 +- .../datasets/test_external_dataset_payload.py | 2 +- .../services/test_account_service.py | 162 ++--- .../services/test_external_dataset_service.py | 638 +++++++++--------- 6 files changed, 447 insertions(+), 413 deletions(-) diff --git a/api/controllers/console/datasets/external.py b/api/controllers/console/datasets/external.py index cff0589f9eb774..88334bad72e88c 100644 --- a/api/controllers/console/datasets/external.py +++ b/api/controllers/console/datasets/external.py @@ -1,4 +1,5 @@ -from datetime import datetime +from __future__ import annotations + from typing import Any from uuid import UUID @@ -31,6 +32,7 @@ from models import Account from services.dataset_service import DatasetService from services.enterprise import rbac_service as enterprise_rbac_service +from services.entities.external_knowledge_entities.external_knowledge_entities import ExternalDatasetCreatePayload from services.external_knowledge_service import ExternalDatasetService from services.hit_testing_service import HitTestingService from services.knowledge_service import BedrockRetrievalSetting, ExternalDatasetTestService @@ -41,14 +43,6 @@ class ExternalKnowledgeApiPayload(BaseModel): settings: dict[str, Any] -class ExternalDatasetCreatePayload(BaseModel): - external_knowledge_api_id: str - external_knowledge_id: str - name: str = Field(..., min_length=1, max_length=100) - description: str | None = Field(None, max_length=400) - external_retrieval_model: dict[str, Any] | None = None - - class ExternalHitTestingPayload(BaseModel): query: str external_retrieval_model: dict[str, Any] | None = None @@ -56,7 +50,7 @@ class ExternalHitTestingPayload(BaseModel): class BedrockRetrievalPayload(BaseModel): - retrieval_setting: "BedrockRetrievalSetting" + retrieval_setting: BedrockRetrievalSetting query: str knowledge_id: str @@ -331,7 +325,6 @@ class ExternalDatasetCreateApi(Resource): def post(self, session: Session, current_tenant_id: str, current_user: Account): # The role of the current user in the ta table must be admin, owner, or editor payload = ExternalDatasetCreatePayload.model_validate(console_ns.payload or {}) - args = payload.model_dump(exclude_none=True) # The role of the current user in the ta table must be admin, owner, or editor, or dataset_operator if not current_user.is_dataset_editor: @@ -341,7 +334,7 @@ def post(self, session: Session, current_tenant_id: str, current_user: Account): dataset = ExternalDatasetService.create_external_dataset( tenant_id=current_tenant_id, user_id=current_user.id, - args=args, + args=payload, session=session, ) except services.errors.dataset.DatasetNameDuplicateError: diff --git a/api/services/entities/external_knowledge_entities/external_knowledge_entities.py b/api/services/entities/external_knowledge_entities/external_knowledge_entities.py index 110dbe5a5e75c8..be4bd4677310b0 100644 --- a/api/services/entities/external_knowledge_entities/external_knowledge_entities.py +++ b/api/services/entities/external_knowledge_entities/external_knowledge_entities.py @@ -1,6 +1,6 @@ from typing import Any, Literal, Union -from pydantic import BaseModel +from pydantic import BaseModel, Field class AuthorizationConfig(BaseModel): @@ -24,3 +24,19 @@ class ExternalKnowledgeApiSetting(BaseModel): request_method: str headers: dict[str, Any] | None = None params: dict[str, Any] | None = None + + +class ExternalDatasetCreatePayload(BaseModel): + """Validated fields required to create an external dataset binding. + + The console controller owns HTTP concerns, but the service also needs this + contract when creating the tenant-scoped dataset and external knowledge + binding. Keep it outside controllers so service imports do not depend on + Flask blueprint initialization. + """ + + external_knowledge_api_id: str + external_knowledge_id: str + name: str = Field(..., min_length=1, max_length=100) + description: str | None = Field(None, max_length=400) + external_retrieval_model: dict[str, object] | None = Field(default=None) diff --git a/api/services/external_knowledge_service.py b/api/services/external_knowledge_service.py index cdd6c48342edb2..64f75964fc9765 100644 --- a/api/services/external_knowledge_service.py +++ b/api/services/external_knowledge_service.py @@ -1,3 +1,4 @@ +from controllers.console.datasets.external import ExternalDatasetCreatePayload import json from copy import deepcopy from typing import Any, cast @@ -274,16 +275,16 @@ def get_external_knowledge_api_settings(settings: dict[str, Any]) -> ExternalKno return ExternalKnowledgeApiSetting.model_validate(settings) @staticmethod - def create_external_dataset(tenant_id: str, user_id: str, args: dict[str, Any], *, session: Session) -> Dataset: + def create_external_dataset(tenant_id: str, user_id: str, args: ExternalDatasetCreatePayload, session: Session) -> Dataset: # check if dataset name already exists if session.scalar( - select(Dataset).where(Dataset.name == args.get("name"), Dataset.tenant_id == tenant_id).limit(1) + select(Dataset).where(Dataset.name == args.name, Dataset.tenant_id == tenant_id).limit(1) ): - raise DatasetNameDuplicateError(f"Dataset with name {args.get('name')} already exists.") + raise DatasetNameDuplicateError(f"Dataset with name {args.name} already exists.") external_knowledge_api = session.scalar( select(ExternalKnowledgeApis) .where( - ExternalKnowledgeApis.id == args.get("external_knowledge_api_id"), + ExternalKnowledgeApis.id == args.external_knowledge_api_id, ExternalKnowledgeApis.tenant_id == tenant_id, ) .limit(1) @@ -294,26 +295,22 @@ def create_external_dataset(tenant_id: str, user_id: str, args: dict[str, Any], dataset = Dataset( tenant_id=tenant_id, - name=args.get("name"), - description=args.get("description", ""), + name=args.name, + description=args.description or "", provider="external", - retrieval_model=args.get("external_retrieval_model"), + retrieval_model=args.external_retrieval_model, created_by=user_id, maintainer=user_id, ) session.add(dataset) session.flush() - if args.get("external_knowledge_id") is None: - raise ValueError("external_knowledge_id is required") - if args.get("external_knowledge_api_id") is None: - raise ValueError("external_knowledge_api_id is required") external_knowledge_binding = ExternalKnowledgeBindings( tenant_id=tenant_id, dataset_id=dataset.id, - external_knowledge_api_id=args.get("external_knowledge_api_id") or "", - external_knowledge_id=args.get("external_knowledge_id") or "", + external_knowledge_api_id=args.external_knowledge_api_id or "", + external_knowledge_id=args.external_knowledge_id or "", created_by=user_id, ) session.add(external_knowledge_binding) diff --git a/api/tests/unit_tests/controllers/console/datasets/test_external_dataset_payload.py b/api/tests/unit_tests/controllers/console/datasets/test_external_dataset_payload.py index 2ea1fcf5441397..49c89b1adfc333 100644 --- a/api/tests/unit_tests/controllers/console/datasets/test_external_dataset_payload.py +++ b/api/tests/unit_tests/controllers/console/datasets/test_external_dataset_payload.py @@ -11,7 +11,7 @@ import pytest from pydantic import ValidationError -from controllers.console.datasets.external import ExternalDatasetCreatePayload +from services.entities.external_knowledge_entities.external_knowledge_entities import ExternalDatasetCreatePayload def test_external_dataset_create_payload_allows_name_length_100() -> None: diff --git a/api/tests/unit_tests/services/test_account_service.py b/api/tests/unit_tests/services/test_account_service.py index c37800d60e2303..0bcb308db38c81 100644 --- a/api/tests/unit_tests/services/test_account_service.py +++ b/api/tests/unit_tests/services/test_account_service.py @@ -6,6 +6,7 @@ import pytest from sqlalchemy import event, select +from sqlalchemy import select from sqlalchemy.orm import Session from configs import dify_config @@ -21,6 +22,7 @@ ) from models.dataset import Dataset from models.model import App, DifySetup +from models.account import Account, AccountStatus, Tenant, TenantAccountJoin, TenantAccountRole, TenantStatus from services.account_service import AccountService, RegisterService, TenantService from services.errors.account import ( AccountAlreadyInTenantError, @@ -33,33 +35,33 @@ class TestAccountAssociatedDataFactory: - """Factory class for creating test data and mock objects for account service tests.""" + """Factory class for creating test data for account service tests.""" @staticmethod def create_account_mock( account_id: str = "user-123", email: str = "test@example.com", name: str = "Test User", - status: str = "active", + status: str | AccountStatus = AccountStatus.ACTIVE, password: str = "hashed_password", password_salt: str = "salt", interface_language: str = "en-US", interface_theme: str = "light", timezone: str = "UTC", **kwargs, - ) -> MagicMock: - """Create a mock account with specified attributes.""" - account = MagicMock(spec=Account) + ) -> Account: + """Create a real Account ORM object with specified attributes.""" + account = Account( + email=email, + name=name, + password=password, + password_salt=password_salt, + interface_language=interface_language, + interface_theme=interface_theme, + timezone=timezone, + status=AccountStatus(status), + ) account.id = account_id - account.email = email - account.name = name - account.status = status - account.password = password - account.password_salt = password_salt - account.interface_language = interface_language - account.interface_theme = interface_theme - account.timezone = timezone - # Set last_active_at to a datetime object that's older than 10 minutes account.last_active_at = datetime.now() - timedelta(minutes=15) account.initialized_at = None for key, value in kwargs.items(): @@ -129,6 +131,21 @@ def sqlite_session(self, sqlite_engine) -> Iterator[Session]: with Session(sqlite_engine, expire_on_commit=False) as session: yield session + @pytest.fixture + def sqlite_session(self, sqlite_engine) -> Iterator[Session]: + """SQLite session with the account/workspace tables registration invite flows touch.""" + tables = [ + model.metadata.tables[model.__tablename__] + for model in ( + Account, + Tenant, + TenantAccountJoin, + ) + ] + Account.metadata.create_all(sqlite_engine, tables=tables) + with Session(sqlite_engine, expire_on_commit=False) as session: + yield session + @pytest.fixture def mock_password_dependencies(self): """Mock setup for password-related functions.""" @@ -162,6 +179,28 @@ def _assert_exception_raised(self, exception_type, callable_func, *args, **kwarg with pytest.raises(exception_type): callable_func(*args, **kwargs) + def _create_tenant_with_inviter( + self, + sqlite_session: Session, + *, + tenant_name: str = "Test Workspace", + inviter_id: str = "inviter-123", + ) -> tuple[Tenant, Account]: + """Create a tenant and owner inviter so invite permission checks use real SQLite rows.""" + tenant = Tenant(name=tenant_name) + inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id=inviter_id, name="Inviter") + sqlite_session.add(tenant) + sqlite_session.flush() + sqlite_session.add( + TenantAccountJoin( + tenant_id=tenant.id, + account_id=inviter.id, + role=TenantAccountRole.OWNER, + ) + ) + sqlite_session.commit() + return tenant, inviter + # ==================== Authentication Tests ==================== def test_authenticate_success(self, sqlite_session: Session, mock_password_dependencies): @@ -1739,11 +1778,7 @@ def test_invite_new_member_new_account( self, sqlite_session: Session, mock_redis_dependencies, mock_task_dependencies ): """Test inviting a new member who doesn't have an account.""" - # Setup test data - mock_tenant = MagicMock() - mock_tenant.id = "tenant-456" - mock_tenant.name = "Test Workspace" - mock_inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id="inviter-123", name="Inviter") + tenant, inviter = self._create_tenant_with_inviter(sqlite_session) with ( patch("services.account_service.AccountService.get_account_by_email_with_case_fallback") as mock_lookup, @@ -1757,9 +1792,7 @@ def test_invite_new_member_new_account( with patch("services.account_service.RegisterService.register") as mock_register: mock_register.return_value = mock_new_account - # Mock TenantService methods with ( - patch("services.account_service.TenantService.check_member_permission") as mock_check_permission, patch("services.account_service.TenantService.create_tenant_member") as mock_create_member, patch("services.account_service.TenantService.switch_tenant") as mock_switch_tenant, patch("services.account_service.RegisterService.generate_invite_token") as mock_generate_token, @@ -1768,11 +1801,11 @@ def test_invite_new_member_new_account( # Execute test result = RegisterService.invite_new_member( - tenant=mock_tenant, + tenant=tenant, email="newuser@example.com", language="en-US", role="normal", - inviter=mock_inviter, + inviter=inviter, session=sqlite_session, ) @@ -1786,15 +1819,14 @@ def test_invite_new_member_new_account( is_setup=True, session=sqlite_session, ) - mock_lookup.assert_called_once_with("newuser@example.com", session=sqlite_session) + mock_lookup.assert_called_once() + assert mock_lookup.call_args.args[1] == "newuser@example.com" def test_invite_new_member_normalizes_new_account_email( self, sqlite_session: Session, mock_redis_dependencies, mock_task_dependencies ): """Ensure inviting with mixed-case email normalizes before registering.""" - mock_tenant = MagicMock() - mock_tenant.id = "tenant-456" - mock_inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id="inviter-123", name="Inviter") + tenant, inviter = self._create_tenant_with_inviter(sqlite_session) mixed_email = "Invitee@Example.com" with ( @@ -1808,7 +1840,6 @@ def test_invite_new_member_normalizes_new_account_email( with patch("services.account_service.RegisterService.register") as mock_register: mock_register.return_value = mock_new_account with ( - patch("services.account_service.TenantService.check_member_permission") as mock_check_permission, patch("services.account_service.TenantService.create_tenant_member") as mock_create_member, patch("services.account_service.TenantService.switch_tenant") as mock_switch_tenant, patch("services.account_service.RegisterService.generate_invite_token") as mock_generate_token, @@ -1816,11 +1847,11 @@ def test_invite_new_member_normalizes_new_account_email( mock_generate_token.return_value = "invite-token-abc" RegisterService.invite_new_member( - tenant=mock_tenant, + tenant=tenant, email=mixed_email, language="en-US", role="normal", - inviter=mock_inviter, + inviter=inviter, session=sqlite_session, ) @@ -1832,18 +1863,12 @@ def test_invite_new_member_normalizes_new_account_email( is_setup=True, session=sqlite_session, ) - mock_lookup.assert_called_once_with(mixed_email, session=sqlite_session) - mock_check_permission.assert_called_once_with( - mock_tenant, - mock_inviter, - None, - "add", - session=sqlite_session, - ) - mock_create_member.assert_called_once_with(mock_tenant, mock_new_account, sqlite_session, "normal") - mock_switch_tenant.assert_called_once_with(mock_new_account, mock_tenant.id, session=sqlite_session) + mock_lookup.assert_called_once() + assert mock_lookup.call_args.args[1] == mixed_email + mock_create_member.assert_called_once_with(tenant, mock_new_account, sqlite_session, "normal") + mock_switch_tenant.assert_called_once_with(mock_new_account, tenant.id, session=sqlite_session) mock_generate_token.assert_called_once_with( - mock_tenant, mock_new_account, "normal", requires_setup=True + tenant, mock_new_account, "normal", requires_setup=True ) mock_task_dependencies.delay.assert_called_once() @@ -1851,11 +1876,7 @@ def test_invite_new_member_existing_account( self, sqlite_session: Session, mock_redis_dependencies, mock_task_dependencies ): """Test inviting a pending account that is not in the tenant yet.""" - # Setup test data - mock_tenant = MagicMock() - mock_tenant.id = "tenant-456" - mock_tenant.name = "Test Workspace" - mock_inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id="inviter-123", name="Inviter") + tenant, inviter = self._create_tenant_with_inviter(sqlite_session) mock_existing_account = TestAccountAssociatedDataFactory.create_account_mock( account_id="existing-user-456", email="existing@example.com", status="pending" ) @@ -1865,9 +1886,7 @@ def test_invite_new_member_existing_account( ): mock_lookup.return_value = mock_existing_account - # Mock TenantService methods with ( - patch("services.account_service.TenantService.check_member_permission") as mock_check_permission, patch("services.account_service.TenantService.create_tenant_member") as mock_create_member, patch("services.account_service.RegisterService.generate_invite_token") as mock_generate_token, ): @@ -1875,31 +1894,29 @@ def test_invite_new_member_existing_account( # Execute test result = RegisterService.invite_new_member( - tenant=mock_tenant, + tenant=tenant, email="existing@example.com", language="en-US", role="normal", - inviter=mock_inviter, + inviter=inviter, session=sqlite_session, ) # Verify results assert result == "invite-token-123" - mock_create_member.assert_called_once_with(mock_tenant, mock_existing_account, sqlite_session, "normal") + mock_create_member.assert_called_once_with(tenant, mock_existing_account, sqlite_session, "normal") mock_generate_token.assert_called_once_with( - mock_tenant, mock_existing_account, "normal", requires_setup=True + tenant, mock_existing_account, "normal", requires_setup=True ) mock_task_dependencies.delay.assert_called_once() - mock_lookup.assert_called_once_with("existing@example.com", session=sqlite_session) + mock_lookup.assert_called_once() + assert mock_lookup.call_args.args[1] == "existing@example.com" def test_invite_existing_active_account_requires_acceptance_before_joining( self, sqlite_session: Session, mock_redis_dependencies, mock_task_dependencies ): """Existing active accounts outside the tenant receive an invite without immediate membership.""" - mock_tenant = MagicMock() - mock_tenant.id = "tenant-456" - mock_tenant.name = "Test Workspace" - mock_inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id="inviter-123", name="Inviter") + tenant, inviter = self._create_tenant_with_inviter(sqlite_session) mock_existing_account = TestAccountAssociatedDataFactory.create_account_mock( account_id="existing-user-456", email="existing@example.com", status="active" ) @@ -1908,69 +1925,54 @@ def test_invite_existing_active_account_requires_acceptance_before_joining( mock_lookup.return_value = mock_existing_account with ( - patch("services.account_service.TenantService.check_member_permission") as mock_check_permission, patch("services.account_service.TenantService.create_tenant_member") as mock_create_member, patch("services.account_service.RegisterService.generate_invite_token") as mock_generate_token, ): mock_generate_token.return_value = "invite-token-123" result = RegisterService.invite_new_member( - tenant=mock_tenant, + tenant=tenant, email="existing@example.com", language="en-US", role="admin", - inviter=mock_inviter, + inviter=inviter, session=sqlite_session, ) assert result == "invite-token-123" - mock_check_permission.assert_called_once_with( - mock_tenant, - mock_inviter, - mock_existing_account, - "add", - session=sqlite_session, - ) mock_create_member.assert_not_called() mock_generate_token.assert_called_once_with( - mock_tenant, mock_existing_account, "admin", requires_setup=False + tenant, mock_existing_account, "admin", requires_setup=False ) mock_task_dependencies.delay.assert_called_once() def test_invite_new_member_already_in_tenant(self, sqlite_session: Session, mock_redis_dependencies): """Test inviting a member who is already in the tenant.""" - # Setup test data - mock_tenant = MagicMock() - mock_tenant.id = "tenant-456" - mock_inviter = TestAccountAssociatedDataFactory.create_account_mock(account_id="inviter-123", name="Inviter") + tenant, inviter = self._create_tenant_with_inviter(sqlite_session) mock_existing_account = TestAccountAssociatedDataFactory.create_account_mock( account_id="existing-user-456", email="existing@example.com", status="active" ) sqlite_session.add( TenantAccountJoin( - tenant_id=mock_tenant.id, + tenant_id=tenant.id, account_id=mock_existing_account.id, role=TenantAccountRole.NORMAL, ) ) sqlite_session.commit() - # Mock TenantService methods - with ( - patch("services.account_service.AccountService.get_account_by_email_with_case_fallback") as mock_lookup, - patch("services.account_service.TenantService.check_member_permission") as mock_check_permission, - ): + with patch("services.account_service.AccountService.get_account_by_email_with_case_fallback") as mock_lookup: mock_lookup.return_value = mock_existing_account # Execute test and verify exception self._assert_exception_raised( AccountAlreadyInTenantError, RegisterService.invite_new_member, - tenant=mock_tenant, + tenant=tenant, email="existing@example.com", language="en-US", role="normal", - inviter=mock_inviter, + inviter=inviter, session=sqlite_session, ) mock_lookup.assert_called_once() diff --git a/api/tests/unit_tests/services/test_external_dataset_service.py b/api/tests/unit_tests/services/test_external_dataset_service.py index 9dff74f8dd54cc..90ca931d3ea91e 100644 --- a/api/tests/unit_tests/services/test_external_dataset_service.py +++ b/api/tests/unit_tests/services/test_external_dataset_service.py @@ -5,6 +5,7 @@ Target: 1500+ lines of comprehensive test coverage. """ +from controllers.console.datasets.external import ExternalDatasetCreatePayload import json import re from datetime import datetime @@ -12,6 +13,8 @@ from unittest.mock import MagicMock, Mock, patch import pytest +from sqlalchemy import select +from sqlalchemy.orm import Session from constants import HIDDEN_VALUE from models.dataset import Dataset, ExternalKnowledgeApis, ExternalKnowledgeBindings @@ -140,6 +143,100 @@ def factory(): return ExternalDatasetServiceTestDataFactory() +def _make_external_knowledge_api( + *, + api_id: str = "api-123", + tenant_id: str = "tenant-123", + name: str = "Test API", + description: str = "Test description", + settings: dict[str, Any] | list[dict[str, Any]] | None = None, + created_by: str = "user-123", + updated_by: str = "user-123", +) -> ExternalKnowledgeApis: + """Build a real ExternalKnowledgeApis row for SQLite-backed service tests.""" + if settings is None: + settings = {"endpoint": "https://api.example.com", "api_key": "test-key-123"} + api = ExternalKnowledgeApis( + tenant_id=tenant_id, + created_by=created_by, + updated_by=updated_by, + name=name, + description=description, + settings=json.dumps(settings, ensure_ascii=False), + ) + api.id = api_id + return api + + +def _make_dataset( + *, + dataset_id: str = "dataset-123", + tenant_id: str = "tenant-123", + name: str = "Test Dataset", + provider: str = "external", + description: str = "", + retrieval_model: dict[str, Any] | None = None, + created_by: str = "user-123", +) -> Dataset: + """Build a real Dataset row with the fields required by ExternalDatasetService.""" + dataset = Dataset( + id=dataset_id, + tenant_id=tenant_id, + name=name, + description=description, + provider=provider, + retrieval_model=retrieval_model or {}, + created_by=created_by, + maintainer=created_by, + ) + return dataset + + +def _make_external_knowledge_binding( + *, + binding_id: str = "binding-123", + tenant_id: str = "tenant-123", + dataset_id: str = "dataset-123", + external_knowledge_api_id: str = "api-123", + external_knowledge_id: str = "knowledge-123", + created_by: str = "user-123", +) -> ExternalKnowledgeBindings: + """Build a real ExternalKnowledgeBindings row for tenant-scoped lookup tests.""" + binding = ExternalKnowledgeBindings( + tenant_id=tenant_id, + dataset_id=dataset_id, + external_knowledge_api_id=external_knowledge_api_id, + external_knowledge_id=external_knowledge_id, + created_by=created_by, + ) + binding.id = binding_id + return binding + + +def _add_and_commit(session: Session, *objects: object) -> None: + """Persist rows so service methods exercise real SQLAlchemy queries.""" + session.add_all(objects) + session.commit() + + +def _seed_external_retrieval_dependencies( + session: Session, + *, + tenant_id: str = "tenant-123", + dataset_id: str = "dataset-123", + api_id: str = "api-123", +) -> tuple[ExternalKnowledgeBindings, ExternalKnowledgeApis]: + """Seed the binding and API template required by fetch_external_knowledge_retrieval.""" + binding = _make_external_knowledge_binding( + tenant_id=tenant_id, + dataset_id=dataset_id, + external_knowledge_api_id=api_id, + ) + api = _make_external_knowledge_api(api_id=api_id, tenant_id=tenant_id) + _add_and_commit(session, binding, api) + return binding, api + + class TestExternalDatasetServiceGetAPIs: """Test get_external_knowledge_apis operations - comprehensive coverage.""" @@ -434,13 +531,14 @@ def test_validate_api_list_with_extra_fields(self, factory: ExternalDatasetServi ExternalDatasetService.validate_api_list(api_settings) +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis,)], indirect=True) class TestExternalDatasetServiceCreateAPI: """Test create_external_knowledge_api operations.""" @patch("services.external_knowledge_service.db") @patch("services.external_knowledge_service.ExternalDatasetService.check_endpoint_and_api_key") def test_create_external_knowledge_api_success_full( - self, mock_check, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, mock_check, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test successful creation with all fields.""" # Arrange @@ -453,7 +551,7 @@ def test_create_external_knowledge_api_success_full( } # Act - result = ExternalDatasetService.create_external_knowledge_api(tenant_id, user_id, args, session=mock_db.session) + result = ExternalDatasetService.create_external_knowledge_api(tenant_id, user_id, args, sqlite_session) # Assert assert result.name == "Test API" @@ -462,13 +560,13 @@ def test_create_external_knowledge_api_success_full( assert result.created_by == user_id assert result.updated_by == user_id mock_check.assert_called_once_with(args["settings"]) - mock_db.session.add.assert_called_once() - mock_db.session.commit.assert_called_once() + persisted_api = sqlite_session.get(ExternalKnowledgeApis, result.id) + assert persisted_api is result @patch("services.external_knowledge_service.db") @patch("services.external_knowledge_service.ExternalDatasetService.check_endpoint_and_api_key") def test_create_external_knowledge_api_minimal_fields( - self, mock_check, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, mock_check, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test creation with minimal required fields.""" # Arrange @@ -478,17 +576,15 @@ def test_create_external_knowledge_api_minimal_fields( } # Act - result = ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + result = ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) # Assert assert result.name == "Minimal API" assert result.description == "" + assert sqlite_session.get(ExternalKnowledgeApis, result.id) is result - @patch("services.external_knowledge_service.db") def test_create_external_knowledge_api_missing_settings( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test creation fails when settings are missing.""" # Arrange @@ -496,26 +592,23 @@ def test_create_external_knowledge_api_missing_settings( # Act & Assert with pytest.raises(ValueError, match="settings is required"): - ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) - @patch("services.external_knowledge_service.db") - def test_create_external_knowledge_api_none_settings(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_create_external_knowledge_api_none_settings( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test creation fails when settings are explicitly None.""" # Arrange args = {"name": "Test API", "settings": None} # Act & Assert with pytest.raises(ValueError, match="settings is required"): - ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) @patch("services.external_knowledge_service.db") @patch("services.external_knowledge_service.ExternalDatasetService.check_endpoint_and_api_key") def test_create_external_knowledge_api_settings_json_serialization( - self, mock_check, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, mock_check, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test that settings are properly JSON serialized.""" # Arrange @@ -527,9 +620,7 @@ def test_create_external_knowledge_api_settings_json_serialization( args = {"name": "Test API", "settings": settings} # Act - result = ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + result = ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) # Assert assert isinstance(result.settings, str) @@ -539,7 +630,7 @@ def test_create_external_knowledge_api_settings_json_serialization( @patch("services.external_knowledge_service.db") @patch("services.external_knowledge_service.ExternalDatasetService.check_endpoint_and_api_key") def test_create_external_knowledge_api_unicode_handling( - self, mock_check, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, mock_check, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test proper handling of Unicode characters in name and description.""" # Arrange @@ -550,9 +641,7 @@ def test_create_external_knowledge_api_unicode_handling( } # Act - result = ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + result = ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) # Assert assert result.name == "测试API" @@ -561,7 +650,7 @@ def test_create_external_knowledge_api_unicode_handling( @patch("services.external_knowledge_service.db") @patch("services.external_knowledge_service.ExternalDatasetService.check_endpoint_and_api_key") def test_create_external_knowledge_api_long_description( - self, mock_check, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, mock_check, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test creation with very long description.""" # Arrange @@ -573,9 +662,7 @@ def test_create_external_knowledge_api_long_description( } # Act - result = ExternalDatasetService.create_external_knowledge_api( - "tenant-123", "user-123", args, session=mock_db.session - ) + result = ExternalDatasetService.create_external_knowledge_api("tenant-123", "user-123", args, sqlite_session) # Assert assert result.description == long_description @@ -835,43 +922,43 @@ def test_check_endpoint_authorization_header_format( assert call_kwargs["headers"]["Authorization"] == "Bearer test-key-123" +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis,)], indirect=True) class TestExternalDatasetServiceGetAPI: """Test get_external_knowledge_api operations.""" - @patch("services.external_knowledge_service.db") - def test_get_external_knowledge_api_success(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_get_external_knowledge_api_success( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test successful retrieval of external knowledge API.""" # Arrange api_id = "api-123" - expected_api = factory.create_external_knowledge_api_mock(api_id=api_id) - - mock_db.session.scalar.return_value = expected_api + expected_api = _make_external_knowledge_api(api_id=api_id) + _add_and_commit(sqlite_session, expected_api) # Act tenant_id = "tenant-123" - result = ExternalDatasetService.get_external_knowledge_api(api_id, tenant_id, session=mock_db.session) + result = ExternalDatasetService.get_external_knowledge_api(sqlite_session, api_id, tenant_id) # Assert assert result.id == api_id - @patch("services.external_knowledge_service.db") - def test_get_external_knowledge_api_not_found(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_get_external_knowledge_api_not_found( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test error when API is not found.""" - # Arrange - mock_db.session.scalar.return_value = None - # Act & Assert with pytest.raises(ValueError, match="api template not found"): - ExternalDatasetService.get_external_knowledge_api("nonexistent-id", "tenant-123", session=mock_db.session) + ExternalDatasetService.get_external_knowledge_api(sqlite_session, "nonexistent-id", "tenant-123") +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis,)], indirect=True) class TestExternalDatasetServiceUpdateAPI: """Test update_external_knowledge_api operations.""" @patch("services.external_knowledge_service.naive_utc_now") @patch("services.external_knowledge_service.db") def test_update_external_knowledge_api_success_all_fields( - self, mock_db, mock_now, factory: ExternalDatasetServiceTestDataFactory + self, mock_now, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test successful update with all fields.""" # Arrange @@ -881,7 +968,8 @@ def test_update_external_knowledge_api_success_all_fields( current_time = datetime(2024, 1, 2, 12, 0) mock_now.return_value = current_time - existing_api = factory.create_external_knowledge_api_mock(api_id=api_id, tenant_id=tenant_id) + existing_api = _make_external_knowledge_api(api_id=api_id, tenant_id=tenant_id) + _add_and_commit(sqlite_session, existing_api) args = { "name": "Updated API", @@ -889,252 +977,258 @@ def test_update_external_knowledge_api_success_all_fields( "settings": {"endpoint": "https://new.example.com", "api_key": "new-key"}, } - mock_db.session.scalar.return_value = existing_api - # Act - result = ExternalDatasetService.update_external_knowledge_api( - tenant_id, user_id, api_id, args, session=mock_db.session - ) + result = ExternalDatasetService.update_external_knowledge_api(sqlite_session, tenant_id, user_id, api_id, args) # Assert assert result.name == "Updated API" assert result.description == "Updated description" assert result.updated_by == user_id assert result.updated_at == current_time - mock_db.session.commit.assert_called_once() + assert sqlite_session.get(ExternalKnowledgeApis, api_id) is result @patch("services.external_knowledge_service.db") def test_update_external_knowledge_api_preserve_hidden_api_key( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test that hidden API key is preserved from existing settings.""" # Arrange api_id = "api-123" tenant_id = "tenant-123" - existing_api = factory.create_external_knowledge_api_mock( + existing_api = _make_external_knowledge_api( api_id=api_id, tenant_id=tenant_id, settings={"endpoint": "https://api.example.com", "api_key": "original-secret-key"}, ) + _add_and_commit(sqlite_session, existing_api) args = { "name": "Updated API", "settings": {"endpoint": "https://api.example.com", "api_key": HIDDEN_VALUE}, } - mock_db.session.scalar.return_value = existing_api - # Act result = ExternalDatasetService.update_external_knowledge_api( - tenant_id, "user-123", api_id, args, session=mock_db.session + sqlite_session, tenant_id, "user-123", api_id, args ) # Assert settings = json.loads(result.settings) assert settings["api_key"] == "original-secret-key" - @patch("services.external_knowledge_service.db") - def test_update_external_knowledge_api_not_found(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_update_external_knowledge_api_not_found( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test error when API is not found.""" # Arrange - mock_db.session.scalar.return_value = None - args = {"name": "Updated API"} # Act & Assert with pytest.raises(ValueError, match="api template not found"): ExternalDatasetService.update_external_knowledge_api( - "tenant-123", "user-123", "api-123", args, session=mock_db.session + sqlite_session, "tenant-123", "user-123", "api-123", args ) - @patch("services.external_knowledge_service.db") def test_update_external_knowledge_api_tenant_mismatch( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when tenant ID doesn't match.""" # Arrange - mock_db.session.scalar.return_value = None - + _add_and_commit(sqlite_session, _make_external_knowledge_api(api_id="api-123", tenant_id="tenant-123")) args = {"name": "Updated API"} # Act & Assert with pytest.raises(ValueError, match="api template not found"): ExternalDatasetService.update_external_knowledge_api( - "wrong-tenant", "user-123", "api-123", args, session=mock_db.session + sqlite_session, "wrong-tenant", "user-123", "api-123", args ) - @patch("services.external_knowledge_service.db") - def test_update_external_knowledge_api_name_only(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_update_external_knowledge_api_name_only( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test updating only the name field.""" # Arrange - existing_api = factory.create_external_knowledge_api_mock( + existing_api = _make_external_knowledge_api( description="Original description", settings={"endpoint": "https://api.example.com", "api_key": "key"}, ) + _add_and_commit(sqlite_session, existing_api) args = {"name": "New Name Only"} - mock_db.session.scalar.return_value = existing_api - # Act result = ExternalDatasetService.update_external_knowledge_api( - "tenant-123", "user-123", "api-123", args, session=mock_db.session + sqlite_session, "tenant-123", "user-123", "api-123", args ) # Assert assert result.name == "New Name Only" +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis,)], indirect=True) class TestExternalDatasetServiceDeleteAPI: """Test delete_external_knowledge_api operations.""" - @patch("services.external_knowledge_service.db") - def test_delete_external_knowledge_api_success(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_delete_external_knowledge_api_success( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test successful deletion of external knowledge API.""" # Arrange api_id = "api-123" tenant_id = "tenant-123" - existing_api = factory.create_external_knowledge_api_mock(api_id=api_id, tenant_id=tenant_id) - - mock_db.session.scalar.return_value = existing_api + existing_api = _make_external_knowledge_api(api_id=api_id, tenant_id=tenant_id) + _add_and_commit(sqlite_session, existing_api) # Act - ExternalDatasetService.delete_external_knowledge_api(tenant_id, api_id, session=mock_db.session) + ExternalDatasetService.delete_external_knowledge_api(sqlite_session, tenant_id, api_id) # Assert - mock_db.session.delete.assert_called_once_with(existing_api) - mock_db.session.commit.assert_called_once() + assert sqlite_session.get(ExternalKnowledgeApis, api_id) is None - @patch("services.external_knowledge_service.db") - def test_delete_external_knowledge_api_not_found(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_delete_external_knowledge_api_not_found( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test error when API is not found.""" - # Arrange - mock_db.session.scalar.return_value = None - # Act & Assert with pytest.raises(ValueError, match="api template not found"): - ExternalDatasetService.delete_external_knowledge_api("tenant-123", "api-123", session=mock_db.session) + ExternalDatasetService.delete_external_knowledge_api(sqlite_session, "tenant-123", "api-123") - @patch("services.external_knowledge_service.db") def test_delete_external_knowledge_api_tenant_mismatch( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when tenant ID doesn't match.""" # Arrange - mock_db.session.scalar.return_value = None + _add_and_commit(sqlite_session, _make_external_knowledge_api(api_id="api-123", tenant_id="tenant-123")) # Act & Assert with pytest.raises(ValueError, match="api template not found"): - ExternalDatasetService.delete_external_knowledge_api("wrong-tenant", "api-123", session=mock_db.session) + ExternalDatasetService.delete_external_knowledge_api(sqlite_session, "wrong-tenant", "api-123") +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeBindings,)], indirect=True) class TestExternalDatasetServiceAPIUseCheck: """Test external_knowledge_api_use_check operations.""" - @patch("services.external_knowledge_service.db") def test_external_knowledge_api_use_check_in_use_single( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test API use check when API has one binding.""" # Arrange api_id = "api-123" tenant_id = "tenant-123" - mock_db.session.scalar.return_value = 1 + _add_and_commit( + sqlite_session, + _make_external_knowledge_binding(external_knowledge_api_id=api_id, tenant_id=tenant_id), + _make_external_knowledge_binding( + binding_id="binding-other", + external_knowledge_api_id=api_id, + tenant_id="other-tenant", + ), + ) # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check( - api_id, tenant_id, session=mock_db.session - ) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(sqlite_session, api_id, tenant_id) # Assert assert in_use is True assert count == 1 - assert "tenant_id" in str(mock_db.session.scalar.call_args.args[0]) - @patch("services.external_knowledge_service.db") def test_external_knowledge_api_use_check_in_use_multiple( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test API use check with multiple bindings.""" # Arrange api_id = "api-123" tenant_id = "tenant-123" - mock_db.session.scalar.return_value = 10 + _add_and_commit( + sqlite_session, + *[ + _make_external_knowledge_binding( + binding_id=f"binding-{index}", + external_knowledge_api_id=api_id, + tenant_id=tenant_id, + dataset_id=f"dataset-{index}", + ) + for index in range(10) + ], + ) # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check( - api_id, tenant_id, session=mock_db.session - ) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(sqlite_session, api_id, tenant_id) # Assert assert in_use is True assert count == 10 - @patch("services.external_knowledge_service.db") - def test_external_knowledge_api_use_check_not_in_use(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_external_knowledge_api_use_check_not_in_use( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test API use check when API is not in use.""" # Arrange api_id = "api-123" tenant_id = "tenant-123" - mock_db.session.scalar.return_value = 0 + _add_and_commit( + sqlite_session, + _make_external_knowledge_binding( + external_knowledge_api_id=api_id, + tenant_id="other-tenant", + ), + ) # Act - in_use, count = ExternalDatasetService.external_knowledge_api_use_check( - api_id, tenant_id, session=mock_db.session - ) + in_use, count = ExternalDatasetService.external_knowledge_api_use_check(sqlite_session, api_id, tenant_id) # Assert assert in_use is False assert count == 0 +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeBindings,)], indirect=True) class TestExternalDatasetServiceGetBinding: """Test get_external_knowledge_binding_with_dataset_id operations.""" - @patch("services.external_knowledge_service.db") - def test_get_external_knowledge_binding_success(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_get_external_knowledge_binding_success( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test successful retrieval of external knowledge binding.""" # Arrange tenant_id = "tenant-123" dataset_id = "dataset-123" - expected_binding = factory.create_external_knowledge_binding_mock(tenant_id=tenant_id, dataset_id=dataset_id) - - mock_db.session.scalar.return_value = expected_binding + expected_binding = _make_external_knowledge_binding(tenant_id=tenant_id, dataset_id=dataset_id) + _add_and_commit(sqlite_session, expected_binding) # Act result = ExternalDatasetService.get_external_knowledge_binding_with_dataset_id( - tenant_id, dataset_id, session=mock_db.session + sqlite_session, tenant_id, dataset_id ) # Assert assert result.dataset_id == dataset_id assert result.tenant_id == tenant_id - @patch("services.external_knowledge_service.db") - def test_get_external_knowledge_binding_not_found(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_get_external_knowledge_binding_not_found( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test error when binding is not found.""" - # Arrange - mock_db.session.scalar.return_value = None - # Act & Assert with pytest.raises(ValueError, match="external knowledge binding not found"): ExternalDatasetService.get_external_knowledge_binding_with_dataset_id( - "tenant-123", "dataset-123", session=mock_db.session + sqlite_session, "tenant-123", "dataset-123" ) +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis,)], indirect=True) class TestExternalDatasetServiceDocumentValidate: """Test document_create_args_validate operations.""" - @patch("services.external_knowledge_service.db") def test_document_create_args_validate_success_all_params( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test successful validation with all required parameters.""" # Arrange @@ -1149,20 +1243,16 @@ def test_document_create_args_validate_success_all_params( ] } - api = factory.create_external_knowledge_api_mock(api_id=api_id, settings=[settings]) - - mock_db.session.scalar.return_value = api + api = _make_external_knowledge_api(api_id=api_id, tenant_id=tenant_id, settings=[settings]) + _add_and_commit(sqlite_session, api) process_parameter = {"param1": "value1", "param2": "value2"} # Act & Assert - should not raise - ExternalDatasetService.document_create_args_validate( - tenant_id, api_id, process_parameter, session=mock_db.session - ) + ExternalDatasetService.document_create_args_validate(sqlite_session, tenant_id, api_id, process_parameter) - @patch("services.external_knowledge_service.db") def test_document_create_args_validate_missing_required_param( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test validation fails when required parameter is missing.""" # Arrange @@ -1171,45 +1261,38 @@ def test_document_create_args_validate_missing_required_param( settings = {"document_process_setting": [{"name": "required_param", "required": True}]} - api = factory.create_external_knowledge_api_mock(api_id=api_id, settings=[settings]) - - mock_db.session.scalar.return_value = api + api = _make_external_knowledge_api(api_id=api_id, tenant_id=tenant_id, settings=[settings]) + _add_and_commit(sqlite_session, api) process_parameter = {} # Act & Assert with pytest.raises(ValueError, match="required_param is required"): - ExternalDatasetService.document_create_args_validate( - tenant_id, api_id, process_parameter, session=mock_db.session - ) + ExternalDatasetService.document_create_args_validate(sqlite_session, tenant_id, api_id, process_parameter) - @patch("services.external_knowledge_service.db") - def test_document_create_args_validate_api_not_found(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_document_create_args_validate_api_not_found( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test validation fails when API is not found.""" - # Arrange - mock_db.session.scalar.return_value = None - # Act & Assert with pytest.raises(ValueError, match="api template not found"): - ExternalDatasetService.document_create_args_validate("tenant-123", "api-123", {}, session=mock_db.session) + ExternalDatasetService.document_create_args_validate(sqlite_session, "tenant-123", "api-123", {}) - @patch("services.external_knowledge_service.db") def test_document_create_args_validate_no_custom_parameters( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test validation succeeds when no custom parameters defined.""" # Arrange settings = {} - api = factory.create_external_knowledge_api_mock(settings=[settings]) - - mock_db.session.scalar.return_value = api + api = _make_external_knowledge_api(settings=[settings]) + _add_and_commit(sqlite_session, api) # Act & Assert - should not raise - ExternalDatasetService.document_create_args_validate("tenant-123", "api-123", {}, session=mock_db.session) + ExternalDatasetService.document_create_args_validate(sqlite_session, "tenant-123", "api-123", {}) @patch("services.external_knowledge_service.db") def test_document_create_args_validate_optional_params_not_required( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test that optional parameters don't cause validation failure.""" # Arrange @@ -1220,16 +1303,13 @@ def test_document_create_args_validate_optional_params_not_required( ] } - api = factory.create_external_knowledge_api_mock(settings=[settings]) - - mock_db.session.scalar.return_value = api + api = _make_external_knowledge_api(settings=[settings]) + _add_and_commit(sqlite_session, api) process_parameter = {"required_param": "value"} # Act & Assert - should not raise - ExternalDatasetService.document_create_args_validate( - "tenant-123", "api-123", process_parameter, session=mock_db.session - ) + ExternalDatasetService.document_create_args_validate(sqlite_session, "tenant-123", "api-123", process_parameter) class TestExternalDatasetServiceProcessAPI: @@ -1516,11 +1596,17 @@ def test_get_external_knowledge_api_settings_success(self, factory: ExternalData assert result.params["key1"] == "value1" +@pytest.mark.parametrize( + "sqlite_session", + [(Dataset, ExternalKnowledgeApis, ExternalKnowledgeBindings)], + indirect=True, +) class TestExternalDatasetServiceCreateDataset: """Test create_external_dataset operations.""" - @patch("services.external_knowledge_service.db") - def test_create_external_dataset_success_full(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_create_external_dataset_success_full( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test successful creation of external dataset with all fields.""" # Arrange tenant_id = "tenant-123" @@ -1533,89 +1619,92 @@ def test_create_external_dataset_success_full(self, mock_db, factory: ExternalDa "external_retrieval_model": {"top_k": 5, "score_threshold": 0.7}, } - api = factory.create_external_knowledge_api_mock(api_id="api-123") - - mock_db.session.scalar.side_effect = [None, api] + api = _make_external_knowledge_api(api_id="api-123", tenant_id=tenant_id) + _add_and_commit(sqlite_session, api) # Act - result = ExternalDatasetService.create_external_dataset(tenant_id, user_id, args, session=mock_db.session) + result = ExternalDatasetService.create_external_dataset(tenant_id, user_id, args, sqlite_session) # Assert assert result.name == "Test External Dataset" assert result.description == "Comprehensive test description" assert result.provider == "external" assert result.created_by == user_id - mock_db.session.add.assert_called() - mock_db.session.commit.assert_called_once() + binding = sqlite_session.scalar( + select(ExternalKnowledgeBindings).where( + ExternalKnowledgeBindings.dataset_id == result.id, + ExternalKnowledgeBindings.tenant_id == tenant_id, + ) + ) + assert binding is not None + assert binding.external_knowledge_api_id == "api-123" + assert binding.external_knowledge_id == "knowledge-123" - @patch("services.external_knowledge_service.db") def test_create_external_dataset_duplicate_name_error( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when dataset name already exists.""" # Arrange - existing_dataset = factory.create_dataset_mock(name="Duplicate Dataset") - - mock_db.session.scalar.return_value = existing_dataset + existing_dataset = _make_dataset(name="Duplicate Dataset") + _add_and_commit(sqlite_session, existing_dataset) - args = {"name": "Duplicate Dataset"} + args = { + "name": "Duplicate Dataset", + "external_knowledge_api_id": "api-123", + "external_knowledge_id": "knowledge-123", + } # Act & Assert with pytest.raises(DatasetNameDuplicateError): - ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, session=mock_db.session) + ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, sqlite_session) - @patch("services.external_knowledge_service.db") - def test_create_external_dataset_api_not_found_error(self, mock_db, factory: ExternalDatasetServiceTestDataFactory): + def test_create_external_dataset_api_not_found_error( + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session + ): """Test error when external knowledge API is not found.""" - # Arrange - mock_db.session.scalar.side_effect = [None, None] - - args = {"name": "Test Dataset", "external_knowledge_api_id": "nonexistent-api"} + args = { + "name": "Test Dataset", + "external_knowledge_api_id": "nonexistent-api", + "external_knowledge_id": "knowledge-123", + } # Act & Assert with pytest.raises(ValueError, match="api template not found"): - ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, session=mock_db.session) + ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, sqlite_session) - @patch("services.external_knowledge_service.db") def test_create_external_dataset_missing_knowledge_id_error( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when external_knowledge_id is missing.""" # Arrange - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [None, api] + api = _make_external_knowledge_api() + _add_and_commit(sqlite_session, api) - args = {"name": "Test Dataset", "external_knowledge_api_id": "api-123"} + args = ExternalDatasetCreatePayload.model_validate({"name": "Test Dataset", "external_knowledge_api_id": "api-123"}) # Act & Assert with pytest.raises(ValueError, match="external_knowledge_id is required"): - ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, session=mock_db.session) + ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, sqlite_session) - @patch("services.external_knowledge_service.db") def test_create_external_dataset_missing_api_id_error( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when external_knowledge_api_id is missing.""" - # Arrange - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [None, api] - - args = {"name": "Test Dataset", "external_knowledge_id": "knowledge-123"} + args = ExternalDatasetCreatePayload.model_validate({"name": "Test Dataset", "external_knowledge_id": "knowledge-123"}) # Act & Assert with pytest.raises(ValueError, match="external_knowledge_api_id is required"): - ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, session=mock_db.session) + ExternalDatasetService.create_external_dataset("tenant-123", "user-123", args, sqlite_session) +@pytest.mark.parametrize("sqlite_session", [(ExternalKnowledgeApis, ExternalKnowledgeBindings)], indirect=True) class TestExternalDatasetServiceFetchRetrieval: """Test fetch_external_knowledge_retrieval operations.""" @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_success_with_results( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory + self, mock_process, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test successful external knowledge retrieval with results.""" # Arrange @@ -1623,12 +1712,7 @@ def test_fetch_external_knowledge_retrieval_success_with_results( dataset_id = "dataset-123" query = "test query for retrieval" - binding = factory.create_external_knowledge_binding_mock( - dataset_id=dataset_id, external_knowledge_api_id="api-123" - ) - api = factory.create_external_knowledge_api_mock(api_id="api-123") - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session, tenant_id=tenant_id, dataset_id=dataset_id) mock_response = MagicMock() mock_response.status_code = 200 @@ -1644,11 +1728,7 @@ def test_fetch_external_knowledge_retrieval_success_with_results( # Act result = ExternalDatasetService.fetch_external_knowledge_retrieval( - tenant_id, - dataset_id, - query, - external_retrieval_parameters, - session=mock_db.session, + sqlite_session, tenant_id, dataset_id, query, external_retrieval_parameters ) # Assert @@ -1658,44 +1738,39 @@ def test_fetch_external_knowledge_retrieval_success_with_results( @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_binding_not_found_error( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when external knowledge binding is not found.""" - # Arrange - mock_db.session.scalar.return_value = None - # Act & Assert with pytest.raises(ExternalKnowledgeRetrievalError, match="external knowledge binding not found"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", "dataset-123", "query", {}, session=mock_db.session + sqlite_session, "tenant-123", "dataset-123", "query", {} ) @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_cross_tenant_api_template_error( - self, mock_db, factory: ExternalDatasetServiceTestDataFactory + self, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test error when a binding points to an API template outside the dataset tenant.""" # Arrange - binding = factory.create_external_knowledge_binding_mock() - mock_db.session.scalar.side_effect = [binding, None] + binding = _make_external_knowledge_binding(tenant_id="tenant-123", external_knowledge_api_id="api-123") + cross_tenant_api = _make_external_knowledge_api(api_id="api-123", tenant_id="other-tenant") + _add_and_commit(sqlite_session, binding, cross_tenant_api) # Act & Assert with pytest.raises(ExternalKnowledgeRetrievalError, match="external api template not found"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", "dataset-123", "query", {}, session=mock_db.session + sqlite_session, "tenant-123", "dataset-123", "query", {} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_empty_results( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory + self, mock_process, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test retrieval with empty results.""" # Arrange - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 200 @@ -1704,11 +1779,7 @@ def test_fetch_external_knowledge_retrieval_empty_results( # Act result = ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) # Assert @@ -1717,14 +1788,11 @@ def test_fetch_external_knowledge_retrieval_empty_results( @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_with_score_threshold( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory + self, mock_process, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test retrieval with score threshold enabled.""" # Arrange - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 200 @@ -1739,11 +1807,7 @@ def test_fetch_external_knowledge_retrieval_with_score_threshold( # Act result = ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - external_retrieval_parameters, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", external_retrieval_parameters ) # Assert @@ -1755,14 +1819,11 @@ def test_fetch_external_knowledge_retrieval_with_score_threshold( @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_non_200_status_raises_exception( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory + self, mock_process, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test that non-200 status code raises Exception with response text.""" # Arrange - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 500 @@ -1772,11 +1833,7 @@ def test_fetch_external_knowledge_retrieval_non_200_status_raises_exception( # Act & Assert with pytest.raises(ExternalKnowledgeRetrievalError, match="Internal Server Error: Database connection failed"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) @pytest.mark.parametrize( @@ -1795,19 +1852,19 @@ def test_fetch_external_knowledge_retrieval_non_200_status_raises_exception( @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_various_error_status_codes( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory, status_code, error_message + self, + mock_process, + factory: ExternalDatasetServiceTestDataFactory, + sqlite_session: Session, + status_code, + error_message, ): """Test that various error status codes raise exceptions with response text.""" # Arrange tenant_id = "tenant-123" dataset_id = "dataset-123" - binding = factory.create_external_knowledge_binding_mock( - dataset_id=dataset_id, external_knowledge_api_id="api-123" - ) - api = factory.create_external_knowledge_api_mock(api_id="api-123") - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session, tenant_id=tenant_id, dataset_id=dataset_id) mock_response = MagicMock() mock_response.status_code = status_code @@ -1817,20 +1874,17 @@ def test_fetch_external_knowledge_retrieval_various_error_status_codes( # Act & Assert with pytest.raises(ExternalKnowledgeRetrievalError, match=re.escape(error_message)): ExternalDatasetService.fetch_external_knowledge_retrieval( - tenant_id, dataset_id, "query", {"top_k": 5}, session=mock_db.session + sqlite_session, tenant_id, dataset_id, "query", {"top_k": 5} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") @patch("services.external_knowledge_service.db") def test_fetch_external_knowledge_retrieval_empty_response_text( - self, mock_db, mock_process, factory: ExternalDatasetServiceTestDataFactory + self, mock_process, factory: ExternalDatasetServiceTestDataFactory, sqlite_session: Session ): """Test exception with empty response text.""" # Arrange - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 503 @@ -1840,21 +1894,15 @@ def test_fetch_external_knowledge_retrieval_empty_response_text( # Act & Assert with pytest.raises(ExternalKnowledgeRetrievalError): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") - @patch("services.external_knowledge_service.db") - def test_fetch_external_knowledge_retrieval_invalid_json_response(self, mock_db, mock_process, factory): + def test_fetch_external_knowledge_retrieval_invalid_json_response( + self, mock_process, factory, sqlite_session: Session + ): """Test malformed JSON success responses are normalized to external retrieval errors.""" - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 200 @@ -1863,21 +1911,15 @@ def test_fetch_external_knowledge_retrieval_invalid_json_response(self, mock_db, with pytest.raises(ExternalKnowledgeRetrievalError, match="invalid external knowledge response"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") - @patch("services.external_knowledge_service.db") - def test_fetch_external_knowledge_retrieval_invalid_success_payload_shape(self, mock_db, mock_process, factory): + def test_fetch_external_knowledge_retrieval_invalid_success_payload_shape( + self, mock_process, factory, sqlite_session: Session + ): """Test malformed success payload shapes are normalized to external retrieval errors.""" - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 200 @@ -1886,21 +1928,15 @@ def test_fetch_external_knowledge_retrieval_invalid_success_payload_shape(self, with pytest.raises(ExternalKnowledgeRetrievalError, match="invalid external knowledge response"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") - @patch("services.external_knowledge_service.db") - def test_fetch_external_knowledge_retrieval_invalid_records_shape(self, mock_db, mock_process, factory): + def test_fetch_external_knowledge_retrieval_invalid_records_shape( + self, mock_process, factory, sqlite_session: Session + ): """Test non-list records payloads are normalized to external retrieval errors.""" - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_response = MagicMock() mock_response.status_code = 200 @@ -1909,28 +1945,18 @@ def test_fetch_external_knowledge_retrieval_invalid_records_shape(self, mock_db, with pytest.raises(ExternalKnowledgeRetrievalError, match="invalid external knowledge response"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} ) @patch("services.external_knowledge_service.ExternalDatasetService.process_external_api") - @patch("services.external_knowledge_service.db") - def test_fetch_external_knowledge_retrieval_wraps_transport_errors(self, mock_db, mock_process, factory): + def test_fetch_external_knowledge_retrieval_wraps_transport_errors( + self, mock_process, factory, sqlite_session: Session + ): """Test transport/runtime failures are normalized to external retrieval errors.""" - binding = factory.create_external_knowledge_binding_mock() - api = factory.create_external_knowledge_api_mock() - - mock_db.session.scalar.side_effect = [binding, api] + _seed_external_retrieval_dependencies(sqlite_session) mock_process.side_effect = RuntimeError("connection reset by peer") with pytest.raises(ExternalKnowledgeRetrievalError, match="connection reset by peer"): ExternalDatasetService.fetch_external_knowledge_retrieval( - "tenant-123", - "dataset-123", - "query", - {"top_k": 5}, - session=mock_db.session, + sqlite_session, "tenant-123", "dataset-123", "query", {"top_k": 5} )