diff --git a/api/core/app/apps/common/workflow_response_converter.py b/api/core/app/apps/common/workflow_response_converter.py index 236b8c58d3a..01ed7c2c842 100644 --- a/api/core/app/apps/common/workflow_response_converter.py +++ b/api/core/app/apps/common/workflow_response_converter.py @@ -76,7 +76,7 @@ from graphon.runtime import GraphRuntimeState from graphon.variables.segments import ArrayFileSegment, FileSegment, Segment from graphon.variables.variables import Variable from graphon.workflow_type_encoder import WorkflowRuntimeTypeConverter -from libs.datetime_utils import naive_utc_now +from libs.datetime_utils import naive_utc_now, to_utc_timestamp from models import Account, EndUser from models.human_input import HumanInputForm from models.workflow import WorkflowRun @@ -371,7 +371,7 @@ class WorkflowResponseConverter: pause_reasons, dispositions_by_form_id=dispositions_by_form_id, expiration_times_by_form_id={ - form_id: int(expiration_time.timestamp()) + form_id: to_utc_timestamp(expiration_time) for form_id, expiration_time in expiration_times_by_form_id.items() }, ) @@ -399,7 +399,7 @@ class WorkflowResponseConverter: form_token=disposition.form_token if disposition else None, approval_channels=list(disposition.approval_channels) if disposition else [], resolved_default_values=reason.resolved_default_values, - expiration_time=int(expiration_time.timestamp()), + expiration_time=to_utc_timestamp(expiration_time), ), ) ) @@ -452,7 +452,7 @@ class WorkflowResponseConverter: data=HumanInputFormTimeoutResponse.Data( node_id=event.node_id, node_title=event.node_title, - expiration_time=int(event.expiration_time.timestamp()), + expiration_time=to_utc_timestamp(event.expiration_time), ), ) diff --git a/api/core/app/llm/model_access.py b/api/core/app/llm/model_access.py index 765268f7a0e..d2b8e3539fa 100644 --- a/api/core/app/llm/model_access.py +++ b/api/core/app/llm/model_access.py @@ -151,6 +151,9 @@ def fetch_model_config( credentials_provider: CredentialsProvider, model_factory: DifyModelFactory, ) -> tuple[ModelInstance, ModelConfigWithCredentialsEntity]: + if not node_data_model.provider or not node_data_model.name: + raise ValueError("LLM provider and model are required.") + if not node_data_model.mode: raise LLMModeRequiredError("LLM mode is required.") diff --git a/api/libs/datetime_utils.py b/api/libs/datetime_utils.py index e0a6ec2cacd..d962d81e78a 100644 --- a/api/libs/datetime_utils.py +++ b/api/libs/datetime_utils.py @@ -35,6 +35,15 @@ def ensure_naive_utc(dt: datetime.datetime) -> datetime.datetime: return dt.astimezone(datetime.UTC).replace(tzinfo=None) +def to_utc_timestamp(dt: datetime.datetime) -> int: + """Convert a datetime to Unix epoch seconds, assuming naive values are UTC. + + Persisted datetimes may be returned without timezone information. Treat + those values as UTC instead of interpreting them in the host timezone. + """ + return int(ensure_naive_utc(dt).replace(tzinfo=datetime.UTC).timestamp()) + + def parse_time_range( start: str | None, end: str | None, tzname: str ) -> tuple[datetime.datetime | None, datetime.datetime | None]: diff --git a/api/providers/trace/trace-tencent/tests/unit_tests/tencent_trace/test_tencent_trace.py b/api/providers/trace/trace-tencent/tests/unit_tests/tencent_trace/test_tencent_trace.py index 71bcabd1416..feea2560ec5 100644 --- a/api/providers/trace/trace-tencent/tests/unit_tests/tencent_trace/test_tencent_trace.py +++ b/api/providers/trace/trace-tencent/tests/unit_tests/tencent_trace/test_tencent_trace.py @@ -1,11 +1,15 @@ +"""Unit tests for Tencent tracing, including SQLite-backed account resolution.""" + import gc import logging import warnings +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest from dify_trace_tencent.config import TencentConfig from dify_trace_tencent.tencent_trace import TencentDataTrace +from sqlalchemy.orm import Session from core.ops.entities.trace_entity import ( DatasetRetrievalTraceInfo, @@ -18,7 +22,9 @@ from core.ops.entities.trace_entity import ( ) from graphon.entities import WorkflowNodeExecution from graphon.enums import BuiltinNodeTypes -from models import Account, App +from models import Account, App, Tenant, TenantAccountJoin +from models.account import TenantAccountRole +from models.model import AppMode, IconType logger = logging.getLogger(__name__) @@ -412,59 +418,100 @@ class TestTencentDataTrace: assert result is None assert len([r for r in caplog.records if r.levelno == logging.DEBUG]) >= 1 - def test_get_workflow_node_executions(self, tencent_data_trace): + @pytest.mark.parametrize( + "sqlite3_session", + [(Account, App, Tenant, TenantAccountJoin)], + indirect=True, + ) + def test_get_workflow_node_executions( + self, + tencent_data_trace, + monkeypatch: pytest.MonkeyPatch, + sqlite3_session: Session, + ) -> None: + account = Account(name="Trace User", email="trace-user@example.com") + tenant = Tenant(name="Trace Tenant") + sqlite3_session.add_all([account, tenant]) + sqlite3_session.flush() + app = App( + id="app-1", + tenant_id=tenant.id, + name="Trace App", + description="", + mode=AppMode.WORKFLOW, + icon_type=IconType.EMOJI, + icon="robot", + icon_background="#FFFFFF", + enable_site=False, + enable_api=False, + created_by=account.id, + max_active_requests=0, + ) + tenant_join = TenantAccountJoin( + tenant_id=tenant.id, + account_id=account.id, + current=True, + role=TenantAccountRole.OWNER, + ) + sqlite3_session.add_all([app, tenant_join]) + sqlite3_session.commit() + trace_info = MagicMock(spec=WorkflowTraceInfo) - trace_info.metadata = {"app_id": "app-1"} + trace_info.metadata = {"app_id": app.id} trace_info.workflow_run_id = "run-1" + database = SimpleNamespace(engine=sqlite3_session.get_bind()) + monkeypatch.setattr("dify_trace_tencent.tencent_trace.db", database) + monkeypatch.setattr("models.account.db", database) - app = MagicMock(spec=App) - app.id = "app-1" - app.created_by = "user-1" - app.tenant_id = "tenant-1" + with patch("dify_trace_tencent.tencent_trace.SQLAlchemyWorkflowNodeExecutionRepository") as mock_repo: + mock_repo.return_value.get_by_workflow_execution.return_value = [] + results = tencent_data_trace._get_workflow_node_executions(trace_info) - account = MagicMock(spec=Account) - account.id = "user-1" + assert results == [] + service_account = mock_repo.call_args.kwargs["user"] + assert isinstance(service_account, Account) + assert service_account.id == account.id + assert mock_repo.call_args.kwargs["tenant_id"] == tenant.id - mock_executions = [MagicMock()] - - with patch("dify_trace_tencent.tencent_trace.db") as mock_db: - mock_db.engine = "engine" - with patch("dify_trace_tencent.tencent_trace.Session") as mock_session_ctx: - session = mock_session_ctx.return_value.__enter__.return_value - session.scalar.side_effect = [app, account] - - with patch("dify_trace_tencent.tencent_trace.SQLAlchemyWorkflowNodeExecutionRepository") as mock_repo: - mock_repo.return_value.get_by_workflow_execution.return_value = mock_executions - - results = tencent_data_trace._get_workflow_node_executions(trace_info) - - assert results == mock_executions - assert mock_repo.call_args.kwargs["tenant_id"] == "tenant-1" - - def test_get_workflow_node_executions_no_app_id(self, tencent_data_trace, caplog: pytest.LogCaptureFixture): + @pytest.mark.parametrize("sqlite3_session", [()], indirect=True) + def test_get_workflow_node_executions_no_app_id( + self, + tencent_data_trace, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + sqlite3_session: Session, + ) -> None: trace_info = MagicMock(spec=WorkflowTraceInfo) trace_info.metadata = {} + monkeypatch.setattr( + "dify_trace_tencent.tencent_trace.db", + SimpleNamespace(engine=sqlite3_session.get_bind()), + ) with caplog.at_level(logging.ERROR): results = tencent_data_trace._get_workflow_node_executions(trace_info) assert results == [] assert len([r for r in caplog.records if r.levelno == logging.ERROR]) >= 1 - def test_get_workflow_node_executions_app_not_found(self, tencent_data_trace, caplog: pytest.LogCaptureFixture): + @pytest.mark.parametrize("sqlite3_session", [(App,)], indirect=True) + def test_get_workflow_node_executions_app_not_found( + self, + tencent_data_trace, + caplog: pytest.LogCaptureFixture, + monkeypatch: pytest.MonkeyPatch, + sqlite3_session: Session, + ) -> None: trace_info = MagicMock(spec=WorkflowTraceInfo) trace_info.metadata = {"app_id": "app-1"} + monkeypatch.setattr( + "dify_trace_tencent.tencent_trace.db", + SimpleNamespace(engine=sqlite3_session.get_bind()), + ) - with patch("dify_trace_tencent.tencent_trace.db") as mock_db: - mock_db.init_app = MagicMock() # Ensure init_app is mocked - mock_db.engine = "engine" - with patch("dify_trace_tencent.tencent_trace.Session") as mock_session_ctx: - session = mock_session_ctx.return_value.__enter__.return_value - session.scalar.return_value = None - - with caplog.at_level(logging.ERROR): - results = tencent_data_trace._get_workflow_node_executions(trace_info) - assert results == [] - assert len([r for r in caplog.records if r.levelno == logging.ERROR]) >= 1 + with caplog.at_level(logging.ERROR): + results = tencent_data_trace._get_workflow_node_executions(trace_info) + assert results == [] + assert len([r for r in caplog.records if r.levelno == logging.ERROR]) >= 1 def test_get_user_id_workflow(self, tencent_data_trace): trace_info = MagicMock(spec=WorkflowTraceInfo) diff --git a/api/providers/vdb/vdb-tidb-on-qdrant/src/dify_vdb_tidb_on_qdrant/tidb_on_qdrant_vector.py b/api/providers/vdb/vdb-tidb-on-qdrant/src/dify_vdb_tidb_on_qdrant/tidb_on_qdrant_vector.py index b352243b92a..6c027d1d267 100644 --- a/api/providers/vdb/vdb-tidb-on-qdrant/src/dify_vdb_tidb_on_qdrant/tidb_on_qdrant_vector.py +++ b/api/providers/vdb/vdb-tidb-on-qdrant/src/dify_vdb_tidb_on_qdrant/tidb_on_qdrant_vector.py @@ -356,7 +356,7 @@ class TidbOnQdrantVector(BaseVector): query_filter=filter, limit=kwargs.get("top_k", 4), with_payload=True, - with_vectors=True, + with_vectors=False, score_threshold=kwargs.get("score_threshold", 0.0), ) docs = [] diff --git a/api/services/workflow_event_snapshot_service.py b/api/services/workflow_event_snapshot_service.py index 1758c13a803..58f42badc66 100644 --- a/api/services/workflow_event_snapshot_service.py +++ b/api/services/workflow_event_snapshot_service.py @@ -43,6 +43,7 @@ from graphon.enums import WorkflowExecutionStatus, WorkflowNodeExecutionStatus from graphon.runtime import GraphRuntimeState from graphon.runtime.graph_runtime_state_protocol import ReadOnlyVariablePool from graphon.workflow_type_encoder import WorkflowRuntimeTypeConverter +from libs.datetime_utils import to_utc_timestamp from models.human_input import HumanInputForm from models.model import AppMode, Message from models.workflow import WorkflowNodeExecutionTriggeredFrom, WorkflowRun @@ -451,7 +452,7 @@ def _build_human_input_required_events( ) with session_maker() as session: for form_id, expiration_time, form_definition in session.execute(stmt): - expiration_times_by_form_id[str(form_id)] = int(expiration_time.timestamp()) + expiration_times_by_form_id[str(form_id)] = to_utc_timestamp(expiration_time) try: definition_payload = json.loads(form_definition) if form_definition else {} except (TypeError, json.JSONDecodeError): @@ -594,7 +595,7 @@ def _build_pause_event( ) for row in session.execute(stmt): form_id, expiration_time, *_rest = row - expiration_times_by_form_id[str(form_id)] = int(expiration_time.timestamp()) + expiration_times_by_form_id[str(form_id)] = to_utc_timestamp(expiration_time) # Reconnect paths must preserve the same pause-reason contract as live streams; # otherwise clients see schema drift after resume. reasons = enrich_human_input_pause_reasons( diff --git a/api/tasks/app_generate/workflow_execute_task.py b/api/tasks/app_generate/workflow_execute_task.py index 9bc09bac781..8a88ff4dfa6 100644 --- a/api/tasks/app_generate/workflow_execute_task.py +++ b/api/tasks/app_generate/workflow_execute_task.py @@ -311,34 +311,45 @@ def _publish_failed_workflow_terminal_events(exc: Exception, exec_params: AppExe topic.publish(json.dumps(finished_payload.model_dump(mode="json"), ensure_ascii=False).encode()) -def _get_event_name(event: str | Mapping[str, Any] | BaseModel) -> str | None: +def _get_event_data(event: str | Mapping[str, Any] | BaseModel) -> Mapping[str, Any] | None: if isinstance(event, BaseModel): # Temporary compatibility for legacy BaseModel stream events; remove after confirming generators always emit # str / Mapping responses. - event_name = getattr(event, "event", None) - elif isinstance(event, Mapping): - event_name = event.get("event") - else: + return event.model_dump() + if isinstance(event, Mapping): + return event + return None + + +def _get_event_name(event: str | Mapping[str, Any] | BaseModel) -> str | None: + event_data = _get_event_data(event) + if event_data is None: return None + event_name = event_data.get("event") if event_name is None: return None return str(event_name) def _get_task_id(event: str | Mapping[str, Any] | BaseModel) -> str | None: - if isinstance(event, BaseModel): - # Temporary compatibility for legacy BaseModel stream events; remove after confirming generators always emit - # str / Mapping responses. - task_id = getattr(event, "task_id", None) - elif isinstance(event, Mapping): - task_id = event.get("task_id") - else: + event_data = _get_event_data(event) + if event_data is None: return None + task_id = event_data.get("task_id") return task_id if isinstance(task_id, str) and task_id else None +def _get_error_message(event: str | Mapping[str, Any] | BaseModel) -> str | None: + event_data = _get_event_data(event) + if event_data is None: + return None + + message = event_data.get("message") + return message if isinstance(message, str) and message else None + + def _publish_streaming_response( response_stream: Generator[str | Mapping[str, Any] | BaseModel, None, None], workflow_run_id: str | uuid.UUID, @@ -406,6 +417,7 @@ def _publish_streaming_response( started_published = False terminal_published = False last_task_id = normalized_workflow_run_id + stream_error_message: str | None = None try: for event in response_stream: @@ -429,6 +441,8 @@ def _publish_streaming_response( started_published = True elif event_name in terminal_events: terminal_published = True + elif event_name == "error": + stream_error_message = _get_error_message(event) or stream_error_message except Exception as exc: if not terminal_published: logger.exception( @@ -448,7 +462,7 @@ def _publish_streaming_response( normalized_workflow_run_id, ) _publish_failed_terminal_event( - error_message=unexpected_stream_end_message, + error_message=stream_error_message or unexpected_stream_end_message, task_id=last_task_id, publish_started=not started_published, ) diff --git a/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py b/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py index e58bed04270..d72b6b736cc 100644 --- a/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py +++ b/api/tests/test_containers_integration_tests/controllers/service_api/dataset/test_dataset.py @@ -1,1020 +1,24 @@ -""" -Integration tests for Service API Dataset controllers. +"""Container integration coverage for Service API dataset controllers.""" -Migrated from unit_tests/controllers/service_api/dataset/test_dataset.py. - -Tests coverage for: -- DatasetCreatePayload, DatasetUpdatePayload Pydantic models -- Tag-related payloads (create, update, delete, binding) -- DatasetListQuery model -- API endpoint error handling and controller behavior - -Services (DatasetService, TagService, DocumentService) remain mocked -since these test controller-level behavior. -""" - -import uuid -from contextlib import ExitStack -from datetime import UTC, datetime -from unittest.mock import ANY, Mock, patch +from unittest.mock import patch import pytest from flask import Flask -from sqlalchemy.orm import Session, scoped_session -from werkzeug.exceptions import Forbidden, NotFound +from sqlalchemy.orm import Session - -class SessionMatcher: - def __eq__(self, other): - return isinstance(other, (Session, scoped_session)) - - -import services -from controllers.service_api.dataset.dataset import ( - DatasetCreatePayload, - DatasetListQuery, - DatasetUpdatePayload, - TagBindingPayload, - TagCreatePayload, - TagDeletePayload, - TagUnbindingPayload, - TagUpdatePayload, -) -from controllers.service_api.dataset.error import DatasetInUseError, DatasetNameDuplicateError, InvalidActionError from models.account import Account -from models.dataset import Dataset, DatasetPermissionEnum from models.enums import TagType from models.model import Tag - -# --------------------------------------------------------------------------- -# Pydantic model validation tests -# --------------------------------------------------------------------------- - - -class TestDatasetCreatePayload: - """Test suite for DatasetCreatePayload Pydantic model.""" - - def test_payload_with_required_name(self): - payload = DatasetCreatePayload(name="Test Dataset") - assert payload.name == "Test Dataset" - assert payload.description == "" - assert payload.permission == DatasetPermissionEnum.ONLY_ME - - def test_payload_with_all_fields(self): - payload = DatasetCreatePayload( - name="Full Dataset", - description="A comprehensive dataset description", - indexing_technique="high_quality", - permission=DatasetPermissionEnum.ALL_TEAM, - provider="vendor", - embedding_model="text-embedding-ada-002", - embedding_model_provider="openai", - ) - assert payload.name == "Full Dataset" - assert payload.description == "A comprehensive dataset description" - assert payload.indexing_technique == "high_quality" - assert payload.permission == DatasetPermissionEnum.ALL_TEAM - assert payload.provider == "vendor" - assert payload.embedding_model == "text-embedding-ada-002" - assert payload.embedding_model_provider == "openai" - - def test_payload_name_length_validation_min(self): - with pytest.raises(ValueError): - DatasetCreatePayload(name="") - - def test_payload_name_length_validation_max(self): - with pytest.raises(ValueError): - DatasetCreatePayload(name="A" * 41) - - def test_payload_description_max_length(self): - with pytest.raises(ValueError): - DatasetCreatePayload(name="Dataset", description="A" * 401) - - @pytest.mark.parametrize("technique", ["high_quality", "economy"]) - def test_payload_valid_indexing_techniques(self, technique): - payload = DatasetCreatePayload(name="Dataset", indexing_technique=technique) - assert payload.indexing_technique == technique - - def test_payload_with_external_knowledge_settings(self): - payload = DatasetCreatePayload( - name="External Dataset", external_knowledge_api_id="api_123", external_knowledge_id="knowledge_456" - ) - assert payload.external_knowledge_api_id == "api_123" - assert payload.external_knowledge_id == "knowledge_456" - - -class TestDatasetUpdatePayload: - """Test suite for DatasetUpdatePayload Pydantic model.""" - - def test_payload_all_optional(self): - payload = DatasetUpdatePayload() - assert payload.name is None - assert payload.description is None - assert payload.permission is None - - def test_payload_with_partial_update(self): - payload = DatasetUpdatePayload(name="Updated Name", description="Updated description") - assert payload.name == "Updated Name" - assert payload.description == "Updated description" - - def test_payload_with_permission_change(self): - payload = DatasetUpdatePayload( - permission=DatasetPermissionEnum.PARTIAL_TEAM, - partial_member_list=[{"user_id": "user_123", "role": "editor"}], - ) - assert payload.permission == DatasetPermissionEnum.PARTIAL_TEAM - assert payload.partial_member_list is not None - assert len(payload.partial_member_list) == 1 - - def test_payload_name_length_validation(self): - with pytest.raises(ValueError): - DatasetUpdatePayload(name="") - with pytest.raises(ValueError): - DatasetUpdatePayload(name="A" * 41) - - -class TestDatasetListQuery: - """Test suite for DatasetListQuery Pydantic model.""" - - def test_query_with_defaults(self): - query = DatasetListQuery() - assert query.page == 1 - assert query.limit == 20 - assert query.keyword is None - assert query.include_all is False - assert query.tag_ids == [] - - def test_query_with_all_filters(self): - query = DatasetListQuery( - page=3, limit=50, keyword="machine learning", include_all=True, tag_ids=["tag1", "tag2", "tag3"] - ) - assert query.page == 3 - assert query.limit == 50 - assert query.keyword == "machine learning" - assert query.include_all is True - assert len(query.tag_ids) == 3 - - def test_query_with_tag_filter(self): - query = DatasetListQuery(tag_ids=["tag_abc", "tag_def"]) - assert query.tag_ids == ["tag_abc", "tag_def"] - - -class TestTagCreatePayload: - """Test suite for TagCreatePayload Pydantic model.""" - - def test_payload_with_name(self): - payload = TagCreatePayload(name="New Tag") - assert payload.name == "New Tag" - - def test_payload_name_length_min(self): - with pytest.raises(ValueError): - TagCreatePayload(name="") - - def test_payload_name_length_max(self): - with pytest.raises(ValueError): - TagCreatePayload(name="A" * 51) - - def test_payload_with_unicode_name(self): - payload = TagCreatePayload(name="标签 🏷️ Тег") - assert payload.name == "标签 🏷️ Тег" - - -class TestTagUpdatePayload: - """Test suite for TagUpdatePayload Pydantic model.""" - - def test_payload_with_name_and_id(self): - payload = TagUpdatePayload(name="Updated Tag", tag_id="tag_123") - assert payload.name == "Updated Tag" - assert payload.tag_id == "tag_123" - - def test_payload_requires_tag_id(self): - with pytest.raises(ValueError): - TagUpdatePayload.model_validate({"name": "Updated Tag"}) - - -class TestTagDeletePayload: - """Test suite for TagDeletePayload Pydantic model.""" - - def test_payload_with_tag_id(self): - payload = TagDeletePayload(tag_id="tag_to_delete") - assert payload.tag_id == "tag_to_delete" - - def test_payload_requires_tag_id(self): - with pytest.raises(ValueError): - TagDeletePayload.model_validate({}) - - -class TestTagBindingPayload: - """Test suite for TagBindingPayload Pydantic model.""" - - def test_payload_with_valid_data(self): - payload = TagBindingPayload(tag_ids=["tag1", "tag2"], target_id="dataset_123") - assert len(payload.tag_ids) == 2 - assert payload.target_id == "dataset_123" - - def test_payload_rejects_empty_tag_ids(self): - with pytest.raises(ValueError) as exc_info: - TagBindingPayload(tag_ids=[], target_id="dataset_123") - assert "Tag IDs is required" in str(exc_info.value) - - def test_payload_single_tag_id(self): - payload = TagBindingPayload(tag_ids=["single_tag"], target_id="dataset_456") - assert payload.tag_ids == ["single_tag"] - - -class TestTagUnbindingPayload: - """Test suite for TagUnbindingPayload Pydantic model.""" - - def test_payload_with_valid_data(self): - payload = TagUnbindingPayload(tag_ids=["tag_123"], target_id="dataset_456") - assert payload.tag_ids == ["tag_123"] - assert payload.target_id == "dataset_456" - - def test_payload_normalizes_legacy_tag_id(self): - payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456") - assert payload.tag_ids == ["tag_123"] - assert payload.target_id == "dataset_456" - - def test_payload_rejects_empty_tag_ids(self): - with pytest.raises(ValueError) as exc_info: - TagUnbindingPayload(tag_ids=[], target_id="dataset_456") - assert "Tag IDs is required" in str(exc_info.value) - - -# --------------------------------------------------------------------------- -# Helpers -# --------------------------------------------------------------------------- - -from inspect import unwrap +from tests.test_containers_integration_tests.controllers.console.helpers import create_console_account_and_tenant @pytest.fixture -def app(flask_app_with_containers: Flask): - # Uses the full containerised app so that Flask config, extensions, and - # blueprint registrations match production. Most tests mock the service - # layer to isolate controller logic; a few (e.g. test_list_tags_from_db) - # exercise the real DB-backed path to validate end-to-end behaviour. +def app(flask_app_with_containers: Flask) -> Flask: return flask_app_with_containers -@pytest.fixture -def mock_tenant(): - tenant = Mock() - tenant.id = str(uuid.uuid4()) - return tenant - - -@pytest.fixture -def mock_dataset(): - return make_dataset(id=str(uuid.uuid4()), tenant_id=str(uuid.uuid4())) - - -@pytest.fixture(autouse=True) -def dataset_model_getter_defaults(): - getters: dict[str, object] = { - "get_app_count": 0, - "get_document_count": 0, - "get_word_count": 0, - "get_author_name": None, - "get_tags": [], - "get_doc_form": None, - "get_external_knowledge_info": None, - "get_doc_metadata": [], - "get_is_published": False, - "get_total_documents": 0, - "get_total_available_documents": 0, - } - - with ExitStack() as stack: - for name, value in getters.items(): - getter_mock = stack.enter_context(patch.object(Dataset, name, autospec=True)) - getter_mock.return_value = value - yield - - -def make_dataset(**overrides) -> Dataset: - base = { - "id": "ds-1", - "tenant_id": "tenant-1", - "name": "Dataset", - "description": "desc", - "provider": "vendor", - "permission": "only_me", - "data_source_type": None, - "indexing_technique": "economy", - "created_by": "account-1", - "created_at": datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC), - "updated_by": None, - "updated_at": datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC), - "embedding_model": None, - "embedding_model_provider": None, - "retrieval_model": None, - "summary_index_setting": None, - "built_in_field_enabled": False, - "pipeline_id": None, - "runtime_mode": "general", - "chunk_structure": None, - "icon_info": None, - "enable_api": False, - "is_multimodal": False, - } - base.update(overrides) - return Dataset(**base) - - -def make_tag(*, id: str, name: str, binding_count: int | None = None) -> Tag: - tag = Tag(tenant_id="tenant-1", type=TagType.KNOWLEDGE, name=name, created_by="account-1") - tag.id = id - if binding_count is not None: - tag.__dict__["binding_count"] = binding_count - return tag - - -DATASET_DETAIL_KEYS = { - "id", - "name", - "description", - "provider", - "permission", - "data_source_type", - "indexing_technique", - "app_count", - "document_count", - "word_count", - "created_by", - "author_name", - "created_at", - "updated_by", - "updated_at", - "embedding_model", - "embedding_model_provider", - "embedding_available", - "retrieval_model_dict", - "summary_index_setting", - "tags", - "doc_form", - "external_knowledge_info", - "external_retrieval_model", - "doc_metadata", - "built_in_field_enabled", - "pipeline_id", - "runtime_mode", - "chunk_structure", - "icon_info", - "is_published", - "total_documents", - "total_available_documents", - "enable_api", - "is_multimodal", - "maintainer", -} - - -def assert_dataset_detail_shape(response: dict, *, with_partial_members: bool = False) -> None: - expected_keys = set(DATASET_DETAIL_KEYS) - if with_partial_members: - expected_keys.add("partial_member_list") - assert set(response) == expected_keys - assert isinstance(response["created_at"], int) - assert isinstance(response["updated_at"], int) - assert set(response["retrieval_model_dict"]) == { - "search_method", - "reranking_enable", - "reranking_mode", - "reranking_model", - "weights", - "top_k", - "score_threshold_enabled", - "score_threshold", - } - if response["external_retrieval_model"] is not None: - assert set(response["external_retrieval_model"]) == { - "top_k", - "score_threshold", - "score_threshold_enabled", - } - if not with_partial_members: - assert "partial_member_list" not in response - - -# --------------------------------------------------------------------------- -# API endpoint tests — DatasetListApi -# --------------------------------------------------------------------------- - - -class TestDatasetListApiGet: - """Test suite for DatasetListApi.get() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_list_datasets_success( - self, - mock_dataset_svc, - mock_current_user, - mock_provider_mgr, - app: Flask, - mock_tenant, - ): - from controllers.service_api.dataset.dataset import DatasetListApi - - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_tenant.id - mock_dataset_svc.get_datasets.return_value = ([make_dataset()], 1) - - mock_configs = Mock() - mock_configs.get_models.return_value = [] - mock_provider_mgr.return_value.get_configurations.return_value = mock_configs - - with app.test_request_context("/datasets?page=1&limit=20", method="GET"): - api = DatasetListApi() - response, status = api.get(tenant_id=mock_tenant.id) - - assert status == 200 - assert set(response) == {"data", "has_more", "limit", "total", "page"} - assert response["has_more"] is False - assert response["limit"] == 20 - assert response["total"] == 1 - assert response["page"] == 1 - assert len(response["data"]) == 1 - assert_dataset_detail_shape(response["data"][0]) - - @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_list_datasets_preserves_repeated_tag_ids( - self, - mock_dataset_svc, - mock_current_user, - mock_provider_mgr, - app: Flask, - mock_tenant, - ): - from controllers.service_api.dataset.dataset import DatasetListApi - - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_tenant.id - mock_dataset_svc.get_datasets.return_value = ([make_dataset()], 1) - - mock_configs = Mock() - mock_configs.get_models.return_value = [] - mock_provider_mgr.return_value.get_configurations.return_value = mock_configs - - with app.test_request_context("/datasets?tag_ids=tag-a&tag_ids=tag-b", method="GET"): - api = DatasetListApi() - response, status = api.get(tenant_id=mock_tenant.id) - - assert status == 200 - assert response["total"] == 1 - mock_dataset_svc.get_datasets.assert_called_once_with( - 1, - 20, - SessionMatcher(), - mock_tenant.id, - mock_current_user, - None, - ["tag-a", "tag-b"], - False, - ) - - -class TestDatasetListApiPost: - """Test suite for DatasetListApi.post() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_create_dataset_success( - self, - mock_dataset_svc, - mock_current_user, - app: Flask, - mock_tenant, - ): - from controllers.service_api.dataset.dataset import DatasetListApi - - mock_current_user.__class__ = Account - mock_dataset_svc.create_empty_dataset.return_value = make_dataset(name="New Dataset") - - with app.test_request_context( - "/datasets", - method="POST", - json={"name": "New Dataset"}, - ): - api = DatasetListApi() - response, status = unwrap(api.post)(api, Mock(spec=Session), tenant_id=mock_tenant.id) - - assert status == 200 - assert_dataset_detail_shape(response) - assert response["name"] == "New Dataset" - mock_dataset_svc.create_empty_dataset.assert_called_once() - - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_create_dataset_duplicate_name( - self, - mock_dataset_svc, - mock_current_user, - app: Flask, - mock_tenant, - ): - from controllers.service_api.dataset.dataset import DatasetListApi - - mock_current_user.__class__ = Account - mock_dataset_svc.create_empty_dataset.side_effect = services.errors.dataset.DatasetNameDuplicateError() - - with app.test_request_context( - "/datasets", - method="POST", - json={"name": "Existing Dataset"}, - ): - api = DatasetListApi() - with pytest.raises(DatasetNameDuplicateError): - unwrap(api.post)(api, Mock(spec=Session), tenant_id=mock_tenant.id) - - -# --------------------------------------------------------------------------- -# API endpoint tests — DatasetApi -# --------------------------------------------------------------------------- - - -class TestDatasetApiGet: - """Test suite for DatasetApi.get() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") - @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_get_dataset_success( - self, - mock_dataset_svc, - mock_current_user, - mock_provider_mgr, - mock_perm_svc, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_dataset.tenant_id - - mock_configs = Mock() - mock_configs.get_models.return_value = [] - mock_provider_mgr.return_value.get_configurations.return_value = mock_configs - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="GET", - ): - api = DatasetApi() - response, status = api.get(_=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - assert status == 200 - assert_dataset_detail_shape(response) - assert response["embedding_available"] is True - assert response["retrieval_model_dict"]["search_method"] == "keyword_search" - - @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") - @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_get_dataset_partial_members_shape( - self, - mock_dataset_svc, - mock_current_user, - mock_provider_mgr, - mock_perm_svc, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset.permission = "partial_members" - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_dataset.tenant_id - mock_perm_svc.get_dataset_partial_member_list.return_value = ["user-1", "user-2"] - - mock_configs = Mock() - mock_configs.get_models.return_value = [] - mock_provider_mgr.return_value.get_configurations.return_value = mock_configs - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="GET", - ): - api = DatasetApi() - response, status = api.get(_=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - assert status == 200 - assert_dataset_detail_shape(response, with_partial_members=True) - assert response["partial_member_list"] == ["user-1", "user-2"] - - @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") - @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_get_dataset_uses_default_external_retrieval_model( - self, - mock_dataset_svc, - mock_current_user, - mock_provider_mgr, - mock_perm_svc, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset.retrieval_model = None - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_dataset.tenant_id - - mock_configs = Mock() - mock_configs.get_models.return_value = [] - mock_provider_mgr.return_value.get_configurations.return_value = mock_configs - - with app.test_request_context(f"/datasets/{mock_dataset.id}", method="GET"): - api = DatasetApi() - response, status = api.get(_=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - assert status == 200 - assert_dataset_detail_shape(response) - assert response["external_retrieval_model"] == { - "top_k": 2, - "score_threshold": 0.0, - "score_threshold_enabled": None, - } - - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_get_dataset_not_found(self, mock_dataset_svc, app, mock_dataset): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.get_dataset.return_value = None - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="GET", - ): - api = DatasetApi() - with pytest.raises(NotFound): - api.get(_=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_get_dataset_no_permission( - self, - mock_dataset_svc, - mock_current_user, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.side_effect = services.errors.account.NoPermissionError() - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="GET", - ): - api = DatasetApi() - with pytest.raises(Forbidden): - api.get(_=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - -class TestDatasetApiPatch: - """Test suite for DatasetApi.patch() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_patch_dataset_success_shape( - self, - mock_dataset_svc, - mock_current_user, - mock_perm_svc, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - updated_dataset = make_dataset(id=mock_dataset.id, tenant_id=mock_dataset.tenant_id, name="Updated Dataset") - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.update_dataset.return_value = updated_dataset - mock_perm_svc.check_permission.return_value = None - mock_perm_svc.get_dataset_partial_member_list.return_value = ["user-1"] - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = mock_dataset.tenant_id - - payload = { - "name": "Updated Dataset", - "permission": "partial_members", - "partial_member_list": [{"user_id": "user-1", "role": "editor"}], - } - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="PATCH", - json=payload, - ): - api = DatasetApi() - response, status = unwrap(api.patch)( - api, - Mock(spec=Session), - _=mock_dataset.tenant_id, - dataset_id=mock_dataset.id, - ) - - assert status == 200 - assert_dataset_detail_shape(response, with_partial_members=True) - assert response["name"] == "Updated Dataset" - assert response["partial_member_list"] == ["user-1"] - mock_dataset_svc.update_dataset.assert_called_once() - _, update_data, _ = mock_dataset_svc.update_dataset.call_args.args - session = mock_dataset_svc.update_dataset.call_args.kwargs["session"] - assert isinstance(session, (Session, scoped_session)) - assert update_data["name"] == "Updated Dataset" - assert update_data["permission"] == "partial_members" - mock_perm_svc.update_partial_member_list.assert_called_once_with( - mock_dataset.tenant_id, - mock_dataset.id, - [{"user_id": "user-1", "role": "editor"}], - SessionMatcher(), - ) - - -class TestDatasetApiDelete: - """Test suite for DatasetApi.delete() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_delete_dataset_success( - self, - mock_dataset_svc, - mock_current_user, - mock_perm_svc, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.delete_dataset.return_value = True - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="DELETE", - ): - api = DatasetApi() - result = unwrap(api.delete)(api, Mock(), _=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - assert result == ("", 204) - - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_delete_dataset_not_found( - self, - mock_dataset_svc, - mock_current_user, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.delete_dataset.return_value = False - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="DELETE", - ): - api = DatasetApi() - with pytest.raises(NotFound): - unwrap(api.delete)(api, Mock(), _=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_delete_dataset_in_use( - self, - mock_dataset_svc, - mock_current_user, - app: Flask, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DatasetApi - - mock_dataset_svc.delete_dataset.side_effect = services.errors.dataset.DatasetInUseError() - - with app.test_request_context( - f"/datasets/{mock_dataset.id}", - method="DELETE", - ): - api = DatasetApi() - with pytest.raises(DatasetInUseError): - unwrap(api.delete)(api, Mock(), _=mock_dataset.tenant_id, dataset_id=mock_dataset.id) - - -# --------------------------------------------------------------------------- -# API endpoint tests — DocumentStatusApi -# --------------------------------------------------------------------------- - - -class TestDocumentStatusApiPatch: - """Test suite for DocumentStatusApi.patch() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.DocumentService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_batch_update_status_success( - self, - mock_dataset_svc, - mock_current_user, - mock_doc_svc, - app: Flask, - mock_tenant, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DocumentStatusApi - - mock_current_user.__class__ = Account - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_dataset_svc.check_dataset_model_setting.return_value = None - mock_doc_svc.batch_update_document_status.return_value = None - - with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/status/enable", - method="PATCH", - json={"document_ids": ["doc-1", "doc-2"]}, - ): - api = DocumentStatusApi() - response, status = api.patch( - tenant_id=mock_tenant.id, - dataset_id=mock_dataset.id, - action="enable", - ) - - assert status == 200 - assert response["result"] == "success" - - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_batch_update_status_dataset_not_found( - self, - mock_dataset_svc, - app: Flask, - mock_tenant, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DocumentStatusApi - - mock_dataset_svc.get_dataset.return_value = None - - with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/status/enable", - method="PATCH", - json={"document_ids": ["doc-1"]}, - ): - api = DocumentStatusApi() - with pytest.raises(NotFound): - api.patch( - tenant_id=mock_tenant.id, - dataset_id=mock_dataset.id, - action="enable", - ) - - @patch("controllers.service_api.dataset.dataset.DocumentService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_batch_update_status_permission_error( - self, - mock_dataset_svc, - mock_current_user, - mock_doc_svc, - app: Flask, - mock_tenant, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DocumentStatusApi - - mock_current_user.__class__ = Account - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.side_effect = services.errors.account.NoPermissionError( - "No permission" - ) - - with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/status/enable", - method="PATCH", - json={"document_ids": ["doc-1"]}, - ): - api = DocumentStatusApi() - with pytest.raises(Forbidden): - api.patch( - tenant_id=mock_tenant.id, - dataset_id=mock_dataset.id, - action="enable", - ) - - @patch("controllers.service_api.dataset.dataset.DocumentService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_batch_update_status_indexing_error( - self, - mock_dataset_svc, - mock_current_user, - mock_doc_svc, - app: Flask, - mock_tenant, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DocumentStatusApi - - mock_current_user.__class__ = Account - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_dataset_svc.check_dataset_model_setting.return_value = None - mock_doc_svc.batch_update_document_status.side_effect = services.errors.document.DocumentIndexingError() - - with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/status/enable", - method="PATCH", - json={"document_ids": ["doc-1"]}, - ): - api = DocumentStatusApi() - with pytest.raises(InvalidActionError): - api.patch( - tenant_id=mock_tenant.id, - dataset_id=mock_dataset.id, - action="enable", - ) - - @patch("controllers.service_api.dataset.dataset.DocumentService") - @patch("controllers.service_api.dataset.dataset.current_user") - @patch("controllers.service_api.dataset.dataset.DatasetService") - def test_batch_update_status_value_error( - self, - mock_dataset_svc, - mock_current_user, - mock_doc_svc, - app: Flask, - mock_tenant, - mock_dataset, - ): - from controllers.service_api.dataset.dataset import DocumentStatusApi - - mock_current_user.__class__ = Account - mock_dataset_svc.get_dataset.return_value = mock_dataset - mock_dataset_svc.check_dataset_permission.return_value = None - mock_dataset_svc.check_dataset_model_setting.return_value = None - mock_doc_svc.batch_update_document_status.side_effect = ValueError("Invalid action") - - with app.test_request_context( - f"/datasets/{mock_dataset.id}/documents/status/enable", - method="PATCH", - json={"document_ids": ["doc-1"]}, - ): - api = DocumentStatusApi() - with pytest.raises(InvalidActionError): - api.patch( - tenant_id=mock_tenant.id, - dataset_id=mock_dataset.id, - action="enable", - ) - - -# --------------------------------------------------------------------------- -# API endpoint tests — Tags -# --------------------------------------------------------------------------- - - class TestDatasetTagsApiGet: - """Test suite for DatasetTagsApi.get() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_list_tags_success( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = "tenant-1" - mock_tag = make_tag(id="tag-1", name="Test Tag", binding_count=0) - mock_tag_svc.get_tags.return_value = [mock_tag] - - with app.test_request_context("/datasets/tags", method="GET"): - api = DatasetTagsApi() - response, status = api.get(_=None) - - assert status == 200 - assert response == [{"id": "tag-1", "name": "Test Tag", "type": "knowledge", "binding_count": "0"}] - mock_tag_svc.get_tags.assert_called_once_with("knowledge", "tenant-1", session=SessionMatcher()) + """Exercise the unmocked tag query against the container database.""" @patch("controllers.service_api.dataset.dataset.current_user") def test_list_tags_from_db( @@ -1022,15 +26,8 @@ class TestDatasetTagsApiGet: mock_current_user, app: Flask, db_session_with_containers: Session, - ): - """Integration test: creates real Tag rows and retrieves them - through the controller without mocking TagService.""" - from tests.test_containers_integration_tests.controllers.console.helpers import ( - create_console_account_and_tenant, - ) - + ) -> None: account, tenant = create_console_account_and_tenant(db_session_with_containers) - tag = Tag( name="Integration Tag", type=TagType.KNOWLEDGE, @@ -1046,336 +43,9 @@ class TestDatasetTagsApiGet: from controllers.service_api.dataset.dataset import DatasetTagsApi with app.test_request_context("/datasets/tags", method="GET"): - api = DatasetTagsApi() - response, status = api.get(_=None) + response, status = DatasetTagsApi().get(_=None) assert status == 200 - assert any(t["name"] == "Integration Tag" for t in response) - assert all(set(t) == {"id", "name", "type", "binding_count"} for t in response) - assert all(isinstance(t["binding_count"], str) for t in response) - - -class TestDatasetTagsApiPost: - """Test suite for DatasetTagsApi.post() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_create_tag_success( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = True - mock_current_user.is_dataset_editor = True - mock_tag = make_tag(id="tag-new", name="New Tag") - mock_tag_svc.save_tags.return_value = mock_tag - - with app.test_request_context( - "/datasets/tags", - method="POST", - json={"name": "New Tag"}, - ): - api = DatasetTagsApi() - response, status = api.post(_=None) - - assert status == 200 - assert response == {"id": "tag-new", "name": "New Tag", "type": "knowledge", "binding_count": "0"} - mock_tag_svc.save_tags.assert_called_once() - - @patch("controllers.service_api.dataset.dataset.current_user") - def test_create_tag_forbidden(self, mock_current_user, app: Flask): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = False - mock_current_user.is_dataset_editor = False - - with app.test_request_context( - "/datasets/tags", - method="POST", - json={"name": "New Tag"}, - ): - api = DatasetTagsApi() - with pytest.raises(Forbidden): - api.post(_=None) - - -class TestDatasetTagsApiPatch: - """Test suite for DatasetTagsApi.patch() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.service_api_ns") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_update_tag_success( - self, - mock_current_user, - mock_service_api_ns, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = True - mock_current_user.is_dataset_editor = True - - mock_tag = make_tag(id="tag-1", name="Updated Tag") - mock_tag_svc.update_tags.return_value = mock_tag - mock_tag_svc.get_tag_binding_count.return_value = 5 - mock_service_api_ns.payload = {"name": "Updated Tag", "tag_id": "tag-1"} - - with app.test_request_context( - "/datasets/tags", - method="PATCH", - json={"name": "Updated Tag", "tag_id": "tag-1"}, - ): - api = DatasetTagsApi() - response, status = api.patch(_=None) - - assert status == 200 - assert response == {"id": "tag-1", "name": "Updated Tag", "type": "knowledge", "binding_count": "5"} - mock_tag_svc.update_tags.assert_called_once() - update_payload, tag_id, session = mock_tag_svc.update_tags.call_args.args - assert update_payload.name == "Updated Tag" - assert tag_id == "tag-1" - - @patch("controllers.service_api.dataset.dataset.current_user") - def test_update_tag_forbidden(self, mock_current_user, app: Flask): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = False - mock_current_user.is_dataset_editor = False - - with app.test_request_context( - "/datasets/tags", - method="PATCH", - json={"name": "Updated Tag", "tag_id": "tag-1"}, - ): - api = DatasetTagsApi() - with pytest.raises(Forbidden): - api.patch(_=None) - - -class TestDatasetTagsApiDelete: - """Test suite for DatasetTagsApi.delete() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.service_api_ns") - @patch("libs.login.current_user") - def test_delete_tag_success( - self, - mock_current_user, - mock_service_api_ns, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - user_obj = Mock(spec=Account) - user_obj.has_edit_permission = True - mock_current_user.has_edit_permission = True - # Assign as plain lambda to avoid AsyncMock returning a coroutine - mock_current_user._get_current_object = lambda: user_obj - - mock_tag_svc.delete_tag.return_value = None - mock_service_api_ns.payload = {"tag_id": "tag-1"} - - with app.test_request_context( - "/datasets/tags", - method="DELETE", - json={"tag_id": "tag-1"}, - ): - api = DatasetTagsApi() - result = api.delete(_=None) - - assert result == ("", 204) - mock_tag_svc.delete_tag.assert_called_once_with("tag-1", ANY, tag_type=TagType.KNOWLEDGE) - - @patch("libs.login.current_user") - def test_delete_tag_forbidden(self, mock_current_user, app: Flask): - from controllers.service_api.dataset.dataset import DatasetTagsApi - - user_obj = Mock(spec=Account) - user_obj.has_edit_permission = False - mock_current_user.has_edit_permission = False - # Assign as plain lambda to avoid AsyncMock returning a coroutine - mock_current_user._get_current_object = lambda: user_obj - - with app.test_request_context( - "/datasets/tags", - method="DELETE", - json={"tag_id": "tag-1"}, - ): - api = DatasetTagsApi() - with pytest.raises(Forbidden): - api.delete(_=None) - - -class TestDatasetTagsBindingStatusApi: - """Test suite for DatasetTagsBindingStatusApi endpoints.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_get_dataset_tags_binding_status( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagsBindingStatusApi - - mock_current_user.__class__ = Account - mock_current_user.current_tenant_id = "tenant_123" - mock_tag = Mock() - mock_tag.id = "tag_1" - mock_tag.name = "Test Tag" - mock_tag_svc.get_tags_by_target_id.return_value = [mock_tag] - - with app.test_request_context("/", method="GET"): - api = DatasetTagsBindingStatusApi() - response, status_code = api.get("tenant_123", dataset_id="dataset_123") - - assert status_code == 200 - assert response["data"] == [{"id": "tag_1", "name": "Test Tag"}] - assert response["total"] == 1 - mock_tag_svc.get_tags_by_target_id.assert_called_once_with("knowledge", "tenant_123", "dataset_123", ANY) - - -class TestDatasetTagBindingApiPost: - """Test suite for DatasetTagBindingApi.post() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_bind_tags_success( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagBindingApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = True - mock_current_user.is_dataset_editor = True - mock_tag_svc.save_tag_binding.return_value = None - - with app.test_request_context( - "/datasets/tags/binding", - method="POST", - json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, - ): - api = DatasetTagBindingApi() - result = api.post(_=None) - - assert result == ("", 204) - from services.tag_service import TagBindingCreatePayload - - mock_tag_svc.save_tag_binding.assert_called_once_with( - TagBindingCreatePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), - ANY, - ) - - @patch("controllers.service_api.dataset.dataset.current_user") - def test_bind_tags_forbidden(self, mock_current_user, app: Flask): - from controllers.service_api.dataset.dataset import DatasetTagBindingApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = False - mock_current_user.is_dataset_editor = False - - with app.test_request_context( - "/datasets/tags/binding", - method="POST", - json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, - ): - api = DatasetTagBindingApi() - with pytest.raises(Forbidden): - api.post(_=None) - - -class TestDatasetTagUnbindingApiPost: - """Test suite for DatasetTagUnbindingApi.post() endpoint.""" - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_unbind_tag_success( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = True - mock_current_user.is_dataset_editor = True - mock_tag_svc.delete_tag_binding.return_value = None - - with app.test_request_context( - "/datasets/tags/unbinding", - method="POST", - json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, - ): - api = DatasetTagUnbindingApi() - result = api.post(_=None) - - assert result == ("", 204) - from services.tag_service import TagBindingDeletePayload - - mock_tag_svc.delete_tag_binding.assert_called_once_with( - TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), - ANY, - ) - - @patch("controllers.service_api.dataset.dataset.TagService") - @patch("controllers.service_api.dataset.dataset.current_user") - def test_unbind_legacy_tag_id_success( - self, - mock_current_user, - mock_tag_svc, - app: Flask, - ): - from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = True - mock_current_user.is_dataset_editor = True - mock_tag_svc.delete_tag_binding.return_value = None - - with app.test_request_context( - "/datasets/tags/unbinding", - method="POST", - json={"tag_id": "tag-1", "target_id": "ds-1"}, - ): - api = DatasetTagUnbindingApi() - result = api.post(_=None) - - assert result == ("", 204) - from services.tag_service import TagBindingDeletePayload - - mock_tag_svc.delete_tag_binding.assert_called_once_with( - TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), - ANY, - ) - - @patch("controllers.service_api.dataset.dataset.current_user") - def test_unbind_tag_forbidden(self, mock_current_user, app: Flask): - from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi - - mock_current_user.__class__ = Account - mock_current_user.has_edit_permission = False - mock_current_user.is_dataset_editor = False - - with app.test_request_context( - "/datasets/tags/unbinding", - method="POST", - json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, - ): - api = DatasetTagUnbindingApi() - with pytest.raises(Forbidden): - api.post(_=None) + assert any(item["name"] == "Integration Tag" for item in response) + assert all(set(item) == {"id", "name", "type", "binding_count"} for item in response) + assert all(isinstance(item["binding_count"], str) for item in response) diff --git a/api/tests/unit_tests/controllers/common/test_session.py b/api/tests/unit_tests/controllers/common/test_session.py index 9da06133a6c..888738bdfd6 100644 --- a/api/tests/unit_tests/controllers/common/test_session.py +++ b/api/tests/unit_tests/controllers/common/test_session.py @@ -1,152 +1,140 @@ from __future__ import annotations +from contextlib import contextmanager +from unittest.mock import patch + import pytest -from sqlalchemy import Engine, literal, select -from sqlalchemy.orm import Session +from sqlalchemy import event, literal, select +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session, sessionmaker from controllers.common import session as session_module +from models import Tenant -class FakeSession: - committed: bool - rolled_back: bool - closed: bool - - def __init__(self) -> None: - self.committed = False - self.rolled_back = False - self.closed = False - - def commit(self) -> None: - self.committed = True - - def rollback(self) -> None: - self.rolled_back = True +@contextmanager +def _bind_session_factory(session: Session): + database_session_factory = sessionmaker( + bind=session.get_bind(), + expire_on_commit=False, + ) + with patch("core.db.session_factory._session_maker", database_session_factory): + yield -class FakeSessionContext: - session: FakeSession - entered: bool - exited: bool - exc_type: object | None - - def __init__(self, session: FakeSession) -> None: - self.session = session - self.entered = False - self.exited = False - self.exc_type = None - - def __enter__(self) -> FakeSession: - self.entered = True - return self.session - - def __exit__(self, exc_type: object | None, *_args: object) -> None: - self.exited = True - self.exc_type = exc_type - self.session.closed = True +def _tenant_names(session: Session) -> list[str]: + session.expire_all() + return list(session.scalars(select(Tenant.name).order_by(Tenant.name)).all()) -def test_with_session_write_commits_on_success(monkeypatch: pytest.MonkeyPatch) -> None: - session = FakeSession() - session_context = FakeSessionContext(session) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: session_context) +@pytest.mark.parametrize("sqlite_session", [(Tenant,)], indirect=True) +def test_with_session_write_commits_on_success(sqlite_session: Session) -> None: + commit_observed = False + injected_session: Session | None = None class Handler: @session_module.with_session(write=True) - def post(self, injected_session): - assert injected_session is session + def post(self, session: Session): + nonlocal commit_observed, injected_session + injected_session = session + + def observe_commit(_session: Session) -> None: + nonlocal commit_observed + commit_observed = True + + event.listen(session, "after_commit", observe_commit) + session.add(Tenant(name="committed tenant")) return "ok" - assert Handler().post() == "ok" + with _bind_session_factory(sqlite_session): + assert Handler().post() == "ok" - assert session.closed - assert session.committed - assert not session.rolled_back - assert session_context.entered - assert session_context.exited - assert session_context.exc_type is None + assert commit_observed + assert injected_session is not None + assert not injected_session.in_transaction() + assert _tenant_names(sqlite_session) == ["committed tenant"] -def test_with_session_default_write_commits_on_success(monkeypatch: pytest.MonkeyPatch) -> None: - session = FakeSession() - session_context = FakeSessionContext(session) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: session_context) - - class Handler: - @session_module.with_session - def post(self, injected_session): - assert injected_session is session - return "ok" - - assert Handler().post() == "ok" - assert session.committed - assert not session.rolled_back - - -def test_with_session_write_rolls_back_on_error(monkeypatch: pytest.MonkeyPatch) -> None: - session = FakeSession() - session_context = FakeSessionContext(session) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: session_context) - - class Handler: - @session_module.with_session(write=True) - def get(self, _session): - raise RuntimeError("boom") - - with pytest.raises(RuntimeError, match="boom"): - Handler().get() - - assert session.closed - assert not session.committed - assert session.rolled_back - assert session_context.entered - assert session_context.exited - assert session_context.exc_type is RuntimeError - - -def test_with_session_write_allows_commit_then_more_database_work( - monkeypatch: pytest.MonkeyPatch, sqlite_engine: Engine -) -> None: - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: Session(sqlite_engine)) - +@pytest.mark.parametrize("sqlite_session", [(Tenant,)], indirect=True) +def test_with_session_default_write_commits_on_success(sqlite_session: Session) -> None: class Handler: @session_module.with_session def post(self, session: Session): - session.commit() - return session.scalar(select(literal(1))) + session.add(Tenant(name="default write tenant")) + return "ok" - assert Handler().post() == 1 + with _bind_session_factory(sqlite_session): + assert Handler().post() == "ok" + + assert _tenant_names(sqlite_session) == ["default write tenant"] -def test_with_session_read_mode_does_not_commit(monkeypatch: pytest.MonkeyPatch) -> None: - session = FakeSession() - session_context = FakeSessionContext(session) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: session_context) +@pytest.mark.parametrize("sqlite_session", [(Tenant,)], indirect=True) +def test_with_session_write_rolls_back_on_error(sqlite_session: Session) -> None: + rollback_observed = False + injected_session: Session | None = None + + class Handler: + @session_module.with_session(write=True) + def get(self, session: Session): + nonlocal rollback_observed, injected_session + injected_session = session + + def observe_rollback(_session: Session) -> None: + nonlocal rollback_observed + rollback_observed = True + + event.listen(session, "after_rollback", observe_rollback) + session.add(Tenant(name="rolled back tenant")) + session.flush() + raise RuntimeError("boom") + + with _bind_session_factory(sqlite_session), pytest.raises(RuntimeError, match="boom"): + Handler().get() + + assert rollback_observed + assert injected_session is not None + assert not injected_session.in_transaction() + assert _tenant_names(sqlite_session) == [] + + +def test_with_session_write_allows_commit_then_more_database_work(sqlite_engine: Engine) -> None: + with Session(sqlite_engine) as sqlite_session, _bind_session_factory(sqlite_session): + + class Handler: + @session_module.with_session + def post(self, session: Session): + session.commit() + return session.scalar(select(literal(1))) + + assert Handler().post() == 1 + + +@pytest.mark.parametrize("sqlite_session", [(Tenant,)], indirect=True) +def test_with_session_read_mode_does_not_commit(sqlite_session: Session) -> None: + injected_session: Session | None = None class Handler: @session_module.with_session(write=False) - def get(self, injected_session): - assert injected_session is session + def get(self, session: Session): + nonlocal injected_session + injected_session = session + session.add(Tenant(name="uncommitted read tenant")) + session.flush() return "ok" - assert Handler().get() == "ok" + with _bind_session_factory(sqlite_session): + assert Handler().get() == "ok" - assert session.closed - assert not session.committed - assert not session.rolled_back - assert session_context.entered - assert session_context.exited - assert session_context.exc_type is None + assert injected_session is not None + assert not injected_session.in_transaction() + assert _tenant_names(sqlite_session) == [] -def test_with_session_preserves_wrapped_metadata(monkeypatch: pytest.MonkeyPatch) -> None: - session = FakeSession() - session_context = FakeSessionContext(session) - monkeypatch.setattr(session_module.session_factory, "create_session", lambda: session_context) - +def test_with_session_preserves_wrapped_metadata() -> None: class Handler: @session_module.with_session - def get(self, _session): + def get(self, _session: Session): """handler docs""" return "ok" diff --git a/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_apis.py b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_apis.py new file mode 100644 index 00000000000..8d306a85dcf --- /dev/null +++ b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_apis.py @@ -0,0 +1,738 @@ +"""Unit tests for Service API dataset controller behavior. + +Service boundaries stay mocked, while ORM collaborators are concrete model instances +persisted in one in-memory SQLite session. The controller's ``db.session`` and the +session passed to unwrapped ``@with_session`` endpoints both use that same session, +so model properties and service call contracts exercise real SQLAlchemy behavior. +""" + +import uuid +from datetime import UTC, datetime +from inspect import unwrap +from typing import cast +from unittest.mock import MagicMock, patch + +import pytest +from flask import Flask +from sqlalchemy.orm import Session, scoped_session, sessionmaker +from werkzeug.exceptions import Forbidden, NotFound + +import services +from controllers.service_api.dataset.error import DatasetInUseError, DatasetNameDuplicateError, InvalidActionError +from extensions.ext_database import db +from models.account import Account, Tenant, TenantAccountRole +from models.dataset import AppDatasetJoin, Dataset, DatasetMetadata, Document +from models.enums import PermissionEnum +from models.model import App, Tag, TagBinding + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- +DATASET_MODEL_TABLES = ( + Account, + Tenant, + Dataset, + Document, + App, + AppDatasetJoin, + DatasetMetadata, + Tag, + TagBinding, +) +pytestmark = pytest.mark.parametrize("sqlite_session", [DATASET_MODEL_TABLES], indirect=True) + + +@pytest.fixture(autouse=True) +def controller_session(sqlite_session: Session, monkeypatch: pytest.MonkeyPatch) -> Session: + """Route controller and model database access through the test's SQLite session.""" + + # Flask-SQLAlchemy exposes a callable registry that also proxies Session methods. + # Seed that registry with this fixture's Session so both access styles share one transaction. + existing_session_factory = cast(sessionmaker[Session], lambda: sqlite_session) + session_registry = scoped_session(existing_session_factory) + monkeypatch.setattr(db, "session", session_registry) + return sqlite_session + + +@pytest.fixture +def tenant(controller_session: Session) -> Tenant: + tenant = Tenant(name="Dataset API Tenant") + controller_session.add(tenant) + controller_session.flush() + return tenant + + +@pytest.fixture +def account(controller_session: Session, tenant: Tenant, monkeypatch: pytest.MonkeyPatch) -> Account: + account = Account(name="Dataset API User", email=f"dataset-api-{uuid.uuid4()}@example.com") + account.role = TenantAccountRole.OWNER + account._current_tenant = tenant + controller_session.add(account) + controller_session.flush() + + # Inject the concrete account at the controller boundary without relying on Flask-Login globals. + from controllers.service_api.dataset import dataset as dataset_module + + monkeypatch.setattr(dataset_module, "current_user", account) + return account + + +def make_dataset( + session: Session, + tenant: Tenant, + account: Account, + **overrides: object, +) -> Dataset: + """Create and flush a real dataset so its database-backed properties can be serialized.""" + + base: dict[str, object] = { + "id": str(uuid.uuid4()), + "tenant_id": tenant.id, + "name": "Dataset", + "description": "desc", + "provider": "vendor", + "permission": PermissionEnum.ONLY_ME, + "data_source_type": None, + "indexing_technique": "economy", + "created_by": account.id, + "created_at": datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC), + "updated_by": None, + "updated_at": datetime(2024, 1, 1, 12, 0, 0, tzinfo=UTC), + "embedding_model": None, + "embedding_model_provider": None, + "retrieval_model": None, + "summary_index_setting": None, + "built_in_field_enabled": False, + "pipeline_id": None, + "runtime_mode": "general", + "chunk_structure": None, + "icon_info": None, + "enable_api": False, + "is_multimodal": False, + } + base.update(overrides) + dataset = Dataset(**base) + session.add(dataset) + session.flush() + return dataset + + +@pytest.fixture +def dataset(controller_session: Session, tenant: Tenant, account: Account) -> Dataset: + return make_dataset(controller_session, tenant, account) + + +DATASET_DETAIL_KEYS = { + "id", + "name", + "description", + "provider", + "permission", + "data_source_type", + "indexing_technique", + "app_count", + "document_count", + "word_count", + "created_by", + "author_name", + "created_at", + "updated_by", + "updated_at", + "embedding_model", + "embedding_model_provider", + "embedding_available", + "retrieval_model_dict", + "summary_index_setting", + "tags", + "doc_form", + "external_knowledge_info", + "external_retrieval_model", + "doc_metadata", + "built_in_field_enabled", + "pipeline_id", + "runtime_mode", + "chunk_structure", + "icon_info", + "is_published", + "total_documents", + "total_available_documents", + "enable_api", + "is_multimodal", + "maintainer", +} + + +def assert_dataset_detail_shape(response: dict[str, object], *, with_partial_members: bool = False) -> None: + expected_keys = set(DATASET_DETAIL_KEYS) + if with_partial_members: + expected_keys.add("partial_member_list") + assert set(response) == expected_keys + assert isinstance(response["created_at"], int) + assert isinstance(response["updated_at"], int) + retrieval_model = response["retrieval_model_dict"] + assert isinstance(retrieval_model, dict) + assert set(retrieval_model) == { + "search_method", + "reranking_enable", + "reranking_mode", + "reranking_model", + "weights", + "top_k", + "score_threshold_enabled", + "score_threshold", + } + external_retrieval_model = response["external_retrieval_model"] + if external_retrieval_model is not None: + assert isinstance(external_retrieval_model, dict) + assert set(external_retrieval_model) == { + "top_k", + "score_threshold", + "score_threshold_enabled", + } + if not with_partial_members: + assert "partial_member_list" not in response + + +# --------------------------------------------------------------------------- +# API endpoint tests — DatasetListApi +# --------------------------------------------------------------------------- + + +class TestDatasetListApiGet: + """Test suite for DatasetListApi.get() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_list_datasets_success( + self, + mock_dataset_svc: MagicMock, + mock_provider_mgr: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetListApi + + mock_dataset_svc.get_datasets.return_value = ([make_dataset(controller_session, tenant, account)], 1) + mock_provider_mgr.return_value.get_configurations.return_value.get_models.return_value = list[object]() + + with app.test_request_context("/datasets?page=1&limit=20", method="GET"): + api = DatasetListApi() + response, status = unwrap(api.get)(api, controller_session, tenant_id=tenant.id) + + assert status == 200 + assert set(response) == {"data", "has_more", "limit", "total", "page"} + assert response["has_more"] is False + assert response["limit"] == 20 + assert response["total"] == 1 + assert response["page"] == 1 + assert len(response["data"]) == 1 + assert_dataset_detail_shape(response["data"][0]) + + @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_list_datasets_preserves_repeated_tag_ids( + self, + mock_dataset_svc: MagicMock, + mock_provider_mgr: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetListApi + + mock_dataset_svc.get_datasets.return_value = ([make_dataset(controller_session, tenant, account)], 1) + mock_provider_mgr.return_value.get_configurations.return_value.get_models.return_value = list[object]() + + with app.test_request_context("/datasets?tag_ids=tag-a&tag_ids=tag-b", method="GET"): + api = DatasetListApi() + response, status = unwrap(api.get)(api, controller_session, tenant_id=tenant.id) + page, limit, session, tenant_id, user, keyword, tag_ids, include_all = ( + mock_dataset_svc.get_datasets.call_args.args + ) + assert user is account + + assert status == 200 + assert response["total"] == 1 + assert (page, limit, session, tenant_id, keyword, tag_ids, include_all) == ( + 1, + 20, + controller_session, + tenant.id, + None, + ["tag-a", "tag-b"], + False, + ) + + +class TestDatasetListApiPost: + """Test suite for DatasetListApi.post() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_create_dataset_success( + self, + mock_dataset_svc: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetListApi + + mock_dataset_svc.create_empty_dataset.return_value = make_dataset( + controller_session, tenant, account, name="New Dataset" + ) + + with app.test_request_context( + "/datasets", + method="POST", + json={"name": "New Dataset"}, + ): + api = DatasetListApi() + response, status = unwrap(api.post)(api, controller_session, tenant_id=tenant.id) + + assert status == 200 + assert_dataset_detail_shape(response) + assert response["name"] == "New Dataset" + mock_dataset_svc.create_empty_dataset.assert_called_once() + + @pytest.mark.usefixtures("account") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_create_dataset_duplicate_name( + self, + mock_dataset_svc: MagicMock, + app: Flask, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetListApi + + mock_dataset_svc.create_empty_dataset.side_effect = services.errors.dataset.DatasetNameDuplicateError() + + with app.test_request_context( + "/datasets", + method="POST", + json={"name": "Existing Dataset"}, + ): + api = DatasetListApi() + with pytest.raises(DatasetNameDuplicateError): + unwrap(api.post)(api, controller_session, tenant_id=tenant.id) + + +# --------------------------------------------------------------------------- +# API endpoint tests — DatasetApi +# --------------------------------------------------------------------------- + + +class TestDatasetApiGet: + """Test suite for DatasetApi.get() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_get_dataset_success( + self, + mock_dataset_svc: MagicMock, + mock_provider_mgr: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_provider_mgr.return_value.get_configurations.return_value.get_models.return_value = list[object]() + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="GET", + ): + api = DatasetApi() + response, status = unwrap(api.get)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + assert status == 200 + assert_dataset_detail_shape(response) + assert response["embedding_available"] is True + assert response["retrieval_model_dict"]["search_method"] == "keyword_search" + + @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") + @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_get_dataset_partial_members_shape( + self, + mock_dataset_svc: MagicMock, + mock_provider_mgr: MagicMock, + mock_perm_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + dataset.permission = PermissionEnum.PARTIAL_TEAM + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_perm_svc.get_dataset_partial_member_list.return_value = ["user-1", "user-2"] + mock_provider_mgr.return_value.get_configurations.return_value.get_models.return_value = list[object]() + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="GET", + ): + api = DatasetApi() + response, status = unwrap(api.get)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + assert status == 200 + assert_dataset_detail_shape(response, with_partial_members=True) + assert response["partial_member_list"] == ["user-1", "user-2"] + + @patch("controllers.service_api.dataset.dataset.create_plugin_provider_manager") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_get_dataset_uses_default_external_retrieval_model( + self, + mock_dataset_svc: MagicMock, + mock_provider_mgr: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + dataset.retrieval_model = None + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_provider_mgr.return_value.get_configurations.return_value.get_models.return_value = list[object]() + + with app.test_request_context(f"/datasets/{dataset.id}", method="GET"): + api = DatasetApi() + response, status = unwrap(api.get)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + assert status == 200 + assert_dataset_detail_shape(response) + assert response["external_retrieval_model"] == { + "top_k": 2, + "score_threshold": 0.0, + "score_threshold_enabled": None, + } + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_get_dataset_not_found( + self, + mock_dataset_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.get_dataset.return_value = None + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="GET", + ): + api = DatasetApi() + with pytest.raises(NotFound): + unwrap(api.get)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_get_dataset_no_permission( + self, + mock_dataset_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.side_effect = services.errors.account.NoPermissionError() + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="GET", + ): + api = DatasetApi() + with pytest.raises(Forbidden): + unwrap(api.get)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + +class TestDatasetApiPatch: + """Test suite for DatasetApi.patch() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_patch_dataset_success_shape( + self, + mock_dataset_svc: MagicMock, + mock_perm_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + dataset.name = "Updated Dataset" + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.update_dataset.return_value = dataset + mock_perm_svc.check_permission.return_value = None + mock_perm_svc.get_dataset_partial_member_list.return_value = ["user-1"] + + payload = { + "name": "Updated Dataset", + "permission": "partial_members", + "partial_member_list": [{"user_id": "user-1", "role": "editor"}], + } + with app.test_request_context( + f"/datasets/{dataset.id}", + method="PATCH", + json=payload, + ): + api = DatasetApi() + response, status = unwrap(api.patch)( + api, + controller_session, + _=dataset.tenant_id, + dataset_id=dataset.id, + ) + + assert status == 200 + assert_dataset_detail_shape(response, with_partial_members=True) + assert response["name"] == "Updated Dataset" + assert response["partial_member_list"] == ["user-1"] + mock_dataset_svc.update_dataset.assert_called_once() + _, update_data, _ = mock_dataset_svc.update_dataset.call_args.args + session = mock_dataset_svc.update_dataset.call_args.kwargs["session"] + assert session is controller_session + assert update_data["name"] == "Updated Dataset" + assert update_data["permission"] == "partial_members" + mock_perm_svc.update_partial_member_list.assert_called_once_with( + dataset.tenant_id, + dataset.id, + [{"user_id": "user-1", "role": "editor"}], + controller_session, + ) + + +class TestDatasetApiDelete: + """Test suite for DatasetApi.delete() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.DatasetPermissionService") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_delete_dataset_success( + self, + mock_dataset_svc: MagicMock, + mock_perm_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.delete_dataset.return_value = True + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="DELETE", + ): + api = DatasetApi() + result = unwrap(api.delete)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + assert result == ("", 204) + mock_perm_svc.clear_partial_member_list.assert_called_once_with(dataset.id, controller_session) + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_delete_dataset_not_found( + self, + mock_dataset_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.delete_dataset.return_value = False + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="DELETE", + ): + api = DatasetApi() + with pytest.raises(NotFound): + unwrap(api.delete)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_delete_dataset_in_use( + self, + mock_dataset_svc: MagicMock, + app: Flask, + dataset: Dataset, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetApi + + mock_dataset_svc.delete_dataset.side_effect = services.errors.dataset.DatasetInUseError() + + with app.test_request_context( + f"/datasets/{dataset.id}", + method="DELETE", + ): + api = DatasetApi() + with pytest.raises(DatasetInUseError): + unwrap(api.delete)(api, controller_session, _=dataset.tenant_id, dataset_id=dataset.id) + + +# --------------------------------------------------------------------------- +# API endpoint tests — DocumentStatusApi +# --------------------------------------------------------------------------- + + +class TestDocumentStatusApiPatch: + """Test suite for DocumentStatusApi.patch() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.DocumentService") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_batch_update_status_success( + self, + mock_dataset_svc: MagicMock, + mock_doc_svc: MagicMock, + app: Flask, + tenant: Tenant, + dataset: Dataset, + ) -> None: + from controllers.service_api.dataset.dataset import DocumentStatusApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_dataset_svc.check_dataset_model_setting.return_value = None + mock_doc_svc.batch_update_document_status.return_value = None + + with app.test_request_context( + f"/datasets/{dataset.id}/documents/status/enable", + method="PATCH", + json={"document_ids": ["doc-1", "doc-2"]}, + ): + api = DocumentStatusApi() + response, status = api.patch( + tenant_id=tenant.id, + dataset_id=dataset.id, + action="enable", + ) + + assert status == 200 + assert response["result"] == "success" + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_batch_update_status_dataset_not_found( + self, + mock_dataset_svc: MagicMock, + app: Flask, + tenant: Tenant, + dataset: Dataset, + ) -> None: + from controllers.service_api.dataset.dataset import DocumentStatusApi + + mock_dataset_svc.get_dataset.return_value = None + + with app.test_request_context( + f"/datasets/{dataset.id}/documents/status/enable", + method="PATCH", + json={"document_ids": ["doc-1"]}, + ): + api = DocumentStatusApi() + with pytest.raises(NotFound): + api.patch( + tenant_id=tenant.id, + dataset_id=dataset.id, + action="enable", + ) + + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_batch_update_status_permission_error( + self, + mock_dataset_svc: MagicMock, + app: Flask, + tenant: Tenant, + dataset: Dataset, + ) -> None: + from controllers.service_api.dataset.dataset import DocumentStatusApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.side_effect = services.errors.account.NoPermissionError( + "No permission" + ) + + with app.test_request_context( + f"/datasets/{dataset.id}/documents/status/enable", + method="PATCH", + json={"document_ids": ["doc-1"]}, + ): + api = DocumentStatusApi() + with pytest.raises(Forbidden): + api.patch( + tenant_id=tenant.id, + dataset_id=dataset.id, + action="enable", + ) + + @patch("controllers.service_api.dataset.dataset.DocumentService") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_batch_update_status_indexing_error( + self, + mock_dataset_svc: MagicMock, + mock_doc_svc: MagicMock, + app: Flask, + tenant: Tenant, + dataset: Dataset, + ) -> None: + from controllers.service_api.dataset.dataset import DocumentStatusApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_dataset_svc.check_dataset_model_setting.return_value = None + mock_doc_svc.batch_update_document_status.side_effect = services.errors.document.DocumentIndexingError() + + with app.test_request_context( + f"/datasets/{dataset.id}/documents/status/enable", + method="PATCH", + json={"document_ids": ["doc-1"]}, + ): + api = DocumentStatusApi() + with pytest.raises(InvalidActionError): + api.patch( + tenant_id=tenant.id, + dataset_id=dataset.id, + action="enable", + ) + + @patch("controllers.service_api.dataset.dataset.DocumentService") + @patch("controllers.service_api.dataset.dataset.DatasetService") + def test_batch_update_status_value_error( + self, + mock_dataset_svc: MagicMock, + mock_doc_svc: MagicMock, + app: Flask, + tenant: Tenant, + dataset: Dataset, + ) -> None: + from controllers.service_api.dataset.dataset import DocumentStatusApi + + mock_dataset_svc.get_dataset.return_value = dataset + mock_dataset_svc.check_dataset_permission.return_value = None + mock_dataset_svc.check_dataset_model_setting.return_value = None + mock_doc_svc.batch_update_document_status.side_effect = ValueError("Invalid action") + + with app.test_request_context( + f"/datasets/{dataset.id}/documents/status/enable", + method="PATCH", + json={"document_ids": ["doc-1"]}, + ): + api = DocumentStatusApi() + with pytest.raises(InvalidActionError): + api.patch( + tenant_id=tenant.id, + dataset_id=dataset.id, + action="enable", + ) diff --git a/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_payloads.py b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_payloads.py new file mode 100644 index 00000000000..d6755e957a6 --- /dev/null +++ b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_payloads.py @@ -0,0 +1,207 @@ +"""Unit tests for Service API dataset request payloads.""" + +from typing import Literal + +import pytest + +from controllers.service_api.dataset.dataset import ( + DatasetCreatePayload, + DatasetListQuery, + DatasetUpdatePayload, + TagBindingPayload, + TagCreatePayload, + TagDeletePayload, + TagUnbindingPayload, + TagUpdatePayload, +) +from models.dataset import DatasetPermissionEnum + + +class TestDatasetCreatePayload: + """Test suite for DatasetCreatePayload Pydantic model.""" + + def test_payload_with_required_name(self) -> None: + payload = DatasetCreatePayload(name="Test Dataset") + assert payload.name == "Test Dataset" + assert payload.description == "" + assert payload.permission == DatasetPermissionEnum.ONLY_ME + + def test_payload_with_all_fields(self) -> None: + payload = DatasetCreatePayload( + name="Full Dataset", + description="A comprehensive dataset description", + indexing_technique="high_quality", + permission=DatasetPermissionEnum.ALL_TEAM, + provider="vendor", + embedding_model="text-embedding-ada-002", + embedding_model_provider="openai", + ) + assert payload.name == "Full Dataset" + assert payload.description == "A comprehensive dataset description" + assert payload.indexing_technique == "high_quality" + assert payload.permission == DatasetPermissionEnum.ALL_TEAM + assert payload.provider == "vendor" + assert payload.embedding_model == "text-embedding-ada-002" + assert payload.embedding_model_provider == "openai" + + def test_payload_name_length_validation_min(self) -> None: + with pytest.raises(ValueError): + DatasetCreatePayload(name="") + + def test_payload_name_length_validation_max(self) -> None: + with pytest.raises(ValueError): + DatasetCreatePayload(name="A" * 41) + + def test_payload_description_max_length(self) -> None: + with pytest.raises(ValueError): + DatasetCreatePayload(name="Dataset", description="A" * 401) + + @pytest.mark.parametrize("technique", ["high_quality", "economy"]) + def test_payload_valid_indexing_techniques(self, technique: Literal["high_quality", "economy"]) -> None: + payload = DatasetCreatePayload(name="Dataset", indexing_technique=technique) + assert payload.indexing_technique == technique + + def test_payload_with_external_knowledge_settings(self) -> None: + payload = DatasetCreatePayload( + name="External Dataset", external_knowledge_api_id="api_123", external_knowledge_id="knowledge_456" + ) + assert payload.external_knowledge_api_id == "api_123" + assert payload.external_knowledge_id == "knowledge_456" + + +class TestDatasetUpdatePayload: + """Test suite for DatasetUpdatePayload Pydantic model.""" + + def test_payload_all_optional(self) -> None: + payload = DatasetUpdatePayload() + assert payload.name is None + assert payload.description is None + assert payload.permission is None + + def test_payload_with_partial_update(self) -> None: + payload = DatasetUpdatePayload(name="Updated Name", description="Updated description") + assert payload.name == "Updated Name" + assert payload.description == "Updated description" + + def test_payload_with_permission_change(self) -> None: + payload = DatasetUpdatePayload( + permission=DatasetPermissionEnum.PARTIAL_TEAM, + partial_member_list=[{"user_id": "user_123", "role": "editor"}], + ) + assert payload.permission == DatasetPermissionEnum.PARTIAL_TEAM + assert payload.partial_member_list is not None + assert len(payload.partial_member_list) == 1 + + def test_payload_name_length_validation(self) -> None: + with pytest.raises(ValueError): + DatasetUpdatePayload(name="") + with pytest.raises(ValueError): + DatasetUpdatePayload(name="A" * 41) + + +class TestDatasetListQuery: + """Test suite for DatasetListQuery Pydantic model.""" + + def test_query_with_defaults(self) -> None: + query = DatasetListQuery() + assert query.page == 1 + assert query.limit == 20 + assert query.keyword is None + assert query.include_all is False + assert query.tag_ids == [] + + def test_query_with_all_filters(self) -> None: + query = DatasetListQuery( + page=3, limit=50, keyword="machine learning", include_all=True, tag_ids=["tag1", "tag2", "tag3"] + ) + assert query.page == 3 + assert query.limit == 50 + assert query.keyword == "machine learning" + assert query.include_all is True + assert len(query.tag_ids) == 3 + + def test_query_with_tag_filter(self) -> None: + query = DatasetListQuery(tag_ids=["tag_abc", "tag_def"]) + assert query.tag_ids == ["tag_abc", "tag_def"] + + +class TestTagCreatePayload: + """Test suite for TagCreatePayload Pydantic model.""" + + def test_payload_with_name(self) -> None: + payload = TagCreatePayload(name="New Tag") + assert payload.name == "New Tag" + + def test_payload_name_length_min(self) -> None: + with pytest.raises(ValueError): + TagCreatePayload(name="") + + def test_payload_name_length_max(self) -> None: + with pytest.raises(ValueError): + TagCreatePayload(name="A" * 51) + + def test_payload_with_unicode_name(self) -> None: + payload = TagCreatePayload(name="标签 🏷️ Тег") + assert payload.name == "标签 🏷️ Тег" + + +class TestTagUpdatePayload: + """Test suite for TagUpdatePayload Pydantic model.""" + + def test_payload_with_name_and_id(self) -> None: + payload = TagUpdatePayload(name="Updated Tag", tag_id="tag_123") + assert payload.name == "Updated Tag" + assert payload.tag_id == "tag_123" + + def test_payload_requires_tag_id(self) -> None: + with pytest.raises(ValueError): + TagUpdatePayload.model_validate({"name": "Updated Tag"}) + + +class TestTagDeletePayload: + """Test suite for TagDeletePayload Pydantic model.""" + + def test_payload_with_tag_id(self) -> None: + payload = TagDeletePayload(tag_id="tag_to_delete") + assert payload.tag_id == "tag_to_delete" + + def test_payload_requires_tag_id(self) -> None: + with pytest.raises(ValueError): + TagDeletePayload.model_validate({}) + + +class TestTagBindingPayload: + """Test suite for TagBindingPayload Pydantic model.""" + + def test_payload_with_valid_data(self) -> None: + payload = TagBindingPayload(tag_ids=["tag1", "tag2"], target_id="dataset_123") + assert len(payload.tag_ids) == 2 + assert payload.target_id == "dataset_123" + + def test_payload_rejects_empty_tag_ids(self) -> None: + with pytest.raises(ValueError) as exc_info: + TagBindingPayload(tag_ids=[], target_id="dataset_123") + assert "Tag IDs is required" in str(exc_info.value) + + def test_payload_single_tag_id(self) -> None: + payload = TagBindingPayload(tag_ids=["single_tag"], target_id="dataset_456") + assert payload.tag_ids == ["single_tag"] + + +class TestTagUnbindingPayload: + """Test suite for TagUnbindingPayload Pydantic model.""" + + def test_payload_with_valid_data(self) -> None: + payload = TagUnbindingPayload(tag_ids=["tag_123"], target_id="dataset_456") + assert payload.tag_ids == ["tag_123"] + assert payload.target_id == "dataset_456" + + def test_payload_normalizes_legacy_tag_id(self) -> None: + payload = TagUnbindingPayload(tag_id="tag_123", target_id="dataset_456") + assert payload.tag_ids == ["tag_123"] + assert payload.target_id == "dataset_456" + + def test_payload_rejects_empty_tag_ids(self) -> None: + with pytest.raises(ValueError) as exc_info: + TagUnbindingPayload(tag_ids=[], target_id="dataset_456") + assert "Tag IDs is required" in str(exc_info.value) diff --git a/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_tag_apis.py b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_tag_apis.py new file mode 100644 index 00000000000..420b1fa1bf7 --- /dev/null +++ b/api/tests/unit_tests/controllers/service_api/dataset/test_dataset_tag_apis.py @@ -0,0 +1,380 @@ +"""Unit tests for Service API dataset tag controller behavior. + +Service boundaries stay mocked, while users, tenants, and tags are real ORM objects +persisted in SQLite. Controller database calls share that SQLite session so assertions +cover the concrete objects and session passed across the controller boundary. +""" + +import uuid +from inspect import unwrap +from typing import cast +from unittest.mock import MagicMock, patch + +import pytest +from flask import Flask +from sqlalchemy.orm import Session, scoped_session, sessionmaker +from werkzeug.exceptions import Forbidden + +from extensions.ext_database import db +from models.account import Account, Tenant, TenantAccountRole +from models.enums import TagType +from models.model import Tag + +TAG_MODEL_TABLES = (Account, Tenant, Tag) +pytestmark = pytest.mark.parametrize("sqlite_session", [TAG_MODEL_TABLES], indirect=True) + + +@pytest.fixture(autouse=True) +def controller_session(sqlite_session: Session, monkeypatch: pytest.MonkeyPatch) -> Session: + """Route controller database access through the test's SQLite session.""" + + # Flask-SQLAlchemy exposes a callable registry that also proxies Session methods. + # Seed that registry with this fixture's Session so both access styles share one transaction. + existing_session_factory = cast(sessionmaker[Session], lambda: sqlite_session) + session_registry = scoped_session(existing_session_factory) + monkeypatch.setattr(db, "session", session_registry) + return sqlite_session + + +@pytest.fixture +def tenant(controller_session: Session) -> Tenant: + tenant = Tenant(name="Dataset Tag API Tenant") + controller_session.add(tenant) + controller_session.flush() + return tenant + + +@pytest.fixture +def account(controller_session: Session, tenant: Tenant, monkeypatch: pytest.MonkeyPatch) -> Account: + account = Account(name="Dataset Tag API User", email=f"dataset-tag-api-{uuid.uuid4()}@example.com") + account.role = TenantAccountRole.OWNER + account._current_tenant = tenant + controller_session.add(account) + controller_session.flush() + + # Inject the concrete account at the controller boundary without relying on Flask-Login globals. + from controllers.service_api.dataset import dataset as dataset_module + + monkeypatch.setattr(dataset_module, "current_user", account) + return account + + +def make_tag( + session: Session, + tenant: Tenant, + account: Account, + *, + id: str, + name: str, + binding_count: int | None = None, +) -> Tag: + """Create and flush a real tag, optionally adding the aggregate count returned by TagService.""" + + tag = Tag(tenant_id=tenant.id, type=TagType.KNOWLEDGE, name=name, created_by=account.id) + tag.id = id + session.add(tag) + session.flush() + if binding_count is not None: + tag.__dict__["binding_count"] = binding_count + return tag + + +class TestDatasetTagsApiGet: + """Test suite for DatasetTagsApi.get() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.TagService") + def test_list_tags_success( + self, + mock_tag_svc: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + tag = make_tag(controller_session, tenant, account, id="tag-1", name="Test Tag", binding_count=0) + mock_tag_svc.get_tags.return_value = [tag] + + with app.test_request_context("/datasets/tags", method="GET"): + api = DatasetTagsApi() + response, status = unwrap(api.get)(api, controller_session, _=None) + + assert status == 200 + assert response == [{"id": "tag-1", "name": "Test Tag", "type": "knowledge", "binding_count": "0"}] + mock_tag_svc.get_tags.assert_called_once_with("knowledge", tenant.id, session=controller_session) + + +class TestDatasetTagsApiPost: + """Test suite for DatasetTagsApi.post() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.TagService") + def test_create_tag_success( + self, + mock_tag_svc: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + tag = make_tag(controller_session, tenant, account, id="tag-new", name="New Tag") + mock_tag_svc.save_tags.return_value = tag + + with app.test_request_context( + "/datasets/tags", + method="POST", + json={"name": "New Tag"}, + ): + api = DatasetTagsApi() + response, status = unwrap(api.post)(api, controller_session, _=None) + + assert status == 200 + assert response == {"id": "tag-new", "name": "New Tag", "type": "knowledge", "binding_count": "0"} + mock_tag_svc.save_tags.assert_called_once() + + def test_create_tag_forbidden(self, app: Flask, account: Account) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + account.role = TenantAccountRole.NORMAL + + with app.test_request_context( + "/datasets/tags", + method="POST", + json={"name": "New Tag"}, + ): + api = DatasetTagsApi() + with pytest.raises(Forbidden): + api.post(_=None) + + +class TestDatasetTagsApiPatch: + """Test suite for DatasetTagsApi.patch() endpoint.""" + + @patch("controllers.service_api.dataset.dataset.TagService") + @patch("controllers.service_api.dataset.dataset.service_api_ns") + def test_update_tag_success( + self, + mock_service_api_ns: MagicMock, + mock_tag_svc: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + tag = make_tag(controller_session, tenant, account, id="tag-1", name="Updated Tag") + mock_tag_svc.update_tags.return_value = tag + mock_tag_svc.get_tag_binding_count.return_value = 5 + mock_service_api_ns.payload = {"name": "Updated Tag", "tag_id": "tag-1"} + + with app.test_request_context( + "/datasets/tags", + method="PATCH", + json={"name": "Updated Tag", "tag_id": "tag-1"}, + ): + api = DatasetTagsApi() + response, status = unwrap(api.patch)(api, controller_session, _=None) + + assert status == 200 + assert response == {"id": "tag-1", "name": "Updated Tag", "type": "knowledge", "binding_count": "5"} + mock_tag_svc.update_tags.assert_called_once() + update_payload, tag_id, session = mock_tag_svc.update_tags.call_args.args + assert update_payload.name == "Updated Tag" + assert tag_id == "tag-1" + assert session is controller_session + + def test_update_tag_forbidden(self, app: Flask, account: Account) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + account.role = TenantAccountRole.NORMAL + + with app.test_request_context( + "/datasets/tags", + method="PATCH", + json={"name": "Updated Tag", "tag_id": "tag-1"}, + ): + api = DatasetTagsApi() + with pytest.raises(Forbidden): + api.patch(_=None) + + +class TestDatasetTagsApiDelete: + """Test suite for DatasetTagsApi.delete() endpoint.""" + + @pytest.mark.usefixtures("account") + @patch("controllers.service_api.dataset.dataset.TagService") + @patch("controllers.service_api.dataset.dataset.service_api_ns") + def test_delete_tag_success( + self, + mock_service_api_ns: MagicMock, + mock_tag_svc: MagicMock, + app: Flask, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsApi + + mock_tag_svc.delete_tag.return_value = None + mock_service_api_ns.payload = {"tag_id": "tag-1"} + + with app.test_request_context( + "/datasets/tags", + method="DELETE", + json={"tag_id": "tag-1"}, + ): + api = DatasetTagsApi() + result = unwrap(api.delete)(api, controller_session, _=None) + + assert result == ("", 204) + mock_tag_svc.delete_tag.assert_called_once_with("tag-1", controller_session, tag_type=TagType.KNOWLEDGE) + + +class TestDatasetTagsBindingStatusApi: + """Test suite for DatasetTagsBindingStatusApi endpoints.""" + + @patch("controllers.service_api.dataset.dataset.TagService") + def test_get_dataset_tags_binding_status( + self, + mock_tag_svc: MagicMock, + app: Flask, + account: Account, + tenant: Tenant, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagsBindingStatusApi + + tag = make_tag(controller_session, tenant, account, id="tag_1", name="Test Tag") + mock_tag_svc.get_tags_by_target_id.return_value = [tag] + + with app.test_request_context("/", method="GET"): + api = DatasetTagsBindingStatusApi() + response, status_code = unwrap(api.get)(api, controller_session, tenant.id, dataset_id="dataset_123") + + assert status_code == 200 + assert response["data"] == [{"id": "tag_1", "name": "Test Tag"}] + assert response["total"] == 1 + mock_tag_svc.get_tags_by_target_id.assert_called_once_with( + "knowledge", tenant.id, "dataset_123", controller_session + ) + + +class TestDatasetTagBindingApiPost: + """Test suite for DatasetTagBindingApi.post() endpoint.""" + + @pytest.mark.usefixtures("account") + @patch("controllers.service_api.dataset.dataset.TagService") + def test_bind_tags_success( + self, + mock_tag_svc: MagicMock, + app: Flask, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagBindingApi + + mock_tag_svc.save_tag_binding.return_value = None + + with app.test_request_context( + "/datasets/tags/binding", + method="POST", + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, + ): + api = DatasetTagBindingApi() + result = unwrap(api.post)(api, controller_session, _=None) + + assert result == ("", 204) + from services.tag_service import TagBindingCreatePayload + + mock_tag_svc.save_tag_binding.assert_called_once_with( + TagBindingCreatePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), + controller_session, + ) + + def test_bind_tags_forbidden(self, app: Flask, account: Account) -> None: + from controllers.service_api.dataset.dataset import DatasetTagBindingApi + + account.role = TenantAccountRole.NORMAL + + with app.test_request_context( + "/datasets/tags/binding", + method="POST", + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, + ): + api = DatasetTagBindingApi() + with pytest.raises(Forbidden): + api.post(_=None) + + +class TestDatasetTagUnbindingApiPost: + """Test suite for DatasetTagUnbindingApi.post() endpoint.""" + + @pytest.mark.usefixtures("account") + @patch("controllers.service_api.dataset.dataset.TagService") + def test_unbind_tag_success( + self, + mock_tag_svc: MagicMock, + app: Flask, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi + + mock_tag_svc.delete_tag_binding.return_value = None + + with app.test_request_context( + "/datasets/tags/unbinding", + method="POST", + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, + ): + api = DatasetTagUnbindingApi() + result = unwrap(api.post)(api, controller_session, _=None) + + assert result == ("", 204) + from services.tag_service import TagBindingDeletePayload + + mock_tag_svc.delete_tag_binding.assert_called_once_with( + TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), + controller_session, + ) + + @pytest.mark.usefixtures("account") + @patch("controllers.service_api.dataset.dataset.TagService") + def test_unbind_legacy_tag_id_success( + self, + mock_tag_svc: MagicMock, + app: Flask, + controller_session: Session, + ) -> None: + from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi + + mock_tag_svc.delete_tag_binding.return_value = None + + with app.test_request_context( + "/datasets/tags/unbinding", + method="POST", + json={"tag_id": "tag-1", "target_id": "ds-1"}, + ): + api = DatasetTagUnbindingApi() + result = unwrap(api.post)(api, controller_session, _=None) + + assert result == ("", 204) + from services.tag_service import TagBindingDeletePayload + + mock_tag_svc.delete_tag_binding.assert_called_once_with( + TagBindingDeletePayload(tag_ids=["tag-1"], target_id="ds-1", type=TagType.KNOWLEDGE), + controller_session, + ) + + def test_unbind_tag_forbidden(self, app: Flask, account: Account) -> None: + from controllers.service_api.dataset.dataset import DatasetTagUnbindingApi + + account.role = TenantAccountRole.NORMAL + + with app.test_request_context( + "/datasets/tags/unbinding", + method="POST", + json={"tag_ids": ["tag-1"], "target_id": "ds-1"}, + ): + api = DatasetTagUnbindingApi() + with pytest.raises(Forbidden): + api.post(_=None) diff --git a/api/tests/unit_tests/core/app/apps/test_workflow_pause_events.py b/api/tests/unit_tests/core/app/apps/test_workflow_pause_events.py index c0052bc5bab..41064ea8830 100644 --- a/api/tests/unit_tests/core/app/apps/test_workflow_pause_events.py +++ b/api/tests/unit_tests/core/app/apps/test_workflow_pause_events.py @@ -3,6 +3,7 @@ from types import SimpleNamespace from unittest.mock import MagicMock import pytest +from sqlalchemy.orm import Session from core.app.apps.common import workflow_response_converter from core.app.apps.common.workflow_response_converter import WorkflowResponseConverter @@ -24,27 +25,7 @@ from graphon.entities.pause_reason import HitlRequired from graphon.graph_events import GraphRunPausedEvent from graphon.runtime import GraphRuntimeState, VariablePool from models.account import Account -from models.human_input import RecipientType - - -class _FakeSession: - """Stub session: `execute` feeds the form-expiration query, `scalars` the recipients.""" - - def __init__(self, *, execute_rows=(), scalars_rows=()): - self._execute_rows = execute_rows - self._scalars_rows = scalars_rows - - def execute(self, _stmt): - return list(self._execute_rows) - - def scalars(self, _stmt): - return list(self._scalars_rows) - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc, tb): - return False +from models.human_input import HumanInputForm, HumanInputFormRecipient, RecipientType class _RecordingWorkflowAppRunner(WorkflowAppRunner): @@ -63,6 +44,46 @@ class _FakeRuntimeState: return ["node-pause-1"] +@pytest.fixture +def sqlite_pause_session(sqlite_session: Session, monkeypatch: pytest.MonkeyPatch) -> Session: + """Bind pause-response queries to the shared SQLite session's database.""" + monkeypatch.setattr(workflow_response_converter, "db", SimpleNamespace(engine=sqlite_session.get_bind())) + return sqlite_session + + +def _persist_human_input_form( + session: Session, + *, + recipients: list[tuple[RecipientType, str]] | None = None, +) -> datetime: + expiration_time = datetime(2024, 1, 1, tzinfo=UTC) + form = HumanInputForm( + id="form-1", + tenant_id="tenant-id", + app_id="app-id", + workflow_run_id="run-id", + node_id="node-id", + form_definition='{"display_in_ui": true}', + rendered_content="Rendered", + expiration_time=expiration_time, + ) + recipient_models = [ + HumanInputFormRecipient( + id=f"recipient-{index}", + form_id=form.id, + delivery_id=f"delivery-{index}", + recipient_type=recipient_type, + recipient_payload="{}", + access_token=access_token, + ) + for index, (recipient_type, access_token) in enumerate(recipients or ()) + ] + session.add(form) + session.add_all(recipient_models) + session.commit() + return expiration_time + + def _build_runner(): app_entity = SimpleNamespace( app_config=SimpleNamespace(app_id="app-id"), @@ -154,7 +175,12 @@ def _build_converter(*, invoke_from: InvokeFrom = InvokeFrom.SERVICE_API): ) -def test_queue_workflow_paused_event_to_stream_responses(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize( + "sqlite_session", + [(HumanInputForm, HumanInputFormRecipient)], + indirect=True, +) +def test_queue_workflow_paused_event_to_stream_responses(sqlite_pause_session: Session): converter = _build_converter() converter.workflow_start_to_stream_response( task_id="task", @@ -163,18 +189,14 @@ def test_queue_workflow_paused_event_to_stream_responses(monkeypatch: pytest.Mon reason=WorkflowStartReason.INITIAL, ) - expiration_time = datetime(2024, 1, 1, tzinfo=UTC) - session = _FakeSession( - execute_rows=[("form-1", expiration_time, '{"display_in_ui": true}')], - scalars_rows=[ - SimpleNamespace(form_id="form-1", recipient_type=RecipientType.CONSOLE, access_token="console-token"), - SimpleNamespace(form_id="form-1", recipient_type=RecipientType.BACKSTAGE, access_token="backstage-token"), + expiration_time = _persist_human_input_form( + sqlite_pause_session, + recipients=[ + (RecipientType.CONSOLE, "console-token"), + (RecipientType.BACKSTAGE, "backstage-token"), ], ) - monkeypatch.setattr(workflow_response_converter, "Session", lambda **_: session) - monkeypatch.setattr(workflow_response_converter, "db", SimpleNamespace(engine=object())) - reason = HumanInputRequired( form_id="form-1", form_content="Rendered", @@ -216,8 +238,11 @@ def test_queue_workflow_paused_event_to_stream_responses(monkeypatch: pytest.Mon assert hi_resp.data.expiration_time == int(expiration_time.timestamp()) -def _build_paused_human_input_response(monkeypatch, recipients): - """Drive the live OPENAPI pause path with the given recipients via a fake session.""" +def _build_paused_human_input_response( + session: Session, + recipients: list[tuple[RecipientType, str]], +): + """Drive the live OPENAPI pause path with persisted forms and recipients.""" converter = _build_converter(invoke_from=InvokeFrom.OPENAPI) converter.workflow_start_to_stream_response( task_id="task", @@ -226,14 +251,7 @@ def _build_paused_human_input_response(monkeypatch, recipients): reason=WorkflowStartReason.INITIAL, ) - expiration_time = datetime(2024, 1, 1, tzinfo=UTC) - session = _FakeSession( - execute_rows=[("form-1", expiration_time, '{"display_in_ui": true}')], - scalars_rows=list(recipients), - ) - - monkeypatch.setattr(workflow_response_converter, "Session", lambda **_: session) - monkeypatch.setattr(workflow_response_converter, "db", SimpleNamespace(engine=object())) + _persist_human_input_form(session, recipients=recipients) reason = HumanInputRequired( form_id="form-1", @@ -259,12 +277,17 @@ def _build_paused_human_input_response(monkeypatch, recipients): return responses -def test_openapi_pause_without_web_app_recipient_emits_approval_channels(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize( + "sqlite_session", + [(HumanInputForm, HumanInputFormRecipient)], + indirect=True, +) +def test_openapi_pause_without_web_app_recipient_emits_approval_channels(sqlite_pause_session: Session): responses = _build_paused_human_input_response( - monkeypatch, + sqlite_pause_session, recipients=[ - SimpleNamespace(form_id="form-1", recipient_type=RecipientType.EMAIL_MEMBER, access_token="email-token"), - SimpleNamespace(form_id="form-1", recipient_type=RecipientType.BACKSTAGE, access_token="backstage-token"), + (RecipientType.EMAIL_MEMBER, "email-token"), + (RecipientType.BACKSTAGE, "backstage-token"), ], ) @@ -276,16 +299,17 @@ def test_openapi_pause_without_web_app_recipient_emits_approval_channels(monkeyp assert pause_resp.data.reasons[0]["approval_channels"] == ["console", "email"] -def test_openapi_pause_with_web_app_recipient_sets_token_and_channels(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize( + "sqlite_session", + [(HumanInputForm, HumanInputFormRecipient)], + indirect=True, +) +def test_openapi_pause_with_web_app_recipient_sets_token_and_channels(sqlite_pause_session: Session): responses = _build_paused_human_input_response( - monkeypatch, + sqlite_pause_session, recipients=[ - SimpleNamespace( - form_id="form-1", - recipient_type=RecipientType.STANDALONE_WEB_APP, - access_token="web-app-token", - ), - SimpleNamespace(form_id="form-1", recipient_type=RecipientType.BACKSTAGE, access_token="backstage-token"), + (RecipientType.STANDALONE_WEB_APP, "web-app-token"), + (RecipientType.BACKSTAGE, "backstage-token"), ], ) @@ -297,7 +321,12 @@ def test_openapi_pause_with_web_app_recipient_sets_token_and_channels(monkeypatc assert pause_resp.data.reasons[0]["approval_channels"] == ["console"] -def test_queue_workflow_paused_event_resolves_variable_select_options(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize( + "sqlite_session", + [(HumanInputForm, HumanInputFormRecipient)], + indirect=True, +) +def test_queue_workflow_paused_event_resolves_variable_select_options(sqlite_pause_session: Session): converter = _build_converter() converter.workflow_start_to_stream_response( task_id="task", @@ -306,11 +335,7 @@ def test_queue_workflow_paused_event_resolves_variable_select_options(monkeypatc reason=WorkflowStartReason.INITIAL, ) - expiration_time = datetime(2024, 1, 1, tzinfo=UTC) - session = _FakeSession(execute_rows=[("form-1", expiration_time, '{"display_in_ui": true}')]) - - monkeypatch.setattr(workflow_response_converter, "Session", lambda **_: session) - monkeypatch.setattr(workflow_response_converter, "db", SimpleNamespace(engine=object())) + _persist_human_input_form(sqlite_pause_session) reason = HumanInputRequired( form_id="form-1", diff --git a/api/tests/unit_tests/core/app/workflow/test_file_runtime.py b/api/tests/unit_tests/core/app/workflow/test_file_runtime.py index 0025c21f437..22b6f0c72bf 100644 --- a/api/tests/unit_tests/core/app/workflow/test_file_runtime.py +++ b/api/tests/unit_tests/core/app/workflow/test_file_runtime.py @@ -3,19 +3,85 @@ from __future__ import annotations import base64 import hashlib import hmac +from collections.abc import Iterator +from datetime import UTC, datetime from types import SimpleNamespace from unittest.mock import MagicMock, patch from urllib.parse import parse_qs, urlparse import pytest +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session, sessionmaker from core.app.entities.app_invoke_entities import InvokeFrom, UserFrom from core.app.file_access import DatabaseFileAccessController, FileAccessScope from core.app.workflow import file_runtime from core.app.workflow.file_runtime import DifyWorkflowFileRuntime, bind_dify_workflow_file_runtime from core.workflow.file_reference import build_file_reference +from extensions.storage.storage_type import StorageType from graphon.file import File, FileTransferMethod, FileType from models import ToolFile, UploadFile +from models.base import TypeBase +from models.enums import CreatorUserRole + + +@pytest.fixture +def file_session(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Iterator[Session]: + """Bind runtime-owned sessions to SQLite with only the two file tables present.""" + tables = [TypeBase.metadata.tables[model.__tablename__] for model in (UploadFile, ToolFile)] + TypeBase.metadata.create_all(sqlite_engine, tables=tables) + session_maker = sessionmaker(bind=sqlite_engine, expire_on_commit=False) + monkeypatch.setattr(file_runtime.session_factory, "create_session", session_maker) + with session_maker() as session: + yield session + + +def _persist_upload_file( + session: Session, + *, + file_id: str = "upload-file-id", + key: str = "canonical-storage-key", + tenant_id: str = "tenant-id", + created_by: str = "end-user-id", +) -> UploadFile: + upload_file = UploadFile( + tenant_id=tenant_id, + storage_type=StorageType.LOCAL, + key=key, + name="diagram.png", + size=128, + extension="png", + mime_type="image/png", + created_by_role=CreatorUserRole.END_USER, + created_by=created_by, + created_at=datetime(2024, 1, 1, tzinfo=UTC), + used=False, + ) + upload_file.id = file_id + session.add(upload_file) + session.commit() + return upload_file + + +def _persist_tool_file( + session: Session, + *, + file_id: str = "tool-file-id", + key: str = "tool-storage-key", +) -> ToolFile: + tool_file = ToolFile( + user_id="end-user-id", + tenant_id="tenant-id", + conversation_id=None, + file_key=key, + mimetype="image/png", + name="diagram.png", + size=128, + ) + tool_file.id = file_id + session.add(tool_file) + session.commit() + return tool_file def _build_file( @@ -164,56 +230,37 @@ def test_verify_preview_signature_validates_signature_and_expiration(monkeypatch ) -def test_load_file_bytes_returns_bytes_and_rejects_non_bytes(monkeypatch: pytest.MonkeyPatch) -> None: +def test_load_file_bytes_returns_bytes_and_rejects_non_bytes( + monkeypatch: pytest.MonkeyPatch, file_session: Session +) -> None: runtime = _build_runtime() file = _build_file( transfer_method=FileTransferMethod.LOCAL_FILE, reference=build_file_reference(record_id="upload-file-id"), ) - session = MagicMock() - session.get.return_value = SimpleNamespace(key="canonical-storage-key") - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) + _persist_upload_file(file_session) monkeypatch.setattr(file_runtime.storage, "load", lambda *args, **kwargs: b"image-bytes") assert runtime.load_file_bytes(file=file) == b"image-bytes" - session.get.assert_called_with(UploadFile, "upload-file-id") monkeypatch.setattr(file_runtime.storage, "load", lambda *args, **kwargs: "not-bytes") with pytest.raises(ValueError, match="is not a bytes object"): runtime.load_file_bytes(file=file) -def test_resolve_storage_key_ignores_encoded_reference_when_unscoped(monkeypatch: pytest.MonkeyPatch) -> None: +def test_resolve_storage_key_ignores_encoded_reference_when_unscoped(file_session: Session) -> None: runtime = _build_runtime() file = _build_file( transfer_method=FileTransferMethod.LOCAL_FILE, reference=build_file_reference(record_id="upload-file-id", storage_key="tampered-storage-key"), ) - session = MagicMock() - session.get.return_value = SimpleNamespace(key="canonical-storage-key") - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) + _persist_upload_file(file_session) assert runtime._resolve_storage_key(file=file) == "canonical-storage-key" - session.get.assert_called_once_with(UploadFile, "upload-file-id") -def test_resolve_storage_key_uses_canonical_record_when_scope_is_bound(monkeypatch: pytest.MonkeyPatch) -> None: +def test_resolve_storage_key_uses_canonical_record_when_scope_is_bound(file_session: Session) -> None: + upload_file = _persist_upload_file(file_session) controller = MagicMock() controller.current_scope.return_value = FileAccessScope( tenant_id="tenant-id", @@ -221,28 +268,19 @@ def test_resolve_storage_key_uses_canonical_record_when_scope_is_bound(monkeypat user_from=UserFrom.END_USER, invoke_from=InvokeFrom.WEB_APP, ) - controller.get_upload_file.return_value = SimpleNamespace(key="canonical-storage-key") + controller.get_upload_file.return_value = upload_file runtime = DifyWorkflowFileRuntime(file_access_controller=controller) file = _build_file( transfer_method=FileTransferMethod.LOCAL_FILE, reference=build_file_reference(record_id="upload-file-id", storage_key="tampered-storage-key"), ) - session = MagicMock() - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) - assert runtime._resolve_storage_key(file=file) == "canonical-storage-key" - controller.get_upload_file.assert_called_once_with(session=session, file_id="upload-file-id") + controller.get_upload_file.assert_called_once() + assert isinstance(controller.get_upload_file.call_args.kwargs["session"], Session) + assert controller.get_upload_file.call_args.kwargs["file_id"] == "upload-file-id" -def test_resolve_upload_file_url_rejects_unauthorized_scoped_access(monkeypatch: pytest.MonkeyPatch) -> None: +def test_resolve_upload_file_url_rejects_unauthorized_scoped_access(file_session: Session) -> None: controller = MagicMock() controller.current_scope.return_value = FileAccessScope( tenant_id="tenant-id", @@ -252,17 +290,6 @@ def test_resolve_upload_file_url_rejects_unauthorized_scoped_access(monkeypatch: ) controller.get_upload_file.return_value = None runtime = DifyWorkflowFileRuntime(file_access_controller=controller) - session = MagicMock() - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) - with pytest.raises(ValueError, match="Upload file upload-file-id not found"): runtime.resolve_upload_file_url(upload_file_id="upload-file-id") @@ -276,7 +303,7 @@ def test_resolve_upload_file_url_rejects_unauthorized_scoped_access(monkeypatch: ], ) def test_resolve_storage_key_loads_database_records( - monkeypatch: pytest.MonkeyPatch, + file_session: Session, transfer_method: FileTransferMethod, record_id: str, expected_storage_key: str, @@ -287,25 +314,10 @@ def test_resolve_storage_key_loads_database_records( reference=build_file_reference(record_id=record_id), extension=".png", ) - session = MagicMock() - - def get(model_class, value): - if transfer_method in {FileTransferMethod.LOCAL_FILE, FileTransferMethod.DATASOURCE_FILE}: - assert model_class is UploadFile - return SimpleNamespace(key="upload-storage-key") - assert model_class is ToolFile - return SimpleNamespace(file_key="tool-storage-key") - - session.get.side_effect = get - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) + if transfer_method in {FileTransferMethod.LOCAL_FILE, FileTransferMethod.DATASOURCE_FILE}: + _persist_upload_file(file_session, key="upload-storage-key") + else: + _persist_tool_file(file_session) assert runtime._resolve_storage_key(file=file) == expected_storage_key @@ -318,7 +330,7 @@ def test_resolve_storage_key_loads_database_records( ], ) def test_resolve_storage_key_raises_when_records_are_missing( - monkeypatch: pytest.MonkeyPatch, + file_session: Session, transfer_method: FileTransferMethod, expected_message: str, ) -> None: @@ -329,18 +341,6 @@ def test_resolve_storage_key_raises_when_records_are_missing( reference=build_file_reference(record_id=record_id), extension=".png", ) - session = MagicMock() - session.get.return_value = None - - class _SessionContext: - def __enter__(self): - return session - - def __exit__(self, exc_type, exc, tb): - return False - - monkeypatch.setattr(file_runtime.session_factory, "create_session", lambda: _SessionContext()) - with pytest.raises(ValueError, match=expected_message): runtime._resolve_storage_key(file=file) diff --git a/api/tests/unit_tests/core/rag/extractor/test_pdf_extractor.py b/api/tests/unit_tests/core/rag/extractor/test_pdf_extractor.py index c41a0752b48..3f5cf0d37cb 100644 --- a/api/tests/unit_tests/core/rag/extractor/test_pdf_extractor.py +++ b/api/tests/unit_tests/core/rag/extractor/test_pdf_extractor.py @@ -1,83 +1,68 @@ -from types import SimpleNamespace +from dataclasses import dataclass from unittest.mock import MagicMock, patch +from uuid import uuid4 import pytest +from sqlalchemy import select +from sqlalchemy.orm import Session import core.rag.extractor.pdf_extractor as pe +from models.model import UploadFile + +TENANT_ID = str(uuid4()) +USER_ID = str(uuid4()) + + +class _Storage: + saves: list[tuple[str, bytes]] + + def __init__(self) -> None: + self.saves = [] + + def save(self, key: str, data: bytes) -> None: + self.saves.append((key, data)) + + +class _DatabaseBinding: + session: Session + + def __init__(self, session: Session) -> None: + self.session = session + + +@dataclass(frozen=True) +class _Dependencies: + storage: _Storage + session: Session @pytest.fixture -def mock_dependencies(monkeypatch: pytest.MonkeyPatch): - # Mock storage - saves = [] - - def save(key, data): - saves.append((key, data)) - - monkeypatch.setattr(pe, "storage", SimpleNamespace(save=save)) - - # Mock db - class DummySession: - def __init__(self): - self.added = [] - self.committed = False - - def add(self, obj): - self.added.append(obj) - - def add_all(self, objs): - self.added.extend(objs) - - def commit(self): - self.committed = True - - db_stub = SimpleNamespace(session=DummySession()) - monkeypatch.setattr(pe, "db", db_stub) - - # Mock UploadFile - class FakeUploadFile: - DEFAULT_ID = "test_file_id" - - def __init__(self, **kwargs): - # Assign id from DEFAULT_ID, allow override via kwargs if needed - self.id = self.DEFAULT_ID - for k, v in kwargs.items(): - setattr(self, k, v) - - monkeypatch.setattr(pe, "UploadFile", FakeUploadFile) - - # Mock config +def mock_dependencies(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session) -> _Dependencies: + storage = _Storage() + monkeypatch.setattr(pe, "storage", storage) + monkeypatch.setattr(pe, "db", _DatabaseBinding(sqlite_session)) monkeypatch.setattr(pe.dify_config, "FILES_URL", "http://files.local") monkeypatch.setattr(pe.dify_config, "INTERNAL_FILES_URL", None) monkeypatch.setattr(pe.dify_config, "STORAGE_TYPE", "local") - - return SimpleNamespace(saves=saves, db=db_stub, UploadFile=FakeUploadFile) + return _Dependencies(storage=storage, session=sqlite_session) @pytest.mark.parametrize( - ("image_bytes", "expected_mime", "expected_ext", "file_id"), + ("image_bytes", "expected_mime", "expected_ext"), [ - (b"\xff\xd8\xff some jpeg", "image/jpeg", "jpg", "test_file_id_jpeg"), - (b"\x89PNG\r\n\x1a\n some png", "image/png", "png", "test_file_id_png"), + (b"\xff\xd8\xff some jpeg", "image/jpeg", "jpg"), + (b"\x89PNG\r\n\x1a\n some png", "image/png", "png"), ], ) +@pytest.mark.parametrize("sqlite_session", [(UploadFile,)], indirect=True) @pytest.mark.parametrize("inject_session", [False, True]) def test_extract_images_formats( - mock_dependencies, - monkeypatch: pytest.MonkeyPatch, - image_bytes, - expected_mime, - expected_ext, - file_id, + mock_dependencies: _Dependencies, + image_bytes: bytes, + expected_mime: str, + expected_ext: str, inject_session: bool, ): - saves = mock_dependencies.saves - db_stub = mock_dependencies.db - - # Customize FakeUploadFile id for this test case. - # Using monkeypatch ensures the class attribute is reset between parameter sets. - monkeypatch.setattr(mock_dependencies.UploadFile, "DEFAULT_ID", file_id) - # Mock page and image objects mock_page = MagicMock() mock_image_obj = MagicMock() @@ -91,25 +76,32 @@ def test_extract_images_formats( extractor = pe.PdfExtractor( file_path="test.pdf", - tenant_id="t1", - user_id="u1", - session=db_stub.session if inject_session else None, + tenant_id=TENANT_ID, + user_id=USER_ID, + session=mock_dependencies.session if inject_session else None, ) # We need to handle the import inside _extract_images - with patch("pypdfium2.raw", autospec=True) as mock_raw: + with ( + patch("pypdfium2.raw", autospec=True) as mock_raw, + patch.object( + mock_dependencies.session, + "commit", + wraps=mock_dependencies.session.commit, + ) as commit, + ): mock_raw.FPDF_PAGEOBJ_IMAGE = 1 result = extractor._extract_images(mock_page) - assert f"![image](http://files.local/files/{file_id}/file-preview)" in result - assert len(saves) == 1 - assert saves[0][1] == image_bytes - assert len(db_stub.session.added) == 1 - assert db_stub.session.added[0].tenant_id == "t1" - assert db_stub.session.added[0].size == len(image_bytes) - assert db_stub.session.added[0].mime_type == expected_mime - assert db_stub.session.added[0].extension == expected_ext - assert db_stub.session.committed is not inject_session + assert commit.called is not inject_session + upload_file = mock_dependencies.session.scalar(select(UploadFile)) + assert upload_file is not None + assert f"![image](http://files.local/files/{upload_file.id}/file-preview)" in result + assert mock_dependencies.storage.saves == [(upload_file.key, image_bytes)] + assert upload_file.tenant_id == TENANT_ID + assert upload_file.size == len(image_bytes) + assert upload_file.mime_type == expected_mime + assert upload_file.extension == expected_ext @pytest.mark.parametrize( @@ -120,14 +112,17 @@ def test_extract_images_formats( (Exception("Failed to get objects"), None), # Exception raised ], ) -def test_extract_images_get_objects_scenarios(mock_dependencies, get_objects_side_effect, get_objects_return_value): +@pytest.mark.parametrize("sqlite_session", [(UploadFile,)], indirect=True) +def test_extract_images_get_objects_scenarios( + mock_dependencies: _Dependencies, get_objects_side_effect, get_objects_return_value +): mock_page = MagicMock() if get_objects_side_effect: mock_page.get_objects.side_effect = get_objects_side_effect else: mock_page.get_objects.return_value = get_objects_return_value - extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id="t1", user_id="u1") + extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id=TENANT_ID, user_id=USER_ID) with patch("pypdfium2.raw", autospec=True) as mock_raw: mock_raw.FPDF_PAGEOBJ_IMAGE = 1 @@ -136,7 +131,8 @@ def test_extract_images_get_objects_scenarios(mock_dependencies, get_objects_sid assert result == "" -def test_extract_calls_extract_images(mock_dependencies, monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize("sqlite_session", [(UploadFile,)], indirect=True) +def test_extract_calls_extract_images(mock_dependencies: _Dependencies, monkeypatch: pytest.MonkeyPatch): # Mock pypdfium2 mock_pdf_doc = MagicMock() mock_page = MagicMock() @@ -152,7 +148,7 @@ def test_extract_calls_extract_images(mock_dependencies, monkeypatch: pytest.Mon mock_blob = MagicMock() mock_blob.source = "test.pdf" with patch("core.rag.extractor.pdf_extractor.Blob.from_path", return_value=mock_blob, autospec=True): - extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id="t1", user_id="u1") + extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id=TENANT_ID, user_id=USER_ID) # Mock _extract_images to return a known string monkeypatch.setattr(extractor, "_extract_images", lambda p: "![image](img_url)") @@ -165,10 +161,8 @@ def test_extract_calls_extract_images(mock_dependencies, monkeypatch: pytest.Mon assert documents[0].metadata["page"] == 0 -def test_extract_images_failures(mock_dependencies): - saves = mock_dependencies.saves - db_stub = mock_dependencies.db - +@pytest.mark.parametrize("sqlite_session", [(UploadFile,)], indirect=True) +def test_extract_images_failures(mock_dependencies: _Dependencies): # Mock page and image objects mock_page = MagicMock() mock_image_obj_fail = MagicMock() @@ -187,14 +181,14 @@ def test_extract_images_failures(mock_dependencies): mock_page.get_objects.return_value = [mock_image_obj_fail, mock_image_obj_ok] - extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id="t1", user_id="u1") + extractor = pe.PdfExtractor(file_path="test.pdf", tenant_id=TENANT_ID, user_id=USER_ID) with patch("pypdfium2.raw", autospec=True) as mock_raw: mock_raw.FPDF_PAGEOBJ_IMAGE = 1 result = extractor._extract_images(mock_page) # Should have one success - assert "![image](http://files.local/files/test_file_id/file-preview)" in result - assert len(saves) == 1 - assert saves[0][1] == jpeg_bytes - assert db_stub.session.committed is True + upload_file = mock_dependencies.session.scalar(select(UploadFile)) + assert upload_file is not None + assert f"![image](http://files.local/files/{upload_file.id}/file-preview)" in result + assert mock_dependencies.storage.saves == [(upload_file.key, jpeg_bytes)] diff --git a/api/tests/unit_tests/core/repositories/test_sqlalchemy_workflow_execution_repository.py b/api/tests/unit_tests/core/repositories/test_sqlalchemy_workflow_execution_repository.py index f247525aedd..01c4905e4cb 100644 --- a/api/tests/unit_tests/core/repositories/test_sqlalchemy_workflow_execution_repository.py +++ b/api/tests/unit_tests/core/repositories/test_sqlalchemy_workflow_execution_repository.py @@ -1,56 +1,59 @@ +import json from datetime import UTC, datetime -from unittest.mock import MagicMock from uuid import uuid4 import pytest from sqlalchemy.engine import Engine -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import Session, sessionmaker from core.repositories.sqlalchemy_workflow_execution_repository import SQLAlchemyWorkflowExecutionRepository from graphon.entities import WorkflowExecution from graphon.enums import WorkflowExecutionStatus, WorkflowType -from models import Account, CreatorUserRole, EndUser, WorkflowRun -from models.enums import WorkflowRunTriggeredFrom +from models import Account, CreatorUserRole, EndUser, Tenant, WorkflowRun +from models.enums import EndUserType, WorkflowRunTriggeredFrom +from models.workflow import WorkflowType as ModelWorkflowType + +TABLES = (WorkflowRun,) RESOURCE_TENANT_ID = "resource-tenant-id" -@pytest.fixture -def mock_session_factory(): - """Mock SQLAlchemy session factory.""" - session_factory = MagicMock(spec=sessionmaker) - session = MagicMock() - session.get.return_value = None - session_factory.return_value.__enter__.return_value = session - return session_factory - - -@pytest.fixture -def mock_engine(): - """Mock SQLAlchemy Engine.""" - return MagicMock(spec=Engine) - - -@pytest.fixture -def mock_account(): - """Mock Account user.""" - account = MagicMock(spec=Account) +def _make_account(*, tenant_id: str | None = None) -> Account: + account = Account(name="Repository User", email=f"{uuid4()}@example.com") account.id = str(uuid4()) - account.current_tenant_id = str(uuid4()) + if tenant_id is not None: + tenant = Tenant(name="Repository Tenant") + tenant.id = tenant_id + account._current_tenant = tenant return account @pytest.fixture -def mock_end_user(): - """Mock EndUser.""" - user = MagicMock(spec=EndUser) - user.id = str(uuid4()) - user.tenant_id = str(uuid4()) - return user +def sqlite_session_factory(sqlite_engine: Engine) -> sessionmaker[Session]: + """Create repository-owned sessions bound to the isolated SQLite engine.""" + return sessionmaker(bind=sqlite_engine, expire_on_commit=False) @pytest.fixture -def sample_workflow_execution(): +def account() -> Account: + return _make_account(tenant_id=str(uuid4())) + + +@pytest.fixture +def end_user() -> EndUser: + return EndUser( + id=str(uuid4()), + tenant_id=str(uuid4()), + app_id=None, + type=EndUserType.SERVICE_API, + external_user_id=None, + name="Repository End User", + session_id=str(uuid4()), + ) + + +@pytest.fixture +def sample_workflow_execution() -> WorkflowExecution: """Sample WorkflowExecution for testing.""" return WorkflowExecution( id_=str(uuid4()), @@ -71,125 +74,147 @@ def sample_workflow_execution(): class TestSQLAlchemyWorkflowExecutionRepository: - def test_init_with_sessionmaker(self, mock_session_factory, mock_account): + def test_init_with_sessionmaker(self, sqlite_session_factory: sessionmaker[Session], account: Account): app_id = "test_app_id" triggered_from = WorkflowRunTriggeredFrom.APP_RUN repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id=app_id, triggered_from=triggered_from, ) - assert repo._session_factory == mock_session_factory + assert repo._session_factory is sqlite_session_factory assert repo._tenant_id == RESOURCE_TENANT_ID assert repo._app_id == app_id assert repo._triggered_from == triggered_from - assert repo._creator_user_id == mock_account.id + assert repo._creator_user_id == account.id assert repo._creator_user_role == CreatorUserRole.ACCOUNT - def test_init_with_engine(self, mock_engine, mock_account): + def test_init_with_engine(self, sqlite_engine: Engine, account: Account): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_engine, + session_factory=sqlite_engine, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app_id", triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) assert isinstance(repo._session_factory, sessionmaker) - assert repo._session_factory.kw["bind"] == mock_engine + assert repo._session_factory.kw["bind"] is sqlite_engine - def test_init_invalid_session_factory(self, mock_account): + def test_init_invalid_session_factory(self, account: Account): with pytest.raises(ValueError, match="Invalid session_factory type"): SQLAlchemyWorkflowExecutionRepository( session_factory="invalid", tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id=None, triggered_from=None, ) - def test_init_no_tenant_id(self, mock_session_factory): - user = MagicMock(spec=Account) - user.current_tenant_id = None + def test_init_no_tenant_id(self, sqlite_session_factory: sessionmaker[Session]): + user = _make_account() with pytest.raises(ValueError, match="tenant_id is required"): SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id="", user=user, app_id=None, triggered_from=None, ) - def test_init_uses_resource_tenant_when_account_has_no_current_tenant(self, mock_session_factory): - user = MagicMock(spec=Account) - user.current_tenant_id = None - user.id = str(uuid4()) + def test_init_uses_resource_tenant_when_account_has_no_current_tenant( + self, sqlite_session_factory: sessionmaker[Session] + ): + user = _make_account() repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, - tenant_id="resource-tenant-id", + session_factory=sqlite_session_factory, + tenant_id=RESOURCE_TENANT_ID, user=user, app_id="test-app", triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) - assert repo._tenant_id == "resource-tenant-id" + assert repo._tenant_id == RESOURCE_TENANT_ID assert repo._creator_user_id == user.id - def test_init_with_end_user(self, mock_session_factory, mock_end_user): + def test_init_with_end_user(self, sqlite_session_factory: sessionmaker[Session], end_user: EndUser): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_end_user, + user=end_user, app_id=None, triggered_from=None, ) assert repo._tenant_id == RESOURCE_TENANT_ID assert repo._creator_user_role == CreatorUserRole.END_USER - def test_to_domain_model(self, mock_session_factory, mock_account): + @pytest.mark.parametrize("sqlite_session", [TABLES], indirect=True) + def test_to_domain_model( + self, + sqlite_session_factory: sessionmaker[Session], + sqlite_session: Session, + account: Account, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id=None, triggered_from=None, ) - db_model = MagicMock(spec=WorkflowRun) - db_model.id = str(uuid4()) - db_model.workflow_id = str(uuid4()) - db_model.type = "workflow" - db_model.version = "1.0" - db_model.inputs_dict = {"in": "val"} - db_model.outputs_dict = {"out": "val"} - db_model.graph_dict = {"nodes": []} - db_model.status = "succeeded" - db_model.error = "some error" - db_model.total_tokens = 50 - db_model.total_steps = 3 - db_model.exceptions_count = 1 - db_model.created_at = datetime.now(UTC) - db_model.finished_at = datetime.now(UTC) + db_model = WorkflowRun( + id=str(uuid4()), + tenant_id=account.current_tenant_id, + app_id=str(uuid4()), + workflow_id=str(uuid4()), + type=ModelWorkflowType.WORKFLOW, + triggered_from=WorkflowRunTriggeredFrom.APP_RUN, + version="1.0", + inputs=json.dumps({"in": "val"}), + outputs=json.dumps({"out": "val"}), + graph=json.dumps({"nodes": []}), + status=WorkflowExecutionStatus.SUCCEEDED, + error="some error", + elapsed_time=1.0, + total_tokens=50, + total_steps=3, + exceptions_count=1, + created_by_role=CreatorUserRole.ACCOUNT, + created_by=account.id, + created_at=datetime.now(UTC), + finished_at=datetime.now(UTC), + ) + sqlite_session.add(db_model) + sqlite_session.commit() + sqlite_session.expunge_all() + persisted_model = sqlite_session.get(WorkflowRun, db_model.id) + assert persisted_model is not None - domain_model = repo._to_domain_model(db_model) + domain_model = repo._to_domain_model(persisted_model) - assert domain_model.id_ == db_model.id - assert domain_model.workflow_id == db_model.workflow_id + assert domain_model.id_ == persisted_model.id + assert domain_model.workflow_id == persisted_model.workflow_id assert domain_model.status == WorkflowExecutionStatus.SUCCEEDED - assert domain_model.inputs == db_model.inputs_dict + assert domain_model.inputs == {"in": "val"} assert domain_model.error_message == "some error" - def test_to_db_model(self, mock_session_factory, mock_account, sample_workflow_execution): + def test_to_db_model( + self, + sqlite_session_factory: sessionmaker[Session], + account: Account, + sample_workflow_execution: WorkflowExecution, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app", triggered_from=WorkflowRunTriggeredFrom.DEBUGGING, ) @@ -208,11 +233,16 @@ class TestSQLAlchemyWorkflowExecutionRepository: assert db_model.total_tokens == sample_workflow_execution.total_tokens assert db_model.elapsed_time == 10.0 - def test_to_db_model_edge_cases(self, mock_session_factory, mock_account, sample_workflow_execution): + def test_to_db_model_edge_cases( + self, + sqlite_session_factory: sessionmaker[Session], + account: Account, + sample_workflow_execution: WorkflowExecution, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app", triggered_from=WorkflowRunTriggeredFrom.DEBUGGING, ) @@ -231,11 +261,16 @@ class TestSQLAlchemyWorkflowExecutionRepository: assert db_model.error is None assert db_model.elapsed_time == 0 - def test_to_db_model_app_id_none(self, mock_session_factory, mock_account, sample_workflow_execution): + def test_to_db_model_app_id_none( + self, + sqlite_session_factory: sessionmaker[Session], + account: Account, + sample_workflow_execution: WorkflowExecution, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id=None, triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) @@ -244,11 +279,16 @@ class TestSQLAlchemyWorkflowExecutionRepository: assert not hasattr(db_model, "app_id") or db_model.app_id is None assert db_model.tenant_id == repo._tenant_id - def test_to_db_model_missing_context(self, mock_session_factory, mock_account, sample_workflow_execution): + def test_to_db_model_missing_context( + self, + sqlite_session_factory: sessionmaker[Session], + account: Account, + sample_workflow_execution: WorkflowExecution, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id=None, triggered_from=None, ) @@ -267,33 +307,47 @@ class TestSQLAlchemyWorkflowExecutionRepository: with pytest.raises(ValueError, match="created_by_role is required"): repo._to_db_model(sample_workflow_execution) - def test_save(self, mock_session_factory, mock_account, sample_workflow_execution): + @pytest.mark.parametrize("sqlite_session", [TABLES], indirect=True) + def test_save( + self, + sqlite_session_factory: sessionmaker[Session], + sqlite_session: Session, + account: Account, + sample_workflow_execution: WorkflowExecution, + ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app", triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) repo.save(sample_workflow_execution) - session = mock_session_factory.return_value.__enter__.return_value - session.merge.assert_called_once() - session.commit.assert_called_once() + persisted_model = sqlite_session.get(WorkflowRun, sample_workflow_execution.id_) + assert persisted_model is not None + assert persisted_model.tenant_id == RESOURCE_TENANT_ID + assert persisted_model.inputs_dict == sample_workflow_execution.inputs + assert persisted_model.outputs_dict == sample_workflow_execution.outputs # Check cache assert sample_workflow_execution.id_ in repo._execution_cache cached_model = repo._execution_cache[sample_workflow_execution.id_] assert cached_model.id == sample_workflow_execution.id_ + @pytest.mark.parametrize("sqlite_session", [TABLES], indirect=True) def test_save_uses_execution_started_at_when_record_does_not_exist( - self, mock_session_factory, mock_account, sample_workflow_execution + self, + sqlite_session_factory: sessionmaker[Session], + sqlite_session: Session, + account: Account, + sample_workflow_execution: WorkflowExecution, ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app", triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) @@ -301,41 +355,71 @@ class TestSQLAlchemyWorkflowExecutionRepository: started_at = datetime(2026, 1, 1, 12, 0, 0, tzinfo=UTC) sample_workflow_execution.started_at = started_at - session = mock_session_factory.return_value.__enter__.return_value - session.get.return_value = None - repo.save(sample_workflow_execution) - saved_model = session.merge.call_args.args[0] - assert saved_model.created_at == started_at - session.commit.assert_called_once() + persisted_model = sqlite_session.get(WorkflowRun, sample_workflow_execution.id_) + assert persisted_model is not None + assert persisted_model.created_at == started_at.replace(tzinfo=None) + @pytest.mark.parametrize("sqlite_session", [TABLES], indirect=True) def test_save_preserves_existing_created_at_when_record_already_exists( - self, mock_session_factory, mock_account, sample_workflow_execution + self, + sqlite_session_factory: sessionmaker[Session], + sqlite_session: Session, + account: Account, + sample_workflow_execution: WorkflowExecution, ): repo = SQLAlchemyWorkflowExecutionRepository( - session_factory=mock_session_factory, + session_factory=sqlite_session_factory, tenant_id=RESOURCE_TENANT_ID, - user=mock_account, + user=account, app_id="test_app", triggered_from=WorkflowRunTriggeredFrom.APP_RUN, ) execution_id = sample_workflow_execution.id_ existing_created_at = datetime(2026, 1, 1, 12, 0, 0, tzinfo=UTC) - - existing_run = WorkflowRun() - existing_run.id = execution_id - existing_run.tenant_id = repo._tenant_id - existing_run.created_at = existing_created_at - - session = mock_session_factory.return_value.__enter__.return_value - session.get.return_value = existing_run + sample_workflow_execution.started_at = existing_created_at + repo.save(sample_workflow_execution) sample_workflow_execution.started_at = datetime(2026, 1, 1, 12, 30, 0, tzinfo=UTC) repo.save(sample_workflow_execution) - saved_model = session.merge.call_args.args[0] - assert saved_model.created_at == existing_created_at - session.commit.assert_called_once() + persisted_model = sqlite_session.get(WorkflowRun, execution_id) + assert persisted_model is not None + assert persisted_model.created_at == existing_created_at.replace(tzinfo=None) + + @pytest.mark.parametrize("sqlite_session", [TABLES], indirect=True) + def test_save_rejects_execution_owned_by_another_tenant( + self, + sqlite_session_factory: sessionmaker[Session], + sqlite_session: Session, + account: Account, + sample_workflow_execution: WorkflowExecution, + ): + other_tenant_id = str(uuid4()) + other_account = _make_account(tenant_id=str(uuid4())) + other_repo = SQLAlchemyWorkflowExecutionRepository( + session_factory=sqlite_session_factory, + tenant_id=other_tenant_id, + user=other_account, + app_id="test_app", + triggered_from=WorkflowRunTriggeredFrom.APP_RUN, + ) + other_repo.save(sample_workflow_execution) + + repo = SQLAlchemyWorkflowExecutionRepository( + session_factory=sqlite_session_factory, + tenant_id=RESOURCE_TENANT_ID, + user=account, + app_id="test_app", + triggered_from=WorkflowRunTriggeredFrom.APP_RUN, + ) + with pytest.raises(ValueError, match="Unauthorized access to workflow run"): + repo.save(sample_workflow_execution) + + sqlite_session.expire_all() + persisted_model = sqlite_session.get(WorkflowRun, sample_workflow_execution.id_) + assert persisted_model is not None + assert persisted_model.tenant_id == other_tenant_id diff --git a/api/tests/unit_tests/core/tools/test_builtin_tools_extra.py b/api/tests/unit_tests/core/tools/test_builtin_tools_extra.py index 4dac9b7260d..d6bb7503793 100644 --- a/api/tests/unit_tests/core/tools/test_builtin_tools_extra.py +++ b/api/tests/unit_tests/core/tools/test_builtin_tools_extra.py @@ -4,10 +4,10 @@ import calendar import math from datetime import date from types import SimpleNamespace -from unittest.mock import MagicMock from zoneinfo import ZoneInfo import pytest +from sqlalchemy.orm import Session from core.app.entities.app_invoke_entities import InvokeFrom from core.tools.__base.tool_runtime import ToolRuntime @@ -51,24 +51,26 @@ def _raise_runtime_error(*_args: object, **_kwargs: object) -> None: raise RuntimeError("boom") -def test_current_time_tool(): +def test_current_time_tool(sqlite_session: Session): current_tool = _build_builtin_tool(CurrentTimeTool) - utc_text = list(current_tool.invoke(session=MagicMock(), user_id="u", tool_parameters={"timezone": "UTC"}))[ + utc_text = list(current_tool.invoke(session=sqlite_session, user_id="u", tool_parameters={"timezone": "UTC"}))[ 0 ].message.text assert utc_text invalid_tz = list( - current_tool.invoke(session=MagicMock(), user_id="u", tool_parameters={"timezone": "Invalid/TZ"}) + current_tool.invoke(session=sqlite_session, user_id="u", tool_parameters={"timezone": "Invalid/TZ"}) )[0].message.text assert "Invalid timezone" in invalid_tz -def test_localtime_to_timestamp_tool(): +def test_localtime_to_timestamp_tool(sqlite_session: Session): localtime_tool = _build_builtin_tool(LocaltimeToTimestampTool) ts_message = list( localtime_tool.invoke( - session=MagicMock(), user_id="u", tool_parameters={"localtime": "2024-01-01 10:00:00", "timezone": "UTC"} + session=sqlite_session, + user_id="u", + tool_parameters={"localtime": "2024-01-01 10:00:00", "timezone": "UTC"}, ) )[0].message.text ts_value = float(ts_message.strip()) @@ -92,11 +94,11 @@ def test_localtime_to_timestamp_tool(): LocaltimeToTimestampTool.localtime_to_timestamp("bad", "%Y-%m-%d %H:%M:%S", "UTC") -def test_timestamp_to_localtime_tool(): +def test_timestamp_to_localtime_tool(sqlite_session: Session): to_local_tool = _build_builtin_tool(TimestampToLocaltimeTool) local_text = list( to_local_tool.invoke( - session=MagicMock(), user_id="u", tool_parameters={"timestamp": 1704067200, "timezone": "UTC"} + session=sqlite_session, user_id="u", tool_parameters={"timestamp": 1704067200, "timezone": "UTC"} ) )[0].message.text assert "2024" in local_text @@ -104,11 +106,11 @@ def test_timestamp_to_localtime_tool(): TimestampToLocaltimeTool.timestamp_to_localtime("bad", "UTC") # type: ignore[arg-type] -def test_timezone_conversion_tool(): +def test_timezone_conversion_tool(sqlite_session: Session): timezone_tool = _build_builtin_tool(TimezoneConversionTool) converted = list( timezone_tool.invoke( - session=MagicMock(), + session=sqlite_session, user_id="u", tool_parameters={ "current_time": "2024-01-01 08:00:00", @@ -122,10 +124,10 @@ def test_timezone_conversion_tool(): TimezoneConversionTool.timezone_convert("bad", "UTC", "Asia/Tokyo") -def test_weekday_tool(): +def test_weekday_tool(sqlite_session: Session): weekday_tool = _build_builtin_tool(WeekdayTool) valid = list( - weekday_tool.invoke(session=MagicMock(), user_id="u", tool_parameters={"year": 2024, "month": 1, "day": 1}) + weekday_tool.invoke(session=sqlite_session, user_id="u", tool_parameters={"year": 2024, "month": 1, "day": 1}) )[0].message.text expected_date = date(2024, 1, 1) expected_message = ( @@ -135,14 +137,14 @@ def test_weekday_tool(): ) assert valid == expected_message invalid = list( - weekday_tool.invoke(session=MagicMock(), user_id="u", tool_parameters={"year": 2024, "month": 2, "day": 31}) + weekday_tool.invoke(session=sqlite_session, user_id="u", tool_parameters={"year": 2024, "month": 2, "day": 31}) )[0].message.text assert "Invalid date" in invalid with pytest.raises(ValueError, match="Month is required"): - list(weekday_tool.invoke(session=MagicMock(), user_id="u", tool_parameters={"year": 2024, "day": 1})) + list(weekday_tool.invoke(session=sqlite_session, user_id="u", tool_parameters={"year": 2024, "day": 1})) -def test_simple_code_valid_execution(monkeypatch: pytest.MonkeyPatch): +def test_simple_code_valid_execution(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): simple_code = _build_builtin_tool(SimpleCode) monkeypatch.setattr( @@ -151,7 +153,7 @@ def test_simple_code_valid_execution(monkeypatch: pytest.MonkeyPatch): ) result = list( simple_code.invoke( - session=MagicMock(), + session=sqlite_session, user_id="u", tool_parameters={"language": "python3", "code": "print(1)"}, ) @@ -159,18 +161,18 @@ def test_simple_code_valid_execution(monkeypatch: pytest.MonkeyPatch): assert result == "ok" -def test_simple_code_invalid_language(): +def test_simple_code_invalid_language(sqlite_session: Session): simple_code = _build_builtin_tool(SimpleCode) with pytest.raises(ValueError, match="Only python3 and javascript"): list( simple_code.invoke( - session=MagicMock(), user_id="u", tool_parameters={"language": "go", "code": "fmt.Println(1)"} + session=sqlite_session, user_id="u", tool_parameters={"language": "go", "code": "fmt.Println(1)"} ) ) -def test_simple_code_execution_error(monkeypatch: pytest.MonkeyPatch): +def test_simple_code_execution_error(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): simple_code = _build_builtin_tool(SimpleCode) monkeypatch.setattr( @@ -180,33 +182,35 @@ def test_simple_code_execution_error(monkeypatch: pytest.MonkeyPatch): with pytest.raises(ToolInvokeError, match="boom"): list( simple_code.invoke( - session=MagicMock(), user_id="u", tool_parameters={"language": "python3", "code": "print(1)"} + session=sqlite_session, + user_id="u", + tool_parameters={"language": "python3", "code": "print(1)"}, ) ) -def test_webscraper_empty_url(): +def test_webscraper_empty_url(sqlite_session: Session): webscraper = _build_builtin_tool(WebscraperTool) - empty = list(webscraper.invoke(session=MagicMock(), user_id="u", tool_parameters={"url": ""}))[0].message.text + empty = list(webscraper.invoke(session=sqlite_session, user_id="u", tool_parameters={"url": ""}))[0].message.text assert empty == "Please input url" -def test_webscraper_fetch(monkeypatch: pytest.MonkeyPatch): +def test_webscraper_fetch(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): webscraper = _build_builtin_tool(WebscraperTool) monkeypatch.setattr("core.tools.builtin_tool.providers.webscraper.tools.webscraper.get_url", lambda *a, **k: "page") - full = list(webscraper.invoke(session=MagicMock(), user_id="u", tool_parameters={"url": "https://example.com"}))[ + full = list(webscraper.invoke(session=sqlite_session, user_id="u", tool_parameters={"url": "https://example.com"}))[ 0 ].message.text assert full == "page" -def test_webscraper_summary(monkeypatch: pytest.MonkeyPatch): +def test_webscraper_summary(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): webscraper = _build_builtin_tool(WebscraperTool) monkeypatch.setattr("core.tools.builtin_tool.providers.webscraper.tools.webscraper.get_url", lambda *a, **k: "page") monkeypatch.setattr(webscraper, "summary", lambda user_id, content: "summary") summarized = list( webscraper.invoke( - session=MagicMock(), + session=sqlite_session, user_id="u", tool_parameters={"url": "https://example.com", "generate_summary": True}, ) @@ -214,26 +218,26 @@ def test_webscraper_summary(monkeypatch: pytest.MonkeyPatch): assert summarized == "summary" -def test_webscraper_fetch_error(monkeypatch: pytest.MonkeyPatch): +def test_webscraper_fetch_error(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): webscraper = _build_builtin_tool(WebscraperTool) monkeypatch.setattr( "core.tools.builtin_tool.providers.webscraper.tools.webscraper.get_url", _raise_runtime_error, ) with pytest.raises(ToolInvokeError, match="boom"): - list(webscraper.invoke(session=MagicMock(), user_id="u", tool_parameters={"url": "https://example.com"})) + list(webscraper.invoke(session=sqlite_session, user_id="u", tool_parameters={"url": "https://example.com"})) -def test_asr_invalid_file(): +def test_asr_invalid_file(sqlite_session: Session): asr = _build_builtin_tool(ASRTool) file_obj = SimpleNamespace(type=FileType.DOCUMENT) - invalid_file = list(asr.invoke(session=MagicMock(), user_id="u", tool_parameters={"audio_file": file_obj}))[ + invalid_file = list(asr.invoke(session=sqlite_session, user_id="u", tool_parameters={"audio_file": file_obj}))[ 0 ].message.text assert "not a valid audio file" in invalid_file -def test_asr_valid_file_invocation(monkeypatch: pytest.MonkeyPatch): +def test_asr_valid_file_invocation(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): asr = _build_builtin_tool(ASRTool) model_instance = type("M", (), {"invoke_speech2text": lambda self, file: "transcript"})() model_manager = type("Mgr", (), {"get_model_instance": lambda *a, **k: model_instance})() @@ -245,9 +249,9 @@ def test_asr_valid_file_invocation(monkeypatch: pytest.MonkeyPatch): lambda **kwargs: captured_manager_kwargs.update(kwargs) or model_manager, ) audio_file = SimpleNamespace(type=FileType.AUDIO) - ok = list(asr.invoke(session=MagicMock(), user_id="u", tool_parameters={"audio_file": audio_file, "model": "p#m"}))[ - 0 - ].message.text + ok = list( + asr.invoke(session=sqlite_session, user_id="u", tool_parameters={"audio_file": audio_file, "model": "p#m"}) + )[0].message.text assert ok == "transcript" assert captured_manager_kwargs == {"tenant_id": "tenant-1", "user_id": "u"} @@ -263,7 +267,7 @@ def test_asr_available_models_and_runtime_parameters(monkeypatch: pytest.MonkeyP assert asr.get_runtime_parameters()[0].name == "model" -def test_tts_invoke_returns_messages(monkeypatch: pytest.MonkeyPatch): +def test_tts_invoke_returns_messages(monkeypatch: pytest.MonkeyPatch, sqlite_session: Session): tts = _build_builtin_tool(TTSTool) captured_manager_kwargs = {} voices_model_instance = type( @@ -281,7 +285,7 @@ def test_tts_invoke_returns_messages(monkeypatch: pytest.MonkeyPatch): or type("M", (), {"get_model_instance": lambda *a, **k: voices_model_instance})() ), ) - messages = list(tts.invoke(session=MagicMock(), user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) + messages = list(tts.invoke(session=sqlite_session, user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) assert [m.type for m in messages] == [ToolInvokeMessage.MessageType.TEXT, ToolInvokeMessage.MessageType.BLOB] assert captured_manager_kwargs == {"tenant_id": "tenant-1", "user_id": "u"} @@ -293,18 +297,18 @@ def test_tts_get_available_models_requires_runtime(): tts.get_available_models() -def test_tts_tool_raises_when_runtime_missing(): +def test_tts_tool_raises_when_runtime_missing(sqlite_session: Session): tts = _build_builtin_tool(TTSTool) tts.runtime = None with pytest.raises(ValueError, match="Runtime is required"): - list(tts.invoke(session=MagicMock(), user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) + list(tts.invoke(session=sqlite_session, user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) @pytest.mark.parametrize( "voices", [[{"value": None}], []], ) -def test_tts_tool_raises_when_voice_unavailable(monkeypatch, voices): +def test_tts_tool_raises_when_voice_unavailable(monkeypatch, voices, sqlite_session: Session): tts = _build_builtin_tool(TTSTool) tts.runtime = ToolRuntime(tenant_id="tenant-1", invoke_from=InvokeFrom.DEBUGGER) model_without_voice = type( @@ -320,7 +324,7 @@ def test_tts_tool_raises_when_voice_unavailable(monkeypatch, voices): lambda **_: type("Manager", (), {"get_model_instance": lambda *args, **kwargs: model_without_voice})(), ) with pytest.raises(ValueError, match="no voice available"): - list(tts.invoke(session=MagicMock(), user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) + list(tts.invoke(session=sqlite_session, user_id="u", tool_parameters={"model": "p#m", "text": "hello"})) def test_tts_tool_get_available_models_and_runtime_parameters(monkeypatch: pytest.MonkeyPatch): diff --git a/api/tests/unit_tests/core/tools/test_tool_engine.py b/api/tests/unit_tests/core/tools/test_tool_engine.py index f38ab2a2fab..f688c68cfa6 100644 --- a/api/tests/unit_tests/core/tools/test_tool_engine.py +++ b/api/tests/unit_tests/core/tools/test_tool_engine.py @@ -1,11 +1,14 @@ from __future__ import annotations from collections.abc import Generator -from types import SimpleNamespace from typing import Any -from unittest.mock import MagicMock, Mock, patch +from unittest.mock import Mock, patch +from uuid import uuid4 import pytest +from sqlalchemy import select +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session from core.app.entities.app_invoke_entities import InvokeFrom from core.tools.__base.tool import Tool @@ -26,6 +29,45 @@ from core.tools.errors import ( ToolParameterValidationError, ) from core.tools.tool_engine import ToolEngine +from models.model import AppMode, Message, MessageFile + + +class _DatabaseBinding: + engine: Engine + + def __init__(self, engine: Engine) -> None: + self.engine = engine + + +def _message() -> Message: + message = Message( + app_id=str(uuid4()), + model_provider="provider", + model_id="model", + override_model_configs=None, + conversation_id=str(uuid4()), + inputs={}, + query="query", + message="", + message_tokens=0, + message_unit_price=0, + message_price_unit=0, + answer="", + answer_tokens=0, + answer_unit_price=0, + answer_price_unit=0, + parent_message_id=None, + provider_response_latency=0, + total_price=0, + currency="USD", + invoke_from="debugger", + from_source="console", + from_end_user_id=None, + from_account_id=str(uuid4()), + app_mode=AppMode.CHAT, + ) + message.id = str(uuid4()) + return message class _DummyTool(Tool): @@ -120,52 +162,41 @@ def test_convert_tool_response_to_str_and_extract_binary_messages(): ) -def test_create_message_files_and_invoke_generator(): +@pytest.mark.parametrize("sqlite_session", [(MessageFile,)], indirect=True) +def test_create_message_files_and_invoke_generator(sqlite_engine: Engine, sqlite_session: Session): binaries = [ ToolInvokeMessageBinary(mimetype="image/png", url="https://example.com/abc.png"), ToolInvokeMessageBinary(mimetype="audio/wav", url="https://example.com/def.wav"), ] - created = [] - - def _message_file_factory(**kwargs): - obj = SimpleNamespace(id=f"mf-{len(created) + 1}", **kwargs) - created.append(obj) - return obj - - file_session = MagicMock() - session_factory = MagicMock() - session_factory.begin.return_value.__enter__.return_value = file_session - with ( - patch("core.tools.tool_engine.MessageFile", side_effect=_message_file_factory), - patch("core.tools.tool_engine.db") as mock_db, - patch("core.tools.tool_engine.sessionmaker", return_value=session_factory) as mock_sessionmaker, - ): + agent_message = _message() + with patch("core.tools.tool_engine.db", _DatabaseBinding(sqlite_engine)): ids = ToolEngine._create_message_files( tool_messages=binaries, - agent_message=SimpleNamespace(id="msg-1"), + agent_message=agent_message, invoke_from=InvokeFrom.DEBUGGER, - user_id="user-1", + user_id=str(uuid4()), ) - assert ids == ["mf-1", "mf-2"] - mock_sessionmaker.assert_called_once_with(bind=mock_db.engine, expire_on_commit=False) - assert file_session.add.call_count == 2 - mock_db.session.close.assert_not_called() + message_files = list(sqlite_session.scalars(select(MessageFile).order_by(MessageFile.created_at)).all()) + assert ids == [message_file.id for message_file in message_files] + assert len(message_files) == 2 + assert {message_file.message_id for message_file in message_files} == {agent_message.id} tool = _build_tool() - invoked = list(ToolEngine._invoke(MagicMock(), tool, {"a": 1}, user_id="u")) + invoked = list(ToolEngine._invoke(sqlite_session, tool, {"a": 1}, user_id="u")) assert invoked[0].type == ToolInvokeMessage.MessageType.TEXT assert isinstance(invoked[-1], ToolInvokeMeta) assert invoked[-1].error is None -def test_generic_invoke_success_and_error_paths(): +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_generic_invoke_success_and_error_paths(sqlite_session: Session): tool = _build_tool() callback = Mock() callback.on_tool_execution.side_effect = lambda **kwargs: kwargs["tool_outputs"] response = list( ToolEngine.generic_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters={"x": 1}, user_id="u1", @@ -186,7 +217,7 @@ def test_generic_invoke_success_and_error_paths(): with pytest.raises(RuntimeError, match="boom"): list( ToolEngine.generic_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters={"x": 1}, user_id="u1", @@ -197,10 +228,11 @@ def test_generic_invoke_success_and_error_paths(): error_callback.on_tool_error.assert_called_once() -def test_agent_invoke_success(): +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_agent_invoke_success(sqlite_session: Session): tool = _build_tool(with_llm_parameter=True) callback = Mock() - message = SimpleNamespace(id="m1", conversation_id="c1") + message = _message() meta = ToolInvokeMeta.empty() with patch.object(ToolEngine, "_invoke", return_value=iter([tool.create_text_message("ok"), meta])): @@ -211,7 +243,7 @@ def test_agent_invoke_success(): with patch.object(ToolEngine, "_extract_tool_response_binary_and_text", return_value=iter([])): with patch.object(ToolEngine, "_create_message_files", return_value=[]): result_text, message_files, result_meta = ToolEngine.agent_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters="hello", user_id="u1", @@ -228,14 +260,15 @@ def test_agent_invoke_success(): callback.on_tool_end.assert_called_once() -def test_agent_invoke_param_validation_error(): +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_agent_invoke_param_validation_error(sqlite_session: Session): tool = _build_tool(with_llm_parameter=True) callback = Mock() - message = SimpleNamespace(id="m1", conversation_id="c1") + message = _message() with patch.object(ToolEngine, "_invoke", side_effect=ToolParameterValidationError("bad-param")): error_text, files, error_meta = ToolEngine.agent_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters={"a": 1}, user_id="u1", @@ -250,15 +283,16 @@ def test_agent_invoke_param_validation_error(): assert error_meta.error -def test_agent_invoke_engine_meta_error(): +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_agent_invoke_engine_meta_error(sqlite_session: Session): tool = _build_tool(with_llm_parameter=True) callback = Mock() - message = SimpleNamespace(id="m1", conversation_id="c1") + message = _message() engine_error = ToolEngineInvokeError(ToolInvokeMeta.error_instance("meta failure")) with patch.object(ToolEngine, "_invoke", side_effect=engine_error): error_text, files, error_meta = ToolEngine.agent_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters={"a": 1}, user_id="u1", @@ -295,14 +329,15 @@ def test_convert_tool_response_excludes_variable_messages(): assert "variable_name" not in result -def test_agent_invoke_tool_invoke_error(): +@pytest.mark.parametrize("sqlite_session", [()], indirect=True) +def test_agent_invoke_tool_invoke_error(sqlite_session: Session): tool = _build_tool(with_llm_parameter=True) callback = Mock() - message = SimpleNamespace(id="m1", conversation_id="c1") + message = _message() with patch.object(ToolEngine, "_invoke", side_effect=ToolInvokeError("invoke boom")): error_text, files, _ = ToolEngine.agent_invoke( - session=MagicMock(), + session=sqlite_session, tool=tool, tool_parameters={"a": 1}, user_id="u1", diff --git a/api/tests/unit_tests/core/workflow/nodes/llm/test_node.py b/api/tests/unit_tests/core/workflow/nodes/llm/test_node.py index d437c565949..d6f6771a5c2 100644 --- a/api/tests/unit_tests/core/workflow/nodes/llm/test_node.py +++ b/api/tests/unit_tests/core/workflow/nodes/llm/test_node.py @@ -351,6 +351,33 @@ def test_fetch_model_config_hydrates_model_instance_runtime_settings(model_confi provider_model.raise_for_status.assert_called_once() +@pytest.mark.parametrize( + ("provider", "model_name"), + [ + ("", "gpt-3.5-turbo"), + ("openai", ""), + ], +) +def test_fetch_model_config_rejects_unconfigured_model(provider: str, model_name: str): + credentials_provider = mock.MagicMock(spec=CredentialsProvider) + model_factory = mock.MagicMock(spec=DifyModelFactory) + + with pytest.raises(ValueError, match="LLM provider and model are required"): + fetch_model_config( + node_data_model=ModelConfig( + provider=provider, + name=model_name, + mode="chat", + completion_params={}, + ), + credentials_provider=credentials_provider, + model_factory=model_factory, + ) + + credentials_provider.fetch.assert_not_called() + model_factory.init_model_instance.assert_not_called() + + def test_fetch_model_config_reuses_validated_provider_model_from_dify_credentials_provider( model_config: ModelConfigWithCredentialsEntity, ): diff --git a/api/tests/unit_tests/libs/test_datetime_utils.py b/api/tests/unit_tests/libs/test_datetime_utils.py index 57314d29d4b..e2dce54d1e0 100644 --- a/api/tests/unit_tests/libs/test_datetime_utils.py +++ b/api/tests/unit_tests/libs/test_datetime_utils.py @@ -4,7 +4,7 @@ from unittest.mock import patch import pytest import pytz -from libs.datetime_utils import naive_utc_now, parse_time_range +from libs.datetime_utils import naive_utc_now, parse_time_range, to_utc_timestamp def test_naive_utc_now(monkeypatch: pytest.MonkeyPatch): @@ -24,6 +24,18 @@ def test_naive_utc_now(monkeypatch: pytest.MonkeyPatch): assert naive_time == utc_time +@pytest.mark.parametrize( + "value", + [ + datetime.datetime(2024, 1, 1), + datetime.datetime(2024, 1, 1, tzinfo=datetime.UTC), + datetime.datetime(2024, 1, 1, 9, tzinfo=datetime.timezone(datetime.timedelta(hours=9))), + ], +) +def test_to_utc_timestamp(value: datetime.datetime): + assert to_utc_timestamp(value) == 1704067200 + + class TestParseTimeRange: """Test cases for parse_time_range function.""" diff --git a/api/tests/unit_tests/libs/test_workspace_member_helper.py b/api/tests/unit_tests/libs/test_workspace_member_helper.py index f4933e7f594..d7f14b878a9 100644 --- a/api/tests/unit_tests/libs/test_workspace_member_helper.py +++ b/api/tests/unit_tests/libs/test_workspace_member_helper.py @@ -1,22 +1,65 @@ -"""Unit tests for require_workspace_member.""" +"""SQLite-backed unit tests for workspace membership enforcement.""" from __future__ import annotations import uuid -from unittest.mock import MagicMock, patch +from collections.abc import Iterator +from dataclasses import dataclass +from unittest.mock import Mock import pytest +from sqlalchemy import event +from sqlalchemy.orm import Session from werkzeug.exceptions import Forbidden +from libs import oauth_bearer from libs.oauth_bearer import AuthContext, Scope, SubjectType, TokenType, require_workspace_member +from models.account import Account, AccountStatus, Tenant, TenantAccountJoin, TenantAccountRole + +pytestmark = pytest.mark.usefixtures("community_edition") -def _ctx(verified: dict[str, bool] | None = None, *, account: bool = True) -> AuthContext: +@dataclass(frozen=True) +class Database: + """Real ORM binding and executed-statement log for one isolated test.""" + + session: Session + statements: list[str] + + +@pytest.fixture +def database(sqlite_session: Session, monkeypatch: pytest.MonkeyPatch) -> Iterator[Database]: + statements: list[str] = [] + + def record_statement(_connection, _cursor, statement, _parameters, _context, _executemany) -> None: + statements.append(statement) + + engine = sqlite_session.get_bind() + event.listen(engine, "before_cursor_execute", record_statement) + binding = Database(session=sqlite_session, statements=statements) + monkeypatch.setattr(oauth_bearer, "db", binding) + try: + yield binding + finally: + event.remove(engine, "before_cursor_execute", record_statement) + + +@pytest.fixture +def community_edition(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(oauth_bearer.dify_config, "ENTERPRISE_ENABLED", False) + + +def _ctx( + verified: dict[str, bool] | None = None, + *, + account_id: uuid.UUID | None = None, + account: bool = True, +) -> AuthContext: return AuthContext( subject_type=SubjectType.ACCOUNT if account else SubjectType.EXTERNAL_SSO, subject_email="e@example.com", subject_issuer=None, - account_id=uuid.uuid4() if account else None, + account_id=account_id or (uuid.uuid4() if account else None), client_id="difyctl", scopes=frozenset({Scope.FULL}), token_id=uuid.uuid4(), @@ -27,68 +70,130 @@ def _ctx(verified: dict[str, bool] | None = None, *, account: bool = True) -> Au ) -@patch("libs.oauth_bearer.dify_config") -def test_skips_when_enterprise_enabled(mock_cfg): - mock_cfg.ENTERPRISE_ENABLED = True - require_workspace_member(_ctx(), "t1") +def _persist_membership( + session: Session, + *, + account_id: uuid.UUID, + tenant_id: str, + status: AccountStatus = AccountStatus.ACTIVE, +) -> None: + account = _account(account_id, status=status) + tenant = Tenant(name=f"Tenant {tenant_id}") + tenant.id = tenant_id + membership = TenantAccountJoin( + tenant_id=tenant_id, + account_id=account_id.hex, + role=TenantAccountRole.NORMAL, + ) + session.add_all([account, tenant, membership]) + session.commit() -@patch("libs.oauth_bearer.dify_config") -def test_skips_for_external_sso(mock_cfg): - mock_cfg.ENTERPRISE_ENABLED = False - require_workspace_member(_ctx(account=False), "t1") +def _account(account_id: uuid.UUID, *, status: AccountStatus = AccountStatus.ACTIVE) -> Account: + account = Account(name="Workspace member", email=f"{account_id}@example.com", status=status) + # SQLite's StringUUID adapter binds UUID objects as compact hex, while + # PostgreSQL binds their dashed string form. Persist the SQLite-bound form + # so the production query can keep accepting the AuthContext UUID object. + account.id = account_id.hex + return account -@patch("libs.oauth_bearer.db") -@patch("libs.oauth_bearer.dify_config") -def test_uses_cached_ok_no_db_access(mock_cfg, mock_db): - mock_cfg.ENTERPRISE_ENABLED = False - require_workspace_member(_ctx({"t1": True}), "t1") - mock_db.session.execute.assert_not_called() +def test_skips_when_enterprise_enabled(database: Database, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(oauth_bearer.dify_config, "ENTERPRISE_ENABLED", True) + before = len(database.statements) + + require_workspace_member(_ctx(), "tenant-1") + + assert len(database.statements) == before -@patch("libs.oauth_bearer.db") -@patch("libs.oauth_bearer.dify_config") -def test_uses_cached_denied(mock_cfg, mock_db): - mock_cfg.ENTERPRISE_ENABLED = False +def test_skips_for_external_sso(database: Database) -> None: + before = len(database.statements) + + require_workspace_member(_ctx(account=False), "tenant-1") + + assert len(database.statements) == before + + +def test_uses_cached_allow_without_database_access(database: Database) -> None: + before = len(database.statements) + + require_workspace_member(_ctx({"tenant-1": True}), "tenant-1") + + assert len(database.statements) == before + + +def test_uses_cached_denial_without_database_access(database: Database) -> None: + before = len(database.statements) + with pytest.raises(Forbidden, match="workspace_membership_revoked"): - require_workspace_member(_ctx({"t1": False}), "t1") - mock_db.session.execute.assert_not_called() + require_workspace_member(_ctx({"tenant-1": False}), "tenant-1") + + assert len(database.statements) == before -@patch("libs.oauth_bearer.record_layer0_verdict") -@patch("libs.oauth_bearer.db") -@patch("libs.oauth_bearer.dify_config") -def test_denies_when_no_membership(mock_cfg, mock_db, mock_record): - mock_cfg.ENTERPRISE_ENABLED = False - mock_db.session.execute.return_value.scalar_one_or_none.return_value = None +@pytest.mark.usefixtures("database") +def test_denies_when_membership_is_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + record_verdict = Mock() + monkeypatch.setattr(oauth_bearer, "record_layer0_verdict", record_verdict) + with pytest.raises(Forbidden, match="workspace_membership_revoked"): - require_workspace_member(_ctx({}), "t1") - mock_record.assert_called_once_with("h1", "t1", False) + require_workspace_member(_ctx(), "tenant-1") + + record_verdict.assert_called_once_with("h1", "tenant-1", False) -@patch("libs.oauth_bearer.record_layer0_verdict") -@patch("libs.oauth_bearer.db") -@patch("libs.oauth_bearer.dify_config") -def test_denies_when_account_inactive(mock_cfg, mock_db, mock_record): - mock_cfg.ENTERPRISE_ENABLED = False - mock_db.session.execute.side_effect = [ - MagicMock(scalar_one_or_none=MagicMock(return_value="join-id")), - MagicMock(scalar_one_or_none=MagicMock(return_value="banned")), - ] +def test_denies_membership_from_another_tenant( + database: Database, + monkeypatch: pytest.MonkeyPatch, +) -> None: + account_id = uuid.uuid4() + requested_tenant_member_id = uuid.uuid4() + status_decoy_id = uuid.uuid4() + _persist_membership(database.session, account_id=account_id, tenant_id="tenant-2") + _persist_membership(database.session, account_id=requested_tenant_member_id, tenant_id="tenant-1") + database.session.add(_account(status_decoy_id, status=AccountStatus.BANNED)) + database.session.commit() + record_verdict = Mock() + monkeypatch.setattr(oauth_bearer, "record_layer0_verdict", record_verdict) + with pytest.raises(Forbidden, match="workspace_membership_revoked"): - require_workspace_member(_ctx({}), "t1") - mock_record.assert_called_once_with("h1", "t1", False) + require_workspace_member(_ctx(account_id=account_id), "tenant-1") + + record_verdict.assert_called_once_with("h1", "tenant-1", False) -@patch("libs.oauth_bearer.record_layer0_verdict") -@patch("libs.oauth_bearer.db") -@patch("libs.oauth_bearer.dify_config") -def test_allows_active_member(mock_cfg, mock_db, mock_record): - mock_cfg.ENTERPRISE_ENABLED = False - mock_db.session.execute.side_effect = [ - MagicMock(scalar_one_or_none=MagicMock(return_value="join-id")), - MagicMock(scalar_one_or_none=MagicMock(return_value="active")), - ] - require_workspace_member(_ctx({}), "t1") - mock_record.assert_called_once_with("h1", "t1", True) +def test_denies_when_account_is_inactive( + database: Database, + monkeypatch: pytest.MonkeyPatch, +) -> None: + account_id = uuid.uuid4() + _persist_membership( + database.session, + account_id=account_id, + tenant_id="tenant-1", + status=AccountStatus.BANNED, + ) + record_verdict = Mock() + monkeypatch.setattr(oauth_bearer, "record_layer0_verdict", record_verdict) + + with pytest.raises(Forbidden, match="workspace_membership_revoked"): + require_workspace_member(_ctx(account_id=account_id), "tenant-1") + + record_verdict.assert_called_once_with("h1", "tenant-1", False) + + +def test_allows_active_member_and_records_verdict( + database: Database, + monkeypatch: pytest.MonkeyPatch, +) -> None: + account_id = uuid.uuid4() + _persist_membership(database.session, account_id=account_id, tenant_id="tenant-1") + record_verdict = Mock() + monkeypatch.setattr(oauth_bearer, "record_layer0_verdict", record_verdict) + + require_workspace_member(_ctx(account_id=account_id), "tenant-1") + + record_verdict.assert_called_once_with("h1", "tenant-1", True) diff --git a/api/tests/unit_tests/services/test_metadata_bug_complete.py b/api/tests/unit_tests/services/test_metadata_bug_complete.py index 00f16f75ac0..01e693b3e81 100644 --- a/api/tests/unit_tests/services/test_metadata_bug_complete.py +++ b/api/tests/unit_tests/services/test_metadata_bug_complete.py @@ -3,6 +3,7 @@ from typing import cast from unittest.mock import Mock import pytest +from sqlalchemy.orm import Session from models import Account, Tenant from services.entities.knowledge_entities.knowledge_entities import MetadataArgs @@ -38,7 +39,8 @@ class TestMetadataBugCompleteValidation: assert valid_args.type == "string" assert valid_args.name == "test_name" - def test_2_business_logic_layer_crashes_on_none(self) -> None: + @pytest.mark.parametrize("sqlite_session", [()], indirect=True) + def test_2_business_logic_layer_crashes_on_none(self, sqlite_session: Session) -> None: """Test Layer 2: Business logic crashes when None values slip through.""" # Create mock that bypasses Pydantic validation mock_metadata_args = Mock() @@ -48,15 +50,18 @@ class TestMetadataBugCompleteValidation: account = _make_account() # Should crash with TypeError with pytest.raises(TypeError, match="object of type 'NoneType' has no len"): - MetadataService.create_metadata("dataset-123", mock_metadata_args, account, "tenant-123", session=Mock()) + MetadataService.create_metadata( + "dataset-123", mock_metadata_args, account, "tenant-123", session=sqlite_session + ) # Test update method as well account = _make_account() none_name = cast(str, None) with pytest.raises(TypeError, match="object of type 'NoneType' has no len"): MetadataService.update_metadata_name( - "dataset-123", "metadata-456", none_name, account, "tenant-123", session=Mock() + "dataset-123", "metadata-456", none_name, account, "tenant-123", session=sqlite_session ) + assert not sqlite_session.in_transaction() def test_3_database_constraints_verification(self) -> None: """Test Layer 3: Verify database model has nullable=False constraints.""" @@ -91,7 +96,8 @@ class TestMetadataBugCompleteValidation: assert args.type == "string" assert args.name == "valid_name" - def test_6_simulated_buggy_behavior(self) -> None: + @pytest.mark.parametrize("sqlite_session", [()], indirect=True) + def test_6_simulated_buggy_behavior(self, sqlite_session: Session) -> None: """Test simulating the original buggy behavior by bypassing Pydantic validation.""" mock_metadata_args = Mock() mock_metadata_args.name = None @@ -99,7 +105,10 @@ class TestMetadataBugCompleteValidation: account = _make_account() with pytest.raises(TypeError, match="object of type 'NoneType' has no len"): - MetadataService.create_metadata("dataset-123", mock_metadata_args, account, "tenant-123", session=Mock()) + MetadataService.create_metadata( + "dataset-123", mock_metadata_args, account, "tenant-123", session=sqlite_session + ) + assert not sqlite_session.in_transaction() def test_7_end_to_end_validation_layers(self) -> None: """Test all validation layers work together correctly.""" diff --git a/api/tests/unit_tests/services/workflow/test_workflow_event_snapshot_service.py b/api/tests/unit_tests/services/workflow/test_workflow_event_snapshot_service.py index 58892f0ebb3..4260f0064c6 100644 --- a/api/tests/unit_tests/services/workflow/test_workflow_event_snapshot_service.py +++ b/api/tests/unit_tests/services/workflow/test_workflow_event_snapshot_service.py @@ -985,7 +985,8 @@ def test_build_snapshot_events_preserves_public_form_token(monkeypatch: pytest.M ) session_maker = _SessionMaker( SimpleNamespace( - execute=lambda _stmt: [("form-1", datetime(2024, 1, 1, tzinfo=UTC), '{"display_in_ui": true}')], + # Persisted UTC datetimes can be loaded as naive values. + execute=lambda _stmt: [("form-1", datetime(2024, 1, 1), '{"display_in_ui": true}')], ) ) pause_entity = _FakePauseEntity( diff --git a/api/tests/unit_tests/tasks/test_human_input_timeout_tasks.py b/api/tests/unit_tests/tasks/test_human_input_timeout_tasks.py index be837acd2f0..b8cca3a1171 100644 --- a/api/tests/unit_tests/tasks/test_human_input_timeout_tasks.py +++ b/api/tests/unit_tests/tasks/test_human_input_timeout_tasks.py @@ -2,74 +2,22 @@ from __future__ import annotations from datetime import datetime, timedelta from types import SimpleNamespace -from typing import Any +from unittest.mock import MagicMock import pytest +from sqlalchemy.engine import Engine +from sqlalchemy.orm import Session, sessionmaker +import core.db.session_factory as session_factory_module +from core.repositories.human_input_repository import HumanInputFormSubmissionRepository +from core.workflow.nodes.human_input.entities import FormDefinition from core.workflow.nodes.human_input.enums import HumanInputFormKind, HumanInputFormStatus +from models.human_input import HumanInputForm from tasks import human_input_timeout_tasks as task_module -class _FakeScalarResult: - def __init__(self, items: list[Any]): - self._items = items - - def all(self) -> list[Any]: - return self._items - - -class _FakeSession: - def __init__(self, items: list[Any], capture: dict[str, Any]): - self._items = items - self._capture = capture - - def __enter__(self): - return self - - def __exit__(self, exc_type, exc, tb): - return False - - def scalars(self, stmt): - self._capture["stmt"] = stmt - return _FakeScalarResult(self._items) - - -class _FakeSessionFactory: - def __init__(self, items: list[Any], capture: dict[str, Any]): - self._items = items - self._capture = capture - self._capture["session_factory"] = self - - def __call__(self): - session = _FakeSession(self._items, self._capture) - self._capture["session"] = session - return session - - -class _FakeFormRepo: - def __init__(self, form_map: dict[str, Any] | None = None): - self.calls: list[dict[str, Any]] = [] - self._form_map = form_map or {} - - def mark_timeout(self, *, form_id: str, timeout_status: HumanInputFormStatus, reason: str | None = None): - self.calls.append( - { - "form_id": form_id, - "timeout_status": timeout_status, - "reason": reason, - } - ) - form = self._form_map.get(form_id) - return SimpleNamespace( - form_id=form_id, - workflow_run_id=getattr(form, "workflow_run_id", None), - conversation_id=getattr(form, "conversation_id", None), - node_id=getattr(form, "node_id", None), - ) - - class _FakeService: - def __init__(self, _session_factory, form_repository=None): + def __init__(self): self.enqueued: list[str] = [] self.agent_app_resumed: list[tuple[str, str]] = [] @@ -90,22 +38,49 @@ def _build_form( workflow_run_id: str | None, node_id: str, conversation_id: str | None = None, -) -> SimpleNamespace: - return SimpleNamespace( +) -> HumanInputForm: + form_definition = FormDefinition( + form_content="", + rendered_content="", + expiration_time=expiration_time, + ) + return HumanInputForm( id=form_id, + tenant_id="tenant-1", + app_id="app-1", form_kind=form_kind, created_at=created_at, expiration_time=expiration_time, workflow_run_id=workflow_run_id, conversation_id=conversation_id, node_id=node_id, + form_definition=form_definition.model_dump_json(), + rendered_content="", status=HumanInputFormStatus.WAITING, ) +@pytest.fixture +def sqlite_task_database( + sqlite_engine: Engine, + sqlite_session: Session, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repository_session_maker = sessionmaker(bind=sqlite_engine, expire_on_commit=False) + monkeypatch.setattr(session_factory_module, "_session_maker", repository_session_maker) + monkeypatch.setattr(task_module, "db", SimpleNamespace(engine=sqlite_engine)) + + def test_is_global_timeout_uses_created_at(): now = datetime(2025, 1, 1, 12, 0, 0) - form = SimpleNamespace(created_at=now - timedelta(seconds=61), workflow_run_id="run-1") + form = _build_form( + form_id="form-1", + form_kind=HumanInputFormKind.RUNTIME, + created_at=now - timedelta(seconds=61), + expiration_time=now + timedelta(hours=1), + workflow_run_id="run-1", + node_id="node-1", + ) assert task_module._is_global_timeout(form, 60, now=now) is True @@ -119,11 +94,16 @@ def test_is_global_timeout_uses_created_at(): assert task_module._is_global_timeout(form, 0, now=now) is False -def test_check_and_handle_human_input_timeouts_marks_and_routes(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize("sqlite_session", [(HumanInputForm,)], indirect=True) +def test_check_and_handle_human_input_timeouts_marks_and_routes( + monkeypatch: pytest.MonkeyPatch, + sqlite_task_database: None, + sqlite_engine: Engine, + sqlite_session: Session, +): now = datetime(2025, 1, 1, 12, 0, 0) monkeypatch.setattr(task_module, "naive_utc_now", lambda: now) monkeypatch.setattr(task_module.dify_config, "HUMAN_INPUT_GLOBAL_TIMEOUT_SECONDS", 3600) - monkeypatch.setattr(task_module, "db", SimpleNamespace(engine=object())) forms = [ _build_form( @@ -151,74 +131,131 @@ def test_check_and_handle_human_input_timeouts_marks_and_routes(monkeypatch: pyt node_id="node-delivery", ), ] + sqlite_session.add_all(forms) + sqlite_session.commit() - capture: dict[str, Any] = {} - monkeypatch.setattr(task_module, "sessionmaker", lambda *args, **kwargs: _FakeSessionFactory(forms, capture)) + repo = HumanInputFormSubmissionRepository() + mark_timeout_spy = MagicMock(wraps=repo.mark_timeout) + monkeypatch.setattr(repo, "mark_timeout", mark_timeout_spy) + service = _FakeService() + service_factory = MagicMock(return_value=service) + global_timeout_handler = MagicMock() - form_map = {form.id: form for form in forms} - repo = _FakeFormRepo(form_map=form_map) - - def _repo_factory(): - return repo - - service = _FakeService(None) - - def _service_factory(_session_factory, form_repository=None): - return service - - global_calls: list[dict[str, Any]] = [] - - monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", _repo_factory) - monkeypatch.setattr(task_module, "HumanInputService", _service_factory) - monkeypatch.setattr(task_module, "_handle_global_timeout", lambda **kwargs: global_calls.append(kwargs)) + monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", lambda: repo) + monkeypatch.setattr(task_module, "HumanInputService", service_factory) + monkeypatch.setattr(task_module, "_handle_global_timeout", global_timeout_handler) task_module.check_and_handle_human_input_timeouts(limit=100) - assert {(call["form_id"], call["timeout_status"], call["reason"]) for call in repo.calls} == { + assert { + (call.kwargs["form_id"], call.kwargs["timeout_status"], call.kwargs["reason"]) + for call in mark_timeout_spy.call_args_list + } == { ("form-global", HumanInputFormStatus.EXPIRED, "global_timeout"), ("form-node", HumanInputFormStatus.TIMEOUT, "node_timeout"), ("form-delivery", HumanInputFormStatus.TIMEOUT, "delivery_test_timeout"), } assert service.enqueued == ["run-node"] - assert global_calls == [ - { - "form_id": "form-global", - "workflow_run_id": "run-global", - "node_id": "node-global", - "session_factory": capture.get("session_factory"), - } - ] + global_timeout_handler.assert_called_once() + global_timeout_call = global_timeout_handler.call_args.kwargs + assert global_timeout_call["form_id"] == "form-global" + assert global_timeout_call["workflow_run_id"] == "run-global" + assert global_timeout_call["node_id"] == "node-global" + task_session_maker = global_timeout_call["session_factory"] + assert isinstance(task_session_maker, sessionmaker) + assert task_session_maker.kw["bind"] is sqlite_engine + service_factory.assert_called_once_with(task_session_maker, form_repository=repo) - stmt = capture.get("stmt") - assert stmt is not None - stmt_text = str(stmt) - assert "created_at <=" in stmt_text - assert "expiration_time <=" in stmt_text - assert "ORDER BY human_input_forms.id" in stmt_text + sqlite_session.expire_all() + assert sqlite_session.get(HumanInputForm, "form-global").status == HumanInputFormStatus.EXPIRED + assert sqlite_session.get(HumanInputForm, "form-node").status == HumanInputFormStatus.TIMEOUT + assert sqlite_session.get(HumanInputForm, "form-delivery").status == HumanInputFormStatus.TIMEOUT -def test_check_and_handle_human_input_timeouts_omits_global_filter_when_disabled(monkeypatch: pytest.MonkeyPatch): +@pytest.mark.parametrize("sqlite_session", [(HumanInputForm,)], indirect=True) +def test_check_and_handle_human_input_timeouts_orders_by_id_before_limit( + monkeypatch: pytest.MonkeyPatch, + sqlite_task_database: None, + sqlite_session: Session, +): now = datetime(2025, 1, 1, 12, 0, 0) monkeypatch.setattr(task_module, "naive_utc_now", lambda: now) monkeypatch.setattr(task_module.dify_config, "HUMAN_INPUT_GLOBAL_TIMEOUT_SECONDS", 0) - monkeypatch.setattr(task_module, "db", SimpleNamespace(engine=object())) - capture: dict[str, Any] = {} - monkeypatch.setattr(task_module, "sessionmaker", lambda *args, **kwargs: _FakeSessionFactory([], capture)) - monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", _FakeFormRepo) - monkeypatch.setattr(task_module, "HumanInputService", _FakeService) - monkeypatch.setattr(task_module, "_handle_global_timeout", lambda **_kwargs: None) + forms = [ + _build_form( + form_id=form_id, + form_kind=HumanInputFormKind.DELIVERY_TEST, + created_at=now - timedelta(minutes=1), + expiration_time=now - timedelta(seconds=1), + workflow_run_id=None, + node_id=f"node-{form_id}", + ) + for form_id in ("form-b", "form-a") + ] + sqlite_session.add_all(forms) + sqlite_session.commit() + + repo = HumanInputFormSubmissionRepository() + mark_timeout_spy = MagicMock(wraps=repo.mark_timeout) + monkeypatch.setattr(repo, "mark_timeout", mark_timeout_spy) + monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", lambda: repo) + monkeypatch.setattr(task_module, "HumanInputService", MagicMock(return_value=_FakeService())) task_module.check_and_handle_human_input_timeouts(limit=1) - stmt = capture.get("stmt") - assert stmt is not None - stmt_text = str(stmt) - assert "created_at <=" not in stmt_text + mark_timeout_spy.assert_called_once_with( + form_id="form-a", + timeout_status=HumanInputFormStatus.TIMEOUT, + reason="delivery_test_timeout", + ) + sqlite_session.expire_all() + assert sqlite_session.get(HumanInputForm, "form-a").status == HumanInputFormStatus.TIMEOUT + assert sqlite_session.get(HumanInputForm, "form-b").status == HumanInputFormStatus.WAITING +@pytest.mark.parametrize("sqlite_session", [(HumanInputForm,)], indirect=True) +def test_check_and_handle_human_input_timeouts_omits_global_filter_when_disabled( + monkeypatch: pytest.MonkeyPatch, + sqlite_task_database: None, + sqlite_session: Session, +): + now = datetime(2025, 1, 1, 12, 0, 0) + monkeypatch.setattr(task_module, "naive_utc_now", lambda: now) + monkeypatch.setattr(task_module.dify_config, "HUMAN_INPUT_GLOBAL_TIMEOUT_SECONDS", 0) + + old_unexpired_form = _build_form( + form_id="form-old", + form_kind=HumanInputFormKind.RUNTIME, + created_at=now - timedelta(hours=2), + expiration_time=now + timedelta(hours=1), + workflow_run_id="run-old", + node_id="node-old", + ) + sqlite_session.add(old_unexpired_form) + sqlite_session.commit() + + repo = HumanInputFormSubmissionRepository() + mark_timeout_spy = MagicMock(wraps=repo.mark_timeout) + monkeypatch.setattr(repo, "mark_timeout", mark_timeout_spy) + monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", lambda: repo) + monkeypatch.setattr(task_module, "HumanInputService", MagicMock(return_value=_FakeService())) + global_timeout_handler = MagicMock() + monkeypatch.setattr(task_module, "_handle_global_timeout", global_timeout_handler) + + task_module.check_and_handle_human_input_timeouts(limit=1) + + mark_timeout_spy.assert_not_called() + global_timeout_handler.assert_not_called() + sqlite_session.refresh(old_unexpired_form) + assert old_unexpired_form.status == HumanInputFormStatus.WAITING + + +@pytest.mark.parametrize("sqlite_session", [(HumanInputForm,)], indirect=True) def test_check_and_handle_human_input_timeouts_routes_conversation_owned_form_to_agent_app_resume( monkeypatch: pytest.MonkeyPatch, + sqlite_task_database: None, + sqlite_session: Session, ): # ENG-635 (review): a conversation-owned Agent v2 chat ask_human form has no # workflow_run_id. On timeout it must enqueue the Agent App resume (so the @@ -227,24 +264,23 @@ def test_check_and_handle_human_input_timeouts_routes_conversation_owned_form_to now = datetime(2025, 1, 1, 12, 0, 0) monkeypatch.setattr(task_module, "naive_utc_now", lambda: now) monkeypatch.setattr(task_module.dify_config, "HUMAN_INPUT_GLOBAL_TIMEOUT_SECONDS", 3600) - monkeypatch.setattr(task_module, "db", SimpleNamespace(engine=object())) - forms = [ - _build_form( - form_id="form-chat", - form_kind=HumanInputFormKind.RUNTIME, - created_at=now - timedelta(minutes=5), - expiration_time=now - timedelta(seconds=1), - workflow_run_id=None, - conversation_id="conv-1", - node_id="agent", - ), - ] - capture: dict[str, Any] = {} - monkeypatch.setattr(task_module, "sessionmaker", lambda *args, **kwargs: _FakeSessionFactory(forms, capture)) + form = _build_form( + form_id="form-chat", + form_kind=HumanInputFormKind.RUNTIME, + created_at=now - timedelta(minutes=5), + expiration_time=now - timedelta(seconds=1), + workflow_run_id=None, + conversation_id="conv-1", + node_id="agent", + ) + sqlite_session.add(form) + sqlite_session.commit() - repo = _FakeFormRepo(form_map={form.id: form for form in forms}) - service = _FakeService(None) + repo = HumanInputFormSubmissionRepository() + mark_timeout_spy = MagicMock(wraps=repo.mark_timeout) + monkeypatch.setattr(repo, "mark_timeout", mark_timeout_spy) + service = _FakeService() monkeypatch.setattr(task_module, "HumanInputFormSubmissionRepository", lambda: repo) monkeypatch.setattr(task_module, "HumanInputService", lambda *_args, **_kwargs: service) monkeypatch.setattr(task_module, "_handle_global_timeout", lambda **_kwargs: None) @@ -252,8 +288,10 @@ def test_check_and_handle_human_input_timeouts_routes_conversation_owned_form_to task_module.check_and_handle_human_input_timeouts(limit=100) # Node timeout (conversation forms are never "global"), routed to Agent App resume. - assert repo.calls == [ - {"form_id": "form-chat", "timeout_status": HumanInputFormStatus.TIMEOUT, "reason": "node_timeout"} - ] + mark_timeout_spy.assert_called_once_with( + form_id="form-chat", timeout_status=HumanInputFormStatus.TIMEOUT, reason="node_timeout" + ) assert service.agent_app_resumed == [("conv-1", "form-chat")] assert service.enqueued == [] + sqlite_session.refresh(form) + assert form.status == HumanInputFormStatus.TIMEOUT diff --git a/api/tests/unit_tests/tasks/test_workflow_execute_task.py b/api/tests/unit_tests/tasks/test_workflow_execute_task.py index 3b9cad30018..f99d4a1d942 100644 --- a/api/tests/unit_tests/tasks/test_workflow_execute_task.py +++ b/api/tests/unit_tests/tasks/test_workflow_execute_task.py @@ -3,6 +3,7 @@ from __future__ import annotations import json import logging import uuid +from collections.abc import Generator, Mapping from contextlib import nullcontext from datetime import datetime from decimal import Decimal @@ -36,6 +37,7 @@ from tasks.app_generate.workflow_execute_task import ( class _StreamEventModel(BaseModel): event: object | None = None task_id: object | None = None + message: object | None = None def _build_advanced_chat_generate_entity(conversation_id: str | None) -> AdvancedChatAppGenerateEntity: @@ -248,6 +250,21 @@ def test_get_task_id(event: object, expected: str | None): assert workflow_execute_task_module._get_task_id(event) == expected +@pytest.mark.parametrize( + ("event", "expected"), + [ + ({"message": "workflow error"}, "workflow error"), + (_StreamEventModel(message="workflow error"), "workflow error"), + ({"message": ""}, None), + ({"message": 123}, None), + ({}, None), + ("workflow error", None), + ], +) +def test_get_error_message(event: str | Mapping[str, object] | BaseModel, expected: str | None): + assert workflow_execute_task_module._get_error_message(event) == expected + + @pytest.fixture def mock_topic(monkeypatch: pytest.MonkeyPatch) -> MagicMock: topic = MagicMock() @@ -486,6 +503,38 @@ def test_publish_streaming_response_publishes_failed_terminal_on_exhaustion_with assert "ended without a terminal event" in caplog.text +def test_publish_streaming_response_uses_error_message_for_failed_terminal(mock_topic: MagicMock): + def response_stream() -> Generator[str | Mapping[str, object] | BaseModel, None, None]: + yield { + "event": "error", + "workflow_run_id": "workflow-run-id", + "code": "invalid_param", + "message": "LLM provider and model are required.", + "status": 400, + } + + _publish_streaming_response( + response_stream(), + "workflow-run-id", + app_mode=AppMode.WORKFLOW, + workflow_id="workflow-id", + inputs={}, + started_reason=WorkflowStartReason.INITIAL, + ) + + payloads = _published_payloads(mock_topic) + error_payload = payloads[0] + finished_payload = payloads[-1] + assert isinstance(error_payload, dict) + assert isinstance(finished_payload, dict) + assert error_payload["status"] == 400 + assert error_payload["message"] == "LLM provider and model are required." + finished_data = finished_payload["data"] + assert isinstance(finished_data, dict) + assert finished_data["status"] == WorkflowExecutionStatus.FAILED + assert finished_data["error"] == "LLM provider and model are required." + + def test_publish_streaming_response_does_not_publish_synthetic_failure_after_terminal_event(mock_topic: MagicMock): response_stream = iter( [ diff --git a/docker/docker-compose-template.yaml b/docker/docker-compose-template.yaml index d7e610ca798..cebd2523ee5 100644 --- a/docker/docker-compose-template.yaml +++ b/docker/docker-compose-template.yaml @@ -281,6 +281,8 @@ services: SERVER_WORKER_CONNECTIONS: ${API_WEBSOCKET_WORKER_CONNECTIONS:-1000} GUNICORN_TIMEOUT: ${API_WEBSOCKET_GUNICORN_TIMEOUT:-360} depends_on: + init_permissions: + condition: service_completed_successfully db_postgres: condition: service_healthy required: false @@ -289,6 +291,9 @@ services: required: false redis: condition: service_started + volumes: + # Mount the storage directory to the container, for storing user files. + - ./volumes/app/storage:/app/api/storage networks: - ssrf_proxy_network - default diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index e61ce926677..ddc21e5e400 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -287,6 +287,8 @@ services: SERVER_WORKER_CONNECTIONS: ${API_WEBSOCKET_WORKER_CONNECTIONS:-1000} GUNICORN_TIMEOUT: ${API_WEBSOCKET_GUNICORN_TIMEOUT:-360} depends_on: + init_permissions: + condition: service_completed_successfully db_postgres: condition: service_healthy required: false @@ -295,6 +297,9 @@ services: required: false redis: condition: service_started + volumes: + # Mount the storage directory to the container, for storing user files. + - ./volumes/app/storage:/app/api/storage networks: - ssrf_proxy_network - default diff --git a/oxlint-suppressions.json b/oxlint-suppressions.json index 87eb09c5b33..e6fab131196 100644 --- a/oxlint-suppressions.json +++ b/oxlint-suppressions.json @@ -5022,11 +5022,6 @@ "count": 3 } }, - "web/app/components/workflow/nodes/parameter-extractor/components/extract-parameter/__tests__/list.spec.tsx": { - "no-unused-vars": { - "count": 1 - } - }, "web/app/components/workflow/nodes/parameter-extractor/components/extract-parameter/item.tsx": { "jsx_a11y/click-events-have-key-events": { "count": 2 diff --git a/web/__tests__/base/notion-page-selector-flow.test.tsx b/web/__tests__/base/notion-page-selector-flow.test.tsx index 524093c437f..14a7ae9535d 100644 --- a/web/__tests__/base/notion-page-selector-flow.test.tsx +++ b/web/__tests__/base/notion-page-selector-flow.test.tsx @@ -4,11 +4,10 @@ import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { beforeEach, describe, expect, it, vi } from 'vitest' import NotionPageSelector from '@/app/components/base/notion-page-selector/base' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { CredentialTypeEnum } from '@/app/components/plugins/plugin-auth/types' const mockInvalidPreImportNotionPages = vi.fn() -const mockSetShowAccountSettingModal = vi.fn() +const mockSetSettingsDestination = vi.fn() const mockUsePreImportNotionPages = vi.fn() vi.mock('@tanstack/react-virtual', () => ({ @@ -29,16 +28,10 @@ vi.mock('@/service/knowledge/use-import', () => ({ useInvalidPreImportNotionPages: () => mockInvalidPreImportNotionPages, })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: () => ({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), - useModalContextSelector: ( - selector: (state: { - setShowAccountSettingModal: typeof mockSetShowAccountSettingModal - }) => unknown, - ) => selector({ setShowAccountSettingModal: mockSetShowAccountSettingModal }), -})) +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) const buildCredential = ( id: string, @@ -200,8 +193,6 @@ describe('Base Notion Page Selector Flow', () => { await user.click( screen.getByRole('button', { name: 'common.dataSource.notion.selector.configure' }), ) - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.DATA_SOURCE, - }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('data-source') }) }) diff --git a/web/__tests__/billing/billing-integration.test.tsx b/web/__tests__/billing/billing-integration.test.tsx index 494b6593054..f522f39159e 100644 --- a/web/__tests__/billing/billing-integration.test.tsx +++ b/web/__tests__/billing/billing-integration.test.tsx @@ -41,7 +41,6 @@ const render = (ui: ReactElement, options: RenderOptions = {}) => { } const mockSetShowPricingModal = vi.fn() -const mockSetShowAccountSettingModal = vi.fn() vi.mock('@/context/provider-context', () => ({ useProviderContext: () => mockProviderCtx, @@ -64,10 +63,6 @@ vi.mock('@/context/modal-context', () => ({ useModalContext: () => ({ setShowPricingModal: mockSetShowPricingModal, }), - useModalContextSelector: (selector: (s: Record) => unknown) => - selector({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), })) vi.mock('@/context/i18n', () => ({ diff --git a/web/__tests__/billing/education-verification-flow.test.tsx b/web/__tests__/billing/education-verification-flow.test.tsx index caa04d1e73d..769a00a30d0 100644 --- a/web/__tests__/billing/education-verification-flow.test.tsx +++ b/web/__tests__/billing/education-verification-flow.test.tsx @@ -42,10 +42,8 @@ const render = (ui: ReactElement, options: RenderOptions = {}) => { // ─── Mock state ────────────────────────────────────────────────────────────── const mockSetShowPricingModal = vi.fn() -const mockSetShowAccountSettingModal = vi.fn() const mockRouterPush = vi.fn() const mockMutateAsync = vi.fn() -const mockSetEducationVerifying = vi.hoisted(() => vi.fn()) // ─── Context mocks ─────────────────────────────────────────────────────────── vi.mock('@/context/provider-context', () => ({ @@ -69,10 +67,6 @@ vi.mock('@/context/modal-context', () => ({ useModalContext: () => ({ setShowPricingModal: mockSetShowPricingModal, }), - useModalContextSelector: (selector: (s: Record) => unknown) => - selector({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), })) // ─── Service mocks ─────────────────────────────────────────────────────────── @@ -102,10 +96,6 @@ vi.mock('@/hooks/use-async-window-open', () => ({ useAsyncWindowOpen: () => vi.fn(), })) -vi.mock('@/app/education-apply/storage', () => ({ - useSetEducationVerifying: () => mockSetEducationVerifying, -})) - // ─── External component mocks ─────────────────────────────────────────────── vi.mock('@/app/education-apply/verify-state-modal', () => ({ default: ({ @@ -249,20 +239,6 @@ describe('Education Verification Flow', () => { expect(mockRouterPush).toHaveBeenCalledWith('/education-apply?token=edu-token-123') }) }) - - it('should clear education verifying flag on success', async () => { - mockMutateAsync.mockResolvedValue({ token: 'token-xyz' }) - setupContexts({}, { enableEducationPlan: true, isEducationAccount: false }) - const user = userEvent.setup() - - render() - - await user.click(screen.getByText(/toVerified/i)) - - await waitFor(() => { - expect(mockSetEducationVerifying).toHaveBeenCalledWith(null) - }) - }) }) // ─── 3. Failed Verification Flow ──────────────────────────────────────── diff --git a/web/app/(commonLayout)/global-mounts.tsx b/web/app/(commonLayout)/global-mounts.tsx index 7d0c118b95f..f85cfff5a5c 100644 --- a/web/app/(commonLayout)/global-mounts.tsx +++ b/web/app/(commonLayout)/global-mounts.tsx @@ -1,5 +1,6 @@ 'use client' +import { SettingsModal } from '@/app/components/header/account-setting/settings-modal' import dynamic from '@/next/dynamic' const InSiteMessageNotification = dynamic( @@ -25,6 +26,7 @@ export function CommonLayoutGlobalMounts() { + ) } diff --git a/web/app/(commonLayout)/providers.tsx b/web/app/(commonLayout)/providers.tsx index 28cd8ab94f5..e0cc043fc2a 100644 --- a/web/app/(commonLayout)/providers.tsx +++ b/web/app/(commonLayout)/providers.tsx @@ -1,5 +1,4 @@ import type { ReactNode } from 'react' -import { EducationVerifyActionRecorder } from '@/app/components/education-verify-action-recorder' import { OAuthRegistrationAnalytics } from '@/app/components/oauth-registration-analytics' import { EventEmitterContextProvider } from '@/context/event-emitter-provider' import { ModalContextProvider } from '@/context/modal-context-provider' @@ -12,7 +11,6 @@ export async function ConsoleRuntimeProviders({ children }: { children: ReactNod return ( <> - diff --git a/web/app/components/__tests__/education-verify-action-recorder.spec.tsx b/web/app/components/__tests__/education-verify-action-recorder.spec.tsx deleted file mode 100644 index 6abcf2226ed..00000000000 --- a/web/app/components/__tests__/education-verify-action-recorder.spec.tsx +++ /dev/null @@ -1,47 +0,0 @@ -import { render, waitFor } from '@testing-library/react' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { EDUCATION_VERIFY_URL_SEARCHPARAMS_ACTION } from '@/app/education-apply/constants' -import { useSearchParams } from '@/next/navigation' -import { EducationVerifyActionRecorder } from '../education-verify-action-recorder' - -const setEducationVerifyingMock = vi.hoisted(() => vi.fn()) - -vi.mock('@/next/navigation', () => ({ - useSearchParams: vi.fn(), -})) - -vi.mock('@/app/education-apply/storage', () => ({ - useSetEducationVerifying: () => setEducationVerifyingMock, -})) - -const mockUseSearchParams = vi.mocked(useSearchParams) - -describe('EducationVerifyActionRecorder', () => { - beforeEach(() => { - vi.clearAllMocks() - window.localStorage.clear() - mockUseSearchParams.mockReturnValue( - new URLSearchParams() as unknown as ReturnType, - ) - }) - - it('should store the education verification flag when the callback action is present', async () => { - mockUseSearchParams.mockReturnValue( - new URLSearchParams( - `action=${EDUCATION_VERIFY_URL_SEARCHPARAMS_ACTION}`, - ) as unknown as ReturnType, - ) - - render() - - await waitFor(() => { - expect(setEducationVerifyingMock).toHaveBeenCalledWith('yes') - }) - }) - - it('should leave localStorage unchanged for unrelated routes', () => { - render() - - expect(setEducationVerifyingMock).not.toHaveBeenCalled() - }) -}) diff --git a/web/app/components/app/configuration/dataset-config/settings-modal/__tests__/index.spec.tsx b/web/app/components/app/configuration/dataset-config/settings-modal/__tests__/index.spec.tsx index cf29f617e13..81d0904bc6c 100644 --- a/web/app/components/app/configuration/dataset-config/settings-modal/__tests__/index.spec.tsx +++ b/web/app/components/app/configuration/dataset-config/settings-modal/__tests__/index.spec.tsx @@ -5,7 +5,6 @@ import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { IndexingType } from '@/app/components/datasets/create/step-two' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { ModelTypeEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { systemFeaturesQueryOptions } from '@/features/system-features/client' import { @@ -50,7 +49,7 @@ vi.mock('@langgenius/dify-ui/toast', () => ({ })) const mockOnCancel = vi.fn() const mockOnSave = vi.fn() -const mockSetShowAccountSettingModal = vi.fn() +const mockSetSettingsDestination = vi.fn() const mockUseModelList = vi.fn() const mockUseModelListAndDefaultModel = vi.fn() @@ -81,11 +80,10 @@ vi.mock('@/service/use-common', async () => ({ useMembers: vi.fn(), })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: () => ({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), -})) +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) vi.mock('@/context/i18n', () => ({ useDocLink: () => (path: string) => `https://docs${path}`, @@ -396,9 +394,7 @@ describe('SettingsModal', () => { ) // Assert - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.PROVIDER, - }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('provider') }) }) diff --git a/web/app/components/app/configuration/dataset-config/settings-modal/index.tsx b/web/app/components/app/configuration/dataset-config/settings-modal/index.tsx index 36a5bf54007..ce20215f8d7 100644 --- a/web/app/components/app/configuration/dataset-config/settings-modal/index.tsx +++ b/web/app/components/app/configuration/dataset-config/settings-modal/index.tsx @@ -9,6 +9,7 @@ import { Textarea } from '@langgenius/dify-ui/textarea' import { toast } from '@langgenius/dify-ui/toast' import { RiCloseLine } from '@remixicon/react' import { isEqual } from 'es-toolkit/predicate' +import { useQueryState } from 'nuqs' import { useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import Input from '@/app/components/base/input' @@ -17,11 +18,13 @@ import { IndexingType } from '@/app/components/datasets/create/step-two' import IndexMethod from '@/app/components/datasets/settings/index-method' import PermissionSelector from '@/app/components/datasets/settings/permission-selector' import { checkShowMultiModalTip } from '@/app/components/datasets/settings/utils' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { ModelTypeEnum } from '@/app/components/header/account-setting/model-provider-page/declarations' import { useModelList } from '@/app/components/header/account-setting/model-provider-page/hooks' import ModelSelector from '@/app/components/header/account-setting/model-provider-page/model-selector' -import { useIntegrationsSetting } from '@/app/components/header/account-setting/use-integrations-setting' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { useDocLink } from '@/context/i18n' import { DatasetPermission } from '@/models/datasets' import { updateDatasetSetting } from '@/service/datasets' @@ -56,7 +59,7 @@ const SettingsModal: FC = ({ const docLink = useDocLink() const ref = useRef(null) const isExternal = currentDataset.provider === 'external' - const openIntegrationsSetting = useIntegrationsSetting() + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const [loading, setLoading] = useState(false) const [localeCurrentDataset, setLocaleCurrentDataset] = useState({ ...currentDataset }) const [topK, setTopK] = useState(localeCurrentDataset?.external_retrieval_model.top_k ?? 2) @@ -315,7 +318,7 @@ const SettingsModal: FC = ({ diff --git a/web/app/components/app/configuration/hooks/__tests__/use-configuration.spec.tsx b/web/app/components/app/configuration/hooks/__tests__/use-configuration.spec.tsx index 6e47677e660..469658233b7 100644 --- a/web/app/components/app/configuration/hooks/__tests__/use-configuration.spec.tsx +++ b/web/app/components/app/configuration/hooks/__tests__/use-configuration.spec.tsx @@ -6,7 +6,7 @@ import { AppModeEnum, ModelModeType } from '@/types/app' import { AppACLPermission } from '@/utils/permission' import { useConfiguration } from '../use-configuration' -const mockSetShowAccountSettingModal = vi.fn() +const mockSetSettingsDestination = vi.fn() const mockSetShowAppConfigureFeaturesModal = vi.fn() const mockSetDetailSidebarMode = vi.fn() const mockHandleMultipleModelConfigsChange = vi.fn() @@ -73,11 +73,10 @@ vi.mock('@/context/permission-state', async () => { })) }) -vi.mock('@/context/modal-context', () => ({ - useModalContext: () => ({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), -})) +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) vi.mock('@/context/provider-context', () => ({ useProviderContext: () => ({ @@ -493,7 +492,7 @@ describe('useConfiguration', () => { expect(mockFormattingChangedDispatcher).toHaveBeenCalled() expect(mockHandleMultipleModelConfigsChange).toHaveBeenCalled() expect(mockSetDetailSidebarMode).toHaveBeenCalledWith('collapse') - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ payload: 'provider' }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('provider') expect(mockSetConversationHistoriesRole).toHaveBeenCalledWith({ assistant_prefix: 'bot', user_prefix: 'user', diff --git a/web/app/components/app/configuration/hooks/use-configuration.ts b/web/app/components/app/configuration/hooks/use-configuration.ts index a9326c3ae8a..3420b0bb752 100644 --- a/web/app/components/app/configuration/hooks/use-configuration.ts +++ b/web/app/components/app/configuration/hooks/use-configuration.ts @@ -29,6 +29,7 @@ import { useBoolean, useGetState } from 'ahooks' import { clone } from 'es-toolkit/object' import { produce } from 'immer' import { useAtomValue } from 'jotai' +import { useQueryState } from 'nuqs' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import { useShallow } from 'zustand/react/shallow' @@ -39,7 +40,6 @@ import { import useAdvancedPromptConfig from '@/app/components/app/configuration/hooks/use-advanced-prompt-config' import { useStore as useAppStore } from '@/app/components/app/store' import { useSetDetailSidebarMode } from '@/app/components/detail-sidebar/storage' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { ModelFeatureEnum, ModelTypeEnum, @@ -48,7 +48,10 @@ import { useModelListAndDefaultModelAndCurrentProviderAndModel, useTextGenerationCurrentProviderAndModelAndModelList, } from '@/app/components/header/account-setting/model-provider-page/hooks' -import { useIntegrationsSetting } from '@/app/components/header/account-setting/use-integrations-setting' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { ANNOTATION_DEFAULT, DATASET_DEFAULT, @@ -128,7 +131,7 @@ export const useConfiguration = (): ConfigurationViewModel => { const currentWorkspace = useAtomValue(currentWorkspaceAtom) const currentUserId = useAtomValue(userProfileIdAtom) const workspacePermissionKeys = useAtomValue(workspacePermissionKeysAtom) - const openIntegrationsSetting = useIntegrationsSetting() + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const { appDetail, showAppConfigureFeaturesModal, setShowAppConfigureFeaturesModal } = useAppStore( @@ -769,7 +772,7 @@ export const useConfiguration = (): ConfigurationViewModel => { onCloseSelectDataSet: hideSelectDataSet, onCompletionParamsChange: setCompletionParams, onConfirmUseGPT4: () => { - openIntegrationsSetting({ payload: ACCOUNT_SETTING_TAB.PROVIDER }) + setSettingsDestination('provider') setShowUseGPT4Confirm(false) }, onEnableMultipleModelDebug: handleDebugWithMultipleModelChange, @@ -777,7 +780,7 @@ export const useConfiguration = (): ConfigurationViewModel => { onHideDebugPanel: hideDebugPanel, onModelChange: setModel, onMultipleModelConfigsChange: handleMultipleModelConfigsChange, - onOpenAccountSettings: () => openIntegrationsSetting({ payload: ACCOUNT_SETTING_TAB.PROVIDER }), + onOpenAccountSettings: () => setSettingsDestination('provider'), onOpenDebugPanel: showDebugPanel, onSaveHistory: (data) => { setConversationHistoriesRole(data) diff --git a/web/app/components/app/log/__tests__/archived-logs-notice.spec.tsx b/web/app/components/app/log/__tests__/archived-logs-notice.spec.tsx index 9f7214d4ae4..3e345357417 100644 --- a/web/app/components/app/log/__tests__/archived-logs-notice.spec.tsx +++ b/web/app/components/app/log/__tests__/archived-logs-notice.spec.tsx @@ -3,8 +3,6 @@ import userEvent from '@testing-library/user-event' import { createMockProviderContextValue } from '@/__mocks__/provider-context' import { defaultPlan } from '@/app/components/billing/config' import { Plan } from '@/app/components/billing/type' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useModalContextSelector } from '@/context/modal-context' import { useProviderContext } from '@/context/provider-context' import { createConsoleQueryWrapper } from '@/test/console/query-data' import { render } from '@/test/console/render' @@ -26,16 +24,13 @@ vi.mock('@/context/provider-context', async (importOriginal) => { } }) -vi.mock('@/context/modal-context', async (importOriginal) => { - const actual = await importOriginal() - return { - ...actual, - useModalContextSelector: vi.fn(), - } +const setSettingsDestination = vi.fn() +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, setSettingsDestination] } }) const mockUseProviderContext = vi.mocked(useProviderContext) -const mockUseModalContextSelector = vi.mocked(useModalContextSelector) function mockProviderPlan(planType: Plan) { mockUseProviderContext.mockReturnValue( @@ -50,7 +45,6 @@ function mockProviderPlan(planType: Plan) { } describe('ArchivedLogsNotice', () => { - const setShowAccountSettingModal = vi.fn() const renderNotice = () => { const { wrapper } = createConsoleQueryWrapper({ systemFeatures: { deployment_edition: 'CLOUD' }, @@ -61,11 +55,6 @@ describe('ArchivedLogsNotice', () => { beforeEach(() => { vi.clearAllMocks() mockProviderPlan(Plan.professional) - mockUseModalContextSelector.mockImplementation((selector) => - selector({ - setShowAccountSettingModal, - } as unknown as Parameters[0]), - ) }) it('should show an accessible notice for paid workspace managers', async () => { @@ -78,9 +67,7 @@ describe('ArchivedLogsNotice', () => { expect(within(notice).getByText('appLog.archives.notice.description')).toBeInTheDocument() await user.click(within(notice).getByRole('button', { name: 'appLog.archives.notice.action' })) - expect(setShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.WORKFLOW_LOG_ARCHIVES, - }) + expect(setSettingsDestination).toHaveBeenCalledWith('workflow-log-archives') }) it('should not show notice for sandbox workspaces', () => { diff --git a/web/app/components/app/log/archived-logs-notice.tsx b/web/app/components/app/log/archived-logs-notice.tsx index ef3f59f1c72..2792e3eade5 100644 --- a/web/app/components/app/log/archived-logs-notice.tsx +++ b/web/app/components/app/log/archived-logs-notice.tsx @@ -3,10 +3,13 @@ import { Button } from '@langgenius/dify-ui/button' import { useSuspenseQuery } from '@tanstack/react-query' import { useAtomValue } from 'jotai' +import { useQueryState } from 'nuqs' import { useTranslation } from 'react-i18next' import { Plan } from '@/app/components/billing/type' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useModalContextSelector } from '@/context/modal-context' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { useProviderContext } from '@/context/provider-context' import { isCurrentWorkspaceManagerAtom } from '@/context/workspace-state' import { systemFeaturesQueryOptions } from '@/features/system-features/client' @@ -19,9 +22,7 @@ export function ArchivedLogsNotice() { }) const isCurrentWorkspaceManager = useAtomValue(isCurrentWorkspaceManagerAtom) const { enableBilling, plan } = useProviderContext() - const setShowAccountSettingModal = useModalContextSelector( - (state) => state.setShowAccountSettingModal, - ) + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) if ( deploymentEdition !== 'CLOUD' || @@ -53,11 +54,7 @@ export function ArchivedLogsNotice() { diff --git a/web/app/components/app/overview/apikey-info-panel/__tests__/cloud.spec.tsx b/web/app/components/app/overview/apikey-info-panel/__tests__/cloud.spec.tsx index 81ed2b9f286..efe8a459829 100644 --- a/web/app/components/app/overview/apikey-info-panel/__tests__/cloud.spec.tsx +++ b/web/app/components/app/overview/apikey-info-panel/__tests__/cloud.spec.tsx @@ -1,9 +1,8 @@ import { cleanup, screen } from '@testing-library/react' import { clearAllMocks, - defaultModalContext, interactions, - mockUseModalContext, + mockSetSettingsDestination, scenarios, setDeploymentEdition, } from './test-utils' @@ -11,15 +10,9 @@ import { afterEach(cleanup) describe('APIKeyInfoPanel - Cloud Edition', () => { - const setShowAccountSettingModal = vi.fn() - beforeEach(() => { clearAllMocks() setDeploymentEdition('CLOUD') - mockUseModalContext.mockReturnValue({ - ...defaultModalContext, - setShowAccountSettingModal, - }) }) it('hides the panel when an API key already exists', () => { @@ -28,9 +21,9 @@ describe('APIKeyInfoPanel - Cloud Edition', () => { }) it('opens provider settings from the primary action', () => { - scenarios.withMockModal(setShowAccountSettingModal) + scenarios.withAPIKeyNotSet() interactions.clickMainButton() - expect(setShowAccountSettingModal).toHaveBeenCalledWith({ payload: 'provider' }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('provider') }) it('does not show the self-hosted Cloud link', () => { diff --git a/web/app/components/app/overview/apikey-info-panel/__tests__/index.spec.tsx b/web/app/components/app/overview/apikey-info-panel/__tests__/index.spec.tsx index 3ae1a19f0ab..75d6c60cef6 100644 --- a/web/app/components/app/overview/apikey-info-panel/__tests__/index.spec.tsx +++ b/web/app/components/app/overview/apikey-info-panel/__tests__/index.spec.tsx @@ -1,9 +1,8 @@ import { cleanup, screen } from '@testing-library/react' import { clearAllMocks, - defaultModalContext, interactions, - mockUseModalContext, + mockSetSettingsDestination, scenarios, setDeploymentEdition, textKeys, @@ -12,15 +11,9 @@ import { afterEach(cleanup) describe('APIKeyInfoPanel - Community Edition', () => { - const setShowAccountSettingModal = vi.fn() - beforeEach(() => { clearAllMocks() setDeploymentEdition('COMMUNITY') - mockUseModalContext.mockReturnValue({ - ...defaultModalContext, - setShowAccountSettingModal, - }) }) it('hides the panel when an API key already exists', () => { @@ -29,9 +22,9 @@ describe('APIKeyInfoPanel - Community Edition', () => { }) it('opens provider settings from the primary action', () => { - scenarios.withMockModal(setShowAccountSettingModal) + scenarios.withAPIKeyNotSet() interactions.clickMainButton() - expect(setShowAccountSettingModal).toHaveBeenCalledWith({ payload: 'provider' }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('provider') }) it('links self-hosted users to Dify Cloud safely', () => { diff --git a/web/app/components/app/overview/apikey-info-panel/__tests__/test-utils.tsx b/web/app/components/app/overview/apikey-info-panel/__tests__/test-utils.tsx index 2864d2fc0fb..321dbe73ed2 100644 --- a/web/app/components/app/overview/apikey-info-panel/__tests__/test-utils.tsx +++ b/web/app/components/app/overview/apikey-info-panel/__tests__/test-utils.tsx @@ -1,20 +1,16 @@ import type { DeploymentEdition } from '@dify/contracts/api/console/system-features/types.gen' import type { RenderOptions } from '@testing-library/react' -import type { Mock, MockedFunction } from 'vitest' -import type { ModalContextState } from '@/context/modal-context' +import type { MockedFunction } from 'vitest' import { fireEvent, screen } from '@testing-library/react' import { noop } from 'es-toolkit/function' import { defaultPlan } from '@/app/components/billing/config' -import { - useModalContext as actualUseModalContext, - useModalContextSelector as actualUseModalContextSelector, -} from '@/context/modal-context' import { useProviderContext as actualUseProviderContext } from '@/context/provider-context' import { renderWithConsoleQuery } from '@/test/console/query-data' import APIKeyInfoPanel from '../index' -const { mockRouterPush } = vi.hoisted(() => ({ +const { mockRouterPush, mockSetSettingsDestination } = vi.hoisted(() => ({ mockRouterPush: vi.fn(), + mockSetSettingsDestination: vi.fn(), })) // Mock the modules before importing the functions @@ -22,10 +18,13 @@ vi.mock('@/context/provider-context', () => ({ useProviderContext: vi.fn(), })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: vi.fn(), - useModalContextSelector: vi.fn(), -})) +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useQueryState: () => [null, mockSetSettingsDestination], + } +}) vi.mock('@/next/navigation', () => ({ useRouter: () => ({ @@ -37,11 +36,6 @@ vi.mock('@/next/navigation', () => ({ const mockUseProviderContext = actualUseProviderContext as MockedFunction< typeof actualUseProviderContext > -const mockUseModalContext = actualUseModalContext as MockedFunction -const mockUseModalContextSelector = actualUseModalContextSelector as MockedFunction< - typeof actualUseModalContextSelector -> - // Default mock data const defaultProviderContext = { modelProviders: [], @@ -78,25 +72,8 @@ const defaultProviderContext = { humanInputEmailDeliveryEnabled: false, } -const defaultModalContext: ModalContextState = { - hasBlockingModalOpen: false, - setShowAccountSettingModal: noop, - setShowModerationSettingModal: noop, - setShowExternalDataToolModal: noop, - setShowPricingModal: noop, - setShowAnnotationFullModal: noop, - setShowModelModal: noop, - setShowExternalKnowledgeAPIModal: noop, - setShowModelLoadBalancingModal: noop, - setShowOpeningModal: noop, - setShowUpdatePluginModal: noop, - setShowEducationExpireNoticeModal: noop, - setShowTriggerEventsLimitModal: noop, -} - type MockOverrides = { providerContext?: Partial - modalContext?: Partial } type APIKeyInfoPanelRenderOptions = { @@ -112,18 +89,6 @@ function setupMocks(overrides: MockOverrides = {}) { ...defaultProviderContext, ...overrides.providerContext, }) - - mockUseModalContext.mockReturnValue({ - ...defaultModalContext, - ...overrides.modalContext, - }) - - mockUseModalContextSelector.mockImplementation((selector) => - selector({ - ...defaultModalContext, - ...overrides.modalContext, - }), - ) } // Custom render function @@ -157,15 +122,6 @@ export const scenarios = { ...overrides, }, }), - - // Render with mock modal function - withMockModal: (mockSetShowAccountSettingModal: Mock, overrides: MockOverrides = {}) => - renderAPIKeyInfoPanel({ - mockOverrides: { - modalContext: { setShowAccountSettingModal: mockSetShowAccountSettingModal }, - ...overrides, - }, - }), } // Common user interactions @@ -211,4 +167,4 @@ export function setDeploymentEdition(value: DeploymentEdition) { } // Export mock functions for external access -export { defaultModalContext, mockUseModalContext } +export { mockSetSettingsDestination } diff --git a/web/app/components/app/overview/apikey-info-panel/index.tsx b/web/app/components/app/overview/apikey-info-panel/index.tsx index 59b5e3a39b8..f670dd13f73 100644 --- a/web/app/components/app/overview/apikey-info-panel/index.tsx +++ b/web/app/components/app/overview/apikey-info-panel/index.tsx @@ -4,12 +4,15 @@ import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' import { RiCloseLine } from '@remixicon/react' import { useSuspenseQuery } from '@tanstack/react-query' +import { useQueryState } from 'nuqs' import * as React from 'react' import { useState } from 'react' import { useTranslation } from 'react-i18next' import { LinkExternal02 } from '@/app/components/base/icons/src/vender/line/general' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useIntegrationsSetting } from '@/app/components/header/account-setting/use-integrations-setting' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { useProviderContext } from '@/context/provider-context' import { systemFeaturesQueryOptions } from '@/features/system-features/client' @@ -21,7 +24,7 @@ const APIKeyInfoPanel: FC = () => { const isCloud = deploymentEdition === 'CLOUD' const { isAPIKeySet } = useProviderContext() - const openIntegrationsSetting = useIntegrationsSetting() + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const { t } = useTranslation() @@ -67,7 +70,7 @@ const APIKeyInfoPanel: FC = () => { + + ), +})) + +vi.mock('@/app/components/integrations/modal', () => ({ + default: ({ + section, + onCancel, + onSectionChange, + }: { + section: string + onCancel: () => void + onSectionChange: (section: 'data-source') => void + }) => ( + <> +
+ {section} +
+ + + + ), +})) + +function PreferencesOpener() { + const [settingsDestination, setSettingsDestination] = useQueryState( + settingsQueryParamName, + settingsQueryParser, + ) + + return ( + + ) +} + +const renderSettingsModal = (searchParams = '', children?: React.ReactNode) => { + const { wrapper, onUrlUpdate } = createNuqsTestWrapper({ searchParams }) + + return { + ...render( + <> + {children} + + , + { wrapper }, + ), + onUrlUpdate, + } +} + +describe('SettingsModal', () => { + it('opens account settings with push and closes them with replace', async () => { + const user = userEvent.setup() + const { onUrlUpdate } = renderSettingsModal('', ) + + await user.click(screen.getByRole('button', { name: 'open preferences' })) + + expect( + await screen.findByRole('status', { name: 'active account setting tab' }), + ).toHaveTextContent(ACCOUNT_SETTING_TAB.PREFERENCES) + expect(onUrlUpdate.mock.calls.at(-1)?.[0].searchParams.get('settings')).toBe('preferences') + expect(onUrlUpdate.mock.calls.at(-1)?.[0].options).toMatchObject({ + history: 'push', + shallow: false, + }) + + await user.click(screen.getByRole('button', { name: 'cancel account setting' })) + + await waitFor(() => { + expect( + screen.queryByRole('status', { name: 'active account setting tab' }), + ).not.toBeInTheDocument() + }) + expect(onUrlUpdate.mock.calls.at(-1)?.[0].searchParams.has('settings')).toBe(false) + expect(onUrlUpdate.mock.calls.at(-1)?.[0].options).toMatchObject({ + history: 'replace', + shallow: true, + }) + }) + + it('renders an integration destination and replaces it when switching sections', async () => { + const user = userEvent.setup() + const { onUrlUpdate } = renderSettingsModal('?settings=provider') + + expect( + await screen.findByRole('status', { name: 'active integration setting section' }), + ).toHaveTextContent('provider') + + await user.click(screen.getByRole('button', { name: 'switch integration section' })) + + expect(onUrlUpdate.mock.calls.at(-1)?.[0].searchParams.get('settings')).toBe('data-source') + expect(onUrlUpdate.mock.calls.at(-1)?.[0].options).toMatchObject({ + history: 'replace', + shallow: true, + }) + }) + + it('ignores invalid settings destinations', () => { + renderSettingsModal('?settings=unknown') + + expect(screen.queryByRole('status')).not.toBeInTheDocument() + }) +}) diff --git a/web/app/components/header/account-setting/__tests__/update-setting-dialog-form.spec.tsx b/web/app/components/header/account-setting/__tests__/update-setting-dialog-form.spec.tsx index c62c6cf6ae0..9fd44622fe3 100644 --- a/web/app/components/header/account-setting/__tests__/update-setting-dialog-form.spec.tsx +++ b/web/app/components/header/account-setting/__tests__/update-setting-dialog-form.spec.tsx @@ -1,3 +1,4 @@ +import type { SettingsDestination } from '@/app/components/header/account-setting/query-params' import { fireEvent, render, screen } from '@testing-library/react' import * as React from 'react' import { @@ -5,20 +6,17 @@ import { AUTO_UPDATE_STRATEGY, } from '@/app/components/plugins/reference-setting-modal/auto-update-setting/types' import { PluginCategoryEnum } from '@/app/components/plugins/types' -import { ACCOUNT_SETTING_TAB } from '../constants' import UpdateSettingDialogForm from '../update-setting-dialog-form' -const mockSetShowAccountSettingModal = vi.fn() - -vi.mock('@/context/modal-context', () => ({ - useModalContextSelector: ( - selector: (s: { - setShowAccountSettingModal: typeof mockSetShowAccountSettingModal - }) => typeof mockSetShowAccountSettingModal, - ) => { - return selector({ setShowAccountSettingModal: mockSetShowAccountSettingModal }) - }, -})) +const mockSetSettingsDestination = vi.fn() +let mockSettingsDestination: SettingsDestination | null = null +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useQueryState: () => [mockSettingsDestination, mockSetSettingsDestination], + } +}) vi.mock('react-i18next', async () => { const { withSelectorKey, withSelectorKeyProps } = await import('@/test/i18n-mock') @@ -64,6 +62,7 @@ vi.mock( describe('UpdateSettingDialogForm', () => { beforeEach(() => { vi.clearAllMocks() + mockSettingsDestination = null }) it('should open preferences after closing the update setting dialog when timezone link is clicked', () => { @@ -96,8 +95,41 @@ describe('UpdateSettingDialogForm', () => { fireEvent.click(screen.getByText('autoUpdate.changeTimezone')) expect(onRequestClose).toHaveBeenCalledTimes(1) - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.PREFERENCES, + expect(mockSetSettingsDestination).toHaveBeenCalledWith('preferences') + }) + + it('should replace the current destination when timezone link is clicked inside settings', () => { + mockSettingsDestination = 'provider' + + render( + minutes} + onAutoUpgradeChange={vi.fn()} + onPluginsChange={vi.fn()} + onRequestClose={vi.fn()} + onUpdateTimeChange={vi.fn()} + renderTimePickerTrigger={() => } + />, + ) + + fireEvent.click(screen.getByText('autoUpdate.changeTimezone')) + + expect(mockSetSettingsDestination).toHaveBeenCalledWith('preferences', { + history: 'replace', + shallow: true, }) }) }) diff --git a/web/app/components/header/account-setting/__tests__/use-integrations-setting.spec.ts b/web/app/components/header/account-setting/__tests__/use-integrations-setting.spec.ts deleted file mode 100644 index 2e0af718046..00000000000 --- a/web/app/components/header/account-setting/__tests__/use-integrations-setting.spec.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { act, renderHook } from '@testing-library/react' -import { ACCOUNT_SETTING_TAB } from '../constants' -import { useIntegrationsSetting } from '../use-integrations-setting' - -const { mockSetShowAccountSettingModal } = vi.hoisted(() => ({ - mockSetShowAccountSettingModal: vi.fn(), -})) - -vi.mock('@/context/modal-context', () => ({ - useModalContext: () => ({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), -})) - -describe('useIntegrationsSetting', () => { - beforeEach(() => { - vi.clearAllMocks() - }) - - it.each([ - [ACCOUNT_SETTING_TAB.PROVIDER, 'provider'], - [ACCOUNT_SETTING_TAB.DATA_SOURCE, 'data-source'], - [ACCOUNT_SETTING_TAB.API_BASED_EXTENSION, 'custom-endpoint'], - ])('should open integrations settings for migrated tab %s', (tab, section) => { - const { result } = renderHook(() => useIntegrationsSetting()) - - act(() => { - result.current({ payload: tab }) - }) - - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ payload: section }) - }) - - it('should open integrations settings from a direct section', () => { - const { result } = renderHook(() => useIntegrationsSetting()) - - act(() => { - result.current({ section: 'mcp' }) - }) - - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ payload: 'mcp' }) - }) - - it('should preserve the agent source for agent-scoped settings', () => { - const { result } = renderHook(() => useIntegrationsSetting()) - - act(() => { - result.current({ payload: ACCOUNT_SETTING_TAB.PROVIDER, source: 'agent' }) - }) - - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: 'provider', - source: 'agent', - }) - }) - - it('should preserve the cancel callback for migrated integrations settings', () => { - const onCancelCallback = vi.fn() - const { result } = renderHook(() => useIntegrationsSetting()) - - act(() => { - result.current({ payload: ACCOUNT_SETTING_TAB.PROVIDER, onCancelCallback }) - }) - - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: 'provider', - onCancelCallback, - }) - }) -}) diff --git a/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx b/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx index ba06a51d27c..75f0b4267cc 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx @@ -1,8 +1,5 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' -import type { ModalContextState } from '@/context/modal-context' import { fireEvent, render, screen, waitFor } from '@testing-library/react' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useModalContext } from '@/context/modal-context' import { ApiBasedExtensionSelector } from '../selector' const { mockApiBasedExtensionsQuery, mockCreateApiBasedExtension, mockUpdateApiBasedExtension } = @@ -12,9 +9,11 @@ const { mockApiBasedExtensionsQuery, mockCreateApiBasedExtension, mockUpdateApiB mockUpdateApiBasedExtension: vi.fn(), })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: vi.fn(), -})) +const mockSetSettingsDestination = vi.fn() +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) vi.mock('@/context/i18n', () => ({ useDocLink: @@ -55,7 +54,6 @@ vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/bas describe('ApiBasedExtensionSelector', () => { const mockOnChange = vi.fn() - const mockSetShowAccountSettingModal = vi.fn() const mockData: ApiBasedExtensionResponse[] = [ { id: '1', name: 'Extension 1', api_endpoint: 'https://api1.test', api_key: 'key1' }, @@ -64,9 +62,6 @@ describe('ApiBasedExtensionSelector', () => { beforeEach(() => { vi.clearAllMocks() - vi.mocked(useModalContext).mockReturnValue({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - } as unknown as ModalContextState) mockApiBasedExtensionsQuery.mockReturnValue({ data: mockData, isPending: false, @@ -131,9 +126,7 @@ describe('ApiBasedExtensionSelector', () => { fireEvent.click(manageButton) // Assert - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.API_BASED_EXTENSION, - }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('custom-endpoint') }) it('should open add modal when clicking add button and close it after save', async () => { diff --git a/web/app/components/header/account-setting/api-based-extension-page/selector.tsx b/web/app/components/header/account-setting/api-based-extension-page/selector.tsx index fb16e6ef1dc..d36bc90a6d4 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/selector.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/selector.tsx @@ -1,9 +1,12 @@ import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { useQuery } from '@tanstack/react-query' +import { useQueryState } from 'nuqs' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useIntegrationsSetting } from '@/app/components/header/account-setting/use-integrations-setting' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { consoleQuery } from '@/service/client' import { ApiBasedExtensionModal } from './modal' @@ -16,7 +19,7 @@ export function ApiBasedExtensionSelector({ value, onChange }: ApiBasedExtension const { t } = useTranslation() const [open, setOpen] = useState(false) const [addModalOpen, setAddModalOpen] = useState(false) - const openIntegrationsSetting = useIntegrationsSetting() + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const { data: apiBasedExtensions = [] } = useQuery( consoleQuery.apiBasedExtension.get.queryOptions(), ) @@ -84,9 +87,7 @@ export function ApiBasedExtensionSelector({ value, onChange }: ApiBasedExtension className="flex cursor-pointer items-center border-none bg-transparent p-0 text-xs text-text-accent" onClick={() => { setOpen(false) - openIntegrationsSetting({ - payload: ACCOUNT_SETTING_TAB.API_BASED_EXTENSION, - }) + setSettingsDestination('custom-endpoint') }} > {t(($) => $['apiBasedExtension.selector.manage'], { ns: 'common' })} diff --git a/web/app/components/header/account-setting/constants.ts b/web/app/components/header/account-setting/constants.ts index c433f6e0ac1..78e0ef3e95e 100644 --- a/web/app/components/header/account-setting/constants.ts +++ b/web/app/components/header/account-setting/constants.ts @@ -1,69 +1,21 @@ -import type { IntegrationSection } from '@/app/components/integrations/routes' -import { INTEGRATION_SECTION_VALUES } from '@/app/components/integrations/routes' - -export const ACCOUNT_SETTING_MODAL_ACTION = 'showSettings' - export const ACCOUNT_SETTING_TAB = { - PROVIDER: 'provider', MEMBERS: 'members', ROLES_AND_PERMISSIONS: 'roles-and-permissions', PERMISSION_SET: 'permission-set', BILLING: 'billing', WORKFLOW_LOG_ARCHIVES: 'workflow-log-archives', - DATA_SOURCE: 'data-source', - API_BASED_EXTENSION: 'custom-endpoint', CUSTOM: 'custom', PREFERENCES: 'preferences', - LANGUAGE: 'language', } as const export type AccountSettingTab = (typeof ACCOUNT_SETTING_TAB)[keyof typeof ACCOUNT_SETTING_TAB] -export const DEFAULT_ACCOUNT_SETTING_TAB = ACCOUNT_SETTING_TAB.MEMBERS - -const WORKSPACE_SETTING_TAB_VALUES = [ +export const ACCOUNT_SETTING_TAB_VALUES = [ ACCOUNT_SETTING_TAB.MEMBERS, ACCOUNT_SETTING_TAB.ROLES_AND_PERMISSIONS, ACCOUNT_SETTING_TAB.PERMISSION_SET, ACCOUNT_SETTING_TAB.BILLING, ACCOUNT_SETTING_TAB.WORKFLOW_LOG_ARCHIVES, ACCOUNT_SETTING_TAB.CUSTOM, -] as const - -export type WorkspaceSettingTab = (typeof WORKSPACE_SETTING_TAB_VALUES)[number] - -const USER_SETTING_TAB_VALUES = [ ACCOUNT_SETTING_TAB.PREFERENCES, - ACCOUNT_SETTING_TAB.LANGUAGE, ] as const - -export type UserSettingTab = (typeof USER_SETTING_TAB_VALUES)[number] - -export type IntegrationSettingTab = IntegrationSection - -export const SETTINGS_TAB_VALUES = [ - ...WORKSPACE_SETTING_TAB_VALUES, - ...USER_SETTING_TAB_VALUES, - ...INTEGRATION_SECTION_VALUES, -] as const - -export type SettingsTab = (typeof SETTINGS_TAB_VALUES)[number] -export const isValidSettingsTab = (tab: string | null): tab is SettingsTab => { - if (!tab) return false - return SETTINGS_TAB_VALUES.includes(tab as SettingsTab) -} - -export const isWorkspaceSettingTab = (tab: SettingsTab | null): tab is WorkspaceSettingTab => { - if (!tab) return false - return WORKSPACE_SETTING_TAB_VALUES.includes(tab as WorkspaceSettingTab) -} - -export const isUserSettingTab = (tab: SettingsTab | null): tab is UserSettingTab => { - if (!tab) return false - return USER_SETTING_TAB_VALUES.includes(tab as UserSettingTab) -} - -export const isIntegrationSettingTab = (tab: SettingsTab | null): tab is IntegrationSettingTab => { - if (!tab) return false - return INTEGRATION_SECTION_VALUES.includes(tab as IntegrationSection) -} diff --git a/web/app/components/header/account-setting/destinations.ts b/web/app/components/header/account-setting/destinations.ts deleted file mode 100644 index 6921fa81a32..00000000000 --- a/web/app/components/header/account-setting/destinations.ts +++ /dev/null @@ -1,11 +0,0 @@ -import type { AccountSettingTab } from './constants' -import type { IntegrationSection } from '@/app/components/integrations/routes' -import { ACCOUNT_SETTING_TAB } from './constants' - -export const integrationSectionByMovedAccountSettingTab = { - [ACCOUNT_SETTING_TAB.PROVIDER]: 'provider', - [ACCOUNT_SETTING_TAB.DATA_SOURCE]: 'data-source', - [ACCOUNT_SETTING_TAB.API_BASED_EXTENSION]: 'custom-endpoint', -} as const satisfies Partial> - -export type MovedAccountSettingTab = keyof typeof integrationSectionByMovedAccountSettingTab diff --git a/web/app/components/header/account-setting/index.tsx b/web/app/components/header/account-setting/index.tsx index 7749686568e..dbd1ec3bd83 100644 --- a/web/app/components/header/account-setting/index.tsx +++ b/web/app/components/header/account-setting/index.tsx @@ -5,7 +5,7 @@ import { cn } from '@langgenius/dify-ui/cn' import { ScrollArea } from '@langgenius/dify-ui/scroll-area' import { useSuspenseQuery } from '@tanstack/react-query' import { useAtomValue } from 'jotai' -import { useCallback, useRef, useState } from 'react' +import { useRef } from 'react' import { useTranslation } from 'react-i18next' import BillingPage from '@/app/components/billing/billing-page' import CustomPage from '@/app/components/custom/custom-page' @@ -21,11 +21,7 @@ import { systemFeaturesQueryOptions } from '@/features/system-features/client' import useBreakpoints, { MediaType } from '@/hooks/use-breakpoints' import { hasPermission } from '@/utils/permission' import AccessRulesPage from './access-rules-page' -import { ApiBasedExtensionPage } from './api-based-extension-page' -import DataSourcePage from './data-source-page-new' import MembersPage from './members-page' -import ModelProviderPage from './model-provider-page' -import { useResetModelProviderListExpanded } from './model-provider-page/atoms' import PermissionsPage from './permissions-page' import PreferencePage from './preference-page' import WorkflowLogArchivesPage from './workflow-log-archives-page' @@ -54,7 +50,6 @@ export default function AccountSetting({ activeTab, onTabChangeAction, }: IAccountSettingProps) { - const resetModelProviderListExpanded = useResetModelProviderListExpanded() const { t } = useTranslation() const { enableBilling, enableReplaceWebAppLogo } = useProviderContext() const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) @@ -67,34 +62,22 @@ export default function AccountSetting({ const canViewBilling = enableBilling && !isCurrentWorkspaceDatasetOperator const canViewWorkflowLogArchives = systemFeatures.deployment_edition === 'CLOUD' && isCurrentWorkspaceManager - // Keep legacy `language` deep links opening Preferences during the tab rename migration. - const normalizedActiveTab = - activeTab === ACCOUNT_SETTING_TAB.LANGUAGE ? ACCOUNT_SETTING_TAB.PREFERENCES : activeTab const activeMenu = (() => { - if (normalizedActiveTab === ACCOUNT_SETTING_TAB.BILLING && !canViewBilling) + if (activeTab === ACCOUNT_SETTING_TAB.BILLING && !canViewBilling) return ACCOUNT_SETTING_TAB.PREFERENCES - if ( - normalizedActiveTab === ACCOUNT_SETTING_TAB.WORKFLOW_LOG_ARCHIVES && - !canViewWorkflowLogArchives - ) + if (activeTab === ACCOUNT_SETTING_TAB.WORKFLOW_LOG_ARCHIVES && !canViewWorkflowLogArchives) return ACCOUNT_SETTING_TAB.MEMBERS if ( - (normalizedActiveTab === ACCOUNT_SETTING_TAB.ROLES_AND_PERMISSIONS || - normalizedActiveTab === ACCOUNT_SETTING_TAB.PERMISSION_SET) && + (activeTab === ACCOUNT_SETTING_TAB.ROLES_AND_PERMISSIONS || + activeTab === ACCOUNT_SETTING_TAB.PERMISSION_SET) && !canManageWorkspaceRoles ) return ACCOUNT_SETTING_TAB.MEMBERS - return normalizedActiveTab + return activeTab })() const scrollContainerRef = useRef(null) const settingItems: GroupItem[] = [ - { - key: ACCOUNT_SETTING_TAB.PROVIDER, - name: t(($) => $['settings.provider'], { ns: 'common' }), - icon: , - activeIcon: , - }, { key: ACCOUNT_SETTING_TAB.MEMBERS, name: t(($) => $['settings.members'], { ns: 'common' }), @@ -128,18 +111,6 @@ export default function AccountSetting({ icon: , activeIcon: , }, - { - key: ACCOUNT_SETTING_TAB.DATA_SOURCE, - name: t(($) => $['settings.dataSource'], { ns: 'common' }), - icon: , - activeIcon: , - }, - { - key: ACCOUNT_SETTING_TAB.API_BASED_EXTENSION, - name: t(($) => $['settings.customEndpoint'], { ns: 'common' }), - icon: , - activeIcon: , - }, { key: ACCOUNT_SETTING_TAB.CUSTOM, name: t(($) => $.custom, { ns: 'custom' }), @@ -193,31 +164,15 @@ export default function AccountSetting({ }, ] - const [searchValue, setSearchValue] = useState('') - - const handleTabChange = useCallback( - (tab: AccountSettingTab) => { - if (tab === ACCOUNT_SETTING_TAB.PROVIDER) resetModelProviderListExpanded() - - onTabChangeAction(tab) - }, - [onTabChangeAction, resetModelProviderListExpanded], - ) - - const handleClose = useCallback(() => { - resetModelProviderListExpanded() - onCancelAction() - }, [onCancelAction, resetModelProviderListExpanded]) - return ( - +
@@ -257,7 +212,7 @@ export default function AccountSetting({ aria-label={item.name} title={item.name} onClick={() => { - handleTabChange(item.key) + onTabChangeAction(item.key) }} > {activeMenu === item.key ? item.activeIcon : item.icon} @@ -289,9 +244,6 @@ export default function AccountSetting({
- {activeMenu === ACCOUNT_SETTING_TAB.PROVIDER && ( - - )} {activeMenu === ACCOUNT_SETTING_TAB.MEMBERS && } {activeMenu === ACCOUNT_SETTING_TAB.ROLES_AND_PERMISSIONS && ( @@ -301,8 +253,6 @@ export default function AccountSetting({ {activeMenu === ACCOUNT_SETTING_TAB.WORKFLOW_LOG_ARCHIVES && ( )} - {activeMenu === ACCOUNT_SETTING_TAB.DATA_SOURCE && } - {activeMenu === ACCOUNT_SETTING_TAB.API_BASED_EXTENSION && } {activeMenu === ACCOUNT_SETTING_TAB.CUSTOM && } {activeMenu === ACCOUNT_SETTING_TAB.PREFERENCES && }
diff --git a/web/app/components/header/account-setting/model-provider-page/__tests__/atoms.spec.tsx b/web/app/components/header/account-setting/model-provider-page/__tests__/atoms.spec.tsx index 890aeade4c8..b1ae492cbb6 100644 --- a/web/app/components/header/account-setting/model-provider-page/__tests__/atoms.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/__tests__/atoms.spec.tsx @@ -5,7 +5,6 @@ import { beforeEach, describe, expect, it } from 'vitest' import { useExpandModelProviderList, useModelProviderListExpanded, - useResetModelProviderListExpanded, useSetModelProviderListExpanded, } from '../atoms' @@ -169,73 +168,6 @@ describe('atoms', () => { }) }) - // Reset hook: clears all expanded state back to empty - describe('useResetModelProviderListExpanded', () => { - it('should reset all expanded providers to false', () => { - const { result } = renderHook( - () => ({ - openaiExpanded: useModelProviderListExpanded('openai'), - anthropicExpanded: useModelProviderListExpanded('anthropic'), - expand: useExpandModelProviderList(), - reset: useResetModelProviderListExpanded(), - }), - { wrapper }, - ) - - act(() => { - result.current.expand('openai') - }) - act(() => { - result.current.expand('anthropic') - }) - act(() => { - result.current.reset() - }) - - expect(result.current.openaiExpanded).toBe(false) - expect(result.current.anthropicExpanded).toBe(false) - }) - - it('should be safe to call when no providers are expanded', () => { - const { result } = renderHook( - () => ({ - expanded: useModelProviderListExpanded('openai'), - reset: useResetModelProviderListExpanded(), - }), - { wrapper }, - ) - - act(() => { - result.current.reset() - }) - - expect(result.current.expanded).toBe(false) - }) - - it('should allow re-expanding providers after reset', () => { - const { result } = renderHook( - () => ({ - expanded: useModelProviderListExpanded('openai'), - expand: useExpandModelProviderList(), - reset: useResetModelProviderListExpanded(), - }), - { wrapper }, - ) - - act(() => { - result.current.expand('openai') - }) - act(() => { - result.current.reset() - }) - act(() => { - result.current.expand('openai') - }) - - expect(result.current.expanded).toBe(true) - }) - }) - // Cross-hook interaction: verify hooks cooperate through the shared atom describe('Cross-hook interaction', () => { it('should reflect state set by useSetModelProviderListExpanded in useModelProviderListExpanded', () => { @@ -290,26 +222,6 @@ describe('atoms', () => { }) expect(result.current.expanded).toBe(false) }) - - it('should reset state set by useSetModelProviderListExpanded via useResetModelProviderListExpanded', () => { - const { result } = renderHook( - () => ({ - expanded: useModelProviderListExpanded('openai'), - setExpanded: useSetModelProviderListExpanded('openai'), - reset: useResetModelProviderListExpanded(), - }), - { wrapper }, - ) - - act(() => { - result.current.setExpanded(true) - }) - act(() => { - result.current.reset() - }) - - expect(result.current.expanded).toBe(false) - }) }) // selectAtom granularity: changing one provider should not affect unrelated reads diff --git a/web/app/components/header/account-setting/model-provider-page/__tests__/index.spec.tsx b/web/app/components/header/account-setting/model-provider-page/__tests__/index.spec.tsx index 6a20a6a08b1..5f0fd4805ef 100644 --- a/web/app/components/header/account-setting/model-provider-page/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/model-provider-page/__tests__/index.spec.tsx @@ -26,8 +26,8 @@ type MockReferenceSetting = { } } -const { mockSetAccountSettingModal, mockSaveAutoUpgrade } = vi.hoisted(() => ({ - mockSetAccountSettingModal: vi.fn(), +const { mockSetSettingsDestination, mockSaveAutoUpgrade } = vi.hoisted(() => ({ + mockSetSettingsDestination: vi.fn(), mockSaveAutoUpgrade: vi.fn(), })) @@ -289,11 +289,10 @@ vi.mock('@langgenius/dify-ui/dialog', () => ({ DialogCloseButton: () => diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx index 0c53e7aa7cb..ea81677eb7b 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/__tests__/index.spec.tsx @@ -8,7 +8,6 @@ import timezone from 'dayjs/plugin/timezone' import utc from 'dayjs/plugin/utc' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { createAccountProfileQueryClient } from '@/test/console/account-profile' import { PluginCategoryEnum, PluginSource } from '../../../types' import AutoUpdateSetting from '../index' @@ -34,17 +33,11 @@ dayjs.extend(timezone) // Mock app context const mockTimezone = 'America/New_York' -// Mock modal context -const mockSetShowAccountSettingModal = vi.fn() -vi.mock('@/context/modal-context', () => ({ - useModalContextSelector: ( - selector: (s: { - setShowAccountSettingModal: typeof mockSetShowAccountSettingModal - }) => typeof mockSetShowAccountSettingModal, - ) => { - return selector({ setShowAccountSettingModal: mockSetShowAccountSettingModal }) - }, -})) +const mockSetSettingsDestination = vi.fn() +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) // Mock i18n context @@ -1396,9 +1389,7 @@ describe('auto-update-setting', () => { fireEvent.click(screen.getByText('autoUpdate.changeTimezone')) // Assert - expect(mockSetShowAccountSettingModal).toHaveBeenCalledWith({ - payload: ACCOUNT_SETTING_TAB.PREFERENCES, - }) + expect(mockSetSettingsDestination).toHaveBeenCalledWith('preferences') }) }) diff --git a/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx b/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx index c974a1f7505..6197cbd320c 100644 --- a/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx +++ b/web/app/components/plugins/reference-setting-modal/auto-update-setting/index.tsx @@ -6,13 +6,16 @@ import { cn } from '@langgenius/dify-ui/cn' import { SegmentedControl, SegmentedControlItem } from '@langgenius/dify-ui/segmented-control' import { RiTimeLine } from '@remixicon/react' import { useQuery } from '@tanstack/react-query' +import { useQueryState } from 'nuqs' import * as React from 'react' import { useCallback, useMemo } from 'react' import { Trans, useTranslation } from 'react-i18next' import TimePicker from '@/app/components/base/date-and-time-picker/time-picker' import { convertTimezoneToOffsetStr } from '@/app/components/base/date-and-time-picker/utils/dayjs' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' -import { useModalContextSelector } from '@/context/modal-context' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { userProfileQueryOptions } from '@/features/account-profile/client' import Label from '../label' import PluginsPicker from './plugins-picker' @@ -35,12 +38,12 @@ type Props = Readonly<{ const SettingTimeZone: FC<{ children?: React.ReactNode }> = ({ children }) => { - const setShowAccountSettingModal = useModalContextSelector((s) => s.setShowAccountSettingModal) + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) return ( diff --git a/web/app/components/step-by-step-tour/__tests__/mount.spec.tsx b/web/app/components/step-by-step-tour/__tests__/mount.spec.tsx index e099f13a36f..f8b9624db1f 100644 --- a/web/app/components/step-by-step-tour/__tests__/mount.spec.tsx +++ b/web/app/components/step-by-step-tour/__tests__/mount.spec.tsx @@ -14,6 +14,7 @@ import { Plan } from '@/app/components/billing/type' import { systemFeaturesQueryOptions } from '@/features/system-features/client' import { seedRegisteredConsoleStateFixture } from '@/test/console/state-fixture' import { createSystemFeaturesFixture } from '@/test/console/system-features' +import { createNuqsTestWrapper } from '@/test/nuqs-testing' import { createTestQueryClient } from '@/test/query-client' import StepByStepTourMount from '../mount' import { stepByStepTourSessionAtom } from '../state' @@ -64,6 +65,9 @@ const mockEnableStepByStepTour = vi.hoisted(() => ({ const mockHasBlockingModalOpen = vi.hoisted(() => ({ value: false, })) +const mockEducationExpireNotice = vi.hoisted(() => ({ + value: false, +})) const mockStepByStepTour = vi.hoisted(() => { const stateQueryKey = ['console', 'onboarding', 'step-by-step-tour', 'state'] as const const createState = ( @@ -210,6 +214,15 @@ vi.mock('@/context/modal-context', () => ({ }), })) +vi.mock('@/app/education-apply/use-expire-notice', () => ({ + useEducationExpireNotice: () => [ + mockEducationExpireNotice.value + ? { accountId: 'user-1', expireAt: 1, expired: false, phase: 'expiring' } + : null, + vi.fn(), + ], +})) + vi.mock('@/next/navigation', () => ({ usePathname: () => mockPathname, useRouter: () => ({ push: mockRouterPush }), @@ -480,7 +493,7 @@ const setStepByStepTourTestState = (state: Partial) } } -const renderStepByStepTourMount = () => { +const renderStepByStepTourMount = (searchParams = '') => { const queryClient = createTestQueryClient() queryClient.setQueryData(mockStepByStepTour.stateQueryKey, mockStepByStepTour.state) queryClient.setQueryData( @@ -495,6 +508,7 @@ const renderStepByStepTourMount = () => { seedRegisteredConsoleStateFixture(jotaiStore) jotaiStore.set(queryClientAtom, queryClient) jotaiStore.set(stepByStepTourSessionAtom, mockStepByStepTour.uiState) + const { wrapper } = createNuqsTestWrapper({ searchParams }) return render( @@ -502,6 +516,7 @@ const renderStepByStepTourMount = () => { , + { wrapper }, ) } @@ -532,6 +547,7 @@ describe('StepByStepTourMount', () => { mockEnableLearnApp.value = true mockEnableStepByStepTour.value = true mockHasBlockingModalOpen.value = false + mockEducationExpireNotice.value = false mockPathname = '/apps' localStorage.clear() mockStepByStepTour.reset() @@ -782,6 +798,57 @@ describe('StepByStepTourMount', () => { expect(document.body.querySelector('[data-base-ui-portal]')).not.toBeInTheDocument() }) + it('hides expanded tour overlays while settings is open', async () => { + setStepByStepTourTestState({ + manuallyEnabledWorkspaceIds: ['workspace-1'], + manuallyDisabledWorkspaceIds: [], + minimized: false, + completedTaskIds: [], + skipped: false, + }) + + renderStepByStepTourMount('?settings=preferences') + + await waitFor(() => { + expect(screen.queryByRole('region', { name: 'Get to know Dify' })).not.toBeInTheDocument() + }) + expect(document.body.querySelector('[data-base-ui-portal]')).not.toBeInTheDocument() + }) + + it('hides expanded tour overlays while the Education expiration notice is open', async () => { + setStepByStepTourTestState({ + manuallyEnabledWorkspaceIds: ['workspace-1'], + manuallyDisabledWorkspaceIds: [], + minimized: false, + completedTaskIds: [], + skipped: false, + }) + + mockEducationExpireNotice.value = true + renderStepByStepTourMount() + + await waitFor(() => { + expect(screen.queryByRole('region', { name: 'Get to know Dify' })).not.toBeInTheDocument() + }) + expect(document.body.querySelector('[data-base-ui-portal]')).not.toBeInTheDocument() + }) + + it('does not block the tour outside the Apps route for an unmounted Education notice', async () => { + mockPathname = '/datasets' + mockEducationExpireNotice.value = true + setStepByStepTourTestState({ + manuallyEnabledWorkspaceIds: ['workspace-1'], + manuallyDisabledWorkspaceIds: [], + minimized: false, + completedTaskIds: [], + skipped: false, + }) + + renderStepByStepTourMount() + + expect(await screen.findByRole('region', { name: 'Get to know Dify' })).toBeInTheDocument() + }) + it('keeps the minimized tour entry available while a blocking modal is open', async () => { mockHasBlockingModalOpen.value = true localStorage.setItem(STEP_BY_STEP_TOUR_SHELL_MODE_STORAGE_KEY, 'collapsed') diff --git a/web/app/components/step-by-step-tour/mount.tsx b/web/app/components/step-by-step-tour/mount.tsx index b50ea058f9d..e173c1e7997 100644 --- a/web/app/components/step-by-step-tour/mount.tsx +++ b/web/app/components/step-by-step-tour/mount.tsx @@ -11,9 +11,15 @@ import { cn } from '@langgenius/dify-ui/cn' import { Popover, PopoverContent } from '@langgenius/dify-ui/popover' import { useSuspenseQuery } from '@tanstack/react-query' import { useAtomValue, useSetAtom } from 'jotai' +import { useQueryState } from 'nuqs' import { useEffect, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { buildIntegrationPath } from '@/app/components/integrations/routes' +import { useEducationExpireNotice } from '@/app/education-apply/use-expire-notice' import { useDocLink } from '@/context/i18n' import { useModalContextSelector } from '@/context/modal-context' import { workspacePermissionKeysAtom } from '@/context/permission-state' @@ -130,6 +136,8 @@ export default function StepByStepTourMount({ className }: StepByStepTourMountPr const isCurrentWorkspaceManager = useAtomValue(isCurrentWorkspaceManagerAtom) const workspacePermissionKeys = useAtomValue(workspacePermissionKeysAtom) const hasBlockingModalOpen = useModalContextSelector((state) => state.hasBlockingModalOpen) + const [educationExpireNotice] = useEducationExpireNotice() + const [settingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const { data: systemFeatures } = useSuspenseQuery(systemFeaturesQueryOptions()) const completedTaskIds = useAtomValue(completedStepByStepTourTaskIdsAtom) const skipped = useAtomValue(stepByStepTourSkippedAtom) @@ -232,7 +240,11 @@ export default function StepByStepTourMount({ className }: StepByStepTourMountPr stepByStepTourFeatureEnabled && enabledForCurrentWorkspace && (hasActiveGuide || !shouldHideOnPathname(pathname)) - const overlayVisible = visible && !hasBlockingModalOpen + const overlayVisible = + visible && + !hasBlockingModalOpen && + !settingsDestination && + !(pathname === '/apps' && educationExpireNotice) const completionPromptVisible = visible && allTasksCompleted && !activeTask const checklistMinimized = completionPromptVisible ? false : minimized const expanded = !checklistMinimized diff --git a/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx b/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx index 7523ef42b7c..39505b3b2c0 100644 --- a/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx +++ b/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx @@ -22,11 +22,9 @@ vi.mock('@/service/tools', () => ({ const parseParamsSchemaMock = vi.mocked(parseParamsSchema) const mockSetShowPricingModal = vi.fn() -const mockSetShowAccountSettingModal = vi.fn() vi.mock('@/context/modal-context', () => ({ useModalContext: (): ModalContextState => ({ hasBlockingModalOpen: false, - setShowAccountSettingModal: mockSetShowAccountSettingModal, setShowModerationSettingModal: vi.fn(), setShowExternalDataToolModal: vi.fn(), setShowPricingModal: mockSetShowPricingModal, @@ -36,7 +34,6 @@ vi.mock('@/context/modal-context', () => ({ setShowModelLoadBalancingModal: vi.fn(), setShowOpeningModal: vi.fn(), setShowUpdatePluginModal: vi.fn(), - setShowEducationExpireNoticeModal: vi.fn(), setShowTriggerEventsLimitModal: vi.fn(), }), })) diff --git a/web/app/components/workflow-app/hooks/__tests__/use-workflow-run-callbacks.spec.ts b/web/app/components/workflow-app/hooks/__tests__/use-workflow-run-callbacks.spec.ts index d75a54101f7..eff299a56c2 100644 --- a/web/app/components/workflow-app/hooks/__tests__/use-workflow-run-callbacks.spec.ts +++ b/web/app/components/workflow-app/hooks/__tests__/use-workflow-run-callbacks.spec.ts @@ -124,13 +124,15 @@ describe('useWorkflowRun callbacks helpers', () => { expect(fetchInspectVars).toHaveBeenCalledWith({}) expect(invalidAllLastRun).toHaveBeenCalled() - callbacks.onError?.({ error: 'failed', node_type: 'llm' } as never) + callbacks.onError?.('LLM provider and model are required.') expect(clearAbortController).toHaveBeenCalled() - expect(handlers.handleWorkflowFailed).toHaveBeenCalled() + expect(handlers.handleWorkflowFailed).toHaveBeenCalledWith( + 'LLM provider and model are required.', + ) expect(userOnError).toHaveBeenCalled() expect(getWorkflowRunningData).toHaveBeenCalled() expect(trackWorkflowRunFailed).toHaveBeenCalledWith( - { error: 'failed', node_type: 'llm' }, + 'LLM provider and model are required.', workflowData, ) diff --git a/web/app/components/workflow-app/hooks/use-workflow-run-callbacks.ts b/web/app/components/workflow-app/hooks/use-workflow-run-callbacks.ts index 30411c04829..c0f7524a8c9 100644 --- a/web/app/components/workflow-app/hooks/use-workflow-run-callbacks.ts +++ b/web/app/components/workflow-app/hooks/use-workflow-run-callbacks.ts @@ -11,7 +11,7 @@ type ContainerSize = { type WorkflowRunEventHandlers = { handleWorkflowStarted: NonNullable handleWorkflowFinished: NonNullable - handleWorkflowFailed: () => void + handleWorkflowFailed: (error?: string) => void handleWorkflowNodeStarted: ( params: Parameters>[0], containerParams: ContainerSize, @@ -150,7 +150,7 @@ export const createBaseWorkflowRunCallbacks = ({ const wrappedOnError: IOtherOptions['onError'] = (params, code) => { clearAbortController() - handleWorkflowFailed() + handleWorkflowFailed(params) const workflowData = getWorkflowRunningData() invalidateRunHistory(runHistoryUrl) clearListeningState() @@ -358,7 +358,7 @@ export const createFinalWorkflowRunCallbacks = ({ }, onError: (params, code) => { clearAbortController() - handleWorkflowFailed() + handleWorkflowFailed(params) const workflowData = getWorkflowRunningData() invalidateRunHistory(runHistoryUrl) clearListeningState() diff --git a/web/app/components/workflow/hooks/use-workflow-run-event/__tests__/use-workflow-failed.spec.ts b/web/app/components/workflow/hooks/use-workflow-run-event/__tests__/use-workflow-failed.spec.ts index b381801aabd..f54e9c4bb27 100644 --- a/web/app/components/workflow/hooks/use-workflow-run-event/__tests__/use-workflow-failed.spec.ts +++ b/web/app/components/workflow/hooks/use-workflow-run-event/__tests__/use-workflow-failed.spec.ts @@ -66,10 +66,13 @@ describe('useWorkflowFailed', () => { }) act(() => { - result.current.handleWorkflowFailed() + result.current.handleWorkflowFailed('LLM provider and model are required.') }) expect(store.getState().workflowRunningData!.result.status).toBe(WorkflowRunningStatus.Failed) + expect(store.getState().workflowRunningData!.result.error).toBe( + 'LLM provider and model are required.', + ) expect(store.getState().workflowRunningData!.tracing!.map((trace) => trace.status)).toEqual([ NodeRunningStatus.Failed, NodeRunningStatus.Succeeded, @@ -115,4 +118,44 @@ describe('useWorkflowFailed', () => { expect(store.getState().workflowRunningData!.result.status).toBe(WorkflowRunningStatus.Stopped) }) + + it('adds a late error message to an already failed workflow', () => { + const { result, store } = renderRunEventHook(() => useWorkflowFailed(), { + initialStoreState: { + workflowRunningData: baseRunningData({ + result: { status: WorkflowRunningStatus.Failed }, + }), + }, + }) + + act(() => { + result.current.handleWorkflowFailed('LLM provider and model are required.') + }) + + expect(store.getState().workflowRunningData!.result).toMatchObject({ + status: WorkflowRunningStatus.Failed, + error: 'LLM provider and model are required.', + }) + }) + + it('keeps the workflow_finished error when a generic error arrives later', () => { + const { result, store } = renderRunEventHook(() => useWorkflowFailed(), { + initialStoreState: { + workflowRunningData: baseRunningData({ + result: { + status: WorkflowRunningStatus.Failed, + error: 'LLM provider and model are required.', + }, + }), + }, + }) + + act(() => { + result.current.handleWorkflowFailed('Server Error') + }) + + expect(store.getState().workflowRunningData!.result.error).toBe( + 'LLM provider and model are required.', + ) + }) }) diff --git a/web/app/components/workflow/hooks/use-workflow-run-event/use-workflow-failed.ts b/web/app/components/workflow/hooks/use-workflow-run-event/use-workflow-failed.ts index b50663dc214..3a56ed87bcd 100644 --- a/web/app/components/workflow/hooks/use-workflow-run-event/use-workflow-failed.ts +++ b/web/app/components/workflow/hooks/use-workflow-run-event/use-workflow-failed.ts @@ -8,52 +8,66 @@ export const useWorkflowFailed = () => { const store = useStoreApi() const workflowStore = useWorkflowStore() - const handleWorkflowFailed = useCallback(() => { - const { workflowRunningData, setWorkflowRunningData } = workflowStore.getState() - if (!workflowRunningData) return - if ( - workflowRunningData.result.status === WorkflowRunningStatus.Succeeded || - workflowRunningData.result.status === WorkflowRunningStatus.Failed || - workflowRunningData.result.status === WorkflowRunningStatus.Stopped - ) - return + const handleWorkflowFailed = useCallback( + (error?: string) => { + const { workflowRunningData, setWorkflowRunningData } = workflowStore.getState() + if (!workflowRunningData) return + if ( + workflowRunningData.result.status === WorkflowRunningStatus.Succeeded || + workflowRunningData.result.status === WorkflowRunningStatus.Stopped + ) + return - const { getNodes, setNodes, edges, setEdges } = store.getState() + if (workflowRunningData.result.status === WorkflowRunningStatus.Failed) { + if (!error || workflowRunningData.result.error) return - setWorkflowRunningData( - produce(workflowRunningData, (draft) => { - draft.result = { - ...draft.result, - status: WorkflowRunningStatus.Failed, - } - draft.tracing?.forEach((trace) => { - if (trace.status === NodeRunningStatus.Running) trace.status = NodeRunningStatus.Failed - }) - }), - ) + setWorkflowRunningData( + produce(workflowRunningData, (draft) => { + draft.result.error = error + }), + ) + return + } - setNodes( - produce(getNodes(), (draft) => { - draft.forEach((node) => { - if (node.data._runningStatus === NodeRunningStatus.Running) - node.data._runningStatus = NodeRunningStatus.Failed - node.data._waitingRun = false - }) - }), - ) - setEdges( - produce(edges, (draft) => { - draft.forEach((edge) => { - if (!edge.data) return - if (edge.data._sourceRunningStatus === NodeRunningStatus.Running) - edge.data._sourceRunningStatus = NodeRunningStatus.Failed - if (edge.data._targetRunningStatus === NodeRunningStatus.Running) - edge.data._targetRunningStatus = NodeRunningStatus.Failed - edge.data._waitingRun = false - }) - }), - ) - }, [store, workflowStore]) + const { getNodes, setNodes, edges, setEdges } = store.getState() + + setWorkflowRunningData( + produce(workflowRunningData, (draft) => { + draft.result = { + ...draft.result, + status: WorkflowRunningStatus.Failed, + error, + } + draft.tracing?.forEach((trace) => { + if (trace.status === NodeRunningStatus.Running) trace.status = NodeRunningStatus.Failed + }) + }), + ) + + setNodes( + produce(getNodes(), (draft) => { + draft.forEach((node) => { + if (node.data._runningStatus === NodeRunningStatus.Running) + node.data._runningStatus = NodeRunningStatus.Failed + node.data._waitingRun = false + }) + }), + ) + setEdges( + produce(edges, (draft) => { + draft.forEach((edge) => { + if (!edge.data) return + if (edge.data._sourceRunningStatus === NodeRunningStatus.Running) + edge.data._sourceRunningStatus = NodeRunningStatus.Failed + if (edge.data._targetRunningStatus === NodeRunningStatus.Running) + edge.data._targetRunningStatus = NodeRunningStatus.Failed + edge.data._waitingRun = false + }) + }), + ) + }, + [store, workflowStore], + ) return { handleWorkflowFailed, diff --git a/web/app/components/workflow/nodes/_base/components/__tests__/field.spec.tsx b/web/app/components/workflow/nodes/_base/components/__tests__/field.spec.tsx index a40d03603de..94af7cf6a39 100644 --- a/web/app/components/workflow/nodes/_base/components/__tests__/field.spec.tsx +++ b/web/app/components/workflow/nodes/_base/components/__tests__/field.spec.tsx @@ -2,28 +2,8 @@ import { fireEvent, render, screen } from '@testing-library/react' import Field from '../field' describe('Field', () => { - it('should render subtitle styling, tooltip, operations, warning dot and required marker', () => { - const { container } = render( - operation} - required - warningDot - isSubTitle - />, - ) - - expect(screen.getByText('Knowledge')).toBeInTheDocument() - expect(screen.getByLabelText('tooltip text')).toBeInTheDocument() - expect(screen.getByRole('button', { name: 'operation' })).toBeInTheDocument() - expect(screen.getByText('*')).toBeInTheDocument() - expect(container.querySelector('.system-xs-medium-uppercase')).not.toBeNull() - expect(container.querySelector('.bg-text-warning-secondary')).not.toBeNull() - }) - it('should toggle folded children when supportFold is enabled', () => { - const { container } = render( + render(
folded content
, @@ -33,20 +13,8 @@ describe('Field', () => { fireEvent.click(screen.getByText('Foldable').closest('.cursor-pointer')!) expect(screen.getByText('folded content')).toBeInTheDocument() - expect(container.querySelector('svg')).toHaveStyle({ transform: 'rotate(0deg)' }) fireEvent.click(screen.getByText('Foldable').closest('.cursor-pointer')!) expect(screen.queryByText('folded content')).not.toBeInTheDocument() }) - - it('should render inline children without folding support', () => { - const { container } = render( - -
always visible
- , - ) - - expect(screen.getByText('always visible')).toBeInTheDocument() - expect(container.firstChild).toHaveClass('flex') - }) }) diff --git a/web/app/components/workflow/nodes/_base/components/code-generator-button.tsx b/web/app/components/workflow/nodes/_base/components/code-generator-button.tsx index e7bda2fc5da..1a935d2242e 100644 --- a/web/app/components/workflow/nodes/_base/components/code-generator-button.tsx +++ b/web/app/components/workflow/nodes/_base/components/code-generator-button.tsx @@ -3,9 +3,8 @@ import type { FC } from 'react' import type { CodeLanguage } from '../../code/types' import type { GenRes } from '@/service/debug' import { cn } from '@langgenius/dify-ui/cn' -import { useBoolean } from 'ahooks' import * as React from 'react' -import { useCallback } from 'react' +import { useCallback, useState } from 'react' import { GetCodeGeneratorResModal } from '@/app/components/app/configuration/config/code-generator/get-code-generator-res' import { ActionButton } from '@/app/components/base/action-button' import { Generator } from '@/app/components/base/icons/src/vender/other' @@ -27,20 +26,19 @@ const CodeGenerateBtn: FC = ({ codeLanguages, onGenerated, }) => { - const [showAutomatic, { setTrue: showAutomaticTrue, setFalse: showAutomaticFalse }] = - useBoolean(false) + const [showAutomatic, setShowAutomatic] = useState(false) const handleAutomaticRes = useCallback( (res: GenRes) => { onGenerated?.(res.modified) - showAutomaticFalse() + setShowAutomatic(false) }, - [onGenerated, showAutomaticFalse], + [onGenerated], ) const configsMap = useHooksStore((s) => s.configsMap) return (
- + setShowAutomatic(true)}> {showAutomatic && ( @@ -48,7 +46,7 @@ const CodeGenerateBtn: FC = ({ mode={AppModeEnum.CHAT} isShow={showAutomatic} codeLanguages={codeLanguages} - onClose={showAutomaticFalse} + onClose={() => setShowAutomatic(false)} onFinished={handleAutomaticRes} flowId={configsMap?.flowId || ''} nodeId={nodeId} diff --git a/web/app/components/workflow/nodes/_base/components/editor/code-editor/editor-support-vars.tsx b/web/app/components/workflow/nodes/_base/components/editor/code-editor/editor-support-vars.tsx index f2174f15b1b..4606e7f4cbd 100644 --- a/web/app/components/workflow/nodes/_base/components/editor/code-editor/editor-support-vars.tsx +++ b/web/app/components/workflow/nodes/_base/components/editor/code-editor/editor-support-vars.tsx @@ -3,7 +3,6 @@ import type { FC } from 'react' import type { Props as EditorProps } from '.' import type { NodeOutPutVar, Variable } from '@/app/components/workflow/types' import { cn } from '@langgenius/dify-ui/cn' -import { useBoolean } from 'ahooks' import * as React from 'react' import { useEffect, useRef, useState } from 'react' import { createPortal } from 'react-dom' @@ -29,7 +28,7 @@ const CodeEditor: FC = ({ availableVars, varList, onAddVar, ...editorProp const monacoRef = useRef(null) const popupRef = useRef(null) - const [isShowVarPicker, { setTrue: showVarPicker, setFalse: hideVarPicker }] = useBoolean(false) + const [isShowVarPicker, setIsShowVarPicker] = useState(false) const [popupPosition, setPopupPosition] = useState({ x: 0, y: 0 }) @@ -48,9 +47,9 @@ const CodeEditor: FC = ({ availableVars, varList, onAddVar, ...editorProp const popupY = editorRect.top + cursorCoords.top + 20 // Adjust the vertical position as needed setPopupPosition({ x: popupX, y: popupY }) - showVarPicker() + setIsShowVarPicker(true) } else { - hideVarPicker() + setIsShowVarPicker(false) } } @@ -137,7 +136,7 @@ const CodeEditor: FC = ({ availableVars, varList, onAddVar, ...editorProp }, ]) - hideVarPicker() + setIsShowVarPicker(false) } return ( diff --git a/web/app/components/workflow/nodes/_base/components/field.tsx b/web/app/components/workflow/nodes/_base/components/field.tsx index 8325c388936..7c399b6ccaf 100644 --- a/web/app/components/workflow/nodes/_base/components/field.tsx +++ b/web/app/components/workflow/nodes/_base/components/field.tsx @@ -2,8 +2,8 @@ import type { FC, ReactNode } from 'react' import { cn } from '@langgenius/dify-ui/cn' import { RiArrowDownSLine } from '@remixicon/react' -import { useBoolean } from 'ahooks' import * as React from 'react' +import { useState } from 'react' import { Infotip } from '@/app/components/base/infotip' type Props = Readonly<{ @@ -40,7 +40,7 @@ const Field: FC = ({ required, warningDot, }) => { - const [fold, { toggle: toggleFold }] = useBoolean(true) + const [fold, setFold] = useState(true) const tooltipLabel = tooltip ? getTextFromNode(tooltip) || getTextFromNode(title) || 'Help' : undefined @@ -48,7 +48,7 @@ const Field: FC = ({ return (
supportFold && toggleFold()} + onClick={() => supportFold && setFold((isFolded) => !isFolded)} className={cn('flex items-center justify-between', supportFold && 'cursor-pointer')} >
diff --git a/web/app/components/workflow/nodes/_base/components/selector.tsx b/web/app/components/workflow/nodes/_base/components/selector.tsx index f072eff6073..c762462cb0f 100644 --- a/web/app/components/workflow/nodes/_base/components/selector.tsx +++ b/web/app/components/workflow/nodes/_base/components/selector.tsx @@ -1,8 +1,9 @@ 'use client' import type { FC } from 'react' import { cn } from '@langgenius/dify-ui/cn' -import { useBoolean, useClickAway } from 'ahooks' +import { useClickAway } from 'ahooks' import * as React from 'react' +import { useState } from 'react' import { ChevronSelectorVertical } from '@/app/components/base/icons/src/vender/line/arrows' import { Check } from '@/app/components/base/icons/src/vender/line/general' @@ -49,10 +50,10 @@ const TypeSelector: FC = ({ const item = allOptions ? allOptions.find((item) => item.value === value) : list.find((item) => item.value === value) - const [showOption, { setFalse: setHide, toggle: toggleShow }] = useBoolean(false) + const [showOption, setShowOption] = useState(false) const ref = React.useRef(null) useClickAway(() => { - setHide() + setShowOption(false) }, ref) return (
= ({ ref={ref} > {trigger ? ( -
+
setShowOption((isShown) => !isShown)} + className={cn(!readonly && 'cursor-pointer')} + > {trigger}
) : (
setShowOption((isShown) => !isShown)} className={cn( showOption && 'bg-state-base-hover', 'flex h-5 cursor-pointer items-center rounded-md pr-0.5 pl-1 text-xs font-semibold text-text-secondary hover:bg-state-base-hover', @@ -96,7 +100,7 @@ const TypeSelector: FC = ({
{ - setHide() + setShowOption(false) onChange(item.value) }} className={cn( diff --git a/web/app/components/workflow/nodes/_base/components/switch-plugin-version.tsx b/web/app/components/workflow/nodes/_base/components/switch-plugin-version.tsx index 55d0c028753..64857e8cb64 100644 --- a/web/app/components/workflow/nodes/_base/components/switch-plugin-version.tsx +++ b/web/app/components/workflow/nodes/_base/components/switch-plugin-version.tsx @@ -3,7 +3,6 @@ import type { FC, ReactNode } from 'react' import { cn } from '@langgenius/dify-ui/cn' import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' -import { useBoolean } from 'ahooks' import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import Badge from '@/app/components/base/badge' @@ -29,8 +28,7 @@ export const SwitchPluginVersion: FC = (props) => { const [pluginId] = uniqueIdentifier?.split(':') || [''] const [isShow, setIsShow] = useState(false) - const [isShowUpdateModal, { setTrue: showUpdateModal, setFalse: hideUpdateModal }] = - useBoolean(false) + const [isShowUpdateModal, setIsShowUpdateModal] = useState(false) const [target, setTarget] = useState<{ version: string pluginUniqueIden: string @@ -43,10 +41,10 @@ export const SwitchPluginVersion: FC = (props) => { const pluginDetail = pluginDetails.data?.plugins.at(0) const handleUpdatedFromMarketplace = useCallback(() => { - hideUpdateModal() + setIsShowUpdateModal(false) pluginDetails.refetch() onChange?.(target!.version) - }, [hideUpdateModal, onChange, pluginDetails, target]) + }, [onChange, pluginDetails, target]) const { getIconUrl } = useGetIcon() const icon = pluginDetail?.declaration.icon ? getIconUrl(pluginDetail.declaration.icon) @@ -77,7 +75,7 @@ export const SwitchPluginVersion: FC = (props) => { > {isShowUpdateModal && pluginDetail && ( setIsShowUpdateModal(false)} plugin={pluginManifestToCardPluginProps({ ...pluginDetail.declaration, icon: icon!, @@ -123,7 +121,7 @@ export const SwitchPluginVersion: FC = (props) => { pluginUniqueIden: state.unique_identifier, version: state.version, }) - showUpdateModal() + setIsShowUpdateModal(true) }} trigger={ = ({ name, payload, depth = 1, required, rootClassName }) const isRoot = depth === 1 const hasChildren = payload.type === Type.object && payload.properties const hasEnum = payload.enum && payload.enum.length > 0 - const [fold, { toggle: toggleFold }] = useBoolean(false) + const [fold, setFold] = useState(false) return (
@@ -36,7 +36,7 @@ const Field: FC = ({ name, payload, depth = 1, required, rootClassName }) 'absolute top-[50%] left-[-18px] h-4 w-4 translate-y-[-50%] cursor-pointer bg-components-panel-bg text-text-tertiary', fold && 'rotate-270 text-text-accent', )} - onClick={toggleFold} + onClick={() => setFold((isFolded) => !isFolded)} /> )}
({ }), })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: () => ({ - setShowAccountSettingModal: mockSetShowAccountSettingModal, - }), -})) +vi.mock('nuqs', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, useQueryState: () => [null, mockSetSettingsDestination] } +}) vi.mock('@/app/components/workflow/utils', async (importOriginal) => { const actual = await importOriginal() @@ -672,7 +671,7 @@ describe('workflow-panel index', () => { fireEvent.click(screen.getByText('authorized-in-datasource-node')) - expect(mockSetShowAccountSettingModal).toHaveBeenCalled() + expect(mockSetSettingsDestination).toHaveBeenCalledWith('data-source') }) it('should react to pending single run actions', () => { diff --git a/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx index 0cab2f85aee..a412af20c76 100644 --- a/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx +++ b/web/app/components/workflow/nodes/_base/components/workflow-panel/index.tsx @@ -7,6 +7,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '@langgenius/dify-ui/too import { RiCloseLine, RiPlayLargeLine } from '@remixicon/react' import { debounce } from 'es-toolkit/compat' import { useAtomValue } from 'jotai' +import { useQueryState } from 'nuqs' import * as React from 'react' import { cloneElement, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' @@ -14,9 +15,11 @@ import { useShallow } from 'zustand/react/shallow' import { useStore as useAppStore } from '@/app/components/app/store' import { Stop } from '@/app/components/base/icons/src/vender/line/mediaAndDevices' import { UserAvatarList } from '@/app/components/base/user-avatar-list' -import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { useLanguage } from '@/app/components/header/account-setting/model-provider-page/hooks' -import { useIntegrationsSetting } from '@/app/components/header/account-setting/use-integrations-setting' +import { + settingsQueryParamName, + settingsQueryParser, +} from '@/app/components/header/account-setting/query-params' import { AuthCategory, AuthorizedInDataSourceNode, @@ -374,11 +377,11 @@ const BasePanel: FC = ({ id, data, children }) => { [handleNodeDataUpdateWithSyncDraft, id], ) - const openIntegrationsSetting = useIntegrationsSetting() + const [, setSettingsDestination] = useQueryState(settingsQueryParamName, settingsQueryParser) const handleJumpToDataSourcePage = useCallback(() => { - openIntegrationsSetting({ payload: ACCOUNT_SETTING_TAB.DATA_SOURCE }) - }, [openIntegrationsSetting]) + setSettingsDestination('data-source') + }, [setSettingsDestination]) const { appendNodeInspectVars } = useInspectVarsCrud() diff --git a/web/app/components/workflow/nodes/_base/hooks/use-output-var-list.ts b/web/app/components/workflow/nodes/_base/hooks/use-output-var-list.ts index 20727d2bdec..a0739807c52 100644 --- a/web/app/components/workflow/nodes/_base/hooks/use-output-var-list.ts +++ b/web/app/components/workflow/nodes/_base/hooks/use-output-var-list.ts @@ -1,6 +1,6 @@ import type { CodeNodeType, OutputVar } from '../../code/types' import type { ValueSelector } from '@/app/components/workflow/types' -import { useBoolean, useDebounceFn } from 'ahooks' +import { useDebounceFn } from 'ahooks' import { produce } from 'immer' import { useCallback, useRef, useState } from 'react' import { ErrorHandleTypeEnum } from '@/app/components/workflow/nodes/_base/components/error-handle/types' @@ -116,10 +116,7 @@ function useOutputVarList({ onOutputKeyOrdersChange([...outputKeyOrders, newKey]) }, [generateNewKey, inputs, setInputs, onOutputKeyOrdersChange, outputKeyOrders, varKey]) - const [ - isShowRemoveVarConfirm, - { setTrue: showRemoveVarConfirm, setFalse: hideRemoveVarConfirm }, - ] = useBoolean(false) + const [isShowRemoveVarConfirm, setIsShowRemoveVarConfirm] = useState(false) const [removedVar, setRemovedVar] = useState([]) const removeVarInNode = useCallback(() => { const varId = nodesWithInspectVars @@ -129,21 +126,14 @@ function useOutputVarList({ })?.id if (varId) deleteInspectVar(id, varId) removeUsedVarInNodes(removedVar) - hideRemoveVarConfirm() - }, [ - deleteInspectVar, - hideRemoveVarConfirm, - id, - nodesWithInspectVars, - removeUsedVarInNodes, - removedVar, - ]) + setIsShowRemoveVarConfirm(false) + }, [deleteInspectVar, id, nodesWithInspectVars, removeUsedVarInNodes, removedVar]) const handleRemoveVariable = useCallback( (index: number) => { const key = outputKeyOrders[index]! if (isVarUsedInNodes([id, key])) { - showRemoveVarConfirm() + setIsShowRemoveVarConfirm(true) setRemovedVar([id, key]) return } @@ -180,7 +170,6 @@ function useOutputVarList({ onOutputKeyOrdersChange, nodesWithInspectVars, deleteInspectVar, - showRemoveVarConfirm, varKey, ], ) @@ -190,7 +179,7 @@ function useOutputVarList({ handleAddVariable, handleRemoveVariable, isShowRemoveVarConfirm, - hideRemoveVarConfirm, + hideRemoveVarConfirm: () => setIsShowRemoveVarConfirm(false), onRemoveVarConfirm: removeVarInNode, } } diff --git a/web/app/components/workflow/nodes/http/hooks/use-key-value-list.ts b/web/app/components/workflow/nodes/http/hooks/use-key-value-list.ts index 1f270666326..0563f041745 100644 --- a/web/app/components/workflow/nodes/http/hooks/use-key-value-list.ts +++ b/web/app/components/workflow/nodes/http/hooks/use-key-value-list.ts @@ -1,5 +1,4 @@ import type { KeyValue } from '../types' -import { useBoolean } from 'ahooks' import { uniqueId } from 'es-toolkit/compat' import { useCallback, useEffect, useState } from 'react' @@ -63,14 +62,14 @@ const useKeyValueList = (value: string, onChange: (value: string) => void, noFil ]) }, [list, setList]) - const [isKeyValueEdit, { toggle: toggleIsKeyValueEdit }] = useBoolean(true) + const [isKeyValueEdit, setIsKeyValueEdit] = useState(true) return { list: list.length === 0 ? [{ id: uniqueId(UNIQUE_ID_PREFIX), key: '', value: '' }] : list, // no item can not add new item setList, addItem, isKeyValueEdit, - toggleIsKeyValueEdit, + toggleIsKeyValueEdit: () => setIsKeyValueEdit((isEditing) => !isEditing), } } diff --git a/web/app/components/workflow/nodes/http/use-config.ts b/web/app/components/workflow/nodes/http/use-config.ts index 7718f5ead61..dc87c7ce2b4 100644 --- a/web/app/components/workflow/nodes/http/use-config.ts +++ b/web/app/components/workflow/nodes/http/use-config.ts @@ -1,6 +1,5 @@ import type { Var } from '../../types' import type { Authorization, Body, HttpNodeType, Method, Timeout } from './types' -import { useBoolean } from 'ahooks' import { produce } from 'immer' import { useCallback, useEffect, useState } from 'react' import useNodeCrud from '@/app/components/workflow/nodes/_base/hooks/use-node-crud' @@ -113,8 +112,7 @@ const useConfig = (id: string, payload: HttpNodeType) => { ) // authorization - const [isShowAuthorization, { setTrue: showAuthorization, setFalse: hideAuthorization }] = - useBoolean(false) + const [isShowAuthorization, setIsShowAuthorization] = useState(false) const setAuthorization = useCallback( (authorization: Authorization) => { @@ -141,7 +139,7 @@ const useConfig = (id: string, payload: HttpNodeType) => { }, []) // curl import panel - const [isShowCurlPanel, { setTrue: showCurlPanel, setFalse: hideCurlPanel }] = useBoolean(false) + const [isShowCurlPanel, setIsShowCurlPanel] = useState(false) const handleCurlImport = useCallback( (newNode: HttpNodeType) => { @@ -194,14 +192,14 @@ const useConfig = (id: string, payload: HttpNodeType) => { handleSSLVerifyChange, // authorization isShowAuthorization, - showAuthorization, - hideAuthorization, + showAuthorization: () => setIsShowAuthorization(true), + hideAuthorization: () => setIsShowAuthorization(false), setAuthorization, setTimeout, // curl import isShowCurlPanel, - showCurlPanel, - hideCurlPanel, + showCurlPanel: () => setIsShowCurlPanel(true), + hideCurlPanel: () => setIsShowCurlPanel(false), handleCurlImport, } } diff --git a/web/app/components/workflow/nodes/human-input/panel.tsx b/web/app/components/workflow/nodes/human-input/panel.tsx index 79426136eef..d1565d004a6 100644 --- a/web/app/components/workflow/nodes/human-input/panel.tsx +++ b/web/app/components/workflow/nodes/human-input/panel.tsx @@ -11,10 +11,9 @@ import { RiExpandDiagonalLine, RiEyeLine, } from '@remixicon/react' -import { useBoolean } from 'ahooks' import copy from 'copy-to-clipboard' import * as React from 'react' -import { useCallback } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' import Divider from '@/app/components/base/divider' @@ -70,10 +69,10 @@ const Panel: FC> = ({ id, data }) => { }, }) - const [isExpandFormContent, { toggle: toggleExpandFormContent }] = useBoolean(false) + const [isExpandFormContent, setIsExpandFormContent] = useState(false) const nodePanelWidth = useStore((state) => state.nodePanelWidth) - const [isPreview, { toggle: togglePreview, setFalse: hidePreview }] = useBoolean(false) + const [isPreview, setIsPreview] = useState(false) const onAddUseAction = useCallback(() => { const index = inputs.user_actions.length + 1 @@ -131,7 +130,7 @@ const Panel: FC> = ({ id, data }) => { 'flex items-center space-x-1 px-2', isPreview && 'bg-state-accent-active text-text-accent', )} - onClick={togglePreview} + onClick={() => setIsPreview((isPreview) => !isPreview)} >
@@ -160,7 +159,7 @@ const Panel: FC> = ({ id, data }) => { 'flex size-6 cursor-pointer items-center justify-center rounded-md border-none bg-transparent p-0 text-text-secondary hover:bg-components-button-ghost-bg-hover', isExpandFormContent && 'bg-state-accent-active text-text-accent', )} - onClick={toggleExpandFormContent} + onClick={() => setIsExpandFormContent((isExpanded) => !isExpanded)} > {isExpandFormContent ? ( @@ -263,7 +262,7 @@ const Panel: FC> = ({ id, data }) => { content={inputs.form_content} formInputs={inputs.inputs} userActions={inputs.user_actions} - onClose={hidePreview} + onClose={() => setIsPreview(false)} /> )}
diff --git a/web/app/components/workflow/nodes/knowledge-retrieval/components/add-dataset.tsx b/web/app/components/workflow/nodes/knowledge-retrieval/components/add-dataset.tsx index 12ff51482e7..96329581225 100644 --- a/web/app/components/workflow/nodes/knowledge-retrieval/components/add-dataset.tsx +++ b/web/app/components/workflow/nodes/knowledge-retrieval/components/add-dataset.tsx @@ -1,9 +1,8 @@ 'use client' import type { FC } from 'react' import type { DataSet } from '@/models/datasets' -import { useBoolean } from 'ahooks' import * as React from 'react' -import { useCallback } from 'react' +import { useCallback, useState } from 'react' import { useTranslation } from 'react-i18next' import SelectDataset from '@/app/components/app/configuration/dataset-config/select-dataset' @@ -15,14 +14,14 @@ type Props = Readonly<{ const AddDataset: FC = ({ selectedIds, modal, onChange }) => { const { t } = useTranslation() - const [isShowModal, { setTrue: showModal, setFalse: hideModal }] = useBoolean(false) + const [isShowModal, setIsShowModal] = useState(false) const handleSelect = useCallback( (datasets: DataSet[]) => { onChange(datasets) - hideModal() + setIsShowModal(false) }, - [onChange, hideModal], + [onChange], ) return (
@@ -30,14 +29,14 @@ const AddDataset: FC = ({ selectedIds, modal, onChange }) => { type="button" aria-label={`${t(($) => $['operation.add'], { ns: 'common' })} ${t(($) => $['nodes.knowledgeRetrieval.knowledge'], { ns: 'workflow' })}`} className="cursor-pointer rounded-md border-none bg-transparent p-1 outline-hidden select-none hover:bg-state-base-hover focus-visible:ring-2 focus-visible:ring-state-accent-solid" - onClick={showModal} + onClick={() => setIsShowModal(true)} >