Skip to content

Commit 8c8c9de

Browse files
authored
fix: fix rewriting an interface object implementing interface (#1265)
<!-- Important: Before developing new features, please open an issue to discuss your ideas with the maintainers. This ensures project alignment and helps avoid unnecessary work for you. Thank you for your contribution! Please provide a detailed description below and ensure you've met all the requirements. Squashed commit messages must follow the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) standard to facilitate changelog generation. Please ensure your PR title follows the Conventional Commits specification, using the appropriate type (e.g., feat, fix, docs) and scope. Examples of good PR titles: - 💥feat!: change implementation in an non-backward compatible way - ✨feat(auth): add support for OAuth2 login - 🐞fix(router): add support for custom metrics - 📚docs(README): update installation instructions - 🧹chore(deps): bump dependencies to latest versions --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of interface objects in federated graph schemas to ensure correct expansion into their concrete types. * **Tests** * Added a new test case to verify correct rewriting of queries involving interface objects and their concrete implementations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Checklist - [ ] I have discussed my proposed changes in an issue and have received approval to proceed. - [ ] I have followed the coding standards of the project. - [ ] Tests or benchmarks have been added or updated. <!-- Please add any additional information or context regarding your changes here. -->
1 parent a490fad commit 8c8c9de

File tree

2 files changed

+117
-4
lines changed

2 files changed

+117
-4
lines changed

v2/pkg/engine/plan/abstract_selection_rewriter_helpers.go

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -544,14 +544,14 @@ func (r *fieldSelectionRewriter) getAllowedInterfaceMemberTypeNames(fieldRef int
544544
return nil, false, errors.New("unexpected error: field type name is not found in the upstream schema")
545545
}
546546

547-
// if typename of a field is not equal to the typename of the interface type
547+
// when typename of a field is not equal to the typename of the interface type
548548
// then it should implement the interface type in the federated graph schema
549549
if interfaceTypeName != fieldTypeName {
550550
if slices.Contains(interfaceTypeNamesFromDefinition, fieldTypeName) {
551551
return []string{fieldTypeName}, false, nil
552552
}
553553

554-
// if it is not a member of the union type the config is corrupted
554+
// if it doesn't implement an interface type the config is corrupted
555555
return nil, false, errors.New("unexpected error: field type do not implement the interface in the federated graph schema")
556556
}
557557

@@ -560,10 +560,36 @@ func (r *fieldSelectionRewriter) getAllowedInterfaceMemberTypeNames(fieldRef int
560560
return nil, false, errors.New("unexpected error: interface type definition not found in the upstream schema")
561561
}
562562

563-
// in case node kind is an interface type definition we just return the implementing types in this datasource
563+
// when node kind is an interface type definition
564564
if interfaceNode.Kind == ast.NodeKindInterfaceTypeDefinition {
565-
interfaceTypeNames, _ := r.upstreamDefinition.InterfaceTypeDefinitionImplementedByObjectWithNames(interfaceNode.Ref)
565+
// we collect the implementing types in this datasource
566+
localInterfaceTypeNames, _ := r.upstreamDefinition.InterfaceTypeDefinitionImplementedByObjectWithNames(interfaceNode.Ref)
567+
568+
interfaceTypeNames := make([]string, 0, len(localInterfaceTypeNames))
569+
570+
// additionally, we need to check if implementing types are interface objects
571+
// and replace such typename with concrete types
572+
for _, typeName := range localInterfaceTypeNames {
573+
isInterfaceObject := false
574+
// when typeName is an interface object, we need to add concrete types instead of the interface object type name
575+
for _, k := range r.dsConfiguration.FederationConfiguration().InterfaceObjects {
576+
if k.InterfaceTypeName == typeName {
577+
interfaceTypeNames = append(interfaceTypeNames, k.ConcreteTypeNames...)
578+
isInterfaceObject = true
579+
break
580+
}
581+
}
582+
// when typename is not an interface object, we can add it as is
583+
if !isInterfaceObject {
584+
interfaceTypeNames = append(interfaceTypeNames, typeName)
585+
}
586+
}
587+
588+
// sort implementing types to be able to compact them
566589
sort.Strings(interfaceTypeNames)
590+
// remove possible consecutive duplicates
591+
interfaceTypeNames = slices.Compact(interfaceTypeNames)
592+
567593
return interfaceTypeNames, false, nil
568594
}
569595

v2/pkg/engine/plan/abstract_selection_rewriter_test.go

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3727,6 +3727,93 @@ func TestInterfaceSelectionRewriter_RewriteOperation(t *testing.T) {
37273727
}`,
37283728
shouldRewrite: false,
37293729
},
3730+
{
3731+
name: "field is an interface with concrete type fragments. one of implementing types is interface object",
3732+
definition: `
3733+
interface Named {
3734+
name: String!
3735+
}
3736+
3737+
type User implements Named {
3738+
id: ID!
3739+
name: String!
3740+
surname: String!
3741+
}
3742+
3743+
type Admin implements Account & Named {
3744+
id: ID!
3745+
name: String!
3746+
title: String!
3747+
}
3748+
3749+
interface Account implements Named {
3750+
id: ID!
3751+
name: String!
3752+
title: String!
3753+
}
3754+
3755+
type Query {
3756+
user: Named!
3757+
}`,
3758+
upstreamDefinition: `
3759+
interface Named {
3760+
name: String!
3761+
}
3762+
3763+
type User implements Named {
3764+
id: ID!
3765+
name: String!
3766+
surname: String!
3767+
}
3768+
3769+
type Account implements Named @interfaceObject @key(fields: "id") {
3770+
id: ID!
3771+
name: String!
3772+
}
3773+
3774+
type Query {
3775+
user: Named!
3776+
}`,
3777+
dsConfiguration: dsb().
3778+
RootNode("Account", "id", "title").
3779+
RootNode("User", "id", "name", "surname").
3780+
WithMetadata(func(m *FederationMetaData) {
3781+
m.InterfaceObjects = []EntityInterfaceConfiguration{
3782+
{
3783+
InterfaceTypeName: "Account",
3784+
ConcreteTypeNames: []string{"Admin", "User"},
3785+
},
3786+
}
3787+
}).
3788+
DS(),
3789+
fieldName: "user",
3790+
operation: `
3791+
query {
3792+
__typename
3793+
user {
3794+
id
3795+
... on Account {
3796+
... on Admin {
3797+
title
3798+
}
3799+
}
3800+
}
3801+
}`,
3802+
expectedOperation: `
3803+
query {
3804+
__typename
3805+
user {
3806+
... on Admin {
3807+
id
3808+
title
3809+
}
3810+
... on User {
3811+
id
3812+
}
3813+
}
3814+
}`,
3815+
shouldRewrite: true,
3816+
},
37303817
}
37313818

37323819
for _, testCase := range testCases {

0 commit comments

Comments
 (0)