diff --git a/CHANGELOG.md b/CHANGELOG.md index 69b5aa27..dee5fa3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # CHANGELOG -[Unreleased] +[v1.7.1] +* [IMPROVEMENT] `SelectedFieldNames` now returns dot-delimited nested field paths (e.g. `products`, `products.id`, `products.category`, `products.category.id`). Intermediate container object/list paths are included so resolvers can check for both a branch (`products.category`) and its leaves (`products.category.id`). `HasSelectedField` and `SortedSelectedFieldNames` operate on these paths. This aligns behavior with typical resolver projection needs and fixes missing nested selections. * [BUGFIX] Reject object, interface, and input object type definitions that declare zero fields/input values (spec compliance). * [IMPROVEMENT] Optimize overlapping field validation to avoid quadratic memory blowups on large sibling field lists. * [FEATURE] Add configurable safety valve for overlapping field comparison count with `OverlapValidationLimit(n)` schema option (0 disables the cap). When exceeded validation aborts early with rule `OverlapValidationLimitExceeded`. Disabled by default. diff --git a/example_selection_test.go b/example_selection_test.go index 70047d64..9bd25c52 100644 --- a/example_selection_test.go +++ b/example_selection_test.go @@ -12,10 +12,16 @@ type ( userResolver struct{ u user } ) -func (r *userResolver) ID() graphql.ID { return graphql.ID(r.u.id) } -func (r *userResolver) Name() *string { return &r.u.name } -func (r *userResolver) Email() *string { return &r.u.email } -func (r *userResolver) Friends(ctx context.Context) []*userResolver { return nil } +func (r *userResolver) ID() graphql.ID { return graphql.ID(r.u.id) } +func (r *userResolver) Name() *string { return &r.u.name } +func (r *userResolver) Email() *string { return &r.u.email } +func (r *userResolver) Friends(ctx context.Context) []*userResolver { + // Return a couple of dummy friends (data itself not important for field selection example) + return []*userResolver{ + {u: user{id: "F1", name: "Bob"}}, + {u: user{id: "F2", name: "Carol"}}, + } +} type root struct{} @@ -34,8 +40,8 @@ func Example_selectedFieldNames() { type User { id: ID! name: String email: String friends: [User!]! } ` schema := graphql.MustParseSchema(s, &root{}) - query := `query { user(id: "U1") { id name } }` + query := `query { user(id: "U1") { id name friends { id name } } }` _ = schema.Exec(context.Background(), query, "", nil) // Output: - // [id name] + // [id name friends friends.id friends.name] } diff --git a/internal/selections/context.go b/internal/selections/context.go index fd56b3c9..a1621d67 100644 --- a/internal/selections/context.go +++ b/internal/selections/context.go @@ -33,27 +33,10 @@ func (l *Lazy) Names() []string { l.once.Do(func() { seen := make(map[string]struct{}, len(l.raw)) ordered := make([]string, 0, len(l.raw)) - for _, s := range l.raw { - switch s := s.(type) { - case *selected.SchemaField: - name := s.Name - if len(name) >= 2 && name[:2] == "__" { - continue - } - if _, ok := seen[name]; !ok { - seen[name] = struct{}{} - ordered = append(ordered, name) - } - case *selected.TypeAssertion: - collectFromTypeAssertion(&ordered, seen, s.Sels) - case *selected.TypenameField: - continue - } - } + collectNestedPaths(&ordered, seen, "", l.raw) l.names = ordered l.set = seen }) - // Return a copy to keep internal slice immutable to callers. out := make([]string, len(l.names)) copy(out, l.names) return out @@ -71,21 +54,27 @@ func (l *Lazy) Has(name string) bool { return ok } -// collectFromTypeAssertion flattens selections under a type assertion fragment. -func collectFromTypeAssertion(dst *[]string, seen map[string]struct{}, sels []selected.Selection) { - for _, s := range sels { - switch s := s.(type) { +func collectNestedPaths(dst *[]string, seen map[string]struct{}, prefix string, sels []selected.Selection) { + for _, sel := range sels { + switch s := sel.(type) { case *selected.SchemaField: name := s.Name if len(name) >= 2 && name[:2] == "__" { continue } - if _, ok := seen[name]; !ok { - seen[name] = struct{}{} - *dst = append(*dst, name) + path := name + if prefix != "" { + path = prefix + "." + name + } + if _, ok := seen[path]; !ok { + seen[path] = struct{}{} + *dst = append(*dst, path) + } + if len(s.Sels) > 0 { + collectNestedPaths(dst, seen, path, s.Sels) } case *selected.TypeAssertion: - collectFromTypeAssertion(dst, seen, s.Sels) + collectNestedPaths(dst, seen, prefix, s.Sels) case *selected.TypenameField: continue } diff --git a/selection.go b/selection.go index f7b03a96..6c1082ae 100644 --- a/selection.go +++ b/selection.go @@ -7,22 +7,21 @@ import ( "github.com/graph-gophers/graphql-go/internal/selections" ) -// SelectedFieldNames returns the set of immediate child field names selected -// on the value returned by the current resolver. It returns an empty slice -// when the current field's return type is a leaf (scalar / enum) or when the -// feature was disabled at schema construction via DisableFieldSelections. -// The returned slice is a copy and is safe for the caller to modify. -// -// It is intentionally simple and does not expose the internal AST. If more -// detailed information is needed in the future (e.g. arguments per child, -// nested trees) a separate API can be added without breaking this one. +// SelectedFieldNames returns the set of selected field paths underneath the +// the current resolver. Paths are dot-delimited for nested structures (e.g. +// "products", "products.id", "products.category.id"). Immediate child field +// names are always present (even when they have further children). Order preserves +// the first appearance in the query after fragment flattening, performing a +// depth-first traversal. +// It returns an empty slice when the current field's return type is a leaf +// (scalar / enum) or when DisableFieldSelections was used at schema creation. +// The returned slice is a copy safe for caller modification. // // Notes: -// - Fragment spreads & inline fragments are flattened; the union of all -// possible child fields is returned (deduplicated, preserving first -// appearance order in the query document). -// - Field aliases are ignored; the original schema field names are returned. +// - Fragment spreads & inline fragments are flattened. +// - Field aliases are ignored; original schema field names are used. // - Meta fields beginning with "__" (including __typename) are excluded. +// - Duplicate paths are removed, preserving the earliest occurrence. func SelectedFieldNames(ctx context.Context) []string { // If no selection info is present (leaf field or no child selections), return empty slice. lazy := selections.FromContext(ctx) @@ -32,9 +31,9 @@ func SelectedFieldNames(ctx context.Context) []string { return lazy.Names() } -// HasSelectedField returns true if the immediate child selection list contains -// the provided field name (case sensitive). It returns false for leaf return -// types and when DisableFieldSelections was used. +// HasSelectedField returns true if the child selection list contains the provided +// (possibly nested) path (case sensitive). It returns false for leaf resolvers +// and when DisableFieldSelections was used. func HasSelectedField(ctx context.Context, name string) bool { lazy := selections.FromContext(ctx) if lazy == nil { diff --git a/selection_test.go b/selection_test.go index b6a6ef07..891e0cd4 100644 --- a/selection_test.go +++ b/selection_test.go @@ -8,9 +8,11 @@ import ( ) const selectionTestSchema = ` - schema { query: Query } - type Query { hero: Human } - type Human { id: ID! name: String } + schema { query: Query } + type Query { customer: Customer } + type Customer { id: ID! name: String items: [Item!]! } + type Item { id: ID! name: String category: Category } + type Category { id: ID! } ` type selectionRoot struct { @@ -19,80 +21,100 @@ type selectionRoot struct { expectSorted []string hasChecks map[string]bool } - -type selectionHuman struct { - t *testing.T - id string - name string +type selectionCustomer struct { + t *testing.T + id, name string } -func (r *selectionRoot) Hero(ctx context.Context) *selectionHuman { - names := graphql.SelectedFieldNames(ctx) - sorted := graphql.SortedSelectedFieldNames(ctx) - if !equalStringSlices(names, r.expectNames) { - r.t.Errorf("SelectedFieldNames = %v, want %v", names, r.expectNames) +func (r *selectionRoot) Customer(ctx context.Context) *selectionCustomer { + if r.expectNames != nil { + names := graphql.SelectedFieldNames(ctx) + if !equalStringSlices(names, r.expectNames) { + r.t.Errorf("SelectedFieldNames = %v, want %v", names, r.expectNames) + } } - if !equalStringSlices(sorted, r.expectSorted) { - r.t.Errorf("SortedSelectedFieldNames = %v, want %v", sorted, r.expectSorted) + if r.expectSorted != nil { + sorted := graphql.SortedSelectedFieldNames(ctx) + if !equalStringSlices(sorted, r.expectSorted) { + r.t.Errorf("SortedSelectedFieldNames = %v, want %v", sorted, r.expectSorted) + } } - for name, want := range r.hasChecks { - if got := graphql.HasSelectedField(ctx, name); got != want { - r.t.Errorf("HasSelectedField(%q) = %v, want %v", name, got, want) + for n, want := range r.hasChecks { + if got := graphql.HasSelectedField(ctx, n); got != want { + r.t.Errorf("HasSelectedField(%q) = %v, want %v", n, got, want) } } - return &selectionHuman{t: r.t, id: "h1", name: "Luke"} + return &selectionCustomer{t: r.t, id: "c1", name: "Alice"} } -// Object-level assertions happen in Hero via a wrapper test function; leaf behavior tested here. -func (h *selectionHuman) ID() graphql.ID { return graphql.ID(h.id) } - -func (h *selectionHuman) Name(ctx context.Context) *string { - // Leaf field: should always produce empty selections regardless of enable/disable. - if got := graphql.SelectedFieldNames(ctx); len(got) != 0 { - h.t.Errorf("leaf field SelectedFieldNames = %v, want empty", got) +func (h *selectionCustomer) ID() graphql.ID { return graphql.ID(h.id) } +func (h *selectionCustomer) Name(ctx context.Context) *string { + if len(graphql.SelectedFieldNames(ctx)) != 0 { + h.t.Errorf("leaf selections should be empty") } if graphql.HasSelectedField(ctx, "anything") { - h.t.Errorf("leaf field HasSelectedField unexpectedly true") + h.t.Errorf("unexpected leaf HasSelectedField true") } - if sorted := graphql.SortedSelectedFieldNames(ctx); len(sorted) != 0 { - h.t.Errorf("leaf field SortedSelectedFieldNames = %v, want empty", sorted) + if len(graphql.SortedSelectedFieldNames(ctx)) != 0 { + h.t.Errorf("leaf sorted selections should be empty") } return &h.name } +// nested types for extended schema +type selectionItem struct { + id, name string + category *selectionCategory +} +type selectionCategory struct{ id string } + +func (h *selectionCustomer) Items() []*selectionItem { + return []*selectionItem{{id: "i1", name: "Item", category: &selectionCategory{id: "cat1"}}} +} +func (p *selectionItem) ID() graphql.ID { return graphql.ID(p.id) } +func (p *selectionItem) Name() *string { return &p.name } +func (p *selectionItem) Category() *selectionCategory { return p.category } +func (c *selectionCategory) ID() graphql.ID { return graphql.ID(c.id) } + func TestFieldSelectionHelpers(t *testing.T) { tests := []struct { name string schemaOpts []graphql.SchemaOpt query string - expectNames []string // expected order from SelectedFieldNames at object boundary - expectSorted []string // expected from SortedSelectedFieldNames at object boundary + expectNames []string + expectSorted []string hasChecks map[string]bool }{ { - name: "enabled object order preserved and sorted copy", - query: `query { hero { name id } }`, // order intentionally name,id + name: "enabled order", + query: `query { customer { name id } }`, expectNames: []string{"name", "id"}, expectSorted: []string{"id", "name"}, - hasChecks: map[string]bool{"id": true, "name": true, "missing": false}, + hasChecks: map[string]bool{"id": true, "name": true}, }, { - name: "enabled only one field selected", - query: `query { hero { id } }`, // order intentionally name,id + name: "one field", + query: `query { customer { id } }`, expectNames: []string{"id"}, expectSorted: []string{"id"}, - hasChecks: map[string]bool{"id": true, "name": false, "missing": false}, + hasChecks: map[string]bool{"id": true, "name": false}, + }, + { + name: "nested paths", + query: `query { customer { items { id name category { id } } id } }`, + expectNames: []string{"items", "items.id", "items.name", "items.category", "items.category.id", "id"}, + expectSorted: []string{"id", "items", "items.category", "items.category.id", "items.id", "items.name"}, + hasChecks: map[string]bool{"items": true, "items.id": true, "items.name": true, "items.category": true, "items.category.id": true, "id": true}, }, { - name: "disabled object returns empty", + name: "disabled", schemaOpts: []graphql.SchemaOpt{graphql.DisableFieldSelections()}, - query: `query { hero { name id } }`, + query: `query { customer { name id } }`, expectNames: []string{}, expectSorted: []string{}, hasChecks: map[string]bool{"id": false, "name": false}, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { root := &selectionRoot{t: t, expectNames: tt.expectNames, expectSorted: tt.expectSorted, hasChecks: tt.hasChecks} @@ -107,37 +129,35 @@ func TestFieldSelectionHelpers(t *testing.T) { func TestSelectedFieldNames_FragmentsAliasesMeta(t *testing.T) { tests := []struct { - name string - query string + name, query string expectNames []string hasChecks map[string]bool }{ { - name: "alias ignored order preserved", - query: `query { hero { idAlias: id name } }`, + name: "alias ignored", + query: `query { customer { idAlias: id name } }`, expectNames: []string{"id", "name"}, hasChecks: map[string]bool{"id": true, "idAlias": false, "name": true}, }, { - name: "fragment spread flattened", - query: `fragment HFields on Human { id name } query { hero { ...HFields } }`, + name: "fragment spread", + query: `fragment CFields on Customer { id name } query { customer { ...CFields } }`, expectNames: []string{"id", "name"}, hasChecks: map[string]bool{"id": true, "name": true}, }, { - name: "inline fragment dedup", - query: `query { hero { id ... on Human { id name } } }`, + name: "inline fragment", + query: `query { customer { id ... on Customer { id name } } }`, expectNames: []string{"id", "name"}, hasChecks: map[string]bool{"id": true, "name": true}, }, { - name: "meta field excluded", - query: `query { hero { id __typename name } }`, + name: "meta excluded", + query: `query { customer { id __typename name } }`, expectNames: []string{"id", "name"}, hasChecks: map[string]bool{"id": true, "name": true, "__typename": false}, }, } - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { root := &selectionRoot{t: t, expectNames: tt.expectNames, expectSorted: tt.expectNames, hasChecks: tt.hasChecks} @@ -150,7 +170,6 @@ func TestSelectedFieldNames_FragmentsAliasesMeta(t *testing.T) { } } -// equalStringSlices compares content and order. func equalStringSlices(a, b []string) bool { if len(a) != len(b) { return false