From d702c48923ce6d1864866e757f235c6737f48f95 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Thu, 18 Nov 2021 14:57:40 +0100 Subject: [PATCH] Fix stable sort order of query response when encoded to JSON (#425) This will make sure that the order of query responses (refIDs) are always stable (increasing order, e.g. A, B, C even if query requests has the order C, A, B. Fixes #366 --- Magefile.go | 8 +++++++- backend/json.go | 13 +++++++++++-- backend/json_test.go | 22 ++++++++++++++++++---- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/Magefile.go b/Magefile.go index 762660587..eefc71edf 100644 --- a/Magefile.go +++ b/Magefile.go @@ -1,4 +1,5 @@ -//+build mage +//go:build mage +// +build mage package main @@ -30,6 +31,11 @@ func Test() error { return sh.RunV("go", "test", "./...") } +// TestRace runs the test suite with the data race detector enabled. +func TestRace() error { + return sh.RunV("go", "test", "-race", "./...") +} + func Lint() error { if err := sh.RunV("golangci-lint", "run", "./..."); err != nil { return err diff --git a/backend/json.go b/backend/json.go index c5894b902..d8bc3fc92 100644 --- a/backend/json.go +++ b/backend/json.go @@ -2,6 +2,7 @@ package backend import ( "fmt" + "sort" "unsafe" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -84,12 +85,20 @@ func writeQueryDataResponseJSON(qdr *QueryDataResponse, stream *jsoniter.Stream) stream.WriteObjectStart() started := false + refIDs := []string{} + for refID := range qdr.Responses { + refIDs = append(refIDs, refID) + } + sort.Strings(refIDs) + // Make sure all keys in the result are written - for id, res := range qdr.Responses { + for _, refID := range refIDs { + res := qdr.Responses[refID] + if started { stream.WriteMore() } - stream.WriteObjectField(id) + stream.WriteObjectField(refID) obj := res // avoid implicit memory writeDataResponseJSON(&obj, stream) started = true diff --git a/backend/json_test.go b/backend/json_test.go index a2c27faf5..ed92f7228 100644 --- a/backend/json_test.go +++ b/backend/json_test.go @@ -2,6 +2,7 @@ package backend_test import ( "encoding/json" + "fmt" "sync" "testing" "time" @@ -83,14 +84,14 @@ func TestDataResponseMarshalJSONConcurrent(t *testing.T) { var wg sync.WaitGroup for i := 0; i < 2; i++ { wg.Add(1) - go func() { + go func(dr backend.DataResponse) { defer wg.Done() for j := 0; j < 100; j++ { jsonData, err := json.Marshal(dr) require.NoError(t, err) require.JSONEq(t, string(initialJSON), string(jsonData)) } - }() + }(dr) } wg.Wait() } @@ -103,14 +104,27 @@ func TestQueryDataResponseMarshalJSONConcurrent(t *testing.T) { var wg sync.WaitGroup for i := 0; i < 2; i++ { wg.Add(1) - go func() { + go func(qdr *backend.QueryDataResponse) { defer wg.Done() for j := 0; j < 100; j++ { jsonData, err := json.Marshal(qdr) require.NoError(t, err) require.JSONEq(t, string(initialJSON), string(jsonData)) } - }() + }(qdr) } wg.Wait() } + +func TestQueryDataResponseOrdering(t *testing.T) { + qdr := backend.NewQueryDataResponse() + qdr.Responses["C"] = testDataResponse() + qdr.Responses["A"] = testDataResponse() + qdr.Responses["B"] = testDataResponse() + b, err := json.Marshal(qdr) + require.NoError(t, err) + + expectedDataResponse := `{"frames":[{"schema":{"name":"simple","fields":[{"name":"time","type":"time","typeInfo":{"frame":"time.Time"}},{"name":"valid","type":"boolean","typeInfo":{"frame":"bool"}}]},"data":{"values":[[1577934240000,1577934300000],[true,false]]}},{"schema":{"name":"other","fields":[{"name":"value","type":"number","typeInfo":{"frame":"float64"}}]},"data":{"values":[[1]]}}]}` + expected := fmt.Sprintf(`{"results":{"A":%s,"B":%s,"C":%s}}`, expectedDataResponse, expectedDataResponse, expectedDataResponse) + require.Equal(t, expected, string(b)) +}