diff --git a/cmd/collectors/rest/rest.go b/cmd/collectors/rest/rest.go index 04f7a4a60..616affc2f 100644 --- a/cmd/collectors/rest/rest.go +++ b/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). diff --git a/cmd/collectors/rest/rest_test.go b/cmd/collectors/rest/rest_test.go index 2c41759ff..b4b4373dc 100644 --- a/cmd/collectors/rest/rest_test.go +++ b/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 { diff --git a/cmd/tools/rest/href_builder.go b/cmd/tools/rest/href_builder.go index be98ee7c3..766accea5 100644 --- a/cmd/tools/rest/href_builder.go +++ b/cmd/tools/rest/href_builder.go @@ -12,6 +12,7 @@ const URLMaxLimit = 8 * 1024 type HrefBuilder struct { apiPath string fields []string + hiddenFields []string counterSchema string filter []string queryFields string @@ -35,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 @@ -83,6 +89,22 @@ func (b *HrefBuilder) Build() string { fmt.Printf("converting to * due to URL max limit") b.fields = []string{"*"} } + + 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 + } + } + } + // Sort fields so that the href is deterministic slices.Sort(b.fields) diff --git a/cmd/tools/rest/href_builder_test.go b/cmd/tools/rest/href_builder_test.go index c9962479a..b7528b04a 100644 --- a/cmd/tools/rest/href_builder_test.go +++ b/cmd/tools/rest/href_builder_test.go @@ -1,21 +1,23 @@ 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 isIgnoreUnknownFieldsEnabled := true - expectedHrefTest1 := "api/storage/volumes?return_records=true&fields=name,svm&return_timeout=10&ignore_unknown_fields=true" + 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(isIgnoreUnknownFieldsEnabled). @@ -30,10 +32,11 @@ func TestBuild(t *testing.T) { testFields = append(testFields, "Test"+strconv.Itoa(i)) } - expectedHrefTest2 := "api/storage/volumes?return_records=true&fields=*&return_timeout=10&ignore_unknown_fields=true" + 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(isIgnoreUnknownFieldsEnabled). @@ -43,3 +46,56 @@ func TestBuild(t *testing.T) { 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) + } + }) + } +}