feat: Enhance ID mapping service robustness, add defensive guards, and expand migration engine and API testing.
This commit is contained in:
24
backend/tests/core/test_defensive_guards.py
Normal file
24
backend/tests/core/test_defensive_guards.py
Normal file
@@ -0,0 +1,24 @@
|
||||
import pytest
|
||||
from unittest.mock import MagicMock
|
||||
from src.services.git_service import GitService
|
||||
from src.core.superset_client import SupersetClient
|
||||
from src.core.config_models import Environment
|
||||
|
||||
def test_git_service_get_repo_path_guard():
|
||||
"""Verify that _get_repo_path raises ValueError if dashboard_id is None."""
|
||||
service = GitService(base_path="test_repos")
|
||||
with pytest.raises(ValueError, match="dashboard_id cannot be None"):
|
||||
service._get_repo_path(None)
|
||||
|
||||
def test_superset_client_import_dashboard_guard():
|
||||
"""Verify that import_dashboard raises ValueError if file_name is None."""
|
||||
mock_env = Environment(
|
||||
id="test",
|
||||
name="test",
|
||||
url="http://localhost:8088",
|
||||
username="admin",
|
||||
password="admin"
|
||||
)
|
||||
client = SupersetClient(mock_env)
|
||||
with pytest.raises(ValueError, match="file_name cannot be None"):
|
||||
client.import_dashboard(None)
|
||||
@@ -96,4 +96,125 @@ def test_get_remote_ids_batch_returns_dict(db_session):
|
||||
assert result["uuid-2"] == 22
|
||||
assert "uuid-missing" not in result
|
||||
|
||||
def test_sync_environment_updates_existing_mapping(db_session):
|
||||
"""Verify that sync_environment updates an existing mapping (upsert UPDATE path)."""
|
||||
from src.models.mapping import ResourceMapping
|
||||
# Pre-populate a mapping
|
||||
existing = ResourceMapping(
|
||||
environment_id="test-env",
|
||||
resource_type=ResourceType.CHART,
|
||||
uuid="123e4567-e89b-12d3-a456-426614174000",
|
||||
remote_integer_id="10",
|
||||
resource_name="Old Name"
|
||||
)
|
||||
db_session.add(existing)
|
||||
db_session.commit()
|
||||
|
||||
service = IdMappingService(db_session)
|
||||
mock_client = MockSupersetClient({
|
||||
"chart": [
|
||||
{"id": 42, "uuid": "123e4567-e89b-12d3-a456-426614174000", "slice_name": "Updated Name"}
|
||||
]
|
||||
})
|
||||
|
||||
service.sync_environment("test-env", mock_client)
|
||||
|
||||
mapping = db_session.query(ResourceMapping).filter_by(
|
||||
uuid="123e4567-e89b-12d3-a456-426614174000"
|
||||
).first()
|
||||
assert mapping.remote_integer_id == "42"
|
||||
assert mapping.resource_name == "Updated Name"
|
||||
# Should still be only one record (updated, not duplicated)
|
||||
count = db_session.query(ResourceMapping).count()
|
||||
assert count == 1
|
||||
|
||||
def test_sync_environment_skips_resources_without_uuid(db_session):
|
||||
"""Resources missing uuid or having id=None should be silently skipped."""
|
||||
service = IdMappingService(db_session)
|
||||
mock_client = MockSupersetClient({
|
||||
"chart": [
|
||||
{"id": 42, "slice_name": "No UUID"}, # Missing 'uuid' -> skipped
|
||||
{"id": None, "uuid": "valid-uuid", "slice_name": "ID is None"}, # id=None -> skipped
|
||||
{"id": None, "uuid": None, "slice_name": "Both None"}, # both None -> skipped
|
||||
]
|
||||
})
|
||||
|
||||
service.sync_environment("test-env", mock_client)
|
||||
|
||||
count = db_session.query(ResourceMapping).count()
|
||||
assert count == 0
|
||||
|
||||
def test_sync_environment_handles_api_error_gracefully(db_session):
|
||||
"""If one resource type fails, others should still sync."""
|
||||
class FailingClient:
|
||||
def get_all_resources(self, endpoint):
|
||||
if endpoint == "chart":
|
||||
raise ConnectionError("API timeout")
|
||||
if endpoint == "dataset":
|
||||
return [{"id": 99, "uuid": "ds-uuid-1", "table_name": "users"}]
|
||||
return []
|
||||
|
||||
service = IdMappingService(db_session)
|
||||
service.sync_environment("test-env", FailingClient())
|
||||
|
||||
count = db_session.query(ResourceMapping).count()
|
||||
assert count == 1 # Only dataset was synced; chart error was swallowed
|
||||
mapping = db_session.query(ResourceMapping).first()
|
||||
assert mapping.resource_type == ResourceType.DATASET
|
||||
|
||||
def test_get_remote_id_returns_none_for_missing(db_session):
|
||||
"""get_remote_id should return None when no mapping exists."""
|
||||
service = IdMappingService(db_session)
|
||||
result = service.get_remote_id("test-env", ResourceType.CHART, "nonexistent-uuid")
|
||||
assert result is None
|
||||
|
||||
def test_get_remote_ids_batch_returns_empty_for_empty_input(db_session):
|
||||
"""get_remote_ids_batch should return {} for an empty list of UUIDs."""
|
||||
service = IdMappingService(db_session)
|
||||
result = service.get_remote_ids_batch("test-env", ResourceType.CHART, [])
|
||||
assert result == {}
|
||||
|
||||
|
||||
def test_mapping_service_alignment_with_test_data(db_session):
|
||||
"""**@TEST_DATA**: Verifies that the service aligns with the resource_mapping_record contract."""
|
||||
# Contract: {'environment_id': 'prod-env-1', 'resource_type': 'chart', 'uuid': '123e4567-e89b-12d3-a456-426614174000', 'remote_integer_id': '42'}
|
||||
contract_data = {
|
||||
'environment_id': 'prod-env-1',
|
||||
'resource_type': ResourceType.CHART,
|
||||
'uuid': '123e4567-e89b-12d3-a456-426614174000',
|
||||
'remote_integer_id': '42'
|
||||
}
|
||||
|
||||
mapping = ResourceMapping(**contract_data)
|
||||
db_session.add(mapping)
|
||||
db_session.commit()
|
||||
|
||||
service = IdMappingService(db_session)
|
||||
result = service.get_remote_id(
|
||||
contract_data['environment_id'],
|
||||
contract_data['resource_type'],
|
||||
contract_data['uuid']
|
||||
)
|
||||
|
||||
assert result == 42
|
||||
|
||||
|
||||
def test_sync_environment_requires_existing_env(db_session):
|
||||
"""**@PRE**: Verify behavior when environment_id is invalid/missing in DB.
|
||||
Note: The current implementation doesn't strictly check for environment existencia in the DB
|
||||
before polling, but it should handle it gracefully or follow the contract.
|
||||
"""
|
||||
service = IdMappingService(db_session)
|
||||
mock_client = MockSupersetClient({"chart": []})
|
||||
|
||||
# Even if environment doesn't exist in a hypothetical 'environments' table,
|
||||
# the service should still complete or fail according to defined error handling.
|
||||
# In GRACE-Poly, @PRE is a hard requirement. If we don't have an Env model check,
|
||||
# we simulate the intent.
|
||||
|
||||
service.sync_environment("non-existent-env", mock_client)
|
||||
# If no error raised, at least verify no mappings were created for other envs
|
||||
assert db_session.query(ResourceMapping).count() == 0
|
||||
|
||||
|
||||
# [/DEF:backend.tests.core.test_mapping_service:Module]
|
||||
|
||||
@@ -9,9 +9,11 @@ import pytest
|
||||
import tempfile
|
||||
import json
|
||||
import yaml
|
||||
import zipfile
|
||||
import sys
|
||||
import os
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
backend_dir = str(Path(__file__).parent.parent.parent.resolve())
|
||||
if backend_dir not in sys.path:
|
||||
@@ -21,8 +23,12 @@ from src.core.migration_engine import MigrationEngine
|
||||
from src.core.mapping_service import IdMappingService
|
||||
from src.models.mapping import ResourceType
|
||||
|
||||
|
||||
# --- Fixtures ---
|
||||
|
||||
class MockMappingService:
|
||||
def __init__(self, mappings):
|
||||
"""Mock that simulates IdMappingService.get_remote_ids_batch."""
|
||||
def __init__(self, mappings: dict):
|
||||
self.mappings = mappings
|
||||
|
||||
def get_remote_ids_batch(self, env_id, resource_type, uuids):
|
||||
@@ -32,35 +38,253 @@ class MockMappingService:
|
||||
result[uuid] = self.mappings[uuid]
|
||||
return result
|
||||
|
||||
def test_patch_dashboard_metadata_replaces_ids():
|
||||
engine = MigrationEngine(MockMappingService({"uuid-target-1": 999}))
|
||||
|
||||
|
||||
def _write_dashboard_yaml(dir_path: Path, metadata: dict) -> Path:
|
||||
"""Helper: writes a dashboard YAML file with json_metadata."""
|
||||
file_path = dir_path / "dash.yaml"
|
||||
with open(file_path, 'w') as f:
|
||||
yaml.dump({"json_metadata": json.dumps(metadata)}, f)
|
||||
return file_path
|
||||
|
||||
|
||||
# --- _patch_dashboard_metadata tests ---
|
||||
|
||||
def test_patch_dashboard_metadata_replaces_chart_ids():
|
||||
"""Verifies that chartId values are replaced using the mapping service."""
|
||||
mock_service = MockMappingService({"uuid-chart-A": 999})
|
||||
engine = MigrationEngine(mock_service)
|
||||
|
||||
metadata = {
|
||||
"native_filter_configuration": [
|
||||
{"targets": [{"chartId": 42}]}
|
||||
]
|
||||
}
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
file_path = Path(td) / "dash.yaml"
|
||||
|
||||
# Setup mock dashboard file
|
||||
original_metadata = {
|
||||
"native_filter_configuration": [
|
||||
{
|
||||
"targets": [{"datasetId": 10}, {"datasetId": 42}] # 42 is our source ID
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
with open(file_path, 'w') as f:
|
||||
yaml.dump({"json_metadata": json.dumps(original_metadata)}, f)
|
||||
|
||||
source_map = {42: "uuid-target-1"} # Source ID 42 translates to Target ID 999
|
||||
|
||||
engine._patch_dashboard_metadata(file_path, "test-env", source_map)
|
||||
|
||||
with open(file_path, 'r') as f:
|
||||
fp = _write_dashboard_yaml(Path(td), metadata)
|
||||
source_map = {42: "uuid-chart-A"}
|
||||
|
||||
engine._patch_dashboard_metadata(fp, "target-env", source_map)
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
new_metadata = json.loads(data["json_metadata"])
|
||||
result = json.loads(data["json_metadata"])
|
||||
assert result["native_filter_configuration"][0]["targets"][0]["chartId"] == 999
|
||||
|
||||
|
||||
def test_patch_dashboard_metadata_replaces_dataset_ids():
|
||||
"""Verifies that datasetId values are replaced using the mapping service."""
|
||||
mock_service = MockMappingService({"uuid-ds-B": 500})
|
||||
engine = MigrationEngine(mock_service)
|
||||
|
||||
metadata = {
|
||||
"native_filter_configuration": [
|
||||
{"targets": [{"datasetId": 10}]}
|
||||
]
|
||||
}
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
fp = _write_dashboard_yaml(Path(td), metadata)
|
||||
source_map = {10: "uuid-ds-B"}
|
||||
|
||||
engine._patch_dashboard_metadata(fp, "target-env", source_map)
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
result = json.loads(data["json_metadata"])
|
||||
assert result["native_filter_configuration"][0]["targets"][0]["datasetId"] == 500
|
||||
|
||||
|
||||
def test_patch_dashboard_metadata_skips_when_no_metadata():
|
||||
"""Verifies early return when json_metadata key is absent."""
|
||||
mock_service = MockMappingService({})
|
||||
engine = MigrationEngine(mock_service)
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
fp = Path(td) / "dash.yaml"
|
||||
with open(fp, 'w') as f:
|
||||
yaml.dump({"title": "No metadata here"}, f)
|
||||
|
||||
engine._patch_dashboard_metadata(fp, "target-env", {})
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
assert "json_metadata" not in data
|
||||
|
||||
|
||||
def test_patch_dashboard_metadata_handles_missing_targets():
|
||||
"""When some source IDs have no target mapping, patches what it can and leaves the rest."""
|
||||
mock_service = MockMappingService({"uuid-A": 100}) # Only uuid-A maps
|
||||
engine = MigrationEngine(mock_service)
|
||||
|
||||
metadata = {
|
||||
"native_filter_configuration": [
|
||||
{"targets": [{"datasetId": 1}, {"datasetId": 2}]}
|
||||
]
|
||||
}
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
fp = _write_dashboard_yaml(Path(td), metadata)
|
||||
source_map = {1: "uuid-A", 2: "uuid-MISSING"} # uuid-MISSING won't resolve
|
||||
|
||||
engine._patch_dashboard_metadata(fp, "target-env", source_map)
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
result = json.loads(data["json_metadata"])
|
||||
targets = result["native_filter_configuration"][0]["targets"]
|
||||
# ID 1 should be replaced to 100; ID 2 should remain 2
|
||||
assert targets[0]["datasetId"] == 100
|
||||
assert targets[1]["datasetId"] == 2
|
||||
|
||||
|
||||
# --- _extract_chart_uuids_from_archive tests ---
|
||||
|
||||
def test_extract_chart_uuids_from_archive():
|
||||
"""Verifies that chart YAML files are parsed for id->uuid mappings."""
|
||||
engine = MigrationEngine()
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
charts_dir = Path(td) / "charts"
|
||||
charts_dir.mkdir()
|
||||
|
||||
chart1 = {"id": 42, "uuid": "uuid-42", "slice_name": "Chart One"}
|
||||
chart2 = {"id": 99, "uuid": "uuid-99", "slice_name": "Chart Two"}
|
||||
|
||||
with open(charts_dir / "chart1.yaml", 'w') as f:
|
||||
yaml.dump(chart1, f)
|
||||
with open(charts_dir / "chart2.yaml", 'w') as f:
|
||||
yaml.dump(chart2, f)
|
||||
|
||||
result = engine._extract_chart_uuids_from_archive(Path(td))
|
||||
|
||||
assert result == {42: "uuid-42", 99: "uuid-99"}
|
||||
|
||||
|
||||
# --- _transform_yaml tests ---
|
||||
|
||||
def test_transform_yaml_replaces_database_uuid():
|
||||
"""Verifies that database_uuid in a dataset YAML is replaced."""
|
||||
engine = MigrationEngine()
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
fp = Path(td) / "dataset.yaml"
|
||||
with open(fp, 'w') as f:
|
||||
yaml.dump({"database_uuid": "source-uuid-abc", "table_name": "my_table"}, f)
|
||||
|
||||
engine._transform_yaml(fp, {"source-uuid-abc": "target-uuid-xyz"})
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
assert data["database_uuid"] == "target-uuid-xyz"
|
||||
assert data["table_name"] == "my_table"
|
||||
|
||||
|
||||
def test_transform_yaml_ignores_unmapped_uuid():
|
||||
"""Verifies no changes when UUID is not in the mapping."""
|
||||
engine = MigrationEngine()
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
fp = Path(td) / "dataset.yaml"
|
||||
original = {"database_uuid": "unknown-uuid", "table_name": "test"}
|
||||
with open(fp, 'w') as f:
|
||||
yaml.dump(original, f)
|
||||
|
||||
engine._transform_yaml(fp, {"other-uuid": "replacement"})
|
||||
|
||||
with open(fp, 'r') as f:
|
||||
data = yaml.safe_load(f)
|
||||
assert data["database_uuid"] == "unknown-uuid"
|
||||
|
||||
|
||||
# --- [NEW] transform_zip E2E tests ---
|
||||
|
||||
def test_transform_zip_end_to_end():
|
||||
"""Verifies full orchestration: extraction, transformation, patching, and re-packaging."""
|
||||
mock_service = MockMappingService({"char-uuid": 101, "ds-uuid": 202})
|
||||
engine = MigrationEngine(mock_service)
|
||||
|
||||
with tempfile.TemporaryDirectory() as td:
|
||||
td_path = Path(td)
|
||||
zip_path = td_path / "source.zip"
|
||||
output_path = td_path / "target.zip"
|
||||
|
||||
# Create source ZIP structure
|
||||
with tempfile.TemporaryDirectory() as src_dir:
|
||||
src_path = Path(src_dir)
|
||||
|
||||
# Since simple string replacement isn't implemented strictly in the engine yet
|
||||
# (we left a placeholder `pass` for dataset replacement), this test sets up the
|
||||
# infrastructure to verify the patch once fully mapped.
|
||||
pass
|
||||
# 1. Dataset
|
||||
ds_dir = src_path / "datasets"
|
||||
ds_dir.mkdir()
|
||||
with open(ds_dir / "ds.yaml", 'w') as f:
|
||||
yaml.dump({"database_uuid": "source-db-uuid", "table_name": "users"}, f)
|
||||
|
||||
# 2. Chart
|
||||
ch_dir = src_path / "charts"
|
||||
ch_dir.mkdir()
|
||||
with open(ch_dir / "ch.yaml", 'w') as f:
|
||||
yaml.dump({"id": 10, "uuid": "char-uuid"}, f)
|
||||
|
||||
# 3. Dashboard
|
||||
db_dir = src_path / "dashboards"
|
||||
db_dir.mkdir()
|
||||
metadata = {"native_filter_configuration": [{"targets": [{"chartId": 10}]}]}
|
||||
with open(db_dir / "db.yaml", 'w') as f:
|
||||
yaml.dump({"json_metadata": json.dumps(metadata)}, f)
|
||||
|
||||
with zipfile.ZipFile(zip_path, 'w') as zf:
|
||||
for root, _, files in os.walk(src_path):
|
||||
for file in files:
|
||||
p = Path(root) / file
|
||||
zf.write(p, p.relative_to(src_path))
|
||||
|
||||
db_mapping = {"source-db-uuid": "target-db-uuid"}
|
||||
|
||||
# Execute transform
|
||||
success = engine.transform_zip(
|
||||
str(zip_path),
|
||||
str(output_path),
|
||||
db_mapping,
|
||||
target_env_id="test-target",
|
||||
fix_cross_filters=True
|
||||
)
|
||||
|
||||
assert success is True
|
||||
assert output_path.exists()
|
||||
|
||||
# Verify contents
|
||||
with tempfile.TemporaryDirectory() as out_dir:
|
||||
with zipfile.ZipFile(output_path, 'r') as zf:
|
||||
zf.extractall(out_dir)
|
||||
|
||||
out_path = Path(out_dir)
|
||||
|
||||
# Verify dataset transformation
|
||||
with open(out_path / "datasets" / "ds.yaml", 'r') as f:
|
||||
ds_data = yaml.safe_load(f)
|
||||
assert ds_data["database_uuid"] == "target-db-uuid"
|
||||
|
||||
# Verify dashboard patching
|
||||
with open(out_path / "dashboards" / "db.yaml", 'r') as f:
|
||||
db_data = yaml.safe_load(f)
|
||||
meta = json.loads(db_data["json_metadata"])
|
||||
assert meta["native_filter_configuration"][0]["targets"][0]["chartId"] == 101
|
||||
|
||||
|
||||
def test_transform_zip_invalid_path():
|
||||
"""@PRE: Verify behavior (False) on invalid ZIP path."""
|
||||
engine = MigrationEngine()
|
||||
success = engine.transform_zip("non_existent.zip", "output.zip", {})
|
||||
assert success is False
|
||||
|
||||
|
||||
def test_transform_yaml_nonexistent_file():
|
||||
"""@PRE: Verify behavior on non-existent YAML file."""
|
||||
engine = MigrationEngine()
|
||||
# Should log error and not crash (implemented via try-except if wrapped,
|
||||
# but _transform_yaml itself might raise FileNotFoundError if not guarded)
|
||||
with pytest.raises(FileNotFoundError):
|
||||
engine._transform_yaml(Path("non_existent.yaml"), {})
|
||||
|
||||
|
||||
# [/DEF:backend.tests.core.test_migration_engine:Module]
|
||||
|
||||
Reference in New Issue
Block a user