From db50c679e28c0d6ed446e796a61bdf76a4f5e9aa Mon Sep 17 00:00:00 2001 From: Emond Papegaaij Date: Fri, 20 Oct 2023 20:10:23 +0200 Subject: [PATCH 1/6] Add testcase to demonstrate broken exploded query parameters --- request_information_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/request_information_test.go b/request_information_test.go index a07f944..0a17f19 100644 --- a/request_information_test.go +++ b/request_information_test.go @@ -157,6 +157,19 @@ func TestItBuildsUrlOnProvidedBaseUrl(t *testing.T) { assert.Equal(t, "http://localhost/users", resultUri.String()) } +func TestItSetsExplodedQueryParameters(t *testing.T) { + value := true + requestInformation := NewRequestInformation() + requestInformation.UrlTemplate = "http://localhost/me{?%24select*}" + requestInformation.AddQueryParameters(getQueryParameters{ + Select_escaped: []string{"id", "displayName"}, + Count: &value, + }) + resultUri, err := requestInformation.GetUri() + assert.Nil(t, err) + assert.Equal(t, "http://localhost/me?%24select=id&%24select=displayName", resultUri.String()) +} + func TestItSetsContentFromParsable(t *testing.T) { requestInformation := NewRequestInformation() requestInformation.UrlTemplate = "{+baseurl}/users{?%24count}" From 999bfb194b83d2b78b3cb0fd43a8bd766cfc2837 Mon Sep 17 00:00:00 2001 From: Emond Papegaaij Date: Fri, 20 Oct 2023 20:48:17 +0200 Subject: [PATCH 2/6] Change type of QueryParameters to map[string]any This fixes support for exploded query parameters. With the current implementation of std-uritemplate, a conversion from []string to []any is required to render the parameters correctly. Also, url encoding turns out a bit different for the comma, but I think that's ok. --- request_information.go | 10 +++++++--- request_information_test.go | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/request_information.go b/request_information.go index 10b8ce7..1a3235f 100644 --- a/request_information.go +++ b/request_information.go @@ -27,7 +27,7 @@ type RequestInformation struct { // The Request Headers. Headers *RequestHeaders // The Query Parameters of the request. - QueryParameters map[string]string + QueryParameters map[string]any // The Request Body. Content []byte // The path parameters to use for the URL template when generating the URI. @@ -43,7 +43,7 @@ const raw_url_key = "request-raw-url" func NewRequestInformation() *RequestInformation { return &RequestInformation{ Headers: NewRequestHeaders(), - QueryParameters: make(map[string]string), + QueryParameters: make(map[string]any), options: make(map[string]RequestOption), PathParameters: make(map[string]string), } @@ -467,7 +467,11 @@ func (request *RequestInformation) AddQueryParameters(source interface{}) { } arr, ok := value.([]string) if ok && len(arr) > 0 { - request.QueryParameters[fieldName] = strings.Join(arr, ",") + tmp := make([]any, len(arr)) + for i, v := range arr { + tmp[i] = v + } + request.QueryParameters[fieldName] = tmp } } } diff --git a/request_information_test.go b/request_information_test.go index 0a17f19..8c97696 100644 --- a/request_information_test.go +++ b/request_information_test.go @@ -61,7 +61,7 @@ func TestItAddsStringArrayQueryParameters(t *testing.T) { Expand: value, } requestInformation.AddQueryParameters(queryParameters) - assert.Equal(t, "somefilter,someotherfilter", requestInformation.QueryParameters["Expand"]) + assert.Equal(t, []any{"somefilter", "someotherfilter"}, requestInformation.QueryParameters["Expand"]) } func TestItSetsTheRawURL(t *testing.T) { @@ -96,7 +96,7 @@ func TestItSetsSelectAndCountQueryParameters(t *testing.T) { }) resultUri, err := requestInformation.GetUri() assert.Nil(t, err) - assert.Equal(t, "http://localhost/me?%24select=id%2CdisplayName&%24count=true", resultUri.String()) + assert.Equal(t, "http://localhost/me?%24select=id,displayName&%24count=true", resultUri.String()) } func TestItDoesNotSetEmptySelectQueryParameters(t *testing.T) { From ca104d1a3785436f767c13af6e3eeeadd17fbc0f Mon Sep 17 00:00:00 2001 From: Emond Papegaaij Date: Fri, 20 Oct 2023 21:32:15 +0200 Subject: [PATCH 3/6] Revert QueryParameters to map[string]string and add QueryParametersAny This keeps source, binary and runtime behavior in a backward compatible way. It does however require to do some bookkeeping twice. The old QueryParameters is still available and works in the same way. Most values are still stored in the old QueryParameters. Only lists are also stored in the new QueryParametersAny. Any value in QueryParametersAny will override the value from QueryParameters. --- request_information.go | 24 ++++++++++++++++++------ request_information_test.go | 7 ++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/request_information.go b/request_information.go index 1a3235f..fab4f97 100644 --- a/request_information.go +++ b/request_information.go @@ -27,7 +27,9 @@ type RequestInformation struct { // The Request Headers. Headers *RequestHeaders // The Query Parameters of the request. - QueryParameters map[string]any + // Deprecated: use QueryParametersAny instead + QueryParameters map[string]string + QueryParametersAny map[string]any // The Request Body. Content []byte // The path parameters to use for the URL template when generating the URI. @@ -42,10 +44,11 @@ const raw_url_key = "request-raw-url" // NewRequestInformation creates a new RequestInformation object with default values. func NewRequestInformation() *RequestInformation { return &RequestInformation{ - Headers: NewRequestHeaders(), - QueryParameters: make(map[string]any), - options: make(map[string]RequestOption), - PathParameters: make(map[string]string), + Headers: NewRequestHeaders(), + QueryParameters: make(map[string]string), + QueryParametersAny: make(map[string]any), + options: make(map[string]RequestOption), + PathParameters: make(map[string]string), } } @@ -59,6 +62,8 @@ func (request *RequestInformation) GetUri() (*u.URL, error) { return nil, errors.New("uri template parameters cannot be nil") } else if request.QueryParameters == nil { return nil, errors.New("uri query parameters cannot be nil") + } else if request.QueryParametersAny == nil { + return nil, errors.New("uri query parameters cannot be nil") } else if request.PathParameters[raw_url_key] != "" { uri, err := u.Parse(request.PathParameters[raw_url_key]) if err != nil { @@ -79,6 +84,9 @@ func (request *RequestInformation) GetUri() (*u.URL, error) { for key, value := range request.QueryParameters { substitutions[key] = value } + for key, value := range request.QueryParametersAny { + substitutions[key] = value + } url, err := stduritemplate.Expand(request.UrlTemplate, substitutions) if err != nil { return nil, err @@ -97,6 +105,9 @@ func (request *RequestInformation) SetUri(url u.URL) { for k := range request.QueryParameters { delete(request.QueryParameters, k) } + for k := range request.QueryParametersAny { + delete(request.QueryParametersAny, k) + } } // AddRequestOptions adds an option to the request to be read by the middleware infrastructure. @@ -467,11 +478,12 @@ func (request *RequestInformation) AddQueryParameters(source interface{}) { } arr, ok := value.([]string) if ok && len(arr) > 0 { + request.QueryParameters[fieldName] = strings.Join(arr, ",") tmp := make([]any, len(arr)) for i, v := range arr { tmp[i] = v } - request.QueryParameters[fieldName] = tmp + request.QueryParametersAny[fieldName] = tmp } } } diff --git a/request_information_test.go b/request_information_test.go index 8c97696..427973d 100644 --- a/request_information_test.go +++ b/request_information_test.go @@ -32,6 +32,7 @@ func TestItAddsStringQueryParameters(t *testing.T) { requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, value, requestInformation.QueryParameters["Filter"]) + assert.Equal(t, nil, requestInformation.QueryParametersAny["Filter"]) } func TestItAddsBoolQueryParameters(t *testing.T) { @@ -42,6 +43,7 @@ func TestItAddsBoolQueryParameters(t *testing.T) { } requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, "true", requestInformation.QueryParameters["Count"]) + assert.Equal(t, nil, requestInformation.QueryParametersAny["Count"]) } func TestItAddsIntQueryParameters(t *testing.T) { @@ -52,6 +54,7 @@ func TestItAddsIntQueryParameters(t *testing.T) { } requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, "42", requestInformation.QueryParameters["Top"]) + assert.Equal(t, nil, requestInformation.QueryParametersAny["Top"]) } func TestItAddsStringArrayQueryParameters(t *testing.T) { @@ -61,7 +64,8 @@ func TestItAddsStringArrayQueryParameters(t *testing.T) { Expand: value, } requestInformation.AddQueryParameters(queryParameters) - assert.Equal(t, []any{"somefilter", "someotherfilter"}, requestInformation.QueryParameters["Expand"]) + assert.Equal(t, "somefilter,someotherfilter", requestInformation.QueryParameters["Expand"]) + assert.Equal(t, []any{"somefilter", "someotherfilter"}, requestInformation.QueryParametersAny["Expand"]) } func TestItSetsTheRawURL(t *testing.T) { @@ -75,6 +79,7 @@ func TestItSetsTheRawURL(t *testing.T) { assert.Nil(t, err) assert.Equal(t, "https://someurl.com", uri.String()) assert.Equal(t, 0, len(requestInformation.QueryParameters)) + assert.Equal(t, 0, len(requestInformation.QueryParametersAny)) } type getQueryParameters struct { From ce2802ab66cee404acd7f603a038a90dcf47e702 Mon Sep 17 00:00:00 2001 From: Emond Papegaaij Date: Tue, 31 Oct 2023 14:33:00 +0100 Subject: [PATCH 4/6] Add support for []any query parameters --- request_information.go | 14 +++++++++----- request_information_test.go | 18 +++++++++++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/request_information.go b/request_information.go index fab4f97..7b8e455 100644 --- a/request_information.go +++ b/request_information.go @@ -476,14 +476,18 @@ func (request *RequestInformation) AddQueryParameters(source interface{}) { if ok && it != nil { request.QueryParameters[fieldName] = strconv.FormatInt(int64(*it), 10) } - arr, ok := value.([]string) - if ok && len(arr) > 0 { - request.QueryParameters[fieldName] = strings.Join(arr, ",") - tmp := make([]any, len(arr)) - for i, v := range arr { + strArr, ok := value.([]string) + if ok && len(strArr) > 0 { + request.QueryParameters[fieldName] = strings.Join(strArr, ",") + tmp := make([]any, len(strArr)) + for i, v := range strArr { tmp[i] = v } request.QueryParametersAny[fieldName] = tmp } + arr, ok := value.([]any) + if ok && len(arr) > 0 { + request.QueryParametersAny[fieldName] = arr + } } } diff --git a/request_information_test.go b/request_information_test.go index 427973d..f5aef8f 100644 --- a/request_information_test.go +++ b/request_information_test.go @@ -15,6 +15,7 @@ import ( type QueryParameters struct { Count *bool Expand []string + ExpandAny []any Filter *string Orderby []string Search *string @@ -32,7 +33,7 @@ func TestItAddsStringQueryParameters(t *testing.T) { requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, value, requestInformation.QueryParameters["Filter"]) - assert.Equal(t, nil, requestInformation.QueryParametersAny["Filter"]) + assert.Nil(t, requestInformation.QueryParametersAny["Filter"]) } func TestItAddsBoolQueryParameters(t *testing.T) { @@ -43,7 +44,7 @@ func TestItAddsBoolQueryParameters(t *testing.T) { } requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, "true", requestInformation.QueryParameters["Count"]) - assert.Equal(t, nil, requestInformation.QueryParametersAny["Count"]) + assert.Nil(t, requestInformation.QueryParametersAny["Count"]) } func TestItAddsIntQueryParameters(t *testing.T) { @@ -54,7 +55,7 @@ func TestItAddsIntQueryParameters(t *testing.T) { } requestInformation.AddQueryParameters(queryParameters) assert.Equal(t, "42", requestInformation.QueryParameters["Top"]) - assert.Equal(t, nil, requestInformation.QueryParametersAny["Top"]) + assert.Nil(t, requestInformation.QueryParametersAny["Top"]) } func TestItAddsStringArrayQueryParameters(t *testing.T) { @@ -68,6 +69,17 @@ func TestItAddsStringArrayQueryParameters(t *testing.T) { assert.Equal(t, []any{"somefilter", "someotherfilter"}, requestInformation.QueryParametersAny["Expand"]) } +func TestItAddsAnyArrayQueryParameters(t *testing.T) { + requestInformation := NewRequestInformation() + value := []any{"somefilter", "someotherfilter"} + queryParameters := QueryParameters{ + ExpandAny: value, + } + requestInformation.AddQueryParameters(queryParameters) + assert.Empty(t, requestInformation.QueryParameters["ExpandAny"]) + assert.Equal(t, []any{"somefilter", "someotherfilter"}, requestInformation.QueryParametersAny["ExpandAny"]) +} + func TestItSetsTheRawURL(t *testing.T) { requestInformation := NewRequestInformation() requestInformation.PathParameters[raw_url_key] = "https://someurl.com" From 2d356d2abc3055cd38223fcfe5cfbb52792f556f Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 31 Oct 2023 10:58:28 -0400 Subject: [PATCH 5/6] - adds a comment for later reference --- request_information.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/request_information.go b/request_information.go index 7b8e455..77b3fff 100644 --- a/request_information.go +++ b/request_information.go @@ -478,7 +478,9 @@ func (request *RequestInformation) AddQueryParameters(source interface{}) { } strArr, ok := value.([]string) if ok && len(strArr) > 0 { + // populating both query parameter fields to avoid breaking compatibility with code reading this field request.QueryParameters[fieldName] = strings.Join(strArr, ",") + tmp := make([]any, len(strArr)) for i, v := range strArr { tmp[i] = v From 20a8c3e016ddaf9d5d3b2c4a3f697c58d74d2bca Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Tue, 31 Oct 2023 11:01:22 -0400 Subject: [PATCH 6/6] - adds changelog entry for expansion fix --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e068cc..b7f64ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +## [1.3.1] - 2023-10-31 + +### Changed + +- Fixed an issue where query parameters of type array of anything else than string would not be expanded properly. [#114](https://github.com/microsoft/kiota-abstractions-go/issues/114) + ## [1.3.0] - 2023-10-12 ### Added