Skip to content

fix: fix conflict of rewriter and required fields #1338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
devsergiy merged 14 commits into from
Oct 31, 2025
Merged

fix: fix conflict of rewriter and required fields #1338

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
835b04e
chore: refactor debug code, refactor requires fragments
devsergiy Sep 29, 2025
d7af710
mark node selections as orphans during cleanup of tree nodes
devsergiy Oct 1, 2025
6812313
fix skip field refs
devsergiy Oct 7, 2025
3604f50
remove has unresolved fields from node selection visitor
devsergiy Oct 21, 2025
f0a0cbb
add test
devsergiy Oct 22, 2025
ec723fd
rewrite at the end of walk
devsergiy Oct 22, 2025
91feccf
update field refs of fields waiting for dependency after rewrite
devsergiy Oct 23, 2025
1cc32a0
Revert "rewrite at the end of walk"
devsergiy Oct 29, 2025
f89666d
add required fields on enter selection set
devsergiy Oct 29, 2025
ec0d318
fix test
devsergiy Oct 29, 2025
af1f877
force rewrite in case we modified already rewrited field
devsergiy Oct 30, 2025
07517e1
fix and edge case for field ref depending on itself
devsergiy Oct 30, 2025
4ac7de9
chore: fix lint
devsergiy Oct 30, 2025
48d8bd9
fix debug ref
devsergiy Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 43 additions & 23 deletions v2/pkg/astprinter/astprinter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,17 @@ func (p *printVisitor) EnterArgument(ref int) {
}

func (p *printVisitor) LeaveArgument(ref int) {
if len(p.document.ArgumentsAfter(p.Ancestors[len(p.Ancestors)-1], ref)) == 0 {
p.write(literal.RPAREN)
if len(p.document.ArgumentsAfter(p.Ancestors[len(p.Ancestors)-1], ref)) > 0 {
// if not the last argument
return
}

// last argument
p.write(literal.RPAREN)

a := p.Ancestors[len(p.Ancestors)-1]
if p.debug && a.Kind == ast.NodeKindField && !p.document.FieldHasSelections(a.Ref) && !p.document.FieldHasDirectives(a.Ref) {
p.printFieldInfo(a.Ref, true)
}
}

Expand Down Expand Up @@ -319,14 +328,23 @@ func (p *printVisitor) LeaveOperationDefinition(ref int) {
}

func (p *printVisitor) EnterSelectionSet(ref int) {
p.write(literal.LBRACE)

if p.debug {
p.writeIndented(literal.LINETERMINATOR)
p.writeIndented([]byte("# SelectionSet ref:"))
p.write([]byte(" # setRef:"))
p.write([]byte(strconv.Itoa(ref)))
p.write(literal.LINETERMINATOR)
p.writeIndented(literal.LBRACE)
} else {
p.write(literal.LBRACE)

if l := len(p.SimpleWalker.Ancestors); l > 0 {
a := p.SimpleWalker.Ancestors[l-1]
switch a.Kind {
case ast.NodeKindField:
p.printFieldInfo(a.Ref, false)
case ast.NodeKindInlineFragment:
p.write([]byte(" fragmentRef:"))
p.write([]byte(strconv.Itoa(ref)))
default:
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

if p.indent != nil {
Expand All @@ -342,15 +360,6 @@ func (p *printVisitor) LeaveSelectionSet(ref int) {
}

func (p *printVisitor) EnterField(ref int) {
if p.debug {
p.writeIndented([]byte("# Field ref:"))
p.write([]byte(strconv.Itoa(ref)))
if p.fieldCallback != nil {
p.fieldCallback(ref, p.out)
}
p.write(literal.LINETERMINATOR)
}

if p.document.Fields[ref].Alias.IsDefined {
p.writeIndented(p.document.Input.ByteSlice(p.document.Fields[ref].Alias.Name))
p.write(literal.COLON)
Expand All @@ -362,6 +371,23 @@ func (p *printVisitor) EnterField(ref int) {
if !p.document.FieldHasArguments(ref) && (p.document.FieldHasSelections(ref) || p.document.FieldHasDirectives(ref)) {
p.write(literal.SPACE)
}

if p.debug && !p.document.FieldHasArguments(ref) && !p.document.FieldHasSelections(ref) && !p.document.FieldHasDirectives(ref) {
p.printFieldInfo(ref, true)
}
}

func (p *printVisitor) printFieldInfo(ref int, comment bool) {
if p.debug {
if comment {
p.write([]byte(" #"))
}
p.write([]byte(" fieldRef:"))
p.write([]byte(strconv.Itoa(ref)))
if p.fieldCallback != nil {
p.fieldCallback(ref, p.out)
}
}
}

func (p *printVisitor) LeaveField(ref int) {
Expand Down Expand Up @@ -399,12 +425,6 @@ func (p *printVisitor) LeaveFragmentSpread(ref int) {
}

func (p *printVisitor) EnterInlineFragment(ref int) {
if p.debug {
p.write(literal.LINETERMINATOR)
p.writeIndented([]byte("# InlineFragment ref:"))
p.write([]byte(strconv.Itoa(ref)))
p.write(literal.LINETERMINATOR)
}
p.writeIndented(literal.SPREAD)

if p.document.InlineFragmentHasTypeCondition(ref) && !p.document.InlineFragmentIsOfTheSameType(ref) {
Expand Down
14 changes: 7 additions & 7 deletions v2/pkg/engine/datasource/graphql_datasource/graphql_datasource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ func (p *Planner[T]) debugPrintQueryPlan(operation *ast.Document) {
if p.hasFederationRoot && p.dataSourcePlannerConfig.HasRequiredFields() { // IsRepresentationsQuery
args = append(args, "Representations:\n")
for _, cfg := range p.dataSourcePlannerConfig.RequiredFields {
key, report := plan.RequiredFieldsFragment(cfg.TypeName, cfg.SelectionSet, cfg.FieldName == "")
key, report := plan.QueryPlanRequiredFieldsFragment(cfg.TypeName, cfg.FieldName, cfg.SelectionSet)
if report.HasErrors() {
continue
}
Expand Down Expand Up @@ -1407,7 +1407,7 @@ func (p *Planner[T]) generateQueryPlansForFetchConfiguration(operation *ast.Docu
if p.hasFederationRoot && p.dataSourcePlannerConfig.HasRequiredFields() { // IsRepresentationsQuery
representations = make([]resolve.Representation, len(p.dataSourcePlannerConfig.RequiredFields))
for i, cfg := range p.dataSourcePlannerConfig.RequiredFields {
fragmentAst, report := plan.QueryPlanRequiredFieldsFragment(cfg.FieldName, cfg.TypeName, cfg.SelectionSet)
fragmentAst, report := plan.QueryPlanRequiredFieldsFragment(cfg.TypeName, cfg.FieldName, cfg.SelectionSet)
if report.HasErrors() {
p.stopWithError(errors.WithStack(fmt.Errorf("generateQueryPlansForFetchConfiguration: failed to build fragment for required fields: %w", report)))
return
Expand Down Expand Up @@ -1446,7 +1446,7 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b
// create empty operation and definition documents
definition, err := p.config.UpstreamSchema()
if err != nil {
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation: failed to read upstream schema: %w", err)))
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: failed to read upstream schema: %w", p.id, err)))
return nil, nil
}

Expand All @@ -1457,7 +1457,7 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b
// normalize upstream operation
kit.normalizer.NormalizeOperation(p.upstreamOperation, definition, kit.report)
if kit.report.HasErrors() {
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation: normalization failed: %w", kit.report)))
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: normalization failed: %w", p.id, kit.report)))
return nil, nil
}

Expand All @@ -1468,7 +1468,7 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b

kit.validator.Validate(p.upstreamOperation, definition, kit.report)
if kit.report.HasErrors() {
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation: validation failed: %w", kit.report)))
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: validation failed: %w", p.id, kit.report)))
return nil, nil
}

Expand All @@ -1478,7 +1478,7 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b
// print upstream operation
err = kit.printer.Print(p.upstreamOperation, kit.buf)
if err != nil {
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation: failed to print: %w", err)))
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: failed to print: %w", p.id, err)))
return nil, nil
}

Expand All @@ -1493,7 +1493,7 @@ func (p *Planner[T]) printOperation() (operationBytes []byte, variablesBytes []b
Pretty: false,
}, kit.buf)
if err != nil {
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation: failed to minify: %w", err)))
p.stopWithError(errors.WithStack(fmt.Errorf("printOperation planner id: %d: failed to minify: %w", p.id, err)))
return nil, nil
}
if madeReplacements && kit.buf.Len() < len(rawOperationBytes) {
Expand Down
56 changes: 28 additions & 28 deletions ...ine/datasource/graphql_datasource/graphql_datasource_federation_entity_interfaces_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4587,16 +4587,17 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
DependsOnFetchIDs: []int{0},
},
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Admin {__typename} ... on Moderator {__typename title} ... on User {__typename title}}}","variables":{"representations":[$$0$$]}}}`,
Input: `{"method":"POST","url":"http://localhost:4004/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {age}}}","variables":{"representations":[$$0$$]}}}`,
Variables: []resolve.Variable{
&resolve.ResolvableObjectVariable{
Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{
Nullable: true,
Fields: []*resolve.Field{
{
Name: []byte("__typename"),
Value: &resolve.String{
Path: []string{"__typename"},
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
},
Expand All @@ -4609,8 +4610,9 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
},
{
Name: []byte("__typename"),
Value: &resolve.String{
Path: []string{"__typename"},
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
},
Expand All @@ -4623,8 +4625,9 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
},
{
Name: []byte("__typename"),
Value: &resolve.String{
Path: []string{"__typename"},
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
},
Expand Down Expand Up @@ -4652,17 +4655,16 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
DependsOnFetchIDs: []int{0},
},
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"method":"POST","url":"http://localhost:4004/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {age}}}","variables":{"representations":[$$0$$]}}}`,
Input: `{"method":"POST","url":"http://localhost:4001/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Admin {__typename} ... on Moderator {__typename title} ... on User {__typename title}}}","variables":{"representations":[$$0$$]}}}`,
Variables: []resolve.Variable{
&resolve.ResolvableObjectVariable{
Renderer: resolve.NewGraphQLVariableResolveRenderer(&resolve.Object{
Nullable: true,
Fields: []*resolve.Field{
{
Name: []byte("__typename"),
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
Value: &resolve.String{
Path: []string{"__typename"},
},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
},
Expand All @@ -4675,9 +4677,8 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
},
{
Name: []byte("__typename"),
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
Value: &resolve.String{
Path: []string{"__typename"},
},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
},
Expand All @@ -4690,9 +4691,8 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
},
{
Name: []byte("__typename"),
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
Value: &resolve.String{
Path: []string{"__typename"},
},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
},
Expand All @@ -4718,7 +4718,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
resolve.SingleWithPath(&resolve.SingleFetch{
FetchDependencies: resolve.FetchDependencies{
FetchID: 3,
DependsOnFetchIDs: []int{0, 1},
DependsOnFetchIDs: []int{0, 2},
},
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"method":"POST","url":"http://localhost:4003/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Admin {__typename title}}}","variables":{"representations":[$$0$$]}}}`,
Expand Down Expand Up @@ -4755,7 +4755,7 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
resolve.SingleWithPath(&resolve.SingleFetch{
FetchDependencies: resolve.FetchDependencies{
FetchID: 4,
DependsOnFetchIDs: []int{0, 1, 3},
DependsOnFetchIDs: []int{0, 2, 3},
},
FetchConfiguration: resolve.FetchConfiguration{
Input: `{"method":"POST","url":"http://localhost:4002/graphql","body":{"query":"query($representations: [_Any!]!){_entities(representations: $representations){... on Account {uniqueTitle}}}","variables":{"representations":[$$0$$]}}}`,
Expand All @@ -4770,65 +4770,65 @@ func TestGraphQLDataSourceFederationEntityInterfaces(t *testing.T) {
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
},
{
Name: []byte("title"),
Value: &resolve.String{
Path: []string{"title"},
},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
},
{
Name: []byte("id"),
Value: &resolve.Scalar{
Path: []string{"id"},
},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
},
{
Name: []byte("__typename"),
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
},
{
Name: []byte("title"),
Value: &resolve.String{
Path: []string{"title"},
},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
},
{
Name: []byte("id"),
Value: &resolve.Scalar{
Path: []string{"id"},
},
OnTypeNames: [][]byte{[]byte("Moderator"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
},
{
Name: []byte("__typename"),
Value: &resolve.StaticString{
Path: []string{"__typename"},
Value: "Account",
},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
},
{
Name: []byte("title"),
Value: &resolve.String{
Path: []string{"title"},
},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
},
{
Name: []byte("id"),
Value: &resolve.Scalar{
Path: []string{"id"},
},
OnTypeNames: [][]byte{[]byte("User"), []byte("Account")},
OnTypeNames: [][]byte{[]byte("Admin"), []byte("Account")},
},
},
}),
Expand Down
Loading