diff --git a/docs/configuration.md b/docs/configuration.md index 4abd9100c..e32b6b604 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -760,13 +760,24 @@ public class BaseGraphType : EfInterfaceGraphType { public BaseGraphType(IEfGraphQLService graphQlService) : - base(graphQlService) => + base(graphQlService) + { AddNavigationConnectionField( name: "childrenFromInterface", projection: _ => _.ChildrenFromBase); + + // An interface field has no resolver, so nothing to declare a projection for. The + // implementing types declare what their resolvers read. + AddField( + new() + { + Name = "statusSummary", + Type = typeof(StringGraphType) + }); + } } ``` -snippet source | anchor +snippet source | anchor @@ -782,13 +793,16 @@ public class DerivedGraphType : name: "childrenFromInterface", projection: _ => _.ChildrenFromBase, resolve: _ => _.Projection); + Field("statusSummary") + .Resolve(_ => $"Status is {_.Source.Status}") + .WithProjection(_ => _.Status); AutoMap(); Interface(); IsTypeOf = obj => obj is DerivedEntity; } } ``` -snippet source | anchor +snippet source | anchor diff --git a/src/Directory.Build.props b/src/Directory.Build.props index bf315647c..b95b7c3ac 100644 --- a/src/Directory.Build.props +++ b/src/Directory.Build.props @@ -2,7 +2,7 @@ CS1591;NU5104;CS1573;CS9107;NU1608;NU1109 - 35.5.0 + 35.5.1 preview 1.0.0 EntityFrameworkCore, EntityFramework, GraphQL diff --git a/src/GraphQL.EntityFramework/IncludeAppender.cs b/src/GraphQL.EntityFramework/IncludeAppender.cs index 7dc448d54..ede44d3d4 100644 --- a/src/GraphQL.EntityFramework/IncludeAppender.cs +++ b/src/GraphQL.EntityFramework/IncludeAppender.cs @@ -301,7 +301,8 @@ FieldProjectionInfo GetProjectionInfo( GetComplexGraphType(context.FieldDefinition), entityType, scalarFields, - context); + context, + root: true); return new(scalarFields, keys, foreignKeyNames, navProjections, derivedNavigations); } @@ -341,6 +342,12 @@ void ProcessField( return; } + if (parentGraphType is IInterfaceGraphType interfaceType && + ProcessImplementingFields(field, interfaceType, navigationProperties, scalarFields, navProjections, context)) + { + return; + } + // A query field runs a query of its own and reads nothing from its source but the keys, // which are always projected. Matched by name it was projected as the navigation it is // named after, so the parent query joined a collection that the field then queried again. @@ -353,6 +360,54 @@ void ProcessField( ProcessNavigationOrScalar(fieldName, field, fieldType, navigationProperties, scalarFields, navProjections, context); } + /// + /// A field selected on an interface is resolved by the field of whichever implementing type + /// the row turns out to be, so it is those fields that say what has to be loaded. The field of + /// the interface has no resolver, and is usually declared with no projection. Only it was + /// looked at, so a resolver's projection was applied when its field was selected through a + /// fragment on its own type, and not when selected on the interface. That went unnoticed + /// while an abstract type was loaded whole. + /// True when the implementing types account for the field: each declares a projection, or + /// runs a query of its own. Otherwise the field is also matched by name, as before. + /// + bool ProcessImplementingFields( + GraphQLField field, + IInterfaceGraphType interfaceType, + IReadOnlyDictionary? navigationProperties, + HashSet scalarFields, + Dictionary navProjections, + IResolveFieldContext context) + { + var accounted = false; + var byName = false; + + foreach (var implementingType in interfaceType.PossibleTypes) + { + var implementingField = implementingType.GetField(field.Name.Value); + if (implementingField is null) + { + continue; + } + + if (TryGetProjectionMetadata(implementingField, out var projection)) + { + ProcessProjectionExpression(field, implementingField, implementingType, projection, navigationProperties, scalarFields, navProjections, context); + accounted = true; + continue; + } + + if (IsQueryField(implementingField)) + { + accounted = true; + continue; + } + + byName = true; + } + + return accounted && !byName; + } + void ProcessSelectionSet( GraphQLSelectionSet? selectionSet, IComplexGraphType? graphType, @@ -394,8 +449,10 @@ void ProcessSelectionSet( IComplexGraphType? graphType, Type entityType, HashSet scalarFields, - IResolveFieldContext context) + IResolveFieldContext context, + bool root = false) { + var rootSelectionSet = selectionSet; IComplexGraphType? leafGraphType; (selectionSet, leafGraphType) = GetLeafSelection(selectionSet, graphType); if (selectionSet?.Selections is null || @@ -404,13 +461,24 @@ void ProcessSelectionSet( return null; } + // The root sub fields exclude fields conditional on another type, so at the root this is + // the only place a field in a fragment on a derived type is seen, and its projection has + // to be read here. It was not, so such a field was matched by name alone. Below the root, + // and under the items of a root connection, the selection set walk has already read it. + var readProjections = root && ReferenceEquals(selectionSet, rootSelectionSet); + Dictionary>? result = null; foreach (var (field, fieldGraphType) in EnumerateFields(selectionSet, leafGraphType, context)) { if (fieldGraphType is null || - ReferenceEquals(fieldGraphType, leafGraphType) || - fieldGraphType.GetField(field.Name.Value) is { } derivedField && IsQueryField(derivedField) || + ReferenceEquals(fieldGraphType, leafGraphType)) + { + continue; + } + + var derivedField = fieldGraphType.GetField(field.Name.Value); + if (derivedField is not null && IsQueryField(derivedField) || !TryFindDerivedClrType(fieldGraphType, out var derivedType) || derivedType == entityType || !entityType.IsAssignableFrom(derivedType)) @@ -419,6 +487,22 @@ void ProcessSelectionSet( } navigations.TryGetValue(derivedType, out var derivedNavProps); + + if (readProjections && + derivedField is not null && + TryGetProjectionMetadata(derivedField, out var projection)) + { + result ??= []; + if (!result.TryGetValue(derivedType, out var projected)) + { + projected = []; + result[derivedType] = projected; + } + + ProcessProjectionExpression(field, derivedField, fieldGraphType, projection, derivedNavProps, scalarFields, projected, context); + continue; + } + if (derivedNavProps is null || !derivedNavProps.TryGetValue(field.Name.StringValue, out var navigation)) { diff --git a/src/GraphQL.EntityFramework/SelectProjection/NavigationProjectionInfo.cs b/src/GraphQL.EntityFramework/SelectProjection/NavigationProjectionInfo.cs index 03d4bd357..dcb270898 100644 --- a/src/GraphQL.EntityFramework/SelectProjection/NavigationProjectionInfo.cs +++ b/src/GraphQL.EntityFramework/SelectProjection/NavigationProjectionInfo.cs @@ -10,7 +10,8 @@ /// /// The field's ids, where and orderBy, to apply inside the collection subquery. Null when the /// navigation was selected more than once, since one loaded collection cannot satisfy two sets -/// of arguments; the field's resolver then applies them in memory. +/// of arguments; the field's resolver then applies them in memory. One selection read through +/// the field of each type implementing an interface is still one selection, and keeps them. /// record NavigationProjectionInfo( Type EntityType, @@ -20,11 +21,16 @@ record NavigationProjectionInfo( NavigationArguments? Arguments = null, bool IsRequired = false) { - public NavigationProjectionInfo Merge(NavigationProjectionInfo other) => - this with + public NavigationProjectionInfo Merge(NavigationProjectionInfo other) + { + var sameSelection = Arguments is not null && + other.Arguments is not null && + ReferenceEquals(Arguments.Field, other.Arguments.Field); + return this with { Projection = Projection.Merge(other.Projection), IsWhole = IsWhole || other.IsWhole, - Arguments = null + Arguments = sameSelection ? Arguments : null }; + } } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/Graphs/Inheritance/BaseGraphType.cs b/src/Tests/IntegrationTests/Graphs/Inheritance/BaseGraphType.cs index 9cdc84df4..a2be8a10c 100644 --- a/src/Tests/IntegrationTests/Graphs/Inheritance/BaseGraphType.cs +++ b/src/Tests/IntegrationTests/Graphs/Inheritance/BaseGraphType.cs @@ -2,8 +2,19 @@ EfInterfaceGraphType { public BaseGraphType(IEfGraphQLService graphQlService) : - base(graphQlService) => + base(graphQlService) + { AddNavigationConnectionField( name: "childrenFromInterface", projection: _ => _.ChildrenFromBase); + + // An interface field has no resolver, so nothing to declare a projection for. The + // implementing types declare what their resolvers read. + AddField( + new() + { + Name = "statusSummary", + Type = typeof(StringGraphType) + }); + } } \ No newline at end of file diff --git a/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedGraphType.cs b/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedGraphType.cs index 03f6ea695..0283b1820 100644 --- a/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedGraphType.cs +++ b/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedGraphType.cs @@ -8,6 +8,9 @@ public DerivedGraphType(IEfGraphQLService graphQlService) name: "childrenFromInterface", projection: _ => _.ChildrenFromBase, resolve: _ => _.Projection); + Field("statusSummary") + .Resolve(_ => $"Status is {_.Source.Status}") + .WithProjection(_ => _.Status); AutoMap(); Interface(); IsTypeOf = obj => obj is DerivedEntity; diff --git a/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedWithNavigationGraphType.cs b/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedWithNavigationGraphType.cs index 7863a6448..6f14b748a 100644 --- a/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedWithNavigationGraphType.cs +++ b/src/Tests/IntegrationTests/Graphs/Inheritance/DerivedWithNavigationGraphType.cs @@ -12,6 +12,9 @@ public DerivedWithNavigationGraphType(IEfGraphQLService gr name: "childrenFromDerived", projection: _ => _.Children, resolve: _ => _.Projection); + Field("statusSummary") + .Resolve(_ => $"Status is {_.Source.Status}") + .WithProjection(_ => _.Status); AutoMap(); Interface(); IsTypeOf = obj => obj is DerivedWithNavigationEntity; diff --git a/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_connection_uses_projection_of_implementing_types.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_connection_uses_projection_of_implementing_types.verified.txt new file mode 100644 index 000000000..251b7cb68 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_connection_uses_projection_of_implementing_types.verified.txt @@ -0,0 +1,32 @@ +{ + target: { + Data: { + interfaceGraphConnection: { + items: [ + { + property: Derived, + statusSummary: Status is Approved + }, + { + property: WithNavigation, + statusSummary: Status is Approved + } + ] + } + } + }, + sql: { + Text: +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Property, + b.Status, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end +from BaseEntities as b +order by b.Property +offset @p rows, + Parameters: { + @p: 0 + } + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_root_fragment_uses_its_projection.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_root_fragment_uses_its_projection.verified.txt new file mode 100644 index 000000000..fccfffacd --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_in_root_fragment_uses_its_projection.verified.txt @@ -0,0 +1,26 @@ +{ + target: { + Data: { + baseEntities: [ + { + property: Derived, + statusSummary: Status is Approved + }, + { + property: WithNavigation, + statusSummary: Status is Approved + } + ] + } + }, + sql: { + Text: +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Property, + b.Status, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end +from BaseEntities as b +order by b.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Interface_field_under_navigation_uses_projection_of_implementing_types.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_under_navigation_uses_projection_of_implementing_types.verified.txt new file mode 100644 index 000000000..77cd33434 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_under_navigation_uses_projection_of_implementing_types.verified.txt @@ -0,0 +1,33 @@ +{ + target: { + Data: { + derivedChildEntities: [ + { + property: Child, + parent: { + statusSummary: Status is Approved + } + } + ] + } + }, + sql: { + Text: +select d.Id, + case when b.Id is null then cast (1 as bit) else cast (0 as bit) end, + case when b.Discriminator = N'DerivedWithNavigationEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Status, + case when b.Discriminator = N'DerivedEntity' + and b.Discriminator is not null then cast (1 as bit) else cast (0 as bit) end, + d.ParentId, + d.Property, + d.TypedParentId +from DerivedChildEntities as d + left outer join + BaseEntities as b + on d.ParentId = b.Id +order by d.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.Interface_field_uses_projection_of_implementing_types.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_uses_projection_of_implementing_types.verified.txt new file mode 100644 index 000000000..fccfffacd --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests.Interface_field_uses_projection_of_implementing_types.verified.txt @@ -0,0 +1,26 @@ +{ + target: { + Data: { + baseEntities: [ + { + property: Derived, + statusSummary: Status is Approved + }, + { + property: WithNavigation, + statusSummary: Status is Approved + } + ] + } + }, + sql: { + Text: +select case when b.Discriminator = N'DerivedWithNavigationEntity' then cast (1 as bit) else cast (0 as bit) end, + b.Id, + b.Property, + b.Status, + case when b.Discriminator = N'DerivedEntity' then cast (1 as bit) else cast (0 as bit) end +from BaseEntities as b +order by b.Property + } +} \ No newline at end of file diff --git a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt index a8fc819df..716ba229b 100644 --- a/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt +++ b/src/Tests/IntegrationTests/IntegrationTests.SchemaPrint.verified.txt @@ -1200,6 +1200,7 @@ interface BaseEntity { where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], ids: [ID!]): DerivedChildConnection! + statusSummary: String } type DerivedChild { @@ -1306,6 +1307,7 @@ type Derived implements BaseEntity { where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], ids: [ID!]): DerivedChildConnection! + statusSummary: String childrenFromBase(id: ID, ids: [ID!], where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], skip: Int, take: Int): [DerivedChild!]! id: ID! property: String @@ -1982,6 +1984,7 @@ type DerivedWithNavigation implements BaseEntity { where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], ids: [ID!]): DerivedChildConnection! + statusSummary: String childrenFromBase(id: ID, ids: [ID!], where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], skip: Int, take: Int): [DerivedChild!]! children(id: ID, ids: [ID!], where: DerivedChildEntityWhere, orderBy: [DerivedChildEntityOrderBy!], skip: Int, take: Int): [DerivedChild!]! id: ID! diff --git a/src/Tests/IntegrationTests/IntegrationTests_interface_field_projection.cs b/src/Tests/IntegrationTests/IntegrationTests_interface_field_projection.cs new file mode 100644 index 000000000..039a478a1 --- /dev/null +++ b/src/Tests/IntegrationTests/IntegrationTests_interface_field_projection.cs @@ -0,0 +1,118 @@ +public partial class IntegrationTests +{ + // statusSummary is declared on the interface with no projection, and on each implementing type + // with a resolver that reads Status and a projection for it. Status defaults to Draft, so the + // summary only says Approved when the projection of the implementing type's field was loaded. + // Selected on the interface it was not: the field of the interface carries no projection, and + // only that field was looked at. + + [Fact] + public async Task Interface_field_uses_projection_of_implementing_types() + { + var query = + """ + { + baseEntities(orderBy: {property: ascending}) + { + property + statusSummary + } + } + """; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, InterfaceProjectionEntities()); + } + + // A fragment at the root of an interface typed query is not among the sub fields, so its + // fields were recorded by name alone and their projections never read. + [Fact] + public async Task Interface_field_in_root_fragment_uses_its_projection() + { + var query = + """ + { + baseEntities(orderBy: {property: ascending}) + { + property + ... on Derived + { + statusSummary + } + ... on DerivedWithNavigation + { + statusSummary + } + } + } + """; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, InterfaceProjectionEntities()); + } + + [Fact] + public async Task Interface_field_in_connection_uses_projection_of_implementing_types() + { + var query = + """ + { + interfaceGraphConnection + { + items + { + property + statusSummary + } + } + } + """; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, InterfaceProjectionEntities()); + } + + [Fact] + public async Task Interface_field_under_navigation_uses_projection_of_implementing_types() + { + var query = + """ + { + derivedChildEntities + { + property + parent + { + statusSummary + } + } + } + """; + + await using var database = await sqlInstance.Build(); + await RunQuery(database, query, null, null, false, InterfaceProjectionEntities()); + } + + static object[] InterfaceProjectionEntities() + { + var derived = new DerivedEntity + { + Id = new("00000000-0000-0000-0000-000000000001"), + Property = "Derived", + Status = "Approved" + }; + var withNavigation = new DerivedWithNavigationEntity + { + Id = new("00000000-0000-0000-0000-000000000002"), + Property = "WithNavigation", + Status = "Approved" + }; + var child = new DerivedChildEntity + { + Id = new("00000000-0000-0000-0000-000000000003"), + Property = "Child", + Parent = derived + }; + return [derived, withNavigation, child]; + } +}