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

Hot Chocolate 14 Migration #2348

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

michaelstaib
Copy link
Collaborator

@michaelstaib michaelstaib commented Aug 23, 2024

Why make this change?

With Hot Chocolate 14 we have invested a lot into security, performance and GraphQL protocol standards.

What is this change?

This PR will modernize the GraphQL stack of dab.

How was this tested?

  • Integration Tests
  • Unit Tests

@@ -4,6 +4,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Security.Claims;
using HotChocolate.AspNetCore.Authorization;
using HotChocolate.Authorization;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Hot Chocolate we have abstracted the authorization hooks to allow users to more easily bring in new. providers without the need to rely on ASP.NET core APIs

@@ -137,7 +137,7 @@ public async ValueTask ExecuteMutateAsync(IMiddlewareContext context)
/// <returns>
/// Returns the runtime field value.
/// </returns>
public static object? ExecuteLeafField(IPureResolverContext context)
public static object? ExecuteLeafField(IResolverContext context)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pure resolver context and the resolver context are now aligned.

@@ -13,7 +12,7 @@ namespace Azure.DataApiBuilder.Service.GraphQLBuilder.CustomScalars
/// </summary>
public class SingleType : FloatTypeBase<float>
{
public static readonly NameString TypeName = new("Single");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed NameString in Hot Chocolate 13 as this semantic string object just caused a lot of allocation.

ArgumentNode? arg = directiveNode?.Arguments.First(a => a.Name.Value == "target");

return (string?)arg?.Value.Value;
Directive? directive = (Directive?)infield.Directives.FirstOrDefault(DirectiveName);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The directive collection has now optimized lookup methods. which reduces the code needed to get things from type system members.

@@ -197,263 +196,260 @@ public static bool CreateAuthorizationDirectiveIfNecessary(
/// <returns>True when name resolution succeeded, false otherwise.</returns>
public static bool TryExtractGraphQLFieldModelName(IDirectiveCollection fieldDirectives, [NotNullWhen(true)] out string? modelName)
{
foreach (Directive dir in fieldDirectives)
foreach (Directive dir in fieldDirectives[ModelDirectiveType.DirectiveName])
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code here looked wrong ... lets chat about the intention here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant