diff --git a/backend/src/api/routes/__init__.py b/backend/src/api/routes/__init__.py index 75a92bf7b..1b6c56dd5 100755 --- a/backend/src/api/routes/__init__.py +++ b/backend/src/api/routes/__init__.py @@ -44,6 +44,7 @@ __all__ = [ "settings", "storage", "tasks", + "tools", "translate", "validation_tasks", ] diff --git a/backend/src/api/routes/mappings.py b/backend/src/api/routes/mappings.py index 7a30aa32f..ede73d8ba 100644 --- a/backend/src/api/routes/mappings.py +++ b/backend/src/api/routes/mappings.py @@ -11,11 +11,11 @@ from fastapi import APIRouter, Depends, HTTPException -from pydantic import BaseModel, ConfigDict +from pydantic import BaseModel, ConfigDict, Field from sqlalchemy.orm import Session from ...core.database import get_db -from ...core.logger import belief_scope +from ...core.logger import belief_scope, logger from ...dependencies import get_config_manager, has_permission from ...models.mapping import DatabaseMapping @@ -110,6 +110,105 @@ async def create_mapping( return new_mapping # #endregion create_mapping +# #region ApplyDatasetDocRequest [TYPE DataClass] +# @ingroup Api +# @BRIEF Request DTO for applying LLM-generated documentation to a dataset. +class ApplyDatasetDocRequest(BaseModel): + env_id: str = "" + description: str | None = None + columns: dict[str, str] = Field(default_factory=dict) +# #endregion ApplyDatasetDocRequest + + +# #region apply_dataset_metadata [C:3] [TYPE Function] [SEMANTICS datasets, metadata, docs, apply] +# @ingroup Api +# @BRIEF Apply LLM-generated documentation (description + column verbose_names) to a dataset. +# @PRE dataset_id is a valid Superset dataset ID. +# @PRE env_id is a valid environment ID. +# @PRE User has plugin:mapper:EXECUTE permission. +# @POST Dataset description and column verbose_names are updated in Superset. +# @SIDE_EFFECT Calls SupersetClient.update_dataset() to persist changes. +# @RELATION DEPENDS_ON -> [SupersetClient] +@router.put("/datasets/{dataset_id}/metadata") +async def apply_dataset_metadata( + dataset_id: int, + request: ApplyDatasetDocRequest, + config_manager=Depends(get_config_manager), + _ = Depends(has_permission("plugin:mapper", "EXECUTE")), +): + with belief_scope("apply_dataset_metadata", f"dataset={dataset_id}, env={request.env_id}"): + if not request.env_id: + logger.explore("Missing env_id in apply_dataset_metadata", error="env_id is required") + raise HTTPException(status_code=400, detail="env_id is required") + + # Get environment config and create Superset client + env_config = config_manager.get_environment(request.env_id) + if not env_config: + logger.explore("Environment not found", payload={"env_id": request.env_id}, error="Environment not found") + raise HTTPException(status_code=404, detail=f"Environment '{request.env_id}' not found") + + from ...core.superset_client import SupersetClient + client = SupersetClient(env_config) + await client.authenticate() + + try: + # Fetch current dataset + dataset_response = await client.get_dataset(dataset_id) + dataset_data = dataset_response.get("result", dataset_response) + + # Build column updates + original_columns = dataset_data.get("columns", []) + updated_columns = [] + changes_made = False + + for column in original_columns: + col_name = column.get("column_name") + new_column = {k: v for k, v in column.items() if v is not None} + + if col_name and col_name in request.columns: + new_desc = request.columns[col_name] + if new_column.get("verbose_name") != new_desc: + new_column["verbose_name"] = new_desc + changes_made = True + + updated_columns.append(new_column) + + # Build update payload + payload = { + "database_id": dataset_data.get("database", {}).get("id"), + "table_name": dataset_data.get("table_name"), + "schema": dataset_data.get("schema"), + "columns": updated_columns, + "owners": [owner["id"] for owner in dataset_data.get("owners", [])], + "metrics": dataset_data.get("metrics", []), + "extra": dataset_data.get("extra"), + } + + # Add description if provided + if ( + request.description is not None + and request.description != dataset_data.get("description") + ): + payload["description"] = request.description + changes_made = True + + # Remove None values + payload = {k: v for k, v in payload.items() if v is not None} + + if changes_made: + await client.update_dataset(dataset_id, payload) + logger.reflect("Dataset metadata updated", payload={"dataset_id": dataset_id}) + return {"status": "success", "dataset_id": dataset_id} + else: + logger.reason("No changes to apply", payload={"dataset_id": dataset_id}) + return {"status": "no_changes", "dataset_id": dataset_id} + + except Exception as e: + logger.explore("Failed to apply dataset metadata", error=str(e)) + raise HTTPException(status_code=500, detail=f"Failed to apply metadata: {e!s}") from e +# #endregion apply_dataset_metadata + + # #region suggest_mappings_api [TYPE Function] # @ingroup Api # @BRIEF Get suggested mappings based on fuzzy matching. diff --git a/backend/src/api/routes/tools.py b/backend/src/api/routes/tools.py new file mode 100644 index 000000000..5b49b263a --- /dev/null +++ b/backend/src/api/routes/tools.py @@ -0,0 +1,154 @@ +# #region ToolsApiRoutes [C:3] [TYPE Module] [SEMANTICS fastapi, tools, mapper, upload, xlsx] +# @defgroup Api Module group. +# @BRIEF API endpoints for tool-specific operations: xlsx upload for mapper, etc. +# @LAYER API +# @RELATION DEPENDS_ON -> [AppDependencies] +# @RELATION DEPENDS_ON -> [LoggerModule] +# @INVARIANT Uploaded files are validated for structure before being accepted. +# @INVARIANT Temp files are cleaned up after task completion (best-effort). +# @RATIONALE File upload needs multipart/form-data which cannot be handled by the JSON-based task API. +# A dedicated endpoint allows proper file validation, temp storage, and error messaging. +# @REJECTED Base64-encoded file data in task params was rejected — it adds latency on large files, +# bypasses server-side file validation before task creation, and complicates the plugin's execute method. + +import os +from pathlib import Path +import uuid + +from fastapi import APIRouter, Depends, File, HTTPException, UploadFile +import pandas as pd + +from ...core.logger import belief_scope, logger +from ...dependencies import has_permission + +router = APIRouter(prefix="/api/tools/mapper", tags=["Tools"]) +MAX_MAPPER_UPLOAD_BYTES = 10 * 1024 * 1024 + +# Temp directory for uploaded mapper files +MAPPER_UPLOAD_DIR = Path("/tmp/ss-tools-mapper-uploads") +MAPPER_UPLOAD_DIR.mkdir(parents=True, exist_ok=True) + + +# #region upload_xlsx_mapping [C:3] [TYPE Function] [SEMANTICS tools, mapper, upload, xlsx, validation] +# @ingroup Api +# @BRIEF Upload and validate an XLSX mapping file. Returns an opaque upload ID for mapper tasks. +# @PRE File is a valid .xlsx with 'column_name' and 'verbose_name' columns. +# @PRE User has plugin:mapper:EXECUTE permission. +# @POST Returns {"upload_id": ""} on success. +# @POST Returns 400 with detail on invalid file (wrong format, missing columns, empty content). +# @SIDE_EFFECT Writes uploaded file to a temporary directory under /tmp/ss-tools-mapper-uploads/. +# @RELATION DEPENDS_ON -> [DatasetMapper] +# @DATA_CONTRACT Input: multipart/form-data (file) -> Output: { upload_id: str } +# @RATIONALE Server-side XLSX validation ensures that only files with the expected structure +# (column_name and verbose_name columns) make it to the mapper task, reducing task failures. +@router.post("/upload-xlsx") +async def upload_xlsx_mapping( + file: UploadFile = File(...), + _ = Depends(has_permission("plugin:mapper", "EXECUTE")), +): + with belief_scope("upload_xlsx_mapping", f"filename={file.filename}"): + # Validate file extension + if not file.filename or not file.filename.lower().endswith(".xlsx"): + logger.explore( + "Invalid file extension", + payload={"filename": file.filename}, + error="Only .xlsx files are accepted", + ) + raise HTTPException(status_code=400, detail="Only .xlsx files are accepted") + + # Read file content with an explicit size limit. + try: + content = await file.read() + except Exception as e: + logger.explore("Failed to read uploaded file", error=str(e)) + raise HTTPException(status_code=400, detail="Failed to read uploaded file") from e + + if len(content) > MAX_MAPPER_UPLOAD_BYTES: + logger.explore( + "Uploaded XLSX exceeds size limit", + payload={"filename": file.filename, "size": len(content)}, + error=f"Maximum size is {MAX_MAPPER_UPLOAD_BYTES} bytes", + ) + raise HTTPException(status_code=413, detail="XLSX file is too large (maximum 10 MB)") + + # Write to temp location for validation + unique_name = f"{uuid.uuid4().hex}.xlsx" + temp_path = MAPPER_UPLOAD_DIR / unique_name + + try: + with open(temp_path, "wb") as f: + f.write(content) + + # Validate with pandas: check structure + df = pd.read_excel(temp_path) + + required_columns = {"column_name", "verbose_name"} + actual_columns = set(df.columns) + missing = required_columns - actual_columns + + if missing: + logger.explore( + "Uploaded XLSX missing required columns", + payload={ + "filename": file.filename, + "missing_columns": list(missing), + "actual_columns": list(actual_columns), + }, + error=f"Missing columns: {', '.join(missing)}", + ) + os.unlink(temp_path) + raise HTTPException( + status_code=400, + detail=f"XLSX file must contain columns: 'column_name' and 'verbose_name'. Missing: {', '.join(missing)}", + ) + + if df.empty: + logger.explore( + "Uploaded XLSX is empty", + payload={"filename": file.filename}, + error="File has no data rows", + ) + os.unlink(temp_path) + raise HTTPException(status_code=400, detail="XLSX file is empty") + + # Validate that verbose_name has non-null values + null_verbose = df["verbose_name"].isna().sum() + if null_verbose == len(df): + logger.explore( + "Uploaded XLSX has no verbose_name values", + payload={"filename": file.filename, "row_count": len(df)}, + error="All verbose_name values are empty", + ) + os.unlink(temp_path) + raise HTTPException( + status_code=400, + detail="XLSX file must have at least one non-empty 'verbose_name' value", + ) + + logger.reflect( + "XLSX file validated and saved", + payload={ + "filename": file.filename, + "path": str(temp_path), + "row_count": len(df), + "column_count": len(df.columns), + }, + ) + + return {"upload_id": unique_name.removesuffix(".xlsx")} + + except HTTPException: + # Re-raise HTTP exceptions (they're already clean) + raise + except Exception as e: + # Clean up temp file on any error + if temp_path.exists(): + os.unlink(temp_path) + logger.explore( + "Failed to validate uploaded XLSX", + payload={"filename": file.filename}, + error=str(e), + ) + raise HTTPException(status_code=400, detail=f"Failed to validate XLSX file: {e!s}") from e +# #endregion upload_xlsx_mapping +# #endregion ToolsApiRoutes diff --git a/backend/src/app.py b/backend/src/app.py index a92ee1d22..32667420a 100755 --- a/backend/src/app.py +++ b/backend/src/app.py @@ -61,6 +61,7 @@ from .api.routes import ( settings, storage, tasks, + tools, translate, ) from .api.routes.validation_tasks import router as validation_tasks @@ -487,6 +488,7 @@ app.include_router(health.router) app.include_router(encryption_health.router) app.include_router(translate.router) app.include_router(validation_tasks, prefix="/api/validation-tasks", tags=["Validation Tasks"]) +app.include_router(tools.router, tags=["Tools"]) app.include_router(maintenance.maintenance_router) diff --git a/backend/src/plugins/mapper.py b/backend/src/plugins/mapper.py index 38a8c802e..d958723e9 100644 --- a/backend/src/plugins/mapper.py +++ b/backend/src/plugins/mapper.py @@ -5,6 +5,7 @@ # @BRIEF Plugin for dataset column mapping. Inherits PluginBase. # @RELATION DEPENDS_ON -> [TaskContext] +from pathlib import Path from typing import Any from ..core.logger import belief_scope, logger @@ -117,7 +118,12 @@ class MapperPlugin(PluginBase): "title": "Excel Path", "description": "Path to the Excel file (for excel source)." } - }, + }, + "upload_id": { + "type": "string", + "title": "Excel Upload ID", + "description": "Opaque ID returned by the mapper XLSX upload endpoint.", + }, "required": ["env", "dataset_id", "source"] } # endregion get_schema @@ -154,7 +160,7 @@ class MapperPlugin(PluginBase): raise ValueError(f"Environment '{env_name}' not found in configuration.") client = SupersetClient(env_config) - client.authenticate() + await client.authenticate() log.info(f"Starting mapping for dataset {dataset_id} in {env_name}") @@ -181,17 +187,40 @@ class MapperPlugin(PluginBase): ) else: # Excel source + upload_id = params.get("upload_id") + excel_path = self._resolve_upload_path(upload_id) if upload_id else params.get("excel_path") or params.get("file_data") await mapper.run_mapping( superset_client=client, dataset_id=dataset_id, source="excel", - excel_path=params.get("excel_path") or params.get("file_data") + excel_path=excel_path ) superset_log.info(f"Mapping completed for dataset {dataset_id}") return {"status": "success", "dataset_id": dataset_id} except Exception as e: log.error(f"Mapping failed: {e}") raise + finally: + uploaded_path = self._resolve_upload_path(params.get("upload_id")) if params.get("upload_id") else params.get("excel_path") or params.get("file_data") + if uploaded_path: + upload_root = Path("/tmp/ss-tools-mapper-uploads").resolve() + candidate = Path(str(uploaded_path)).resolve() + if candidate.is_file() and upload_root in candidate.parents: + candidate.unlink(missing_ok=True) + + # #region resolve_upload_path [C:2] [TYPE Function] [SEMANTICS mapper, upload, security] + # @ingroup MapperPlugin + # @BRIEF Resolve an opaque upload ID to a mapper temp file without allowing path traversal. + # @PRE upload_id is a non-empty opaque ID returned by the upload endpoint. + # @POST Returns a path inside the mapper upload directory or raises ValueError. + @staticmethod + def _resolve_upload_path(upload_id: str) -> str: + upload_root = Path("/tmp/ss-tools-mapper-uploads").resolve() + candidate = (upload_root / f"{upload_id}.xlsx").resolve() + if upload_root not in candidate.parents or not candidate.is_file(): + raise ValueError("Uploaded XLSX file not found") + return str(candidate) + # #endregion resolve_upload_path # endregion execute # #endregion MapperPlugin diff --git a/backend/tests/api/test_mappings.py b/backend/tests/api/test_mappings.py index 48b0244a5..39bf4667e 100644 --- a/backend/tests/api/test_mappings.py +++ b/backend/tests/api/test_mappings.py @@ -194,4 +194,146 @@ class TestSuggestMappingsApi: "source_env_id": "env-1", "target_env_id": "env-2", }) assert resp.status_code == 500 +# #region Test.ApplyDatasetMetadata [C:3] [TYPE Module] [SEMANTICS test,mappings,metadata,apply] +# @BRIEF Tests for apply_dataset_metadata PUT endpoint — doc application to datasets. +# @TEST_EDGE: missing_env_id -> 400 "env_id is required" +# @TEST_EDGE: env_not_found -> 404 "Environment not found" +# @TEST_EDGE: success_with_changes -> 200 status=success, dataset updated +# @TEST_EDGE: no_changes -> 200 status=no_changes, dataset not updated +# @TEST_EDGE: superset_error -> 500 propagated as HTTPException +class TestApplyDatasetMetadata: + """PUT /mappings/datasets/{id}/metadata""" + + BASE_PAYLOAD = { + "env_id": "env-1", + "description": "Dataset description", + "columns": {"col_a": "Column A verbose"}, + } + + MOCK_DATASET_RESPONSE = { + "result": { + "id": 42, + "table_name": "test_table", + "schema": "public", + "database": {"id": 1}, + "owners": [{"id": 1}], + "columns": [ + {"column_name": "col_a", "id": 1, "verbose_name": "Old Name", "type": "string"}, + {"column_name": "col_b", "id": 2, "verbose_name": "Col B", "type": "integer"}, + ], + "metrics": [], + "extra": None, + } + } + + # #region test_missing_env_id [C:2] [TYPE Function] + # @BRIEF Request without env_id returns 400. + def test_missing_env_id(self): + mock_config = MagicMock() + mock_config.get_environment.return_value = None + + from src.dependencies import get_config_manager + client = _make_client({get_config_manager: lambda: mock_config}) + + resp = client.put("/api/mappings/datasets/42/metadata", json={ + "description": "test", + "columns": {}, + }) + assert resp.status_code == 400 + assert "env_id" in resp.json()["detail"] + # #endregion test_missing_env_id + + # #region test_env_not_found [C:2] [TYPE Function] + # @BRIEF Unknown env_id returns 404. + def test_env_not_found(self): + mock_config = MagicMock() + mock_config.get_environment.return_value = None + + from src.dependencies import get_config_manager + client = _make_client({get_config_manager: lambda: mock_config}) + + resp = client.put("/api/mappings/datasets/42/metadata", json=self.BASE_PAYLOAD) + assert resp.status_code == 404 + assert "not found" in resp.json()["detail"].lower() + # #endregion test_env_not_found + + # #region test_success_with_changes [C:2] [TYPE Function] + # @BRIEF Valid request updates dataset and returns success. + def test_success_with_changes(self): + mock_config = MagicMock() + mock_config.get_environment.return_value = MagicMock() + + mock_superset_client = AsyncMock() + mock_superset_client.authenticate = AsyncMock() + mock_superset_client.get_dataset.return_value = self.MOCK_DATASET_RESPONSE + mock_superset_client.update_dataset.return_value = {"result": "ok"} + + from src.dependencies import get_config_manager + with patch("src.core.superset_client.SupersetClient", return_value=mock_superset_client): + client = _make_client({get_config_manager: lambda: mock_config}) + resp = client.put("/api/mappings/datasets/42/metadata", json=self.BASE_PAYLOAD) + + assert resp.status_code == 200 + data = resp.json() + assert data["status"] == "success" + assert data["dataset_id"] == 42 + + # Verify update_dataset was called with the description + assert mock_superset_client.update_dataset.call_count >= 1 + _call_args = mock_superset_client.update_dataset.call_args + # update_dataset(dataset_id, data) -> positional args + payload = _call_args[0][1] + assert payload["description"] == "Dataset description" + # col_a verbose_name should be updated + col_a = next(c for c in payload["columns"] if c["column_name"] == "col_a") + assert col_a["verbose_name"] == "Column A verbose" + # col_b should keep its original verbose_name (not in request.columns) + col_b = next(c for c in payload["columns"] if c["column_name"] == "col_b") + assert col_b["verbose_name"] == "Col B" + # #endregion test_success_with_changes + + # #region test_no_changes [C:2] [TYPE Function] + # @BRIEF When same values are sent, status returns no_changes and update_dataset is NOT called. + def test_no_changes(self): + mock_config = MagicMock() + mock_config.get_environment.return_value = MagicMock() + + mock_superset_client = AsyncMock() + mock_superset_client.authenticate = AsyncMock() + mock_superset_client.get_dataset.return_value = self.MOCK_DATASET_RESPONSE + + from src.dependencies import get_config_manager + with patch("src.core.superset_client.SupersetClient", return_value=mock_superset_client): + client = _make_client({get_config_manager: lambda: mock_config}) + # Send columns that match existing verbose_name values + resp = client.put("/api/mappings/datasets/42/metadata", json={ + "env_id": "env-1", + "columns": {"col_b": "Col B"}, # already matches + }) + + assert resp.status_code == 200 + data = resp.json() + assert data["status"] == "no_changes" + mock_superset_client.update_dataset.assert_not_called() + # #endregion test_no_changes + + # #region test_superset_error_propagated [C:2] [TYPE Function] + # @BRIEF When SupersetClient fails, endpoint returns 500 with error detail. + def test_superset_error_propagated(self): + mock_config = MagicMock() + mock_config.get_environment.return_value = MagicMock() + + mock_superset_client = AsyncMock() + mock_superset_client.authenticate = AsyncMock() + mock_superset_client.get_dataset.side_effect = RuntimeError("Superset API unavailable") + + from src.dependencies import get_config_manager + with patch("src.core.superset_client.SupersetClient", return_value=mock_superset_client): + client = _make_client({get_config_manager: lambda: mock_config}) + resp = client.put("/api/mappings/datasets/42/metadata", json=self.BASE_PAYLOAD) + + assert resp.status_code == 500 + assert "Superset API unavailable" in resp.json()["detail"] + # #endregion test_superset_error_propagated +# #endregion TestApplyDatasetMetadata # #endregion Test.Api.Mappings diff --git a/backend/tests/api/test_tools_mapper.py b/backend/tests/api/test_tools_mapper.py new file mode 100644 index 000000000..5ec6863db --- /dev/null +++ b/backend/tests/api/test_tools_mapper.py @@ -0,0 +1,184 @@ +# #region Test.Api.Tools.Mapper [C:3] [TYPE Module] [SEMANTICS test,tools,mapper,upload,xlsx] +# @BRIEF Tests for tools/mapper API routes — xlsx upload, validation. +# @RELATION BINDS_TO -> [ToolsApiRoutes] +# @TEST_CONTRACT: File upload -> { path: str } +# @TEST_EDGE: invalid_extension -> 400 "Only .xlsx files are accepted" +# @TEST_EDGE: missing_columns -> 400 "Missing columns: 'column_name' and 'verbose_name'" +# @TEST_EDGE: empty_file -> 400 "XLSX file is empty" +# @TEST_EDGE: all_verbose_null -> 400 "at least one non-empty 'verbose_name' value" +# @TEST_EDGE: unauthorized -> 403 when permission denied +# @TEST_EDGE: success -> 200 with temp path + +import io +import os +import uuid + +os.environ.setdefault("DATABASE_URL", "sqlite:///:memory:") +os.environ.setdefault("AUTH_DATABASE_URL", "sqlite:///:memory:") +os.environ.setdefault("SECRET_KEY", "test-secret-key-for-tests") +os.environ.setdefault("DEV_MODE", "true") + +import sys +from pathlib import Path +from unittest.mock import patch + +import pandas as pd +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +_src = str(Path(__file__).resolve().parent.parent.parent / "src") +if _src not in sys.path: + sys.path.insert(0, _src) + + +def _make_xlsx_bytes(columns: dict[str, list]) -> bytes: + """Create an in-memory xlsx file from column data.""" + df = pd.DataFrame(columns) + buf = io.BytesIO() + df.to_excel(buf, index=False) + buf.seek(0) + return buf.read() + + +def _make_client(unauthorized: bool = False) -> TestClient: + """Build a TestClient with tools router and overridden deps.""" + from src.api.routes.tools import router + from src.dependencies import get_current_user + from src.schemas.auth import User, RoleSchema + + app = FastAPI() + app.include_router(router) + + if unauthorized: + # Return 403 when unauthorized flag is set + from fastapi import HTTPException + async def _raise_403(): + raise HTTPException(status_code=403, detail="Forbidden") + app.dependency_overrides[get_current_user] = _raise_403 + else: + # Admin user — passes all permission checks via the Admin role bypass + mock_user = User( + id="admin-1", username="admin", email="admin@x.com", + auth_source="LOCAL", + created_at=__import__("datetime").datetime.now(), + roles=[RoleSchema(id="r1", name="Admin", description="", permissions=[])], + ) + app.dependency_overrides[get_current_user] = lambda: mock_user + + return TestClient(app) + + +class TestUploadXlsxMapping: + """POST /api/tools/mapper/upload-xlsx""" + + # #region test_success_upload_valid_xlsx [C:2] [TYPE Function] + # @BRIEF Valid xlsx with column_name and verbose_name returns 200 + path. + def test_success_upload_valid_xlsx(self): + client = _make_client() + xlsx_data = _make_xlsx_bytes({ + "column_name": ["col_a", "col_b"], + "verbose_name": ["Column A", "Column B"], + }) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("test.xlsx", xlsx_data, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 200 + data = resp.json() + assert "upload_id" in data + assert len(data["upload_id"]) == 32 + # #endregion test_success_upload_valid_xlsx + + # #region test_invalid_extension [C:2] [TYPE Function] + # @BRIEF Non-xlsx file extension returns 400. + def test_invalid_extension(self): + client = _make_client() + xlsx_data = _make_xlsx_bytes({ + "column_name": ["col_a"], + "verbose_name": ["desc"], + }) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("test.csv", xlsx_data, "text/csv")}, + ) + assert resp.status_code == 400 + assert "Only .xlsx files are accepted" in resp.json()["detail"] + # #endregion test_invalid_extension + + # #region test_missing_required_columns [C:2] [TYPE Function] + # @BRIEF Xlsx missing column_name or verbose_name returns 400. + def test_missing_required_columns(self): + client = _make_client() + # Missing verbose_name column + xlsx_data = _make_xlsx_bytes({ + "column_name": ["col_a"], + "other_col": ["val"], + }) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("test.xlsx", xlsx_data, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 400 + assert "verbose_name" in resp.json()["detail"] + # #endregion test_missing_required_columns + + # #region test_empty_xlsx [C:2] [TYPE Function] + # @BRIEF Empty xlsx (no data rows) returns 400. + def test_empty_xlsx(self): + client = _make_client() + df = pd.DataFrame({"column_name": pd.Series(dtype=str), "verbose_name": pd.Series(dtype=str)}) + buf = io.BytesIO() + df.to_excel(buf, index=False) + buf.seek(0) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("empty.xlsx", buf.read(), "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 400 + assert "empty" in resp.json()["detail"].lower() + # #endregion test_empty_xlsx + + # #region test_all_verbose_name_null [C:2] [TYPE Function] + # @BRIEF Xlsx with all-null verbose_name values returns 400. + def test_all_verbose_name_null(self): + client = _make_client() + xlsx_data = _make_xlsx_bytes({ + "column_name": ["col_a", "col_b"], + "verbose_name": [None, None], + }) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("test.xlsx", xlsx_data, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 400 + assert "verbose_name" in resp.json()["detail"].lower() + # #endregion test_all_verbose_name_null + + # #region test_unauthorized [C:2] [TYPE Function] + # @BRIEF Missing permission returns 403. + def test_unauthorized(self): + client = _make_client(unauthorized=True) + xlsx_data = _make_xlsx_bytes({ + "column_name": ["col_a"], + "verbose_name": ["desc"], + }) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("test.xlsx", xlsx_data, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 403 + # #endregion test_unauthorized + + # #region test_oversized_upload [C:2] [TYPE Function] + # @BRIEF Uploads larger than the configured limit return 413. + def test_oversized_upload(self): + client = _make_client() + oversized = b"x" * (10 * 1024 * 1024 + 1) + resp = client.post( + "/api/tools/mapper/upload-xlsx", + files={"file": ("large.xlsx", oversized, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet")}, + ) + assert resp.status_code == 413 + # #endregion test_oversized_upload +# #endregion TestUploadXlsxMapping diff --git a/backend/tests/plugins/test_mapper.py b/backend/tests/plugins/test_mapper.py index e95de82d0..afd08264c 100644 --- a/backend/tests/plugins/test_mapper.py +++ b/backend/tests/plugins/test_mapper.py @@ -147,7 +147,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() with ( patch("src.dependencies.get_config_manager", return_value=mock_cm), @@ -166,7 +166,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() mock_executor = AsyncMock() mock_executor.resolve_database_id = AsyncMock() @@ -190,6 +190,7 @@ class TestMapperPluginExecute: assert result["status"] == "success" assert result["dataset_id"] == 42 + mock_client.authenticate.assert_awaited_once() mock_executor.resolve_database_id.assert_called_once_with(target_database_id="10") mock_mapper.run_mapping.assert_called_once() @@ -201,7 +202,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() mock_executor = AsyncMock() mock_executor.resolve_database_id = AsyncMock() @@ -226,6 +227,7 @@ class TestMapperPluginExecute: assert result["status"] == "success" assert result["dataset_id"] == 7 + mock_client.authenticate.assert_awaited_once() ctx.logger.with_source.assert_called() # ── Excel source — success ── @@ -238,7 +240,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() mock_mapper = MagicMock() mock_mapper.run_mapping = AsyncMock() @@ -257,6 +259,7 @@ class TestMapperPluginExecute: assert result["status"] == "success" assert result["dataset_id"] == 99 + mock_client.authenticate.assert_awaited_once() mock_mapper.run_mapping.assert_called_once_with( superset_client=mock_client, dataset_id=99, @@ -272,7 +275,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() mock_mapper = MagicMock() mock_mapper.run_mapping = AsyncMock() @@ -324,7 +327,7 @@ class TestMapperPluginExecute: mock_cm = _mock_config_manager(env=env) mock_client = MagicMock() - mock_client.authenticate = MagicMock() + mock_client.authenticate = AsyncMock() mock_executor = AsyncMock() mock_executor.resolve_database_id = AsyncMock() diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 4f8875b91..c8d4f8fdf 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -483,6 +483,55 @@ interface ValidationRunQueryParams { page_size?: number; } +// #region uploadFile [C:2] [TYPE Function] [SEMANTICS api, upload, file, multipart] +// @BRIEF Upload a file via multipart/form-data to the given endpoint. +// @PRE endpoint is a non-empty string path. +// @PRE file is a File object. +// @POST Returns Promise with parsed JSON. +// @SIDE_EFFECT Sends HTTP POST with multipart/form-data. +// @RATIONALE Uses native fetch because the existing fetchApi/postApi/requestApi wrappers +// always set Content-Type: application/json, which breaks multipart uploads (the browser +// must auto-set Content-Type with the boundary). This is the ONLY exception to the +// "never native fetch" invariant. +// @RELATION CALLED_BY -> [MapperTool] +async function uploadFile(endpoint: string, file: File): Promise { + const _start = performance.now(); + const formData = new FormData(); + formData.append('file', file); + try { + log('ApiClient', 'REASON', 'Upload file', { endpoint, filename: file.name, size: file.size }); + const headers: Record = {}; + if (typeof window !== 'undefined') { + const token = localStorage.getItem('auth_token'); + if (token) headers['Authorization'] = `Bearer ${token}`; + const tid = getTraceId(); + if (tid && tid !== 'no-trace') { + headers['X-Trace-ID'] = tid; + } + } + const response = await fetch(`${API_BASE_URL}${endpoint}`, { + method: 'POST', + headers, + body: formData, + }); + if (!response.ok) throw await buildApiError(response); + _captureTraceId(response); + const data = await response.json() as T; + log('ApiClient', 'REFLECT', 'Upload completed', { + endpoint, filename: file.name, status: response.status, + elapsed_ms: Math.round(performance.now() - _start), + }); + return data; + } catch (error) { + const apiError = error as ApiError; + log('ApiClient', 'EXPLORE', 'Upload failed', { endpoint, filename: file.name }, apiError?.message || 'unknown'); + notifyApiError(apiError); + throw error; + } +} +// #endregion uploadFile + + // #region ApiRegistry [C:3] [TYPE Block] [SEMANTICS api, endpoints, registry] // @BRIEF Named endpoint registry — maps backend API paths to typed frontend methods. // @LAYER API @@ -491,7 +540,8 @@ interface ValidationRunQueryParams { // @RELATION DEPENDS_ON -> [deleteApi] // @RELATION DEPENDS_ON -> [requestApi] // @RELATION DEPENDS_ON -> [fetchApiBlob] -// @INVARIANT Every method delegates to fetchApi/postApi/deleteApi/requestApi — never native fetch. +// @RELATION DEPENDS_ON -> [uploadFile] +// @INVARIANT Every method delegates to fetchApi/postApi/deleteApi/requestApi/uploadFile — never native fetch. // @RATIONALE The registry pattern keeps endpoint paths in one place and eliminates path-string duplication across components. export const api = { fetchApi: fetchApi as (endpoint: string, options?: FetchOptions) => Promise, @@ -499,6 +549,7 @@ export const api = { deleteApi: deleteApi as (endpoint: string, options?: FetchOptions) => Promise, requestApi: requestApi as (endpoint: string, method?: string, body?: unknown, requestOptions?: FetchOptions) => Promise, fetchApiBlob: fetchApiBlob as (endpoint: string, options?: FetchOptions) => Promise, + uploadFile: uploadFile as (endpoint: string, file: File) => Promise, // ═══ Tasks ════════════════════════════════════════════════════ @@ -1163,7 +1214,7 @@ export const api = { // #endregion ApiRegistry // #endregion ApiModule -export { fetchApi, postApi, deleteApi, requestApi }; +export { fetchApi, postApi, deleteApi, requestApi, uploadFile }; export const getPlugins = api.getPlugins; export const getTasks = api.getTasks; export const getTask = api.getTask; diff --git a/frontend/src/lib/api/__tests__/api.test.ts b/frontend/src/lib/api/__tests__/api.test.ts index 64a964fda..dcaff90cd 100644 --- a/frontend/src/lib/api/__tests__/api.test.ts +++ b/frontend/src/lib/api/__tests__/api.test.ts @@ -1016,6 +1016,128 @@ describe('ApiModule — registry methods', () => { expect(await api.getStorageFileBlob('/some/file.csv')).toBe(blob); expect(fetch).toHaveBeenCalledWith('/api/storage/file?path=%2Fsome%2Ffile.csv', expect.any(Object)); }); + + // #region uploadFileTests [C:2] [TYPE Test] [SEMANTICS test,api,upload,file] + // @BRIEF Verify uploadFile sends FormData with auth headers and handles responses. + describe('uploadFile', () => { + beforeEach(() => { + vi.stubGlobal('fetch', vi.fn()); + vi.stubGlobal('window', { + location: { protocol: 'http:', host: 'localhost:5173' }, + }); + localStorage.clear(); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('sends POST with FormData and auth headers, returns JSON on success', async () => { + localStorage.setItem('auth_token', 'test-token'); + vi.mocked(fetch).mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve({ upload_id: 'upload-42' }), + } as Response); + + const { uploadFile } = await import('$lib/api.js'); + const file = new File(['test'], 'mapping.xlsx', { type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' }); + const result = await uploadFile('/tools/mapper/upload-xlsx', file); + + expect(result).toEqual({ upload_id: 'upload-42' }); + expect(fetch).toHaveBeenCalledWith( + '/api/tools/mapper/upload-xlsx', + expect.objectContaining({ + method: 'POST', + headers: expect.objectContaining({ + Authorization: 'Bearer test-token', + }), + body: expect.any(FormData), + }), + ); + // Verify FormData contains the file + const callArgs = vi.mocked(fetch).mock.calls[0]; + const formData = callArgs[1].body as FormData; + expect(formData.get('file')).toBe(file); + }); + + it('omits Content-Type header to let browser set multipart boundary', async () => { + localStorage.setItem('auth_token', 'tok'); + vi.mocked(fetch).mockResolvedValue({ + ok: true, status: 200, + json: () => Promise.resolve({ path: '/tmp/f.xlsx' }), + } as Response); + + const { uploadFile } = await import('$lib/api.js'); + const file = new File(['data'], 'f.xlsx'); + await uploadFile('/tools/mapper/upload-xlsx', file); + + const headers = vi.mocked(fetch).mock.calls[0][1].headers as Record; + // Should NOT have Content-Type (browser sets it automatically for FormData) + expect(headers['Content-Type']).toBeUndefined(); + }); + + it('throws ApiError on non-ok response and dispatches error toast', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: false, + status: 400, + json: () => Promise.resolve({ detail: 'Only .xlsx files are accepted' }), + } as Response); + + const { addToast } = await import('$lib/toasts.svelte.js'); + addToast.mockClear(); + const { uploadFile } = await import('$lib/api.js'); + const file = new File(['data'], 'bad.csv'); + + await expect(uploadFile('/tools/mapper/upload-xlsx', file)).rejects.toMatchObject({ + status: 400, + message: 'Only .xlsx files are accepted', + }); + expect(addToast).toHaveBeenCalled(); + }); + + it('includes X-Trace-ID header when trace ID is set', async () => { + const { getTraceId } = await import('$lib/cot-logger.js'); + vi.mocked(getTraceId).mockReturnValue('my-trace-42'); + + vi.mocked(fetch).mockResolvedValue({ + ok: true, status: 200, + json: () => Promise.resolve({ path: '/tmp/t.xlsx' }), + } as Response); + + const { uploadFile } = await import('$lib/api.js'); + const file = new File(['data'], 't.xlsx'); + await uploadFile('/tools/mapper/upload-xlsx', file); + + const headers = vi.mocked(fetch).mock.calls[0][1].headers as Record; + expect(headers['X-Trace-ID']).toBe('my-trace-42'); + }); + + it('works without auth token (no Authorization header)', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: true, status: 200, + json: () => Promise.resolve({ path: '/tmp/t.xlsx' }), + } as Response); + + const { uploadFile } = await import('$lib/api.js'); + const file = new File(['data'], 't.xlsx'); + await uploadFile('/tools/mapper/upload-xlsx', file); + + const headers = vi.mocked(fetch).mock.calls[0][1].headers as Record; + expect(headers['Authorization']).toBeUndefined(); + }); + + it('uploadFile is available on api.uploadFile', async () => { + vi.mocked(fetch).mockResolvedValue({ + ok: true, status: 200, + json: () => Promise.resolve({ path: '/tmp/t.xlsx' }), + } as Response); + + const { api } = await import('$lib/api.js'); + expect(typeof api.uploadFile).toBe('function'); + }); + }); + // #endregion uploadFileTests it('updateGlobalSettings sends PATCH', async () => { vi.mocked(fetch).mockResolvedValue(await _okJson({ success: true })); const { api } = await import('$lib/api.js'); diff --git a/frontend/src/lib/components/tools/MapperTool.svelte b/frontend/src/lib/components/tools/MapperTool.svelte index 0b65b95ff..51be3ad9c 100644 --- a/frontend/src/lib/components/tools/MapperTool.svelte +++ b/frontend/src/lib/components/tools/MapperTool.svelte @@ -1,28 +1,35 @@ - + + + + - - - + + + @@ -187,15 +309,12 @@ />
- -
@@ -210,6 +329,7 @@