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

Add APQ support #555

Merged
merged 18 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions GraphQL.Client.sln.DotSettings
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=APQ/@EntryIndexedValue">APQ</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=QL/@EntryIndexedValue">QL</s:String></wpf:ResourceDictionary>
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ The Library will try to follow the following standards and documents:

## Usage

The intended use of `GraphQLHttpClient` is to keep one instance alive per endpoint (obvious in case you're
operating full websocket, but also true for regular requests) and is built with thread-safety in mind.

### Create a GraphQLHttpClient

```csharp
Expand Down
4 changes: 0 additions & 4 deletions src/GraphQL.Client.Abstractions/GraphQLClientExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@ public static Task<GraphQLResponse<TResponse>> SendQueryAsync<TResponse>(this IG
cancellationToken: cancellationToken);
}

#if NET6_0_OR_GREATER
public static Task<GraphQLResponse<TResponse>> SendQueryAsync<TResponse>(this IGraphQLClient client,
GraphQLQuery query, object? variables = null,
string? operationName = null, Func<TResponse>? defineResponseType = null,
CancellationToken cancellationToken = default)
=> SendQueryAsync(client, query.Text, variables, operationName, defineResponseType,
cancellationToken);
#endif

public static Task<GraphQLResponse<TResponse>> SendMutationAsync<TResponse>(this IGraphQLClient client,
[StringSyntax("GraphQL")] string query, object? variables = null,
Expand All @@ -31,13 +29,11 @@ public static Task<GraphQLResponse<TResponse>> SendMutationAsync<TResponse>(this
cancellationToken: cancellationToken);
}

#if NET6_0_OR_GREATER
public static Task<GraphQLResponse<TResponse>> SendMutationAsync<TResponse>(this IGraphQLClient client,
GraphQLQuery query, object? variables = null, string? operationName = null, Func<TResponse>? defineResponseType = null,
CancellationToken cancellationToken = default)
=> SendMutationAsync(client, query.Text, variables, operationName, defineResponseType,
cancellationToken);
#endif

public static Task<GraphQLResponse<TResponse>> SendQueryAsync<TResponse>(this IGraphQLClient client,
GraphQLRequest request, Func<TResponse> defineResponseType, CancellationToken cancellationToken = default)
Expand Down
54 changes: 50 additions & 4 deletions src/GraphQL.Client/GraphQLHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public class GraphQLHttpClient : IGraphQLWebSocketClient, IDisposable
private readonly CancellationTokenSource _cancellationTokenSource = new();

private readonly bool _disposeHttpClient = false;

/// <summary>
/// the json serializer
/// </summary>
Expand All @@ -33,6 +32,12 @@ public class GraphQLHttpClient : IGraphQLWebSocketClient, IDisposable
/// </summary>
public GraphQLHttpClientOptions Options { get; }

/// <summary>
/// This flag is set to <see langword="true"/> when an error has occurred on an APQ and <see cref="GraphQLHttpClientOptions.DisableAPQ"/>
/// has returned <see langword="true"/>. To reset this, the instance of <see cref="GraphQLHttpClient"/> has to be disposed and a new one must be created.
/// </summary>
public bool APQDisabledForSession { get; private set; }

/// <inheritdoc />
public IObservable<Exception> WebSocketReceiveErrors => GraphQlHttpWebSocket.ReceiveErrors;

Expand Down Expand Up @@ -84,12 +89,49 @@ public GraphQLHttpClient(string endPoint, IGraphQLWebsocketJsonSerializer serial

#region IGraphQLClient

private const int APQ_SUPPORTED_VERSION = 1;

/// <inheritdoc />
public async Task<GraphQLResponse<TResponse>> SendQueryAsync<TResponse>(GraphQLRequest request, CancellationToken cancellationToken = default)
{
return Options.UseWebSocketForQueriesAndMutations || Options.WebSocketEndPoint is not null && Options.EndPoint is null || Options.EndPoint.HasWebSocketScheme()
? await GraphQlHttpWebSocket.SendRequestAsync<TResponse>(request, cancellationToken).ConfigureAwait(false)
: await SendHttpRequestAsync<TResponse>(request, cancellationToken).ConfigureAwait(false);
cancellationToken.ThrowIfCancellationRequested();

string? savedQuery = null;
bool useAPQ = false;

if (request.Query != null && !APQDisabledForSession && Options.EnableAutomaticPersistedQueries(request))
{
// https://www.apollographql.com/docs/react/api/link/persisted-queries/
useAPQ = true;
request.GeneratePersistedQueryExtension();
savedQuery = request.Query;
request.Query = null;
}

var response = await SendQueryInternalAsync<TResponse>(request, cancellationToken);

if (useAPQ)
{
if (response.Errors?.Any(error => string.Equals(error.Message, "PersistedQueryNotFound", StringComparison.CurrentCultureIgnoreCase)) == true)
{
// GraphQL server supports APQ!

// Alas, for the first time we did not guess and in vain removed Query, so we return Query and
// send request again. This is one-time "cache miss", not so scary.
request.Query = savedQuery;
return await SendQueryInternalAsync<TResponse>(request, cancellationToken);
}
else
{
// GraphQL server either supports APQ of some other version, or does not support it at all.
// Send a request for the second time. This is better than returning an error. Let the client work with APQ disabled.
APQDisabledForSession = Options.DisableAPQ(response);
request.Query = savedQuery;
return await SendQueryInternalAsync<TResponse>(request, cancellationToken);
}
}

return response;
}

/// <inheritdoc />
Expand Down Expand Up @@ -123,6 +165,10 @@ public IObservable<GraphQLResponse<TResponse>> CreateSubscriptionStream<TRespons
public Task SendPongAsync(object? payload) => GraphQlHttpWebSocket.SendPongAsync(payload);

#region Private Methods
private async Task<GraphQLResponse<TResponse>> SendQueryInternalAsync<TResponse>(GraphQLRequest request, CancellationToken cancellationToken = default) =>
Options.UseWebSocketForQueriesAndMutations || Options.WebSocketEndPoint is not null && Options.EndPoint is null || Options.EndPoint.HasWebSocketScheme()
? await GraphQlHttpWebSocket.SendRequestAsync<TResponse>(request, cancellationToken).ConfigureAwait(false)
: await SendHttpRequestAsync<TResponse>(request, cancellationToken).ConfigureAwait(false);

private async Task<GraphQLHttpResponse<TResponse>> SendHttpRequestAsync<TResponse>(GraphQLRequest request, CancellationToken cancellationToken = default)
{
Expand Down
19 changes: 18 additions & 1 deletion src/GraphQL.Client/GraphQLHttpClientOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class GraphQLHttpClientOptions
public Uri? WebSocketEndPoint { get; set; }

/// <summary>
/// The GraphQL websocket protocol to be used. Defaults to the older "graphql-ws" protocol to not break old code.
/// The GraphQL websocket protocol to be used. Defaults to the older "graphql-ws" protocol to not break old code.
/// </summary>
public string? WebSocketProtocol { get; set; } = WebSocketProtocols.AUTO_NEGOTIATE;

Expand Down Expand Up @@ -99,4 +99,21 @@ public static bool DefaultIsValidResponseToDeserialize(HttpResponseMessage r)
/// </summary>
public ProductInfoHeaderValue? DefaultUserAgentRequestHeader { get; set; }
= new ProductInfoHeaderValue(typeof(GraphQLHttpClient).Assembly.GetName().Name, typeof(GraphQLHttpClient).Assembly.GetName().Version.ToString());

/// <summary>
/// Delegate permitting use of <see href="https://www.apollographql.com/docs/react/api/link/persisted-queries/">Automatic Persisted Queries (APQ)</see>.
/// By default, returns <see langword="false" /> for all requests. Note that GraphQL server should support APQ. Otherwise, the client disables APQ completely
/// after an unsuccessful attempt to send an APQ request and then send only regular requests.
/// </summary>
public Func<GraphQLRequest, bool> EnableAutomaticPersistedQueries { get; set; } = _ => false;

/// <summary>
/// A delegate which takes an <see cref="IGraphQLResponse"/> and returns a boolean to disable any future persisted queries for that session.
/// This defaults to disabling on PersistedQueryNotSupported or a 400 or 500 HTTP error.
/// </summary>
public Func<IGraphQLResponse, bool> DisableAPQ { get; set; } = response =>
{
return response.Errors?.Any(error => string.Equals(error.Message, "PersistedQueryNotSupported", StringComparison.CurrentCultureIgnoreCase)) == true
|| response is IGraphQLHttpResponse httpResponse && (int)httpResponse.StatusCode >= 400 && (int)httpResponse.StatusCode < 600;
};
}
3 changes: 0 additions & 3 deletions src/GraphQL.Client/GraphQLHttpRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ public GraphQLHttpRequest([StringSyntax("GraphQL")] string query, object? variab
: base(query, variables, operationName, extensions)
{
}

#if NET6_0_OR_GREATER
public GraphQLHttpRequest(GraphQLQuery query, object? variables = null, string? operationName = null, Dictionary<string, object?>? extensions = null)
: base(query, variables, operationName, extensions)
{
}
#endif

public GraphQLHttpRequest(GraphQLRequest other)
: base(other)
Expand Down
9 changes: 8 additions & 1 deletion src/GraphQL.Client/GraphQLHttpResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

namespace GraphQL.Client.Http;

public class GraphQLHttpResponse<T> : GraphQLResponse<T>
public class GraphQLHttpResponse<T> : GraphQLResponse<T>, IGraphQLHttpResponse
{
public GraphQLHttpResponse(GraphQLResponse<T> response, HttpResponseHeaders responseHeaders, HttpStatusCode statusCode)
{
Expand All @@ -19,6 +19,13 @@ public GraphQLHttpResponse(GraphQLResponse<T> response, HttpResponseHeaders resp
public HttpStatusCode StatusCode { get; set; }
}

public interface IGraphQLHttpResponse : IGraphQLResponse
Copy link
Member Author

Choose a reason for hiding this comment

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

This interface is needed to pass non-generic instance of response into GraphQLHttpClientOptions.DisableAPQ.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should put the content in there, too... then people can evaluate plaintext error messages...

Always having the raw content in there might be beneficial for general debugging, too, cause then people could cast their responses to IGraphQLHttpResponse and get the raw body of the response...

Copy link
Member Author

Choose a reason for hiding this comment

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

Then I would put entire HttpResponseMessage instead of pulling discrete properties from it. Also note that content may be already unaccessable since stream was read in SendHttpRequestAsync.

Copy link
Member Author

Choose a reason for hiding this comment

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

#559 , not a goal for this PR

{
HttpResponseHeaders ResponseHeaders { get; set; }

HttpStatusCode StatusCode { get; set; }
}

public static class GraphQLResponseExtensions
{
public static GraphQLHttpResponse<T> ToGraphQLHttpResponse<T>(this GraphQLResponse<T> response, HttpResponseHeaders responseHeaders, HttpStatusCode statusCode) => new(response, responseHeaders, statusCode);
Expand Down
3 changes: 3 additions & 0 deletions src/GraphQL.Primitives/GraphQL.Primitives.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,7 @@
<TargetFrameworks>netstandard2.0;net6.0;net7.0;net8.0</TargetFrameworks>
</PropertyGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0'">
<PackageReference Include="System.Memory" Version="4.5.5" />
rose-a marked this conversation as resolved.
Show resolved Hide resolved
</ItemGroup>
</Project>
33 changes: 26 additions & 7 deletions src/GraphQL.Primitives/GraphQLQuery.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,34 @@
#if NET6_0_OR_GREATER
using System.Diagnostics.CodeAnalysis;

namespace GraphQL;

/// <summary>
/// Value record for a GraphQL query string
/// Value object representing a GraphQL query string and storing the corresponding APQ hash. <br />
/// Use this to hold query strings you want to use more than once.
/// </summary>
/// <param name="Text">the actual query string</param>
public readonly record struct GraphQLQuery([StringSyntax("GraphQL")] string Text)
public class GraphQLQuery : IEquatable<GraphQLQuery>
rose-a marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// The actual query string
/// </summary>
public string Text { get; }

/// <summary>
/// The SHA256 hash used for the automatic persisted queries feature (APQ)
/// </summary>
public string Sha256Hash { get; }

public GraphQLQuery([StringSyntax("GraphQL")] string text)
{
Text = text;
Sha256Hash = Hash.Compute(Text);
}

public static implicit operator string(GraphQLQuery query)
=> query.Text;
};
#endif

public bool Equals(GraphQLQuery other) => Sha256Hash == other.Sha256Hash;

public override bool Equals(object? obj) => obj is GraphQLQuery other && Equals(other);

public override int GetHashCode() => Sha256Hash.GetHashCode();
}
34 changes: 28 additions & 6 deletions src/GraphQL.Primitives/GraphQLRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,29 @@ public class GraphQLRequest : Dictionary<string, object>, IEquatable<GraphQLRequ
public const string QUERY_KEY = "query";
public const string VARIABLES_KEY = "variables";
public const string EXTENSIONS_KEY = "extensions";
public const string EXTENSIONS_PERSISTED_QUERY_KEY = "persistedQuery";
public const int APQ_SUPPORTED_VERSION = 1;

private string? _sha265Hash;

/// <summary>
/// The Query
/// The query string
/// </summary>
[StringSyntax("GraphQL")]
public string Query
public string? Query
{
get => TryGetValue(QUERY_KEY, out object value) ? (string)value : null;
set => this[QUERY_KEY] = value;
set
{
this[QUERY_KEY] = value;
// if the query string gets overwritten, reset the hash value
if (_sha265Hash is not null)
_sha265Hash = null;
rose-a marked this conversation as resolved.
Show resolved Hide resolved
}
}

/// <summary>
/// The name of the Operation
/// The operation to execute
/// </summary>
public string? OperationName
{
Expand Down Expand Up @@ -59,16 +69,28 @@ public GraphQLRequest([StringSyntax("GraphQL")] string query, object? variables
Extensions = extensions;
}

#if NET6_0_OR_GREATER
public GraphQLRequest(GraphQLQuery query, object? variables = null, string? operationName = null,
Dictionary<string, object?>? extensions = null)
: this(query.Text, variables, operationName, extensions)
{
_sha265Hash = query.Sha256Hash;
}
#endif

public GraphQLRequest(GraphQLRequest other) : base(other) { }

public void GeneratePersistedQueryExtension()
{
if (Query is null)
throw new InvalidOperationException($"{nameof(Query)} is null");

Extensions ??= new();
Extensions[EXTENSIONS_PERSISTED_QUERY_KEY] = new Dictionary<string, object>
{
["version"] = APQ_SUPPORTED_VERSION,
["sha256Hash"] = _sha265Hash ?? Hash.Compute(Query),
rose-a marked this conversation as resolved.
Show resolved Hide resolved
};
}

/// <summary>
/// Returns a value that indicates whether this instance is equal to a specified object
/// </summary>
Expand Down
44 changes: 44 additions & 0 deletions src/GraphQL.Primitives/Hash.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System.Buffers;
using System.Diagnostics;
using System.Security.Cryptography;
using System.Text;

namespace GraphQL;

internal static class Hash
{
private static SHA256? _sha256;

internal static string Compute(string query)
{
int expected = Encoding.UTF8.GetByteCount(query);
byte[]? inputBytes = ArrayPool<byte>.Shared.Rent(expected);
int written = Encoding.UTF8.GetBytes(query, 0, query.Length, inputBytes, 0);
Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

This could be rewritten to use Span / stackalloc also.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you please elaborate? 😉

Copy link
Member

Choose a reason for hiding this comment

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

I saw that stackalloc was used on lines 21-27. Stackalloc could be used for the call to GetBytes as well. This is faster than ArrayPool.

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/stackalloc

The easy recommended pattern is to write code like this:

int length = 1000;
Span<byte> buffer = length <= 1024 ? stackalloc byte[length] : new byte[length];

This assumes that typically the buffer length is relatively small, and in such cases, is ideal. It also includes a fallback for large buffer requirements; but then you'd probably lose the ArrayPool (?). And if it was known that the query would commonly be larger than you'd want to allocate on the stack, then maybe the cost of the heap allocation for the long queries might be worse than the benefits from stackalloc. Or you'd have to write a separate if block for the ArrayPool path vs not. I'm not really sure; maybe better off leaving it as-is.

Another possible optimization would be to guess at the length, rather than using GetByteCount. You'd have to write a fallback routine in case the encoded string length was longer than the preallocated buffer though, and you might need to use the underlying Encoder rather than the static method.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I'll leave it as it is. Using Span<byte> would require separate code for old frameworks again, and the best optimization is using the GraphQLQuery class to compute the hash only once... but anyway, thanks for the explanation!

Debug.Assert(written == expected, (string)$"Encoding.UTF8.GetBytes returned unexpected bytes: {written} instead of {expected}");

var shaShared = Interlocked.Exchange(ref _sha256, null) ?? SHA256.Create();

#if NET5_0_OR_GREATER
Span<byte> bytes = stackalloc byte[32];
if (!shaShared.TryComputeHash(inputBytes.AsSpan().Slice(0, written), bytes, out int bytesWritten)) // bytesWritten ignored since it is always 32
throw new InvalidOperationException("Too small buffer for hash");
#else
byte[] bytes = shaShared.ComputeHash(inputBytes, 0, written);
#endif

ArrayPool<byte>.Shared.Return(inputBytes);
Interlocked.CompareExchange(ref _sha256, shaShared, null);

#if NET5_0_OR_GREATER
return Convert.ToHexString(bytes);
#else
var builder = new StringBuilder(bytes.Length * 2);
foreach (byte item in bytes)
{
builder.Append(item.ToString("x2"));
}

return builder.ToString();
#endif
}
}
Loading