From c762100fa3bdb84c0304bde6894309d526c3f3dc Mon Sep 17 00:00:00 2001 From: "Sean T. Allen" Date: Tue, 26 Mar 2024 09:45:54 -0400 Subject: [PATCH] Don't check for a convention when not one isn't needed When setting up filtering and sorting with UseFiltering and UseSorting, there are 3 different code paths that are traversed. Prior to this change, all 3 of them required that a convention be registered otherwise an exception would occur. However, for both filtering and sorting, only 1 of the code paths actually needs a convention. The other two do not use the convention at all. This commit changes the filtering and sorting object field descriptor extensions to attempt to get a convention only within the block of code that requires a convention. By making this small change, the other two code paths that do not require a convention will no longer encounter an exception. Instead, filtering and sorting can be set up as expected. This commit partially addresses #6983. We are able locally, with these changes applied, to use filtering and sorting from a type module by calling the versions of `UseFiltering` and `UseSorting` that take a type. Without the changes in this commit, we get the "no convention registered" error. This change does not close #6983 as it is still possible to encounter the "no convention registered" exception and its suggested remedy of "Call `AddFiltering()` on the schema builder" doesn't work. From what we can see, and have detailed in #6983, no conventions registered with the schema builder either directly or the defaults provided by `AddFiltering` and `AddSorting` are available at the time that a type module is run. The result of this commit is to allow the usage `UseFiltering` and `UseSorting` with types created via a type module. The other option we are aware of is detailed by @jimitndiaye in https://github.com/ChilliCream/graphql-platform/discussions/6975. Jimit routes around the convention problem by not using a convention at all in his custom middleware. A close examination of `UseFilteringInternal` at https://github.com/ChilliCream/graphql-platform/discussions/6975#discussioncomment-8778742 will show this lack of convention usage as key to his fix. Instead of going the custom middleware route, we made these two small changes to Hot Chocolate. We prefer this approach. --- .../Extensions/FilterObjectFieldDescriptorExtensions.cs | 9 +++++---- .../Extensions/SortObjectFieldDescriptorExtensions.cs | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/HotChocolate/Data/src/Data/Filters/Extensions/FilterObjectFieldDescriptorExtensions.cs b/src/HotChocolate/Data/src/Data/Filters/Extensions/FilterObjectFieldDescriptorExtensions.cs index d7d8537a278..9001af27ddc 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Extensions/FilterObjectFieldDescriptorExtensions.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Extensions/FilterObjectFieldDescriptorExtensions.cs @@ -138,7 +138,6 @@ public static class FilterObjectFieldDescriptorExtensions .OnBeforeCreate( (c, definition) => { - var convention = c.GetFilterConvention(scope); TypeReference argumentTypeReference; if (filterTypeInstance is not null) @@ -147,6 +146,8 @@ public static class FilterObjectFieldDescriptorExtensions } else if (filterType is null) { + var convention = c.GetFilterConvention(scope); + if (definition.ResultType is null || definition.ResultType == typeof(object) || !c.TypeInspector.TryCreateTypeInfo(definition.ResultType, out var typeInfo)) @@ -215,11 +216,11 @@ public static class FilterObjectFieldDescriptorExtensions var factory = _factoryTemplate.MakeGenericMethod(type.EntityType.Source); var middleware = CreateDataMiddleware((IQueryBuilder)factory.Invoke(null, [convention,])!); - + var index = definition.MiddlewareDefinitions.IndexOf(placeholder); definition.MiddlewareDefinitions[index] = new(middleware, key: WellKnownMiddleware.Filtering); } - + private static IQueryBuilder CreateMiddleware(IFilterConvention convention) => convention.CreateBuilder(); -} \ No newline at end of file +} diff --git a/src/HotChocolate/Data/src/Data/Sorting/Extensions/SortObjectFieldDescriptorExtensions.cs b/src/HotChocolate/Data/src/Data/Sorting/Extensions/SortObjectFieldDescriptorExtensions.cs index c57d4701999..b11a68bb5b9 100644 --- a/src/HotChocolate/Data/src/Data/Sorting/Extensions/SortObjectFieldDescriptorExtensions.cs +++ b/src/HotChocolate/Data/src/Data/Sorting/Extensions/SortObjectFieldDescriptorExtensions.cs @@ -141,7 +141,6 @@ public static class SortObjectFieldDescriptorExtensions .OnBeforeCreate( (c, definition) => { - var convention = c.GetSortConvention(scope); TypeReference argumentTypeReference; if (sortTypeInstance is not null) { @@ -150,6 +149,8 @@ public static class SortObjectFieldDescriptorExtensions } else if (sortType is null) { + var convention = c.GetSortConvention(scope); + if (definition.ResultType is null || definition.ResultType == typeof(object) || !c.TypeInspector.TryCreateTypeInfo(definition.ResultType, out var typeInfo)) @@ -234,12 +235,12 @@ public static class SortObjectFieldDescriptorExtensions var factory = _factoryTemplate.MakeGenericMethod(type.EntityType.Source); var middleware = CreateDataMiddleware((IQueryBuilder)factory.Invoke(null, [convention,])!); - + var index = definition.MiddlewareDefinitions.IndexOf(placeholder); definition.MiddlewareDefinitions[index] = new(middleware, key: WellKnownMiddleware.Sorting); } - + private static IQueryBuilder CreateMiddleware( ISortConvention convention) => convention.CreateBuilder(); -} \ No newline at end of file +}