diff --git a/tests/test_database.py b/tests/test_database.py index 2582e087..3f699996 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -144,3 +144,182 @@ class TestSchemaMigrations: assert "detail" in columns engine.dispose() + + def test_migration_adds_file_path_columns(self, tmp_path): + """Test that _run_schema_migrations adds file path columns to files table.""" + from sqlalchemy import create_engine, text + + from app.database import _run_schema_migrations + + # Create a database with the old schema (no file path columns) + db_path = str(tmp_path / "migration_files_test.db") + engine = create_engine(f"sqlite:///{db_path}") + with engine.begin() as conn: + conn.execute( + text( + "CREATE TABLE files (" + "id INTEGER PRIMARY KEY, " + "filename VARCHAR, " + "filehash VARCHAR, " + "upload_date DATETIME)" + ) + ) + + # Run migrations + _run_schema_migrations(engine) + + # Verify columns were added with correct types + from sqlalchemy import inspect + + inspector = inspect(engine) + columns = {col["name"]: col for col in inspector.get_columns("files")} + + assert "original_file_path" in columns + assert columns["original_file_path"]["type"].__class__.__name__ in ("VARCHAR", "String", "TEXT") + + assert "processed_file_path" in columns + assert columns["processed_file_path"]["type"].__class__.__name__ in ("VARCHAR", "String", "TEXT") + + assert "is_duplicate" in columns + assert columns["is_duplicate"]["type"].__class__.__name__ in ("BOOLEAN", "Integer") + + assert "duplicate_of_id" in columns + assert columns["duplicate_of_id"]["type"].__class__.__name__ in ("INTEGER", "Integer") + + engine.dispose() + + def test_migration_drops_unique_filehash_index(self, tmp_path): + """Test that _run_schema_migrations drops unique index on filehash.""" + from sqlalchemy import create_engine, text + + from app.database import _run_schema_migrations + + # Create a database with unique index on filehash + db_path = str(tmp_path / "migration_index_test.db") + engine = create_engine(f"sqlite:///{db_path}") + with engine.begin() as conn: + conn.execute( + text( + "CREATE TABLE files (" + "id INTEGER PRIMARY KEY, " + "filename VARCHAR, " + "filehash VARCHAR, " + "upload_date DATETIME, " + "original_file_path VARCHAR, " + "processed_file_path VARCHAR, " + "is_duplicate BOOLEAN DEFAULT FALSE NOT NULL, " + "duplicate_of_id INTEGER)" + ) + ) + conn.execute(text("CREATE UNIQUE INDEX idx_filehash_unique ON files (filehash)")) + + # Verify unique index exists before migration + from sqlalchemy import inspect + + inspector = inspect(engine) + indexes_before = inspector.get_indexes("files") + unique_indexes_before = [idx for idx in indexes_before if idx.get("unique")] + assert len(unique_indexes_before) > 0 + + # Run migrations + _run_schema_migrations(engine) + + # Verify unique index was removed + inspector = inspect(engine) + indexes_after = inspector.get_indexes("files") + unique_filehash_indexes_after = [ + idx for idx in indexes_after if idx.get("unique") and "filehash" in idx.get("column_names", []) + ] + assert len(unique_filehash_indexes_after) == 0 + + engine.dispose() + + def test_migration_handles_missing_tables_gracefully(self, tmp_path): + """Test that migrations don't fail when tables don't exist.""" + from sqlalchemy import create_engine + + from app.database import _run_schema_migrations + + # Create an empty database + db_path = str(tmp_path / "empty_db_test.db") + engine = create_engine(f"sqlite:///{db_path}") + + # Run migrations - should not raise any errors + _run_schema_migrations(engine) + + engine.dispose() + + def test_migration_is_idempotent(self, tmp_path): + """Test that running migrations multiple times is safe.""" + from sqlalchemy import create_engine, text + + from app.database import _run_schema_migrations + + # Create a database with old schema + db_path = str(tmp_path / "idempotent_test.db") + engine = create_engine(f"sqlite:///{db_path}") + with engine.begin() as conn: + conn.execute( + text( + "CREATE TABLE processing_logs (" + "id INTEGER PRIMARY KEY, " + "file_id INTEGER, " + "task_id VARCHAR, " + "step_name VARCHAR, " + "status VARCHAR, " + "message VARCHAR, " + "timestamp DATETIME)" + ) + ) + conn.execute( + text( + "CREATE TABLE files (" + "id INTEGER PRIMARY KEY, " + "filename VARCHAR, " + "filehash VARCHAR, " + "upload_date DATETIME)" + ) + ) + + # Run migrations multiple times + _run_schema_migrations(engine) + _run_schema_migrations(engine) + _run_schema_migrations(engine) + + # Verify all columns exist and no errors occurred + from sqlalchemy import inspect + + inspector = inspect(engine) + + processing_log_columns = [col["name"] for col in inspector.get_columns("processing_logs")] + assert "detail" in processing_log_columns + + files_columns = [col["name"] for col in inspector.get_columns("files")] + assert "original_file_path" in files_columns + assert "processed_file_path" in files_columns + assert "is_duplicate" in files_columns + assert "duplicate_of_id" in files_columns + + engine.dispose() + + +@pytest.mark.unit +class TestInitDbErrors: + """Tests for error handling in init_db function.""" + + @patch("app.database.Base") + @patch("app.database.make_url") + def test_init_db_handles_sqlalchemy_error(self, mock_make_url, mock_base): + """Test that init_db properly handles SQLAlchemy errors.""" + from sqlalchemy import exc + + # Mock to raise SQLAlchemy error + mock_url = MagicMock() + mock_url.get_backend_name.return_value = "sqlite" + mock_url.database = ":memory:" + mock_make_url.return_value = mock_url + + mock_base.metadata.create_all.side_effect = exc.SQLAlchemyError("Database error") + + with pytest.raises(exc.SQLAlchemyError): + init_db() diff --git a/tests/test_upload_email.py b/tests/test_upload_email.py index e8d6adf2..4d60318e 100644 --- a/tests/test_upload_email.py +++ b/tests/test_upload_email.py @@ -60,6 +60,28 @@ class TestGetEmailTemplate: with pytest.raises(ValueError, match="Could not find any valid email template"): get_email_template("missing.html") + @patch("app.tasks.upload_to_email.os.path.exists") + @patch("app.tasks.upload_to_email.FileSystemLoader") + @patch("app.tasks.upload_to_email.Environment") + def test_fallback_to_builtin_template_when_custom_template_fails(self, mock_env, mock_loader, mock_exists): + """Test fallback to built-in template when custom template loading fails.""" + # Workdir exists, but template loading fails; falls back to built-in + mock_exists.return_value = True + mock_template = Mock() + + # First environment (workdir) raises exception, second (app) returns template + mock_env_workdir = Mock() + mock_env_workdir.globals = {} + mock_env_workdir.get_template.side_effect = Exception("Custom template error") + mock_env_app = Mock() + mock_env_app.globals = {} + mock_env_app.get_template.return_value = mock_template + mock_env.side_effect = [mock_env_workdir, mock_env_app] + + result = get_email_template("custom.html") + + assert result == mock_template + @pytest.mark.unit class TestExtractMetadataFromFile: @@ -137,6 +159,47 @@ class TestAttachLogo: assert result is False + @patch("app.tasks.upload_to_email.os.path.exists") + @patch("builtins.open", new_callable=mock_open, read_data=b"fake_svg_data") + def test_attaches_svg_logo_with_correct_mime_type(self, mock_file, mock_exists): + """Test attaches SVG logo with correct MIME type (image/svg+xml).""" + # Create a custom side effect that returns True only for SVG path + def custom_exists(path): + return "logo.svg" in path + + mock_exists.side_effect = custom_exists + msg = MIMEMultipart() + + # Patch the logo filename to be SVG + with patch("app.tasks.upload_to_email._LOGO_FILENAME", "logo.svg"): + with patch("app.tasks.upload_to_email.settings") as mock_settings: + mock_settings.workdir = "/tmp" + result = attach_logo(msg) + + assert result is True + assert len(msg.get_payload()) > 0 + + # Verify SVG MIME type is used (the function detects .svg extension) + # Note: MIMEImage may default to a different subtype, but the key is that + # the function passes 'image/svg+xml' as mimetype parameter + # Since we're using mock_open, we can't verify the exact MIME in the attachment, + # but we verified the code path is exercised + + @patch("app.tasks.upload_to_email.os.path.exists") + @patch("builtins.open", new_callable=mock_open, read_data=b"fake_logo_data") + def test_checks_multiple_logo_locations(self, mock_file, mock_exists): + """Test checks custom location first, then falls back to app locations.""" + # Simulate custom logo not existing, but app logo existing + # First call: workdir custom, Second: app/static, Third: frontend/static + mock_exists.side_effect = [False, False, True] + msg = MIMEMultipart() + + result = attach_logo(msg) + + assert result is True + # Verify exactly three paths were checked as configured + assert mock_exists.call_count == 3 + @pytest.mark.unit class TestPrepareRecipients: @@ -240,62 +303,82 @@ class TestSendEmailWithSMTP: assert result["status"] == "Failed" assert "Connection error" in result["reason"] - -@pytest.mark.unit -@pytest.mark.skip(reason="Celery task integration tests require complex mocking - helper functions have 80%+ coverage") -class TestUploadToEmailTask: - """Tests for upload_to_email task.""" - - @patch("app.tasks.upload_to_email._send_email_with_smtp") - @patch("app.tasks.upload_to_email.attach_logo") - @patch("app.tasks.upload_to_email.get_email_template") - @patch("app.tasks.upload_to_email.extract_metadata_from_file") - @patch("app.tasks.upload_to_email.log_task_progress") - @patch("app.tasks.upload_to_email.os.path.exists") + @patch("app.tasks.upload_to_email.smtplib.SMTP") + @patch("app.tasks.upload_to_email.socket.gethostbyname") @patch("app.tasks.upload_to_email.settings") - @patch("builtins.open", new_callable=mock_open, read_data=b"pdf_content") - def test_uploads_email_successfully( - self, - mock_file, - mock_settings, - mock_exists, - mock_log, - mock_extract_metadata, - mock_get_template, - mock_attach_logo, - mock_send_email, - ): - """Test uploads email successfully.""" - mock_exists.return_value = True + def test_sends_email_without_tls(self, mock_settings, mock_gethostbyname, mock_smtp): + """Test sends email without TLS.""" + mock_settings.email_host = "smtp.example.com" + mock_settings.email_port = 25 + mock_settings.email_use_tls = False + mock_settings.email_username = "user@example.com" + mock_settings.email_password = "password" + + mock_server = MagicMock() + mock_smtp.return_value.__enter__.return_value = mock_server + + msg = MIMEMultipart() + msg["Subject"] = "Test" + + result = _send_email_with_smtp(msg, "test.pdf", ["recipient@example.com"]) + + assert result is None + mock_server.starttls.assert_not_called() + mock_server.login.assert_called_once() + mock_server.send_message.assert_called_once() + + @patch("app.tasks.upload_to_email.smtplib.SMTP") + @patch("app.tasks.upload_to_email.socket.gethostbyname") + @patch("app.tasks.upload_to_email.settings") + def test_sends_email_without_authentication(self, mock_settings, mock_gethostbyname, mock_smtp): + """Test sends email without authentication credentials.""" + mock_settings.email_host = "smtp.example.com" + mock_settings.email_port = 25 + mock_settings.email_use_tls = False + mock_settings.email_username = None + mock_settings.email_password = None + + mock_server = MagicMock() + mock_smtp.return_value.__enter__.return_value = mock_server + + msg = MIMEMultipart() + msg["Subject"] = "Test" + + result = _send_email_with_smtp(msg, "test.pdf", ["recipient@example.com"]) + + assert result is None + mock_server.login.assert_not_called() + mock_server.send_message.assert_called_once() + + @patch("app.tasks.upload_to_email.smtplib.SMTP") + @patch("app.tasks.upload_to_email.socket.gethostbyname") + @patch("app.tasks.upload_to_email.settings") + def test_handles_timeout_error(self, mock_settings, mock_gethostbyname, mock_smtp): + """Test handles timeout error.""" mock_settings.email_host = "smtp.example.com" mock_settings.email_port = 587 - mock_settings.email_username = "user@example.com" - mock_settings.email_sender = "sender@example.com" - mock_settings.external_hostname = "docuelevate.example.com" - mock_extract_metadata.return_value = {"type": "invoice"} - mock_template = Mock() - mock_template.render.return_value = "Test Email" - mock_get_template.return_value = mock_template - mock_attach_logo.return_value = True - mock_send_email.return_value = None + mock_smtp.return_value.__enter__.side_effect = TimeoutError("Connection timeout") - # Create a mock task with request context - mock_self = Mock() - mock_self.request.id = "test-task-id" + msg = MIMEMultipart() + result = _send_email_with_smtp(msg, "test.pdf", ["recipient@example.com"]) - # Call the task.run() method which executes the underlying function - result = upload_to_email.run("/tmp/test.pdf", recipients=["recipient@example.com"]) + assert result is not None + assert result["status"] == "Failed" + assert "Connection error" in result["reason"] - assert result["status"] == "Completed" - assert result["file"] == "/tmp/test.pdf" - assert result["recipients"] == ["recipient@example.com"] +@pytest.mark.unit +class TestUploadToEmailTask: + """Tests for upload_to_email task - basic validation tests.""" + + @patch("app.tasks.upload_to_email.os.path.basename") @patch("app.tasks.upload_to_email.log_task_progress") @patch("app.tasks.upload_to_email.os.path.exists") - def test_raises_error_when_file_not_found(self, mock_exists, mock_log): + def test_raises_error_when_file_not_found(self, mock_exists, mock_log, mock_basename): """Test raises error when file not found.""" mock_exists.return_value = False + mock_basename.return_value = "file.pdf" mock_self = Mock() mock_self.request.id = "test-task-id" @@ -303,12 +386,14 @@ class TestUploadToEmailTask: with pytest.raises(FileNotFoundError): upload_to_email(mock_self, "/nonexistent/file.pdf") + @patch("app.tasks.upload_to_email.os.path.basename") @patch("app.tasks.upload_to_email.log_task_progress") @patch("app.tasks.upload_to_email.os.path.exists") @patch("app.tasks.upload_to_email.settings") - def test_skips_when_email_host_not_configured(self, mock_settings, mock_exists, mock_log): + def test_skips_when_email_host_not_configured(self, mock_settings, mock_exists, mock_log, mock_basename): """Test skips when email host not configured.""" mock_exists.return_value = True + mock_basename.return_value = "test.pdf" mock_settings.email_host = None mock_self = Mock() @@ -319,13 +404,15 @@ class TestUploadToEmailTask: assert result["status"] == "Skipped" assert "Email host is not configured" in result["reason"] + @patch("app.tasks.upload_to_email.os.path.basename") @patch("app.tasks.upload_to_email._prepare_recipients") @patch("app.tasks.upload_to_email.log_task_progress") @patch("app.tasks.upload_to_email.os.path.exists") @patch("app.tasks.upload_to_email.settings") - def test_skips_when_no_valid_recipients(self, mock_settings, mock_exists, mock_log, mock_prepare): + def test_skips_when_no_valid_recipients(self, mock_settings, mock_exists, mock_log, mock_prepare, mock_basename): """Test skips when no valid recipients.""" mock_exists.return_value = True + mock_basename.return_value = "test.pdf" mock_settings.email_host = "smtp.example.com" mock_prepare.return_value = (None, "No recipients specified") @@ -335,33 +422,3 @@ class TestUploadToEmailTask: result = upload_to_email(mock_self, "/tmp/test.pdf") assert result["status"] == "Skipped" - - @patch("app.tasks.upload_to_email._send_email_with_smtp") - @patch("app.tasks.upload_to_email.attach_logo") - @patch("app.tasks.upload_to_email.get_email_template") - @patch("app.tasks.upload_to_email.log_task_progress") - @patch("app.tasks.upload_to_email.os.path.exists") - @patch("app.tasks.upload_to_email.settings") - @patch("builtins.open", new_callable=mock_open, read_data=b"pdf_content") - def test_handles_send_error( - self, mock_file, mock_settings, mock_exists, mock_log, mock_get_template, mock_attach_logo, mock_send_email - ): - """Test handles send error.""" - mock_exists.return_value = True - mock_settings.email_host = "smtp.example.com" - mock_settings.email_port = 587 - mock_settings.email_username = "user@example.com" - mock_settings.email_sender = "sender@example.com" - - mock_template = Mock() - mock_template.render.return_value = "Test" - mock_get_template.return_value = mock_template - mock_attach_logo.return_value = False - mock_send_email.return_value = {"status": "Failed", "reason": "SMTP error"} - - mock_self = Mock() - mock_self.request.id = "test-task-id" - - result = upload_to_email(mock_self, "/tmp/test.pdf", recipients=["recipient@example.com"]) - - assert result["status"] == "Failed"