Skip to content

Commit

Permalink
feat: handle url limit in rest (#3186)
Browse files Browse the repository at this point in the history
* feat: handle url limit in rest
  • Loading branch information
Hardikl authored Oct 4, 2024
1 parent 53a6dc5 commit 5f6ef52
Show file tree
Hide file tree
Showing 4 changed files with 140 additions and 108 deletions.
16 changes: 2 additions & 14 deletions cmd/collectors/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,6 @@ func (r *Rest) Fields(prop *prop) []string {
fields = []string{"*"}
}
}
if len(prop.HiddenFields) > 0 {
fieldsMap := make(map[string]bool)
for _, field := range fields {
fieldsMap[field] = true
}

// append hidden fields
for _, hiddenField := range prop.HiddenFields {
if _, exists := fieldsMap[hiddenField]; !exists {
fields = append(fields, hiddenField)
fieldsMap[hiddenField] = true
}
}
}
return fields
}

Expand Down Expand Up @@ -368,6 +354,7 @@ func (r *Rest) updateHref() {
r.Prop.Href = rest.NewHrefBuilder().
APIPath(r.Prop.Query).
Fields(r.Fields(r.Prop)).
HiddenFields(r.Prop.HiddenFields).
Filter(r.Prop.Filter).
ReturnTimeout(r.Prop.ReturnTimeOut).
IsIgnoreUnknownFieldsEnabled(r.isIgnoreUnknownFieldsEnabled).
Expand All @@ -377,6 +364,7 @@ func (r *Rest) updateHref() {
e.prop.Href = rest.NewHrefBuilder().
APIPath(r.query(e)).
Fields(r.Fields(e.prop)).
HiddenFields(e.prop.HiddenFields).
Filter(r.filter(e)).
ReturnTimeout(r.Prop.ReturnTimeOut).
IsIgnoreUnknownFieldsEnabled(r.isIgnoreUnknownFieldsEnabled).
Expand Down
98 changes: 4 additions & 94 deletions cmd/collectors/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ func TestFields(t *testing.T) {
expectedResult []string
}{
{
name: "Test with valid fields and no hidden fields",
name: "Test with valid fields",
r: &Rest{
isIgnoreUnknownFieldsEnabled: true,
},
Expand All @@ -287,7 +287,7 @@ func TestFields(t *testing.T) {
},
},
{
name: "Test with invalid fields and no hidden fields",
name: "Test with invalid fields",
r: &Rest{
isIgnoreUnknownFieldsEnabled: true,
},
Expand All @@ -302,32 +302,7 @@ func TestFields(t *testing.T) {
expectedResult: []string{"*"},
},
{
name: "Test with valid fields and hidden fields",
r: &Rest{
isIgnoreUnknownFieldsEnabled: true,
},
p: &prop{
Fields: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
},
HiddenFields: []string{
"hidden_field1",
"hidden_field2",
},
IsPublic: true,
},
expectedResult: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
"hidden_field1",
"hidden_field2",
},
},
{
name: "Test with valid fields and hidden fields and prior versions to 9.11.1",
name: "Test with valid fields and prior versions to 9.11.1",
r: &Rest{
isIgnoreUnknownFieldsEnabled: false,
},
Expand All @@ -337,20 +312,14 @@ func TestFields(t *testing.T) {
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
},
HiddenFields: []string{
"hidden_field1",
"hidden_field2",
},
IsPublic: true,
},
expectedResult: []string{
"*",
"hidden_field1",
"hidden_field2",
},
},
{
name: "Test with valid fields and no hidden fields for private API",
name: "Test with valid fields for private API",
r: &Rest{
isIgnoreUnknownFieldsEnabled: false,
},
Expand All @@ -368,65 +337,6 @@ func TestFields(t *testing.T) {
"block_storage.primary.raid_type",
},
},
{
name: "Test with invalid fields and no hidden fields, ignore unknown fields disabled",
r: &Rest{
isIgnoreUnknownFieldsEnabled: false,
},
p: &prop{
Fields: []string{
"uuid",
"cloud_storage.stores.0.cloud_store.name",
"block_storage.primary.raid_type",
},
IsPublic: true,
},
expectedResult: []string{"*"},
},
{
name: "Test with invalid fields and no hidden fields for private API",
r: &Rest{
isIgnoreUnknownFieldsEnabled: true,
},
p: &prop{
Fields: []string{
"uuid",
"cloud_storage.stores.0.cloud_store.name",
"block_storage.primary.raid_type",
},
IsPublic: false,
},
expectedResult: []string{
"uuid",
"cloud_storage.stores.0.cloud_store.name",
"block_storage.primary.raid_type",
},
},
{
name: "Test with valid fields and hidden fields for private API",
r: &Rest{
isIgnoreUnknownFieldsEnabled: true,
},
p: &prop{
Fields: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
},
HiddenFields: []string{
"hidden_field1",
"hidden_field2",
},
IsPublic: true,
},
expectedResult: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
"hidden_field1",
"hidden_field2",
},
},
}

