fix: slug is empty if filename is non-ASCII (#22118)

superset/charts/commands/export.py

@@ -21,14 +21,14 @@
 from typing import Iterator, Tuple
 
 import yaml
-from werkzeug.utils import secure_filename
 
 from superset.charts.commands.exceptions import ChartNotFoundError
 from superset.charts.dao import ChartDAO
 from superset.datasets.commands.export import ExportDatasetsCommand
 from superset.commands.export.models import ExportModelsCommand
 from superset.models.slice import Slice
 from superset.utils.dict_import_export import EXPORT_VERSION
+from superset.utils.file import get_filename
 
 logger = logging.getLogger(__name__)
 
@@ -44,8 +44,8 @@ class ExportChartsCommand(ExportModelsCommand):
 
     @staticmethod
     def _export(model: Slice, export_related: bool = True) -> Iterator[Tuple[str, str]]:
-        chart_slug = secure_filename(model.slice_name)
-        file_name = f"charts/{chart_slug}_{model.id}.yaml"
+        file_name = get_filename(model.slice_name, model.id)
+        file_path = f"charts/{file_name}.yaml"
 
         payload = model.export_to_dict(
             recursive=False,
@@ -70,7 +70,7 @@ def _export(model: Slice, export_related: bool = True) -> Iterator[Tuple[str, st
             payload["dataset_uuid"] = str(model.table.uuid)
 
         file_content = yaml.safe_dump(payload, sort_keys=False)
-        yield file_name, file_content
+        yield file_path, file_content
 
         if model.table and export_related:
             yield from ExportDatasetsCommand([model.table.id]).run()

superset/dashboards/commands/export.py

@@ -23,7 +23,6 @@
 from typing import Any, Dict, Iterator, Optional, Set, Tuple
 
 import yaml
-from werkzeug.utils import secure_filename
 
 from superset.charts.commands.export import ExportChartsCommand
 from superset.dashboards.commands.exceptions import DashboardNotFoundError
@@ -35,6 +34,7 @@
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
 from superset.utils.dict_import_export import EXPORT_VERSION
+from superset.utils.file import get_filename
 
 logger = logging.getLogger(__name__)
 
@@ -111,8 +111,8 @@ class ExportDashboardsCommand(ExportModelsCommand):
     def _export(
         model: Dashboard, export_related: bool = True
     ) -> Iterator[Tuple[str, str]]:
-        dashboard_slug = secure_filename(model.dashboard_title)
-        file_name = f"dashboards/{dashboard_slug}_{model.id}.yaml"
+        file_name = get_filename(model.dashboard_title, model.id)
+        file_path = f"dashboards/{file_name}.yaml"
 
         payload = model.export_to_dict(
             recursive=False,
@@ -163,7 +163,7 @@ def _export(
         payload["version"] = EXPORT_VERSION
 
         file_content = yaml.safe_dump(payload, sort_keys=False)
-        yield file_name, file_content
+        yield file_path, file_content
 
         if export_related:
             chart_ids = [chart.id for chart in model.slices]

superset/databases/commands/export.py

@@ -21,13 +21,13 @@
 from typing import Any, Dict, Iterator, Tuple
 
 import yaml
-from werkzeug.utils import secure_filename
 
 from superset.databases.commands.exceptions import DatabaseNotFoundError
 from superset.databases.dao import DatabaseDAO
 from superset.commands.export.models import ExportModelsCommand
 from superset.models.core import Database
 from superset.utils.dict_import_export import EXPORT_VERSION
+from superset.utils.file import get_filename
 
 logger = logging.getLogger(__name__)
 
@@ -58,8 +58,8 @@ class ExportDatabasesCommand(ExportModelsCommand):
     def _export(
         model: Database, export_related: bool = True
     ) -> Iterator[Tuple[str, str]]:
-        database_slug = secure_filename(model.database_name)
-        file_name = f"databases/{database_slug}.yaml"
+        db_file_name = get_filename(model.database_name, model.id, skip_id=True)
+        file_path = f"databases/{db_file_name}.yaml"
 
         payload = model.export_to_dict(
             recursive=False,
@@ -90,12 +90,14 @@ def _export(
         payload["version"] = EXPORT_VERSION
 
         file_content = yaml.safe_dump(payload, sort_keys=False)
-        yield file_name, file_content
+        yield file_path, file_content
 
         if export_related:
             for dataset in model.tables:
-                dataset_slug = secure_filename(dataset.table_name)
-                file_name = f"datasets/{database_slug}/{dataset_slug}.yaml"
+                ds_file_name = get_filename(
+                    dataset.table_name, dataset.id, skip_id=True
+                )
+                file_path = f"datasets/{db_file_name}/{ds_file_name}.yaml"
 
                 payload = dataset.export_to_dict(
                     recursive=True,
@@ -107,4 +109,4 @@ def _export(
                 payload["database_uuid"] = str(model.uuid)
 
                 file_content = yaml.safe_dump(payload, sort_keys=False)
-                yield file_name, file_content
+                yield file_path, file_content

superset/datasets/commands/export.py

@@ -21,13 +21,13 @@
 from typing import Iterator, Tuple
 
 import yaml
-from werkzeug.utils import secure_filename
 
 from superset.commands.export.models import ExportModelsCommand
 from superset.connectors.sqla.models import SqlaTable
 from superset.datasets.commands.exceptions import DatasetNotFoundError
 from superset.datasets.dao import DatasetDAO
 from superset.utils.dict_import_export import EXPORT_VERSION
+from superset.utils.file import get_filename
 
 logger = logging.getLogger(__name__)
 
@@ -43,9 +43,11 @@ class ExportDatasetsCommand(ExportModelsCommand):
     def _export(
         model: SqlaTable, export_related: bool = True
     ) -> Iterator[Tuple[str, str]]:
-        database_slug = secure_filename(model.database.database_name)
-        dataset_slug = secure_filename(model.table_name)
-        file_name = f"datasets/{database_slug}/{dataset_slug}.yaml"
+        db_file_name = get_filename(
+            model.database.database_name, model.database.id, skip_id=True
+        )
+        ds_file_name = get_filename(model.table_name, model.id, skip_id=True)
+        file_path = f"datasets/{db_file_name}/{ds_file_name}.yaml"
 
         payload = model.export_to_dict(
             recursive=True,
@@ -75,11 +77,11 @@ def _export(
         payload["database_uuid"] = str(model.database.uuid)
 
         file_content = yaml.safe_dump(payload, sort_keys=False)
-        yield file_name, file_content
+        yield file_path, file_content
 
         # include database as well
         if export_related:
-            file_name = f"databases/{database_slug}.yaml"
+            file_path = f"databases/{db_file_name}.yaml"
 
             payload = model.database.export_to_dict(
                 recursive=False,
@@ -98,4 +100,4 @@ def _export(
             payload["version"] = EXPORT_VERSION
 
             file_content = yaml.safe_dump(payload, sort_keys=False)
-            yield file_name, file_content
+            yield file_path, file_content

superset/utils/file.py

@@ -0,0 +1,23 @@
+# 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.
+from werkzeug.utils import secure_filename
+
+
+def get_filename(model_name: str, model_id: int, skip_id: bool = False) -> str:
+    slug = secure_filename(model_name)
+    filename = slug if skip_id else f"{slug}_{model_id}"
+    return filename if slug else str(model_id)

tests/unit_tests/utils/test_file.py

@@ -0,0 +1,44 @@
+# -*- coding: utf-8 -*-
+# 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.
+import pytest
+
+from superset.utils.file import get_filename
+
+
+@pytest.mark.parametrize(
+    "model_name,model_id,skip_id,expected_filename",
+    [
+        ("Energy Sankey", 132, False, "Energy_Sankey_132"),
+        ("Energy Sankey", 132, True, "Energy_Sankey"),
+        ("folder1/Energy Sankey", 132, True, "folder1_Energy_Sankey"),
+        ("D:\\Charts\\Energy Sankey", 132, True, "DChartsEnergy_Sankey"),
+        ("🥴🥴🥴", 4751, False, "4751"),
+        ("🥴🥴🥴", 4751, True, "4751"),
+        ("Energy Sankey 🥴🥴🥴", 4751, False, "Energy_Sankey_4751"),
+        ("Energy Sankey 🥴🥴🥴", 4751, True, "Energy_Sankey"),
+        ("你好", 475, False, "475"),
+        ("你好", 475, True, "475"),
+        ("Energy Sankey 你好", 475, False, "Energy_Sankey_475"),
+        ("Energy Sankey 你好", 475, True, "Energy_Sankey"),
+    ],
+)
+def test_get_filename(
+    model_name: str, model_id: int, skip_id: bool, expected_filename: str
+) -> None:
+    original_filename = get_filename(model_name, model_id, skip_id)
+    assert expected_filename == original_filename