Skip to content

Commit

Permalink
Nits: Addressing review comments in the compressor
Browse files Browse the repository at this point in the history
  • Loading branch information
rnishtala-sumo committed Nov 20, 2024
1 parent a70ad2c commit e843c4b
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 30 deletions.
1 change: 0 additions & 1 deletion config/configcompression/compressiontype.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ func (ct *TypeWithLevel) UnmarshalText(in []byte) error {
}

return fmt.Errorf("unsupported compression type and level(default if not specified) %s - %d", compressionTyp, ct.Level)

}

// Checks the validity of zlib/gzip/flate compression levels
Expand Down
30 changes: 11 additions & 19 deletions config/confighttp/compression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"io"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
"testing/iotest"
Expand All @@ -37,12 +36,11 @@ func TestHTTPClientCompression(t *testing.T) {
compressedLz4Body := compressLz4(t, testBody)

const (
GzipLevel configcompression.Type = "gzip/1"
ZlibLevel configcompression.Type = "zlib/1"
DeflateLevel configcompression.Type = "deflate/1"
ZstdLevel configcompression.Type = "zstd/11"
gzipLevel configcompression.Type = "gzip/1"
zlibLevel configcompression.Type = "zlib/1"
deflateLevel configcompression.Type = "deflate/1"
zstdLevel configcompression.Type = "zstd/11"
)
var level int

tests := []struct {
name string
Expand All @@ -64,19 +62,19 @@ func TestHTTPClientCompression(t *testing.T) {
},
{
name: "ValidGzip",
encoding: GzipLevel,
encoding: gzipLevel,
reqBody: compressedGzipBody.Bytes(),
shouldError: false,
},
{
name: "ValidZlib",
encoding: ZlibLevel,
encoding: zlibLevel,
reqBody: compressedZlibBody.Bytes(),
shouldError: false,
},
{
name: "ValidDeflate",
encoding: DeflateLevel,
encoding: deflateLevel,
reqBody: compressedDeflateBody.Bytes(),
shouldError: false,
},
Expand All @@ -88,7 +86,7 @@ func TestHTTPClientCompression(t *testing.T) {
},
{
name: "ValidZstd",
encoding: ZstdLevel,
encoding: zstdLevel,
reqBody: compressedZstdBody.Bytes(),
shouldError: false,
},
Expand All @@ -113,15 +111,9 @@ func TestHTTPClientCompression(t *testing.T) {

req, err := http.NewRequest(http.MethodGet, srv.URL, reqBody)
require.NoError(t, err, "failed to create request to test handler")
parts := strings.Split(string(tt.encoding), "/")
compressionLevel := configcompression.Level(gzip.BestSpeed)
compressionType := configcompression.Type(parts[0])
if len(parts) == 2 {
level, err = strconv.Atoi(parts[1])
require.NoError(t, err)
compressionLevel = configcompression.Level(level)
}
compression := configcompression.TypeWithLevel{Type: compressionType, Level: compressionLevel}
compression := configcompression.TypeWithLevel{}
err = compression.UnmarshalText([]byte(tt.encoding))
require.NoError(t, err)
clientSettings := ClientConfig{
Endpoint: srv.URL,
Compression: compression,
Expand Down
16 changes: 6 additions & 10 deletions config/confighttp/compressor.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ type compressor struct {

var (
compressorPools = &compressorMap{pools: make(map[string]*compressor)}
gZipCompressor = &compressor{}
snappyCompressor = &compressor{}
zstdCompressor = &compressor{}
zlibCompressor = &compressor{}
lz4Compressor = &compressor{}
_ writeCloserReset = (*gzip.Writer)(nil)
_ writeCloserReset = (*snappy.Writer)(nil)
Expand All @@ -50,11 +47,10 @@ var (
// The validity of input is already checked when NewCompressRoundTripper was called in confighttp,
func newCompressor(compressionType configcompression.TypeWithLevel) (*compressor, error) {
mapKey := fmt.Sprintf("%s/%d", compressionType.Type, compressionType.Level)
var exists bool
switch compressionType.Type {
case configcompression.TypeGzip:
gZipCompressor, exists = compressorPools.pools[mapKey]
if exists {
gZipCompressor, gzipExists := compressorPools.pools[mapKey]
if gzipExists {
return gZipCompressor, nil
}
gZipCompressor = &compressor{}
Expand All @@ -68,18 +64,18 @@ func newCompressor(compressionType configcompression.TypeWithLevel) (*compressor
}
return snappyCompressor, nil

Check warning on line 65 in config/confighttp/compressor.go

View check run for this annotation

Codecov / codecov/patch

config/confighttp/compressor.go#L65

Added line #L65 was not covered by tests
case configcompression.TypeZstd:
zstdCompressor, exists = compressorPools.pools[mapKey]
zstdCompressor, zstdExists := compressorPools.pools[mapKey]
compression := zstd.EncoderLevelFromZstd(int(compressionType.Level))
encoderLevel := zstd.WithEncoderLevel(compression)
if exists {
if zstdExists {
return zstdCompressor, nil
}

Check warning on line 72 in config/confighttp/compressor.go

View check run for this annotation

Codecov / codecov/patch

config/confighttp/compressor.go#L71-L72

Added lines #L71 - L72 were not covered by tests
zstdCompressor = &compressor{}
zstdCompressor.pool = sync.Pool{New: func() any { zw, _ := zstd.NewWriter(nil, zstd.WithEncoderConcurrency(1), encoderLevel); return zw }}
return zstdCompressor, nil
case configcompression.TypeZlib, configcompression.TypeDeflate:
zlibCompressor, exists = compressorPools.pools[mapKey]
if exists {
zlibCompressor, zlibExists := compressorPools.pools[mapKey]
if zlibExists {
return zlibCompressor, nil
}

Check warning on line 80 in config/confighttp/compressor.go

View check run for this annotation

Codecov / codecov/patch

config/confighttp/compressor.go#L79-L80

Added lines #L79 - L80 were not covered by tests
zlibCompressor = &compressor{}
Expand Down

0 comments on commit e843c4b

Please sign in to comment.