diff --git a/backend/apps/data_training/api/data_training.py b/backend/apps/data_training/api/data_training.py index ecec66fd6..5cc4c6888 100644 --- a/backend/apps/data_training/api/data_training.py +++ b/backend/apps/data_training/api/data_training.py @@ -3,11 +3,10 @@ import io import os import uuid -from http.client import HTTPException from typing import Optional import pandas as pd -from fastapi import APIRouter, File, UploadFile, Query +from fastapi import APIRouter, File, HTTPException, UploadFile, Query from fastapi.responses import StreamingResponse from apps.chat.models.chat_model import AxisObj diff --git a/backend/apps/settings/api/base.py b/backend/apps/settings/api/base.py index 6de5bdf19..c9c59548e 100644 --- a/backend/apps/settings/api/base.py +++ b/backend/apps/settings/api/base.py @@ -1,7 +1,6 @@ import os -from http.client import HTTPException -from fastapi import APIRouter +from fastapi import APIRouter, HTTPException from fastapi.responses import FileResponse from apps.swagger.i18n import PLACEHOLDER_PREFIX diff --git a/backend/apps/system/crud/user_excel.py b/backend/apps/system/crud/user_excel.py index 914743ec3..80c9e5fbb 100644 --- a/backend/apps/system/crud/user_excel.py +++ b/backend/apps/system/crud/user_excel.py @@ -1,13 +1,13 @@ import asyncio -from http.client import HTTPException import io import sys import tempfile import uuid import atexit import threading +from fastapi import HTTPException from fastapi.responses import StreamingResponse, FileResponse import os from openai import BaseModel diff --git a/backend/apps/terminology/api/terminology.py b/backend/apps/terminology/api/terminology.py index 1384803f2..d14b845fb 100644 --- a/backend/apps/terminology/api/terminology.py +++ b/backend/apps/terminology/api/terminology.py @@ -3,11 +3,10 @@ import io import os import uuid -from http.client import HTTPException from typing import Optional import pandas as pd -from fastapi import APIRouter, File, UploadFile, Query +from fastapi import APIRouter, File, HTTPException, UploadFile, Query from fastapi.responses import StreamingResponse from apps.chat.models.chat_model import AxisObj diff --git a/backend/tests/test_excel_http_errors.py b/backend/tests/test_excel_http_errors.py new file mode 100644 index 000000000..d6e072309 --- /dev/null +++ b/backend/tests/test_excel_http_errors.py @@ -0,0 +1,208 @@ +"""Offline HTTP regressions for TAPD 1078034 using real routers and handlers. + +Identity, audit persistence and database sessions are test adapters. No application +startup, external database, provider or deployed service is used. +""" + +import io +from types import SimpleNamespace +from unittest.mock import AsyncMock, Mock + +import pandas as pd +import pytest +import sqlbot_xpack # noqa: F401 # Initialize the published wheel before API imports. +from fastapi import FastAPI, HTTPException +from fastapi.testclient import TestClient +from starlette.exceptions import HTTPException as StarletteHTTPException + +from apps.data_training.api import data_training +from apps.settings.api import base +from apps.system.api import user +from apps.system.crud import user_excel +from apps.system.schemas.permission import RequestContextMiddleware +from apps.terminology.api import terminology +from common.audit.schemas.logger_decorator import SystemLogger +from common.audit.schemas.request_context import RequestContextMiddlewareCommon +from common.core.db import get_session +from common.core.deps import get_i18n +from common.core.response_middleware import ResponseMiddleware, exception_handler + +UPLOAD_URLS = [ + "/system/terminology/uploadExcel", + "/system/data-training/uploadExcel", + "/user/batchImport", +] + + +@pytest.fixture +def api(monkeypatch, tmp_path): + app = FastAPI() + for module in (terminology, data_training, base, user): + app.include_router(module.router) + app.add_exception_handler( + StarletteHTTPException, exception_handler.http_exception_handler + ) + app.add_exception_handler(Exception, exception_handler.global_exception_handler) + app.add_middleware(ResponseMiddleware) + app.add_middleware(RequestContextMiddleware) + app.add_middleware(RequestContextMiddlewareCommon) + + @app.middleware("http") + async def identity(request, call_next): + request.state.current_user = SimpleNamespace( + isAdmin=True, weight=1, id=1, oid=1, account="test", name="test" + ) + return await call_next(request) + + session = Mock() + app.dependency_overrides[get_session] = lambda: session + app.dependency_overrides[get_i18n] = lambda: lambda key: key + monkeypatch.setattr(SystemLogger, "create_log_record", AsyncMock()) + for module in (terminology, data_training, base): + monkeypatch.setattr(module, "path", str(tmp_path)) + monkeypatch.setattr(user_excel, "_TEMP_FILE_MAP", {}) + monkeypatch.setattr(user_excel.tempfile, "gettempdir", lambda: str(tmp_path)) + # ResponseMiddleware tracks live instances globally in the application. + monkeypatch.setattr(ResponseMiddleware, "instances", []) + with TestClient(app, raise_server_exceptions=False) as client: + yield client, session + + +def workbook(columns): + buffer = io.BytesIO() + pd.DataFrame(columns=columns).to_excel(buffer, index=False) + return buffer.getvalue() + + +@pytest.mark.parametrize("url", UPLOAD_URLS) +@pytest.mark.parametrize("filename", ["bad.csv", "bad", "bad.xlsx.csv"]) +def test_unsupported_upload_returns_400(api, tmp_path, url, filename): + client, session = api + response = client.post(url, files={"file": (filename, b"invalid spreadsheet")}) + assert response.status_code == 400, response.text + assert response.json() == "Only support .xlsx/.xls" + assert list(tmp_path.iterdir()) == [] + session.add_all.assert_not_called() + session.commit.assert_not_called() + + +def test_invalid_user_excel_headers_return_400(api): + client, session = api + response = client.post( + "/user/batchImport", files={"file": ("users.xlsx", workbook(["wrong"]))} + ) + assert response.status_code == 400, response.text + assert response.json() == "Excel header validation failed" + session.add_all.assert_not_called() + session.commit.assert_not_called() + + +def test_valid_empty_user_workbook_is_accepted(api): + client, session = api + response = client.post( + "/user/batchImport", + files={"file": ("users.xlsx", workbook(user_excel.get_i18n_head_list()))}, + ) + assert response.status_code == 200, response.text + assert response.json()["data"] == { + "successCount": 0, + "errorCount": 0, + "dataKey": None, + } + session.commit.assert_not_called() + + +@pytest.mark.parametrize( + "module,batch_name,column_count", + [ + (terminology, "batch_create_terminology", 6), + (data_training, "batch_create_training", 4), + ], +) +@pytest.mark.parametrize("filename", ["valid.xlsx", "valid.XLSX"]) +def test_valid_knowledge_workbook_reaches_batch_import( + api, monkeypatch, module, batch_name, column_count, filename +): + client, session = api + monkeypatch.setattr(module, "session_maker", lambda: session) + batch = Mock( + return_value={ + "failed_records": [], + "success_count": 0, + "duplicate_count": 0, + "original_count": 0, + } + ) + monkeypatch.setattr(module, batch_name, batch) + response = client.post( + module.router.prefix + "/uploadExcel", + files={"file": (filename, workbook(range(column_count)))}, + ) + assert response.status_code == 200, response.text + assert response.json()["data"]["failed_count"] == 0 + batch.assert_called_once() + assert batch.call_args.args[:3] == (session, [], 1) + + +@pytest.mark.parametrize( + "filename,exists,status,message", + [ + ("missing_error.xlsx", False, 404, "File Not Exists"), + ("bad.csv", True, 400, "Only support _error.xlsx"), + ], +) +def test_knowledge_error_download_returns_client_error( + api, tmp_path, filename, exists, status, message +): + client, _session = api + if exists: + (tmp_path / filename).write_bytes(b"test file") + response = client.post("/system/download-fail-info", json={"file": filename}) + assert response.status_code == status, response.text + assert response.json() == message + + +def test_user_error_download_requires_file_id(): + with pytest.raises(HTTPException) as raised: + user_excel.download_error_file("") + assert raised.value.status_code == 400 + assert raised.value.detail == "file_id required" + + +@pytest.mark.parametrize("registered", [False, True]) +def test_missing_user_error_file_returns_404(api, tmp_path, registered): + client, _session = api + if registered: + user_excel._TEMP_FILE_MAP["missing"] = str(tmp_path / "missing.xlsx") + response = client.get("/user/errorRecord/missing") + assert response.status_code == 404, response.text + assert response.json() == "File not found" + + +def test_user_error_file_outside_temp_directory_returns_403(api, tmp_path, monkeypatch): + client, _session = api + monkeypatch.setattr( + user_excel.tempfile, "gettempdir", lambda: str(tmp_path / "allowed") + ) + outside = tmp_path / "outside.xlsx" + outside.write_bytes(b"private data") + user_excel._TEMP_FILE_MAP["outside"] = str(outside) + response = client.get("/user/errorRecord/outside") + assert response.status_code == 403, response.text + assert response.json() == "Unauthorized file access" + assert outside.read_bytes() == b"private data" + + +@pytest.mark.parametrize("kind", ["knowledge", "user"]) +def test_valid_error_file_can_be_downloaded(api, tmp_path, kind): + client, _session = api + content = workbook(["error"]) + target = tmp_path / "valid_error.xlsx" + target.write_bytes(content) + if kind == "knowledge": + response = client.post("/system/download-fail-info", json={"file": target.name}) + else: + user_excel._TEMP_FILE_MAP["valid"] = str(target) + response = client.get("/user/errorRecord/valid") + assert response.status_code == 200, response.text + assert response.content == content