From 8f2b53275c91c1bd8910fa4fe1ac0c641234d9cb Mon Sep 17 00:00:00 2001 From: QuantumGhost Date: Thu, 18 Sep 2025 02:55:19 +0800 Subject: [PATCH 1/4] fix(api): Remove `postgresql_nulls_not_distinct=False` in unique indexes This option would generate upgrading / table creating sql with `NULLS DISTINCT` part and causing syntax error while running testcontainer tests. The `NULLS DISTINCT` syntax is only supported by PG 15+. --- ...7_1515-68519ad5cd18_knowledge_pipeline_migrate.py | 2 +- api/models/workflow.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/api/migrations/versions/2025_09_17_1515-68519ad5cd18_knowledge_pipeline_migrate.py b/api/migrations/versions/2025_09_17_1515-68519ad5cd18_knowledge_pipeline_migrate.py index c6f8818852..742cfc345a 100644 --- a/api/migrations/versions/2025_09_17_1515-68519ad5cd18_knowledge_pipeline_migrate.py +++ b/api/migrations/versions/2025_09_17_1515-68519ad5cd18_knowledge_pipeline_migrate.py @@ -156,7 +156,7 @@ def upgrade(): sa.Column('type', sa.String(20), nullable=False), sa.Column('file_id', models.types.StringUUID(), nullable=False), sa.PrimaryKeyConstraint('id', name=op.f('workflow_node_execution_offload_pkey')), - sa.UniqueConstraint('node_execution_id', 'type', name=op.f('workflow_node_execution_offload_node_execution_id_key'), postgresql_nulls_not_distinct=False) + sa.UniqueConstraint('node_execution_id', 'type', name=op.f('workflow_node_execution_offload_node_execution_id_key')) ) with op.batch_alter_table('datasets', schema=None) as batch_op: batch_op.add_column(sa.Column('keyword_number', sa.Integer(), server_default=sa.text('10'), nullable=True)) diff --git a/api/models/workflow.py b/api/models/workflow.py index 5f604a51a8..e61005953e 100644 --- a/api/models/workflow.py +++ b/api/models/workflow.py @@ -890,12 +890,18 @@ class WorkflowNodeExecutionModel(Base): # This model is expected to have `offlo class WorkflowNodeExecutionOffload(Base): __tablename__ = "workflow_node_execution_offload" __table_args__ = ( + # PostgreSQL 14 treats NULL values as distinct in unique constraints by default, + # allowing multiple records with NULL values for the same column combination. + # + # This behavior allows us to have multiple records with NULL node_execution_id, + # simplifying garbage collection process. UniqueConstraint( "node_execution_id", "type", - # Treat `NULL` as distinct for this unique index, so - # we can have mutitple records with `NULL` node_exeution_id, simplify garbage collection process. - postgresql_nulls_not_distinct=False, + # Note: PostgreSQL 15+ supports explicit `nulls distinct` behavior through + # `postgresql_nulls_not_distinct=False`, which would make our intention clearer. + # We rely on PostgreSQL's default behavior of treating NULLs as distinct values. + # postgresql_nulls_not_distinct=False, ), ) _HASH_COL_SIZE = 64 From 7fb1a903aebd59783704863078d0feb2cc19d198 Mon Sep 17 00:00:00 2001 From: QuantumGhost Date: Thu, 18 Sep 2025 02:57:38 +0800 Subject: [PATCH 2/4] test(api): fix testcontainer tests for FileService --- .../services/test_file_service.py | 217 ++++++++++-------- 1 file changed, 123 insertions(+), 94 deletions(-) diff --git a/api/tests/test_containers_integration_tests/services/test_file_service.py b/api/tests/test_containers_integration_tests/services/test_file_service.py index 5e5e680a5d..5598c5bc0c 100644 --- a/api/tests/test_containers_integration_tests/services/test_file_service.py +++ b/api/tests/test_containers_integration_tests/services/test_file_service.py @@ -4,6 +4,7 @@ from unittest.mock import create_autospec, patch import pytest from faker import Faker +from sqlalchemy import Engine from werkzeug.exceptions import NotFound from configs import dify_config @@ -17,6 +18,12 @@ from services.file_service import FileService class TestFileService: """Integration tests for FileService using testcontainers.""" + @pytest.fixture + def engine(self, db_session_with_containers): + bind = db_session_with_containers.get_bind() + assert isinstance(bind, Engine) + return bind + @pytest.fixture def mock_external_service_dependencies(self): """Mock setup for external service dependencies.""" @@ -156,7 +163,7 @@ class TestFileService: return upload_file # Test upload_file method - def test_upload_file_success(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_success(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test successful file upload with valid parameters. """ @@ -167,7 +174,7 @@ class TestFileService: content = b"test file content" mimetype = "application/pdf" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -187,13 +194,9 @@ class TestFileService: # Verify storage was called mock_external_service_dependencies["storage"].save.assert_called_once() - # Verify database state - from extensions.ext_database import db - - db.session.refresh(upload_file) assert upload_file.id is not None - def test_upload_file_with_end_user(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_with_end_user(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test file upload with end user instead of account. """ @@ -204,7 +207,7 @@ class TestFileService: content = b"test image content" mimetype = "image/jpeg" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -215,7 +218,9 @@ class TestFileService: assert upload_file.created_by == end_user.id assert upload_file.created_by_role == CreatorUserRole.END_USER.value - def test_upload_file_with_datasets_source(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_with_datasets_source( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test file upload with datasets source parameter. """ @@ -226,7 +231,7 @@ class TestFileService: content = b"test file content" mimetype = "application/pdf" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -239,7 +244,7 @@ class TestFileService: assert upload_file.source_url == "https://example.com/source" def test_upload_file_invalid_filename_characters( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file upload with invalid filename characters. @@ -252,14 +257,16 @@ class TestFileService: mimetype = "text/plain" with pytest.raises(ValueError, match="Filename contains invalid characters"): - FileService.upload_file( + FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, user=account, ) - def test_upload_file_filename_too_long(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_filename_too_long( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test file upload with filename that exceeds length limit. """ @@ -272,7 +279,7 @@ class TestFileService: content = b"test content" mimetype = "text/plain" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -288,7 +295,7 @@ class TestFileService: assert len(base_name) <= 200 def test_upload_file_datasets_unsupported_type( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file upload for datasets with unsupported file type. @@ -301,7 +308,7 @@ class TestFileService: mimetype = "image/jpeg" with pytest.raises(UnsupportedFileTypeError): - FileService.upload_file( + FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -309,7 +316,7 @@ class TestFileService: source="datasets", ) - def test_upload_file_too_large(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_too_large(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test file upload with file size exceeding limit. """ @@ -322,7 +329,7 @@ class TestFileService: mimetype = "image/jpeg" with pytest.raises(FileTooLargeError): - FileService.upload_file( + FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -331,7 +338,7 @@ class TestFileService: # Test is_file_size_within_limit method def test_is_file_size_within_limit_image_success( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for image files within limit. @@ -339,12 +346,12 @@ class TestFileService: extension = "jpg" file_size = dify_config.UPLOAD_IMAGE_FILE_SIZE_LIMIT * 1024 * 1024 # Exactly at limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True def test_is_file_size_within_limit_video_success( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for video files within limit. @@ -352,12 +359,12 @@ class TestFileService: extension = "mp4" file_size = dify_config.UPLOAD_VIDEO_FILE_SIZE_LIMIT * 1024 * 1024 # Exactly at limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True def test_is_file_size_within_limit_audio_success( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for audio files within limit. @@ -365,12 +372,12 @@ class TestFileService: extension = "mp3" file_size = dify_config.UPLOAD_AUDIO_FILE_SIZE_LIMIT * 1024 * 1024 # Exactly at limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True def test_is_file_size_within_limit_document_success( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for document files within limit. @@ -378,12 +385,12 @@ class TestFileService: extension = "pdf" file_size = dify_config.UPLOAD_FILE_SIZE_LIMIT * 1024 * 1024 # Exactly at limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True def test_is_file_size_within_limit_image_exceeded( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for image files exceeding limit. @@ -391,12 +398,12 @@ class TestFileService: extension = "jpg" file_size = dify_config.UPLOAD_IMAGE_FILE_SIZE_LIMIT * 1024 * 1024 + 1 # Exceeds limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is False def test_is_file_size_within_limit_unknown_extension( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file size check for unknown file extension. @@ -404,12 +411,12 @@ class TestFileService: extension = "xyz" file_size = dify_config.UPLOAD_FILE_SIZE_LIMIT * 1024 * 1024 # Uses default limit - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True # Test upload_text method - def test_upload_text_success(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_text_success(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test successful text upload. """ @@ -422,21 +429,25 @@ class TestFileService: mock_current_user.current_tenant_id = str(fake.uuid4()) mock_current_user.id = str(fake.uuid4()) - with patch("services.file_service.current_user", mock_current_user): - upload_file = FileService.upload_text(text=text, text_name=text_name) + upload_file = FileService(engine).upload_text( + text=text, + text_name=text_name, + user_id=mock_current_user.id, + tenant_id=mock_current_user.current_tenant_id, + ) - assert upload_file is not None - assert upload_file.name == text_name - assert upload_file.size == len(text) - assert upload_file.extension == "txt" - assert upload_file.mime_type == "text/plain" - assert upload_file.used is True - assert upload_file.used_by == mock_current_user.id + assert upload_file is not None + assert upload_file.name == text_name + assert upload_file.size == len(text) + assert upload_file.extension == "txt" + assert upload_file.mime_type == "text/plain" + assert upload_file.used is True + assert upload_file.used_by == mock_current_user.id - # Verify storage was called - mock_external_service_dependencies["storage"].save.assert_called_once() + # Verify storage was called + mock_external_service_dependencies["storage"].save.assert_called_once() - def test_upload_text_name_too_long(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_text_name_too_long(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test text upload with name that exceeds length limit. """ @@ -449,15 +460,19 @@ class TestFileService: mock_current_user.current_tenant_id = str(fake.uuid4()) mock_current_user.id = str(fake.uuid4()) - with patch("services.file_service.current_user", mock_current_user): - upload_file = FileService.upload_text(text=text, text_name=long_name) + upload_file = FileService(engine).upload_text( + text=text, + text_name=long_name, + user_id=mock_current_user.id, + tenant_id=mock_current_user.current_tenant_id, + ) - # Verify name was truncated - assert len(upload_file.name) <= 200 - assert upload_file.name == "a" * 200 + # Verify name was truncated + assert len(upload_file.name) <= 200 + assert upload_file.name == "a" * 200 # Test get_file_preview method - def test_get_file_preview_success(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_file_preview_success(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test successful file preview generation. """ @@ -473,12 +488,14 @@ class TestFileService: db.session.commit() - result = FileService.get_file_preview(file_id=upload_file.id) + result = FileService(engine).get_file_preview(file_id=upload_file.id) assert result == "extracted text content" mock_external_service_dependencies["extract_processor"].load_from_upload_file.assert_called_once() - def test_get_file_preview_file_not_found(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_file_preview_file_not_found( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test file preview with non-existent file. """ @@ -486,10 +503,10 @@ class TestFileService: non_existent_id = str(fake.uuid4()) with pytest.raises(NotFound, match="File not found"): - FileService.get_file_preview(file_id=non_existent_id) + FileService(engine).get_file_preview(file_id=non_existent_id) def test_get_file_preview_unsupported_file_type( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file preview with unsupported file type. @@ -507,9 +524,11 @@ class TestFileService: db.session.commit() with pytest.raises(UnsupportedFileTypeError): - FileService.get_file_preview(file_id=upload_file.id) + FileService(engine).get_file_preview(file_id=upload_file.id) - def test_get_file_preview_text_truncation(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_file_preview_text_truncation( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test file preview with text that exceeds preview limit. """ @@ -529,13 +548,13 @@ class TestFileService: long_text = "x" * 5000 # Longer than PREVIEW_WORDS_LIMIT mock_external_service_dependencies["extract_processor"].load_from_upload_file.return_value = long_text - result = FileService.get_file_preview(file_id=upload_file.id) + result = FileService(engine).get_file_preview(file_id=upload_file.id) assert len(result) == 3000 # PREVIEW_WORDS_LIMIT assert result == "x" * 3000 # Test get_image_preview method - def test_get_image_preview_success(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_image_preview_success(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test successful image preview generation. """ @@ -555,7 +574,7 @@ class TestFileService: nonce = "test_nonce" sign = "test_signature" - generator, mime_type = FileService.get_image_preview( + generator, mime_type = FileService(engine).get_image_preview( file_id=upload_file.id, timestamp=timestamp, nonce=nonce, @@ -566,7 +585,9 @@ class TestFileService: assert mime_type == upload_file.mime_type mock_external_service_dependencies["file_helpers"].verify_image_signature.assert_called_once() - def test_get_image_preview_invalid_signature(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_image_preview_invalid_signature( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test image preview with invalid signature. """ @@ -584,14 +605,16 @@ class TestFileService: sign = "invalid_signature" with pytest.raises(NotFound, match="File not found or signature is invalid"): - FileService.get_image_preview( + FileService(engine).get_image_preview( file_id=upload_file.id, timestamp=timestamp, nonce=nonce, sign=sign, ) - def test_get_image_preview_file_not_found(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_image_preview_file_not_found( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test image preview with non-existent file. """ @@ -603,7 +626,7 @@ class TestFileService: sign = "test_signature" with pytest.raises(NotFound, match="File not found or signature is invalid"): - FileService.get_image_preview( + FileService(engine).get_image_preview( file_id=non_existent_id, timestamp=timestamp, nonce=nonce, @@ -611,7 +634,7 @@ class TestFileService: ) def test_get_image_preview_unsupported_file_type( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test image preview with non-image file type. @@ -633,7 +656,7 @@ class TestFileService: sign = "test_signature" with pytest.raises(UnsupportedFileTypeError): - FileService.get_image_preview( + FileService(engine).get_image_preview( file_id=upload_file.id, timestamp=timestamp, nonce=nonce, @@ -642,7 +665,7 @@ class TestFileService: # Test get_file_generator_by_file_id method def test_get_file_generator_by_file_id_success( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test successful file generator retrieval. @@ -657,7 +680,7 @@ class TestFileService: nonce = "test_nonce" sign = "test_signature" - generator, file_obj = FileService.get_file_generator_by_file_id( + generator, file_obj = FileService(engine).get_file_generator_by_file_id( file_id=upload_file.id, timestamp=timestamp, nonce=nonce, @@ -665,11 +688,11 @@ class TestFileService: ) assert generator is not None - assert file_obj == upload_file + assert file_obj.id == upload_file.id mock_external_service_dependencies["file_helpers"].verify_file_signature.assert_called_once() def test_get_file_generator_by_file_id_invalid_signature( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file generator retrieval with invalid signature. @@ -688,7 +711,7 @@ class TestFileService: sign = "invalid_signature" with pytest.raises(NotFound, match="File not found or signature is invalid"): - FileService.get_file_generator_by_file_id( + FileService(engine).get_file_generator_by_file_id( file_id=upload_file.id, timestamp=timestamp, nonce=nonce, @@ -696,7 +719,7 @@ class TestFileService: ) def test_get_file_generator_by_file_id_file_not_found( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file generator retrieval with non-existent file. @@ -709,7 +732,7 @@ class TestFileService: sign = "test_signature" with pytest.raises(NotFound, match="File not found or signature is invalid"): - FileService.get_file_generator_by_file_id( + FileService(engine).get_file_generator_by_file_id( file_id=non_existent_id, timestamp=timestamp, nonce=nonce, @@ -717,7 +740,9 @@ class TestFileService: ) # Test get_public_image_preview method - def test_get_public_image_preview_success(self, db_session_with_containers, mock_external_service_dependencies): + def test_get_public_image_preview_success( + self, db_session_with_containers, engine, mock_external_service_dependencies + ): """ Test successful public image preview generation. """ @@ -733,14 +758,14 @@ class TestFileService: db.session.commit() - generator, mime_type = FileService.get_public_image_preview(file_id=upload_file.id) + generator, mime_type = FileService(engine).get_public_image_preview(file_id=upload_file.id) assert generator is not None assert mime_type == upload_file.mime_type mock_external_service_dependencies["storage"].load.assert_called_once() def test_get_public_image_preview_file_not_found( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test public image preview with non-existent file. @@ -749,10 +774,10 @@ class TestFileService: non_existent_id = str(fake.uuid4()) with pytest.raises(NotFound, match="File not found or signature is invalid"): - FileService.get_public_image_preview(file_id=non_existent_id) + FileService(engine).get_public_image_preview(file_id=non_existent_id) def test_get_public_image_preview_unsupported_file_type( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test public image preview with non-image file type. @@ -770,10 +795,10 @@ class TestFileService: db.session.commit() with pytest.raises(UnsupportedFileTypeError): - FileService.get_public_image_preview(file_id=upload_file.id) + FileService(engine).get_public_image_preview(file_id=upload_file.id) # Test edge cases and boundary conditions - def test_upload_file_empty_content(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_empty_content(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test file upload with empty content. """ @@ -784,7 +809,7 @@ class TestFileService: content = b"" mimetype = "text/plain" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -795,7 +820,7 @@ class TestFileService: assert upload_file.size == 0 def test_upload_file_special_characters_in_name( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file upload with special characters in filename (but valid ones). @@ -807,7 +832,7 @@ class TestFileService: content = b"test content" mimetype = "text/plain" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -818,7 +843,7 @@ class TestFileService: assert upload_file.name == filename def test_upload_file_different_case_extensions( - self, db_session_with_containers, mock_external_service_dependencies + self, db_session_with_containers, engine, mock_external_service_dependencies ): """ Test file upload with different case extensions. @@ -830,7 +855,7 @@ class TestFileService: content = b"test content" mimetype = "application/pdf" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -840,7 +865,7 @@ class TestFileService: assert upload_file is not None assert upload_file.extension == "pdf" # Should be converted to lowercase - def test_upload_text_empty_text(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_text_empty_text(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test text upload with empty text. """ @@ -853,13 +878,17 @@ class TestFileService: mock_current_user.current_tenant_id = str(fake.uuid4()) mock_current_user.id = str(fake.uuid4()) - with patch("services.file_service.current_user", mock_current_user): - upload_file = FileService.upload_text(text=text, text_name=text_name) + upload_file = FileService(engine).upload_text( + text=text, + text_name=text_name, + user_id=mock_current_user.id, + tenant_id=mock_current_user.current_tenant_id, + ) - assert upload_file is not None - assert upload_file.size == 0 + assert upload_file is not None + assert upload_file.size == 0 - def test_file_size_limits_edge_cases(self, db_session_with_containers, mock_external_service_dependencies): + def test_file_size_limits_edge_cases(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test file size limits with edge case values. """ @@ -871,15 +900,15 @@ class TestFileService: ("pdf", dify_config.UPLOAD_FILE_SIZE_LIMIT), ]: file_size = limit_config * 1024 * 1024 - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is True # Test one byte over limit file_size = limit_config * 1024 * 1024 + 1 - result = FileService.is_file_size_within_limit(extension=extension, file_size=file_size) + result = FileService(engine).is_file_size_within_limit(extension=extension, file_size=file_size) assert result is False - def test_upload_file_with_source_url(self, db_session_with_containers, mock_external_service_dependencies): + def test_upload_file_with_source_url(self, db_session_with_containers, engine, mock_external_service_dependencies): """ Test file upload with source URL that gets overridden by signed URL. """ @@ -891,7 +920,7 @@ class TestFileService: mimetype = "application/pdf" source_url = "https://original-source.com/file.pdf" - upload_file = FileService.upload_file( + upload_file = FileService(engine).upload_file( filename=filename, content=content, mimetype=mimetype, @@ -904,7 +933,7 @@ class TestFileService: # The signed URL should only be set when source_url is empty # Let's test that scenario - upload_file2 = FileService.upload_file( + upload_file2 = FileService(engine).upload_file( filename="test2.pdf", content=b"test content 2", mimetype="application/pdf", From fda15ef018112453e1746abad2443cef7ff1fb10 Mon Sep 17 00:00:00 2001 From: QuantumGhost Date: Thu, 18 Sep 2025 02:58:07 +0800 Subject: [PATCH 3/4] test(api): Fix testscontainer tests for WorkflowDraftVariableService --- .../services/test_workflow_draft_variable_service.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/tests/test_containers_integration_tests/services/test_workflow_draft_variable_service.py b/api/tests/test_containers_integration_tests/services/test_workflow_draft_variable_service.py index d73fb7e4be..ee155021e3 100644 --- a/api/tests/test_containers_integration_tests/services/test_workflow_draft_variable_service.py +++ b/api/tests/test_containers_integration_tests/services/test_workflow_draft_variable_service.py @@ -108,6 +108,7 @@ class TestWorkflowDraftVariableService: created_by=app.created_by, environment_variables=[], conversation_variables=[], + rag_pipeline_variables=[], ) from extensions.ext_database import db From a678dd1a324340daf699dd1b32cba58e4dfd73cb Mon Sep 17 00:00:00 2001 From: QuantumGhost Date: Thu, 18 Sep 2025 03:00:57 +0800 Subject: [PATCH 4/4] WIP: test(api): fix broken tests for WebsiteService --- .../services/test_website_service.py | 137 +++++------------- 1 file changed, 39 insertions(+), 98 deletions(-) diff --git a/api/tests/test_containers_integration_tests/services/test_website_service.py b/api/tests/test_containers_integration_tests/services/test_website_service.py index 5ac9ce820a..897e31c88a 100644 --- a/api/tests/test_containers_integration_tests/services/test_website_service.py +++ b/api/tests/test_containers_integration_tests/services/test_website_service.py @@ -5,6 +5,7 @@ import pytest from faker import Faker from models.account import Account, Tenant, TenantAccountJoin, TenantAccountRole +from services.datasource_provider_service import DatasourceProviderService from services.website_service import ( CrawlOptions, ScrapeRequest, @@ -21,19 +22,27 @@ class TestWebsiteService: def mock_external_service_dependencies(self): """Mock setup for external service dependencies.""" with ( - patch("services.website_service.ApiKeyAuthService") as mock_api_key_auth_service, + # patch("services.website_service.ApiKeyAuthService") as mock_api_key_auth_service, patch("services.website_service.FirecrawlApp") as mock_firecrawl_app, patch("services.website_service.WaterCrawlProvider") as mock_watercrawl_provider, patch("services.website_service.requests") as mock_requests, patch("services.website_service.redis_client") as mock_redis_client, patch("services.website_service.storage") as mock_storage, patch("services.website_service.encrypter") as mock_encrypter, + patch( + "services.website_service.DatasourceProviderService", + ) as mock_datasource_provider_service, ): # Setup default mock returns - mock_api_key_auth_service.get_auth_credentials.return_value = { - "config": {"api_key": "encrypted_api_key", "base_url": "https://api.example.com"} + # mock_api_key_auth_service.get_auth_credentials.return_value = { + # "config": {"api_key": "encrypted_api_key", "base_url": "https://api.example.com"} + # } + mock_datasource_provider_service_instance = MagicMock(spec=DatasourceProviderService) + mock_datasource_provider_service_instance.get_datasource_credentials.return_value = { + "firecrawl_api_key": "firecrawl_api_key", + "api_key": "api_key", } - mock_encrypter.decrypt_token.return_value = "decrypted_api_key" + mock_datasource_provider_service.return_value = mock_datasource_provider_service_instance # Mock FirecrawlApp mock_firecrawl_instance = MagicMock() @@ -85,7 +94,8 @@ class TestWebsiteService: mock_storage.load_once.return_value = None yield { - "api_key_auth_service": mock_api_key_auth_service, + "mock_datasource_provider_service": mock_datasource_provider_service, + "mock_datasource_provider_service_instance": mock_datasource_provider_service_instance, "firecrawl_app": mock_firecrawl_app, "watercrawl_provider": mock_watercrawl_provider, "requests": mock_requests, @@ -250,6 +260,12 @@ class TestWebsiteService: }, ) + mock_provider_instance = mock_external_service_dependencies["mock_datasource_provider_service_instance"] + credential = { + "firecrawl_api_key": "decrypted_api_key", + "base_url": "https://api.example.com", + } + mock_provider_instance.get_datasource_credentials.return_value = credential # Act: Execute crawl operation result = WebsiteService.crawl_url(api_request) @@ -258,13 +274,12 @@ class TestWebsiteService: assert result["status"] == "active" assert result["job_id"] == "test_job_id_123" + mock_provider_instance.get_datasource_credentials.assert_called_once_with( + tenant_id=current_tenant.id, + provider="firecrawl", + plugin_id="langgenius/firecrawl_datasource", + ) # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "firecrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) mock_external_service_dependencies["firecrawl_app"].assert_called_once_with( api_key="decrypted_api_key", base_url="https://api.example.com" ) @@ -304,7 +319,12 @@ class TestWebsiteService: "use_sitemap": False, }, ) - + mock_provider_instance = mock_external_service_dependencies["mock_datasource_provider_service_instance"] + credential = { + "api_key": "decrypted_api_key", + "base_url": "https://api.example.com", + } + mock_provider_instance.get_datasource_credentials.return_value = credential # Act: Execute crawl operation result = WebsiteService.crawl_url(api_request) @@ -314,11 +334,10 @@ class TestWebsiteService: assert result["job_id"] == "watercrawl_job_123" # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "watercrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" + mock_provider_instance.get_datasource_credentials.assert_called_once_with( + tenant_id=current_tenant.id, + provider="watercrawl", + plugin_id="langgenius/watercrawl_datasource", ) mock_external_service_dependencies["watercrawl_provider"].assert_called_once_with( api_key="decrypted_api_key", base_url="https://api.example.com" @@ -365,14 +384,6 @@ class TestWebsiteService: assert result["status"] == "active" assert result["data"] is not None - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "jinareader" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify HTTP request was made mock_external_service_dependencies["requests"].get.assert_called_once_with( "https://r.jina.ai/https://example.com", @@ -442,14 +453,6 @@ class TestWebsiteService: assert "data" in result assert "time_consuming" in result - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "firecrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify Redis cache was accessed and cleaned up mock_external_service_dependencies["redis_client"].get.assert_called_once() mock_external_service_dependencies["redis_client"].delete.assert_called_once() @@ -486,14 +489,6 @@ class TestWebsiteService: assert result["current"] == 3 assert "data" in result - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "watercrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - def test_get_crawl_status_jinareader_success(self, db_session_with_containers, mock_external_service_dependencies): """ Test successful crawl status retrieval with JinaReader provider. @@ -527,14 +522,6 @@ class TestWebsiteService: assert "data" in result assert "time_consuming" in result - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "jinareader" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify HTTP request was made mock_external_service_dependencies["requests"].post.assert_called_once() @@ -582,7 +569,9 @@ class TestWebsiteService: with patch("services.website_service.current_user", mock_current_user): # Mock missing credentials - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.return_value = None + mock_external_service_dependencies[ + "mock_datasource_provider_service" + ].get_datasource_credentials.return_value = None # Create API request api_request = WebsiteCrawlStatusApiRequest(provider="firecrawl", job_id="test_job_id_123") @@ -661,14 +650,6 @@ class TestWebsiteService: assert result["description"] == "Test Description" assert result["markdown"] == "# Test Content" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "firecrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify storage was accessed mock_external_service_dependencies["storage"].exists.assert_called_once() mock_external_service_dependencies["storage"].load_once.assert_called_once() @@ -703,14 +684,6 @@ class TestWebsiteService: assert result["description"] == "Test description" assert result["markdown"] == "# Test Content" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "watercrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - def test_get_crawl_url_data_jinareader_success( self, db_session_with_containers, mock_external_service_dependencies ): @@ -751,14 +724,6 @@ class TestWebsiteService: assert result["description"] == "Test description" assert result["content"] == "# Test Content" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "jinareader" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify HTTP request was made mock_external_service_dependencies["requests"].get.assert_called_once_with( "https://r.jina.ai/https://example.com", @@ -802,14 +767,6 @@ class TestWebsiteService: assert result["url"] == "https://example.com" assert result["description"] == "Page description" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "firecrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify FirecrawlApp was called with correct parameters mock_external_service_dependencies["firecrawl_app"].assert_called_once_with( api_key="decrypted_api_key", base_url="https://api.example.com" @@ -847,14 +804,6 @@ class TestWebsiteService: assert result["content"] == "Test content" assert result["url"] == "https://example.com" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "watercrawl" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify WaterCrawlProvider was called with correct parameters mock_external_service_dependencies["watercrawl_provider"].assert_called_once_with( api_key="decrypted_api_key", base_url="https://api.example.com" @@ -1032,14 +981,6 @@ class TestWebsiteService: assert result["status"] == "active" assert result["job_id"] == "jina_job_123" - # Verify external service interactions - mock_external_service_dependencies["api_key_auth_service"].get_auth_credentials.assert_called_once_with( - account.current_tenant.id, "website", "jinareader" - ) - mock_external_service_dependencies["encrypter"].decrypt_token.assert_called_once_with( - tenant_id=account.current_tenant.id, token="encrypted_api_key" - ) - # Verify HTTP POST request was made for sub-page crawling mock_external_service_dependencies["requests"].post.assert_called_once_with( "https://adaptivecrawl-kir3wx7b3a-uc.a.run.app",