rusackas commented on code in PR #41780:
URL: https://github.com/apache/superset/pull/41780#discussion_r3565311600
##########
superset/views/core.py:
##########
@@ -932,6 +933,52 @@ def language_pack(self, lang: str) -> FlaskResponse:
"Language pack doesn't exist on the server", status=404
)
+ @expose("/language_pack/<lang>/<version>/script.js")
+ def language_pack_script(self, lang: str, version: str) -> FlaskResponse:
+ """Serve the language pack as a content-addressed classic script.
+
+ spa.html loads this BEFORE the entry bundle so translations are
+ configured synchronously (no race with code-split chunks), while the
+ versioned URL lets browsers cache the pack as immutable and pick up a
+ fresh copy whenever translations change.
+
+ Deliberately unauthenticated: translation catalogs are static, public
+ content shipped in the Superset repo, contain no user or tenant data,
+ and must load for anonymous principals (login page, embedded).
+ """
+ # Only allow expected language formats like "en", "pt_BR", etc.
+ if not re.match(r"^[a-z]{2,3}(_[A-Z]{2})?$", lang):
+ abort(400, "Invalid language code")
+ if not re.match(r"^[0-9a-f]{12}$", version):
+ abort(400, "Invalid language pack version")
+
+ current_version = get_language_pack_version(lang)
+ if current_version is None:
+ return json_error_response(
+ "Language pack doesn't exist on the server", status=404
+ )
+ pack = get_language_pack(lang)
+ if pack is None:
+ return json_error_response(
+ "Language pack doesn't exist on the server", status=404
+ )
+
+ response = Response(
+ f"window.__SUPERSET_LANGUAGE_PACK__ = {json.dumps(pack)};",
+ mimetype="application/javascript; charset=utf-8",
+ )
Review Comment:
Fair — annotated `current_version`, `pack`, and `response` in 95d1f6b.
##########
tests/unit_tests/views/test_bootstrap_auth.py:
##########
@@ -156,71 +156,104 @@ def test_bootstrap_does_not_crash_without_recaptcha_key(
assert "RECAPTCHA_PUBLIC_KEY" not in payload["conf"]
-# --- language_pack injection --------------------------------------------
+# --- language pack delivery ----------------------------------------------
#
-# The Jed pack is injected by `common_bootstrap_payload` (outside the
-# memoized `cached_common_bootstrap_data`) using the shared
-# `superset.translations.utils.get_language_pack`. Tests here cover the
-# wrapper to confirm the pack lands on the payload for non-English
-# locales and is None for English.
+# The pack is NOT embedded in the bootstrap payload; spa.html loads it via a
+# content-addressed script tag whose URL comes from
+# `get_language_pack_template_context`. A pack supplied through
+# COMMON_BOOTSTRAP_OVERRIDES_FUNC still rides the payload and suppresses the
+# script tag.
-def test_common_bootstrap_payload_includes_language_pack_for_non_english(
+def test_common_bootstrap_payload_does_not_embed_language_pack(
app_context: None,
) -> None:
- """common.language_pack carries the shared utility's pack for non-en."""
- fake_pack = {"domain": "superset", "locale_data": {"superset": {}}}
+ """The payload stays small: no full pack even for non-English locales."""
with (
patch(
"superset.views.base.cached_common_bootstrap_data",
return_value={"locale": "fr"},
),
- patch(
- "superset.views.base.get_language_pack",
- return_value=fake_pack,
- ) as mock_get,
patch("superset.views.base.utils.get_user_id", return_value=1),
patch("superset.views.base.get_locale", return_value="fr"),
):
payload = common_bootstrap_payload()
Review Comment:
Annotated the `payload` locals to match the rest of the file (95d1f6b).
##########
tests/unit_tests/views/test_language_pack_script.py:
##########
@@ -0,0 +1,115 @@
+# 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 pathlib import Path
+from typing import Any
+from unittest.mock import patch
+
+FAKE_PACK = {"domain": "superset", "locale_data": {"superset": {"": {}}}}
+FAKE_VERSION = "abc123def456"
+
+
+def test_script_served_immutable_when_version_matches(client: Any) -> None:
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ),
+ patch("superset.views.core.get_language_pack", return_value=FAKE_PACK),
+ ):
+ response = client.get(f"/language_pack/fr/{FAKE_VERSION}/script.js")
+
+ assert response.status_code == 200
+ assert response.mimetype == "application/javascript"
+ body = response.get_data(as_text=True)
+ assert body.startswith("window.__SUPERSET_LANGUAGE_PACK__ = ")
+ assert '"domain": "superset"' in body
+ cache_control = response.headers["Cache-Control"]
+ assert "immutable" in cache_control
+ assert "max-age=31536000" in cache_control
+ assert "public" in cache_control
+
+
+def test_script_not_cacheable_when_version_stale(client: Any) -> None:
+ """A pre-upgrade HTML page may reference an old version: serve fresh
+ content, but do not let caches pin it under the stale address."""
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ),
+ patch("superset.views.core.get_language_pack", return_value=FAKE_PACK),
+ ):
+ response = client.get("/language_pack/fr/000000000000/script.js")
+
+ assert response.status_code == 200
+ assert "no-cache" in response.headers["Cache-Control"]
+ assert "immutable" not in response.headers["Cache-Control"]
+
+
+def test_script_404_when_pack_missing(client: Any) -> None:
+ with patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=None,
+ ):
+ response = client.get(f"/language_pack/xx/{FAKE_VERSION}/script.js")
+
+ assert response.status_code == 404
+
+
+def test_script_rejects_malformed_lang_and_version(client: Any) -> None:
+ assert
client.get(f"/language_pack/../{FAKE_VERSION}/script.js").status_code in (
+ 400,
+ 404,
+ )
+ assert client.get("/language_pack/fr/not-a-hash!/script.js").status_code
== 400
+
+
+def test_script_serves_only_the_requested_locale(client: Any) -> None:
+ """The endpoint resolves exactly one pack: the locale in the URL."""
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ) as mock_version,
+ patch(
+ "superset.views.core.get_language_pack", return_value=FAKE_PACK
+ ) as mock_pack,
+ ):
+ client.get(f"/language_pack/pt_BR/{FAKE_VERSION}/script.js")
+
+ mock_version.assert_called_once_with("pt_BR")
+ mock_pack.assert_called_once_with("pt_BR")
+
+
+def test_spa_template_loads_pack_before_entry_bundle() -> None:
+ """Static guard on spa.html: the language pack script tag must precede
+ the entry bundle and stay a classic script (no async/defer). A deferred
+ or reordered tag would let code-split chunks evaluate module-level
+ `t('...')` calls before the translator is configured (issue #35330)."""
+ import superset
+
+ template: str = (
+ Path(superset.__file__).parent / "templates" / "superset" / "spa.html"
+ ).read_text()
+
+ tag_start: int = template.index('<script src="{{ language_pack_src }}"')
+ entry_start = template.index("js_bundle(assets_prefix, entry)")
Review Comment:
Yep, `entry_start` should match the annotated neighbors above it. Done in
95d1f6b.
##########
tests/unit_tests/views/test_language_pack_script.py:
##########
@@ -0,0 +1,115 @@
+# 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 pathlib import Path
+from typing import Any
+from unittest.mock import patch
+
+FAKE_PACK = {"domain": "superset", "locale_data": {"superset": {"": {}}}}
+FAKE_VERSION = "abc123def456"
+
+
+def test_script_served_immutable_when_version_matches(client: Any) -> None:
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ),
+ patch("superset.views.core.get_language_pack", return_value=FAKE_PACK),
+ ):
+ response = client.get(f"/language_pack/fr/{FAKE_VERSION}/script.js")
+
+ assert response.status_code == 200
+ assert response.mimetype == "application/javascript"
+ body = response.get_data(as_text=True)
+ assert body.startswith("window.__SUPERSET_LANGUAGE_PACK__ = ")
+ assert '"domain": "superset"' in body
+ cache_control = response.headers["Cache-Control"]
+ assert "immutable" in cache_control
+ assert "max-age=31536000" in cache_control
+ assert "public" in cache_control
+
+
+def test_script_not_cacheable_when_version_stale(client: Any) -> None:
+ """A pre-upgrade HTML page may reference an old version: serve fresh
+ content, but do not let caches pin it under the stale address."""
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ),
+ patch("superset.views.core.get_language_pack", return_value=FAKE_PACK),
+ ):
+ response = client.get("/language_pack/fr/000000000000/script.js")
+
+ assert response.status_code == 200
+ assert "no-cache" in response.headers["Cache-Control"]
+ assert "immutable" not in response.headers["Cache-Control"]
+
+
+def test_script_404_when_pack_missing(client: Any) -> None:
+ with patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=None,
+ ):
+ response = client.get(f"/language_pack/xx/{FAKE_VERSION}/script.js")
+
+ assert response.status_code == 404
+
+
+def test_script_rejects_malformed_lang_and_version(client: Any) -> None:
+ assert
client.get(f"/language_pack/../{FAKE_VERSION}/script.js").status_code in (
+ 400,
+ 404,
+ )
+ assert client.get("/language_pack/fr/not-a-hash!/script.js").status_code
== 400
+
+
+def test_script_serves_only_the_requested_locale(client: Any) -> None:
+ """The endpoint resolves exactly one pack: the locale in the URL."""
+ with (
+ patch(
+ "superset.views.core.get_language_pack_version",
+ return_value=FAKE_VERSION,
+ ) as mock_version,
+ patch(
+ "superset.views.core.get_language_pack", return_value=FAKE_PACK
+ ) as mock_pack,
+ ):
+ client.get(f"/language_pack/pt_BR/{FAKE_VERSION}/script.js")
+
+ mock_version.assert_called_once_with("pt_BR")
+ mock_pack.assert_called_once_with("pt_BR")
+
+
+def test_spa_template_loads_pack_before_entry_bundle() -> None:
+ """Static guard on spa.html: the language pack script tag must precede
+ the entry bundle and stay a classic script (no async/defer). A deferred
+ or reordered tag would let code-split chunks evaluate module-level
+ `t('...')` calls before the translator is configured (issue #35330)."""
+ import superset
+
+ template: str = (
+ Path(superset.__file__).parent / "templates" / "superset" / "spa.html"
+ ).read_text()
+
+ tag_start: int = template.index('<script src="{{ language_pack_src }}"')
+ entry_start = template.index("js_bundle(assets_prefix, entry)")
+ assert tag_start < entry_start
+
+ script_tag = template[tag_start : template.index(">", tag_start)]
Review Comment:
Same pass — `script_tag` annotated in 95d1f6b.
--
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]
