From 328965ed7ce0d2bad2d476925fc7b080c6eecdbd Mon Sep 17 00:00:00 2001 From: KVOJJJin Date: Tue, 19 Nov 2024 14:15:18 +0800 Subject: [PATCH] Fix: crash of workflow file upload (#10831) Co-authored-by: -LAN- Co-authored-by: StyleZhang --- .../features/file_upload/manager.py | 4 +- api/core/app/apps/base_app_generator.py | 8 ++-- api/core/file/models.py | 4 +- api/factories/file_factory.py | 4 +- api/models/workflow.py | 4 +- .../workflow/test_sync_workflow.py | 4 +- api/tests/unit_tests/core/test_file.py | 42 ++++++++++++++++++- .../components/base/file-uploader/utils.ts | 17 ++++---- .../share/text-generation/index.tsx | 2 +- 9 files changed, 65 insertions(+), 24 deletions(-) diff --git a/api/core/app/app_config/features/file_upload/manager.py b/api/core/app/app_config/features/file_upload/manager.py index 2043ea0e41..0dc4efc47a 100644 --- a/api/core/app/app_config/features/file_upload/manager.py +++ b/api/core/app/app_config/features/file_upload/manager.py @@ -16,9 +16,7 @@ class FileUploadConfigManager: file_upload_dict = config.get("file_upload") if file_upload_dict: if file_upload_dict.get("enabled"): - transform_methods = file_upload_dict.get("allowed_file_upload_methods") or file_upload_dict.get( - "allowed_upload_methods", [] - ) + transform_methods = file_upload_dict.get("allowed_file_upload_methods", []) data = { "image_config": { "number_limits": file_upload_dict["number_limits"], diff --git a/api/core/app/apps/base_app_generator.py b/api/core/app/apps/base_app_generator.py index 3325e30088..2c78d95778 100644 --- a/api/core/app/apps/base_app_generator.py +++ b/api/core/app/apps/base_app_generator.py @@ -33,8 +33,8 @@ class BaseAppGenerator: tenant_id=app_config.tenant_id, config=FileUploadConfig( allowed_file_types=entity_dictionary[k].allowed_file_types, - allowed_extensions=entity_dictionary[k].allowed_file_extensions, - allowed_upload_methods=entity_dictionary[k].allowed_file_upload_methods, + allowed_file_extensions=entity_dictionary[k].allowed_file_extensions, + allowed_file_upload_methods=entity_dictionary[k].allowed_file_upload_methods, ), ) for k, v in user_inputs.items() @@ -47,8 +47,8 @@ class BaseAppGenerator: tenant_id=app_config.tenant_id, config=FileUploadConfig( allowed_file_types=entity_dictionary[k].allowed_file_types, - allowed_extensions=entity_dictionary[k].allowed_file_extensions, - allowed_upload_methods=entity_dictionary[k].allowed_file_upload_methods, + allowed_file_extensions=entity_dictionary[k].allowed_file_extensions, + allowed_file_upload_methods=entity_dictionary[k].allowed_file_upload_methods, ), ) for k, v in user_inputs.items() diff --git a/api/core/file/models.py b/api/core/file/models.py index 0142893787..3e7e189c62 100644 --- a/api/core/file/models.py +++ b/api/core/file/models.py @@ -28,8 +28,8 @@ class FileUploadConfig(BaseModel): image_config: Optional[ImageConfig] = None allowed_file_types: Sequence[FileType] = Field(default_factory=list) - allowed_extensions: Sequence[str] = Field(default_factory=list) - allowed_upload_methods: Sequence[FileTransferMethod] = Field(default_factory=list) + allowed_file_extensions: Sequence[str] = Field(default_factory=list) + allowed_file_upload_methods: Sequence[FileTransferMethod] = Field(default_factory=list) number_limits: int = 0 diff --git a/api/factories/file_factory.py b/api/factories/file_factory.py index 15e9d7f34f..94bbeebd6d 100644 --- a/api/factories/file_factory.py +++ b/api/factories/file_factory.py @@ -233,10 +233,10 @@ def _is_file_valid_with_config(*, file: File, config: FileUploadConfig) -> bool: if config.allowed_file_types and file.type not in config.allowed_file_types and file.type != FileType.CUSTOM: return False - if config.allowed_extensions and file.extension not in config.allowed_extensions: + if config.allowed_file_extensions and file.extension not in config.allowed_file_extensions: return False - if config.allowed_upload_methods and file.transfer_method not in config.allowed_upload_methods: + if config.allowed_file_upload_methods and file.transfer_method not in config.allowed_file_upload_methods: return False if file.type == FileType.IMAGE and config.image_config: diff --git a/api/models/workflow.py b/api/models/workflow.py index 4f0e9a5e03..c6b3000083 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -169,9 +169,9 @@ class Workflow(db.Model): ) features["file_upload"]["enabled"] = image_enabled features["file_upload"]["number_limits"] = image_number_limits - features["file_upload"]["allowed_upload_methods"] = image_transfer_methods + features["file_upload"]["allowed_file_upload_methods"] = image_transfer_methods features["file_upload"]["allowed_file_types"] = ["image"] - features["file_upload"]["allowed_extensions"] = [] + features["file_upload"]["allowed_file_extensions"] = [] del features["file_upload"]["image"] self._features = json.dumps(features) return self._features diff --git a/api/tests/integration_tests/workflow/test_sync_workflow.py b/api/tests/integration_tests/workflow/test_sync_workflow.py index df2ec95ebc..be270cdc49 100644 --- a/api/tests/integration_tests/workflow/test_sync_workflow.py +++ b/api/tests/integration_tests/workflow/test_sync_workflow.py @@ -27,8 +27,8 @@ NEW_VERSION_WORKFLOW_FEATURES = { "file_upload": { "enabled": True, "allowed_file_types": ["image"], - "allowed_extensions": [], - "allowed_upload_methods": ["remote_url", "local_file"], + "allowed_file_extensions": [], + "allowed_file_upload_methods": ["remote_url", "local_file"], "number_limits": 6, }, "opening_statement": "", diff --git a/api/tests/unit_tests/core/test_file.py b/api/tests/unit_tests/core/test_file.py index aa61c1c6f7..4edbc01cc7 100644 --- a/api/tests/unit_tests/core/test_file.py +++ b/api/tests/unit_tests/core/test_file.py @@ -1,4 +1,7 @@ -from core.file import FILE_MODEL_IDENTITY, File, FileTransferMethod, FileType +import json + +from core.file import FILE_MODEL_IDENTITY, File, FileTransferMethod, FileType, FileUploadConfig +from models.workflow import Workflow def test_file_loads_and_dumps(): @@ -38,3 +41,40 @@ def test_file_to_dict(): file_dict = file.to_dict() assert "_extra_config" not in file_dict assert "url" in file_dict + + +def test_workflow_features_with_image(): + # Create a feature dict that mimics the old structure with image config + features = { + "file_upload": { + "image": {"enabled": True, "number_limits": 5, "transfer_methods": ["remote_url", "local_file"]} + } + } + + # Create a workflow instance with the features + workflow = Workflow( + tenant_id="tenant-1", + app_id="app-1", + type="chat", + version="1.0", + graph="{}", + features=json.dumps(features), + created_by="user-1", + environment_variables=[], + conversation_variables=[], + ) + + # Get the converted features through the property + converted_features = json.loads(workflow.features) + + # Create FileUploadConfig from the converted features + file_upload_config = FileUploadConfig.model_validate(converted_features["file_upload"]) + + # Validate the config + assert file_upload_config.number_limits == 5 + assert list(file_upload_config.allowed_file_types) == [FileType.IMAGE] + assert list(file_upload_config.allowed_file_upload_methods) == [ + FileTransferMethod.REMOTE_URL, + FileTransferMethod.LOCAL_FILE, + ] + assert list(file_upload_config.allowed_file_extensions) == [] diff --git a/web/app/components/base/file-uploader/utils.ts b/web/app/components/base/file-uploader/utils.ts index 465673435f..aa8625f221 100644 --- a/web/app/components/base/file-uploader/utils.ts +++ b/web/app/components/base/file-uploader/utils.ts @@ -44,21 +44,24 @@ export const fileUpload: FileUpload = ({ } export const getFileExtension = (fileName: string, fileMimetype: string, isRemote?: boolean) => { + let extension = '' if (fileMimetype) - return mime.getExtension(fileMimetype) || '' + extension = mime.getExtension(fileMimetype) || '' - if (isRemote) - return '' - - if (fileName) { + if (fileName && !extension) { const fileNamePair = fileName.split('.') const fileNamePairLength = fileNamePair.length if (fileNamePairLength > 1) - return fileNamePair[fileNamePairLength - 1] + extension = fileNamePair[fileNamePairLength - 1] + else + extension = '' } - return '' + if (isRemote) + extension = '' + + return extension } export const getFileAppearanceType = (fileName: string, fileMimetype: string) => { diff --git a/web/app/components/share/text-generation/index.tsx b/web/app/components/share/text-generation/index.tsx index f9f3c27dc6..27ea46cbff 100644 --- a/web/app/components/share/text-generation/index.tsx +++ b/web/app/components/share/text-generation/index.tsx @@ -391,7 +391,7 @@ const TextGeneration: FC = ({ setVisionConfig({ // legacy of image upload compatible ...file_upload, - transfer_methods: file_upload.allowed_upload_methods, + transfer_methods: file_upload.allowed_file_upload_methods || file_upload.allowed_upload_methods, // legacy of image upload compatible image_file_size_limit: appParams?.system_parameters?.image_file_size_limit, fileUploadConfig: appParams?.system_parameters,