From 1782e4b0b785cabab89b0d1eba54ace4684cce8c Mon Sep 17 00:00:00 2001 From: Amir Fathi Date: Sat, 22 Aug 2026 03:32:50 +0000 Subject: [PATCH] fix(context): reject v1 view upgrades that map two views to the same target _plan_v1_to_v2 already refuses to upgrade when two legacy cube files resolve to the same target directory (seen_cube_targets), but the views loop right above it has no equivalent check. Two views.yml entries sharing the same name resolve to the same views// directory, and _apply_v1_to_v2 writes both in place before deleting views.yml, the only other copy, so the first view's definition is silently discarded. Mirror the existing cube guard for views: track resolved metadata.yml targets in seen_view_targets and raise UpgradeError on a repeat before any file is written. --- core/wren/src/wren/context.py | 10 +++++++++- core/wren/tests/unit/test_context.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/core/wren/src/wren/context.py b/core/wren/src/wren/context.py index a35829d055..453b057a54 100644 --- a/core/wren/src/wren/context.py +++ b/core/wren/src/wren/context.py @@ -1723,6 +1723,7 @@ def _plan_v1_to_v2(project_path: Path) -> tuple[list[str], list[str]]: deleted.append(f"models/{source_dir}.yml") # Views: single file → directories + seen_view_targets: set[str] = set() views = _load_views_v1(project_path) for view in views: name = view.get("name") @@ -1736,7 +1737,14 @@ def _plan_v1_to_v2(project_path: Path) -> tuple[list[str], list[str]]: created.append(sql_file.relative_to(project_root).as_posix()) metadata_file = _resolve_upgrade_file(view_dir, "metadata.yml") - created.append(metadata_file.relative_to(project_root).as_posix()) + target = metadata_file.relative_to(project_root).as_posix() + if target in seen_view_targets: + raise UpgradeError( + f"Cannot upgrade: multiple legacy views map to '{target}'" + ) + seen_view_targets.add(target) + + created.append(target) views_file = project_path / "views.yml" if views_file.exists(): diff --git a/core/wren/tests/unit/test_context.py b/core/wren/tests/unit/test_context.py index 9dcf68a97d..68a3c72618 100644 --- a/core/wren/tests/unit/test_context.py +++ b/core/wren/tests/unit/test_context.py @@ -1461,6 +1461,35 @@ def test_plan_upgrade_v1_to_v2_rejects_duplicate_cube_targets(tmp_path): plan_upgrade(tmp_path, target_version=2) +def test_plan_upgrade_v1_to_v2_rejects_duplicate_view_names(tmp_path): + _make_v1_project(tmp_path) + source_contents = _snapshot_v1_sources(tmp_path) + (tmp_path / "views.yml").write_text( + "views:\n" + " - name: summary\n" + " statement: SELECT 1\n" + " - name: summary\n" + " statement: SELECT 2\n" + ) + + # Use fresh import to avoid stale class reference after importlib.reload in earlier tests. + from wren.context import UpgradeError as _UE # noqa: PLC0415 + + with pytest.raises(_UE, match="multiple legacy views"): + plan_upgrade(tmp_path, target_version=2) + + assert get_schema_version(tmp_path) == 1 + assert (tmp_path / "views.yml").exists() + for relative_path in ( + "models/orders.yml", + "models/revenue.yml", + "cubes/order_metrics.yml", + ): + assert (tmp_path / relative_path).read_text( + encoding="utf-8" + ) == source_contents[relative_path] + + def test_plan_upgrade_v2_to_v3(tmp_path): _make_v2_project(tmp_path) result = plan_upgrade(tmp_path, target_version=3)