langgenius/dify
1
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-07-21 20:15:31 -04:00
Code Issues 775 Packages Projects Releases 10 Wiki Activity

[PR #28892] feat: Add comprehensive unit tests for FileService (document uploads) #32226

New Issue
Closed
opened 2026-02-21 20:50:59 -05:00 by yindo · 0 comments
yindo commented 2026-02-21 20:50:59 -05:00
Owner
Copy Link
Copy Source

Original Pull Request: https://github.com/langgenius/dify/pull/28892

State: closed
Merged: No

Summary

This PR adds comprehensive unit tests for the FileService class located in api/services/file_service.py. The test suite provides complete coverage for all file upload, validation, storage, and retrieval operations, ensuring robust file handling functionality across the platform.

Fixes #28887

What's Changed

  • Test Coverage: Added comprehensive unit tests covering all 10 methods in FileService
  • Documentation: Extensive inline documentation, comments, and docstrings explaining test logic and expected behavior
  • Test Organization: Tests organized into six main test classes:
    • TestFileServiceInitialization: Service initialization with Engine and sessionmaker
    • TestFileServiceUpload: File upload operations (upload_file, upload_text)
    • TestFileServiceValidation: Filename, extension, and file type validation
    • TestFileServiceSizeLimits: File size limit validation for different file types
    • TestFileServiceRetrieval: File retrieval operations (previews, content, generators)
    • TestFileServiceDeletion: File deletion from storage and database

Test Coverage Details

File Upload Handling

  • ✅ upload_file: File upload with validation, storage, database persistence, UUID generation, hash computation, source URL generation
  • ✅ upload_text: Text content upload with UTF-8 encoding, filename truncation
  • ✅ Account vs EndUser role assignment
  • ✅ Custom source URL preservation

File Validation

  • ✅ Filename validation (invalid character detection: /, \, :, *, ?, ", <, >, |)
  • ✅ Filename length truncation (>200 characters)
  • ✅ Extension blacklist checking (BlockedFileExtensionError)
  • ✅ Dataset source file type restrictions (UnsupportedFileTypeError)
  • ✅ File size validation integration

File Size Limits and Quota Checking

  • ✅ is_file_size_within_limit: Static method testing
    • Image files (uses UPLOAD_IMAGE_FILE_SIZE_LIMIT)
    • Video files (uses UPLOAD_VIDEO_FILE_SIZE_LIMIT)
    • Audio files (uses UPLOAD_AUDIO_FILE_SIZE_LIMIT)
    • Document/other files (uses UPLOAD_FILE_SIZE_LIMIT)
  • ✅ Boundary testing (at limit, over limit)
  • ✅ FileTooLargeError exception handling

File Storage Operations

  • ✅ File save to storage (storage.save())
  • ✅ File load from storage (storage.load())
  • ✅ File delete from storage (storage.delete())
  • ✅ Storage key generation with tenant isolation
  • ✅ UTF-8 encoding handling for text files

File Metadata Management

  • ✅ UploadFile model creation with all attributes
  • ✅ SHA3-256 hash computation and verification
  • ✅ Source URL generation (get_signed_file_url())
  • ✅ Tenant isolation in storage paths
  • ✅ User role assignment (Account vs EndUser)
  • ✅ File metadata persistence

File Retrieval Operations

  • ✅ get_file_preview: Document preview with PREVIEW_WORDS_LIMIT (3000) truncation
  • ✅ get_image_preview: Image preview with signature verification
  • ✅ get_file_generator_by_file_id: File generator with signature verification
  • ✅ get_public_image_preview: Public image preview (no signature)
  • ✅ get_file_content: File content retrieval as UTF-8 string
  • ✅ NotFound exception handling for missing files
  • ✅ UnsupportedFileTypeError for invalid file types

File Deletion

  • ✅ delete_file: File deletion from storage and database
  • ✅ Transaction management with session.begin()
  • ✅ NotFound handling (no-op if file doesn't exist)

Constructor and Initialization

  • ✅ Initialization with Engine
  • ✅ Initialization with sessionmaker
  • ✅ AssertionError for invalid initialization (None, wrong type)

Key Features

  • 100% Function Coverage: All methods in FileService are tested
  • Edge Case Handling: Tests cover empty inputs, NotFound exceptions, file size limits, invalid filenames
  • Security Testing: Extension blacklist, signature verification, tenant isolation
  • Mocking Strategy: All external dependencies (storage, database session, file helpers, extract processor) are properly mocked
  • Factory Pattern: Uses FileServiceTestDataFactory for consistent test data creation
  • AAA Pattern: All tests follow Arrange-Act-Assert structure

Testing Approach

  • Mocked storage operations for isolated unit tests
  • Mocked database session and queries
  • Mocked file helper functions (signature verification, URL generation)
  • Mocked ExtractProcessor for file preview generation
  • Comprehensive error handling tests (NotFound, FileTooLargeError, BlockedFileExtensionError, UnsupportedFileTypeError)
  • Validation tests for filenames, extensions, file types, and sizes
  • Tenant isolation verification

Screenshots

N/A (Backend unit tests, no UI changes)

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran dev/reformat(backend) and cd web && npx lint-staged(frontend) to appease the lint gods

Related Files

  • api/services/file_service.py - Service under test (246 lines, 10 methods)
  • api/tests/unit_tests/services/test_file_service.py - Test suite (1,625+ lines)
  • api/core/file/helpers.py - File helper functions (get_signed_file_url, verify signatures)
  • api/extensions/ext_storage.py - Storage abstraction (save, load, delete)
  • api/models/model.py - UploadFile model
  • api/configs/dify_config.py - Configuration (file size limits, extension blacklist)
  • api/constants.py - File extension constants (DOCUMENT_EXTENSIONS, IMAGE_EXTENSIONS, etc.)

Benefits

  • Improved Code Reliability: Comprehensive test coverage ensures file operations work correctly
  • Enhanced Security: Tests validate security controls (extension blacklist, size limits, signature verification)
  • Easier Refactoring: Tests provide confidence when making changes to file upload functionality
  • Better Documentation: Extensive comments serve as documentation for expected behavior
  • Regression Prevention: Tests catch regressions when making changes to file-related code
  • Multi-tenancy Validation: Tests verify tenant isolation is properly enforced
  • Data Integrity: Tests verify file storage, metadata persistence, and deletion operations
  • Validation Logic: Tests confirm all validation rules work correctly

Contribution by Gittensor, learn more at https://gittensor.io/

**Original Pull Request:** https://github.com/langgenius/dify/pull/28892 **State:** closed **Merged:** No --- ## Summary This PR adds comprehensive unit tests for the `FileService` class located in `api/services/file_service.py`. The test suite provides complete coverage for all file upload, validation, storage, and retrieval operations, ensuring robust file handling functionality across the platform. Fixes #28887 ### What's Changed - **Test Coverage**: Added comprehensive unit tests covering all 10 methods in `FileService` - **Documentation**: Extensive inline documentation, comments, and docstrings explaining test logic and expected behavior - **Test Organization**: Tests organized into six main test classes: - `TestFileServiceInitialization`: Service initialization with Engine and sessionmaker - `TestFileServiceUpload`: File upload operations (upload_file, upload_text) - `TestFileServiceValidation`: Filename, extension, and file type validation - `TestFileServiceSizeLimits`: File size limit validation for different file types - `TestFileServiceRetrieval`: File retrieval operations (previews, content, generators) - `TestFileServiceDeletion`: File deletion from storage and database ### Test Coverage Details #### File Upload Handling - ✅ `upload_file`: File upload with validation, storage, database persistence, UUID generation, hash computation, source URL generation - ✅ `upload_text`: Text content upload with UTF-8 encoding, filename truncation - ✅ Account vs EndUser role assignment - ✅ Custom source URL preservation #### File Validation - ✅ Filename validation (invalid character detection: `/`, `\`, `:`, `*`, `?`, `"`, `<`, `>`, `|`) - ✅ Filename length truncation (>200 characters) - ✅ Extension blacklist checking (`BlockedFileExtensionError`) - ✅ Dataset source file type restrictions (`UnsupportedFileTypeError`) - ✅ File size validation integration #### File Size Limits and Quota Checking - ✅ `is_file_size_within_limit`: Static method testing - Image files (uses `UPLOAD_IMAGE_FILE_SIZE_LIMIT`) - Video files (uses `UPLOAD_VIDEO_FILE_SIZE_LIMIT`) - Audio files (uses `UPLOAD_AUDIO_FILE_SIZE_LIMIT`) - Document/other files (uses `UPLOAD_FILE_SIZE_LIMIT`) - ✅ Boundary testing (at limit, over limit) - ✅ `FileTooLargeError` exception handling #### File Storage Operations - ✅ File save to storage (`storage.save()`) - ✅ File load from storage (`storage.load()`) - ✅ File delete from storage (`storage.delete()`) - ✅ Storage key generation with tenant isolation - ✅ UTF-8 encoding handling for text files #### File Metadata Management - ✅ UploadFile model creation with all attributes - ✅ SHA3-256 hash computation and verification - ✅ Source URL generation (`get_signed_file_url()`) - ✅ Tenant isolation in storage paths - ✅ User role assignment (Account vs EndUser) - ✅ File metadata persistence #### File Retrieval Operations - ✅ `get_file_preview`: Document preview with PREVIEW_WORDS_LIMIT (3000) truncation - ✅ `get_image_preview`: Image preview with signature verification - ✅ `get_file_generator_by_file_id`: File generator with signature verification - ✅ `get_public_image_preview`: Public image preview (no signature) - ✅ `get_file_content`: File content retrieval as UTF-8 string - ✅ NotFound exception handling for missing files - ✅ UnsupportedFileTypeError for invalid file types #### File Deletion - ✅ `delete_file`: File deletion from storage and database - ✅ Transaction management with `session.begin()` - ✅ NotFound handling (no-op if file doesn't exist) #### Constructor and Initialization - ✅ Initialization with Engine - ✅ Initialization with sessionmaker - ✅ AssertionError for invalid initialization (None, wrong type) ### Key Features - **100% Function Coverage**: All methods in `FileService` are tested - **Edge Case Handling**: Tests cover empty inputs, NotFound exceptions, file size limits, invalid filenames - **Security Testing**: Extension blacklist, signature verification, tenant isolation - **Mocking Strategy**: All external dependencies (storage, database session, file helpers, extract processor) are properly mocked - **Factory Pattern**: Uses `FileServiceTestDataFactory` for consistent test data creation - **AAA Pattern**: All tests follow Arrange-Act-Assert structure ### Testing Approach - Mocked storage operations for isolated unit tests - Mocked database session and queries - Mocked file helper functions (signature verification, URL generation) - Mocked ExtractProcessor for file preview generation - Comprehensive error handling tests (NotFound, FileTooLargeError, BlockedFileExtensionError, UnsupportedFileTypeError) - Validation tests for filenames, extensions, file types, and sizes - Tenant isolation verification ## Screenshots N/A (Backend unit tests, no UI changes) ## Checklist - [x] This change requires a documentation update, included: [Dify Document](https://github.com/langgenius/dify-docs) - [x] I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!) - [x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change. - [x] I've updated the documentation accordingly. - [x] I ran `dev/reformat`(backend) and `cd web && npx lint-staged`(frontend) to appease the lint gods ### Related Files - `api/services/file_service.py` - Service under test (246 lines, 10 methods) - `api/tests/unit_tests/services/test_file_service.py` - Test suite (1,625+ lines) - `api/core/file/helpers.py` - File helper functions (get_signed_file_url, verify signatures) - `api/extensions/ext_storage.py` - Storage abstraction (save, load, delete) - `api/models/model.py` - UploadFile model - `api/configs/dify_config.py` - Configuration (file size limits, extension blacklist) - `api/constants.py` - File extension constants (DOCUMENT_EXTENSIONS, IMAGE_EXTENSIONS, etc.) ### Benefits - **Improved Code Reliability**: Comprehensive test coverage ensures file operations work correctly - **Enhanced Security**: Tests validate security controls (extension blacklist, size limits, signature verification) - **Easier Refactoring**: Tests provide confidence when making changes to file upload functionality - **Better Documentation**: Extensive comments serve as documentation for expected behavior - **Regression Prevention**: Tests catch regressions when making changes to file-related code - **Multi-tenancy Validation**: Tests verify tenant isolation is properly enforced - **Data Integrity**: Tests verify file storage, metadata persistence, and deletion operations - **Validation Logic**: Tests confirm all validation rules work correctly Contribution by Gittensor, learn more at https://gittensor.io/
yindo added the pull-request label 2026-02-21 20:50:59 -05:00
yindo closed this issue 2026-02-21 20:50:59 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Knowledge Pipeline
Proposal
WIP
cloud
dependencies
feat:webapp
good first issue
outdated
performance
pull-request
Mirrored from GitHub Pull Request
 python:uv
refactor
size:L
stale
status: accepting prs
status: blocked
status: in discussion
stt & tts
vulnerablity
web
wont-fix
🌊 feat:workflow
🌚 invalid
🐞 bug
👻 feat:rag
💪 enhancement
📚 documentation
🔨 feat:tools
🙋‍♂️ question
🙏 help wanted
🤔 cant-reproduce
🤖 feat:agent
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: langgenius/dify#32226
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?