for _, tt := range tests {
Expand Down
34 changes: 34 additions & 0 deletions cmd/tools/rest/href_builder.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package rest

import (
"log/slog"
"slices"
"strconv"
"strings"
)

const URLMaxLimit = 8 * 1024

type HrefBuilder struct {
apiPath string
fields []string
hiddenFields []string
counterSchema string
filter []string
queryFields string
Expand All @@ -32,6 +36,11 @@ func (b *HrefBuilder) Fields(fields []string) *HrefBuilder {
return b
}

func (b *HrefBuilder) HiddenFields(hiddenFields []string) *HrefBuilder {
b.hiddenFields = hiddenFields
return b
}

func (b *HrefBuilder) CounterSchema(counterSchema []string) *HrefBuilder {
b.counterSchema = strings.Join(counterSchema, ",")
return b
Expand Down Expand Up @@ -76,6 +85,31 @@ func (b *HrefBuilder) Build() string {

href.WriteString("?return_records=true")

if len(b.hiddenFields) > 0 {
fieldsMap := make(map[string]bool)
for _, field := range b.fields {
fieldsMap[field] = true
}

// append hidden fields
for _, hiddenField := range b.hiddenFields {
if _, exists := fieldsMap[hiddenField]; !exists {
b.fields = append(b.fields, hiddenField)
fieldsMap[hiddenField] = true
}
}
}

if len(strings.Join(b.fields, ",")) > URLMaxLimit {
b.fields = append([]string{"*"}, b.hiddenFields...)
if len(strings.Join(b.fields, ",")) > URLMaxLimit {
slog.Info("fields converting to * due to URL max limit")
b.fields = []string{"*"}
} else {
slog.Info("fields converting to *,hiddenFields due to URL max limit")
}
}

// Sort fields so that the href is deterministic
slices.Sort(b.fields)

Expand Down
100 changes: 100 additions & 0 deletions cmd/tools/rest/href_builder_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package rest

import (
"github.com/google/go-cmp/cmp"
"strconv"
"testing"
)

func TestBuild(t *testing.T) {
testQuery := "storage/volumes"
testFields := []string{"name", "svm"}
testHiddenFields := []string{"statistics"}
testFilter := []string{""}
testReturnTimeout := 10
expectedHrefTest1 := "api/storage/volumes?return_records=true&fields=name,statistics,svm&return_timeout=10&ignore_unknown_fields=true"
hrefTest1 := NewHrefBuilder().
APIPath(testQuery).
Fields(testFields).
HiddenFields(testHiddenFields).
Filter(testFilter).
ReturnTimeout(&testReturnTimeout).
IsIgnoreUnknownFieldsEnabled(true).
Build()

if hrefTest1 != expectedHrefTest1 {
t.Errorf("hrefTest1 should be %s but got %s", expectedHrefTest1, hrefTest1)
}

testFields = make([]string, 0)
for i := range URLMaxLimit / len("Test") {
testFields = append(testFields, "Test"+strconv.Itoa(i))
}

expectedHrefTest2 := "api/storage/volumes?return_records=true&fields=*,statistics&return_timeout=10&ignore_unknown_fields=true"
hrefTest2 := NewHrefBuilder().
APIPath(testQuery).
Fields(testFields).
HiddenFields(testHiddenFields).
Filter(testFilter).
ReturnTimeout(&testReturnTimeout).
IsIgnoreUnknownFieldsEnabled(true).
Build()

if hrefTest2 != expectedHrefTest2 {
t.Errorf("hrefTest2 should be %s but got %s", expectedHrefTest2, hrefTest2)
}
}

func TestFields(t *testing.T) {
tests := []struct {
name string
fields []string
hiddenFields []string
expectedResult []string
}{
{
name: "Test with fields and no hidden fields",
fields: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
},
expectedResult: []string{
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
"uuid",
},
},
{
name: "Test with fields and hidden fields",
fields: []string{
"uuid",
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
},
hiddenFields: []string{
"hidden_field1",
"hidden_field2",
},
expectedResult: []string{
"block_storage.primary.disk_type",
"block_storage.primary.raid_type",
"hidden_field1",
"hidden_field2",
"uuid",
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
hBuilder := NewHrefBuilder()
hBuilder.Fields(tt.fields).HiddenFields(tt.hiddenFields).Build()
diff := cmp.Diff(hBuilder.fields, tt.expectedResult)
if diff != "" {
t.Errorf("Mismatch (-got +want):\n%s", diff)
}
})
}
}

0 comments on commit 5f6ef52

Please sign in to comment.