diff --git a/src/HotChocolate/Core/src/Types/Internal/CustomParameterExpressionBuilder.cs b/src/HotChocolate/Core/src/Types/Internal/CustomParameterExpressionBuilder.cs index d77759ea1e3..bccb73b325d 100644 --- a/src/HotChocolate/Core/src/Types/Internal/CustomParameterExpressionBuilder.cs +++ b/src/HotChocolate/Core/src/Types/Internal/CustomParameterExpressionBuilder.cs @@ -1,5 +1,7 @@ +using System.Diagnostics; using System.Linq.Expressions; using System.Reflection; +using System.Runtime.CompilerServices; using HotChocolate.Resolvers; namespace HotChocolate.Internal; @@ -8,9 +10,11 @@ namespace HotChocolate.Internal; /// A custom parameter expression builder allows to implement custom resolver parameter /// injection logic. /// -public abstract class CustomParameterExpressionBuilder : IParameterExpressionBuilder +public abstract class CustomParameterExpressionBuilder + : IParameterExpressionBuilder + , IParameterBindingFactory { - private readonly bool _isPure; + private protected readonly bool _isPure; /// /// Initializes a new instance of @@ -36,6 +40,12 @@ internal CustomParameterExpressionBuilder(bool isPure) bool IParameterExpressionBuilder.IsDefaultHandler => false; + ArgumentKind IParameterBindingFactory.Kind => ArgumentKind.Custom; + + bool IParameterBindingFactory.IsPure => _isPure; + + bool IParameterBindingFactory.IsDefaultHandler => false; + /// /// Checks if this expression builder can handle the following parameter. /// @@ -48,6 +58,25 @@ internal CustomParameterExpressionBuilder(bool isPure) /// public abstract bool CanHandle(ParameterInfo parameter); + /// + /// Checks if this expression builder can handle the parameter described by the + /// source generator (which has no ). + /// + public virtual bool CanHandle(ParameterDescriptor parameter) => false; + + /// + /// Creates the runtime binding used by source-generated resolvers. + /// + public virtual IParameterBinding Create(ParameterDescriptor parameter) + => throw new NotSupportedException(); + + /// + /// Returns true when this builder targets the described parameter but can only + /// decide via a predicate, which the source generator cannot + /// evaluate. Used to raise a clear error instead of silently misclassifying the parameter. + /// + internal virtual bool RequiresParameterInfo(ParameterDescriptor parameter) => false; + /// /// Builds an expression that resolves a resolver parameter. /// @@ -69,6 +98,8 @@ public class CustomParameterExpressionBuilder : CustomParameterExpressionB { private readonly Func _canHandle; private readonly Expression> _expression; + private readonly bool _usesDefaultTypePredicate; + private Func? _compiled; /// /// Initializes a new instance of . @@ -82,6 +113,7 @@ public CustomParameterExpressionBuilder( { _canHandle = p => p.ParameterType == typeof(TArg); _expression = expression; + _usesDefaultTypePredicate = true; } /// @@ -100,6 +132,7 @@ internal CustomParameterExpressionBuilder( { _canHandle = p => p.ParameterType == typeof(TArg); _expression = expression; + _usesDefaultTypePredicate = true; } /// @@ -118,6 +151,7 @@ public CustomParameterExpressionBuilder( { _expression = expression; _canHandle = canHandle; + _usesDefaultTypePredicate = false; } /// @@ -140,6 +174,7 @@ internal CustomParameterExpressionBuilder( { _expression = expression; _canHandle = canHandle; + _usesDefaultTypePredicate = false; } /// @@ -167,4 +202,33 @@ public override bool CanHandle(ParameterInfo parameter) /// public override Expression Build(ParameterExpressionBuilderContext context) => Expression.Invoke(_expression, context.ResolverContext); + + public override bool CanHandle(ParameterDescriptor parameter) + => _usesDefaultTypePredicate && parameter.Type == typeof(TArg); + + internal override bool RequiresParameterInfo(ParameterDescriptor parameter) + // A custom ParameterInfo predicate can match any parameter a TArg value is assignable to + // (e.g. an interface or base type), not just an exact-TArg parameter. The source generator + // cannot evaluate the predicate, so flag those parameters to fail loudly rather than + // silently fall through to argument binding. + => !_usesDefaultTypePredicate && parameter.Type.IsAssignableFrom(typeof(TArg)); + + public override IParameterBinding Create(ParameterDescriptor parameter) + => new CustomBinding(_compiled ??= _expression.Compile(), _isPure); + + private sealed class CustomBinding( + Func compiled, + bool isPure) + : IParameterBinding + { + public bool IsPure => isPure; + + public T Execute(IResolverContext context) + { + // the binding is only created for a parameter of type TArg, so T == TArg here + Debug.Assert(typeof(T) == typeof(TArg)); + var value = compiled(context); + return Unsafe.As(ref value); + } + } } diff --git a/src/HotChocolate/Core/src/Types/Resolvers/ParameterBindingResolver.cs b/src/HotChocolate/Core/src/Types/Resolvers/ParameterBindingResolver.cs index e4b3eafd3fa..e54e02eed3b 100644 --- a/src/HotChocolate/Core/src/Types/Resolvers/ParameterBindingResolver.cs +++ b/src/HotChocolate/Core/src/Types/Resolvers/ParameterBindingResolver.cs @@ -72,6 +72,7 @@ public IParameterBinding GetBinding(ParameterDescriptor parameter) } } + EnsureParameterInfoNotRequired(parameter); return _defaultBinding.Create(parameter); } @@ -85,6 +86,28 @@ public IParameterBinding GetBinding(ParameterDescriptor parameter) } } + EnsureParameterInfoNotRequired(parameter); return (_defaultBinding.Kind, _defaultBinding.IsPure); } + + private void EnsureParameterInfoNotRequired(ParameterDescriptor parameter) + { + foreach (var binding in _bindings) + { + if (binding is CustomParameterExpressionBuilder customBuilder + && customBuilder.RequiresParameterInfo(parameter)) + { + throw new SchemaException( + SchemaErrorBuilder.New() + .SetMessage( + $"The parameter '{parameter.Name}' of type " + + $"'{parameter.Type.FullName ?? parameter.Type.Name}' is handled by a " + + "custom parameter expression builder that was registered with a " + + "ParameterInfo predicate, which is not supported by the source " + + "generator. Register it using the type-based " + + $"AddParameterExpressionBuilder<{parameter.Type.Name}>(...) overload.") + .Build()); + } + } + } } diff --git a/src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorOffsetPagingReproTests.cs b/src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorReproTests.cs similarity index 79% rename from src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorOffsetPagingReproTests.cs rename to src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorReproTests.cs index 676d9838af5..e945bee3810 100644 --- a/src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorOffsetPagingReproTests.cs +++ b/src/HotChocolate/Core/test/Types.Analyzers.Tests/SourceGeneratorReproTests.cs @@ -19,7 +19,7 @@ namespace HotChocolate.Types; -public class SourceGeneratorOffsetPagingReproTests +public class SourceGeneratorReproTests { [Fact] public async Task QueryType_SourceGenerator_Path_Works_Like_AddQueryType_Path() @@ -312,6 +312,123 @@ private static MethodInfo FindRegistrationMethod( .Single(predicate); } + [Fact] + public async Task Module_SourceGen_CustomParameterExpressionBuilder_IsInjected_NotAnArgument() + { + var assembly = CompileCustomParameterReproAssembly(); + + var result = await ExecuteWithSourceGeneratorRegistrationAsync( + assembly, + registrationMethodName: "AddDemo", + query: "mutation { doThing(name: \"!\") }", + configureBuilder: static b => + b.AddParameterExpressionBuilder(_ => new InjectedUser { Name = "injected" })); + + // the custom parameter is not exposed as a GraphQL argument (only 'name' is) + Assert.Contains("doThing(name: String!): String!", result.Schema, StringComparison.Ordinal); + + // the custom parameter value is injected into the resolver + using var json = JsonDocument.Parse(result.Result); + var value = json.RootElement.GetProperty("data").GetProperty("doThing").GetString(); + Assert.Equal("injected!", value); + } + + [Fact] + public async Task Module_SourceGen_CustomPredicateBuilder_AssignableInterfaceParameter_FailsLoud() + { + var assembly = CompileInterfaceParameterReproAssembly(); + + var builder = new ServiceCollection().AddGraphQLServer(disableDefaultSecurity: true); + var addModule = FindRegistrationMethod( + assembly, + m => + { + var p = m.GetParameters(); + return m.Name.Equals("AddDemo", StringComparison.Ordinal) + && m.ReturnType == typeof(IRequestExecutorBuilder) + && p.Length == 1 + && p[0].ParameterType == typeof(IRequestExecutorBuilder); + }); + addModule.Invoke(null, [builder]); + + // a custom ParameterInfo predicate whose value type (ConcreteInjectedUser) is assignable to + // the resolver parameter type (IInjectedUser). The source generator cannot evaluate the + // predicate, so the build must fail with a clear error instead of misbinding the parameter. + builder.AddParameterExpressionBuilder( + _ => new ConcreteInjectedUser(), + p => p.Name == "user"); + + var exception = await Record.ExceptionAsync( + async () => await builder.BuildSchemaAsync( + cancellationToken: TestContext.Current.CancellationToken)); + + Assert.IsType(exception); + Assert.Contains( + "custom parameter expression builder", + exception.ToString(), + StringComparison.Ordinal); + } + + private static Assembly CompileInterfaceParameterReproAssembly() + { + const string source = """ + using HotChocolate; + using HotChocolate.Types; + + [assembly: Module("Demo")] + + namespace Repro; + + [QueryType] + public static partial class SourceGeneratedQuery + { + public static string Ping() => "pong"; + } + + [MutationType] + public static partial class SourceGeneratedMutation + { + public static string DoThing(IInjectedUser user, string name) + => user.Name + name; + } + """; + + return CompileReproAssembly( + source, + "SourceGeneratorInterfaceParameterRepro", + [MetadataReference.CreateFromFile(typeof(IInjectedUser).Assembly.Location)]); + } + + private static Assembly CompileCustomParameterReproAssembly() + { + const string source = """ + using HotChocolate; + using HotChocolate.Types; + + [assembly: Module("Demo")] + + namespace Repro; + + [QueryType] + public static partial class SourceGeneratedQuery + { + public static string Ping() => "pong"; + } + + [MutationType] + public static partial class SourceGeneratedMutation + { + public static string DoThing(InjectedUser user, string name) + => user.Name + name; + } + """; + + return CompileReproAssembly( + source, + "SourceGeneratorCustomParameterRepro", + [MetadataReference.CreateFromFile(typeof(InjectedUser).Assembly.Location)]); + } + private static Assembly CompileOffsetPagingReproAssembly() { const string source = """ @@ -508,7 +625,10 @@ public record Foo(string Description); return CompileReproAssembly(source, "SourceGeneratorAnyTypeEscapingRepro"); } - private static Assembly CompileReproAssembly(string source, string assemblyName) + private static Assembly CompileReproAssembly( + string source, + string assemblyName, + IEnumerable? extraReferences = null) { var parseOptions = CSharpParseOptions.Default; var syntaxTree = CSharpSyntaxTree.ParseText(source, parseOptions); @@ -547,9 +667,11 @@ private static Assembly CompileReproAssembly(string source, string assemblyName) MetadataReference.CreateFromFile(typeof(IFilterContext).Assembly.Location), MetadataReference.CreateFromFile(typeof(WebApplication).Assembly.Location), MetadataReference.CreateFromFile(typeof(IServiceCollection).Assembly.Location), - MetadataReference.CreateFromFile(typeof(Microsoft.AspNetCore.Authorization.AuthorizeAttribute).Assembly.Location), + MetadataReference.CreateFromFile( + typeof(Microsoft.AspNetCore.Authorization.AuthorizeAttribute).Assembly.Location), MetadataReference.CreateFromFile(typeof(Authorization.AuthorizeAttribute).Assembly.Location), - MetadataReference.CreateFromFile(typeof(UseOffsetPagingAttribute).Assembly.Location) + MetadataReference.CreateFromFile(typeof(UseOffsetPagingAttribute).Assembly.Location), + .. extraReferences ?? [] ]; var compilation = CSharpCompilation.Create( @@ -595,3 +717,24 @@ private static Assembly CompileReproAssembly(string source, string assemblyName) private sealed record ExecutionResult(string Schema, string Result); } + +// Shared with the source-generated repro assembly (must be a compile-time type so the test can register +// AddParameterExpressionBuilder). Supplied via a custom parameter expression builder, +// never a GraphQL argument. +public sealed class InjectedUser +{ + public string Name { get; set; } = string.Empty; +} + +// Interface + implementation shared with the source-generated repro assembly. Used to verify that a +// custom ParameterInfo predicate whose value type is assignable to an interface/base parameter type is +// detected by the source-generator binding path. +public interface IInjectedUser +{ + string Name { get; } +} + +public sealed class ConcreteInjectedUser : IInjectedUser +{ + public string Name { get; set; } = string.Empty; +} diff --git a/src/HotChocolate/Core/test/Types.Tests/Resolvers/ParameterBindingResolverTests.cs b/src/HotChocolate/Core/test/Types.Tests/Resolvers/ParameterBindingResolverTests.cs new file mode 100644 index 00000000000..125b302b75e --- /dev/null +++ b/src/HotChocolate/Core/test/Types.Tests/Resolvers/ParameterBindingResolverTests.cs @@ -0,0 +1,81 @@ +using HotChocolate.Internal; +using Microsoft.Extensions.DependencyInjection; + +namespace HotChocolate.Resolvers; + +public class ParameterBindingResolverTests +{ + [Fact] + public void GetBindingInfo_DefaultCustomBuilder_ClassifiedAsCustom() + { + // arrange + var resolver = CreateResolver( + new CustomParameterExpressionBuilder(_ => new CustomState("hello"))); + var parameter = new ParameterDescriptor("state", typeof(CustomState), false, []); + + // act + var (kind, _) = resolver.GetBindingInfo(parameter); + + // assert + Assert.Equal(ArgumentKind.Custom, kind); + } + + [Fact] + public void GetBinding_DefaultCustomBuilder_ExecutesLambda() + { + // arrange + var resolver = CreateResolver( + new CustomParameterExpressionBuilder(_ => new CustomState("hello"))); + var parameter = new ParameterDescriptor("state", typeof(CustomState), false, []); + + // act + var value = resolver.GetBinding(parameter).Execute(null!); + + // assert + Assert.Equal("hello", value.Greetings); + } + + [Fact] + public void GetBindingInfo_CustomPredicateBuilder_MatchingType_Throws() + { + // arrange + var resolver = CreateResolver( + new CustomParameterExpressionBuilder( + _ => new CustomState("hello"), + canHandle: p => p.Name == "state")); + var parameter = new ParameterDescriptor("state", typeof(CustomState), false, []); + + // act + void Act() => resolver.GetBindingInfo(parameter); + + // assert + Assert.Throws(Act); + } + + [Fact] + public void GetBindingInfo_CustomPredicateBuilder_NonMatchingType_FallsBackToArgument() + { + // arrange + // a custom-predicate builder for CustomState must not affect an unrelated string parameter + var resolver = CreateResolver( + new CustomParameterExpressionBuilder( + _ => new CustomState("hello"), + canHandle: p => p.Name == "state")); + var parameter = new ParameterDescriptor("name", typeof(string), false, []); + + // act + var (kind, _) = resolver.GetBindingInfo(parameter); + + // assert + Assert.Equal(ArgumentKind.Argument, kind); + } + + private static ParameterBindingResolver CreateResolver( + params IParameterExpressionBuilder[] builders) + => new(new ServiceCollection().BuildServiceProvider(), builders); + + public class CustomState(string greetings) + { + public string Greetings { get; } = greetings; + } +} diff --git a/src/HotChocolate/Data/src/Data/PagingArgumentsParameterExpressionBuilder.cs b/src/HotChocolate/Data/src/Data/PagingArgumentsParameterExpressionBuilder.cs index a24b73911ec..023a8c55adf 100644 --- a/src/HotChocolate/Data/src/Data/PagingArgumentsParameterExpressionBuilder.cs +++ b/src/HotChocolate/Data/src/Data/PagingArgumentsParameterExpressionBuilder.cs @@ -13,10 +13,10 @@ internal sealed class PagingArgumentsParameterExpressionBuilder() { public bool IsDefaultHandler => false; - public bool CanHandle(ParameterDescriptor parameter) + public override bool CanHandle(ParameterDescriptor parameter) => parameter.Type == typeof(PagingArguments); - public IParameterBinding Create(ParameterDescriptor parameter) + public override IParameterBinding Create(ParameterDescriptor parameter) => this; public ArgumentKind Kind => ArgumentKind.Custom; diff --git a/src/HotChocolate/Data/src/EntityFramework/ContextFactoryParameterExpressionBuilder.cs b/src/HotChocolate/Data/src/EntityFramework/ContextFactoryParameterExpressionBuilder.cs index 51f530c31fa..98c64ec474a 100644 --- a/src/HotChocolate/Data/src/EntityFramework/ContextFactoryParameterExpressionBuilder.cs +++ b/src/HotChocolate/Data/src/EntityFramework/ContextFactoryParameterExpressionBuilder.cs @@ -17,10 +17,10 @@ internal sealed class ContextFactoryParameterExpressionBuilder() public bool IsDefaultHandler => false; - public bool CanHandle(ParameterDescriptor parameter) + public override bool CanHandle(ParameterDescriptor parameter) => parameter.Type == typeof(TDbContext); - public IParameterBinding Create(ParameterDescriptor parameter) => this; + public override IParameterBinding Create(ParameterDescriptor parameter) => this; public T Execute(IResolverContext context) {