Jitmisra opened a new issue, #1049:
URL: https://github.com/apache/incubator-seata-go/issues/1049

   ### ✅ Verification Checklist
   
   - [x] 🔍 I have searched the [existing 
issues](https://github.com/apache/incubator-seata-go/issues) and confirmed this 
is not a duplicate
   - [x] 🛠️ I am willing to try to fix this bug myself.
   
   ### 🚀 Go Version
   
   1.20+
   
   ### 📦 Seata-go Version
   
   v2 (master branch)
   
   ### 💾 Operating System
   
   🍎 macOS
   
   ### 📝 Bug Description
   
   The `CompressorSevenz` type is registered as `"Sevenz"` in 
[`compressor_type.go` (Line 
27)](https://github.com/apache/incubator-seata-go/blob/master/pkg/compressor/compressor_type.go#L27),
 but there are two critical problems:
   
   1. **`7z_compress.go` is an empty file** — it contains only the Apache 
license header and `package compressor` declaration, with no struct or method 
implementations.
   2. **`GetCompressor()` has no `case CompressorSevenz:`** — so when a user 
configures Sevenz compression, the switch statement silently falls through to 
`default: return &NoneCompressor{}`.
   
   This means **users who configure `"Sevenz"` compression get no compression 
at all**, with no error, no warning, and no log output. Data is transmitted 
uncompressed while the user believes it is being compressed.
   
   This is the **same class of bug** as 
[#1012](https://github.com/apache/incubator-seata-go/issues/1012) (fixed in PR 
[#1013](https://github.com/apache/incubator-seata-go/pull/1013)), where 
`Zip.GetCompressorType()` returned `CompressorZstd` instead of `CompressorZip`.
   
   ### Current State of `7z_compress.go`
   
   The file only contains:
   
   ```go
   package compressor
   // (empty — no struct, no Compress/Decompress methods)
   ```
   
   ### Current State of `GetCompressor()` switch
   
   ```go
   func (c CompressorType) GetCompressor() Compressor {
       switch c {
       case CompressorNone:    return &NoneCompressor{}
       case CompressorGzip:    return &Gzip{}
       case CompressorZip:     return &Zip{}
       // ❌ No case for CompressorSevenz — falls to default!
       case CompressorBzip2:   return &Bzip2{}
       case CompressorLz4:     return &Lz4{}
       case CompressorZstd:    return &Zstd{}
       case CompressorSnappy:  return &Snappy{}
       case CompressorDeflate: return &DeflateCompress{}
       default:                return &NoneCompressor{}
       }
   }
   ```
   
   ### Additional Finding: Missing Unit Tests
   
   While investigating, I also found that the following files have **no unit 
tests**:
   
   | File                 | Test File  | Status                                |
   | -------------------- | ---------- | ------------------------------------- |
   | `7z_compress.go`     | ❌ Missing | No test file exists                   |
   | `none_compressor.go` | ❌ Missing | No test file exists                   |
   | `compressor_type.go` | ❌ Missing | `GetCompressor()` switch has no tests |
   
   A test for `GetCompressor()` covering all registered types would have 
**caught this bug** (and the previous #1012 bug) automatically.
   
   ### 🔄 Steps to Reproduce
   
   ## 🔄 Steps to Reproduce
   
   1. Configure the seata-go client with `CompressorSevenz` (`"Sevenz"`) 
compression type in `seatago.yml`:
      ```yaml
      seata:
        transport:
          compressor: Sevenz
      ```
   2. Start the application
   3. Observe that `GetCompressor()` is called with `CompressorSevenz`
   4. The switch statement has no matching case → falls to `default`
   5. `NoneCompressor` is returned → **data is sent uncompressed**
   
   
   ### ✅ Expected Behavior
   
   When a user configures `CompressorSevenz`, the system should:
   
   1. Return a working `Sevenz` compressor that uses LZMA compression (the core 
algorithm behind 7-Zip)
   2. Compress/decompress data correctly using LZMA
   3. Return the correct `CompressorSevenz` type from `GetCompressorType()`
   
   ```
   User configures CompressorSevenz
       → GetCompressor() called
       → Switch on CompressorType
       → case CompressorSevenz: return &Sevenz{}
       → ✅ Data compressed using LZMA algorithm
   ```
   
   ### ❌ Actual Behavior
   
   The system silently ignores the Sevenz configuration and returns 
`NoneCompressor`:
   
   ```
   User configures CompressorSevenz
       → GetCompressor() called
       → Switch on CompressorType
       → ❌ No matching case for CompressorSevenz
       → Falls to default
       → Returns NoneCompressor
       → Data sent UNCOMPRESSED
       → No error, no warning, no log
   
   ### 💡 Possible Solution
   
   I'd like to fix this. Here is my proposed approach:
   
   ### 1. Implement `Sevenz` compressor in `7z_compress.go`
   
   Implement the `Sevenz` struct using LZMA compression, matching the approach 
used in [Seata Java's 
SevenZCompressor](https://github.com/apache/incubator-seata/blob/develop/compressor/sevenzip/src/main/java/org/apache/seata/compressor/sevenz/SevenZCompressor.java)
 which uses LZMA internally.
   
   The Go implementation can use `github.com/ulikunitz/xz/lzma` (already 
present as a transitive dependency in `go.sum`).
   
   Following the same pattern as existing compressors (Snappy, Bzip2, etc.):
   
   ```
   Compressor (interface)
   ├── Compress([]byte) ([]byte, error)
   ├── Decompress([]byte) ([]byte, error)
   └── GetCompressorType() CompressorType
   
   Implementations:
   ├── ✅ NoneCompressor
   ├── ✅ Gzip
   ├── ✅ Zip
   ├── ✅ Bzip2
   ├── ✅ Lz4
   ├── ✅ Zstd
   ├── ✅ Snappy
   ├── ✅ DeflateCompress
   └── 🆕 Sevenz  ← NEW (using LZMA algorithm)
   ```
   
   ### 2. Add switch case in `compressor_type.go`
   
   Add `case CompressorSevenz: return &Sevenz{}` to the `GetCompressor()` 
switch.
   
   ### 3. Add missing unit tests
   
   - `7z_compress_test.go` — compress/decompress roundtrip + type assertion
   - `none_compressor_test.go` — pass-through verification + type assertion
   - `compressor_type_test.go` — table-driven test verifying `GetCompressor()` 
returns the correct type for **every** registered `CompressorType`, including 
the default fallback
   
   This would prevent similar regressions in the future (like #1012).
   


-- 
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]

Reply via email to