Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Reviewer's Guide添加基于地理位置的 GitHub 仓库重定向，将 WebUI 静态资源处理切换为自托管、可缓存且对反向代理友好的路径，移除外部字体/CDN 依赖，并新增测试以覆盖与位置相关的部署行为和静态资源服务。 基于地理位置的 GitHub 仓库重定向时序图sequenceDiagram
actor User
participant ConfigModel
participant geo as get_country_code
participant logger
User->>ConfigModel: __init__(file)
ConfigModel->>ConfigModel: read()
ConfigModel->>ConfigModel: config_redirect()
ConfigModel->>ConfigModel: _redirect_github_repository()
alt _github_location_checked or Repository != GITHUB_REPOSITORY
ConfigModel-->>ConfigModel: return
else first_time_and_github
ConfigModel->>geo: get_country_code(timeout)
geo-->>ConfigModel: country_code
alt country_code == 'cn'
ConfigModel->>logger: info
ConfigModel->>ConfigModel: Repository = GIT_OVER_CDN_REPOSITORY
ConfigModel->>ConfigModel: config['Repository'] = GIT_OVER_CDN_REPOSITORY
else country_code is None
ConfigModel->>logger: warning
else other_country
ConfigModel->>logger: info
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your Experience访问你的 dashboard 以：
Getting HelpOriginal review guide in EnglishReviewer's GuideAdds geo-based GitHub repository redirection, switches WebUI static asset handling to self-hosted, cache-safe, reverse-proxy-friendly paths, removes external font/CDN dependencies, and introduces tests to cover location-aware deploy behavior and static asset serving. Sequence diagram for geo-based GitHub repository redirectionsequenceDiagram
actor User
participant ConfigModel
participant geo as get_country_code
participant logger
User->>ConfigModel: __init__(file)
ConfigModel->>ConfigModel: read()
ConfigModel->>ConfigModel: config_redirect()
ConfigModel->>ConfigModel: _redirect_github_repository()
alt _github_location_checked or Repository != GITHUB_REPOSITORY
ConfigModel-->>ConfigModel: return
else first_time_and_github
ConfigModel->>geo: get_country_code(timeout)
geo-->>ConfigModel: country_code
alt country_code == 'cn'
ConfigModel->>logger: info
ConfigModel->>ConfigModel: Repository = GIT_OVER_CDN_REPOSITORY
ConfigModel->>ConfigModel: config['Repository'] = GIT_OVER_CDN_REPOSITORY
else country_code is None
ConfigModel->>logger: warning
else other_country
ConfigModel->>logger: info
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些高层次的反馈：
_redirect_github_repository逻辑同时出现在deploy/config.py和deploy/Windows/config.py中；建议抽取一个共享的辅助函数（例如放在deploy.geo或一个小的公共模块里），以避免未来更新该行为时，不同平台之间产生偏差。_versioned_static_asset辅助函数在导入时直接读取资源文件并计算哈希，而且没有错误处理；如果文件缺失或不可读（例如在打包/捆绑环境中），导入module.webui.app时会抛异常——建议改为惰性求值，或者用 try/except 包裹 IO 并提供合理的回退路径。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_redirect_github_repository` logic is duplicated in both `deploy/config.py` and `deploy/Windows/config.py`; consider extracting a shared helper (e.g., in `deploy.geo` or a small common module) to avoid drift between platforms when this behavior is updated in future.
- The `_versioned_static_asset` helper reads asset files and computes hashes at import time without error handling; if the file is missing or unreadable (e.g., in a packaged/bundled environment), importing `module.webui.app` will raise—consider lazy evaluation or wrapping the IO in a try/except with a sensible fallback path.
## Individual Comments
### Comment 1
<location path="module/webui/app.py" line_range="84-87" />
<code_context>
+PROJECT_ROOT = Path(__file__).resolve().parents[2]
+
+
+def _versioned_static_asset(relative_path: str) -> str:
+ """返回带内容哈希的相对静态资源地址。"""
+ digest = sha256((PROJECT_ROOT / relative_path).read_bytes()).hexdigest()[:12]
+ return f"static/{relative_path}?v={digest}"
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle missing/invalid asset files in _versioned_static_asset to avoid startup-time crashes.
If this file doesn’t exist or the path is wrong, this will raise (e.g. `FileNotFoundError`) when `INITIAL_WEBUI_CSS` is resolved and can prevent the app from starting. Please catch file/IO errors here and either fall back to an unversioned path or return/log a safe default so a missing asset doesn’t break the web UI.
</issue_to_address>
### Comment 2
<location path="tests/test_webui_static_assets.py" line_range="43-52" />
<code_context>
+ def tearDown(self):
+ self._temporary_directory.cleanup()
+
+ def test_mounts_only_public_static_directories(self):
+ css_response = self.client.get("/static/assets/gui/css/test.css")
+ logo_response = self.client.get("/static/doc/logo.webp")
+
+ self.assertEqual(css_response.status_code, 200)
+ self.assertEqual(css_response.headers["content-type"], "text/css; charset=utf-8")
+ self.assertEqual(css_response.headers["cache-control"], "no-cache")
+ self.assertEqual(logo_response.status_code, 200)
+ self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
+ self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
+
+ def test_relative_css_url_preserves_reverse_proxy_prefix(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Also assert cache-control headers for non-CSS assets and tightening static asset expectations
Since the middleware sets `Cache-Control: no-cache` for cacheable static assets, please also assert `logo_response.headers["cache-control"] == "no-cache"` to cover binary assets. Additionally, consider a focused test for the versioned CSS helper (e.g. calling `_versioned_static_asset` on a test file and asserting it returns a relative URL with a `?v=` query parameter of the expected length) to directly validate the cache-busting behavior.
Suggested implementation:
```python
self.assertEqual(css_response.headers["cache-control"], "no-cache")
self.assertEqual(logo_response.status_code, 200)
self.assertEqual(logo_response.headers["cache-control"], "no-cache")
self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
def test_versioned_static_asset_adds_cache_busting_query_param(self):
# Use a representative CSS asset path that exists in the test assets directory
path = "assets/gui/css/test.css"
versioned_path = _versioned_static_asset(path)
# The helper should return a relative URL under /static that includes the original path
self.assertTrue(versioned_path.startswith("static/"))
self.assertIn(path, versioned_path)
# Expect a cache-busting query parameter named "v" with a non-empty value
self.assertIn("?v=", versioned_path)
version = versioned_path.split("?v=", 1)[1]
# Assert a minimum length to ensure it's not just an empty or trivial token
self.assertGreaterEqual(len(version), 8)
def test_relative_css_url_preserves_reverse_proxy_prefix(self):
css_url = urljoin(
```
1. Ensure `_versioned_static_asset` is imported or otherwise available in this test module, e.g.:
- `from <your_webui_module> import _versioned_static_asset`
Adjust `<your_webui_module>` to match the actual module that defines the helper.
2. If the helper returns URLs with a different prefix (e.g. `/static/` instead of `static/`), update the `startswith` assertion accordingly.
3. If the implementation uses a fixed-length version token (e.g. 12 or 16 characters), you may tighten the `len(version)` assertion to match that exact length rather than a minimum of 8.
4. Confirm that `"assets/gui/css/test.css"` corresponds to an existing file in the test assets directory; if not, adjust `path` to a file that is present so the helper behaves as expected.
</issue_to_address>
### Comment 3
<location path="tests/test_webui_static_assets.py" line_range="69-74" />
<code_context>
+ self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
+ self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
+
+ def test_relative_css_url_preserves_reverse_proxy_prefix(self):
+ css_url = urljoin(
+ "https://example.test/azur/", "static/assets/gui/css/alas.css"
+ )
+ font_url = urljoin(css_url, "../../spa/MiSans-Demibold.ttf")
+ logo_url = urljoin(css_url, "../../../doc/logo.webp")
+
+ self.assertEqual(
+ css_url, "https://example.test/azur/static/assets/gui/css/alas.css"
+ )
+ self.assertEqual(
+ font_url, "https://example.test/azur/static/assets/spa/MiSans-Demibold.ttf"
+ )
+ self.assertEqual(logo_url, "https://example.test/azur/static/doc/logo.webp")
+
+ def test_default_pywebio_assets_are_self_hosted(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for manifest and JS asset relative URLs injected by the Web UI
You’re already covering relative paths in CSS. There’s similar logic in `app_home._load_deferred_client_assets` that switches the PWA manifest and JS bundle from `/static/...` to `static/...`. To cover reverse proxies and non-root paths end-to-end, consider a functional test that renders the index page (or triggers the PyWebIO injection) and asserts that the `link[rel="manifest"]` and `alas-utils-script` tags use `static/...` URLs rather than `/static/...`.
```suggestion
def test_default_pywebio_assets_are_self_hosted(self):
response = TestClient(asgi_app({"index": lambda: None})).get("/")
self.assertEqual(response.status_code, 200)
self.assertIn('href="pywebio_static/css/app.css?v=', response.text)
self.assertNotIn("cdn.jsdelivr.net", response.text)
# PWA manifest should be injected with a relative static path, not an absolute /static path
self.assertIn('rel="manifest"', response.text)
self.assertIn('href="static/', response.text)
self.assertNotIn('href="/static/', response.text)
# Deferred JS bundle should also use a relative static path
self.assertIn('id="alas-utils-script"', response.text)
self.assertIn('src="static/', response.text)
self.assertNotIn('src="/static/', response.text)
```
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- The
_redirect_github_repositorylogic is duplicated in bothdeploy/config.pyanddeploy/Windows/config.py; consider extracting a shared helper (e.g., indeploy.geoor a small common module) to avoid drift between platforms when this behavior is updated in future. - The
_versioned_static_assethelper reads asset files and computes hashes at import time without error handling; if the file is missing or unreadable (e.g., in a packaged/bundled environment), importingmodule.webui.appwill raise—consider lazy evaluation or wrapping the IO in a try/except with a sensible fallback path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_redirect_github_repository` logic is duplicated in both `deploy/config.py` and `deploy/Windows/config.py`; consider extracting a shared helper (e.g., in `deploy.geo` or a small common module) to avoid drift between platforms when this behavior is updated in future.
- The `_versioned_static_asset` helper reads asset files and computes hashes at import time without error handling; if the file is missing or unreadable (e.g., in a packaged/bundled environment), importing `module.webui.app` will raise—consider lazy evaluation or wrapping the IO in a try/except with a sensible fallback path.
## Individual Comments
### Comment 1
<location path="module/webui/app.py" line_range="84-87" />
<code_context>
+PROJECT_ROOT = Path(__file__).resolve().parents[2]
+
+
+def _versioned_static_asset(relative_path: str) -> str:
+ """返回带内容哈希的相对静态资源地址。"""
+ digest = sha256((PROJECT_ROOT / relative_path).read_bytes()).hexdigest()[:12]
+ return f"static/{relative_path}?v={digest}"
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle missing/invalid asset files in _versioned_static_asset to avoid startup-time crashes.
If this file doesn’t exist or the path is wrong, this will raise (e.g. `FileNotFoundError`) when `INITIAL_WEBUI_CSS` is resolved and can prevent the app from starting. Please catch file/IO errors here and either fall back to an unversioned path or return/log a safe default so a missing asset doesn’t break the web UI.
</issue_to_address>
### Comment 2
<location path="tests/test_webui_static_assets.py" line_range="43-52" />
<code_context>
+ def tearDown(self):
+ self._temporary_directory.cleanup()
+
+ def test_mounts_only_public_static_directories(self):
+ css_response = self.client.get("/static/assets/gui/css/test.css")
+ logo_response = self.client.get("/static/doc/logo.webp")
+
+ self.assertEqual(css_response.status_code, 200)
+ self.assertEqual(css_response.headers["content-type"], "text/css; charset=utf-8")
+ self.assertEqual(css_response.headers["cache-control"], "no-cache")
+ self.assertEqual(logo_response.status_code, 200)
+ self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
+ self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
+
+ def test_relative_css_url_preserves_reverse_proxy_prefix(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Also assert cache-control headers for non-CSS assets and tightening static asset expectations
Since the middleware sets `Cache-Control: no-cache` for cacheable static assets, please also assert `logo_response.headers["cache-control"] == "no-cache"` to cover binary assets. Additionally, consider a focused test for the versioned CSS helper (e.g. calling `_versioned_static_asset` on a test file and asserting it returns a relative URL with a `?v=` query parameter of the expected length) to directly validate the cache-busting behavior.
Suggested implementation:
```python
self.assertEqual(css_response.headers["cache-control"], "no-cache")
self.assertEqual(logo_response.status_code, 200)
self.assertEqual(logo_response.headers["cache-control"], "no-cache")
self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
def test_versioned_static_asset_adds_cache_busting_query_param(self):
# Use a representative CSS asset path that exists in the test assets directory
path = "assets/gui/css/test.css"
versioned_path = _versioned_static_asset(path)
# The helper should return a relative URL under /static that includes the original path
self.assertTrue(versioned_path.startswith("static/"))
self.assertIn(path, versioned_path)
# Expect a cache-busting query parameter named "v" with a non-empty value
self.assertIn("?v=", versioned_path)
version = versioned_path.split("?v=", 1)[1]
# Assert a minimum length to ensure it's not just an empty or trivial token
self.assertGreaterEqual(len(version), 8)
def test_relative_css_url_preserves_reverse_proxy_prefix(self):
css_url = urljoin(
```
1. Ensure `_versioned_static_asset` is imported or otherwise available in this test module, e.g.:
- `from <your_webui_module> import _versioned_static_asset`
Adjust `<your_webui_module>` to match the actual module that defines the helper.
2. If the helper returns URLs with a different prefix (e.g. `/static/` instead of `static/`), update the `startswith` assertion accordingly.
3. If the implementation uses a fixed-length version token (e.g. 12 or 16 characters), you may tighten the `len(version)` assertion to match that exact length rather than a minimum of 8.
4. Confirm that `"assets/gui/css/test.css"` corresponds to an existing file in the test assets directory; if not, adjust `path` to a file that is present so the helper behaves as expected.
</issue_to_address>
### Comment 3
<location path="tests/test_webui_static_assets.py" line_range="69-74" />
<code_context>
+ self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
+ self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
+
+ def test_relative_css_url_preserves_reverse_proxy_prefix(self):
+ css_url = urljoin(
+ "https://example.test/azur/", "static/assets/gui/css/alas.css"
+ )
+ font_url = urljoin(css_url, "../../spa/MiSans-Demibold.ttf")
+ logo_url = urljoin(css_url, "../../../doc/logo.webp")
+
+ self.assertEqual(
+ css_url, "https://example.test/azur/static/assets/gui/css/alas.css"
+ )
+ self.assertEqual(
+ font_url, "https://example.test/azur/static/assets/spa/MiSans-Demibold.ttf"
+ )
+ self.assertEqual(logo_url, "https://example.test/azur/static/doc/logo.webp")
+
+ def test_default_pywebio_assets_are_self_hosted(self):
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for manifest and JS asset relative URLs injected by the Web UI
You’re already covering relative paths in CSS. There’s similar logic in `app_home._load_deferred_client_assets` that switches the PWA manifest and JS bundle from `/static/...` to `static/...`. To cover reverse proxies and non-root paths end-to-end, consider a functional test that renders the index page (or triggers the PyWebIO injection) and asserts that the `link[rel="manifest"]` and `alas-utils-script` tags use `static/...` URLs rather than `/static/...`.
```suggestion
def test_default_pywebio_assets_are_self_hosted(self):
response = TestClient(asgi_app({"index": lambda: None})).get("/")
self.assertEqual(response.status_code, 200)
self.assertIn('href="pywebio_static/css/app.css?v=', response.text)
self.assertNotIn("cdn.jsdelivr.net", response.text)
# PWA manifest should be injected with a relative static path, not an absolute /static path
self.assertIn('rel="manifest"', response.text)
self.assertIn('href="static/', response.text)
self.assertNotIn('href="/static/', response.text)
# Deferred JS bundle should also use a relative static path
self.assertIn('id="alas-utils-script"', response.text)
self.assertIn('src="static/', response.text)
self.assertNotIn('src="/static/', response.text)
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+84
to
+87
| def _versioned_static_asset(relative_path: str) -> str: | ||
| """返回带内容哈希的相对静态资源地址。""" | ||
| digest = sha256((PROJECT_ROOT / relative_path).read_bytes()).hexdigest()[:12] | ||
| return f"static/{relative_path}?v={digest}" |
There was a problem hiding this comment.
issue (bug_risk): Handle missing/invalid asset files in _versioned_static_asset to avoid startup-time crashes.
If this file doesn’t exist or the path is wrong, this will raise (e.g. FileNotFoundError) when INITIAL_WEBUI_CSS is resolved and can prevent the app from starting. Please catch file/IO errors here and either fall back to an unversioned path or return/log a safe default so a missing asset doesn’t break the web UI.
Original comment in English
issue (bug_risk): Handle missing/invalid asset files in _versioned_static_asset to avoid startup-time crashes.
If this file doesn’t exist or the path is wrong, this will raise (e.g. FileNotFoundError) when INITIAL_WEBUI_CSS is resolved and can prevent the app from starting. Please catch file/IO errors here and either fall back to an unversioned path or return/log a safe default so a missing asset doesn’t break the web UI.
Comment on lines
+43
to
+52
| def test_mounts_only_public_static_directories(self): | ||
| css_response = self.client.get("/static/assets/gui/css/test.css") | ||
| logo_response = self.client.get("/static/doc/logo.webp") | ||
|
|
||
| self.assertEqual(css_response.status_code, 200) | ||
| self.assertEqual(css_response.headers["content-type"], "text/css; charset=utf-8") | ||
| self.assertEqual(css_response.headers["cache-control"], "no-cache") | ||
| self.assertEqual(logo_response.status_code, 200) | ||
| self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404) | ||
| self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404) |
There was a problem hiding this comment.
suggestion (testing): Also assert cache-control headers for non-CSS assets and tightening static asset expectations
Since the middleware sets Cache-Control: no-cache for cacheable static assets, please also assert logo_response.headers["cache-control"] == "no-cache" to cover binary assets. Additionally, consider a focused test for the versioned CSS helper (e.g. calling _versioned_static_asset on a test file and asserting it returns a relative URL with a ?v= query parameter of the expected length) to directly validate the cache-busting behavior.
Suggested implementation:
self.assertEqual(css_response.headers["cache-control"], "no-cache")
self.assertEqual(logo_response.status_code, 200)
self.assertEqual(logo_response.headers["cache-control"], "no-cache")
self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
def test_versioned_static_asset_adds_cache_busting_query_param(self):
# Use a representative CSS asset path that exists in the test assets directory
path = "assets/gui/css/test.css"
versioned_path = _versioned_static_asset(path)
# The helper should return a relative URL under /static that includes the original path
self.assertTrue(versioned_path.startswith("static/"))
self.assertIn(path, versioned_path)
# Expect a cache-busting query parameter named "v" with a non-empty value
self.assertIn("?v=", versioned_path)
version = versioned_path.split("?v=", 1)[1]
# Assert a minimum length to ensure it's not just an empty or trivial token
self.assertGreaterEqual(len(version), 8)
def test_relative_css_url_preserves_reverse_proxy_prefix(self):
css_url = urljoin(- Ensure
_versioned_static_assetis imported or otherwise available in this test module, e.g.:from <your_webui_module> import _versioned_static_asset
Adjust<your_webui_module>to match the actual module that defines the helper.
- If the helper returns URLs with a different prefix (e.g.
/static/instead ofstatic/), update thestartswithassertion accordingly. - If the implementation uses a fixed-length version token (e.g. 12 or 16 characters), you may tighten the
len(version)assertion to match that exact length rather than a minimum of 8. - Confirm that
"assets/gui/css/test.css"corresponds to an existing file in the test assets directory; if not, adjustpathto a file that is present so the helper behaves as expected.
Original comment in English
suggestion (testing): Also assert cache-control headers for non-CSS assets and tightening static asset expectations
Since the middleware sets Cache-Control: no-cache for cacheable static assets, please also assert logo_response.headers["cache-control"] == "no-cache" to cover binary assets. Additionally, consider a focused test for the versioned CSS helper (e.g. calling _versioned_static_asset on a test file and asserting it returns a relative URL with a ?v= query parameter of the expected length) to directly validate the cache-busting behavior.
Suggested implementation:
self.assertEqual(css_response.headers["cache-control"], "no-cache")
self.assertEqual(logo_response.status_code, 200)
self.assertEqual(logo_response.headers["cache-control"], "no-cache")
self.assertEqual(self.client.get("/static/config/deploy.yaml").status_code, 404)
self.assertEqual(self.client.get("/static/.git/HEAD").status_code, 404)
def test_versioned_static_asset_adds_cache_busting_query_param(self):
# Use a representative CSS asset path that exists in the test assets directory
path = "assets/gui/css/test.css"
versioned_path = _versioned_static_asset(path)
# The helper should return a relative URL under /static that includes the original path
self.assertTrue(versioned_path.startswith("static/"))
self.assertIn(path, versioned_path)
# Expect a cache-busting query parameter named "v" with a non-empty value
self.assertIn("?v=", versioned_path)
version = versioned_path.split("?v=", 1)[1]
# Assert a minimum length to ensure it's not just an empty or trivial token
self.assertGreaterEqual(len(version), 8)
def test_relative_css_url_preserves_reverse_proxy_prefix(self):
css_url = urljoin(- Ensure
_versioned_static_assetis imported or otherwise available in this test module, e.g.:from <your_webui_module> import _versioned_static_asset
Adjust<your_webui_module>to match the actual module that defines the helper.
- If the helper returns URLs with a different prefix (e.g.
/static/instead ofstatic/), update thestartswithassertion accordingly. - If the implementation uses a fixed-length version token (e.g. 12 or 16 characters), you may tighten the
len(version)assertion to match that exact length rather than a minimum of 8. - Confirm that
"assets/gui/css/test.css"corresponds to an existing file in the test assets directory; if not, adjustpathto a file that is present so the helper behaves as expected.
Comment on lines
+69
to
+74
| def test_default_pywebio_assets_are_self_hosted(self): | ||
| response = TestClient(asgi_app({"index": lambda: None})).get("/") | ||
|
|
||
| self.assertEqual(response.status_code, 200) | ||
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | ||
| self.assertNotIn("cdn.jsdelivr.net", response.text) |
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for manifest and JS asset relative URLs injected by the Web UI
You’re already covering relative paths in CSS. There’s similar logic in app_home._load_deferred_client_assets that switches the PWA manifest and JS bundle from /static/... to static/.... To cover reverse proxies and non-root paths end-to-end, consider a functional test that renders the index page (or triggers the PyWebIO injection) and asserts that the link[rel="manifest"] and alas-utils-script tags use static/... URLs rather than /static/....
Suggested change
| def test_default_pywebio_assets_are_self_hosted(self): | |
| response = TestClient(asgi_app({"index": lambda: None})).get("/") | |
| self.assertEqual(response.status_code, 200) | |
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | |
| self.assertNotIn("cdn.jsdelivr.net", response.text) | |
| def test_default_pywebio_assets_are_self_hosted(self): | |
| response = TestClient(asgi_app({"index": lambda: None})).get("/") | |
| self.assertEqual(response.status_code, 200) | |
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | |
| self.assertNotIn("cdn.jsdelivr.net", response.text) | |
| # PWA manifest should be injected with a relative static path, not an absolute /static path | |
| self.assertIn('rel="manifest"', response.text) | |
| self.assertIn('href="static/', response.text) | |
| self.assertNotIn('href="/static/', response.text) | |
| # Deferred JS bundle should also use a relative static path | |
| self.assertIn('id="alas-utils-script"', response.text) | |
| self.assertIn('src="static/', response.text) | |
| self.assertNotIn('src="/static/', response.text) |
Original comment in English
suggestion (testing): Consider adding tests for manifest and JS asset relative URLs injected by the Web UI
You’re already covering relative paths in CSS. There’s similar logic in app_home._load_deferred_client_assets that switches the PWA manifest and JS bundle from /static/... to static/.... To cover reverse proxies and non-root paths end-to-end, consider a functional test that renders the index page (or triggers the PyWebIO injection) and asserts that the link[rel="manifest"] and alas-utils-script tags use static/... URLs rather than /static/....
Suggested change
| def test_default_pywebio_assets_are_self_hosted(self): | |
| response = TestClient(asgi_app({"index": lambda: None})).get("/") | |
| self.assertEqual(response.status_code, 200) | |
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | |
| self.assertNotIn("cdn.jsdelivr.net", response.text) | |
| def test_default_pywebio_assets_are_self_hosted(self): | |
| response = TestClient(asgi_app({"index": lambda: None})).get("/") | |
| self.assertEqual(response.status_code, 200) | |
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | |
| self.assertNotIn("cdn.jsdelivr.net", response.text) | |
| # PWA manifest should be injected with a relative static path, not an absolute /static path | |
| self.assertIn('rel="manifest"', response.text) | |
| self.assertIn('href="static/', response.text) | |
| self.assertNotIn('href="/static/', response.text) | |
| # Deferred JS bundle should also use a relative static path | |
| self.assertIn('id="alas-utils-script"', response.text) | |
| self.assertIn('src="static/', response.text) | |
| self.assertNotIn('src="/static/', response.text) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
自动检测部署位置以选择合适的更新源，并通过自托管、带缓存破坏参数的资源来强化 Web UI 静态资源的交付。
New Features:
Enhancements:
Build:
Documentation:
Tests:
Original summary in English
Summary by Sourcery
Auto-detect deployment location to choose appropriate update sources and harden Web UI static asset delivery with self-hosted, cache-busted resources.
New Features:
Enhancements:
Build:
Documentation:
Tests: