Skip to content

Commit

Permalink
Don't check for a convention when not one isn't needed
Browse files Browse the repository at this point in the history
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 ChilliCream#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 ChilliCream#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 ChilliCream#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 ChilliCream#6975.
Jimit routes around the convention problem by not using a convention at all
in his custom middleware. A close examination of `UseFilteringInternal` at
ChilliCream#6975 (reply in thread)
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.
  • Loading branch information
SeanTAllen committed Mar 26, 2024
1 parent 7000436 commit c762100
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ public static class FilterObjectFieldDescriptorExtensions
.OnBeforeCreate(
(c, definition) =>
{
var convention = c.GetFilterConvention(scope);
TypeReference argumentTypeReference;
if (filterTypeInstance is not null)
Expand All @@ -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))
Expand Down Expand Up @@ -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<TEntity>(IFilterConvention convention) =>
convention.CreateBuilder<TEntity>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ public static class SortObjectFieldDescriptorExtensions
.OnBeforeCreate(
(c, definition) =>
{
var convention = c.GetSortConvention(scope);
TypeReference argumentTypeReference;
if (sortTypeInstance is not null)
{
Expand All @@ -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))
Expand Down Expand Up @@ -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<TEntity>(
ISortConvention convention) =>
convention.CreateBuilder<TEntity>();
}
}

0 comments on commit c762100

Please sign in to comment.