bito-code-review[bot] commented on code in PR #44396:
URL: https://github.com/apache/superset/pull/44396#discussion_r4043734986


##########
tests/unit_tests/semantic_layers/import_export_test.py:
##########
@@ -0,0 +1,804 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Typed bundle identities must not become table IDs or grant datasource 
access."""
+
+import copy
+import importlib
+import inspect
+from types import ModuleType
+from typing import Any
+from unittest.mock import Mock
+from uuid import UUID
+
+import pytest
+import yaml
+from flask import current_app, Response
+from flask_appbuilder.api import safe
+from marshmallow import ValidationError
+from sqlalchemy.orm import Session
+
+from superset import security_manager
+from superset.charts.schemas import ImportV1ChartSchema
+from superset.commands.exceptions import CommandInvalidError, ImportFailedError
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.semantic_layers import import_export as refs
+from superset.semantic_layers.models import SemanticLayer, SemanticView
+from superset.utils import json
+
+VIEW_UUID: str = "efafe588-5c67-4d1c-a9bb-ab1839f8cf59"
+CHART_UUID: str = "d766261b-5cb0-4ea1-b62c-0da4860116c1"
+
+
[email protected]
+def view(app_context: None, monkeypatch: pytest.MonkeyPatch) -> SemanticView:
+    """A real model and access predicate, with no provider instantiation."""
+    model: SemanticView = SemanticView(
+        id=81,
+        uuid=UUID(VIEW_UUID),
+        name="existing semantic view",
+        perm="view-grant",
+        semantic_layer=SemanticLayer(type="test-provider", perm="layer-grant"),
+    )
+    monkeypatch.setattr(
+        refs.feature_flag_manager,
+        "is_feature_enabled",
+        lambda flag: flag == "SEMANTIC_LAYERS",
+    )
+    monkeypatch.setattr(security_manager, "can_access_all_datasources", 
lambda: False)
+    monkeypatch.setattr(
+        security_manager,
+        "can_access",
+        lambda permission, resource: resource == "view-grant",
+    )
+    monkeypatch.setitem(
+        refs.registry,
+        "test-provider",
+        Mock(spec=[], side_effect=AssertionError("provider must not be 
instantiated")),
+    )
+    query: Mock = Mock()
+    query.options.return_value = query
+    query.filter.return_value.all.return_value = [model]
+    monkeypatch.setattr(refs.db.session, "query", Mock(return_value=query))
+    return model
+
+
+def chart_config() -> dict[str, Any]:
+    """Use deliberately different archived and destination IDs."""
+    return {
+        "uuid": CHART_UUID,
+        "version": "1.0.0",
+        "slice_name": "semantic chart",
+        "viz_type": "table",
+        "datasource_ref": {"type": "semantic_view", "uuid": VIEW_UUID},
+        "params": {"datasource": "7__semantic_view", "metrics": ["revenue"]},
+        "query_context": json.dumps(
+            {
+                "datasource": {"id": 7, "type": "semantic_view"},
+                "form_data": {"datasource": "7__semantic_view"},
+                "queries": [
+                    {
+                        "datasource": {"id": 7, "type": "semantic_view"},
+                        "metrics": ["revenue"],
+                    }
+                ],
+            }
+        ),
+    }
+
+
[email protected](
+    "reference",
+    [
+        None,
+        {},
+        {"type": "table", "uuid": VIEW_UUID},
+        {"type": "semantic_view", "uuid": "bad"},
+        {
+            "type": "semantic_view",
+            "uuid": VIEW_UUID,
+            "configuration": {"token": "not-allowed"},
+        },
+    ],
+)
+def test_chart_schema_rejects_invalid_reference(reference: Any) -> None:
+    """A missing dataset_uuid is permitted only with a valid semantic 
reference."""
+    config: dict[str, Any] = chart_config()
+    config["datasource_ref"] = reference
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+
+
+def test_chart_schema_rejects_ambiguous_and_missing_reference() -> None:
+    """Never pick one of two contradictory source representations."""
+    config: dict[str, Any] = chart_config()
+    config["dataset_uuid"] = VIEW_UUID
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+    config.pop("datasource_ref")
+    ImportV1ChartSchema().load(config)
+    config.pop("dataset_uuid")
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+
+
[email protected](
+    "module_name,command_name",
+    [
+        ("superset.commands.chart.importers.v1", "ImportChartsCommand"),
+        ("superset.commands.dashboard.importers.v1", 
"ImportDashboardsCommand"),
+        ("superset.commands.importers.v1.assets", "ImportAssetsCommand"),
+    ],
+)
+def test_each_importer_rebinds_semantic_chart(
+    view: SemanticView,
+    monkeypatch: pytest.MonkeyPatch,
+    module_name: str,
+    command_name: str,
+) -> None:
+    """Exercise real entry-point orchestration/remapping, stubbing only 
writers."""
+    module: ModuleType = importlib.import_module(module_name)
+    importer: Any = getattr(module, command_name)
+    writer: Mock = Mock(
+        return_value=Mock(id=91, uuid=UUID(CHART_UUID), viz_type="table")
+    )
+    monkeypatch.setattr(module, "import_chart", writer)
+    monkeypatch.setattr(module, "get_default_viewers_for_current_user", 
lambda: [])
+    configs: dict[str, Any] = {"charts/chart.yaml": chart_config()}
+    importer._import(configs, overwrite=True)
+    writer.assert_called_once()
+    actual: dict[str, Any] = writer.call_args.args[0]
+    assert actual["datasource_id"] == 81
+    assert actual["datasource_type"] == "semantic_view"
+    assert "datasource_ref" not in actual
+    assert "dataset_uuid" not in actual
+    assert actual["params"] == {
+        "datasource": "81__semantic_view",
+        "metrics": ["revenue"],
+    }
+    context: dict[str, Any] = json.loads(actual["query_context"])
+    assert context["datasource"] == {"id": 81, "type": "semantic_view"}
+    assert context["form_data"]["datasource"] == "81__semantic_view"
+    assert context["queries"][0] == {
+        "datasource": {"id": 81, "type": "semantic_view"},
+        "metrics": ["revenue"],
+    }
+
+
[email protected]("failure", ["missing", "denied", "disabled", 
"provider"])
[email protected](
+    "module_name,command_name",
+    [
+        ("superset.commands.chart.importers.v1", "ImportChartsCommand"),
+        ("superset.commands.dashboard.importers.v1", 
"ImportDashboardsCommand"),
+        ("superset.commands.importers.v1.assets", "ImportAssetsCommand"),
+    ],
+)
+def test_dependency_failure_precedes_any_bundle_write(
+    view: SemanticView,
+    monkeypatch: pytest.MonkeyPatch,
+    failure: str,
+    module_name: str,
+    command_name: str,
+) -> None:
+    """Even unrelated database/dataset assets must not write before 
preflight."""
+    module: ModuleType = importlib.import_module(module_name)
+    importer: Any = getattr(module, command_name)
+    writes: list[Mock] = []
+    for name in ("import_database", "import_dataset", "import_chart"):
+        writer: Mock = Mock(side_effect=AssertionError("write before 
preflight"))
+        monkeypatch.setattr(module, name, writer)
+        writes.append(writer)
+    if failure == "missing":
+        
refs.db.session.query.return_value.filter.return_value.all.return_value = []
+    elif failure == "denied":
+        monkeypatch.setattr(security_manager, "can_access", lambda *args: 
False)
+    elif failure == "disabled":
+        monkeypatch.setattr(
+            refs.feature_flag_manager, "is_feature_enabled", lambda flag: False
+        )
+    else:
+        monkeypatch.delitem(refs.registry, "test-provider")
+    configs: dict[str, Any] = {
+        "databases/database.yaml": {"uuid": "db"},
+        "datasets/dataset.yaml": {"uuid": "table", "database_uuid": "db"},
+        "charts/chart.yaml": chart_config(),
+    }
+    with pytest.raises(refs.SemanticReferenceError):
+        importer._import(configs, overwrite=True)
+    for writer in writes:
+        writer.assert_not_called()
+
+
[email protected](
+    "control", ["native_filter_configuration", "chart_customization_config"]
+)
+def test_dashboard_semantic_target_roundtrip(view: SemanticView, control: str) 
-> None:
+    """Typed target serialization preserves the semantic view UUID and type."""
+    metadata: dict[str, Any] = {
+        control: [
+            {
+                "targets": [
+                    {
+                        "datasetId": 81,
+                        "datasourceType": "semantic_view",
+                        "column": {"name": "country"},
+                    }
+                ]
+            }
+        ]
+    }
+    refs.export_dashboard_references(metadata)
+    target: dict[str, Any] = metadata[control][0]["targets"][0]
+    assert "datasetId" not in target
+    assert "datasetUuid" not in target
+    assert target["datasourceRef"] == {"type": "semantic_view", "uuid": 
VIEW_UUID}
+    info: dict[str, dict[str, Any]] = refs.resolve_bundle_references(
+        {"dashboards/d.yaml": {"metadata": copy.deepcopy(metadata)}}
+    )
+    refs.restore_dashboard_references(metadata, info)
+    assert target == {
+        "datasetId": 81,
+        "datasourceType": "semantic_view",
+        "column": {"name": "country"},
+    }
+
+
+def test_layer_grant_authorizes_reference_without_view_grant(
+    view: SemanticView, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Use the model's actual parent-layer grant rule, not a new permission 
rule."""
+    monkeypatch.setattr(
+        security_manager,
+        "can_access",
+        lambda permission, resource: resource == "layer-grant",
+    )
+    assert refs.export_view_reference(view) == {
+        "type": "semantic_view",
+        "uuid": VIEW_UUID,
+    }
+
+
+def test_table_only_bundle_does_not_need_semantic_feature(
+    app_context: None, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Do not query semantic models or flags for a legacy bundle."""
+    forbidden: Mock = Mock(side_effect=AssertionError("semantic lookup for 
table"))
+    monkeypatch.setattr(refs.db.session, "query", forbidden)
+    monkeypatch.setattr(refs.feature_flag_manager, "is_feature_enabled", 
forbidden)
+    assert (
+        refs.resolve_bundle_references({"charts/a.yaml": {"dataset_uuid": 
VIEW_UUID}})
+        == {}
+    )
+
+
[email protected]("dataset_id", [None, True, False, "", "invalid", 1.5])
+def test_export_rejects_invalid_semantic_target_id_before_lookup(
+    app_context: None, monkeypatch: pytest.MonkeyPatch, dataset_id: Any
+) -> None:
+    """Invalid local IDs cannot reach a semantic lookup or mutate the 
target."""
+    query: Mock = Mock(side_effect=AssertionError("invalid ID reached lookup"))
+    monkeypatch.setattr(refs.db.session, "query", query)
+    target: dict[str, Any] = {
+        "datasourceType": "semantic_view",
+        "datasetId": dataset_id,
+    }
+    metadata: dict[str, Any] = {"native_filter_configuration": [{"targets": 
[target]}]}

Review Comment:
   <!-- Bito Reply -->
   The suggestion to scope the `can_access` predicate in the `view` fixture is 
correct and improves the test's reliability. By replacing the unconditional 
`True` grant with a specific check for `resource == "view-grant"`, the test can 
now correctly identify and fail on unauthorized access attempts, preventing 
potential false positives where incorrect permissions might otherwise be 
ignored.
   
   **tests/unit_tests/semantic_layers/import_export_test.py**
   ```
   monkeypatch.setattr(security_manager, "can_access_all_datasources", lambda: 
False)
       monkeypatch.setattr(
           security_manager,
           "can_access",
           lambda permission, resource: resource == "view-grant",
       )
   ```



##########
tests/unit_tests/semantic_layers/import_export_test.py:
##########
@@ -0,0 +1,804 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Typed bundle identities must not become table IDs or grant datasource 
access."""
+
+import copy
+import importlib
+import inspect
+from types import ModuleType
+from typing import Any
+from unittest.mock import Mock
+from uuid import UUID
+
+import pytest
+import yaml
+from flask import current_app, Response
+from flask_appbuilder.api import safe
+from marshmallow import ValidationError
+from sqlalchemy.orm import Session
+
+from superset import security_manager
+from superset.charts.schemas import ImportV1ChartSchema
+from superset.commands.exceptions import CommandInvalidError, ImportFailedError
+from superset.connectors.sqla.models import SqlaTable
+from superset.models.core import Database
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.semantic_layers import import_export as refs
+from superset.semantic_layers.models import SemanticLayer, SemanticView
+from superset.utils import json
+
+VIEW_UUID: str = "efafe588-5c67-4d1c-a9bb-ab1839f8cf59"
+CHART_UUID: str = "d766261b-5cb0-4ea1-b62c-0da4860116c1"
+
+
[email protected]
+def view(app_context: None, monkeypatch: pytest.MonkeyPatch) -> SemanticView:
+    """A real model and access predicate, with no provider instantiation."""
+    model: SemanticView = SemanticView(
+        id=81,
+        uuid=UUID(VIEW_UUID),
+        name="existing semantic view",
+        perm="view-grant",
+        semantic_layer=SemanticLayer(type="test-provider", perm="layer-grant"),
+    )
+    monkeypatch.setattr(
+        refs.feature_flag_manager,
+        "is_feature_enabled",
+        lambda flag: flag == "SEMANTIC_LAYERS",
+    )
+    monkeypatch.setattr(security_manager, "can_access_all_datasources", 
lambda: False)
+    monkeypatch.setattr(
+        security_manager,
+        "can_access",
+        lambda permission, resource: resource == "view-grant",
+    )
+    monkeypatch.setitem(
+        refs.registry,
+        "test-provider",
+        Mock(spec=[], side_effect=AssertionError("provider must not be 
instantiated")),
+    )
+    query: Mock = Mock()
+    query.options.return_value = query
+    query.filter.return_value.all.return_value = [model]
+    monkeypatch.setattr(refs.db.session, "query", Mock(return_value=query))
+    return model
+
+
+def chart_config() -> dict[str, Any]:
+    """Use deliberately different archived and destination IDs."""
+    return {
+        "uuid": CHART_UUID,
+        "version": "1.0.0",
+        "slice_name": "semantic chart",
+        "viz_type": "table",
+        "datasource_ref": {"type": "semantic_view", "uuid": VIEW_UUID},
+        "params": {"datasource": "7__semantic_view", "metrics": ["revenue"]},
+        "query_context": json.dumps(
+            {
+                "datasource": {"id": 7, "type": "semantic_view"},
+                "form_data": {"datasource": "7__semantic_view"},
+                "queries": [
+                    {
+                        "datasource": {"id": 7, "type": "semantic_view"},
+                        "metrics": ["revenue"],
+                    }
+                ],
+            }
+        ),
+    }
+
+
[email protected](
+    "reference",
+    [
+        None,
+        {},
+        {"type": "table", "uuid": VIEW_UUID},
+        {"type": "semantic_view", "uuid": "bad"},
+        {
+            "type": "semantic_view",
+            "uuid": VIEW_UUID,
+            "configuration": {"token": "not-allowed"},
+        },
+    ],
+)
+def test_chart_schema_rejects_invalid_reference(reference: Any) -> None:
+    """A missing dataset_uuid is permitted only with a valid semantic 
reference."""
+    config: dict[str, Any] = chart_config()
+    config["datasource_ref"] = reference
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+
+
+def test_chart_schema_rejects_ambiguous_and_missing_reference() -> None:
+    """Never pick one of two contradictory source representations."""
+    config: dict[str, Any] = chart_config()
+    config["dataset_uuid"] = VIEW_UUID
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+    config.pop("datasource_ref")
+    ImportV1ChartSchema().load(config)
+    config.pop("dataset_uuid")
+    with pytest.raises(ValidationError):
+        ImportV1ChartSchema().load(config)
+
+
[email protected](
+    "module_name,command_name",
+    [
+        ("superset.commands.chart.importers.v1", "ImportChartsCommand"),
+        ("superset.commands.dashboard.importers.v1", 
"ImportDashboardsCommand"),
+        ("superset.commands.importers.v1.assets", "ImportAssetsCommand"),
+    ],
+)
+def test_each_importer_rebinds_semantic_chart(
+    view: SemanticView,
+    monkeypatch: pytest.MonkeyPatch,
+    module_name: str,
+    command_name: str,
+) -> None:
+    """Exercise real entry-point orchestration/remapping, stubbing only 
writers."""
+    module: ModuleType = importlib.import_module(module_name)
+    importer: Any = getattr(module, command_name)
+    writer: Mock = Mock(
+        return_value=Mock(id=91, uuid=UUID(CHART_UUID), viz_type="table")
+    )
+    monkeypatch.setattr(module, "import_chart", writer)
+    monkeypatch.setattr(module, "get_default_viewers_for_current_user", 
lambda: [])
+    configs: dict[str, Any] = {"charts/chart.yaml": chart_config()}
+    importer._import(configs, overwrite=True)
+    writer.assert_called_once()
+    actual: dict[str, Any] = writer.call_args.args[0]
+    assert actual["datasource_id"] == 81
+    assert actual["datasource_type"] == "semantic_view"
+    assert "datasource_ref" not in actual
+    assert "dataset_uuid" not in actual
+    assert actual["params"] == {
+        "datasource": "81__semantic_view",
+        "metrics": ["revenue"],
+    }
+    context: dict[str, Any] = json.loads(actual["query_context"])
+    assert context["datasource"] == {"id": 81, "type": "semantic_view"}
+    assert context["form_data"]["datasource"] == "81__semantic_view"
+    assert context["queries"][0] == {
+        "datasource": {"id": 81, "type": "semantic_view"},
+        "metrics": ["revenue"],
+    }
+
+
[email protected]("failure", ["missing", "denied", "disabled", 
"provider"])
[email protected](
+    "module_name,command_name",
+    [
+        ("superset.commands.chart.importers.v1", "ImportChartsCommand"),
+        ("superset.commands.dashboard.importers.v1", 
"ImportDashboardsCommand"),
+        ("superset.commands.importers.v1.assets", "ImportAssetsCommand"),
+    ],
+)
+def test_dependency_failure_precedes_any_bundle_write(
+    view: SemanticView,
+    monkeypatch: pytest.MonkeyPatch,
+    failure: str,
+    module_name: str,
+    command_name: str,
+) -> None:
+    """Even unrelated database/dataset assets must not write before 
preflight."""
+    module: ModuleType = importlib.import_module(module_name)
+    importer: Any = getattr(module, command_name)
+    writes: list[Mock] = []
+    for name in ("import_database", "import_dataset", "import_chart"):
+        writer: Mock = Mock(side_effect=AssertionError("write before 
preflight"))
+        monkeypatch.setattr(module, name, writer)
+        writes.append(writer)
+    if failure == "missing":
+        
refs.db.session.query.return_value.filter.return_value.all.return_value = []
+    elif failure == "denied":
+        monkeypatch.setattr(security_manager, "can_access", lambda *args: 
False)
+    elif failure == "disabled":
+        monkeypatch.setattr(
+            refs.feature_flag_manager, "is_feature_enabled", lambda flag: False
+        )
+    else:
+        monkeypatch.delitem(refs.registry, "test-provider")
+    configs: dict[str, Any] = {
+        "databases/database.yaml": {"uuid": "db"},
+        "datasets/dataset.yaml": {"uuid": "table", "database_uuid": "db"},
+        "charts/chart.yaml": chart_config(),
+    }
+    with pytest.raises(refs.SemanticReferenceError):
+        importer._import(configs, overwrite=True)
+    for writer in writes:
+        writer.assert_not_called()
+
+
[email protected](
+    "control", ["native_filter_configuration", "chart_customization_config"]
+)
+def test_dashboard_semantic_target_roundtrip(view: SemanticView, control: str) 
-> None:
+    """Typed target serialization preserves the semantic view UUID and type."""
+    metadata: dict[str, Any] = {
+        control: [
+            {
+                "targets": [
+                    {
+                        "datasetId": 81,
+                        "datasourceType": "semantic_view",
+                        "column": {"name": "country"},
+                    }
+                ]
+            }
+        ]
+    }
+    refs.export_dashboard_references(metadata)
+    target: dict[str, Any] = metadata[control][0]["targets"][0]
+    assert "datasetId" not in target
+    assert "datasetUuid" not in target
+    assert target["datasourceRef"] == {"type": "semantic_view", "uuid": 
VIEW_UUID}
+    info: dict[str, dict[str, Any]] = refs.resolve_bundle_references(
+        {"dashboards/d.yaml": {"metadata": copy.deepcopy(metadata)}}
+    )
+    refs.restore_dashboard_references(metadata, info)
+    assert target == {
+        "datasetId": 81,
+        "datasourceType": "semantic_view",
+        "column": {"name": "country"},
+    }
+
+
+def test_layer_grant_authorizes_reference_without_view_grant(
+    view: SemanticView, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Use the model's actual parent-layer grant rule, not a new permission 
rule."""
+    monkeypatch.setattr(
+        security_manager,
+        "can_access",
+        lambda permission, resource: resource == "layer-grant",
+    )
+    assert refs.export_view_reference(view) == {
+        "type": "semantic_view",
+        "uuid": VIEW_UUID,
+    }
+
+
+def test_table_only_bundle_does_not_need_semantic_feature(
+    app_context: None, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Do not query semantic models or flags for a legacy bundle."""
+    forbidden: Mock = Mock(side_effect=AssertionError("semantic lookup for 
table"))
+    monkeypatch.setattr(refs.db.session, "query", forbidden)
+    monkeypatch.setattr(refs.feature_flag_manager, "is_feature_enabled", 
forbidden)
+    assert (
+        refs.resolve_bundle_references({"charts/a.yaml": {"dataset_uuid": 
VIEW_UUID}})
+        == {}
+    )
+
+
[email protected]("dataset_id", [None, True, False, "", "invalid", 1.5])
+def test_export_rejects_invalid_semantic_target_id_before_lookup(
+    app_context: None, monkeypatch: pytest.MonkeyPatch, dataset_id: Any
+) -> None:
+    """Invalid local IDs cannot reach a semantic lookup or mutate the 
target."""
+    query: Mock = Mock(side_effect=AssertionError("invalid ID reached lookup"))
+    monkeypatch.setattr(refs.db.session, "query", query)
+    target: dict[str, Any] = {
+        "datasourceType": "semantic_view",
+        "datasetId": dataset_id,
+    }
+    metadata: dict[str, Any] = {"native_filter_configuration": [{"targets": 
[target]}]}
+    before: dict[str, Any] = copy.deepcopy(metadata)
+    with pytest.raises(refs.SemanticReferenceError, match="requires a 
datasetId"):
+        refs.export_dashboard_references(metadata)
+    query.assert_not_called()
+    assert metadata == before
+
+
+def test_bundle_preflight_rejects_ambiguous_chart_before_lookup(
+    app_context: None, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Preflight itself rejects both identities, even without schema 
loading."""
+    query: Mock = Mock(side_effect=AssertionError("ambiguous chart reached 
lookup"))
+    monkeypatch.setattr(refs.db.session, "query", query)
+    config: dict[str, Any] = chart_config()
+    config["dataset_uuid"] = VIEW_UUID
+    with pytest.raises(
+        refs.SemanticReferenceError, match="Specify only one chart datasource 
reference"
+    ) as excinfo:
+        refs.resolve_bundle_references({"charts/chart.yaml": config})
+    assert isinstance(excinfo.value.__cause__, ValidationError)
+    query.assert_not_called()
+    assert config["dataset_uuid"] == config["datasource_ref"]["uuid"] == 
VIEW_UUID
+
+
+def test_chart_semantic_info_preserves_table_reference() -> None:
+    """A table UUID must not resolve through the colliding semantic UUID 
map."""
+    config: dict[str, Any] = {"dataset_uuid": VIEW_UUID}
+    semantic_info: dict[str, dict[str, Any]] = {
+        VIEW_UUID: {"datasource_id": 81, "datasource_type": "semantic_view"}
+    }
+    assert refs.chart_semantic_info(config, semantic_info) is None
+    assert config == {"dataset_uuid": VIEW_UUID}
+
+
[email protected](
+    "control", ["native_filter_configuration", "chart_customization_config"]
+)
+def test_restore_mixed_targets_preserves_table_identity(control: str) -> None:
+    """Rebind a semantic target without changing a preceding same-UUID 
table."""
+    table: dict[str, Any] = {"datasetUuid": VIEW_UUID, "datasourceType": 
"table"}
+    semantic: dict[str, Any] = {
+        "datasourceRef": {"type": "semantic_view", "uuid": VIEW_UUID},
+        "column": {"name": "country"},
+    }
+    metadata: dict[str, Any] = {control: [{"targets": [table, semantic]}]}
+    refs.restore_dashboard_references(metadata, {VIEW_UUID: {"datasource_id": 
81}})
+    assert table == {"datasetUuid": VIEW_UUID, "datasourceType": "table"}
+    assert semantic == {

Review Comment:
   <!-- Bito Reply -->
   The suggestion to move the import to the module level is correct and follows 
standard Python practices to avoid potential circular dependencies and maintain 
clean code structure. You should apply this change by moving the import of 
`superset.commands.chart.export` to the top of the file alongside the other 
`superset.commands.*` imports.
   
   **tests/unit_tests/semantic_layers/import_export_test.py**
   ```
   from superset.commands.chart.export import ExportChartsCommand
   from superset.commands.exceptions import CommandInvalidError, 
ImportFailedError
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to