Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServiceKind isn't correctly respected by resolvers added to field descriptors using the code-first approach #4945

Closed
1 task done
rpendleton opened this issue Apr 6, 2022 · 2 comments · Fixed by #5677
Closed
1 task done
Labels
🐛 bug Something isn't working 🌶️ hot chocolate 🔍 investigate Indicates that an issue or pull request needs more information. 📌 pinned
Milestone

Comments

@rpendleton
Copy link

rpendleton commented Apr 6, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When adding a resolver using IObjectFieldDescriptor.ResolveWith<T> that uses a service registered with a ServiceKind other than ServiceKind.Default, the resolver either runs with the incorrect execution strategy or completely fails to obtain the dependency.

Specifically:

  • ServiceKind.Default
    • correctly uses the default execution strategy
    • correctly uses a single request-scoped instance of Random
  • ServiceKind.Synchronized
    • incorrectly uses the default execution strategy when it should have used serial execution
    • correctly uses a single request-scoped instance of Random
  • ServiceKind.Resolver
    • unexpectedly throws "The specified key System.Random does not exist on context.ScopedContextData"
  • ServiceKind.Pooled
    • unexpectedly throws "The specified key System.Random does not exist on context.ScopedContextData"

If you take those same methods and place them directly in an object or in an object extension, then everything works as expected.

Steps to reproduce

I've created a gist that has the following code as well as all of the necessary boilerplate. You can find it here: Program.cs.

  1. Register Random in a service collection as a scoped service.

  2. Register an ObjectPool<Random> in a service collection as a scoped service. (The implementation can be naive and just return a new instance of Random each time.)

  3. Create a QueryType that relies on injecting Random using a code-first approach:

    public class QueryType : ObjectType
    {
        protected override void Configure(IObjectTypeDescriptor descriptor)
        {
            descriptor.Field("randomNumber")
                .Type<LongType>()
                .ResolveWith<Resolver>(r => r.GetRandomNumber(default!));
    
            descriptor.Field("executionStrategy")
                .Type<StringType>()
                .ResolveWith<Resolver>(r => r.GetExecutionStrategy(default!, default!));
        }
    }
    
    public class Resolver
    {
        public int GetRandomNumber(Random generator) => generator.Next();
    
        // The generator property isn't actually used, but it needs to be specified as a parameter
        // so that the execution strategy is actually changed for this resolver
        public string GetExecutionStrategy(IResolverContext context, Random generator)
        {
            // I'm not sure if there's a better way to determine the execution strategy, but hopefully
            // this is good enough for now since its only purpose is to demonstrate the issue
            return ((Selection)context.Selection).Strategy.ToString();
        }
    }
  4. Register the QueryType as the query type and Random as service using a any of the non-default ServiceKinds:

    builder.Services
        .AddQueryType<QueryType>()
        .RegisterService<Random>(ServiceKind.Pooled);
    
    var app = builder.Build();
    app.MapGraphQL();
    app.Run();
  5. Execute the following GraphQL query:

    query {
      randomNumber
      executionStrategy
    }

Relevant log output

No response

Additional Context?

I originally found this issue while trying to troubleshoot concurrency exceptions thrown by DbContext. Although I'm demonstrating the issue with just plain old services to keep things a bit simpler, the issue affects more than just RegisterService.

Specifically, it seems this issue affects any parameters that rely on IParameterFieldConfiguration.

It looks like this is happening because ObjectFieldDescription.CompleteArguments checks Parameters.Count > 0 before applying configurations, but because the field was created using descriptor.Field(...) rather than using a method, there aren't actually any parameters so the configuration is never applied.

This results in:

  • the execution strategy not being updated when needed
  • the middleware for locally scoped services not being added

I also ran into similar issues with node resolvers, but that doesn't seem to be strictly related to this issue. It would be nice if those were looked into as well, but I haven't researched them as thoroughly.

Product

Hot Chocolate

Version

12.7.0

@rpendleton rpendleton added the 🐛 bug Something isn't working label Apr 6, 2022
@rpendleton rpendleton changed the title ServiceKind isn't correctly respected by resolvers added to field descriptors using the using code-first approach ServiceKind isn't correctly respected by resolvers added to field descriptors using the code-first approach Apr 6, 2022
@michaelstaib michaelstaib added this to the HC-13.0.0 milestone Apr 6, 2022
@stale
Copy link

stale bot commented Jun 5, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the ⌛ stale Nothing happened with this issue in quite a while label Jun 5, 2022
@PascalSenn PascalSenn added 🔍 investigate Indicates that an issue or pull request needs more information. 📌 pinned and removed ⌛ stale Nothing happened with this issue in quite a while labels Jun 5, 2022
@michaelstaib
Copy link
Member

This is now fixed with Hot Chocolate 13 preview 96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🌶️ hot chocolate 🔍 investigate Indicates that an issue or pull request needs more information. 📌 pinned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants