From 53f4cef40161f2ce731b778f930a822fb8f3bbfb Mon Sep 17 00:00:00 2001 From: filipecosta90 Date: Sat, 18 Jul 2020 01:02:17 +0100 Subject: [PATCH 1/2] [add] Add support for TS.MGET with options. minor refactor and go fmt. --- .circleci/config.yml | 1 + Makefile | 11 ++++++++ README.md | 8 +++--- client.go | 21 +++++++++++---- client_test.go | 62 ++++++++++++++++++++++++++++++++++++++++++++ multiget.go | 34 ++++++++++++++++++++++++ multiget_test.go | 28 ++++++++++++++++++++ pool.go | 1 - reply_parser.go | 2 +- reply_parser_test.go | 4 +-- 10 files changed, 160 insertions(+), 12 deletions(-) create mode 100644 multiget.go create mode 100644 multiget_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 74449e5..31c9fea 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -11,6 +11,7 @@ jobs: working_directory: /go/src/github.com/RedisTimeSeries/redistimeseries-go steps: - checkout + - run: make checkfmt - run: make get - run: make coverage - run: bash <(curl -s https://codecov.io/bash) -t ${CODECOV_TOKEN} diff --git a/Makefile b/Makefile index e4adfab..a5d59d1 100644 --- a/Makefile +++ b/Makefile @@ -6,14 +6,25 @@ GOCLEAN=$(GOCMD) clean GOTEST=$(GOCMD) test GOGET=$(GOCMD) get GOMOD=$(GOCMD) mod +GOFMT=$(GOCMD) fmt .PHONY: all test coverage all: test coverage +checkfmt: + @echo 'Checking gofmt';\ + bash -c "diff -u <(echo -n) <(gofmt -d .)";\ + EXIT_CODE=$$?;\ + if [ "$$EXIT_CODE" -ne 0 ]; then \ + echo '$@: Go files must be formatted with gofmt'; \ + fi && \ + exit $$EXIT_CODE + get: $(GOGET) -t -v ./... test: get + $(GOFMT) ./... $(GOTEST) -race -covermode=atomic ./... coverage: get test diff --git a/README.md b/README.md index 1052bb6..7d66657 100644 --- a/README.md +++ b/README.md @@ -69,10 +69,12 @@ func main() { | [TS.INCRBY/TS.DECRBY](https://oss.redislabs.com/redistimeseries/commands/#tsincrbytsdecrby) | [IncrBy](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.IncrBy) / [DecrBy](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.DecrBy) | | [TS.CREATERULE](https://oss.redislabs.com/redistimeseries/commands/#tscreaterule) | [CreateRule](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.CreateRule) | | [TS.DELETERULE](https://oss.redislabs.com/redistimeseries/commands/#tsdeleterule) | [DeleteRule](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.DeleteRule) | -| [TS.RANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrange) | [RangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.RangeWithOptions) | -| [TS.MRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrange) | [MultiRangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiRangeWithOptions) | +| [TS.RANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrangetsrevrange) | [RangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.RangeWithOptions) | +| [TS.REVRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsrangetsrevrange) | N/A | +| [TS.MRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrangetsmrevrange) | [MultiRangeWithOptions](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiRangeWithOptions) | +| [TS.MREVRANGE](https://oss.redislabs.com/redistimeseries/commands/#tsmrangetsmrevrange) | N/A | | [TS.GET](https://oss.redislabs.com/redistimeseries/commands/#tsget) | [Get](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.Get) | -| [TS.MGET](https://oss.redislabs.com/redistimeseries/commands/#tsmget) | [MultiGet](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.MultiGet) | +| [TS.MGET](https://oss.redislabs.com/redistimeseries/commands/#tsmget) | | | [TS.INFO](https://oss.redislabs.com/redistimeseries/commands/#tsinfo) | [Info](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.Info) | | [TS.QUERYINDEX](https://oss.redislabs.com/redistimeseries/commands/#tsqueryindex) | [QueryIndex](https://godoc.org/github.com/RedisTimeSeries/redistimeseries-go#Client.QueryIndex) | diff --git a/client.go b/client.go index 5522aef..fdf1f7e 100644 --- a/client.go +++ b/client.go @@ -29,7 +29,6 @@ func NewClient(addr, name string, authPass *string) *Client { return ret } - // NewClientFromPool creates a new Client with the given pool and client name func NewClientFromPool(pool *redis.Pool, name string) *Client { ret := &Client{ @@ -275,13 +274,25 @@ func (client *Client) MultiGet(filters ...string) (ranges []Range, if len(filters) == 0 { return } - args := []interface{}{"FILTER"} - for _, filter := range filters { - args = append(args, filter) + args := createMultiGetCmdArguments(DefaultMultiGetOptions, filters) + reply, err = conn.Do("TS.MGET", args...) + if err != nil { + return } + ranges, err = ParseRangesSingleDataPoint(reply) + return +} +// MultiGetWithOptions - Get the last samples matching the specific filters. +// args: +// multiGetOptions - MultiGetOptions options. You can use the default DefaultMultiGetOptions +// filters - list of filters e.g. "a=bb", "b!=aa" +func (client *Client) MultiGetWithOptions(multiGetOptions MultiGetOptions, filters ...string) (ranges []Range, err error) { + conn := client.Pool.Get() + defer conn.Close() + var reply interface{} + args := createMultiGetCmdArguments(multiGetOptions, filters) reply, err = conn.Do("TS.MGET", args...) - if err != nil { return } diff --git a/client_test.go b/client_test.go index bb6b234..770c314 100644 --- a/client_test.go +++ b/client_test.go @@ -416,6 +416,68 @@ func TestClient_MultiGet(t *testing.T) { } } +func TestClient_MultiGetWithOptions(t *testing.T) { + client.FlushAll() + key1 := "test_TestClient_MultiGet_key1" + key2 := "test_TestClient_MultiGet_key2" + labels1 := map[string]string{ + "metric": "cpu", + "country": "US", + } + labels2 := map[string]string{ + "metric": "cpu", + "country": "UK", + } + + _, err := client.AddWithOptions(key1, 1, 5.0, CreateOptions{Labels: labels1}) + if err != nil { + t.Errorf("TestClient_MultiGetWithOptions Add() error = %v", err) + return + } + _, err = client.Add(key1, 2, 15.0) + _, err = client.Add(key1, 3, 15.0) + + _, err = client.AddWithOptions(key2, 1, 5.0, CreateOptions{Labels: labels2}) + + if err != nil { + t.Errorf("TestClient_MultiGetWithOptions Add() error = %v", err) + return + } + + type fields struct { + Pool ConnPool + Name string + } + type args struct { + filters []string + } + tests := []struct { + name string + fields fields + args args + wantRanges []Range + wantErr bool + }{ + {"multi key", fields{client.Pool, "test"}, args{[]string{"metric=cpu", "country=UK"}}, []Range{Range{key2, map[string]string{"country": "UK", "metric": "cpu"}, []DataPoint{DataPoint{1, 5.0}}}}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &Client{ + Pool: tt.fields.Pool, + Name: tt.fields.Name, + } + gotRanges, err := client.MultiGetWithOptions(*NewMultiGetOptions().SetWithLabels(true), tt.args.filters...) + if (err != nil) != tt.wantErr { + t.Errorf("MultiGet() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(gotRanges, tt.wantRanges) { + t.Errorf("MultiGet() gotRanges = %v, want %v", gotRanges, tt.wantRanges) + } + }) + } +} + func TestClient_Range(t *testing.T) { client.FlushAll() key1 := "TestClient_Range_key1" diff --git a/multiget.go b/multiget.go new file mode 100644 index 0000000..06156bf --- /dev/null +++ b/multiget.go @@ -0,0 +1,34 @@ +package redis_timeseries_go + +// MultiGetOptions represent the options for querying across multiple time-series +type MultiGetOptions struct { + WithLabels bool +} + +// MultiGetOptions are the default options for querying across multiple time-series +var DefaultMultiGetOptions = MultiGetOptions{ + WithLabels: false, +} + +func NewMultiGetOptions() *MultiGetOptions { + return &MultiGetOptions{ + WithLabels: false, + } +} + +func (mgetopts *MultiGetOptions) SetWithLabels(value bool) *MultiGetOptions { + mgetopts.WithLabels = value + return mgetopts +} + +func createMultiGetCmdArguments(mgetOptions MultiGetOptions, filters []string) []interface{} { + args := []interface{}{} + if mgetOptions.WithLabels == true { + args = append(args, "WITHLABELS") + } + args = append(args, "FILTER") + for _, filter := range filters { + args = append(args, filter) + } + return args +} diff --git a/multiget_test.go b/multiget_test.go new file mode 100644 index 0000000..e17389f --- /dev/null +++ b/multiget_test.go @@ -0,0 +1,28 @@ +package redis_timeseries_go + +import ( + "reflect" + "testing" +) + +func Test_createMultiGetCmdArguments(t *testing.T) { + type args struct { + mgetOptions MultiGetOptions + filters []string + } + tests := []struct { + name string + args args + want []interface{} + }{ + {"default", args{DefaultMultiGetOptions, []string{"labels!="}}, []interface{}{"FILTER", "labels!="}}, + {"withlabels", args{*(NewMultiGetOptions().SetWithLabels(true)), []string{"labels!="}}, []interface{}{"WITHLABELS", "FILTER", "labels!="}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := createMultiGetCmdArguments(tt.args.mgetOptions, tt.args.filters); !reflect.DeepEqual(got, tt.want) { + t.Errorf("createMultiGetCmdArguments() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pool.go b/pool.go index 021ad79..79a6eab 100644 --- a/pool.go +++ b/pool.go @@ -82,7 +82,6 @@ func testOnBorrow(c redis.Conn, t time.Time) (err error) { return err } - func (p *MultiHostPool) Close() (err error) { p.Lock() defer p.Unlock() diff --git a/reply_parser.go b/reply_parser.go index 427ec21..e520bc6 100644 --- a/reply_parser.go +++ b/reply_parser.go @@ -3,8 +3,8 @@ package redis_timeseries_go import ( "errors" "fmt" - "strconv" "github.com/gomodule/redigo/redis" + "strconv" ) func toAggregationType(aggType interface{}) (aggTypeStr AggregationType, err error) { diff --git a/reply_parser_test.go b/reply_parser_test.go index 81e0630..8ca6003 100644 --- a/reply_parser_test.go +++ b/reply_parser_test.go @@ -61,7 +61,7 @@ func TestParseRangesSingleDataPoint(t *testing.T) { false, }, {"incorrect input ( 2 elements on inner array )", - args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{},}}}, + args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{}}}}, []Range{}, true, }, @@ -201,7 +201,7 @@ func TestParseRanges(t *testing.T) { false, }, {"incorrect input ( 2 elements on inner array )", - args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{},}}}, + args{[]interface{}{[]interface{}{[]byte("serie 1"), []interface{}{}}}}, []Range{}, true, }, From 5298f8503fb2ec91fd4351a2e072347d195e1526 Mon Sep 17 00:00:00 2001 From: filipecosta90 Date: Sun, 19 Jul 2020 11:46:32 +0100 Subject: [PATCH 2/2] [fix] per PR review on merging MultiGet with MultiGetWithOptions with defaults --- client.go | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/client.go b/client.go index fdf1f7e..a93b97c 100644 --- a/client.go +++ b/client.go @@ -266,21 +266,8 @@ func (client *Client) Get(key string) (dataPoint *DataPoint, // MultiGet - Get the last sample across multiple time-series, matching the specific filters. // args: // filters - list of filters e.g. "a=bb", "b!=aa" -func (client *Client) MultiGet(filters ...string) (ranges []Range, - err error) { - conn := client.Pool.Get() - defer conn.Close() - var reply interface{} - if len(filters) == 0 { - return - } - args := createMultiGetCmdArguments(DefaultMultiGetOptions, filters) - reply, err = conn.Do("TS.MGET", args...) - if err != nil { - return - } - ranges, err = ParseRangesSingleDataPoint(reply) - return +func (client *Client) MultiGet(filters ...string) (ranges []Range, err error) { + return client.MultiGetWithOptions(DefaultMultiGetOptions, filters...) } // MultiGetWithOptions - Get the last samples matching the specific filters. @@ -291,6 +278,9 @@ func (client *Client) MultiGetWithOptions(multiGetOptions MultiGetOptions, filte conn := client.Pool.Get() defer conn.Close() var reply interface{} + if len(filters) == 0 { + return + } args := createMultiGetCmdArguments(multiGetOptions, filters) reply, err = conn.Do("TS.MGET", args...) if err != nil {