mirror of https://github.com/langgenius/dify.git
feat(api): limit conversation variable description length
Previously, conversation variable descriptions had no length limit, causing errors when debugging chatflows. These descriptions are saved as draft variable descriptions during debugging, which have a 256-character limit. Variables with descriptions exceeding this limit could not be saved, resulting in 500 errors and "Internal Server Error" messages during chatflow debugging. This commit implements description length limits for conversation variables: - New chatflows: Hard limit of 256 characters for descriptions. Longer descriptions are rejected and prevent chatflow saving. - Existing chatflows: If all conversation variable descriptions are under 256 characters, the same limit applies as new chatflows. Otherwise, no limit is enforced to maintain backward compatibility.
This commit is contained in:
parent
215e318600
commit
81d9a9d00d
|
|
@ -1,3 +1,6 @@
|
|||
from libs.exception import BaseHTTPException
|
||||
|
||||
|
||||
class WorkflowInUseError(ValueError):
|
||||
"""Raised when attempting to delete a workflow that's in use by an app"""
|
||||
|
||||
|
|
@ -8,3 +11,17 @@ class DraftWorkflowDeletionError(ValueError):
|
|||
"""Raised when attempting to delete a draft workflow"""
|
||||
|
||||
pass
|
||||
|
||||
|
||||
class ConversationVariableDescriptionTooLongError(BaseHTTPException):
|
||||
"""Raised when conversation variable description exceeds maximum length"""
|
||||
|
||||
error_code = "conversation_variable_description_too_long"
|
||||
code = 400
|
||||
|
||||
def __init__(self, current_length: int, max_length: int):
|
||||
description = (
|
||||
f"Conversation variable description exceeds maximum length of "
|
||||
f"{max_length} characters. Current length: {current_length} characters."
|
||||
)
|
||||
super().__init__(description)
|
||||
|
|
|
|||
|
|
@ -42,8 +42,16 @@ from services.enterprise.plugin_manager_service import PluginCredentialType
|
|||
from services.errors.app import IsDraftWorkflowError, WorkflowHashNotEqualError
|
||||
from services.workflow.workflow_converter import WorkflowConverter
|
||||
|
||||
from .errors.workflow_service import DraftWorkflowDeletionError, WorkflowInUseError
|
||||
from .workflow_draft_variable_service import DraftVariableSaver, DraftVarLoader, WorkflowDraftVariableService
|
||||
from .errors.workflow_service import (
|
||||
ConversationVariableDescriptionTooLongError,
|
||||
DraftWorkflowDeletionError,
|
||||
WorkflowInUseError,
|
||||
)
|
||||
from .workflow_draft_variable_service import (
|
||||
DraftVariableSaver,
|
||||
DraftVarLoader,
|
||||
WorkflowDraftVariableService,
|
||||
)
|
||||
|
||||
|
||||
class WorkflowService:
|
||||
|
|
@ -189,6 +197,42 @@ class WorkflowService:
|
|||
|
||||
return workflows, has_more
|
||||
|
||||
def _validate_conversation_variable_descriptions(
|
||||
self,
|
||||
conversation_variables: Sequence[Variable],
|
||||
existing_workflow: Workflow | None = None,
|
||||
) -> None:
|
||||
"""
|
||||
Validate conversation variable descriptions for length constraints.
|
||||
|
||||
Args:
|
||||
conversation_variables: List of conversation variables to validate
|
||||
existing_workflow: Existing workflow for backward compatibility check (optional)
|
||||
|
||||
Raises:
|
||||
ConversationVariableDescriptionTooLongError: If any description exceeds the length limit and shouldn't be allowed
|
||||
"""
|
||||
MAX_CONVERSATION_VARIABLE_DESCRIPTION_LENGTH = 256
|
||||
|
||||
# Check if existing workflow has any conversation variables with long descriptions
|
||||
has_existing_long_descriptions = False
|
||||
if existing_workflow and existing_workflow.conversation_variables:
|
||||
for var in existing_workflow.conversation_variables:
|
||||
if len(var.description) > MAX_CONVERSATION_VARIABLE_DESCRIPTION_LENGTH:
|
||||
has_existing_long_descriptions = True
|
||||
break
|
||||
|
||||
# Validate new conversation variables
|
||||
for variable in conversation_variables:
|
||||
description_length = len(variable.description)
|
||||
if description_length > MAX_CONVERSATION_VARIABLE_DESCRIPTION_LENGTH:
|
||||
# Allow existing workflows with long descriptions to be updated
|
||||
# But don't allow new long descriptions or extending existing ones
|
||||
if not has_existing_long_descriptions:
|
||||
raise ConversationVariableDescriptionTooLongError(
|
||||
current_length=description_length, max_length=MAX_CONVERSATION_VARIABLE_DESCRIPTION_LENGTH
|
||||
)
|
||||
|
||||
def sync_draft_workflow(
|
||||
self,
|
||||
*,
|
||||
|
|
@ -213,6 +257,12 @@ class WorkflowService:
|
|||
# validate features structure
|
||||
self.validate_features_structure(app_model=app_model, features=features)
|
||||
|
||||
# validate conversation variable descriptions
|
||||
self._validate_conversation_variable_descriptions(
|
||||
conversation_variables=conversation_variables,
|
||||
existing_workflow=workflow,
|
||||
)
|
||||
|
||||
# create draft workflow if not found
|
||||
if not workflow:
|
||||
workflow = Workflow(
|
||||
|
|
|
|||
|
|
@ -0,0 +1,304 @@
|
|||
"""
|
||||
Simplified integration tests for conversation variable description length validation.
|
||||
|
||||
This test suite provides focused testing of the sync_draft_workflow method
|
||||
through the service layer with minimal API mocking.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.variables.variables import StringVariable
|
||||
from models.account import Account
|
||||
from models.model import App
|
||||
from models.workflow import Workflow
|
||||
from services.errors.workflow_service import ConversationVariableDescriptionTooLongError
|
||||
from services.workflow_service import WorkflowService
|
||||
|
||||
|
||||
class TestWorkflowConversationVariableValidationSimplified:
|
||||
"""Simplified integration tests focusing on the service layer."""
|
||||
|
||||
@pytest.fixture
|
||||
def workflow_service(self):
|
||||
# Mock the database dependencies to avoid Flask app context issues
|
||||
with patch("services.workflow_service.db") as mock_db:
|
||||
with patch("services.workflow_service.sessionmaker") as mock_sessionmaker:
|
||||
mock_db.engine = MagicMock()
|
||||
mock_sessionmaker.return_value = MagicMock()
|
||||
return WorkflowService()
|
||||
|
||||
@pytest.fixture
|
||||
def mock_app(self):
|
||||
app = MagicMock(spec=App)
|
||||
app.id = "test-app-id"
|
||||
app.tenant_id = "test-tenant-id"
|
||||
app.mode = "workflow"
|
||||
return app
|
||||
|
||||
@pytest.fixture
|
||||
def mock_account(self):
|
||||
account = MagicMock(spec=Account)
|
||||
account.id = "test-user-id"
|
||||
return account
|
||||
|
||||
@pytest.fixture
|
||||
def sample_graph(self):
|
||||
return {"nodes": [], "edges": []}
|
||||
|
||||
@pytest.fixture
|
||||
def sample_features(self):
|
||||
return {"retriever_resource": {"enabled": True}}
|
||||
|
||||
def test_service_rejects_long_description_new_workflow(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that the service rejects long descriptions for new workflows."""
|
||||
long_description = "This description exceeds the 256 character limit. " * 6 # ~282 chars
|
||||
conversation_variables = [
|
||||
StringVariable(id="test-var-1", name="test_var", description=long_description, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
assert "300 characters" in str(exc_info.value)
|
||||
|
||||
def test_service_accepts_valid_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that the service accepts valid description lengths."""
|
||||
conversation_variables = [
|
||||
StringVariable(id="test-var-1", name="short_var", description="Short description", value="short_value"),
|
||||
StringVariable(
|
||||
id="test-var-2",
|
||||
name="medium_var",
|
||||
description="This is a medium length description that is well within the 256 character limit.",
|
||||
value="medium_value",
|
||||
),
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
def test_service_accepts_exactly_256_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that descriptions with exactly 256 characters are accepted."""
|
||||
exact_description = "a" * 256
|
||||
conversation_variables = [
|
||||
StringVariable(id="test-var-1", name="exact_var", description=exact_description, value="exact_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
def test_service_rejects_257_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that descriptions with 257 characters are rejected."""
|
||||
over_limit_description = "a" * 257
|
||||
conversation_variables = [
|
||||
StringVariable(
|
||||
id="test-var-1", name="over_limit_var", description=over_limit_description, value="over_limit_value"
|
||||
)
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
assert "257 characters" in str(exc_info.value)
|
||||
|
||||
def test_service_allows_existing_workflow_with_long_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that existing workflows with long descriptions can be updated."""
|
||||
long_description = (
|
||||
"This description exceeds 256 characters and should be allowed for existing workflows. " * 3
|
||||
) # ~309 chars
|
||||
|
||||
# Mock existing workflow with long description
|
||||
existing_variable = StringVariable(
|
||||
id="existing-var", name="existing_var", description=long_description, value="existing_value"
|
||||
)
|
||||
existing_workflow = MagicMock(spec=Workflow)
|
||||
existing_workflow.unique_hash = "test-hash"
|
||||
existing_workflow.conversation_variables = [existing_variable]
|
||||
|
||||
# New variable with long description should be allowed
|
||||
new_conversation_variables = [
|
||||
StringVariable(id="new-var", name="new_var", description=long_description, value="new_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=existing_workflow):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=new_conversation_variables,
|
||||
)
|
||||
|
||||
assert result == existing_workflow
|
||||
|
||||
def test_service_handles_unicode_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that Unicode characters are properly handled."""
|
||||
# Unicode description within limit
|
||||
unicode_description = "测试描述包含中文字符和emoji 🚀✨ " * 8 # Should be around 240 chars
|
||||
conversation_variables = [
|
||||
StringVariable(id="test-var-1", name="unicode_var", description=unicode_description, value="unicode_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
def test_service_rejects_unicode_characters_exceeding_limit(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that Unicode characters exceeding limit are rejected."""
|
||||
# Unicode description exceeding limit - increased to ensure it exceeds 256 chars
|
||||
unicode_description = "测试描述包含中文字符和emoji 🚀✨ " * 15 # Should exceed 256 chars (300 chars)
|
||||
conversation_variables = [
|
||||
StringVariable(
|
||||
id="test-var-1", name="unicode_long_var", description=unicode_description, value="unicode_long_value"
|
||||
)
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
|
||||
def test_service_handles_mixed_valid_and_invalid_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that validation fails when mixing valid and invalid description lengths."""
|
||||
conversation_variables = [
|
||||
StringVariable(
|
||||
id="test-var-1",
|
||||
name="valid_var",
|
||||
description="This is a valid description within the limit.",
|
||||
value="valid_value",
|
||||
),
|
||||
StringVariable(
|
||||
id="test-var-2",
|
||||
name="invalid_var",
|
||||
description="This description is way too long and exceeds the 256 character limit by a significant margin. "
|
||||
* 3, # ~297 chars
|
||||
value="invalid_value",
|
||||
),
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=conversation_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
|
||||
def test_service_handles_empty_conversation_variables(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features
|
||||
):
|
||||
"""Test that empty conversation variables list is handled correctly."""
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=[],
|
||||
conversation_variables=[],
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
|
@ -1,26 +1,40 @@
|
|||
from unittest.mock import MagicMock
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from core.variables.variables import StringVariable
|
||||
from models.account import Account
|
||||
from models.model import App
|
||||
from models.workflow import Workflow
|
||||
from services.errors.workflow_service import ConversationVariableDescriptionTooLongError
|
||||
from services.workflow_service import WorkflowService
|
||||
|
||||
|
||||
class TestWorkflowService:
|
||||
@pytest.fixture
|
||||
def workflow_service(self):
|
||||
mock_session_maker = MagicMock()
|
||||
return WorkflowService(mock_session_maker)
|
||||
# Mock the database dependencies to avoid Flask app context issues
|
||||
with patch("services.workflow_service.db") as mock_db:
|
||||
with patch("services.workflow_service.sessionmaker") as mock_sessionmaker:
|
||||
mock_db.engine = MagicMock()
|
||||
mock_sessionmaker.return_value = MagicMock()
|
||||
return WorkflowService()
|
||||
|
||||
@pytest.fixture
|
||||
def mock_app(self):
|
||||
app = MagicMock(spec=App)
|
||||
app.id = "app-id-1"
|
||||
app.workflow_id = "workflow-id-1"
|
||||
app.tenant_id = "tenant-id-1"
|
||||
app.mode = "workflow"
|
||||
return app
|
||||
|
||||
@pytest.fixture
|
||||
def mock_account(self):
|
||||
account = MagicMock(spec=Account)
|
||||
account.id = "user-id-1"
|
||||
return account
|
||||
|
||||
@pytest.fixture
|
||||
def mock_workflows(self):
|
||||
workflows = []
|
||||
|
|
@ -47,6 +61,7 @@ class TestWorkflowService:
|
|||
mock_session.scalars.assert_not_called()
|
||||
|
||||
def test_get_all_published_workflow_basic(self, workflow_service, mock_app, mock_workflows):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
mock_scalar_result.all.return_value = mock_workflows[:3]
|
||||
|
|
@ -61,6 +76,7 @@ class TestWorkflowService:
|
|||
mock_session.scalars.assert_called_once()
|
||||
|
||||
def test_get_all_published_workflow_pagination(self, workflow_service, mock_app, mock_workflows):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
# Return 4 items when limit is 3, which should indicate has_more=True
|
||||
|
|
@ -88,6 +104,7 @@ class TestWorkflowService:
|
|||
assert has_more is False
|
||||
|
||||
def test_get_all_published_workflow_user_filter(self, workflow_service, mock_app, mock_workflows):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
# Filter workflows for user-id-1
|
||||
|
|
@ -108,6 +125,7 @@ class TestWorkflowService:
|
|||
assert "created_by" in str(args)
|
||||
|
||||
def test_get_all_published_workflow_named_only(self, workflow_service, mock_app, mock_workflows):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
# Filter workflows that have a marked_name
|
||||
|
|
@ -128,6 +146,7 @@ class TestWorkflowService:
|
|||
assert "marked_name !=" in str(args)
|
||||
|
||||
def test_get_all_published_workflow_combined_filters(self, workflow_service, mock_app, mock_workflows):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
# Combined filter: user-id-1 and has marked_name
|
||||
|
|
@ -149,6 +168,7 @@ class TestWorkflowService:
|
|||
assert "marked_name !=" in str(args)
|
||||
|
||||
def test_get_all_published_workflow_empty_result(self, workflow_service, mock_app):
|
||||
mock_app.workflow_id = "workflow-id-1"
|
||||
mock_session = MagicMock()
|
||||
mock_scalar_result = MagicMock()
|
||||
mock_scalar_result.all.return_value = []
|
||||
|
|
@ -161,3 +181,365 @@ class TestWorkflowService:
|
|||
assert workflows == []
|
||||
assert has_more is False
|
||||
mock_session.scalars.assert_called_once()
|
||||
|
||||
|
||||
class TestSyncDraftWorkflow:
|
||||
"""Test cases focused on the sync_draft_workflow method."""
|
||||
|
||||
@pytest.fixture
|
||||
def workflow_service(self):
|
||||
# Mock the database dependencies to avoid Flask app context issues
|
||||
with patch("services.workflow_service.db") as mock_db:
|
||||
with patch("services.workflow_service.sessionmaker") as mock_sessionmaker:
|
||||
mock_db.engine = MagicMock()
|
||||
mock_sessionmaker.return_value = MagicMock()
|
||||
return WorkflowService()
|
||||
|
||||
@pytest.fixture
|
||||
def mock_app(self):
|
||||
app = MagicMock(spec=App)
|
||||
app.id = "app-id-1"
|
||||
app.tenant_id = "tenant-id-1"
|
||||
app.mode = "workflow"
|
||||
return app
|
||||
|
||||
@pytest.fixture
|
||||
def mock_account(self):
|
||||
account = MagicMock(spec=Account)
|
||||
account.id = "user-id-1"
|
||||
return account
|
||||
|
||||
@pytest.fixture
|
||||
def sample_graph(self):
|
||||
return {"nodes": [], "edges": []}
|
||||
|
||||
@pytest.fixture
|
||||
def sample_features(self):
|
||||
return {"retriever_resource": {"enabled": True}}
|
||||
|
||||
@pytest.fixture
|
||||
def sample_environment_variables(self):
|
||||
return []
|
||||
|
||||
@pytest.fixture
|
||||
def sample_conversation_variables(self):
|
||||
return [
|
||||
StringVariable(
|
||||
id="var-1", name="test_var", description="A valid description within limits", value="test_value"
|
||||
)
|
||||
]
|
||||
|
||||
@patch("services.workflow_service.db.session")
|
||||
@patch("services.workflow_service.app_draft_workflow_was_synced")
|
||||
def test_sync_draft_workflow_creates_new_workflow(
|
||||
self,
|
||||
mock_signal,
|
||||
mock_db_session,
|
||||
workflow_service,
|
||||
mock_app,
|
||||
mock_account,
|
||||
sample_graph,
|
||||
sample_features,
|
||||
sample_environment_variables,
|
||||
sample_conversation_variables,
|
||||
):
|
||||
"""Test that sync_draft_workflow creates a new workflow when none exists."""
|
||||
# Mock get_draft_workflow to return None (no existing workflow)
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=sample_conversation_variables,
|
||||
)
|
||||
|
||||
# Verify a new workflow was created
|
||||
assert result is not None
|
||||
mock_db_session.add.assert_called_once()
|
||||
mock_db_session.commit.assert_called_once()
|
||||
mock_signal.send.assert_called_once()
|
||||
|
||||
@patch("services.workflow_service.db.session")
|
||||
@patch("services.workflow_service.app_draft_workflow_was_synced")
|
||||
def test_sync_draft_workflow_updates_existing_workflow(
|
||||
self,
|
||||
mock_signal,
|
||||
mock_db_session,
|
||||
workflow_service,
|
||||
mock_app,
|
||||
mock_account,
|
||||
sample_graph,
|
||||
sample_features,
|
||||
sample_environment_variables,
|
||||
sample_conversation_variables,
|
||||
):
|
||||
"""Test that sync_draft_workflow updates an existing workflow."""
|
||||
# Mock existing workflow
|
||||
existing_workflow = MagicMock(spec=Workflow)
|
||||
existing_workflow.unique_hash = "test-hash"
|
||||
existing_workflow.conversation_variables = []
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=existing_workflow):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=sample_conversation_variables,
|
||||
)
|
||||
|
||||
# Verify existing workflow was updated
|
||||
assert result == existing_workflow
|
||||
assert existing_workflow.graph == json.dumps(sample_graph)
|
||||
assert existing_workflow.features == json.dumps(sample_features)
|
||||
assert existing_workflow.updated_by == mock_account.id
|
||||
assert existing_workflow.environment_variables == sample_environment_variables
|
||||
assert existing_workflow.conversation_variables == sample_conversation_variables
|
||||
mock_db_session.add.assert_not_called() # Should not add new workflow
|
||||
mock_db_session.commit.assert_called_once()
|
||||
mock_signal.send.assert_called_once()
|
||||
|
||||
def test_sync_draft_workflow_validates_conversation_variable_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that sync_draft_workflow validates conversation variable descriptions."""
|
||||
# Create conversation variable with description exceeding limit
|
||||
long_description = "a" * 300 # Exceeds 256 character limit
|
||||
long_desc_variables = [
|
||||
StringVariable(id="var-1", name="test_var", description=long_description, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=long_desc_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
assert "Current length: 300 characters" in str(exc_info.value)
|
||||
|
||||
def test_sync_draft_workflow_allows_existing_long_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that sync_draft_workflow allows updates when existing workflow has long descriptions."""
|
||||
# Create existing workflow with long description
|
||||
long_description = "a" * 300
|
||||
existing_variable = StringVariable(
|
||||
id="existing-var", name="existing_var", description=long_description, value="existing_value"
|
||||
)
|
||||
existing_workflow = MagicMock(spec=Workflow)
|
||||
existing_workflow.unique_hash = "test-hash"
|
||||
existing_workflow.conversation_variables = [existing_variable]
|
||||
|
||||
# New variables with long descriptions should be allowed
|
||||
new_long_desc_variables = [
|
||||
StringVariable(id="new-var", name="new_var", description=long_description, value="new_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=existing_workflow):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
# Should not raise exception
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=new_long_desc_variables,
|
||||
)
|
||||
|
||||
assert result == existing_workflow
|
||||
|
||||
def test_sync_draft_workflow_validates_exactly_256_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that descriptions with exactly 256 characters are accepted."""
|
||||
exact_description = "a" * 256
|
||||
exact_limit_variables = [
|
||||
StringVariable(id="var-1", name="test_var", description=exact_description, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
# Should not raise exception
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=exact_limit_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
def test_sync_draft_workflow_rejects_257_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that descriptions with 257 characters are rejected."""
|
||||
over_limit_description = "a" * 257
|
||||
over_limit_variables = [
|
||||
StringVariable(id="var-1", name="test_var", description=over_limit_description, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=over_limit_variables,
|
||||
)
|
||||
|
||||
assert "Current length: 257 characters" in str(exc_info.value)
|
||||
|
||||
def test_sync_draft_workflow_handles_unicode_characters(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that Unicode characters are properly handled in description length validation."""
|
||||
# Unicode characters that take multiple bytes in UTF-8
|
||||
unicode_description = "测试中文字符" * 42 + "测试中文" # Exactly 256 characters
|
||||
unicode_variables = [
|
||||
StringVariable(id="var-1", name="test_var", description=unicode_description, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
# Should not raise exception as it's exactly 256 characters
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=unicode_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
# Test with 257 Unicode characters
|
||||
unicode_description_over = "测试中文字符" * 42 + "测试中文测" # 257 characters
|
||||
unicode_variables_over = [
|
||||
StringVariable(id="var-2", name="test_var_2", description=unicode_description_over, value="test_value")
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=unicode_variables_over,
|
||||
)
|
||||
|
||||
assert "Current length: 257 characters" in str(exc_info.value)
|
||||
|
||||
def test_sync_draft_workflow_handles_empty_conversation_variables(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that empty conversation variables list is handled correctly."""
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=[],
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
||||
def test_sync_draft_workflow_handles_multiple_variables_one_invalid(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that validation fails when one of multiple variables exceeds limit."""
|
||||
mixed_variables = [
|
||||
StringVariable(id="var-1", name="valid_var", description="Valid description", value="valid_value"),
|
||||
StringVariable(
|
||||
id="var-2",
|
||||
name="invalid_var",
|
||||
description="a" * 300, # Exceeds limit
|
||||
value="invalid_value",
|
||||
),
|
||||
StringVariable(
|
||||
id="var-3",
|
||||
name="another_valid_var",
|
||||
description="Another valid description",
|
||||
value="another_valid_value",
|
||||
),
|
||||
]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with pytest.raises(ConversationVariableDescriptionTooLongError) as exc_info:
|
||||
workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=mixed_variables,
|
||||
)
|
||||
|
||||
assert "exceeds maximum length of 256 characters" in str(exc_info.value)
|
||||
assert "Current length: 300 characters" in str(exc_info.value)
|
||||
|
||||
def test_sync_draft_workflow_handles_empty_descriptions(
|
||||
self, workflow_service, mock_app, mock_account, sample_graph, sample_features, sample_environment_variables
|
||||
):
|
||||
"""Test that empty description strings are handled correctly."""
|
||||
empty_desc_variables = [StringVariable(id="var-1", name="empty_desc_var", description="", value="test_value")]
|
||||
|
||||
with patch.object(workflow_service, "get_draft_workflow", return_value=None):
|
||||
with patch.object(workflow_service, "validate_features_structure"):
|
||||
with patch("services.workflow_service.db.session"):
|
||||
with patch("services.workflow_service.app_draft_workflow_was_synced"):
|
||||
# Should not raise exception
|
||||
result = workflow_service.sync_draft_workflow(
|
||||
app_model=mock_app,
|
||||
graph=sample_graph,
|
||||
features=sample_features,
|
||||
unique_hash="test-hash",
|
||||
account=mock_account,
|
||||
environment_variables=sample_environment_variables,
|
||||
conversation_variables=empty_desc_variables,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
|
|
|
|||
Loading…
Reference in New Issue