Incorrect @cost when auto-injecting global state with AddParameterExpressionBuilder

[cmeeren]

Product

Hot Chocolate

Version

14.0.0-rc.1

Link to minimal reproduction

See zip below

Steps to reproduce

Repro solution: HotChocolateBugRepro.zip

Code for quick reference:

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddGraphQLServer().AddQueryType<Query>()
    .AddParameterExpressionBuilder(ctx => ctx.GetGlobalState<MyGlobalState>("MyGlobalState"));
var app = builder.Build();
app.MapGraphQL();
app.Run();

public record MyGlobalState();

public record MyThing(string Id);

public class Query
{
    public MyThing Test(MyGlobalState state) => new("Foo");
}

What is expected?

The following SDL:

schema {
  query: Query
}

type MyThing {
  id: String
}

type Query {
  test: MyThing
}

What is actually happening?

The following SDL:

schema {
  query: Query
}

type MyThing {
  id: String
}

type Query {
  test: MyThing @cost(weight: "10")
}

"The purpose of the `cost` directive is to define a `weight` for GraphQL types, fields, and arguments. Static analysis can use these weights when calculating the overall cost of a query or response."
directive @cost("The `weight` argument defines what value to add to the overall cost for every appearance, or possible appearance, of a type, field, argument, etc." weight: String!) on SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM | INPUT_FIELD_DEFINITION

Relevant log output

No response

Additional context

This only happens when using AddParameterExpressionBuilder as shown above. If using [GlobalState("MyGlobalSate")] before the parameter, the cost is correct.

[michaelstaib]

In this case its kind of correct. Since we cannot fully inspect what the impact is of custom expressions the field will be executed on the standard path which is async. This has not really something to do with cost. The cost interceptor just inspects if the resolver is pure or not. In this case its not.

You can work around that by either implementing a full parameter expression builder that sets the context to pure ... in this case it really is a pure. Or you could combine it with a configuration builder which sets a cost.

[michaelstaib]

We could however add another configuration method that allows mapping states ... like

builder.Services.AddGraphQLServer().AddQueryType<Query>().MapGlobalState<Fooo>(key);

which we know is pure and then weight it as pure.

[michaelstaib]

Thanks for putting these issues together ... this helps us alot!

[cmeeren]

Happy to help!

I haven't yet been able to look in detail at the implications of your replies, but the fact that the cost disappears just by adding GlobalStateAttribute seems to be an inconsistency regardless.

If more context helps, in our case the global state is the fully loaded principal (a custom User object) that is pre-loaded for all requests and used in most resolves (at least all root level ones) for authorization.

[cmeeren]

More context, in case I'm using the wrong tool: The only thing I'm trying to achieve with AddParameterExpressionBuilder is to inject the custom principal in any resolver I want with as little verbosity as possible. The method I use allows me to omit GlobalStateAttribute. I think I found it in the official docs somewhere. On mobile now, so can't easily find it, but probably related to docs on global state.

[cmeeren]

Hi again! Just wondering if you're able to have a look at this. Would be great to not have to manually specify the cost for half the resolvers.

Again, just adding GlobalStateAttribute makes the cost correct (i.e., no cost), so this is an inconsistency in HotChocolate.

[michaelstaib]

As outlined in my first answer ... this is correct and expected as you really have the impact to the execution engine as the pipeline becomes async. If you implement a custom IParameterExpressionBuilder and mark it as pure this issue should go away for you. We will at some point look at this... but also are working at the moment on the new base to move towards source generators. We are at a transition point and do not want to introduce to much things that need to be redone.

[cmeeren]

Thanks. You say "move toward source generators"; I assume you'll still support the current loosely typed APIs using Type, as well as the generic APIs like AddType<'T>? Source generators are C# only, so requiring their use would mean HC cannot be used at all from F#.

[michaelstaib]

@cmeeren yes, we discussed this internally. While going source generators only would make live soooo much easier :D, we are introducing an abstraction and will provide for F# still the expression compiler. This is while it takes a bit longer to get all the things into place. HotChocolate.Skimmed is a new schema building lib we created for fusion which we will use internally also yo build up the HotChocolate core schemas. We will remove any System.Type, MemberInfo and so on from the type system itself and have a cleaner schema again at its core. We have a new feature concept which allows to add typed metadata to the type. This way we can build both experiences without compromising. Doing this with minimal disruption to current users is what makes this whole process take longer.

But yeah we are aware and I can see that F# will have a hard stance with future libraries as source generators and AOT are taking up priorities for library developers. But we will keep it compatible with F#