From 2b54ffc047cf487e4468f087a6b4f44c0abb6de7 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Wed, 21 Sep 2022 13:16:55 +0200 Subject: [PATCH] Fixed the mutation result handling in the resolver pipeline --- .../Core/src/Abstractions/IMutationResult.cs | 12 +- .../Core/src/Abstractions/MutationResult.cs | 171 +++++++++++++----- .../src/Abstractions/WellKnownMiddleware.cs | 5 + .../MutationConventionMiddleware.cs | 17 +- .../MutationConventionTypeInterceptor.cs | 34 ++++ 5 files changed, 180 insertions(+), 59 deletions(-) diff --git a/src/HotChocolate/Core/src/Abstractions/IMutationResult.cs b/src/HotChocolate/Core/src/Abstractions/IMutationResult.cs index 564295c0a22..efd3356c481 100644 --- a/src/HotChocolate/Core/src/Abstractions/IMutationResult.cs +++ b/src/HotChocolate/Core/src/Abstractions/IMutationResult.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace HotChocolate; /// @@ -8,10 +10,18 @@ public interface IMutationResult /// /// Gets the mutation result value. /// - object Value { get; } + object? Value { get; } /// /// Defines if the mutation was successful and if the result represents a success result. /// bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// +#if NET5_0_OR_GREATER + [MemberNotNullWhen(true, nameof(Value))] +#endif + bool IsError { get; } } diff --git a/src/HotChocolate/Core/src/Abstractions/MutationResult.cs b/src/HotChocolate/Core/src/Abstractions/MutationResult.cs index 87b137d6ef2..0a3f2725348 100644 --- a/src/HotChocolate/Core/src/Abstractions/MutationResult.cs +++ b/src/HotChocolate/Core/src/Abstractions/MutationResult.cs @@ -23,11 +23,12 @@ namespace HotChocolate; /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -49,6 +50,7 @@ public MutationResult(object error) Result = default; Errors = new[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -89,6 +91,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = temp; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -109,6 +112,8 @@ public MutationResult(params object[] errors) Result = default; Errors = errors ?? throw new ArgumentNullException(nameof(errors)); IsSuccess = false; + IsError = !IsSuccess; + IsError = !IsSuccess; if (errors.Length == 0) { @@ -139,16 +144,20 @@ public MutationResult(params object[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// @@ -196,11 +205,12 @@ public MutationResult(params object[] errors) /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -222,6 +232,7 @@ public MutationResult([DisallowNull] TError error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -264,6 +275,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -308,6 +320,7 @@ public MutationResult(params TError[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -325,16 +338,20 @@ public MutationResult(params TError[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// @@ -409,11 +426,12 @@ public MutationResult(params TError[] errors) /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -435,6 +453,7 @@ public MutationResult([DisallowNull] TError1 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -477,6 +496,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -521,6 +541,7 @@ public MutationResult(params TError1[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -542,6 +563,7 @@ public MutationResult([DisallowNull] TError2 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -584,6 +606,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -628,6 +651,7 @@ public MutationResult(params TError2[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -668,6 +692,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = temp; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -688,6 +713,7 @@ public MutationResult(params object[] errors) Result = default; Errors = errors ?? throw new ArgumentNullException(nameof(errors)); IsSuccess = false; + IsError = !IsSuccess; if (errors.Length == 0) { @@ -718,16 +744,20 @@ public MutationResult(params object[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// @@ -832,11 +862,12 @@ public MutationResult(params object[] errors) /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -858,6 +889,7 @@ public MutationResult([DisallowNull] TError1 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -900,6 +932,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -944,6 +977,7 @@ public MutationResult(params TError1[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -965,6 +999,7 @@ public MutationResult([DisallowNull] TError2 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1007,6 +1042,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1051,6 +1087,7 @@ public MutationResult(params TError2[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1072,6 +1109,7 @@ public MutationResult([DisallowNull] TError3 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1114,6 +1152,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1158,6 +1197,7 @@ public MutationResult(params TError3[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1198,6 +1238,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = temp; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1218,6 +1259,7 @@ public MutationResult(params object[] errors) Result = default; Errors = errors ?? throw new ArgumentNullException(nameof(errors)); IsSuccess = false; + IsError = !IsSuccess; if (errors.Length == 0) { @@ -1248,16 +1290,20 @@ public MutationResult(params object[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// @@ -1390,11 +1436,12 @@ public MutationResult(params object[] errors) /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -1416,6 +1463,7 @@ public MutationResult([DisallowNull] TError1 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1458,6 +1506,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1502,6 +1551,7 @@ public MutationResult(params TError1[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1523,6 +1573,7 @@ public MutationResult([DisallowNull] TError2 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1565,6 +1616,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1609,6 +1661,7 @@ public MutationResult(params TError2[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1630,6 +1683,7 @@ public MutationResult([DisallowNull] TError3 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1672,6 +1726,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1716,6 +1771,7 @@ public MutationResult(params TError3[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1737,6 +1793,7 @@ public MutationResult([DisallowNull] TError4 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1779,6 +1836,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1823,6 +1881,7 @@ public MutationResult(params TError4[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1863,6 +1922,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = temp; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -1883,6 +1943,7 @@ public MutationResult(params object[] errors) Result = default; Errors = errors ?? throw new ArgumentNullException(nameof(errors)); IsSuccess = false; + IsError = !IsSuccess; if (errors.Length == 0) { @@ -1913,16 +1974,20 @@ public MutationResult(params object[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// @@ -2083,11 +2148,12 @@ public MutationResult(params object[] errors) /// /// is null. /// - public MutationResult([DisallowNull] TResult result) + public MutationResult(TResult result) { - Result = result ?? throw new ArgumentNullException(nameof(result)); + Result = result; Errors = null; IsSuccess = true; + IsError = !IsSuccess; } /// @@ -2109,6 +2175,7 @@ public MutationResult([DisallowNull] TError1 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2151,6 +2218,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2195,6 +2263,7 @@ public MutationResult(params TError1[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2216,6 +2285,7 @@ public MutationResult([DisallowNull] TError2 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2258,6 +2328,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2302,6 +2373,7 @@ public MutationResult(params TError2[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2323,6 +2395,7 @@ public MutationResult([DisallowNull] TError3 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2365,6 +2438,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2409,6 +2483,7 @@ public MutationResult(params TError3[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2430,6 +2505,7 @@ public MutationResult([DisallowNull] TError4 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2472,6 +2548,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2516,6 +2593,7 @@ public MutationResult(params TError4[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2537,6 +2615,7 @@ public MutationResult([DisallowNull] TError5 error) Result = default; Errors = new object[] { error }; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2579,6 +2658,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = list; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2623,6 +2703,7 @@ public MutationResult(params TError5[] errors) Result = default; Errors = array; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2663,6 +2744,7 @@ public MutationResult(IEnumerable errors) Result = default; Errors = temp; IsSuccess = false; + IsError = !IsSuccess; } /// @@ -2683,6 +2765,7 @@ public MutationResult(params object[] errors) Result = default; Errors = errors ?? throw new ArgumentNullException(nameof(errors)); IsSuccess = false; + IsError = !IsSuccess; if (errors.Length == 0) { @@ -2713,16 +2796,20 @@ public MutationResult(params object[] errors) /// /// Defines if this mutation result represents a success result. /// + public bool IsSuccess { get; } + + /// + /// Defines if the mutation had an error and if the result represents a error result. + /// #if NET5_0_OR_GREATER - [MemberNotNullWhen(true, nameof(Result))] - [MemberNotNullWhen(false, nameof(Errors))] + [MemberNotNullWhen(true, nameof(Errors))] #endif - public bool IsSuccess { get; } + public bool IsError { get; } #if NET5_0_OR_GREATER - object IMutationResult.Value => IsSuccess ? Result : Errors; + object? IMutationResult.Value => IsSuccess ? Result : Errors; #else - object IMutationResult.Value => IsSuccess ? Result! : Errors!; + object? IMutationResult.Value => IsSuccess ? Result : Errors!; #endif /// diff --git a/src/HotChocolate/Core/src/Abstractions/WellKnownMiddleware.cs b/src/HotChocolate/Core/src/Abstractions/WellKnownMiddleware.cs index 13824ab1861..8a2821ff1a3 100644 --- a/src/HotChocolate/Core/src/Abstractions/WellKnownMiddleware.cs +++ b/src/HotChocolate/Core/src/Abstractions/WellKnownMiddleware.cs @@ -74,4 +74,9 @@ public static class WellKnownMiddleware /// This key identifies the mutation convention middleware. /// public const string MutationErrors = "HotChocolate.Types.Mutations.Errors"; + + /// + ///The key identifies the mutation result middleware. + /// + public const string MutationResult = "HotChocolate.Types.Mutations.Result"; } diff --git a/src/HotChocolate/Core/src/Types.Mutations/MutationConventionMiddleware.cs b/src/HotChocolate/Core/src/Types.Mutations/MutationConventionMiddleware.cs index 93716780897..d09b9489872 100644 --- a/src/HotChocolate/Core/src/Types.Mutations/MutationConventionMiddleware.cs +++ b/src/HotChocolate/Core/src/Types.Mutations/MutationConventionMiddleware.cs @@ -62,22 +62,7 @@ public async ValueTask InvokeAsync(IMiddlewareContext context) try { - await _next(context); - - if (context.Result is IMutationResult result) - { - if (result.IsSuccess) - { - context.Result = result.Value; - } - else - { - context.SetScopedState(ErrorContextDataKeys.Errors, result.Value); - context.Result = MarkerObjects.ErrorObject; - } - } - - context.Result ??= MarkerObjects.Null; + await _next(context).ConfigureAwait(false); } finally { diff --git a/src/HotChocolate/Core/src/Types.Mutations/MutationConventionTypeInterceptor.cs b/src/HotChocolate/Core/src/Types.Mutations/MutationConventionTypeInterceptor.cs index f843f535357..87027e30951 100644 --- a/src/HotChocolate/Core/src/Types.Mutations/MutationConventionTypeInterceptor.cs +++ b/src/HotChocolate/Core/src/Types.Mutations/MutationConventionTypeInterceptor.cs @@ -112,6 +112,7 @@ public override void OnAfterMergeTypeExtensions() { // if the mutation options indicate that we shall apply the mutation // conventions we will start rewriting the field. + ApplyResultMiddleware(mutationField); TryApplyInputConvention(mutationField, mutationOptions); TryApplyPayloadConvention(mutationField, cd?.PayloadFieldName, mutationOptions); } @@ -124,6 +125,38 @@ public override void OnAfterMergeTypeExtensions() } } + private void ApplyResultMiddleware(ObjectFieldDefinition mutation) + { + var middlewareDef = new FieldMiddlewareDefinition( + next => async context => + { + await next(context).ConfigureAwait(false); + + if (context.Result is IMutationResult result) + { + // by checking if it is not an error we can accept the default + // value of the struct as null. + if (!result.IsError) + { + context.Result = result.Value; + } + else + { + context.SetScopedState(Errors, result.Value); + context.Result = MarkerObjects.ErrorObject; + } + } + + // we will replace null with our null marker object so + // that + context.Result ??= MarkerObjects.Null; + }, + isRepeatable: false, + key: MutationResult); + + mutation.MiddlewareDefinitions.Insert(0, middlewareDef); + } + private void TryApplyInputConvention(ObjectFieldDefinition mutation, Options options) { if (mutation.Arguments.Count is 0) @@ -437,6 +470,7 @@ private static Type[] GetErrorResultTypes(ObjectFieldDefinition mutation) if (resultType?.IsGenericType ?? false) { var typeDefinition = resultType.GetGenericTypeDefinition(); + if (typeDefinition == typeof(Task<>) || typeDefinition == typeof(ValueTask<>)) { resultType = resultType.GenericTypeArguments[0];