From 07a655bccb8a384bf646649636ba1f8552a3a826 Mon Sep 17 00:00:00 2001 From: Georg von Kries Date: Fri, 7 Jun 2024 18:03:19 +0200 Subject: [PATCH] Fixes several minor issues when building the GraphQL schema (#16151) Co-authored-by: Hisham Bin Ateya --- .../Extensions/FieldTypeExtensions.cs | 23 ------ .../Extensions/GraphQLTypeExtensions.cs | 40 ++++++++++ .../Types/DynamicContentTypeBuilder.cs | 76 ++++++++++++------- .../Queries/Types/TypedContentTypeBuilder.cs | 13 ++-- src/docs/releases/2.0.0.md | 4 + 5 files changed, 100 insertions(+), 56 deletions(-) delete mode 100644 src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/FieldTypeExtensions.cs create mode 100644 src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/GraphQLTypeExtensions.cs diff --git a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/FieldTypeExtensions.cs b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/FieldTypeExtensions.cs deleted file mode 100644 index 6bd87f3ec3d..00000000000 --- a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/FieldTypeExtensions.cs +++ /dev/null @@ -1,23 +0,0 @@ -using GraphQL.Types; - -namespace OrchardCore.ContentManagement.GraphQL; - -public static class FieldTypeExtensions -{ - public static FieldType WithPartCollapsedMetaData(this FieldType fieldType, bool collapsed = true) - => fieldType.WithMetaData("PartCollapsed", collapsed); - - public static FieldType WithPartNameMetaData(this FieldType fieldType, string partName) - => fieldType.WithMetaData("PartName", partName); - - private static FieldType WithMetaData(this FieldType fieldType, string name, object value) - { - // TODO: Understand if locking is the best solution to https://github.com/OrchardCMS/OrchardCore/issues/15308 - lock (fieldType.Metadata) - { - fieldType.Metadata.TryAdd(name, value); - } - - return fieldType; - } -} diff --git a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/GraphQLTypeExtensions.cs b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/GraphQLTypeExtensions.cs new file mode 100644 index 00000000000..0c51e230b3e --- /dev/null +++ b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Extensions/GraphQLTypeExtensions.cs @@ -0,0 +1,40 @@ +using System; +using System.Linq; +using GraphQL.Types; + +namespace OrchardCore.ContentManagement.GraphQL; + +public static class GraphQLTypeExtensions +{ + public static FieldType WithPartCollapsedMetaData(this FieldType fieldType, bool collapsed = true) + => fieldType.WithMetaData("PartCollapsed", collapsed); + + public static FieldType WithPartNameMetaData(this FieldType fieldType, string partName) + => fieldType.WithMetaData("PartName", partName); + + /// + /// Checks if the field exists in the GraphQL type in a case-insensitive way. + /// + /// + /// + /// This is the same as calling but in a case-insensitive way. OC + /// fields may be added with different casings, and we want to avoid collisions even then. + /// + /// + /// See and its corresponding issues for context. + /// + /// + public static bool HasFieldIgnoreCase(this IComplexGraphType graphType, string fieldName) + => graphType.Fields.Any(field => field.Name.Equals(fieldName, StringComparison.OrdinalIgnoreCase)); + + private static FieldType WithMetaData(this FieldType fieldType, string name, object value) + { + // TODO: Understand if locking is the best solution to https://github.com/OrchardCMS/OrchardCore/issues/15308 + lock (fieldType.Metadata) + { + fieldType.Metadata.TryAdd(name, value); + } + + return fieldType; + } +} diff --git a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs index 3dd4ecb7450..61ce13d7121 100644 --- a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs +++ b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/DynamicContentTypeBuilder.cs @@ -1,3 +1,4 @@ +using System; using System.Collections.Generic; using System.Linq; using GraphQL.Types; @@ -70,7 +71,8 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition if (fieldType != null) { - if (_contentOptions.ShouldSkip(fieldType.Type, fieldType.Name)) + if (_contentOptions.ShouldSkip(fieldType.Type, fieldType.Name) || + contentItemType.HasFieldIgnoreCase(fieldType.Name)) { continue; } @@ -84,45 +86,63 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition else { // Check if another builder has already added a field for this part. - var existingField = contentItemType.GetField(partName.ToFieldName()); - if (existingField != null) + var partFieldName = partName.ToFieldName(); + var partFieldType = contentItemType.GetField(partFieldName); + + if (partFieldType != null) { - // Add content field types. - foreach (var field in part.PartDefinition.Fields) + // Add dynamic content field types to the static part type. + var partContentItemType = schema.AdditionalTypeInstances + .OfType() + .Where(type => type.GetType() == partFieldType.Type) + .FirstOrDefault(); + + if (partContentItemType != null) { - foreach (var fieldProvider in contentFieldProviders) + foreach (var field in part.PartDefinition.Fields) { - var contentFieldType = fieldProvider.GetField(schema, field, part.Name); - - if (contentFieldType != null && !contentItemType.HasField(contentFieldType.Name)) + foreach (var fieldProvider in contentFieldProviders) { - contentItemType.AddField(contentFieldType); - break; + var contentFieldType = fieldProvider.GetField(schema, field, part.Name); + + if (contentFieldType != null) + { + if (_contentOptions.ShouldSkip(contentFieldType.Type, contentFieldType.Name) || + partContentItemType.HasFieldIgnoreCase(contentFieldType.Name)) + { + continue; + } + + + partContentItemType.AddField(contentFieldType); + break; + } } } } - continue; - } - - if (_dynamicPartFields.TryGetValue(partName, out var fieldType)) - { - contentItemType.AddField(fieldType); } else { - var field = contentItemType - .Field(partName.ToFieldName()) - .Description(S["Represents a {0}.", part.PartDefinition.Name]) - .Resolve(context => - { - var nameToResolve = partName; - var typeToResolve = context.FieldDefinition.ResolvedType.GetType().BaseType.GetGenericArguments().First(); + if (_dynamicPartFields.TryGetValue(partName, out var fieldType)) + { + contentItemType.AddField(fieldType); + } + else + { + var field = contentItemType + .Field(partFieldName) + .Description(S["Represents a {0}.", part.PartDefinition.Name]) + .Resolve(context => + { + var nameToResolve = partName; + var typeToResolve = context.FieldDefinition.ResolvedType.GetType().BaseType.GetGenericArguments().First(); - return context.Source.Get(typeToResolve, nameToResolve); - }); + return context.Source.Get(typeToResolve, nameToResolve); + }); - field.Type(new DynamicPartGraphType(part)); - _dynamicPartFields[partName] = field.FieldType; + field.Type(new DynamicPartGraphType(part)); + _dynamicPartFields[partName] = field.FieldType; + } } } } diff --git a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/TypedContentTypeBuilder.cs b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/TypedContentTypeBuilder.cs index 58d239c2f40..0db71acc602 100644 --- a/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/TypedContentTypeBuilder.cs +++ b/src/OrchardCore/OrchardCore.ContentManagement.GraphQL/Queries/Types/TypedContentTypeBuilder.cs @@ -1,3 +1,4 @@ +using System; using System.Linq; using GraphQL; using GraphQL.Resolvers; @@ -40,9 +41,10 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition } var partName = part.Name; + var partFieldName = partName.ToFieldName(); // Check if another builder has already added a field for this part. - if (contentItemType.HasField(partName)) + if (contentItemType.HasField(partFieldName)) { continue; } @@ -58,7 +60,8 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition { foreach (var field in queryGraphType.Fields) { - if (_contentOptions.ShouldSkip(queryGraphType.GetType(), field.Name)) + if (_contentOptions.ShouldSkip(queryGraphType.GetType(), field.Name) || + contentItemType.HasFieldIgnoreCase(field.Name)) { continue; } @@ -94,11 +97,11 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition { var field = new FieldType { - Name = partName.ToFieldName(), + Name = partFieldName, Type = queryGraphType.GetType(), Description = queryGraphType.Description, }; - contentItemType.Field(partName.ToFieldName(), queryGraphType.GetType()) + contentItemType.Field(partFieldName, queryGraphType.GetType()) .Description(queryGraphType.Description) .Resolve(context => { @@ -135,7 +138,7 @@ public void Build(ISchema schema, FieldType contentQuery, ContentTypeDefinition whereInput.AddField(new FieldType { Type = inputGraphTypeResolved.GetType(), - Name = partName.ToFieldName(), + Name = partFieldName, Description = inputGraphTypeResolved.Description }.WithPartNameMetaData(partName)); } diff --git a/src/docs/releases/2.0.0.md b/src/docs/releases/2.0.0.md index 2894c957ca5..6f971f317d7 100644 --- a/src/docs/releases/2.0.0.md +++ b/src/docs/releases/2.0.0.md @@ -246,6 +246,10 @@ Here are the updated signatures: These adjustments ensure compatibility and adherence to the latest conventions within the `SectionDisplayDriver` class. +The GraphQL schema may change because fields are now always added to the correct part. Previously, additional fields may have been added to the parent content item type directly. + +You may have to adjust your GraphQL queries in that case. + ## Change Logs ### Azure AI Search Module