fix: implement CompressorSevenz and add missing compressor unit tests#1050
fix: implement CompressorSevenz and add missing compressor unit tests#1050Jitmisra wants to merge 6 commits intoapache:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1050 +/- ##
==========================================
+ Coverage 57.18% 57.36% +0.18%
==========================================
Files 267 268 +1
Lines 17526 17549 +23
==========================================
+ Hits 10022 10067 +45
+ Misses 6680 6653 -27
- Partials 824 829 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return nil, err | ||
| } | ||
|
|
||
| decompressed, err := ioutil.ReadAll(reader) |
There was a problem hiding this comment.
ioutil is deprecated. Wouldn't it be better to use io.ReadAll(reader) instead?
There was a problem hiding this comment.
ioutilis deprecated. Wouldn't it be better to useio.ReadAll(reader)instead?
sure thanks
| return nil, err | ||
| } | ||
|
|
||
| if err := writer.Close(); err != nil { |
There was a problem hiding this comment.
The write operation here throws an error, causing the Close operation to fail. Would defer be better?
|
If you are interested in SeataGo or would like to continue contributing, you can add me on WeChat |
9013a65 to
d015bab
Compare
Hi thunguo, Thank you for offering to connect on WeChat. Unfortunately, WeChat isn’t available in my region, so I’m unable to use it. Would it be possible to continue the discussion via email or any other platform you prefer? I’m very interested in continuing to contribute to Seata-Go, and I’m especially keen on working on the GSoC multi-database AT support idea this year. I’d really appreciate the opportunity to discuss how I can prepare and contribute effectively. Thank you again! |
d015bab to
e07f5ac
Compare
Okay, the community is very welcoming to new contributors, you can join through tew@apache.org Contact me via email or through zfeng@apache.org Contact Feng, the mentor of the multi-database project, via email. |
Thank you very much for the guidance and for sharing the contact details. I’ll reach out via email to continue the discussion and start contributing more actively to SeataGo. I truly appreciate the welcoming environment of the community and look forward to contributing further. |
AlexStocks
left a comment
There was a problem hiding this comment.
Code Review: fix: implement CompressorSevenz and add missing compressor unit tests
评价
问题修复正确,实现了缺失的 CompressorSevenz,并添加了完整的单元测试。测试覆盖率达到 77.2%。
[P1] 规范级问题
1. pkg/compressor/7z_compress.go - 命名混淆
文件名为 7z_compress.go,但实际使用的是 LZMA 算法(来自 github.com/ulikunitz/xz/lzma):
import "github.com/ulikunitz/xz/lzma"
func (s *Sevenz) Compress(data []byte) ([]byte, error) {
writer, err := lzma.NewWriter(&buffer)
// ...
}7z 格式支持多种算法(LZMA, LZMA2, PPMd 等),当前实现仅支持 LZMA。
建议:
- 重命名为
lzma_compress.go并将CompressorSevenz改为CompressorLzma - 或在注释中明确说明使用 LZMA 算法的原因
// Sevenz implements compression using LZMA algorithm.
// Note: This implementation uses the LZMA codec from xz library,
// which is the core algorithm used in 7-Zip. It does not produce
// actual .7z container format.
type Sevenz struct{}2. 边界测试缺失
测试用例缺少:
- 空数据压缩/解压
- 超大数据(>1MB)压缩
- 损坏数据解压错误处理
建议补充:
func TestSevenzCompress_EmptyData(t *testing.T) {
s := &Sevenz{}
compressed, err := s.Compress([]byte{})
assert.NoError(t, err)
decompressed, err := s.Decompress(compressed)
assert.NoError(t, err)
assert.Empty(t, decompressed)
}
func TestSevenzDecompress_InvalidData(t *testing.T) {
s := &Sevenz{}
_, err := s.Decompress([]byte{0x00, 0x01, 0x02})
assert.Error(t, err)
}3. changes/dev.md 格式问题
修改后的文件格式不一致:
- [[#130](...)] getty session auto close bug
+ - [[#130](...)] getty session auto close bug总体评价:修复正确有效,建议明确命名与实际实现的关系。
What this PR does
Fixes #1049 —
CompressorSevenztype was registered incompressor_type.gobut never implemented, causing users who configure"Sevenz"compression to silently receive no compression at all.This PR:
Sevenzcompressor in7z_compress.gousing LZMA (the core algorithm behind 7-Zip)CompressorSevenzinGetCompressor()7z_compress.go,none_compressor.go, and a table-driven test forcompressor_type.goWhich issue(s) this PR fixes
Fixes #1049
This is the same class of bug as #1012 (fixed in #1013), where
Zip.GetCompressorType()returnedCompressorZstdinstead ofCompressorZip.Bug Explained (Before → After)
Before (Broken)
flowchart LR A["User configures\ncompressor: Sevenz"] --> B["GetCompressor()\ncalled"] B --> C{"switch on\nCompressorType"} C -->|"No matching case"| D["default:\nreturn NoneCompressor"] D --> E["❌ Data sent\nUNCOMPRESSED"] style A fill:#ff6b6b,color:#fff style D fill:#ff6b6b,color:#fff style E fill:#ff6b6b,color:#fffAfter (Fixed)
flowchart LR A["User configures\ncompressor: Sevenz"] --> B["GetCompressor()\ncalled"] B --> C{"switch on\nCompressorType"} C -->|"case CompressorSevenz"| D["return &Sevenz{}"] D --> E["✅ Data compressed\nusing LZMA"] style A fill:#51cf66,color:#fff style D fill:#51cf66,color:#fff style E fill:#51cf66,color:#fffCompressor Architecture
All compressors implement the same
Compressorinterface:classDiagram class Compressor { <<interface>> +Compress([]byte) ([]byte, error) +Decompress([]byte) ([]byte, error) +GetCompressorType() CompressorType } Compressor <|.. NoneCompressor Compressor <|.. Gzip Compressor <|.. Zip Compressor <|.. Sevenz : 🆕 NEW Compressor <|.. Bzip2 Compressor <|.. Lz4 Compressor <|.. Zstd Compressor <|.. Snappy Compressor <|.. DeflateCompress class Sevenz { +Compress(data []byte) ([]byte, error) +Decompress(data []byte) ([]byte, error) +GetCompressorType() CompressorType } note for Sevenz "Uses github.com/ulikunitz/xz/lzma\n(LZMA algorithm — core of 7-Zip)"Changed Files
pkg/compressor/7z_compress.goSevenzstruct with LZMACompress/Decompressmethodspkg/compressor/compressor_type.gocase CompressorSevenz: return &Sevenz{}toGetCompressor()pkg/compressor/7z_compress_test.gopkg/compressor/none_compressor_test.gopkg/compressor/compressor_type_test.goCompressorTypevalues + unknown fallbackgo.modgithub.com/ulikunitz/xzto direct dependencygo.sumchanges/dev.mdKey Code Changes
1.
7z_compress.go— Sevenz Implementation2.
compressor_type.go— Switch Case Additioncase CompressorZip: return &Zip{} + case CompressorSevenz: + return &Sevenz{} case CompressorBzip2: return &Bzip2{}3.
compressor_type_test.go— Table-Driven Test (Prevents Future Regressions)Verification Results
Unit Tests — 10/10 PASS ✅
Additional Checks
go vetgo test -race)