Skip to content

Commit

Permalink
Fix stable sort order of query response when encoded to JSON (#425)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
marefr authored and sunker committed Jan 26, 2022
1 parent 03a2da2 commit 793be91
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 7 deletions.
8 changes: 7 additions & 1 deletion Magefile.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//+build mage
//go:build mage
// +build mage

package main

Expand Down Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions backend/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package backend

import (
"fmt"
"sort"
"unsafe"

"github.com/grafana/grafana-plugin-sdk-go/data"
Expand Down Expand Up @@ -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
Expand Down
22 changes: 18 additions & 4 deletions backend/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package backend_test

import (
"encoding/json"
"fmt"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -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()
}
Expand All @@ -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))
}

0 comments on commit 793be91

Please sign in to comment.