From 39cb46fc1d65b8944b156b3d766090418164871f Mon Sep 17 00:00:00 2001 From: Jon Lundy Date: Fri, 22 Jun 2018 14:31:22 -0600 Subject: [PATCH 1/5] fix template error on generated defaults --- codegen/templates/templates.go | 1 + 1 file changed, 1 insertion(+) diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go index fa3a7507e3a..c048b69e72b 100644 --- a/codegen/templates/templates.go +++ b/codegen/templates/templates.go @@ -115,6 +115,7 @@ func dump(val interface{}) string { buf.WriteString(strconv.Quote(key)) buf.WriteString(":") buf.WriteString(dump(data)) + buf.WriteString(",") } buf.WriteString("}") return buf.String() From ec88817f73d99afae800d12d5880945204644268 Mon Sep 17 00:00:00 2001 From: Jon Lundy Date: Fri, 22 Jun 2018 14:38:29 -0600 Subject: [PATCH 2/5] go fmt --- codegen/model.go | 2 +- codegen/templates/templates.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen/model.go b/codegen/model.go index 3b31f46d75f..164a04d5b50 100644 --- a/codegen/model.go +++ b/codegen/model.go @@ -8,7 +8,7 @@ type Model struct { type ModelField struct { *Type - GQLName string + GQLName string GoVarName string GoFKName string GoFKType string diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go index c048b69e72b..d6c7a2542d7 100644 --- a/codegen/templates/templates.go +++ b/codegen/templates/templates.go @@ -115,7 +115,7 @@ func dump(val interface{}) string { buf.WriteString(strconv.Quote(key)) buf.WriteString(":") buf.WriteString(dump(data)) - buf.WriteString(",") + buf.WriteString(",") } buf.WriteString("}") return buf.String() From 31be9471f31fd1655e25b7a192c35f1a33e459b7 Mon Sep 17 00:00:00 2001 From: Jon Lundy Date: Sat, 23 Jun 2018 12:40:07 -0600 Subject: [PATCH 3/5] add test for default fix --- example/dataloader/addressloader_gen.go | 30 ++++++++++++++-------- example/dataloader/itemsliceloader_gen.go | 27 +++++++++++-------- example/dataloader/ordersliceloader_gen.go | 27 +++++++++++-------- example/scalars/generated.go | 4 +-- example/scalars/schema.graphql | 2 +- 5 files changed, 57 insertions(+), 33 deletions(-) diff --git a/example/dataloader/addressloader_gen.go b/example/dataloader/addressloader_gen.go index ffa2f6c9445..b99ad3635d3 100644 --- a/example/dataloader/addressloader_gen.go +++ b/example/dataloader/addressloader_gen.go @@ -1,4 +1,4 @@ -// generated by github.com/vektah/dataloaden ; DO NOT EDIT +// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. package dataloader @@ -73,17 +73,14 @@ func (l *AddressLoader) LoadThunk(key int) func() (*Address, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[pos] + err = batch.error[0] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - if l.cache == nil { - l.cache = map[int]*Address{} - } - l.cache[key] = data + l.unsafeSet(key, data) l.mu.Unlock() } @@ -108,14 +105,20 @@ func (l *AddressLoader) LoadAll(keys []int) ([]*Address, []error) { return addresss, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made. +// Prime the cache with the provided key and value. If the key already exists, no change is made +// and false is returned. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *AddressLoader) Prime(key int, value *Address) { +func (l *AddressLoader) Prime(key int, value *Address) bool { l.mu.Lock() - if _, found := l.cache[key]; !found { - l.cache[key] = value + var found bool + if _, found = l.cache[key]; !found { + // make a copy when writing to the cache, its easy to pass a pointer in from a loop var + // and end up with the whole cache pointing to the same value. + cpy := *value + l.unsafeSet(key, &cpy) } l.mu.Unlock() + return !found } // Clear the value at key from the cache, if it exists @@ -125,6 +128,13 @@ func (l *AddressLoader) Clear(key int) { l.mu.Unlock() } +func (l *AddressLoader) unsafeSet(key int, value *Address) { + if l.cache == nil { + l.cache = map[int]*Address{} + } + l.cache[key] = value +} + // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *addressBatch) keyIndex(l *AddressLoader, key int) int { diff --git a/example/dataloader/itemsliceloader_gen.go b/example/dataloader/itemsliceloader_gen.go index d71837d2cbc..55620a50560 100644 --- a/example/dataloader/itemsliceloader_gen.go +++ b/example/dataloader/itemsliceloader_gen.go @@ -1,4 +1,4 @@ -// generated by github.com/vektah/dataloaden ; DO NOT EDIT +// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. package dataloader @@ -73,17 +73,14 @@ func (l *ItemSliceLoader) LoadThunk(key int) func() ([]Item, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[pos] + err = batch.error[0] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - if l.cache == nil { - l.cache = map[int][]Item{} - } - l.cache[key] = data + l.unsafeSet(key, data) l.mu.Unlock() } @@ -108,14 +105,17 @@ func (l *ItemSliceLoader) LoadAll(keys []int) ([][]Item, []error) { return items, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made. +// Prime the cache with the provided key and value. If the key already exists, no change is made +// and false is returned. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *ItemSliceLoader) Prime(key int, value []Item) { +func (l *ItemSliceLoader) Prime(key int, value []Item) bool { l.mu.Lock() - if _, found := l.cache[key]; !found { - l.cache[key] = value + var found bool + if _, found = l.cache[key]; !found { + l.unsafeSet(key, value) } l.mu.Unlock() + return !found } // Clear the value at key from the cache, if it exists @@ -125,6 +125,13 @@ func (l *ItemSliceLoader) Clear(key int) { l.mu.Unlock() } +func (l *ItemSliceLoader) unsafeSet(key int, value []Item) { + if l.cache == nil { + l.cache = map[int][]Item{} + } + l.cache[key] = value +} + // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *itemSliceBatch) keyIndex(l *ItemSliceLoader, key int) int { diff --git a/example/dataloader/ordersliceloader_gen.go b/example/dataloader/ordersliceloader_gen.go index e8f705d183e..432559040da 100644 --- a/example/dataloader/ordersliceloader_gen.go +++ b/example/dataloader/ordersliceloader_gen.go @@ -1,4 +1,4 @@ -// generated by github.com/vektah/dataloaden ; DO NOT EDIT +// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. package dataloader @@ -73,17 +73,14 @@ func (l *OrderSliceLoader) LoadThunk(key int) func() ([]Order, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[pos] + err = batch.error[0] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - if l.cache == nil { - l.cache = map[int][]Order{} - } - l.cache[key] = data + l.unsafeSet(key, data) l.mu.Unlock() } @@ -108,14 +105,17 @@ func (l *OrderSliceLoader) LoadAll(keys []int) ([][]Order, []error) { return orders, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made. +// Prime the cache with the provided key and value. If the key already exists, no change is made +// and false is returned. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *OrderSliceLoader) Prime(key int, value []Order) { +func (l *OrderSliceLoader) Prime(key int, value []Order) bool { l.mu.Lock() - if _, found := l.cache[key]; !found { - l.cache[key] = value + var found bool + if _, found = l.cache[key]; !found { + l.unsafeSet(key, value) } l.mu.Unlock() + return !found } // Clear the value at key from the cache, if it exists @@ -125,6 +125,13 @@ func (l *OrderSliceLoader) Clear(key int) { l.mu.Unlock() } +func (l *OrderSliceLoader) unsafeSet(key int, value []Order) { + if l.cache == nil { + l.cache = map[int][]Order{} + } + l.cache[key] = value +} + // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *orderSliceBatch) keyIndex(l *OrderSliceLoader, key int) int { diff --git a/example/scalars/generated.go b/example/scalars/generated.go index e176b3e04db..4ce81dd6236 100644 --- a/example/scalars/generated.go +++ b/example/scalars/generated.go @@ -203,7 +203,7 @@ func (ec *executionContext) _Query_search(ctx context.Context, field graphql.Col return graphql.Null } } else { - var tmp interface{} = map[string]interface{}{"location": "37,144"} + var tmp interface{} = map[string]interface{}{"location": "37,144", "isBanned": false} var err error arg0, err = UnmarshalSearchArgs(tmp) if err != nil { @@ -1236,7 +1236,7 @@ func (ec *executionContext) introspectType(name string) *introspection.Type { var parsedSchema = schema.MustParse(`type Query { user(id: ID!): User - search(input: SearchArgs = {location: "37,144"}): [User!]! + search(input: SearchArgs = {location: "37,144", isBanned: false}): [User!]! } type User { diff --git a/example/scalars/schema.graphql b/example/scalars/schema.graphql index 48463421e20..032e3f3a991 100644 --- a/example/scalars/schema.graphql +++ b/example/scalars/schema.graphql @@ -1,6 +1,6 @@ type Query { user(id: ID!): User - search(input: SearchArgs = {location: "37,144"}): [User!]! + search(input: SearchArgs = {location: "37,144", isBanned: false}): [User!]! } type User { From 0bd805d4a118d6a18a0a89ac5f8aa2152dad8505 Mon Sep 17 00:00:00 2001 From: Jon Lundy Date: Sun, 24 Jun 2018 18:43:54 -0600 Subject: [PATCH 4/5] . --- example/scalars/generated.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/scalars/generated.go b/example/scalars/generated.go index 4ce81dd6236..5df780fa2af 100644 --- a/example/scalars/generated.go +++ b/example/scalars/generated.go @@ -203,7 +203,7 @@ func (ec *executionContext) _Query_search(ctx context.Context, field graphql.Col return graphql.Null } } else { - var tmp interface{} = map[string]interface{}{"location": "37,144", "isBanned": false} + var tmp interface{} = map[string]interface{}{"isBanned": false, "location": "37,144"} var err error arg0, err = UnmarshalSearchArgs(tmp) if err != nil { From 0eb19a2ac1fcc4021e59f97a5102a5fca3ed9390 Mon Sep 17 00:00:00 2001 From: Jon Lundy Date: Mon, 25 Jun 2018 12:01:36 -0600 Subject: [PATCH 5/5] add key sort for default values --- codegen/templates/templates.go | 11 +++++++- example/dataloader/addressloader_gen.go | 30 ++++++++-------------- example/dataloader/itemsliceloader_gen.go | 27 ++++++++----------- example/dataloader/ordersliceloader_gen.go | 27 ++++++++----------- 4 files changed, 40 insertions(+), 55 deletions(-) diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go index d6c7a2542d7..5a17ec98b7d 100644 --- a/codegen/templates/templates.go +++ b/codegen/templates/templates.go @@ -9,6 +9,7 @@ import ( "strings" "text/template" "unicode" + "sort" ) func Run(name string, tpldata interface{}) (*bytes.Buffer, error) { @@ -111,7 +112,15 @@ func dump(val interface{}) string { case map[string]interface{}: buf := bytes.Buffer{} buf.WriteString("map[string]interface{}{") - for key, data := range val { + var keys []string + for key := range val { + keys = append(keys, key) + } + sort.Strings(keys) + + for _, key := range keys { + data := val[key] + buf.WriteString(strconv.Quote(key)) buf.WriteString(":") buf.WriteString(dump(data)) diff --git a/example/dataloader/addressloader_gen.go b/example/dataloader/addressloader_gen.go index b99ad3635d3..ffa2f6c9445 100644 --- a/example/dataloader/addressloader_gen.go +++ b/example/dataloader/addressloader_gen.go @@ -1,4 +1,4 @@ -// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. +// generated by github.com/vektah/dataloaden ; DO NOT EDIT package dataloader @@ -73,14 +73,17 @@ func (l *AddressLoader) LoadThunk(key int) func() (*Address, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[0] + err = batch.error[pos] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - l.unsafeSet(key, data) + if l.cache == nil { + l.cache = map[int]*Address{} + } + l.cache[key] = data l.mu.Unlock() } @@ -105,20 +108,14 @@ func (l *AddressLoader) LoadAll(keys []int) ([]*Address, []error) { return addresss, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made -// and false is returned. +// Prime the cache with the provided key and value. If the key already exists, no change is made. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *AddressLoader) Prime(key int, value *Address) bool { +func (l *AddressLoader) Prime(key int, value *Address) { l.mu.Lock() - var found bool - if _, found = l.cache[key]; !found { - // make a copy when writing to the cache, its easy to pass a pointer in from a loop var - // and end up with the whole cache pointing to the same value. - cpy := *value - l.unsafeSet(key, &cpy) + if _, found := l.cache[key]; !found { + l.cache[key] = value } l.mu.Unlock() - return !found } // Clear the value at key from the cache, if it exists @@ -128,13 +125,6 @@ func (l *AddressLoader) Clear(key int) { l.mu.Unlock() } -func (l *AddressLoader) unsafeSet(key int, value *Address) { - if l.cache == nil { - l.cache = map[int]*Address{} - } - l.cache[key] = value -} - // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *addressBatch) keyIndex(l *AddressLoader, key int) int { diff --git a/example/dataloader/itemsliceloader_gen.go b/example/dataloader/itemsliceloader_gen.go index 55620a50560..d71837d2cbc 100644 --- a/example/dataloader/itemsliceloader_gen.go +++ b/example/dataloader/itemsliceloader_gen.go @@ -1,4 +1,4 @@ -// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. +// generated by github.com/vektah/dataloaden ; DO NOT EDIT package dataloader @@ -73,14 +73,17 @@ func (l *ItemSliceLoader) LoadThunk(key int) func() ([]Item, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[0] + err = batch.error[pos] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - l.unsafeSet(key, data) + if l.cache == nil { + l.cache = map[int][]Item{} + } + l.cache[key] = data l.mu.Unlock() } @@ -105,17 +108,14 @@ func (l *ItemSliceLoader) LoadAll(keys []int) ([][]Item, []error) { return items, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made -// and false is returned. +// Prime the cache with the provided key and value. If the key already exists, no change is made. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *ItemSliceLoader) Prime(key int, value []Item) bool { +func (l *ItemSliceLoader) Prime(key int, value []Item) { l.mu.Lock() - var found bool - if _, found = l.cache[key]; !found { - l.unsafeSet(key, value) + if _, found := l.cache[key]; !found { + l.cache[key] = value } l.mu.Unlock() - return !found } // Clear the value at key from the cache, if it exists @@ -125,13 +125,6 @@ func (l *ItemSliceLoader) Clear(key int) { l.mu.Unlock() } -func (l *ItemSliceLoader) unsafeSet(key int, value []Item) { - if l.cache == nil { - l.cache = map[int][]Item{} - } - l.cache[key] = value -} - // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *itemSliceBatch) keyIndex(l *ItemSliceLoader, key int) int { diff --git a/example/dataloader/ordersliceloader_gen.go b/example/dataloader/ordersliceloader_gen.go index 432559040da..e8f705d183e 100644 --- a/example/dataloader/ordersliceloader_gen.go +++ b/example/dataloader/ordersliceloader_gen.go @@ -1,4 +1,4 @@ -// Code generated by github.com/vektah/dataloaden, DO NOT EDIT. +// generated by github.com/vektah/dataloaden ; DO NOT EDIT package dataloader @@ -73,14 +73,17 @@ func (l *OrderSliceLoader) LoadThunk(key int) func() ([]Order, error) { var err error // its convenient to be able to return a single error for everything if len(batch.error) == 1 { - err = batch.error[0] + err = batch.error[pos] } else if batch.error != nil { err = batch.error[pos] } if err == nil { l.mu.Lock() - l.unsafeSet(key, data) + if l.cache == nil { + l.cache = map[int][]Order{} + } + l.cache[key] = data l.mu.Unlock() } @@ -105,17 +108,14 @@ func (l *OrderSliceLoader) LoadAll(keys []int) ([][]Order, []error) { return orders, errors } -// Prime the cache with the provided key and value. If the key already exists, no change is made -// and false is returned. +// Prime the cache with the provided key and value. If the key already exists, no change is made. // (To forcefully prime the cache, clear the key first with loader.clear(key).prime(key, value).) -func (l *OrderSliceLoader) Prime(key int, value []Order) bool { +func (l *OrderSliceLoader) Prime(key int, value []Order) { l.mu.Lock() - var found bool - if _, found = l.cache[key]; !found { - l.unsafeSet(key, value) + if _, found := l.cache[key]; !found { + l.cache[key] = value } l.mu.Unlock() - return !found } // Clear the value at key from the cache, if it exists @@ -125,13 +125,6 @@ func (l *OrderSliceLoader) Clear(key int) { l.mu.Unlock() } -func (l *OrderSliceLoader) unsafeSet(key int, value []Order) { - if l.cache == nil { - l.cache = map[int][]Order{} - } - l.cache[key] = value -} - // keyIndex will return the location of the key in the batch, if its not found // it will add the key to the batch func (b *orderSliceBatch) keyIndex(l *OrderSliceLoader, key int) int {