fix(parse): preserve source_name when zip single root differs#1991
Merged
qin-ctx merged 2 commits intoMay 12, 2026
Conversation
ZipParser collapsed any zip whose top level held a single directory and unconditionally dropped source_name. For CLI add-resource on a directory whose only child differs in name (e.g. user uploads "access/" containing just "access-dns/"), this silently dropped the user-named parent layer: the resource landed at "<parent>/access-dns" instead of "<parent>/access/access-dns". Only collapse when source_name is absent or its stem matches the single root dir name (the legacy "tt_b.zip" wrapping "tt_b/" case). When source_name names a distinct outer layer, parse the extract root and keep source_name so DirectoryParser preserves it as dir_name.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
qin-ctx
requested changes
May 12, 2026
Collaborator
qin-ctx
left a comment
qin-ctx
left a comment
There was a problem hiding this comment.
本次 review 发布 2 条评论：
- blocking：
source_name使用Path(...).stem可能导致v1.2这类同名目录不再折叠，产生v1.2/v1.2路径回归。 - non-blocking：建议增加 processor/API 级测试覆盖最终
root_uri，避免只验证 parser 调用参数。
Path(source_name).stem drops trailing dotted segments, so source_name "v1.2" against a zip whose single root is "v1.2/" would fail the equality check and re-wrap the content, producing "viking://resources/v1.2/v1.2/...". Compare the leaf name first so dotted names match exactly, and keep the stem fallback for the "tt_b.zip" wrapping "tt_b/" case. Also adds end-to-end regression tests that drive ZipParser + DirectoryParser + TreeBuilder.finalize_from_temp against an in-memory VikingFS and assert the final root_uri, so future changes in DirectoryParser or TreeBuilder can't silently drop the outer layer.
qin-ctx
approved these changes
May 12, 2026
ZaynJarvis
pushed a commit
that referenced
this pull request
May 13, 2026
* fix(parse): preserve source_name when zip single root differs ZipParser collapsed any zip whose top level held a single directory and unconditionally dropped source_name. For CLI add-resource on a directory whose only child differs in name (e.g. user uploads "access/" containing just "access-dns/"), this silently dropped the user-named parent layer: the resource landed at "<parent>/access-dns" instead of "<parent>/access/access-dns". Only collapse when source_name is absent or its stem matches the single root dir name (the legacy "tt_b.zip" wrapping "tt_b/" case). When source_name names a distinct outer layer, parse the extract root and keep source_name so DirectoryParser preserves it as dir_name. * fix(parse): compare basename before stem in zip single-root collapse Path(source_name).stem drops trailing dotted segments, so source_name "v1.2" against a zip whose single root is "v1.2/" would fail the equality check and re-wrap the content, producing "viking://resources/v1.2/v1.2/...". Compare the leaf name first so dotted names match exactly, and keep the stem fallback for the "tt_b.zip" wrapping "tt_b/" case. Also adds end-to-end regression tests that drive ZipParser + DirectoryParser + TreeBuilder.finalize_from_temp against an in-memory VikingFS and assert the final root_uri, so future changes in DirectoryParser or TreeBuilder can't silently drop the outer layer. --------- Co-authored-by: jinze <yanzexu.yzx@antgroup.com>
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
2 participants
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.
Description
ZipParser collapsed any zip whose top level held a single directory and unconditionally dropped source_name. For CLI add-resource on a directory whose only child differs in name (e.g. user uploads "access/" containing just "access-dns/"), this silently dropped the user-named parent layer: the resource landed at "/access-dns" instead of "/access/access-dns".
Only collapse when source_name is absent or its stem matches the single root dir name (the legacy "tt_b.zip" wrapping "tt_b/" case). When source_name names a distinct outer layer, parse the extract root and keep source_name so DirectoryParser preserves it as dir_name.
Related Issue
Type of Change
Changes Made
openviking/parse/parsers/zip_parser.py: gate the single-root-directory collapse onsource_namebeing absent or its stem matching the inner directory's name; otherwise parse the extract root and keepsource_name.tests/misc/test_media_processor_zip_root.py: add regressiontest_zip_single_top_level_dir_preserves_distinct_source_namefor theaccess/->access-dns/case.Testing
Checklist
Screenshots (if applicable)
Additional Notes