Skip to content

Commit

Permalink
adds code review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
gbuenodevsuse committed Jul 9, 2024
1 parent e8db2d0 commit e5497a0
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 16 deletions.
4 changes: 2 additions & 2 deletions pkg/lib/datastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func (d *DatabaseStore) GetItems(bundleIds ...any) (itemRowIds []int64, itemRows
}

// ItemData can be stored as compressed data
itemRow.ItemData, err = utils.ShouldDecompress(itemRow.ItemData, itemRow.Compression)
itemRow.ItemData, err = utils.DecompressWhenNeeded(itemRow.ItemData, itemRow.Compression)

itemRows = append(itemRows, &itemRow)
itemRowIds = append(itemRowIds, itemRow.Id)
Expand Down Expand Up @@ -359,7 +359,7 @@ func (d *DatabaseStore) GetDataItemRowsInABundle(bundleId string) (itemRows []*T
}

// ItemData can be stored as compressed data
dataitemRow.ItemData, err = utils.ShouldDecompress(dataitemRow.ItemData, dataitemRow.Compression)
dataitemRow.ItemData, err = utils.DecompressWhenNeeded(dataitemRow.ItemData, dataitemRow.Compression)

itemRows = append(itemRows, &dataitemRow)

Expand Down
2 changes: 1 addition & 1 deletion pkg/lib/items.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (t *TelemetryDataItemRow) Exists(db *sql.DB) bool {
}

func (t *TelemetryDataItemRow) Insert(db *sql.DB) (err error) {
itemData, compression, err := utils.ShouldCompress(t.ItemData)
itemData, compression, err := utils.CompressWhenNeeded(t.ItemData)
if err != nil {
return
}
Expand Down
14 changes: 9 additions & 5 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,25 +76,29 @@ func DecompressGZIP(compressedData []byte) (decompressedData []byte, err error)
// TODO: check if it's worth trying to compress the data prior to compressing it (e.g: using entropy algorithms)
// This would save some CPU usage client side
// TODO: have telemetry data type be passed in as a parameter to further check if we should compress data and which algorithm to use (e.g: deflate or gzip)
func ShouldCompress(data []byte) (resultData []byte, compression *string, err error) {
func CompressWhenNeeded(data []byte) (resultData []byte, compression *string, err error) {
// 'compression' is inserted as a sql.NullString, hence it is returned as a nullable string
var nilStr *string = nil
var validStr string = "gzip"

// check whether it's worth compressing
if len(data) <= 80 {
return data, nil, nil
}

compressedData, err := CompressGZIP(data)
if err != nil {
log.Fatal(err)
return data, nil, err
}

if len(data) <= len(compressedData) {
return data, nilStr, nil
return data, nil, nil
}

return compressedData, &validStr, nil
}

// TODO: have telemetry data type be passed in as a parameter to further check if we should decompress data and which algorithm to use (e.g: deflate or gzip)
func ShouldDecompress(data []byte, compression sql.NullString) (resultData []byte, err error) {
func DecompressWhenNeeded(data []byte, compression sql.NullString) (resultData []byte, err error) {
if compression.Valid {
resultData, err = DecompressGZIP(data)
if err != nil {
Expand Down
16 changes: 8 additions & 8 deletions pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,22 @@ func TestCompressDecompressGZIP(t *testing.T) {
// Compress data
compressedData, err := CompressGZIP(mockData)
if err != nil {
t.Fatalf("Error compressing data: %v", err)
t.Errorf("Error compressing data: %v", err)
}

// Decompress data to verify
decompressedData, err := DecompressGZIP(compressedData)
if err != nil {
t.Fatalf("Error decompressing data: %v", err)
t.Errorf("Error decompressing data: %v", err)
}

if !bytes.Equal(mockData, decompressedData) {
t.Fatalf("Decompressed data does not match original data:\n Expected: %v\n Got: %v", string(mockData), string(decompressedData))
}
}

// TestShouldCompress tests ShouldCompress function
func TestShouldCompress(t *testing.T) {
// TestCompressWhenNeeded tests CompressWhenNeeded function
func TestCompressWhenNeeded(t *testing.T) {
tests := []struct {
name string
data []byte
Expand All @@ -54,7 +54,7 @@ func TestShouldCompress(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
compressedData, compression, err := ShouldCompress(tt.data)
compressedData, compression, err := CompressWhenNeeded(tt.data)
if err != nil {
t.Fatalf("Error determining whether or not to compress data: %v", err)
}
Expand All @@ -70,8 +70,8 @@ func TestShouldCompress(t *testing.T) {
}
}

// TestShouldDecompress tests ShouldDecompress function
func TestShouldDecompress(t *testing.T) {
// TestDecompressWhenNeeded tests DecompressWhenNeeded function
func TestDecompressWhenNeeded(t *testing.T) {
compressedData, _ := CompressGZIP([]byte("test data"))
tests := []struct {
name string
Expand All @@ -98,7 +98,7 @@ func TestShouldDecompress(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
resultData, err := ShouldDecompress(tt.data, tt.compression)
resultData, err := DecompressWhenNeeded(tt.data, tt.compression)
if (err != nil) != tt.expectErr {
t.Errorf("expected error: %v, got: %v", tt.expectErr, err)
}
Expand Down

0 comments on commit e5497a0

Please sign in to comment.