From 7680146f85b05775a2fdeac9e9b20983cb43bf59 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Sat, 13 Jun 2020 09:27:39 +0200 Subject: [PATCH 1/3] Add .idea to the .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 48b8bf9..ae09aba 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ vendor/ +.idea \ No newline at end of file From 3afb6aefab8b9149f47fcab0bf2d13b454bfd9f1 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Sat, 13 Jun 2020 09:28:05 +0200 Subject: [PATCH 2/3] Synced the lockfile of Gopkg --- Gopkg.lock | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 2930c69..214e4e9 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -1,33 +1,69 @@ # This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. +[[projects]] + digest = "1:e0419edd44e362e2ab972a6f68c91a20adaa79ff24a6fd511055d1f0fcd93b79" + name = "github.com/graph-gophers/dataloader" + packages = ["."] + pruneopts = "" + revision = "78139374585c29dcb97b8f33089ed11959e4be59" + version = "v5" + [[projects]] branch = "master" + digest = "1:43987212a2f16bfacc1a286e9118f212d60c136ed53c6c9477c18921db53140b" name = "github.com/hashicorp/golang-lru" - packages = [".","simplelru"] + packages = [ + ".", + "simplelru", + ] + pruneopts = "" revision = "0a025b7e63adc15a622f29b0b2c4c3848243bbf6" [[projects]] + digest = "1:e0419edd44e362e2ab972a6f68c91a20adaa79ff24a6fd511055d1f0fcd93b79" + name = "github.com/nicksrandall/dataloader" + packages = ["."] + pruneopts = "" + revision = "78139374585c29dcb97b8f33089ed11959e4be59" + version = "v5" + +[[projects]] + digest = "1:78fb99d6011c2ae6c72f3293a83951311147b12b06a5ffa43abf750c4fab6ac5" name = "github.com/opentracing/opentracing-go" - packages = [".","log"] + packages = [ + ".", + "log", + ] + pruneopts = "" revision = "1949ddbfd147afd4d964a9f00b24eb291e0e7c38" version = "v1.0.2" [[projects]] + digest = "1:4c0404dc03d974acd5fcd8b8d3ce687b13bd169db032b89275e8b9d77b98ce8c" name = "github.com/patrickmn/go-cache" packages = ["."] + pruneopts = "" revision = "a3647f8e31d79543b2d0f0ae2fe5c379d72cedc0" version = "v2.1.0" [[projects]] branch = "master" + digest = "1:447831205e1c85dbf1e6f97cb8ca54931452c6aff79c630ea091ecbbdc2e921e" name = "golang.org/x/net" packages = ["context"] + pruneopts = "" revision = "a8b9294777976932365dabb6640cf1468d95c70f" [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "a0b8606d9f2ed9df7e69cae570c65c7d7b090bb7a08f58d3535b584693d44da9" + input-imports = [ + "github.com/graph-gophers/dataloader", + "github.com/hashicorp/golang-lru", + "github.com/nicksrandall/dataloader", + "github.com/opentracing/opentracing-go", + "github.com/patrickmn/go-cache", + ] solver-name = "gps-cdcl" solver-version = 1 From 98e283a5696b7249f50b22fab0c49eea6bf294b0 Mon Sep 17 00:00:00 2001 From: Joost Pastoor Date: Sat, 13 Jun 2020 09:28:41 +0200 Subject: [PATCH 3/3] Prevent panic when batchloader returns nil values by replacing them with Result errors --- dataloader.go | 7 +++++++ dataloader_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/dataloader.go b/dataloader.go index 8f04581..d210103 100644 --- a/dataloader.go +++ b/dataloader.go @@ -460,6 +460,13 @@ func (b *batcher) batch(originalContext context.Context) { return } + // When a batchFunc returns a nil in it's items, we replace those by a Result struct containing an error + for key, value := range items { + if value == nil { + items[key] = &Result{Error: fmt.Errorf("no value for key")} + } + } + for i, req := range reqs { req.channel <- items[i] close(req.channel) diff --git a/dataloader_test.go b/dataloader_test.go index d676731..9c97afe 100644 --- a/dataloader_test.go +++ b/dataloader_test.go @@ -211,6 +211,32 @@ func TestLoader(t *testing.T) { // TODO: expect to get some kind of warning }) + t.Run("first result is a nil", func(t *testing.T) { + t.Parallel() + faultyLoader, _ := LoaderNilInsteadOfResult() + ctx := context.Background() + + n := 10 + reqs := []Thunk{} + keys := Keys{} + for i := 0; i < n; i++ { + key := StringKey(strconv.Itoa(i)) + reqs = append(reqs, faultyLoader.Load(ctx, key)) + keys = append(keys, key) + } + + for i, future := range reqs { + _, err := future() + if i == 0 && err == nil { + t.Error("expected first result to contain an error") + } + + if i != 0 && err != nil { + t.Error("expected rest of results not to contain an error") + } + } + }) + t.Run("responds to max batch size", func(t *testing.T) { t.Parallel() identityLoader, loadCalls := IDLoader(2) @@ -586,6 +612,30 @@ func FaultyLoader() (*Loader, *[][]string) { return loader, &loadCalls } +// LoaderNilInsteadOfResult gives a nil result back for the first key. +func LoaderNilInsteadOfResult() (*Loader, *[][]string) { + var mu sync.Mutex + var loadCalls [][]string + + loader := NewBatchedLoader(func(_ context.Context, keys Keys) []*Result { + var results []*Result + mu.Lock() + loadCalls = append(loadCalls, keys.Keys()) + mu.Unlock() + + for i, key := range keys { + if i == 0 { + results = append(results, nil) + } else { + results = append(results, &Result{key, nil}) + } + } + return results + }) + + return loader, &loadCalls +} + /////////////////////////////////////////////////// // Benchmarks ///////////////////////////////////////////////////