feat: handle url limit in rest

cmd/collectors/rest/rest.go

@@ -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
 }
 
@@ -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).
@@ -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).

cmd/collectors/rest/rest_test.go

@@ -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,
 			},
@@ -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,
 			},
@@ -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,
 			},
@@ -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,
 			},
@@ -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 {

cmd/tools/rest/href_builder.go

@@ -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
@@ -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
@@ -76,6 +85,26 @@ 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 {
+		slog.Info("converting to * due to URL max limit")
+		b.fields = []string{"*"}
+	}
+
 	// Sort fields so that the href is deterministic
 	slices.Sort(b.fields)
 

cmd/tools/rest/href_builder_test.go

@@ -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=*&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)
+			}
+		})
+	}
+}