From 6650e1ab7b151c29dee669e5e03e13fc6cf18c10 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Thu, 3 Oct 2019 18:49:29 +0200 Subject: [PATCH] logcli: introduced QueryStringBuilder utility to clean up query string encoding --- pkg/logcli/client/client.go | 18 ++++---- pkg/util/query_string_builder.go | 55 +++++++++++++++++++++++ pkg/util/query_string_builder_test.go | 65 +++++++++++++++++++++++++++ 3 files changed, 129 insertions(+), 9 deletions(-) create mode 100644 pkg/util/query_string_builder.go create mode 100644 pkg/util/query_string_builder_test.go diff --git a/pkg/logcli/client/client.go b/pkg/logcli/client/client.go index d06104b096b8..d9e91b5b1b14 100644 --- a/pkg/logcli/client/client.go +++ b/pkg/logcli/client/client.go @@ -8,11 +8,11 @@ import ( "log" "net/http" "net/url" - "strconv" "strings" "time" "github.com/grafana/loki/pkg/loghttp" + "github.com/grafana/loki/pkg/util" "github.com/gorilla/websocket" "github.com/prometheus/common/config" @@ -54,20 +54,20 @@ func (c *Client) Query(queryStr string, limit int, time time.Time, direction log // excluding interfacer b/c it suggests taking the interface promql.Node instead of logproto.Direction b/c it happens to have a String() method // nolint:interfacer func (c *Client) QueryRange(queryStr string, limit int, from, through time.Time, direction logproto.Direction, step time.Duration, quiet bool) (*loghttp.QueryResponse, error) { - params := url.Values{} - params.Set("query", queryStr) - params.Set("limit", strconv.Itoa(limit)) - params.Set("start", strconv.FormatInt(from.UnixNano(), 10)) - params.Set("end", strconv.FormatInt(through.UnixNano(), 10)) - params.Set("direction", direction.String()) + params := util.NewQueryStringBuilder() + params.SetString("query", queryStr) + params.SetInt32("limit", limit) + params.SetInt("start", from.UnixNano()) + params.SetInt("end", through.UnixNano()) + params.SetString("direction", direction.String()) // The step is optional, so we do set it only if provided, // otherwise we do leverage on the API defaults if step != 0 { - params.Set("step", strconv.FormatInt(int64(step.Seconds()), 10)) + params.SetInt("step", int64(step.Seconds())) } - return c.doQuery(queryRangePath+"?"+params.Encode(), quiet) + return c.doQuery(params.EncodeWithPath(queryRangePath), quiet) } // ListLabelNames uses the /api/v1/label endpoint to list label names diff --git a/pkg/util/query_string_builder.go b/pkg/util/query_string_builder.go new file mode 100644 index 000000000000..74ad0c7bdc1e --- /dev/null +++ b/pkg/util/query_string_builder.go @@ -0,0 +1,55 @@ +package util + +import ( + "net/url" + "strconv" +) + +type QueryStringBuilder struct { + values url.Values +} + +func NewQueryStringBuilder() *QueryStringBuilder { + return &QueryStringBuilder{ + values: url.Values{}, + } +} + +func (b *QueryStringBuilder) SetString(name, value string) { + b.values.Set(name, value) +} + +func (b *QueryStringBuilder) SetInt(name string, value int64) { + b.SetString(name, strconv.FormatInt(value, 10)) +} + +func (b *QueryStringBuilder) SetInt32(name string, value int) { + b.SetString(name, strconv.Itoa(value)) +} + +func (b *QueryStringBuilder) SetFloat(name string, value float64) { + b.SetString(name, strconv.FormatFloat(value, 'f', -1, 64)) +} + +func (b *QueryStringBuilder) SetFloat32(name string, value float32) { + b.SetString(name, strconv.FormatFloat(float64(value), 'f', -1, 32)) +} + +// Encode returns the URL-encoded query string based on key-value +// parameters added to the builder calling Set functions. +func (b *QueryStringBuilder) Encode() string { + return b.values.Encode() +} + +// Encode returns the URL-encoded query string, prefixing it with the +// input URL path. +func (b *QueryStringBuilder) EncodeWithPath(path string) string { + queryString := b.Encode() + if path == "" { + return queryString + } else if queryString == "" { + return path + } + + return path + "?" + queryString +} diff --git a/pkg/util/query_string_builder_test.go b/pkg/util/query_string_builder_test.go new file mode 100644 index 000000000000..5de517111187 --- /dev/null +++ b/pkg/util/query_string_builder_test.go @@ -0,0 +1,65 @@ +package util + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestQueryStringBuilder(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + input map[string]interface{} + expectedEncoded string + expectedPath string + }{ + "should return an empty query string on no params": { + input: map[string]interface{}{}, + expectedEncoded: "", + expectedPath: "/test", + }, + "should return the URL encoded query string parameters": { + input: map[string]interface{}{ + "float32": float32(123.456), + "float64": float64(123.456), + "float64int": float64(12345.0), + "int32": 32, + "int64": int64(64), + "string": "foo", + }, + expectedEncoded: "float32=123.456&float64=123.456&float64int=12345&int32=32&int64=64&string=foo", + expectedPath: "/test?float32=123.456&float64=123.456&float64int=12345&int32=32&int64=64&string=foo", + }, + } + + for testName, testData := range tests { + testData := testData + + t.Run(testName, func(t *testing.T) { + params := NewQueryStringBuilder() + + for name, value := range testData.input { + switch value := value.(type) { + case string: + params.SetString(name, value) + case float32: + params.SetFloat32(name, value) + case float64: + params.SetFloat(name, value) + case int: + params.SetInt32(name, value) + case int64: + params.SetInt(name, value) + default: + require.Fail(t, fmt.Sprintf("Unknown data type for test fixture with name '%s'", name)) + } + } + + assert.Equal(t, testData.expectedEncoded, params.Encode()) + assert.Equal(t, testData.expectedPath, params.EncodeWithPath("/test")) + }) + } +}