codeant-ai-for-open-source[bot] commented on code in PR #43188:
URL: https://github.com/apache/superset/pull/43188#discussion_r3818868240
##########
superset/utils/file.py:
##########
@@ -32,5 +37,13 @@ def sanitize_title(title: str) -> str:
def get_filename(model_name: str, model_id: int, skip_id: bool = False) -> str:
model_name = sanitize_title(model_name)
slug = secure_filename(model_name)
- filename = slug if skip_id else f"{slug}_{model_id}"
- return filename if slug else str(model_id)
+ suffix = "" if skip_id else f"_{model_id}"
+ # The name goes into a ZIP entry that already carries an
+ # `<asset>_export_<timestamp>/<type>/` prefix and a `.yaml` suffix, and the
+ # user's own extraction directory sits in front of all of it. A chart
titled
+ # with a couple of hundred characters therefore produced an entry Windows
+ # refuses to extract, even though the archive itself was written fine. Trim
+ # the slug rather than the id: the id is what keeps two similarly titled
+ # assets from colliding inside one archive.
+ slug = slug[: max(MAX_FILENAME_LENGTH - len(suffix), 0)].rstrip("._-")
Review Comment:
Yes, the issue is valid: with `skip_id=True`, truncation can collapse
distinct long titles into the same archive path, allowing the export `seen` set
to omit an asset.
Use a deterministic digest of the full slug when truncation is required,
while keeping the result within `MAX_FILENAME_LENGTH`:
```python
import hashlib
```
```python
suffix = "" if skip_id else f"_{model_id}"
max_slug_length = max(MAX_FILENAME_LENGTH - len(suffix), 0)
if len(slug) + len(suffix) > MAX_FILENAME_LENGTH and skip_id:
disambiguator = hashlib.sha256(slug.encode("utf-8")).hexdigest()[:8]
suffix = f"_{disambiguator}"
max_slug_length = max(MAX_FILENAME_LENGTH - len(suffix), 0)
slug = slug[:max_slug_length].rstrip("._-")
return f"{slug}{suffix}" if slug else str(model_id)
```
This preserves existing short filenames and ID-based names, while producing
distinct paths such as:
```text
Very_long_database_name_..._a1b2c3d4
Very_long_database_name_..._e5f6a7b8
```
A test should also assert that two long, otherwise-colliding `skip_id=True`
names produce different filenames.
--
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]
